drm/i915/icl: Fix pipe config mismatch warnings

Commit Message

Lisovskiy, Stanislav Dec. 7, 2018, 8:59 a.m. UTC

Fixes hblank, vblank, vsync_start/vsync_end,
hsync_start//hsync_end, pipe_bpp, port clock,
pixel rate mismatches for dsi which
happen during pipe_config comparation in
intel_atomic_check.

Signed-off-by: Stanislav Lisovskiy <[e-mail]>
---
 drivers/gpu/drm/i915/icl_dsi.c | 93 +++++++++++++++++++++++++++++-----
 1 file changed, 81 insertions(+), 12 deletions(-)

Comments

kernel test robot Dec. 7, 2018, 4:24 p.m. UTC | #1

Hi Stanislav,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20181207]
[cannot apply to v4.20-rc5]
[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/Stanislav-Lisovskiy/drm-i915-icl-Fix-pipe-config-mismatch-warnings/20181207-234855
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x073-201848 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/icl_dsi.c: In function 'gen11_dsi_calc_transcoder_timings':
>> drivers/gpu/drm/i915/icl_dsi.c:843:6: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
         &pipe_config->base.adjusted_mode;
         ^
   drivers/gpu/drm/i915/icl_dsi.c:845:18: warning: unused variable 'dsi_trans' [-Wunused-variable]
     enum transcoder dsi_trans;
                     ^~~~~~~~~
   drivers/gpu/drm/i915/icl_dsi.c:844:12: warning: unused variable 'port' [-Wunused-variable]
     enum port port;
               ^~~~
   drivers/gpu/drm/i915/icl_dsi.c:840:27: warning: unused variable 'dev_priv' [-Wunused-variable]
     struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
                              ^~~~~~~~

vim +/const +843 drivers/gpu/drm/i915/icl_dsi.c

   834	
   835	static
   836	void
   837	gen11_dsi_calc_transcoder_timings(struct intel_encoder *encoder,
   838					 const struct intel_crtc_state *pipe_config)
   839	{
   840		struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
   841		struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
   842		struct drm_display_mode *adjusted_mode =
 > 843						&pipe_config->base.adjusted_mode;
   844		enum port port;
   845		enum transcoder dsi_trans;
   846		/* horizontal timings */
   847		u16 htotal, hactive, hsync_start, hsync_end, hsync_size;
   848		u16 hfront_porch, hback_porch;
   849		/* vertical timings */
   850		u16 vtotal, vactive, vsync_start, vsync_end, vsync_shift;
   851	
   852		hactive = adjusted_mode->crtc_hdisplay;
   853		htotal = adjusted_mode->crtc_htotal;
   854		hsync_start = adjusted_mode->crtc_hsync_start;
   855		hsync_end = adjusted_mode->crtc_hsync_end;
   856		hsync_size  = hsync_end - hsync_start;
   857		hfront_porch = (adjusted_mode->crtc_hsync_start -
   858				adjusted_mode->crtc_hdisplay);
   859		hback_porch = (adjusted_mode->crtc_htotal -
   860			       adjusted_mode->crtc_hsync_end);
   861		vactive = adjusted_mode->crtc_vdisplay;
   862		vtotal = adjusted_mode->crtc_vtotal;
   863		vsync_start = adjusted_mode->crtc_vsync_start;
   864		vsync_end = adjusted_mode->crtc_vsync_end;
   865		vsync_shift = hsync_start - htotal / 2;
   866	
   867		if (intel_dsi->dual_link) {
   868			hactive /= 2;
   869			if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
   870				hactive += intel_dsi->pixel_overlap;
   871			htotal /= 2;
   872		}
   873	
   874		/* TRANS_HSYNC register to be programmed only for video mode */
   875		if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
   876	
   877			if (intel_dsi->dual_link) {
   878				hsync_start /= 2;
   879				hsync_end /= 2;
   880			}
   881		}
   882	
   883		adjusted_mode->crtc_hdisplay = hactive;
   884		adjusted_mode->crtc_htotal = htotal;
   885		adjusted_mode->crtc_hsync_start = hsync_start;
   886		adjusted_mode->crtc_hsync_end = hsync_end;
   887		adjusted_mode->crtc_vdisplay = vactive;
   888		adjusted_mode->crtc_vtotal = vtotal;
   889		adjusted_mode->crtc_vsync_start = vsync_start;
   890		adjusted_mode->crtc_vsync_end = vsync_end;
   891	}
   892	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 4dd793b78996..0fb6b140bc4b 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -760,13 +760,6 @@  gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 	vsync_end = adjusted_mode->crtc_vsync_end;
 	vsync_shift = hsync_start - htotal / 2;
 
-	if (intel_dsi->dual_link) {
-		hactive /= 2;
-		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
-			hactive += intel_dsi->pixel_overlap;
-		htotal /= 2;
-	}
-
 	/* minimum hactive as per bspec: 256 pixels */
 	if (adjusted_mode->crtc_hdisplay < 256)
 		DRM_ERROR("hactive is less then 256 pixels\n");
@@ -794,11 +787,6 @@  gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 		if (hback_porch < 16)
 			DRM_ERROR("hback porch < 16 pixels\n");
 
-		if (intel_dsi->dual_link) {
-			hsync_start /= 2;
-			hsync_end /= 2;
-		}
-
 		for_each_dsi_port(port, intel_dsi->ports) {
 			dsi_trans = dsi_port_to_transcoder(port);
 			I915_WRITE(HSYNC(dsi_trans),
@@ -844,6 +832,64 @@  gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 	}
 }
 
+static
+void
+gen11_dsi_calc_transcoder_timings(struct intel_encoder *encoder,
+				 const 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;
+	enum port port;
+	enum transcoder dsi_trans;
+	/* horizontal timings */
+	u16 htotal, hactive, hsync_start, hsync_end, hsync_size;
+	u16 hfront_porch, hback_porch;
+	/* vertical timings */
+	u16 vtotal, vactive, vsync_start, vsync_end, vsync_shift;
+
+	hactive = adjusted_mode->crtc_hdisplay;
+	htotal = adjusted_mode->crtc_htotal;
+	hsync_start = adjusted_mode->crtc_hsync_start;
+	hsync_end = adjusted_mode->crtc_hsync_end;
+	hsync_size  = hsync_end - hsync_start;
+	hfront_porch = (adjusted_mode->crtc_hsync_start -
+			adjusted_mode->crtc_hdisplay);
+	hback_porch = (adjusted_mode->crtc_htotal -
+		       adjusted_mode->crtc_hsync_end);
+	vactive = adjusted_mode->crtc_vdisplay;
+	vtotal = adjusted_mode->crtc_vtotal;
+	vsync_start = adjusted_mode->crtc_vsync_start;
+	vsync_end = adjusted_mode->crtc_vsync_end;
+	vsync_shift = hsync_start - htotal / 2;
+
+	if (intel_dsi->dual_link) {
+		hactive /= 2;
+		if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK)
+			hactive += intel_dsi->pixel_overlap;
+		htotal /= 2;
+	}
+
+	/* TRANS_HSYNC register to be programmed only for video mode */
+	if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
+
+		if (intel_dsi->dual_link) {
+			hsync_start /= 2;
+			hsync_end /= 2;
+		}
+	}
+
+	adjusted_mode->crtc_hdisplay = hactive;
+	adjusted_mode->crtc_htotal = htotal;
+	adjusted_mode->crtc_hsync_start = hsync_start;
+	adjusted_mode->crtc_hsync_end = hsync_end;
+	adjusted_mode->crtc_vdisplay = vactive;
+	adjusted_mode->crtc_vtotal = vtotal;
+	adjusted_mode->crtc_vsync_start = vsync_start;
+	adjusted_mode->crtc_vsync_end = vsync_end;
+}
+
 static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -1169,6 +1215,9 @@  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);
+	struct drm_display_mode *adjusted_mode =
+					&pipe_config->base.adjusted_mode;
+	u32 bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
 	u32 pll_id;
 
 	/* FIXME: adapt icl_ddi_clock_get() for DSI and use that? */
@@ -1176,6 +1225,19 @@  static void gen11_dsi_get_config(struct intel_encoder *encoder,
 	pipe_config->port_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
 	pipe_config->base.adjusted_mode.crtc_clock = intel_dsi->pclk;
 	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
+	pipe_config->pipe_bpp = bpp;
+
+	if (intel_dsi->dual_link) {
+		adjusted_mode->crtc_hblank_start = adjusted_mode->crtc_hdisplay * 2;
+		adjusted_mode->crtc_hblank_end = adjusted_mode->crtc_htotal * 2;
+		adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay;
+		adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
+	} else {
+		adjusted_mode->crtc_hblank_start = adjusted_mode->crtc_hdisplay;
+		adjusted_mode->crtc_hblank_end = adjusted_mode->crtc_htotal;
+		adjusted_mode->crtc_vblank_start = adjusted_mode->crtc_vdisplay;
+		adjusted_mode->crtc_vblank_end = adjusted_mode->crtc_vtotal;
+	}
 }
 
 static bool gen11_dsi_compute_config(struct intel_encoder *encoder,
@@ -1204,6 +1266,13 @@  static bool gen11_dsi_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->clock_set = true;
 	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi) / 5;
+	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
+
+	if (intel_dsi->dual_link) {
+		pipe_config->base.adjusted_mode.crtc_clock /= 2;
+	}
+
+	gen11_dsi_calc_transcoder_timings(encoder, pipe_config);
 
 	return true;
 }