[Intel-gfx] [PATCH 1/4] drm/i915/tgl/dsi: Program TRANS_VBLANK register

[Kulkarni, Vandita]

> -----Original Message-----
> From: Shankar, Uma
> Sent: Tuesday, July 16, 2019 3:29 PM
> To: Kulkarni, Vandita <vandita.kulkarni at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: ville.syrjala at linux.intel.com; Nikula, Jani <jani.nikula at intel.com>
> Subject: RE: [PATCH 1/4] drm/i915/tgl/dsi: Program TRANS_VBLANK register
> 
> 
> 
> >-----Original Message-----
> >From: Kulkarni, Vandita
> >Sent: Tuesday, July 2, 2019 9:49 AM
> >To: intel-gfx at lists.freedesktop.org
> >Cc: ville.syrjala at linux.intel.com; Nikula, Jani
> ><jani.nikula at intel.com>; Shankar, Uma <uma.shankar at intel.com>;
> >Kulkarni, Vandita <vandita.kulkarni at intel.com>
> >Subject: [PATCH 1/4] drm/i915/tgl/dsi: Program TRANS_VBLANK register
> >
> >Program vblank register for mipi dsi in video mode on TGL.
> >
> >Signed-off-by: Vandita Kulkarni <vandita.kulkarni at intel.com>
> >---
> > drivers/gpu/drm/i915/display/icl_dsi.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> >b/drivers/gpu/drm/i915/display/icl_dsi.c
> >index b8673debf932..556eba2636fe 100644
> >--- a/drivers/gpu/drm/i915/display/icl_dsi.c
> >+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> >@@ -866,6 +866,15 @@ gen11_dsi_set_transcoder_timings(struct
> >intel_encoder *encoder,
> > 		dsi_trans = dsi_port_to_transcoder(port);
> > 		I915_WRITE(VSYNCSHIFT(dsi_trans), vsync_shift);
> > 	}
> >+
> >+	/* program TRANS_VBLANK register, should be same as vtotal
> progammed
> >+*/
> 
> Typo here in programmed.
Thanks for the review.
Will fix.
> 
> >+	if (INTEL_GEN(dev_priv) >= 12) {
> >+		for_each_dsi_port(port, intel_dsi->ports) {
> >+			dsi_trans = dsi_port_to_transcoder(port);
> >+			I915_WRITE(VBLANK(dsi_trans),
> >+				   (vactive - 1) | ((vtotal - 1) << 16));
> 
> We can put this line along with VTOTAL and get rid of this extra for loop.
But looks like the rest of the code is written in the similar fashion. It gives better readability as it has platform check too.
Also it is called during modeset, it is not very clear to me if it is significant enough.

-Thanks
Vandita
> 
> >+		}
> >+	}
> > }
> >
> > static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
> >--
> >2.21.0.5.gaeb582a