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 |
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);
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 --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);