| Message ID | 1500351730-26391-1-git-send-email-john.stultz@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi John, On 18-07-2017 05:22, John Stultz wrote: > Currently the hikey dsi logic cannot generate accurate byte > clocks values for all pixel clock values. Thus if a mode clock > is selected that cannot match the calculated byte clock, the > device will boot with a blank screen. > > This patch uses the new mode_valid callback (many thanks to > Jose Abreu for upstreaming it!) to ensure we don't select > modes we cannot generate. > > NOTE: Stylistically I suspect there are better ways to do what > I'm trying to do here. The encoder -> crtc bit is terrible, and > getting the crtc adjusted mode from the encoder logic feels > less then ideal. So feedback would be greatly appreciated! > > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: David Airlie <airlied@linux.ie> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Xinliang Liu <xinliang.liu@linaro.org> > Cc: Xinliang Liu <z.liuxinliang@hisilicon.com> > Cc: Rongrong Zou <zourongrong@gmail.com> > Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com> > Cc: Chen Feng <puck.chen@hisilicon.com> > Cc: Jose Abreu <Jose.Abreu@synopsys.com> > Cc: Archit Taneja <architt@codeaurora.org> > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > v2: Reworked to calculate if modeclock matches the phy's byteclock, > rather then using a whitelist of known modes. Something like Daniel suggested would be simpler maybe: encoder_mode_valid_aux() { ... } dsi_encoder_mode_valid(...) { drm_for_each_crtc(crtc, dev) { crtc->mode_fixup(crtc, mode, adjusted_mode); ret = encoder_mode_valid_aux(adjusted_mode); if (ret != MODE_OK) return ret; } } BTW, I think at commit stage you have encoder->crtc populated, not sure though. But at probbing stage it will not be bound. And also if you have more than one crtc this may be wrong. (How will you know which crtc will be bound to the encoder so that you can get the right clock?). Best regards, Jose Miguel Abreu
diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c index f77dcfa..9a553e7 100644 --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c @@ -24,6 +24,7 @@ #include <drm/drm_encoder_slave.h> #include <drm/drm_atomic_helper.h> +#include "kirin_drm_drv.h" #include "dw_dsi_reg.h" #define MAX_TX_ESC_CLK 10 @@ -603,6 +604,49 @@ static void dsi_encoder_enable(struct drm_encoder *encoder) dsi->enable = true; } +static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder *encoder, + const struct drm_display_mode *mode) +{ + struct dw_dsi *dsi = encoder_to_dsi(encoder); + struct drm_crtc *crtc = NULL; + struct mipi_phy_params phy; + u32 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); + u32 adjusted_clock; + u32 req_kHz, act_kHz, lane_byte_clk_kHz; + + /* Figure out what the adjusted modeclock will be */ + /* XXX There's got to be a better way to go encoder->crtc */ + drm_for_each_crtc(crtc, encoder->dev) + if (crtc) + break; + if (crtc) + adjusted_clock = kirin_ade_adj_mode_clk(crtc, mode->clock); + else + adjusted_clock = mode->clock; + + /* Calculate the lane byte clk using the adjusted mode clk */ + memset(&phy, 0, sizeof(phy)); + req_kHz = adjusted_clock * bpp / dsi->lanes; + act_kHz = dsi_calc_phy_rate(req_kHz, &phy); + lane_byte_clk_kHz = act_kHz / 8; + + DRM_DEBUG_DRIVER("Checking mode %ix%i-%i@%i clock: %i adj_clock: %i...", + mode->hdisplay, mode->vdisplay, bpp, + drm_mode_vrefresh(mode), mode->clock, adjusted_clock); + + /* + * Make sure the adjused mode clock and the lane byte clk + * have a common denominator base frequency + */ + if (adjusted_clock/dsi->lanes == lane_byte_clk_kHz/3) { + DRM_DEBUG_DRIVER("OK!\n"); + return MODE_OK; + } + + DRM_DEBUG_DRIVER("BAD!\n"); + return MODE_BAD; +} + static void dsi_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adj_mode) @@ -622,6 +666,7 @@ static int dsi_encoder_atomic_check(struct drm_encoder *encoder, static const struct drm_encoder_helper_funcs dw_encoder_helper_funcs = { .atomic_check = dsi_encoder_atomic_check, + .mode_valid = dsi_encoder_mode_valid, .mode_set = dsi_encoder_mode_set, .enable = dsi_encoder_enable, .disable = dsi_encoder_disable diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c index 074b0af..dffcf76 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c @@ -178,6 +178,14 @@ static void ade_init(struct ade_hw_ctx *ctx) FRM_END_START_MASK, REG_EFFECTIVE_IN_ADEEN_FRMEND); } +u32 kirin_ade_adj_mode_clk(struct drm_crtc *crtc, u32 modeclk) +{ + struct ade_crtc *acrtc = to_ade_crtc(crtc); + struct ade_hw_ctx *ctx = acrtc->ctx; + + return clk_round_rate(ctx->ade_pix_clk, modeclk * 1000) / 1000; +} + static void ade_set_pix_clk(struct ade_hw_ctx *ctx, struct drm_display_mode *mode, struct drm_display_mode *adj_mode) diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h index 7f60c649..85f69ee 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h @@ -27,4 +27,6 @@ struct kirin_drm_private { extern const struct kirin_dc_ops ade_dc_ops; +u32 kirin_ade_adj_mode_clk(struct drm_crtc *crtc, u32 modeclk); + #endif /* __KIRIN_DRM_DRV_H__ */
Currently the hikey dsi logic cannot generate accurate byte clocks values for all pixel clock values. Thus if a mode clock is selected that cannot match the calculated byte clock, the device will boot with a blank screen. This patch uses the new mode_valid callback (many thanks to Jose Abreu for upstreaming it!) to ensure we don't select modes we cannot generate. NOTE: Stylistically I suspect there are better ways to do what I'm trying to do here. The encoder -> crtc bit is terrible, and getting the crtc adjusted mode from the encoder logic feels less then ideal. So feedback would be greatly appreciated! Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: David Airlie <airlied@linux.ie> Cc: Rob Clark <robdclark@gmail.com> Cc: Xinliang Liu <xinliang.liu@linaro.org> Cc: Xinliang Liu <z.liuxinliang@hisilicon.com> Cc: Rongrong Zou <zourongrong@gmail.com> Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com> Cc: Chen Feng <puck.chen@hisilicon.com> Cc: Jose Abreu <Jose.Abreu@synopsys.com> Cc: Archit Taneja <architt@codeaurora.org> Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- v2: Reworked to calculate if modeclock matches the phy's byteclock, rather then using a whitelist of known modes. --- drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 45 +++++++++++++++++++++++++ drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 8 +++++ drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 2 ++ 3 files changed, 55 insertions(+) -- 2.7.4