diff mbox series

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

Message ID 1554365187-30575-2-git-send-email-vandita.kulkarni@intel.com (mailing list archive)
State New, archived
Headers show
Series Fix mipi dsi pipe_config mismatch for icl | expand

Commit Message

Kulkarni, Vandita April 4, 2019, 8:06 a.m. UTC 
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(-)

Comments

Ville Syrjala April 4, 2019, 8:29 p.m. UTC | #1 
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.
Kulkarni, Vandita April 5, 2019, 8:25 a.m. UTC | #2 
> -----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.

Thanks,
Vandita
> 
> --
> Ville Syrjälä
> Intel
Jani Nikula April 17, 2019, 11:16 a.m. UTC | #3 
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
diff mbox series

Patch

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;
+		}
+	}
+	adjusted_mode->crtc_hsync_start = hsync_start;
+	adjusted_mode->crtc_hsync_end = hsync_end;
+
+	tmp = I915_READ(VTOTAL(cpu_transcoder));
+	adjusted_mode->crtc_vdisplay = (tmp & 0xffff) + 1;
+	adjusted_mode->crtc_vtotal = ((tmp >> 16) & 0xffff) + 1;
+	adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay;
+	adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
+
+	tmp = I915_READ(VSYNC(cpu_transcoder));
+	adjusted_mode->crtc_vsync_start = (tmp & 0xffff) + 1;
+	adjusted_mode->crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
+}
+
 static void gen11_dsi_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config)
 {
@@ -1186,6 +1236,7 @@  static void gen11_dsi_get_config(struct intel_encoder *encoder,
 	pipe_config->port_clock =
 		cnl_calc_wrpll_link(dev_priv, &pipe_config->dpll_hw_state);
 	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
+	gen11_dsi_get_timings(encoder, pipe_config);
 	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7ecfb7d..9f7e4f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9965,8 +9965,9 @@  static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	if (!transcoder_is_dsi(pipe_config->cpu_transcoder) ||
 	    INTEL_GEN(dev_priv) >= 11) {
 		haswell_get_ddi_port_state(crtc, pipe_config);
-		intel_get_pipe_timings(crtc, pipe_config);
 	}
+	if (!transcoder_is_dsi(pipe_config->cpu_transcoder))
+		intel_get_pipe_timings(crtc, pipe_config);
 
 	intel_get_pipe_src_size(crtc, pipe_config);
 	intel_get_crtc_ycbcr_config(crtc, pipe_config);