[PATCH 0/3] Fix mipi dsi pipe_config mismatch for icl

[PATCH 0/3] Fix mipi dsi pipe_config mismatch for icl

[Vandita Kulkarni]

This is series fixes the WARN_ON that we see due to
pipe_config mismatch on mipi dsi for icl.
Only DSI0 trancoder regs are read even in case of dual link mode
as the values programmed for DSI0 and DSI1 transcoder registers
are same.

Vandita Kulkarni (3):
  drm/i915: Fix pipe config timing mismatch warnings
  drm/i915: Fix pipe config mismatch for bpp, output format
  drm/i915: Fix pixel clock and crtc clock config mismatch

 drivers/gpu/drm/i915/icl_dsi.c       | 88 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c |  3 +-
 2 files changed, 89 insertions(+), 2 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Vandita Kulkarni]

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;
+		}
+	}
+	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);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Fix pipe config mismatch for bpp, output format

[Vandita Kulkarni]

Read back the pixel fomrat register and get the bpp.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/icl_dsi.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index db6bc3d..69cd6b2 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -1226,6 +1226,30 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
 	adjusted_mode->crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
 }
 
+enum mipi_dsi_pixel_format
+gen11_dsi_get_pixel_fmt(struct drm_i915_private *dev_priv,
+			struct intel_crtc_state *pipe_config)
+{
+	u32 tmp;
+	/* get config for dsi0 transcoder only */
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+
+	tmp = I915_READ(DSI_TRANS_FUNC_CONF(cpu_transcoder));
+	tmp &= PIX_FMT_MASK;
+
+	switch (tmp) {
+	default:
+	case PIX_FMT_RGB565:
+		return MIPI_DSI_FMT_RGB565;
+	case PIX_FMT_RGB666_PACKED:
+		return MIPI_DSI_FMT_RGB666_PACKED;
+	case PIX_FMT_RGB666_LOOSE:
+		return MIPI_DSI_FMT_RGB666;
+	case PIX_FMT_RGB888:
+		return MIPI_DSI_FMT_RGB888;
+	}
+}
+
 static void gen11_dsi_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config)
 {
@@ -1238,6 +1262,9 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
 	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);
+	pipe_config->pipe_bpp = mipi_dsi_pixel_format_to_bpp
+					(gen11_dsi_get_pixel_fmt(dev_priv,
+								  pipe_config));
 }
 
 static int gen11_dsi_compute_config(struct intel_encoder *encoder,
@@ -1253,6 +1280,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
 	struct drm_display_mode *adjusted_mode =
 					&pipe_config->base.adjusted_mode;
 
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 	intel_pch_panel_fitting(crtc, pipe_config, conn_state->scaling_mode);
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Fix pixel clock and crtc clock config mismatch

[Vandita Kulkarni]

In case of dual link mode, the mode clock that we get
from the VBT is halved.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/icl_dsi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 69cd6b2..c77960f 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -1255,11 +1255,18 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	int crtc_clock;
 
 	/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
 	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;
+
+	if (intel_dsi->dual_link)
+		crtc_clock = intel_dsi->pclk * 2;
+	else
+		crtc_clock = intel_dsi->pclk;
+
+	pipe_config->base.adjusted_mode.crtc_clock = crtc_clock;
 	gen11_dsi_get_timings(encoder, pipe_config);
 	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
 	pipe_config->pipe_bpp = mipi_dsi_pixel_format_to_bpp
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Fix pipe config mismatch for bpp, output format

[kbuild test robot]

Hi Vandita,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v5.1-rc3 next-20190404]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vandita-Kulkarni/Fix-mipi-dsi-pipe_config-mismatch-for-icl/20190404-192608
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'


sparse warnings: (new ones prefixed by >>)

   include/uapi/linux/perf_event.h:147:56: sparse: cast truncates bits from constant value (8000000000000000 becomes 0)
   drivers/gpu/drm/i915/icl_dsi.c:129:33: sparse: expression using sizeof(void)
>> drivers/gpu/drm/i915/icl_dsi.c:1230:1: sparse: symbol 'gen11_dsi_get_pixel_fmt' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH] drm/i915: gen11_dsi_get_pixel_fmt can be static

[kbuild test robot]

Fixes: bab7d9431d53 ("drm/i915: Fix pipe config mismatch for bpp, output format")
Signed-off-by: kbuild test robot <lkp@intel.com>
---
 icl_dsi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 69cd6b2..163dc54 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -1226,7 +1226,7 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
 	adjusted_mode->crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
 }
 
-enum mipi_dsi_pixel_format
+static enum mipi_dsi_pixel_format
 gen11_dsi_get_pixel_fmt(struct drm_i915_private *dev_priv,
 			struct intel_crtc_state *pipe_config)
 {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Fix mipi dsi pipe_config mismatch for icl (rev2)

[Patchwork]

== Series Details ==

Series: Fix mipi dsi pipe_config mismatch for icl (rev2)
URL   : https://patchwork.freedesktop.org/series/58990/
State : failure

== Summary ==

Applying: drm/i915: Fix pipe config timing mismatch warnings
Applying: drm/i915: gen11_dsi_get_pixel_fmt can be static
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/icl_dsi.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0002 drm/i915: gen11_dsi_get_pixel_fmt can be static
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Kulkarni, Vandita]

> -----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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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

Re: [PATCH 2/3] drm/i915: Fix pipe config mismatch for bpp, output format

[Jani Nikula]

On Thu, 04 Apr 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> Read back the pixel fomrat register and get the bpp.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index db6bc3d..69cd6b2 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -1226,6 +1226,30 @@ static void gen11_dsi_get_timings(struct intel_encoder *encoder,
>  	adjusted_mode->crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
>  }
>  
> +enum mipi_dsi_pixel_format
> +gen11_dsi_get_pixel_fmt(struct drm_i915_private *dev_priv,
> +			struct intel_crtc_state *pipe_config)
> +{
> +	u32 tmp;
> +	/* get config for dsi0 transcoder only */
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +
> +	tmp = I915_READ(DSI_TRANS_FUNC_CONF(cpu_transcoder));
> +	tmp &= PIX_FMT_MASK;
> +
> +	switch (tmp) {
> +	default:
> +	case PIX_FMT_RGB565:
> +		return MIPI_DSI_FMT_RGB565;
> +	case PIX_FMT_RGB666_PACKED:
> +		return MIPI_DSI_FMT_RGB666_PACKED;
> +	case PIX_FMT_RGB666_LOOSE:
> +		return MIPI_DSI_FMT_RGB666;
> +	case PIX_FMT_RGB888:
> +		return MIPI_DSI_FMT_RGB888;
> +	}
> +}
> +
>  static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *pipe_config)
>  {
> @@ -1238,6 +1262,9 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  	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);
> +	pipe_config->pipe_bpp = mipi_dsi_pixel_format_to_bpp
> +					(gen11_dsi_get_pixel_fmt(dev_priv,
> +								  pipe_config));

Please see how this is done for VLV DSI. It now uses PIPEMISC for
this. I'd like the two to be the same.

BR,
Jani.


>  }
>  
>  static int gen11_dsi_compute_config(struct intel_encoder *encoder,
> @@ -1253,6 +1280,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  	struct drm_display_mode *adjusted_mode =
>  					&pipe_config->base.adjusted_mode;
>  
> +	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	intel_fixed_panel_mode(fixed_mode, adjusted_mode);
>  	intel_pch_panel_fitting(crtc, pipe_config, conn_state->scaling_mode);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Fix pixel clock and crtc clock config mismatch

[Jani Nikula]

On Thu, 04 Apr 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> In case of dual link mode, the mode clock that we get
> from the VBT is halved.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index 69cd6b2..c77960f 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -1255,11 +1255,18 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	int crtc_clock;
>  
>  	/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
>  	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;
> +
> +	if (intel_dsi->dual_link)
> +		crtc_clock = intel_dsi->pclk * 2;
> +	else
> +		crtc_clock = intel_dsi->pclk;
> +
> +	pipe_config->base.adjusted_mode.crtc_clock = crtc_clock;

I'd do this with simply:

	if (intel_dsi->dual_link)
		pipe_config->base.adjusted_mode.crtc_clock *= 2;

BR,
Jani.


>  	gen11_dsi_get_timings(encoder, pipe_config);
>  	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
>  	pipe_config->pipe_bpp = mipi_dsi_pixel_format_to_bpp

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx