[PATCH 1/2] drm/i915: Check dual_link instead of port==A|C for DSI

[PATCH 1/2] drm/i915: Check dual_link instead of port==A|C for DSI

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Everywhere else we check for intel_dsi->dual_link instead of the
port mask. So do the same when setting up the lane configuration
in MIPI_PORT_CTRL.

Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 781c267..57768c0 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -304,7 +304,7 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
 		temp &= ~LANE_CONFIGURATION_MASK;
 		temp &= ~DUAL_LINK_MODE_MASK;
 
-		if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
+		if (intel_dsi->dual_link) {
 			temp |= (intel_dsi->dual_link - 1)
 						<< DUAL_LINK_MODE_SHIFT;
 			temp |= intel_crtc->pipe ?
-- 
2.4.6

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

[RFC][PATCH 2/2] drm/i915: Don't write shared DSI port bits multiple time

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Some DSI bits are present only in one port A register, and not present
in a port C register. Currently we write such bits multiple times
while looping over the ports. This doesn't seem entirely sane as the
effect of the bit has already occurred by the time we've written it for
port A, so any port C register we're supposed to write before frobbing
the shared bit are now done after the frobbing.

I'm not ure if this is a real problem or not, but it's suspicious at the
very least. So change the code so that we write the appropriate registers
in both ports first, then frob the shared bit, and the continue with the
rest of the registers on both ports.

So this patch is totally untested, and potentially utter garbage,
hence sending out as RFC only.

Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 55 +++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 57768c0..0e8dcee 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -352,18 +352,19 @@ static void intel_dsi_device_ready(struct intel_encoder *encoder)
 	band_gap_reset(dev_priv);
 
 	for_each_dsi_port(port, intel_dsi->ports) {
-
 		I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_ENTER);
 		usleep_range(2500, 3000);
+	}
 
-		/* Enable MIPI PHY transparent latch
-		 * Common bit for both MIPI Port A & MIPI Port C
-		 * No similar bit in MIPI Port C reg
-		 */
-		val = I915_READ(MIPI_PORT_CTRL(PORT_A));
-		I915_WRITE(MIPI_PORT_CTRL(PORT_A), val | LP_OUTPUT_HOLD);
-		usleep_range(1000, 1500);
+	/* Enable MIPI PHY transparent latch
+	 * Common bit for both MIPI Port A & MIPI Port C
+	 * No similar bit in MIPI Port C reg
+	 */
+	val = I915_READ(MIPI_PORT_CTRL(PORT_A));
+	I915_WRITE(MIPI_PORT_CTRL(PORT_A), val | LP_OUTPUT_HOLD);
+	usleep_range(1000, 1500);
 
+	for_each_dsi_port(port, intel_dsi->ports) {
 		I915_WRITE(MIPI_DEVICE_READY(port), ULPS_STATE_EXIT);
 		usleep_range(2500, 3000);
 
@@ -538,21 +539,23 @@ static void intel_dsi_clear_device_ready(struct intel_encoder *encoder)
 		I915_WRITE(MIPI_DEVICE_READY(port), DEVICE_READY |
 							ULPS_STATE_ENTER);
 		usleep_range(2000, 2500);
+	}
 
-		/* Wait till Clock lanes are in LP-00 state for MIPI Port A
-		 * only. MIPI Port C has no similar bit for checking
-		 */
-		if (wait_for(((I915_READ(MIPI_PORT_CTRL(PORT_A)) & AFE_LATCHOUT)
-							== 0x00000), 30))
-			DRM_ERROR("DSI LP not going Low\n");
+	/* Wait till Clock lanes are in LP-00 state for MIPI Port A
+	 * only. MIPI Port C has no similar bit for checking
+	 */
+	if (wait_for(((I915_READ(MIPI_PORT_CTRL(PORT_A)) & AFE_LATCHOUT)
+		      == 0x00000), 30))
+		DRM_ERROR("DSI LP not going Low\n");
 
