Re: [PATCH 1/3] drm/i915: Fix pipe config timing mismatch warnings

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 05 Apr 2019, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote:
>> -----Original Message-----
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Sent: Friday, April 5, 2019 2:00 AM
>> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>
>> Subject: Re: [Intel-gfx] [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@intel.com>
>> > ---
>> >  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx