All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 1/2] drm/i915/dsi: Remove readback of panel orientation on BYT / CHT
Date: Mon, 16 Dec 2019 15:38:23 +0200	[thread overview]
Message-ID: <20191216133823.GO1208@intel.com> (raw)
In-Reply-To: <20191215213307.689830-1-hdegoede@redhat.com>

On Sun, Dec 15, 2019 at 10:33:06PM +0100, Hans de Goede wrote:
> Commit 82daca297506 ("drm/i915: Add "panel orientation" property to the
> panel connector, v6.") uses hardware state readback to determine if the
> GOP is rotating the image by 180 degrees to compensate for upside-down
> mounted panels.
> 
> When I wrote that commit I tried to find the VBT bits the GOP used to
> decide to rotate the image, but I could not find them. Back then I only
> looked at the rotation bits in struct mipi_config and these read 0 on
> the 1 BYT device I have with an upside-down mounted panel
> (a GP-electronic T701 tablet). While working on a similar problem on a
> BYT device with an eDP panel I noticed that the new
> intel_dsi_get_panel_orientation() helper which gets used on newer
> SoCs (Apollo-Lake, etc.) checks the rotate_180 bit in the
> BDB_GENERAL_FEATURES VBT block.
> 
> I've checked and this bit indeed is set on the GP-electronic T701 tablet,
> so using the generic intel_dsi_get_panel_orientation() helper there does
> the right thing without needing any extra readback of hw state.
> 
> This commit removes the special handling of the panel orientation for
> DSI panels on BYT/CHT devices, bringing the handling in line with the
> handling of DSI panels on other devices.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Same as
https://patchwork.freedesktop.org/patch/257787/?series=51274&rev=4
which I apparently never pushed. But feel free to go with your version
since you have more meat in the commit msg.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/vlv_dsi.c | 55 +-------------------------
>  1 file changed, 1 insertion(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 0ca49b1604c6..18f7616e6c27 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1563,59 +1563,6 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
>  	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>  };
>  
> -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);
> -	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;
> -	intel_wakeref_t wakeref;
> -	enum pipe pipe;
> -	u32 val;
> -
> -	if (!encoder->get_hw_state(encoder, &pipe))
> -		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> -
> -	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> -	plane = to_intel_plane(crtc->base.primary);
> -
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> -	wakeref = intel_display_power_get_if_enabled(dev_priv, power_domain);
> -	if (!wakeref)
> -		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, wakeref);
> -
> -	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 intel_dsi_get_panel_orientation(connector);
> -}
> -
>  static void vlv_dsi_add_properties(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> @@ -1633,7 +1580,7 @@ static void vlv_dsi_add_properties(struct intel_connector *connector)
>  		connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  
>  		connector->base.display_info.panel_orientation =
> -			vlv_dsi_get_panel_orientation(connector);
> +			intel_dsi_get_panel_orientation(connector);
>  		drm_connector_init_panel_orientation_property(
>  				&connector->base,
>  				connector->panel.fixed_mode->hdisplay,
> -- 
> 2.23.0

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/dsi: Remove readback of panel orientation on BYT / CHT
Date: Mon, 16 Dec 2019 15:38:23 +0200	[thread overview]
Message-ID: <20191216133823.GO1208@intel.com> (raw)
In-Reply-To: <20191215213307.689830-1-hdegoede@redhat.com>

On Sun, Dec 15, 2019 at 10:33:06PM +0100, Hans de Goede wrote:
> Commit 82daca297506 ("drm/i915: Add "panel orientation" property to the
> panel connector, v6.") uses hardware state readback to determine if the
> GOP is rotating the image by 180 degrees to compensate for upside-down
> mounted panels.
> 
> When I wrote that commit I tried to find the VBT bits the GOP used to
> decide to rotate the image, but I could not find them. Back then I only
> looked at the rotation bits in struct mipi_config and these read 0 on
> the 1 BYT device I have with an upside-down mounted panel
> (a GP-electronic T701 tablet). While working on a similar problem on a
> BYT device with an eDP panel I noticed that the new
> intel_dsi_get_panel_orientation() helper which gets used on newer
> SoCs (Apollo-Lake, etc.) checks the rotate_180 bit in the
> BDB_GENERAL_FEATURES VBT block.
> 
> I've checked and this bit indeed is set on the GP-electronic T701 tablet,
> so using the generic intel_dsi_get_panel_orientation() helper there does
> the right thing without needing any extra readback of hw state.
> 
> This commit removes the special handling of the panel orientation for
> DSI panels on BYT/CHT devices, bringing the handling in line with the
> handling of DSI panels on other devices.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Same as
https://patchwork.freedesktop.org/patch/257787/?series=51274&rev=4
which I apparently never pushed. But feel free to go with your version
since you have more meat in the commit msg.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/vlv_dsi.c | 55 +-------------------------
>  1 file changed, 1 insertion(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 0ca49b1604c6..18f7616e6c27 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1563,59 +1563,6 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
>  	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>  };
>  
> -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);
> -	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;
> -	intel_wakeref_t wakeref;
> -	enum pipe pipe;
> -	u32 val;
> -
> -	if (!encoder->get_hw_state(encoder, &pipe))
> -		return DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> -
> -	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> -	plane = to_intel_plane(crtc->base.primary);
> -
> -	power_domain = POWER_DOMAIN_PIPE(pipe);
> -	wakeref = intel_display_power_get_if_enabled(dev_priv, power_domain);
> -	if (!wakeref)
> -		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, wakeref);
> -
> -	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 intel_dsi_get_panel_orientation(connector);
> -}
> -
>  static void vlv_dsi_add_properties(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> @@ -1633,7 +1580,7 @@ static void vlv_dsi_add_properties(struct intel_connector *connector)
>  		connector->base.state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  
>  		connector->base.display_info.panel_orientation =
> -			vlv_dsi_get_panel_orientation(connector);
> +			intel_dsi_get_panel_orientation(connector);
>  		drm_connector_init_panel_orientation_property(
>  				&connector->base,
>  				connector->panel.fixed_mode->hdisplay,
> -- 
> 2.23.0

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

  parent reply	other threads:[~2019-12-16 13:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-15 21:33 [PATCH 1/2] drm/i915/dsi: Remove readback of panel orientation on BYT / CHT Hans de Goede
2019-12-15 21:33 ` [Intel-gfx] " Hans de Goede
2019-12-15 21:33 ` [PATCH 2/2] drm/i915/dp: Use BDB_GENERAL_FEATURES VBT block info for builtin panel-orientation Hans de Goede
2019-12-15 21:33   ` [Intel-gfx] " Hans de Goede
2019-12-16 13:39   ` Ville Syrjälä
2019-12-16 13:39     ` [Intel-gfx] " Ville Syrjälä
2019-12-16 18:54     ` Hans de Goede
2019-12-16 18:54       ` [Intel-gfx] " Hans de Goede
2019-12-15 21:57 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/dsi: Remove readback of panel orientation on BYT / CHT Patchwork
2019-12-15 22:25 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2019-12-16 13:38 ` Ville Syrjälä [this message]
2019-12-16 13:38   ` [Intel-gfx] [PATCH 1/2] " Ville Syrjälä
2019-12-16 16:11 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/dsi: Remove readback of panel orientation on BYT / CHT (rev2) Patchwork
2019-12-16 16:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2019-12-16 20:55 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2019-12-17  8:34   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191216133823.GO1208@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.