-		/* Disable MIPI PHY transparent latch
-		 * Common bit for both MIPI Port A & MIPI Port C
-		 */
-		val = I915_READ(MIPI_PORT_CTRL(PORT_A));
-		I915_WRITE(MIPI_PORT_CTRL(PORT_A), val & ~LP_OUTPUT_HOLD);
-		usleep_range(1000, 1500);
+	/* Disable MIPI PHY transparent latch
+	 * Common bit for both MIPI Port A & MIPI Port C
+	 */
+	val = I915_READ(MIPI_PORT_CTRL(PORT_A));
+	I915_WRITE(MIPI_PORT_CTRL(PORT_A), val & ~LP_OUTPUT_HOLD);
+	usleep_range(1000, 1500);
 
+	for_each_dsi_port(port, intel_dsi->ports) {
 		I915_WRITE(MIPI_DEVICE_READY(port), 0x00);
 		usleep_range(2000, 2500);
 	}
@@ -776,13 +779,13 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 			mode_hdisplay += intel_dsi->pixel_overlap;
 	}
 
-	for_each_dsi_port(port, intel_dsi->ports) {
-		/* escape clock divider, 20MHz, shared for A and C.
-		 * device ready must be off when doing this! txclkesc? */
-		tmp = I915_READ(MIPI_CTRL(PORT_A));
-		tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
-		I915_WRITE(MIPI_CTRL(PORT_A), tmp | ESCAPE_CLOCK_DIVIDER_1);
+	/* escape clock divider, 20MHz, shared for A and C.
+	 * device ready must be off when doing this! txclkesc? */
+	tmp = I915_READ(MIPI_CTRL(PORT_A));
+	tmp &= ~ESCAPE_CLOCK_DIVIDER_MASK;
+	I915_WRITE(MIPI_CTRL(PORT_A), tmp | ESCAPE_CLOCK_DIVIDER_1);
 
+	for_each_dsi_port(port, intel_dsi->ports) {
 		/* read request priority is per pipe */
 		tmp = I915_READ(MIPI_CTRL(port));
 		tmp &= ~READ_REQUEST_PRIORITY_MASK;
-- 
2.4.6

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

Re: [PATCH 1/2] drm/i915: Check dual_link instead of port==A|C for DSI

[Jani Nikula]

On Thu, 10 Sep 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Everywhere else we check for intel_dsi->dual_link instead of the
> port mask. So do the same when setting up the lane configuration
> in MIPI_PORT_CTRL.
>
> Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 781c267..57768c0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -304,7 +304,7 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
>  		temp &= ~LANE_CONFIGURATION_MASK;
>  		temp &= ~DUAL_LINK_MODE_MASK;
>  
> -		if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
> +		if (intel_dsi->dual_link) {

I think having the whole intel_dsi->dual_link field is an unnecessary
extra level of indirection. If there's more than one port, it's dual
link. There could be a helper to get that info from ->ports.

That said, being in line for the time being is more important than doing
the above.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>  			temp |= (intel_dsi->dual_link - 1)
>  						<< DUAL_LINK_MODE_SHIFT;
>  			temp |= intel_crtc->pipe ?
> -- 
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 1/2] drm/i915: Check dual_link instead of port==A|C for DSI

[Ville Syrjälä]

On Thu, Sep 10, 2015 at 04:35:12PM +0300, Jani Nikula wrote:
> On Thu, 10 Sep 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Everywhere else we check for intel_dsi->dual_link instead of the
> > port mask. So do the same when setting up the lane configuration
> > in MIPI_PORT_CTRL.
> >
> > Cc: Gaurav K Singh <gaurav.k.singh@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 781c267..57768c0 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -304,7 +304,7 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
> >  		temp &= ~LANE_CONFIGURATION_MASK;
> >  		temp &= ~DUAL_LINK_MODE_MASK;
> >  
> > -		if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
> > +		if (intel_dsi->dual_link) {
> 
> I think having the whole intel_dsi->dual_link field is an unnecessary
> extra level of indirection. If there's more than one port, it's dual
> link. There could be a helper to get that info from ->ports.

I considered it, but then I realized that intel_dsi->dual_link actually
houses the dual link mode (of which there are two), so we still need it.

> 
> That said, being in line for the time being is more important than doing
> the above.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> >  			temp |= (intel_dsi->dual_link - 1)
> >  						<< DUAL_LINK_MODE_SHIFT;
> >  			temp |= intel_crtc->pipe ?
> > -- 
> > 2.4.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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