On Fri, 05 Apr 2019, "Kulkarni, Vandita" <vandita.kulkarni@xxxxxxxxx> wrote: >> -----Original Message----- >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Sent: Friday, April 5, 2019 2:00 AM >> To: Kulkarni, Vandita <vandita.kulkarni@xxxxxxxxx> >> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nikula, Jani <jani.nikula@xxxxxxxxx> >> Subject: Re: [PATCH 1/3] drm/i915: Fix pipe config timing mismatch >> warnings >> >> On Thu, Apr 04, 2019 at 01:36:25PM +0530, Vandita Kulkarni wrote: >> > Mipi dsi programs the transcoder timings as part of encoder enable >> > sequence, with dual link or single link in consideration. Hence add >> > get transcoder timings as part of the encoder's get_config function. >> > >> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx> >> > --- >> > drivers/gpu/drm/i915/icl_dsi.c | 51 >> ++++++++++++++++++++++++++++++++++++ >> > drivers/gpu/drm/i915/intel_display.c | 3 ++- >> > 2 files changed, 53 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/icl_dsi.c >> > b/drivers/gpu/drm/i915/icl_dsi.c index b67ffaa..db6bc3d 100644 >> > --- a/drivers/gpu/drm/i915/icl_dsi.c >> > +++ b/drivers/gpu/drm/i915/icl_dsi.c >> > @@ -1176,6 +1176,56 @@ static void gen11_dsi_disable(struct intel_encoder >> *encoder, >> > gen11_dsi_disable_io_power(encoder); >> > } >> > >> > +static void gen11_dsi_get_timings(struct intel_encoder *encoder, >> > + struct intel_crtc_state *pipe_config) { >> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> > + struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> > + struct drm_display_mode *adjusted_mode = >> > + &pipe_config->base.adjusted_mode; >> > + /* get config for dsi0 transcoder only */ >> > + enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; >> > + /* horizontal timings */ >> > + u16 htotal, hactive, hsync_start, hsync_end; >> > + u32 tmp; >> > + >> > + tmp = I915_READ(HTOTAL(cpu_transcoder)); >> > + hactive = (tmp & 0xffff) + 1; >> > + htotal = ((tmp >> 16) & 0xffff) + 1; >> > + if (intel_dsi->dual_link) { >> > + hactive *= 2; >> > + if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) >> > + hactive -= intel_dsi->pixel_overlap; >> > + htotal *= 2; >> > + } >> > + adjusted_mode->crtc_hdisplay = hactive; >> > + adjusted_mode->crtc_htotal = htotal; >> > + adjusted_mode->crtc_hblank_start = adjusted_mode->crtc_hdisplay; >> > + adjusted_mode->crtc_hblank_end = adjusted_mode->crtc_htotal; >> > + >> > + tmp = I915_READ(HSYNC(cpu_transcoder)); >> > + hsync_start = (tmp & 0xffff) + 1; >> > + hsync_end = ((tmp >> 16) & 0xffff) + 1; >> > + if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) { >> > + if (intel_dsi->dual_link) { >> > + hsync_start *= 2; >> > + hsync_end *= 2; >> > + } >> > + } >> >> This looks like a hand rolled intel_get_pipe_timings() with an extra twist. I would >> suggest trying to reuse intel_get_pipe_timings() and just adjusting what it gave >> you a bit. > In that case we will have set in encoder specific function and get in > display get_pipe_config function, adjustments done in encoder > function. If that is ok, I will make this change in v2. For ICL DSI intel_get_pipe_timings() has already been called in intel_display.c. I.e. you'll only need to do the DSI specific adjustments in the encoder get config hook. BR, Jani. > > Thanks, > Vandita >> >> -- >> Ville Syrjälä >> Intel -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx