All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout
@ 2018-10-19 19:59 Ville Syrjala
  2018-10-19 19:59 ` [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT Ville Syrjala
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Ville Syrjala @ 2018-10-19 19:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede

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;
+}
+
 static void intel_dsi_add_properties(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
-- 
2.18.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT
  2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
@ 2018-10-19 19:59 ` Ville Syrjala
  2018-10-22 13:07   ` Jani Nikula
  2018-10-22 14:20   ` [PATCH v2 " Ville Syrjala
  2018-10-19 19:59 ` [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation Ville Syrjala
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjala @ 2018-10-19 19:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede

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

VBT appears to have two (or possibly three) ways to indicate the panel
rotation. The first is in the MIPI config block, but that apparenly
usually (maybe always?) indicates 0 degrees despite the actual panel
orientation. The second way to indicate this is in the general features
block, which can just indicate whether 180 degress rotation is used.
The third might be a separate rotation data block, but that is not
at all documented so who knows what it may contain.

Let's try the first two. We first try the DSI specicic VBT
information, and it it doesn't look trustworthy (ie. indicates
0 degrees) we fall back to the 180 degree thing. Just to avoid too
many changes in one go we shall also keep the hardware readout path
for now.

If this works for more than just my VLV FFRD the question becomes
how many of the panel orientation quirks are now redundant?

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 31 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/vlv_dsi.c    | 11 ++++++++++-
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3017ef037fed..115d5963e5a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1102,6 +1102,7 @@ struct intel_vbt_data {
 	unsigned int panel_type:4;
 	int lvds_ssc_freq;
 	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+	enum drm_panel_orientation orientation;
 
 	enum drrs_support_type drrs_type;
 
@@ -1147,6 +1148,7 @@ struct intel_vbt_data {
 		u8 *data;
 		const u8 *sequence[MIPI_SEQ_MAX];
 		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
+		enum drm_panel_orientation orientation;
 	} dsi;
 
 	int crt_ddc_pin;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 1faa494e2bc9..a4bfd92212df 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private *dev_priv,
 		intel_bios_ssc_frequency(dev_priv, general->ssc_freq);
 	dev_priv->vbt.display_clock_mode = general->display_clock_mode;
 	dev_priv->vbt.fdi_rx_polarity_inverted = general->fdi_rx_polarity_inverted;
+	if (bdb->version >= 181) {
+		dev_priv->vbt.orientation = general->rotate_180 ?
+			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP :
+			DRM_MODE_PANEL_ORIENTATION_NORMAL;
+	} else {
+		dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+	}
 	DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d fdi_rx_polarity_inverted %d\n",
 		      dev_priv->vbt.int_tv_support,
 		      dev_priv->vbt.int_crt_support,
@@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 
 	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
 
+	/* FIXME is the 90 vs. 270 correct? */
+	switch (config->rotation) {
+	case ENABLE_ROTATION_0:
+		/*
+		 * Most (all?) VBTs claim 0 degrees despite having
+		 * an upside down panel, thus we do not trust this.
+		 */
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+		break;
+	case ENABLE_ROTATION_90:
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+		break;
+	case ENABLE_ROTATION_180:
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+		break;
+	case ENABLE_ROTATION_270:
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+		break;
+	}
+
 	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
 }
diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index 893839ea0ff9..a3119ec3189a 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -1704,7 +1704,8 @@ vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector)
 	return orientation;
 }
 
-static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
+static enum drm_panel_orientation
+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;
@@ -1715,6 +1716,14 @@ static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
 			return orientation;
 	}
 
+	orientation = dev_priv->vbt.dsi.orientation;
+	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		return orientation;
+
+	orientation = dev_priv->vbt.orientation;
+	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		return orientation;
+
 	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
 }
 
-- 
2.18.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation
  2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
  2018-10-19 19:59 ` [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT Ville Syrjala
@ 2018-10-19 19:59 ` Ville Syrjala
  2018-10-19 20:11   ` Chris Wilson
                     ` (2 more replies)
  2018-10-22 12:51 ` [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Jani Nikula
                   ` (4 subsequent siblings)
  6 siblings, 3 replies; 20+ messages in thread
From: Ville Syrjala @ 2018-10-19 19:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede

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

Now that we can actually grab the rotation data from the VBT,
maybe we can get rid of the hardware readout path? My VLV
FFRD is still happy.

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 | 42 ----------------------------------
 1 file changed, 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index a3119ec3189a..6f25571b5800 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -1668,54 +1668,12 @@ 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;
-	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);
-	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
 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;
-	}
-
 	orientation = dev_priv->vbt.dsi.orientation;
 	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
 		return orientation;
-- 
2.18.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation
  2018-10-19 19:59 ` [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation Ville Syrjala
@ 2018-10-19 20:11   ` Chris Wilson
  2018-10-19 20:12     ` Chris Wilson
  2018-10-22 13:09   ` Jani Nikula
  2018-10-22 14:20   ` [PATCH v2 " Ville Syrjala
  2 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2018-10-19 20:11 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Hans de Goede

Quoting Ville Syrjala (2018-10-19 20:59:48)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that we can actually grab the rotation data from the VBT,
> maybe we can get rid of the hardware readout path? My VLV
> FFRD is still happy.

Module reload? kexec? hibernation?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation
  2018-10-19 20:11   ` Chris Wilson
@ 2018-10-19 20:12     ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2018-10-19 20:12 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Hans de Goede

Quoting Chris Wilson (2018-10-19 21:11:11)
> Quoting Ville Syrjala (2018-10-19 20:59:48)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Now that we can actually grab the rotation data from the VBT,
> > maybe we can get rid of the hardware readout path? My VLV
> > FFRD is still happy.
> 
> Module reload? kexec? hibernation?

Panel orientation, not fb orientation. Apologies for the noise.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout
  2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
  2018-10-19 19:59 ` [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT Ville Syrjala
  2018-10-19 19:59 ` [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation Ville Syrjala
@ 2018-10-22 12:51 ` Jani Nikula
  2018-10-22 13:13   ` Ville Syrjälä
  2018-10-22 14:19 ` [PATCH v2 " Ville Syrjala
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2018-10-22 12:51 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Hans de Goede

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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT
  2018-10-19 19:59 ` [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT Ville Syrjala
@ 2018-10-22 13:07   ` Jani Nikula
  2018-10-22 13:25     ` Ville Syrjälä
  2018-10-22 14:20   ` [PATCH v2 " Ville Syrjala
  1 sibling, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2018-10-22 13:07 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Hans de Goede

On Fri, 19 Oct 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VBT appears to have two (or possibly three) ways to indicate the panel
> rotation. The first is in the MIPI config block, but that apparenly
> usually (maybe always?) indicates 0 degrees despite the actual panel
> orientation. The second way to indicate this is in the general features
> block, which can just indicate whether 180 degress rotation is used.
> The third might be a separate rotation data block, but that is not
> at all documented so who knows what it may contain.
>
> Let's try the first two. We first try the DSI specicic VBT
> information, and it it doesn't look trustworthy (ie. indicates
> 0 degrees) we fall back to the 180 degree thing. Just to avoid too
> many changes in one go we shall also keep the hardware readout path
> for now.
>
> If this works for more than just my VLV FFRD the question becomes
> how many of the panel orientation quirks are now redundant?
>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 31 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/vlv_dsi.c    | 11 ++++++++++-
>  3 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3017ef037fed..115d5963e5a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1102,6 +1102,7 @@ struct intel_vbt_data {
>  	unsigned int panel_type:4;
>  	int lvds_ssc_freq;
>  	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
> +	enum drm_panel_orientation orientation;
>  
>  	enum drrs_support_type drrs_type;
>  
> @@ -1147,6 +1148,7 @@ struct intel_vbt_data {
>  		u8 *data;
>  		const u8 *sequence[MIPI_SEQ_MAX];
>  		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
> +		enum drm_panel_orientation orientation;

Would be nice to expose just one orientation field, but I guess we need
the duplication... *sigh*

>  	} dsi;
>  
>  	int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 1faa494e2bc9..a4bfd92212df 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private *dev_priv,
>  		intel_bios_ssc_frequency(dev_priv, general->ssc_freq);
>  	dev_priv->vbt.display_clock_mode = general->display_clock_mode;
>  	dev_priv->vbt.fdi_rx_polarity_inverted = general->fdi_rx_polarity_inverted;
> +	if (bdb->version >= 181) {
> +		dev_priv->vbt.orientation = general->rotate_180 ?
> +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP :
> +			DRM_MODE_PANEL_ORIENTATION_NORMAL;
> +	} else {
> +		dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +	}
>  	DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d fdi_rx_polarity_inverted %d\n",
>  		      dev_priv->vbt.int_tv_support,
>  		      dev_priv->vbt.int_crt_support,
> @@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  
>  	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
>  
> +	/* FIXME is the 90 vs. 270 correct? */

*sigh* Your guess is as good as mine.

> +	switch (config->rotation) {
> +	case ENABLE_ROTATION_0:
> +		/*
> +		 * Most (all?) VBTs claim 0 degrees despite having
> +		 * an upside down panel, thus we do not trust this.
> +		 */
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +		break;
> +	case ENABLE_ROTATION_90:
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> +		break;
> +	case ENABLE_ROTATION_180:
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> +		break;
> +	case ENABLE_ROTATION_270:
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> +		break;
> +	}
> +
>  	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
>  }
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index 893839ea0ff9..a3119ec3189a 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -1704,7 +1704,8 @@ vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector)
>  	return orientation;
>  }
>  
> -static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
> +static enum drm_panel_orientation
> +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;
> @@ -1715,6 +1716,14 @@ static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
>  			return orientation;
>  	}
>  
> +	orientation = dev_priv->vbt.dsi.orientation;
> +	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		return orientation;
> +
> +	orientation = dev_priv->vbt.orientation;
> +	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		return orientation;

I take it you wrote patch 3/3 separately to perhaps apply it much later?
Otherwise the series could be simplified. So in the mean time with just
the first two applied, would it make sense to debug log if there's a
discrepancy between what we read out and what the VBT says?

Either way,

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

> +
>  	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
>  }

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation
  2018-10-19 19:59 ` [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation Ville Syrjala
  2018-10-19 20:11   ` Chris Wilson
@ 2018-10-22 13:09   ` Jani Nikula
  2018-10-22 14:02     ` Hans de Goede
  2018-10-22 14:20   ` [PATCH v2 " Ville Syrjala
  2 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2018-10-22 13:09 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Hans de Goede

On Fri, 19 Oct 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that we can actually grab the rotation data from the VBT,
> maybe we can get rid of the hardware readout path? My VLV
> FFRD is still happy.
>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I think we need input from Hans on this for the reality check.

Whatever you decide,

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

One note below.

> ---
>  drivers/gpu/drm/i915/vlv_dsi.c | 42 ----------------------------------
>  1 file changed, 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index a3119ec3189a..6f25571b5800 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -1668,54 +1668,12 @@ 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;
> -	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);
> -	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
>  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;
> -	}
> -

After this, the function definitely belongs in intel_dsi.c.

>  	orientation = dev_priv->vbt.dsi.orientation;
>  	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>  		return orientation;

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout
  2018-10-22 12:51 ` [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Jani Nikula
@ 2018-10-22 13:13   ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2018-10-22 13:13 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Hans de Goede, intel-gfx

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.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT
  2018-10-22 13:07   ` Jani Nikula
@ 2018-10-22 13:25     ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2018-10-22 13:25 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Hans de Goede, intel-gfx

On Mon, Oct 22, 2018 at 04:07:27PM +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>
> >
> > VBT appears to have two (or possibly three) ways to indicate the panel
> > rotation. The first is in the MIPI config block, but that apparenly
> > usually (maybe always?) indicates 0 degrees despite the actual panel
> > orientation. The second way to indicate this is in the general features
> > block, which can just indicate whether 180 degress rotation is used.
> > The third might be a separate rotation data block, but that is not
> > at all documented so who knows what it may contain.
> >
> > Let's try the first two. We first try the DSI specicic VBT
> > information, and it it doesn't look trustworthy (ie. indicates
> > 0 degrees) we fall back to the 180 degree thing. Just to avoid too
> > many changes in one go we shall also keep the hardware readout path
> > for now.
> >
> > If this works for more than just my VLV FFRD the question becomes
> > how many of the panel orientation quirks are now redundant?
> >
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
> >  drivers/gpu/drm/i915/intel_bios.c | 31 +++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/vlv_dsi.c    | 11 ++++++++++-
> >  3 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 3017ef037fed..115d5963e5a6 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1102,6 +1102,7 @@ struct intel_vbt_data {
> >  	unsigned int panel_type:4;
> >  	int lvds_ssc_freq;
> >  	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
> > +	enum drm_panel_orientation orientation;
> >  
> >  	enum drrs_support_type drrs_type;
> >  
> > @@ -1147,6 +1148,7 @@ struct intel_vbt_data {
> >  		u8 *data;
> >  		const u8 *sequence[MIPI_SEQ_MAX];
> >  		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
> > +		enum drm_panel_orientation orientation;
> 
> Would be nice to expose just one orientation field, but I guess we need
> the duplication... *sigh*

There is maybe a theoretical concern about systems with mixed panel
types (eDP+DSI for example) where we might need different rotation
values for each type. But if we assume that this stuff only matters
for DSI panels then we could leave the policy of "which field do we
prefer" in the VBT parsing code.

The whole thing is a mess anyway. There really should just be a
per-panel orientation knob in the VBT.

> 
> >  	} dsi;
> >  
> >  	int crt_ddc_pin;
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index 1faa494e2bc9..a4bfd92212df 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private *dev_priv,
> >  		intel_bios_ssc_frequency(dev_priv, general->ssc_freq);
> >  	dev_priv->vbt.display_clock_mode = general->display_clock_mode;
> >  	dev_priv->vbt.fdi_rx_polarity_inverted = general->fdi_rx_polarity_inverted;
> > +	if (bdb->version >= 181) {
> > +		dev_priv->vbt.orientation = general->rotate_180 ?
> > +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP :
> > +			DRM_MODE_PANEL_ORIENTATION_NORMAL;
> > +	} else {
> > +		dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > +	}
> >  	DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d fdi_rx_polarity_inverted %d\n",
> >  		      dev_priv->vbt.int_tv_support,
> >  		      dev_priv->vbt.int_crt_support,
> > @@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
> >  
> >  	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
> >  
> > +	/* FIXME is the 90 vs. 270 correct? */
> 
> *sigh* Your guess is as good as mine.
> 
> > +	switch (config->rotation) {
> > +	case ENABLE_ROTATION_0:
> > +		/*
> > +		 * Most (all?) VBTs claim 0 degrees despite having
> > +		 * an upside down panel, thus we do not trust this.
> > +		 */
> > +		dev_priv->vbt.dsi.orientation =
> > +			DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > +		break;
> > +	case ENABLE_ROTATION_90:
> > +		dev_priv->vbt.dsi.orientation =
> > +			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> > +		break;
> > +	case ENABLE_ROTATION_180:
> > +		dev_priv->vbt.dsi.orientation =
> > +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> > +		break;
> > +	case ENABLE_ROTATION_270:
> > +		dev_priv->vbt.dsi.orientation =
> > +			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> > +		break;
> > +	}
> > +
> >  	/* We have mandatory mipi config blocks. Initialize as generic panel */
> >  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> >  }
> > diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> > index 893839ea0ff9..a3119ec3189a 100644
> > --- a/drivers/gpu/drm/i915/vlv_dsi.c
> > +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> > @@ -1704,7 +1704,8 @@ vlv_dsi_get_hw_panel_orientation(struct intel_connector *connector)
> >  	return orientation;
> >  }
> >  
> > -static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
> > +static enum drm_panel_orientation
> > +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;
> > @@ -1715,6 +1716,14 @@ static int intel_dsi_get_panel_orientation(struct intel_connector *connector)
> >  			return orientation;
> >  	}
> >  
> > +	orientation = dev_priv->vbt.dsi.orientation;
> > +	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > +		return orientation;
> > +
> > +	orientation = dev_priv->vbt.orientation;
> > +	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > +		return orientation;
> 
> I take it you wrote patch 3/3 separately to perhaps apply it much later?
> Otherwise the series could be simplified.

I did it this way to make a revert of 3/3 as easy as possible.

> So in the mean time with just
> the first two applied, would it make sense to debug log if there's a
> discrepancy between what we read out and what the VBT says?

I suppose logging all three values could be nice. Would avoid
having to ask for the VBT dump every time.

> 
> Either way,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> > +
> >  	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
> >  }
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation
  2018-10-22 13:09   ` Jani Nikula
@ 2018-10-22 14:02     ` Hans de Goede
  0 siblings, 0 replies; 20+ messages in thread
From: Hans de Goede @ 2018-10-22 14:02 UTC (permalink / raw)
  To: Jani Nikula, Ville Syrjala, intel-gfx

Hi,

On 22-10-18 15:09, 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>
>>
>> Now that we can actually grab the rotation data from the VBT,
>> maybe we can get rid of the hardware readout path? My VLV
>> FFRD is still happy.
>>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I think we need input from Hans on this for the reality check.

Yes please wait a bit with merging this, I will try to give this a
test run sometime the coming days.

Regards,

Hans



> 
> Whatever you decide,
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> One note below.
> 
>> ---
>>   drivers/gpu/drm/i915/vlv_dsi.c | 42 ----------------------------------
>>   1 file changed, 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
>> index a3119ec3189a..6f25571b5800 100644
>> --- a/drivers/gpu/drm/i915/vlv_dsi.c
>> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
>> @@ -1668,54 +1668,12 @@ 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;
>> -	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);
>> -	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
>>   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;
>> -	}
>> -
> 
> After this, the function definitely belongs in intel_dsi.c.
> 
>>   	orientation = dev_priv->vbt.dsi.orientation;
>>   	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>>   		return orientation;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout
  2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
                   ` (2 preceding siblings ...)
  2018-10-22 12:51 ` [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Jani Nikula
@ 2018-10-22 14:19 ` Ville Syrjala
  2018-10-22 19:41   ` Hans de Goede
  2018-10-22 16:26 ` ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4) Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjala @ 2018-10-22 14:19 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede

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

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,
-- 
2.18.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/3] drm/i915: Determine DSI panel orientation from VBT
  2018-10-19 19:59 ` [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT Ville Syrjala
  2018-10-22 13:07   ` Jani Nikula
@ 2018-10-22 14:20   ` Ville Syrjala
  2018-10-22 14:52     ` Jani Nikula
  1 sibling, 1 reply; 20+ messages in thread
From: Ville Syrjala @ 2018-10-22 14:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede

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

VBT appears to have two (or possibly three) ways to indicate the panel
rotation. The first is in the MIPI config block, but that apparenly
usually (maybe always?) indicates 0 degrees despite the actual panel
orientation. The second way to indicate this is in the general features
block, which can just indicate whether 180 degress rotation is used.
The third might be a separate rotation data block, but that is not
at all documented so who knows what it may contain.

Let's try the first two. We first try the DSI specicic VBT
information, and it it doesn't look trustworthy (ie. indicates
0 degrees) we fall back to the 180 degree thing. Just to avoid too
many changes in one go we shall also keep the hardware readout path
for now.

If this works for more than just my VLV FFRD the question becomes
how many of the panel orientation quirks are now redundant?

v2: Move the code into intel_dsi.c (Jani)

Cc: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  2 ++
 drivers/gpu/drm/i915/intel_bios.c | 31 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.c  | 17 +++++++++++++++++
 drivers/gpu/drm/i915/intel_dsi.h  |  2 ++
 drivers/gpu/drm/i915/vlv_dsi.c    |  2 +-
 5 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3017ef037fed..115d5963e5a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1102,6 +1102,7 @@ struct intel_vbt_data {
 	unsigned int panel_type:4;
 	int lvds_ssc_freq;
 	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+	enum drm_panel_orientation orientation;
 
 	enum drrs_support_type drrs_type;
 
@@ -1147,6 +1148,7 @@ struct intel_vbt_data {
 		u8 *data;
 		const u8 *sequence[MIPI_SEQ_MAX];
 		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
+		enum drm_panel_orientation orientation;
 	} dsi;
 
 	int crt_ddc_pin;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 1faa494e2bc9..a4bfd92212df 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private *dev_priv,
 		intel_bios_ssc_frequency(dev_priv, general->ssc_freq);
 	dev_priv->vbt.display_clock_mode = general->display_clock_mode;
 	dev_priv->vbt.fdi_rx_polarity_inverted = general->fdi_rx_polarity_inverted;
+	if (bdb->version >= 181) {
+		dev_priv->vbt.orientation = general->rotate_180 ?
+			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP :
+			DRM_MODE_PANEL_ORIENTATION_NORMAL;
+	} else {
+		dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+	}
 	DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d fdi_rx_polarity_inverted %d\n",
 		      dev_priv->vbt.int_tv_support,
 		      dev_priv->vbt.int_crt_support,
@@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
 
 	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
 
+	/* FIXME is the 90 vs. 270 correct? */
+	switch (config->rotation) {
+	case ENABLE_ROTATION_0:
+		/*
+		 * Most (all?) VBTs claim 0 degrees despite having
+		 * an upside down panel, thus we do not trust this.
+		 */
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
+		break;
+	case ENABLE_ROTATION_90:
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+		break;
+	case ENABLE_ROTATION_180:
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+		break;
+	case ENABLE_ROTATION_270:
+		dev_priv->vbt.dsi.orientation =
+			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+		break;
+	}
+
 	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
 }
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index a32cc1f4b384..ccbbebf118c1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -28,3 +28,20 @@ int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi)
 		return 200;
 	}
 }
+
+enum drm_panel_orientation
+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;
+
+	orientation = dev_priv->vbt.dsi.orientation;
+	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		return orientation;
+
+	orientation = dev_priv->vbt.orientation;
+	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		return orientation;
+
+	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
+}
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 14567929de9a..3f1bf884ff74 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -148,6 +148,8 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
 /* intel_dsi.c */
 int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
 int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);
+enum drm_panel_orientation
+intel_dsi_get_panel_orientation(struct intel_connector *connector);
 
 /* vlv_dsi.c */
 void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index dcc59f653e5b..99aa7abd15a8 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -1695,7 +1695,7 @@ vlv_dsi_get_panel_orientation(struct intel_connector *connector)
 			return orientation;
 	}
 
-	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
+	return intel_dsi_get_panel_orientation(connector);
 }
 
 static void intel_dsi_add_properties(struct intel_connector *connector)
-- 
2.18.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation
  2018-10-19 19:59 ` [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation Ville Syrjala
  2018-10-19 20:11   ` Chris Wilson
  2018-10-22 13:09   ` Jani Nikula
@ 2018-10-22 14:20   ` Ville Syrjala
  2 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2018-10-22 14:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede

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

Now that we can actually grab the rotation data from the VBT,
maybe we can get rid of the hardware readout path? My VLV
FFRD is still happy.

v2: Rebase due to moving code to intel_dsi.c

Cc: Hans de Goede <hdegoede@redhat.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/vlv_dsi.c | 53 +---------------------------------
 1 file changed, 1 insertion(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index 99aa7abd15a8..ae733cc330c1 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -1647,57 +1647,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;
-	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);
-	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 intel_dsi_get_panel_orientation(connector);
-}
-
 static void intel_dsi_add_properties(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
@@ -1715,7 +1664,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 =
-			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.18.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] drm/i915: Determine DSI panel orientation from VBT
  2018-10-22 14:20   ` [PATCH v2 " Ville Syrjala
@ 2018-10-22 14:52     ` Jani Nikula
  0 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2018-10-22 14:52 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Hans de Goede

On Mon, 22 Oct 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VBT appears to have two (or possibly three) ways to indicate the panel
> rotation. The first is in the MIPI config block, but that apparenly
> usually (maybe always?) indicates 0 degrees despite the actual panel
> orientation. The second way to indicate this is in the general features
> block, which can just indicate whether 180 degress rotation is used.
> The third might be a separate rotation data block, but that is not
> at all documented so who knows what it may contain.
>
> Let's try the first two. We first try the DSI specicic VBT
> information, and it it doesn't look trustworthy (ie. indicates
> 0 degrees) we fall back to the 180 degree thing. Just to avoid too
> many changes in one go we shall also keep the hardware readout path
> for now.
>
> If this works for more than just my VLV FFRD the question becomes
> how many of the panel orientation quirks are now redundant?
>
> v2: Move the code into intel_dsi.c (Jani)
>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1

v2 too

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c | 31 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.c  | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.h  |  2 ++
>  drivers/gpu/drm/i915/vlv_dsi.c    |  2 +-
>  5 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3017ef037fed..115d5963e5a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1102,6 +1102,7 @@ struct intel_vbt_data {
>  	unsigned int panel_type:4;
>  	int lvds_ssc_freq;
>  	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
> +	enum drm_panel_orientation orientation;
>  
>  	enum drrs_support_type drrs_type;
>  
> @@ -1147,6 +1148,7 @@ struct intel_vbt_data {
>  		u8 *data;
>  		const u8 *sequence[MIPI_SEQ_MAX];
>  		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
> +		enum drm_panel_orientation orientation;
>  	} dsi;
>  
>  	int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 1faa494e2bc9..a4bfd92212df 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -420,6 +420,13 @@ parse_general_features(struct drm_i915_private *dev_priv,
>  		intel_bios_ssc_frequency(dev_priv, general->ssc_freq);
>  	dev_priv->vbt.display_clock_mode = general->display_clock_mode;
>  	dev_priv->vbt.fdi_rx_polarity_inverted = general->fdi_rx_polarity_inverted;
> +	if (bdb->version >= 181) {
> +		dev_priv->vbt.orientation = general->rotate_180 ?
> +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP :
> +			DRM_MODE_PANEL_ORIENTATION_NORMAL;
> +	} else {
> +		dev_priv->vbt.orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +	}
>  	DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d fdi_rx_polarity_inverted %d\n",
>  		      dev_priv->vbt.int_tv_support,
>  		      dev_priv->vbt.int_crt_support,
> @@ -852,6 +859,30 @@ parse_mipi_config(struct drm_i915_private *dev_priv,
>  
>  	parse_dsi_backlight_ports(dev_priv, bdb->version, port);
>  
> +	/* FIXME is the 90 vs. 270 correct? */
> +	switch (config->rotation) {
> +	case ENABLE_ROTATION_0:
> +		/*
> +		 * Most (all?) VBTs claim 0 degrees despite having
> +		 * an upside down panel, thus we do not trust this.
> +		 */
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> +		break;
> +	case ENABLE_ROTATION_90:
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> +		break;
> +	case ENABLE_ROTATION_180:
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> +		break;
> +	case ENABLE_ROTATION_270:
> +		dev_priv->vbt.dsi.orientation =
> +			DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> +		break;
> +	}
> +
>  	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index a32cc1f4b384..ccbbebf118c1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -28,3 +28,20 @@ int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi)
>  		return 200;
>  	}
>  }
> +
> +enum drm_panel_orientation
> +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;
> +
> +	orientation = dev_priv->vbt.dsi.orientation;
> +	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		return orientation;
> +
> +	orientation = dev_priv->vbt.orientation;
> +	if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		return orientation;
> +
> +	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 14567929de9a..3f1bf884ff74 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -148,6 +148,8 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
>  /* intel_dsi.c */
>  int intel_dsi_bitrate(const struct intel_dsi *intel_dsi);
>  int intel_dsi_tlpx_ns(const struct intel_dsi *intel_dsi);
> +enum drm_panel_orientation
> +intel_dsi_get_panel_orientation(struct intel_connector *connector);
>  
>  /* vlv_dsi.c */
>  void vlv_dsi_wait_for_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index dcc59f653e5b..99aa7abd15a8 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -1695,7 +1695,7 @@ vlv_dsi_get_panel_orientation(struct intel_connector *connector)
>  			return orientation;
>  	}
>  
> -	return DRM_MODE_PANEL_ORIENTATION_NORMAL;
> +	return intel_dsi_get_panel_orientation(connector);
>  }
>  
>  static void intel_dsi_add_properties(struct intel_connector *connector)

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4)
  2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
                   ` (3 preceding siblings ...)
  2018-10-22 14:19 ` [PATCH v2 " Ville Syrjala
@ 2018-10-22 16:26 ` Patchwork
  2018-10-22 16:49 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-10-22 19:53 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-10-22 16:26 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4)
URL   : https://patchwork.freedesktop.org/series/51274/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Fix the VLV/CHV DSI panel orientation hw readout
Okay!

Commit: drm/i915: Determine DSI panel orientation from VBT
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3725:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3727:16: warning: expression using sizeof(void)

Commit: drm/i915: Remove the hardware readout path for DSI panel orientation
Okay!

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4)
  2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
                   ` (4 preceding siblings ...)
  2018-10-22 16:26 ` ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4) Patchwork
@ 2018-10-22 16:49 ` Patchwork
  2018-10-22 19:53 ` ✓ Fi.CI.IGT: " Patchwork
  6 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-10-22 16:49 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4)
URL   : https://patchwork.freedesktop.org/series/51274/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5017 -> Patchwork_10526 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51274/revisions/4/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10526 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_store@basic-all:
      fi-icl-u:           NOTRUN -> DMESG-WARN (fdo#107732) +4

    igt@gem_exec_suspend@basic:
      fi-icl-u:           NOTRUN -> DMESG-WARN (fdo#107724) +25

    igt@gem_exec_suspend@basic-s3:
      fi-kbl-soraka:      NOTRUN -> INCOMPLETE (fdo#107859, fdo#107774, fdo#107556)
      fi-icl-u:           NOTRUN -> DMESG-WARN (fdo#108512)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-icl-u:           NOTRUN -> INCOMPLETE (fdo#107713)
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128, fdo#107139) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-cfl-8700k:       FAIL (fdo#104008) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#107732 https://bugs.freedesktop.org/show_bug.cgi?id=107732
  fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
  fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859
  fdo#108512 https://bugs.freedesktop.org/show_bug.cgi?id=108512


== Participating hosts (51 -> 46) ==

  Additional (2): fi-kbl-soraka fi-icl-u 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5017 -> Patchwork_10526

  CI_DRM_5017: 9510f8e44127260f92b5b6c3127aafa22b15f741 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4685: 78619fde4008424c472906041edb1d204e014f7c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10526: 9befd3a60624f214defefc4eda2711de5254ddc0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9befd3a60624 drm/i915: Remove the hardware readout path for DSI panel orientation
ffbf661104c9 drm/i915: Determine DSI panel orientation from VBT
1578efe78de1 drm/i915: Fix the VLV/CHV DSI panel orientation hw readout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10526/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout
  2018-10-22 14:19 ` [PATCH v2 " Ville Syrjala
@ 2018-10-22 19:41   ` Hans de Goede
  2018-11-13 16:00     ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2018-10-22 19:41 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

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,
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4)
  2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
                   ` (5 preceding siblings ...)
  2018-10-22 16:49 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-22 19:53 ` Patchwork
  6 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-10-22 19:53 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4)
URL   : https://patchwork.freedesktop.org/series/51274/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5017_full -> Patchwork_10526_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10526_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10526_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10526_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_10526_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@gem_userptr_blits@readonly-unsync:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108074)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          PASS -> FAIL (fdo#108145)

    igt@kms_cursor_crc@cursor-64x21-sliding:
      shard-glk:          PASS -> FAIL (fdo#103232)

    igt@kms_draw_crc@draw-method-rgb565-blt-ytiled:
      shard-skl:          PASS -> FAIL (fdo#103184)

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-apl:          PASS -> FAIL (fdo#103060)
      shard-glk:          PASS -> FAIL (fdo#103060)

    igt@kms_flip_tiling@flip-x-tiled:
      shard-apl:          PASS -> DMESG-WARN (fdo#103558, fdo#105602) +10

    igt@kms_flip_tiling@flip-yf-tiled:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-apl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
      shard-glk:          PASS -> FAIL (fdo#103167) +1

    igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
      shard-skl:          NOTRUN -> FAIL (fdo#108145, fdo#107815)

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166) +1

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@syncobj_wait@wait-for-submit-complex:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108490)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-modeset-hang-newfb-render-c:
      shard-hsw:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
      shard-kbl:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_color@pipe-a-legacy-gamma:
      shard-skl:          FAIL (fdo#108145, fdo#104782) -> PASS

    igt@kms_draw_crc@draw-method-rgb565-pwrite-xtiled:
      shard-skl:          FAIL (fdo#103184) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-apl:          FAIL (fdo#105363, fdo#102887) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
      shard-glk:          FAIL (fdo#103167) -> PASS +2

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
      shard-skl:          FAIL (fdo#103166) -> PASS

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-apl:          FAIL (fdo#103166) -> PASS +2

    igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
      shard-apl:          FAIL (fdo#108145) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-b-ts-continuation-dpms-rpm:
      shard-glk:          DMESG-WARN (fdo#106538, fdo#105763) -> PASS +1

    igt@pm_rps@reset:
      shard-apl:          FAIL (fdo#102250) -> PASS

    
  fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108490 https://bugs.freedesktop.org/show_bug.cgi?id=108490
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5017 -> Patchwork_10526

  CI_DRM_5017: 9510f8e44127260f92b5b6c3127aafa22b15f741 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4685: 78619fde4008424c472906041edb1d204e014f7c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10526: 9befd3a60624f214defefc4eda2711de5254ddc0 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10526/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout
  2018-10-22 19:41   ` Hans de Goede
@ 2018-11-13 16:00     ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2018-11-13 16:00 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

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,
> > 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-11-13 16:00 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-19 19:59 [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Ville Syrjala
2018-10-19 19:59 ` [PATCH 2/3] drm/i915: Determine DSI panel orientation from VBT Ville Syrjala
2018-10-22 13:07   ` Jani Nikula
2018-10-22 13:25     ` Ville Syrjälä
2018-10-22 14:20   ` [PATCH v2 " Ville Syrjala
2018-10-22 14:52     ` Jani Nikula
2018-10-19 19:59 ` [PATCH 3/3] drm/i915: Remove the hardware readout path for DSI panel orientation Ville Syrjala
2018-10-19 20:11   ` Chris Wilson
2018-10-19 20:12     ` Chris Wilson
2018-10-22 13:09   ` Jani Nikula
2018-10-22 14:02     ` Hans de Goede
2018-10-22 14:20   ` [PATCH v2 " Ville Syrjala
2018-10-22 12:51 ` [PATCH 1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout Jani Nikula
2018-10-22 13:13   ` Ville Syrjälä
2018-10-22 14:19 ` [PATCH v2 " Ville Syrjala
2018-10-22 19:41   ` Hans de Goede
2018-11-13 16:00     ` Ville Syrjälä
2018-10-22 16:26 ` ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915: Fix the VLV/CHV DSI panel orientation hw readout (rev4) Patchwork
2018-10-22 16:49 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-22 19:53 ` ✓ Fi.CI.IGT: " Patchwork

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.