diff mbox series

[v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout

Message ID 20181022141953.3889-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout | expand

Commit Message

Ville Syrjälä Oct. 22, 2018, 2:19 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's make sure the DSI port is actually on before we go
poking at the plane register to determine which way
it's rotated. Otherwise we could be looking at a plane
that is feeding a HDMI port for instance.

And in order to read the plane register we need the power
well to be on. Make sure that is indeed the case. We'll
also make sure the plane is actually enabled before we
trust the rotation bit to tell us the truth.

v2: s/intel_dsi/vlv_dsi/

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/vlv_dsi.c | 56 ++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 13 deletions(-)

Comments

Hans de Goede Oct. 22, 2018, 7:41 p.m. UTC | #1 
Hi,

On 22-10-18 16:19, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's make sure the DSI port is actually on before we go
> poking at the plane register to determine which way
> it's rotated. Otherwise we could be looking at a plane
> that is feeding a HDMI port for instance.
> 
> And in order to read the plane register we need the power
> well to be on. Make sure that is indeed the case. We'll
> also make sure the plane is actually enabled before we
> trust the rotation bit to tell us the truth.
> 
> v2: s/intel_dsi/vlv_dsi/
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, this series correctly detects the panel being upside-down
on the BYT tablet with an upside down panel which I have, so
this series is:

Tested-by: Hans de Goede <hdegoede@redhat.com>

I assume that most tablets like this one will have the general->rotate_180
bit set so that the firmware setup / EFI console will show up the right way
up. So I think going ahead with this is fine.

Regards,

Hans




> ---
>   drivers/gpu/drm/i915/vlv_dsi.c | 56 ++++++++++++++++++++++++++--------
>   1 file changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index ee0cd5d0bf91..dcc59f653e5b 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -1647,27 +1647,57 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
>   	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>   };
>   
> -static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
> +static enum drm_panel_orientation
> +vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector)
>   {
>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> -	int orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> -	enum i9xx_plane_id i9xx_plane;
> +	struct intel_encoder *encoder = connector->encoder;
> +	enum intel_display_power_domain power_domain;
> +	enum drm_panel_orientation orientation;
> +	struct intel_plane *plane;
> +	struct intel_crtc *crtc;
> +	enum pipe pipe;
>   	u32 val;
>   
> -	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> -		if (connector->encoder->crtc_mask == BIT(PIPE_B))
> -			i9xx_plane = PLANE_B;
> -		else
> -			i9xx_plane = PLANE_A;
> +	if (!encoder->get_hw_state(encoder, &pipe))
> +		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
>   
> -		val = I915_READ(DSPCNTR(i9xx_plane));
> -		if (val & DISPPLANE_ROTATE_180)
> -			orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> -	}
> +	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +	plane = to_intel_plane(crtc->base.primary);
> +
> +	power_domain = POWER_DOMAIN_PIPE(pipe);
> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> +		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +
> +	val = I915_READ(DSPCNTR(plane->i9xx_plane));
> +
> +	if (!(val & DISPLAY_PLANE_ENABLE))
> +		orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +	else if (val & DISPPLANE_ROTATE_180)
> +		orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> +	else
> +		orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> +
> +	intel_display_power_put(dev_priv, power_domain);
>   
>   	return orientation;
>   }
>   
> +static enum drm_panel_orientation
> +vlv_dsi_get_panel_orientation(struct intel_connector *connector)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	enum drm_panel_orientation orientation;
> +
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +		orientation = vlv_dsi_get_hw_panel_orientation(connector);
> +		if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +			return orientation;
> +	}
> +
> +	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
> +}
> +
>   static void intel_dsi_add_properties(struct intel_connector *connector)
>   {
>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> @@ -1685,7 +1715,7 @@ static void intel_dsi_add_properties(struct intel_connector *connector)
>   		connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>   
>   		connector->base.display_info.panel_orientation =
> -			intel_dsi_get_panel_orientation(connector);
> +			vlv_dsi_get_panel_orientation(connector);
>   		drm_connector_init_panel_orientation_property(
>   				&connector->base,
>   				connector->panel.fixed_mode->hdisplay,
>
Ville Syrjälä Nov. 13, 2018, 4 p.m. UTC | #2 
On Mon, Oct 22, 2018 at 09:41:36PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-10-18 16:19, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Let's make sure the DSI port is actually on before we go
> > poking at the plane register to determine which way
> > it's rotated. Otherwise we could be looking at a plane
> > that is feeding a HDMI port for instance.
> > 
> > And in order to read the plane register we need the power
> > well to be on. Make sure that is indeed the case. We'll
> > also make sure the plane is actually enabled before we
> > trust the rotation bit to tell us the truth.
> > 
> > v2: s/intel_dsi/vlv_dsi/
> > 
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Ok, this series correctly detects the panel being upside-down
> on the BYT tablet with an upside down panel which I have, so
> this series is:
> 
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> 
> I assume that most tablets like this one will have the general->rotate_180
> bit set so that the firmware setup / EFI console will show up the right way
> up. So I think going ahead with this is fine.

Thanks. Pushed patch 1 and 2, with Maarten's irc r-b for patch 1.

I might leave patch 3 to sit for a while longer.

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> > ---
> >   drivers/gpu/drm/i915/vlv_dsi.c | 56 ++++++++++++++++++++++++++--------
> >   1 file changed, 43 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> > index ee0cd5d0bf91..dcc59f653e5b 100644
> > --- a/drivers/gpu/drm/i915/vlv_dsi.c
> > +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> > @@ -1647,27 +1647,57 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
> >   	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
> >   };
> >   
> > -static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
> > +static enum drm_panel_orientation
> > +vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector)
> >   {
> >   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > -	int orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> > -	enum i9xx_plane_id i9xx_plane;
> > +	struct intel_encoder *encoder = connector->encoder;
> > +	enum intel_display_power_domain power_domain;
> > +	enum drm_panel_orientation orientation;
> > +	struct intel_plane *plane;
> > +	struct intel_crtc *crtc;
> > +	enum pipe pipe;
> >   	u32 val;
> >   
> > -	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > -		if (connector->encoder->crtc_mask == BIT(PIPE_B))
> > -			i9xx_plane = PLANE_B;
> > -		else
> > -			i9xx_plane = PLANE_A;
> > +	if (!encoder->get_hw_state(encoder, &pipe))
> > +		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> >   
> > -		val = I915_READ(DSPCNTR(i9xx_plane));
> > -		if (val & DISPPLANE_ROTATE_180)
> > -			orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> > -	}
> > +	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > +	plane = to_intel_plane(crtc->base.primary);
> > +
> > +	power_domain = POWER_DOMAIN_PIPE(pipe);
> > +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> > +		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > +
> > +	val = I915_READ(DSPCNTR(plane->i9xx_plane));
> > +
> > +	if (!(val & DISPLAY_PLANE_ENABLE))
> > +		orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > +	else if (val & DISPPLANE_ROTATE_180)
> > +		orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> > +	else
> > +		orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> > +
> > +	intel_display_power_put(dev_priv, power_domain);
> >   
> >   	return orientation;
> >   }
> >   
> > +static enum drm_panel_orientation
> > +vlv_dsi_get_panel_orientation(struct intel_connector *connector)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > +	enum drm_panel_orientation orientation;
> > +
> > +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > +		orientation = vlv_dsi_get_hw_panel_orientation(connector);
> > +		if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > +			return orientation;
> > +	}
> > +
> > +	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
> > +}
> > +
> >   static void intel_dsi_add_properties(struct intel_connector *connector)
> >   {
> >   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > @@ -1685,7 +1715,7 @@ static void intel_dsi_add_properties(struct intel_connector *connector)
> >   		connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> >   
> >   		connector->base.display_info.panel_orientation =
> > -			intel_dsi_get_panel_orientation(connector);
> > +			vlv_dsi_get_panel_orientation(connector);
> >   		drm_connector_init_panel_orientation_property(
> >   				&connector->base,
> >   				connector->panel.fixed_mode->hdisplay,
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index ee0cd5d0bf91..dcc59f653e5b 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -1647,27 +1647,57 @@  static const struct drm_connector_funcs intel_dsi_connector_funcs = {
 	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
 };
 
-static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
+static enum drm_panel_orientation
+vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
-	int orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
-	enum i9xx_plane_id i9xx_plane;
+	struct intel_encoder *encoder = connector->encoder;
+	enum intel_display_power_domain power_domain;
+	enum drm_panel_orientation orientation;
+	struct intel_plane *plane;
+	struct intel_crtc *crtc;
+	enum pipe pipe;
 	u32 val;
 
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		if (connector->encoder->crtc_mask == BIT(PIPE_B))
-			i9xx_plane = PLANE_B;
-		else
-			i9xx_plane = PLANE_A;
+	if (!encoder->get_hw_state(encoder, &pipe))
+		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
 
-		val = I915_READ(DSPCNTR(i9xx_plane));
-		if (val & DISPPLANE_ROTATE_180)
-			orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
-	}
+	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+	plane = to_intel_plane(crtc->base.primary);
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+
+	val = I915_READ(DSPCNTR(plane->i9xx_plane));
+
+	if (!(val & DISPLAY_PLANE_ENABLE))
+		orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+	else if (val & DISPPLANE_ROTATE_180)
+		orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+	else
+		orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
+
+	intel_display_power_put(dev_priv, power_domain);
 
 	return orientation;
 }
 
+static enum drm_panel_orientation
+vlv_dsi_get_panel_orientation(struct intel_connector *connector)
+{
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+	enum drm_panel_orientation orientation;
+
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		orientation = vlv_dsi_get_hw_panel_orientation(connector);
+		if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+			return orientation;
+	}
+
+	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
+}
+
 static void intel_dsi_add_properties(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
@@ -1685,7 +1715,7 @@  static void intel_dsi_add_properties(struct intel_connector *connector)
 		connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
 
 		connector->base.display_info.panel_orientation =
-			intel_dsi_get_panel_orientation(connector);
+			vlv_dsi_get_panel_orientation(connector);
 		drm_connector_init_panel_orientation_property(
 				&connector->base,
 				connector->panel.fixed_mode->hdisplay,