diff mbox series

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

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

Commit Message

Ville Syrjala Oct. 19, 2018, 7:59 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.

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 | 53 ++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 12 deletions(-)

Comments

Jani Nikula Oct. 22, 2018, 12:51 p.m. UTC | #1 
On Fri, 19 Oct 2018, Ville Syrjala <ville.syrjala@linux.intel.com> 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.
>
> 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 | 53 ++++++++++++++++++++++++++--------
>  1 file changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index dbca30460a6b..893839ea0ff9 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -1668,27 +1668,56 @@ 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 int intel_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;
> +}

Long term I think this should be shoved to intel_dsi.c, the resurrected
dumping ground for platform independent DSI code. But I'm fine with
landing it here first and moving later as the only user is still here.

BR,
Jani.

> +
>  static void intel_dsi_add_properties(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
Ville Syrjala Oct. 22, 2018, 1:13 p.m. UTC | #2 
On Mon, Oct 22, 2018 at 03:51:08PM +0300, Jani Nikula wrote:
> On Fri, 19 Oct 2018, Ville Syrjala <ville.syrjala@linux.intel.com> 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.
> >
> > 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 | 53 ++++++++++++++++++++++++++--------
> >  1 file changed, 41 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> > index dbca30460a6b..893839ea0ff9 100644
> > --- a/drivers/gpu/drm/i915/vlv_dsi.c
> > +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> > @@ -1668,27 +1668,56 @@ 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 int intel_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;
> > +}
> 
> Long term I think this should be shoved to intel_dsi.c, the resurrected
> dumping ground for platform independent DSI code. But I'm fine with
> landing it here first and moving later as the only user is still here.

I suppose I could do it more like that from the start. That would
make it easier to revert the "drop the hw readout path" patch should
it become necessary.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index dbca30460a6b..893839ea0ff9 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -1668,27 +1668,56 @@  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 int intel_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);