All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH resend 0/2] drm/connector: Add support for specifying panel_orientation on the kernel cmdline
@ 2019-12-16 11:51 ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2019-12-16 11:51 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: Hans de Goede, intel-gfx, dri-devel

Hi All,

This is a resend of the last 2 remaining patches of my series for adding
support for specifying panel_orientation on the kernel cmdline.

I've already pushed the other 11 patches which were mostly cleanups /
bug-fixes to the cmdline-parsing code and where all acked by Maxime
to drm-misc-next.

The first patch of these 2 still needs to be reviewed, if someone can
review it that would be great.

Once this is reviewed the question becomes how to merge this, both
patches are mostly drm-misc material and the second patch depends on
the changes I just pushed to drm-misc-next, so this series should
probably be pushed to drm-misc-next, but it also makes some changes
to i915 code. I've checked and there is no conflict with these patches
as they apply to drm-misc-next vs drm-intel-next-queued (atm). So it
should be fine to push these 2 patches to drm-misc-next and then do a
back-merge to drm-intel-next-queued.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [Intel-gfx] [PATCH resend 0/2] drm/connector: Add support for specifying panel_orientation on the kernel cmdline
@ 2019-12-16 11:51 ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2019-12-16 11:51 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: intel-gfx, dri-devel

Hi All,

This is a resend of the last 2 remaining patches of my series for adding
support for specifying panel_orientation on the kernel cmdline.

I've already pushed the other 11 patches which were mostly cleanups /
bug-fixes to the cmdline-parsing code and where all acked by Maxime
to drm-misc-next.

The first patch of these 2 still needs to be reviewed, if someone can
review it that would be great.

Once this is reviewed the question becomes how to merge this, both
patches are mostly drm-misc material and the second patch depends on
the changes I just pushed to drm-misc-next, so this series should
probably be pushed to drm-misc-next, but it also makes some changes
to i915 code. I've checked and there is no conflict with these patches
as they apply to drm-misc-next vs drm-intel-next-queued (atm). So it
should be fine to push these 2 patches to drm-misc-next and then do a
back-merge to drm-intel-next-queued.

Regards,

Hans

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

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

* [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
  2019-12-16 11:51 ` [Intel-gfx] " Hans de Goede
@ 2019-12-16 11:51   ` Hans de Goede
  -1 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2019-12-16 11:51 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: Hans de Goede, intel-gfx, Derek Basehore, dri-devel

From: Derek Basehore <dbasehore@chromium.org>

Not every platform needs quirk detection for panel orientation, so
split the drm_connector_init_panel_orientation_property into two
functions. One for platforms without the need for quirks, and the
other for platforms that need quirks.

Hans de Goede (changes in v2):

Rename the function from drm_connector_init_panel_orientation_property
to drm_connector_set_panel_orientation[_with_quirk] and pass in the
panel-orientation to set.

Beside the rename, also make the function set the passed in value
only once, if the value was set before (to a value other then
DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.

This change is preparation for allowing the user to override the
panel-orientation for any connector from the kernel commandline.
When the panel-orientation is overridden this way, then we must ignore
the panel-orientation detection done by the driver.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
 drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
 drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
 drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
 include/drm/drm_connector.h             |  9 ++-
 5 files changed, 71 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 0965632008a9..f4fa5c59717d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
  *	coordinates, so if userspace rotates the picture to adjust for
  *	the orientation it must also apply the same transformation to the
  *	touchscreen input coordinates. This property is initialized by calling
- *	drm_connector_init_panel_orientation_property().
+ *	drm_connector_set_panel_orientation() or
+ *	drm_connector_set_panel_orientation_with_quirk()
  *
  * scaling mode:
  *	This property defines how a non-native mode is upscaled to the native
@@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
 EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
 
 /**
- * drm_connector_init_panel_orientation_property -
- *	initialize the connecters panel_orientation property
- * @connector: connector for which to init the panel-orientation property.
- * @width: width in pixels of the panel, used for panel quirk detection
- * @height: height in pixels of the panel, used for panel quirk detection
+ * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
+ * @connector: connector for which to set the panel-orientation property.
+ * @panel_orientation: drm_panel_orientation value to set
+ *
+ * This function sets the connector's panel_orientation and attaches
+ * a "panel orientation" property to the connector.
  *
- * This function should only be called for built-in panels, after setting
- * connector->display_info.panel_orientation first (if known).
+ * Calling this function on a connector where the panel_orientation has
+ * already been set is a no-op (e.g. the orientation has been overridden with
+ * a kernel commandline option).
  *
- * This function will check for platform specific (e.g. DMI based) quirks
- * overriding display_info.panel_orientation first, then if panel_orientation
- * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
- * "panel orientation" property to the connector.
+ * It is allowed to call this function with a panel_orientation of
+ * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
  *
  * Returns:
  * Zero on success, negative errno on failure.
  */
-int drm_connector_init_panel_orientation_property(
-	struct drm_connector *connector, int width, int height)
+int drm_connector_set_panel_orientation(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_display_info *info = &connector->display_info;
 	struct drm_property *prop;
-	int orientation_quirk;
 
-	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
-	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
-		info->panel_orientation = orientation_quirk;
+	/* Already set? */
+	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		return 0;
 
-	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+	/* Don't attach the property if the orientation is unknown */
+	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
 		return 0;
 
+	info->panel_orientation = panel_orientation;
+
 	prop = dev->mode_config.panel_orientation_property;
 	if (!prop) {
 		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
@@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
 				   info->panel_orientation);
 	return 0;
 }
-EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
+EXPORT_SYMBOL(drm_connector_set_panel_orientation);
+
+/**
+ * drm_connector_set_panel_orientation_with_quirk -
+ *	set the connecter's panel_orientation after checking for quirks
+ * @connector: connector for which to init the panel-orientation property.
+ * @panel_orientation: drm_panel_orientation value to set
+ * @width: width in pixels of the panel, used for panel quirk detection
+ * @height: height in pixels of the panel, used for panel quirk detection
+ *
+ * Like drm_connector_set_panel_orientation(), but with a check for platform
+ * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_set_panel_orientation_with_quirk(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation,
+	int width, int height)
+{
+	int orientation_quirk;
+
+	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
+	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		panel_orientation = orientation_quirk;
+
+	return drm_connector_set_panel_orientation(connector,
+						   panel_orientation);
+}
+EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
 
 int drm_connector_set_obj_prop(struct drm_mode_object *obj,
 				    struct drm_property *property,
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 6e398c33a524..8cd51cf67d02 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1536,9 +1536,8 @@ static void icl_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);
-	drm_connector_init_panel_orientation_property(&connector->base,
+	drm_connector_set_panel_orientation_with_quirk(&connector->base,
+				intel_dsi_get_panel_orientation(connector),
 				connector->panel.fixed_mode->hdisplay,
 				connector->panel.fixed_mode->vdisplay);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index aa515261cb9f..9f4425f8d0ac 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	intel_connector->panel.backlight.power = intel_edp_backlight_power;
 	intel_panel_setup_backlight(connector, pipe);
 
-	if (fixed_mode)
-		drm_connector_init_panel_orientation_property(
-			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
+	if (fixed_mode) {
+		/* We do not know the orientation, but their might be a quirk */
+		drm_connector_set_panel_orientation_with_quirk(connector,
+				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
+				fixed_mode->hdisplay, fixed_mode->vdisplay);
+	}
 
 	return true;
 
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 50064cde0724..a0de8e70e426 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -1632,10 +1632,9 @@ 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);
-		drm_connector_init_panel_orientation_property(
+		drm_connector_set_panel_orientation_with_quirk(
 				&connector->base,
+				vlv_dsi_get_panel_orientation(connector),
 				connector->panel.fixed_mode->hdisplay,
 				connector->panel.fixed_mode->vdisplay);
 	}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 221910948b37..2113500b4075 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
 					    uint64_t link_status);
 void drm_connector_set_vrr_capable_property(
 		struct drm_connector *connector, bool capable);
-int drm_connector_init_panel_orientation_property(
-	struct drm_connector *connector, int width, int height);
+int drm_connector_set_panel_orientation(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation);
+int drm_connector_set_panel_orientation_with_quirk(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation,
+	int width, int height);
 int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 					  int min, int max);
 
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [Intel-gfx] [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
@ 2019-12-16 11:51   ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2019-12-16 11:51 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: intel-gfx, Derek Basehore, dri-devel

From: Derek Basehore <dbasehore@chromium.org>

Not every platform needs quirk detection for panel orientation, so
split the drm_connector_init_panel_orientation_property into two
functions. One for platforms without the need for quirks, and the
other for platforms that need quirks.

Hans de Goede (changes in v2):

Rename the function from drm_connector_init_panel_orientation_property
to drm_connector_set_panel_orientation[_with_quirk] and pass in the
panel-orientation to set.

Beside the rename, also make the function set the passed in value
only once, if the value was set before (to a value other then
DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.

This change is preparation for allowing the user to override the
panel-orientation for any connector from the kernel commandline.
When the panel-orientation is overridden this way, then we must ignore
the panel-orientation detection done by the driver.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
 drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
 drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
 drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
 include/drm/drm_connector.h             |  9 ++-
 5 files changed, 71 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 0965632008a9..f4fa5c59717d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
  *	coordinates, so if userspace rotates the picture to adjust for
  *	the orientation it must also apply the same transformation to the
  *	touchscreen input coordinates. This property is initialized by calling
- *	drm_connector_init_panel_orientation_property().
+ *	drm_connector_set_panel_orientation() or
+ *	drm_connector_set_panel_orientation_with_quirk()
  *
  * scaling mode:
  *	This property defines how a non-native mode is upscaled to the native
@@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
 EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
 
 /**
- * drm_connector_init_panel_orientation_property -
- *	initialize the connecters panel_orientation property
- * @connector: connector for which to init the panel-orientation property.
- * @width: width in pixels of the panel, used for panel quirk detection
- * @height: height in pixels of the panel, used for panel quirk detection
+ * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
+ * @connector: connector for which to set the panel-orientation property.
+ * @panel_orientation: drm_panel_orientation value to set
+ *
+ * This function sets the connector's panel_orientation and attaches
+ * a "panel orientation" property to the connector.
  *
- * This function should only be called for built-in panels, after setting
- * connector->display_info.panel_orientation first (if known).
+ * Calling this function on a connector where the panel_orientation has
+ * already been set is a no-op (e.g. the orientation has been overridden with
+ * a kernel commandline option).
  *
- * This function will check for platform specific (e.g. DMI based) quirks
- * overriding display_info.panel_orientation first, then if panel_orientation
- * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
- * "panel orientation" property to the connector.
+ * It is allowed to call this function with a panel_orientation of
+ * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
  *
  * Returns:
  * Zero on success, negative errno on failure.
  */
-int drm_connector_init_panel_orientation_property(
-	struct drm_connector *connector, int width, int height)
+int drm_connector_set_panel_orientation(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_display_info *info = &connector->display_info;
 	struct drm_property *prop;
-	int orientation_quirk;
 
-	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
-	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
-		info->panel_orientation = orientation_quirk;
+	/* Already set? */
+	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		return 0;
 
-	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+	/* Don't attach the property if the orientation is unknown */
+	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
 		return 0;
 
+	info->panel_orientation = panel_orientation;
+
 	prop = dev->mode_config.panel_orientation_property;
 	if (!prop) {
 		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
@@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
 				   info->panel_orientation);
 	return 0;
 }
-EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
+EXPORT_SYMBOL(drm_connector_set_panel_orientation);
+
+/**
+ * drm_connector_set_panel_orientation_with_quirk -
+ *	set the connecter's panel_orientation after checking for quirks
+ * @connector: connector for which to init the panel-orientation property.
+ * @panel_orientation: drm_panel_orientation value to set
+ * @width: width in pixels of the panel, used for panel quirk detection
+ * @height: height in pixels of the panel, used for panel quirk detection
+ *
+ * Like drm_connector_set_panel_orientation(), but with a check for platform
+ * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_set_panel_orientation_with_quirk(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation,
+	int width, int height)
+{
+	int orientation_quirk;
+
+	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
+	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
+		panel_orientation = orientation_quirk;
+
+	return drm_connector_set_panel_orientation(connector,
+						   panel_orientation);
+}
+EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
 
 int drm_connector_set_obj_prop(struct drm_mode_object *obj,
 				    struct drm_property *property,
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 6e398c33a524..8cd51cf67d02 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1536,9 +1536,8 @@ static void icl_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);
-	drm_connector_init_panel_orientation_property(&connector->base,
+	drm_connector_set_panel_orientation_with_quirk(&connector->base,
+				intel_dsi_get_panel_orientation(connector),
 				connector->panel.fixed_mode->hdisplay,
 				connector->panel.fixed_mode->vdisplay);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index aa515261cb9f..9f4425f8d0ac 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	intel_connector->panel.backlight.power = intel_edp_backlight_power;
 	intel_panel_setup_backlight(connector, pipe);
 
-	if (fixed_mode)
-		drm_connector_init_panel_orientation_property(
-			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
+	if (fixed_mode) {
+		/* We do not know the orientation, but their might be a quirk */
+		drm_connector_set_panel_orientation_with_quirk(connector,
+				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
+				fixed_mode->hdisplay, fixed_mode->vdisplay);
+	}
 
 	return true;
 
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 50064cde0724..a0de8e70e426 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -1632,10 +1632,9 @@ 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);
-		drm_connector_init_panel_orientation_property(
+		drm_connector_set_panel_orientation_with_quirk(
 				&connector->base,
+				vlv_dsi_get_panel_orientation(connector),
 				connector->panel.fixed_mode->hdisplay,
 				connector->panel.fixed_mode->vdisplay);
 	}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 221910948b37..2113500b4075 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
 					    uint64_t link_status);
 void drm_connector_set_vrr_capable_property(
 		struct drm_connector *connector, bool capable);
-int drm_connector_init_panel_orientation_property(
-	struct drm_connector *connector, int width, int height);
+int drm_connector_set_panel_orientation(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation);
+int drm_connector_set_panel_orientation_with_quirk(
+	struct drm_connector *connector,
+	enum drm_panel_orientation panel_orientation,
+	int width, int height);
 int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 					  int min, int max);
 
-- 
2.23.0

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

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

* [PATCH resend 2/2] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
  2019-12-16 11:51 ` [Intel-gfx] " Hans de Goede
@ 2019-12-16 11:51   ` Hans de Goede
  -1 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2019-12-16 11:51 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: Hans de Goede, intel-gfx, dri-devel

If the new video=... panel_orientation option is set for a connector, honor
it and setup a matching "panel orientation" property on the connector.

BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_connector.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index f4fa5c59717d..d9d7fef26275 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
 		connector->force = mode->force;
 	}
 
+	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
+		DRM_INFO("setting connector %s panel_orientation to %d\n",
+			 connector->name, mode->panel_orientation);
+		drm_connector_set_panel_orientation(connector,
+						    mode->panel_orientation);
+	}
+
 	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
 		      connector->name, mode->name,
 		      mode->xres, mode->yres,
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [Intel-gfx] [PATCH resend 2/2] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
@ 2019-12-16 11:51   ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2019-12-16 11:51 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: intel-gfx, Maxime Ripard, dri-devel

If the new video=... panel_orientation option is set for a connector, honor
it and setup a matching "panel orientation" property on the connector.

BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_connector.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index f4fa5c59717d..d9d7fef26275 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
 		connector->force = mode->force;
 	}
 
+	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
+		DRM_INFO("setting connector %s panel_orientation to %d\n",
+			 connector->name, mode->panel_orientation);
+		drm_connector_set_panel_orientation(connector,
+						    mode->panel_orientation);
+	}
+
 	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
 		      connector->name, mode->name,
 		      mode->xres, mode->yres,
-- 
2.23.0

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

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/connector: Add support for specifying panel_orientation on the kernel cmdline
  2019-12-16 11:51 ` [Intel-gfx] " Hans de Goede
                   ` (2 preceding siblings ...)
  (?)
@ 2019-12-16 16:45 ` Patchwork
  -1 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-12-16 16:45 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: drm/connector: Add support for specifying panel_orientation on the kernel cmdline
URL   : https://patchwork.freedesktop.org/series/70975/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
0adf9e2ded45 drm/connector: Split out orientation quirk detection (v2)
-:77: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#77: FILE: drivers/gpu/drm/drm_connector.c:2067:
+int drm_connector_set_panel_orientation(

-:124: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#124: FILE: drivers/gpu/drm/drm_connector.c:2117:
+int drm_connector_set_panel_orientation_with_quirk(

-:154: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#154: FILE: drivers/gpu/drm/i915/display/icl_dsi.c:1660:
+	drm_connector_set_panel_orientation_with_quirk(&connector->base,
+				intel_dsi_get_panel_orientation(connector),

-:172: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#172: FILE: drivers/gpu/drm/i915/display/intel_dp.c:7407:
+		drm_connector_set_panel_orientation_with_quirk(connector,
+				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,

-:189: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#189: FILE: drivers/gpu/drm/i915/display/vlv_dsi.c:1635:
+		drm_connector_set_panel_orientation_with_quirk(

-:205: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#205: FILE: include/drm/drm_connector.h:1555:
+int drm_connector_set_panel_orientation(

-:208: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#208: FILE: include/drm/drm_connector.h:1558:
+int drm_connector_set_panel_orientation_with_quirk(

total: 0 errors, 0 warnings, 7 checks, 159 lines checked
874c08fe92f2 drm/connector: Hookup the new drm_cmdline_mode panel_orientation member

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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/connector: Add support for specifying panel_orientation on the kernel cmdline
  2019-12-16 11:51 ` [Intel-gfx] " Hans de Goede
                   ` (3 preceding siblings ...)
  (?)
@ 2019-12-16 17:07 ` Patchwork
  -1 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-12-16 17:07 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: drm/connector: Add support for specifying panel_orientation on the kernel cmdline
URL   : https://patchwork.freedesktop.org/series/70975/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7574 -> Patchwork_15787
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-n2820:       [PASS][1] -> [TIMEOUT][2] ([i915#816])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/fi-byt-n2820/igt@gem_close_race@basic-threads.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/fi-byt-n2820/igt@gem_close_race@basic-threads.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][3] ([i915#178]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [DMESG-FAIL][5] ([i915#725]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][7] ([fdo#111096] / [i915#323]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Warnings ####

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-kbl-x1275:       [DMESG-WARN][9] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][10] ([i915#62] / [i915#92]) +5 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - fi-kbl-x1275:       [DMESG-WARN][11] ([i915#62] / [i915#92]) -> [DMESG-WARN][12] ([i915#62] / [i915#92] / [i915#95]) +2 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [i915#178]: https://gitlab.freedesktop.org/drm/intel/issues/178
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#476]: https://gitlab.freedesktop.org/drm/intel/issues/476
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (53 -> 46)
------------------------------

  Additional (1): fi-hsw-4770 
  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-tgl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7574 -> Patchwork_15787

  CI-20190529: 20190529
  CI_DRM_7574: 950244ca586c6f0efe243bf8c505c01ea5e579fa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5349: 048f58513d8b8ec6bb307a939f0ac959bc0f0e10 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15787: 874c08fe92f2f8f25178d40f120502ec393057af @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

874c08fe92f2 drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
0adf9e2ded45 drm/connector: Split out orientation quirk detection (v2)

== Logs ==

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

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/connector: Add support for specifying panel_orientation on the kernel cmdline
  2019-12-16 11:51 ` [Intel-gfx] " Hans de Goede
                   ` (4 preceding siblings ...)
  (?)
@ 2019-12-16 21:28 ` Patchwork
  -1 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-12-16 21:28 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: drm/connector: Add support for specifying panel_orientation on the kernel cmdline
URL   : https://patchwork.freedesktop.org/series/70975/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7574_full -> Patchwork_15787_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vcs1-reset:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#109276] / [fdo#112080]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb2/igt@gem_ctx_isolation@vcs1-reset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb3/igt@gem_ctx_isolation@vcs1-reset.html

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          [PASS][3] -> [DMESG-WARN][4] ([i915#180]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-kbl2/igt@gem_eio@in-flight-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-kbl7/igt@gem_eio@in-flight-suspend.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#110854])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb2/igt@gem_exec_balancer@smoke.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb3/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_parallel@vcs0:
    - shard-tglb:         [PASS][7] -> [INCOMPLETE][8] ([fdo#111593])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb5/igt@gem_exec_parallel@vcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb4/igt@gem_exec_parallel@vcs0.html

  * igt@gem_exec_schedule@fifo-bsd1:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#109276]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb1/igt@gem_exec_schedule@fifo-bsd1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb8/igt@gem_exec_schedule@fifo-bsd1.html

  * igt@gem_exec_schedule@preempt-queue-bsd:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#112146])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb6/igt@gem_exec_schedule@preempt-queue-bsd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
    - shard-apl:          [PASS][13] -> [FAIL][14] ([i915#520])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-apl2/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-apl7/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html

  * igt@gem_pipe_control_store_loop@reused-buffer:
    - shard-skl:          [PASS][15] -> [INCOMPLETE][16] ([i915#198] / [i915#796])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl6/igt@gem_pipe_control_store_loop@reused-buffer.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl4/igt@gem_pipe_control_store_loop@reused-buffer.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [PASS][17] -> [FAIL][18] ([i915#644])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-glk8/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-glk5/igt@gem_ppgtt@flink-and-close-vma-leak.html
    - shard-tglb:         [PASS][19] -> [FAIL][20] ([i915#644])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb9/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb4/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-snb:          [PASS][21] -> [DMESG-WARN][22] ([fdo#111870]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-snb2/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-snb5/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@i915_selftest@live_gt_timelines:
    - shard-tglb:         [PASS][23] -> [INCOMPLETE][24] ([i915#455])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb5/igt@i915_selftest@live_gt_timelines.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb3/igt@i915_selftest@live_gt_timelines.html

  * igt@i915_selftest@mock_sanitycheck:
    - shard-hsw:          [PASS][25] -> [DMESG-WARN][26] ([i915#747])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-hsw7/igt@i915_selftest@mock_sanitycheck.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-hsw5/igt@i915_selftest@mock_sanitycheck.html
    - shard-skl:          [PASS][27] -> [DMESG-WARN][28] ([i915#747])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl4/igt@i915_selftest@mock_sanitycheck.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl7/igt@i915_selftest@mock_sanitycheck.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([i915#54]) +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl7/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl7/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][31] -> [DMESG-WARN][32] ([i915#180]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-apl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-tglb:         [PASS][33] -> [INCOMPLETE][34] ([i915#435] / [i915#456] / [i915#460])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb6/igt@kms_fbcon_fbt@fbc-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb7/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [PASS][35] -> [INCOMPLETE][36] ([i915#61])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-hsw5/igt@kms_flip@flip-vs-suspend.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-hsw4/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt:
    - shard-tglb:         [PASS][37] -> [FAIL][38] ([i915#49]) +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render:
    - shard-skl:          [PASS][39] -> [FAIL][40] ([i915#49])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-skl:          [PASS][41] -> [INCOMPLETE][42] ([i915#648] / [i915#667])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl9/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl5/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-tglb:         [PASS][43] -> [INCOMPLETE][44] ([i915#456] / [i915#460])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][45] -> [FAIL][46] ([fdo#108145]) +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][47] -> [FAIL][48] ([fdo#108145] / [i915#265])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [PASS][49] -> [SKIP][50] ([fdo#109441])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb2/igt@kms_psr@psr2_dpms.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb3/igt@kms_psr@psr2_dpms.html

  * igt@perf_pmu@busy-accuracy-98-vcs1:
    - shard-iclb:         [PASS][51] -> [SKIP][52] ([fdo#112080]) +6 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb2/igt@perf_pmu@busy-accuracy-98-vcs1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb3/igt@perf_pmu@busy-accuracy-98-vcs1.html

  
#### Possible fixes ####

  * igt@gem_eio@hibernate:
    - shard-tglb:         [INCOMPLETE][53] ([i915#456]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb8/igt@gem_eio@hibernate.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb6/igt@gem_eio@hibernate.html

  * {igt@gem_exec_schedule@pi-userfault-bsd}:
    - shard-iclb:         [SKIP][55] ([i915#677]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb2/igt@gem_exec_schedule@pi-userfault-bsd.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb3/igt@gem_exec_schedule@pi-userfault-bsd.html

  * {igt@gem_exec_schedule@pi-userfault-bsd2}:
    - shard-iclb:         [SKIP][57] ([fdo#109276]) -> [PASS][58] +7 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb3/igt@gem_exec_schedule@pi-userfault-bsd2.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb2/igt@gem_exec_schedule@pi-userfault-bsd2.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][59] ([fdo#112146]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb2/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb3/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_exec_schedule@preempt-queue-contexts-chain-vebox:
    - shard-tglb:         [INCOMPLETE][61] ([fdo#111606] / [fdo#111677]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb6/igt@gem_exec_schedule@preempt-queue-contexts-chain-vebox.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb7/igt@gem_exec_schedule@preempt-queue-contexts-chain-vebox.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive:
    - shard-glk:          [TIMEOUT][63] ([i915#530]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-glk7/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-glk2/igt@gem_persistent_relocs@forked-faulting-reloc-thrash-inactive.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [DMESG-WARN][65] ([i915#180]) -> [PASS][66] +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-apl8/igt@gem_softpin@noreloc-s3.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-apl3/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [DMESG-WARN][67] ([fdo#111870]) -> [PASS][68] +1 similar issue
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-snb6/igt@gem_userptr_blits@sync-unmap.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-snb7/igt@gem_userptr_blits@sync-unmap.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-offscreen:
    - shard-hsw:          [DMESG-WARN][69] ([IGT#6]) -> [PASS][70] +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-hsw6/igt@kms_cursor_crc@pipe-a-cursor-256x256-offscreen.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-hsw1/igt@kms_cursor_crc@pipe-a-cursor-256x256-offscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding:
    - shard-apl:          [DMESG-WARN][71] ([IGT#6]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-apl3/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html

  * igt@kms_flip@2x-dpms-vs-vblank-race:
    - shard-hsw:          [DMESG-FAIL][73] ([i915#407] / [i915#44]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-hsw5/igt@kms_flip@2x-dpms-vs-vblank-race.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-hsw8/igt@kms_flip@2x-dpms-vs-vblank-race.html

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-hsw:          [DMESG-FAIL][75] ([i915#44]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-hsw5/igt@kms_flip@plain-flip-fb-recreate.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-hsw4/igt@kms_flip@plain-flip-fb-recreate.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [DMESG-WARN][77] ([i915#180]) -> [PASS][78] +2 similar issues
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move:
    - shard-tglb:         [INCOMPLETE][79] ([i915#474] / [i915#667]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-pwrite:
    - shard-tglb:         [FAIL][81] ([i915#49]) -> [PASS][82] +1 similar issue
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-pwrite.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-tglb:         [INCOMPLETE][83] ([i915#456] / [i915#460] / [i915#474]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render:
    - shard-iclb:         [INCOMPLETE][85] ([i915#123] / [i915#140]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [SKIP][87] ([fdo#109441]) -> [PASS][88] +1 similar issue
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb3/igt@kms_psr@psr2_suspend.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb2/igt@kms_psr@psr2_suspend.html

  * igt@perf@create-destroy-userspace-config:
    - shard-hsw:          [INCOMPLETE][89] ([i915#61]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-hsw6/igt@perf@create-destroy-userspace-config.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-hsw1/igt@perf@create-destroy-userspace-config.html

  * igt@perf_pmu@busy-no-semaphores-vcs1:
    - shard-iclb:         [SKIP][91] ([fdo#112080]) -> [PASS][92] +3 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-iclb3/igt@perf_pmu@busy-no-semaphores-vcs1.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-iclb2/igt@perf_pmu@busy-no-semaphores-vcs1.html

  
#### Warnings ####

  * igt@gem_tiled_blits@interruptible:
    - shard-hsw:          [FAIL][93] ([i915#832]) -> [FAIL][94] ([i915#818])
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-hsw6/igt@gem_tiled_blits@interruptible.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-hsw1/igt@gem_tiled_blits@interruptible.html

  * igt@i915_pm_rps@waitboost:
    - shard-tglb:         [FAIL][95] ([i915#39]) -> [FAIL][96] ([i915#413])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb5/igt@i915_pm_rps@waitboost.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb4/igt@i915_pm_rps@waitboost.html

  * igt@kms_atomic_transition@6x-modeset-transitions-nonblocking:
    - shard-tglb:         [SKIP][97] ([fdo#112016] / [fdo#112021]) -> [SKIP][98] ([fdo#112021])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-tglb3/igt@kms_atomic_transition@6x-modeset-transitions-nonblocking.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-tglb9/igt@kms_atomic_transition@6x-modeset-transitions-nonblocking.html

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-skl:          [INCOMPLETE][99] ([fdo#112347] / [fdo#112391] / [i915#648] / [i915#667]) -> [INCOMPLETE][100] ([fdo#112347] / [i915#648] / [i915#667])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7574/shard-skl9/igt@kms_plane@pixel-format-pipe-a-planes.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15787/shard-skl10/igt@kms_plane@pixel-format-pipe-a-planes.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#6]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/6
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
  [fdo#111606]: https://bugs.freedesktop.org/show_bug.cgi?id=111606
  [fdo#111677]: https://bugs.freedesktop.org/show_bug.cgi?id=111677
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [fdo#112016]: https://bugs.freedesktop.org/show_bug.cgi?id=112016
  [fdo#112021]: https://bugs.freedesktop.org/show_bug.cgi?id=112021
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [fdo#112347]: https://bugs.freedesktop.org/show_bug.cgi?id=112347
  [fdo#112391]: https://bugs.freedesktop.org/show_bug.cgi?id=112391
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#39]: https://gitlab.freedesktop.org/drm/intel/issues/39
  [i915#407]: https://gitlab.freedesktop.org/drm/intel/issues/407
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#435]: https://gitlab.freedesktop.org/drm/intel/issues/435
  [i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44
  [i915#455]: https://gitlab.freedesktop.org/drm/intel/issues/455
  [i915#456]: https://gitlab.freedesktop.org/drm/intel/issues/456
  [i915#460]: https://gitlab.freedesktop.org/drm/intel/issues/460
  [i915#474]: https://gitlab.freedesktop.org/drm/intel/issues/474
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#520]: https://gitlab.freedesktop.org/drm/intel/issues/520
  [i915#530]: https://gitlab.freedesktop.org/drm/intel/issues/530
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#648]: https://gitlab.freedesktop.org/drm/intel/issues/648
  [i915#667]: https://gitlab.freedesktop.org/drm/intel/issues/667
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#747]: https://gitlab.freedesktop.org/drm/intel/issues/747
  [i915#796]: https://gitlab.freedesktop.org/drm/intel/issues/796
  [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818
  [i915#832]: https://gitlab.freedesktop.org/drm/intel/issues/832


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7574 -> Patchwork_15787

  CI-20190529: 20190529
  CI_DRM_7574: 950244ca586c6f0efe243bf8c505c01ea5e579fa @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5349: 048f58513d8b8ec6bb307a939f0ac959bc0f0e10 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15787: 874c08fe92f2f8f25178d40f120502ec393057af @ 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_15787/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH resend 0/2] drm/connector: Add support for specifying panel_orientation on the kernel cmdline
  2019-12-16 11:51 ` [Intel-gfx] " Hans de Goede
@ 2020-01-01 17:04   ` Hans de Goede
  -1 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2020-01-01 17:04 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: intel-gfx, dri-devel

Hi all,

Ping? it would be really nice if we can get these last 2 patches from
my series for specifying panel_orientation on the kernel cmdline upstream.

If someone can review the first patch (the second one has already
been reviewed) then that would be great.

Thanks & Regards,

Hans



On 16-12-2019 12:51, Hans de Goede wrote:
> Hi All,
> 
> This is a resend of the last 2 remaining patches of my series for adding
> support for specifying panel_orientation on the kernel cmdline.
> 
> I've already pushed the other 11 patches which were mostly cleanups /
> bug-fixes to the cmdline-parsing code and where all acked by Maxime
> to drm-misc-next.
> 
> The first patch of these 2 still needs to be reviewed, if someone can
> review it that would be great.
> 
> Once this is reviewed the question becomes how to merge this, both
> patches are mostly drm-misc material and the second patch depends on
> the changes I just pushed to drm-misc-next, so this series should
> probably be pushed to drm-misc-next, but it also makes some changes
> to i915 code. I've checked and there is no conflict with these patches
> as they apply to drm-misc-next vs drm-intel-next-queued (atm). So it
> should be fine to push these 2 patches to drm-misc-next and then do a
> back-merge to drm-intel-next-queued.
> 
> Regards,
> 
> Hans
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH resend 0/2] drm/connector: Add support for specifying panel_orientation on the kernel cmdline
@ 2020-01-01 17:04   ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2020-01-01 17:04 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Daniel Vetter
  Cc: intel-gfx, dri-devel

Hi all,

Ping? it would be really nice if we can get these last 2 patches from
my series for specifying panel_orientation on the kernel cmdline upstream.

If someone can review the first patch (the second one has already
been reviewed) then that would be great.

Thanks & Regards,

Hans



On 16-12-2019 12:51, Hans de Goede wrote:
> Hi All,
> 
> This is a resend of the last 2 remaining patches of my series for adding
> support for specifying panel_orientation on the kernel cmdline.
> 
> I've already pushed the other 11 patches which were mostly cleanups /
> bug-fixes to the cmdline-parsing code and where all acked by Maxime
> to drm-misc-next.
> 
> The first patch of these 2 still needs to be reviewed, if someone can
> review it that would be great.
> 
> Once this is reviewed the question becomes how to merge this, both
> patches are mostly drm-misc material and the second patch depends on
> the changes I just pushed to drm-misc-next, so this series should
> probably be pushed to drm-misc-next, but it also makes some changes
> to i915 code. I've checked and there is no conflict with these patches
> as they apply to drm-misc-next vs drm-intel-next-queued (atm). So it
> should be fine to push these 2 patches to drm-misc-next and then do a
> back-merge to drm-intel-next-queued.
> 
> Regards,
> 
> Hans
> 

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

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

* Re: [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
  2019-12-16 11:51   ` [Intel-gfx] " Hans de Goede
@ 2020-01-02 18:17     ` Rodrigo Vivi
  -1 siblings, 0 replies; 21+ messages in thread
From: Rodrigo Vivi @ 2020-01-02 18:17 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx, Derek Basehore, dri-devel

On Mon, Dec 16, 2019 at 12:51:57PM +0100, Hans de Goede wrote:
> From: Derek Basehore <dbasehore@chromium.org>
> 
> Not every platform needs quirk detection for panel orientation, so
> split the drm_connector_init_panel_orientation_property into two
> functions. One for platforms without the need for quirks, and the
> other for platforms that need quirks.
> 
> Hans de Goede (changes in v2):
> 
> Rename the function from drm_connector_init_panel_orientation_property
> to drm_connector_set_panel_orientation[_with_quirk] and pass in the
> panel-orientation to set.
> 
> Beside the rename, also make the function set the passed in value
> only once, if the value was set before (to a value other then
> DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
> 
> This change is preparation for allowing the user to override the
> panel-orientation for any connector from the kernel commandline.
> When the panel-orientation is overridden this way, then we must ignore
> the panel-orientation detection done by the driver.
> 
> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
>  drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
>  drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
>  drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
>  include/drm/drm_connector.h             |  9 ++-
>  5 files changed, 71 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 0965632008a9..f4fa5c59717d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>   *	coordinates, so if userspace rotates the picture to adjust for
>   *	the orientation it must also apply the same transformation to the
>   *	touchscreen input coordinates. This property is initialized by calling
> - *	drm_connector_init_panel_orientation_property().
> + *	drm_connector_set_panel_orientation() or
> + *	drm_connector_set_panel_orientation_with_quirk()

do we have a better name than quirks for these dsi modes?

>   *
>   * scaling mode:
>   *	This property defines how a non-native mode is upscaled to the native
> @@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
>  EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
>  
>  /**
> - * drm_connector_init_panel_orientation_property -
> - *	initialize the connecters panel_orientation property
> - * @connector: connector for which to init the panel-orientation property.
> - * @width: width in pixels of the panel, used for panel quirk detection
> - * @height: height in pixels of the panel, used for panel quirk detection
> + * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
> + * @connector: connector for which to set the panel-orientation property.
> + * @panel_orientation: drm_panel_orientation value to set
> + *
> + * This function sets the connector's panel_orientation and attaches
> + * a "panel orientation" property to the connector.
>   *
> - * This function should only be called for built-in panels, after setting
> - * connector->display_info.panel_orientation first (if known).
> + * Calling this function on a connector where the panel_orientation has
> + * already been set is a no-op (e.g. the orientation has been overridden with
> + * a kernel commandline option).
>   *
> - * This function will check for platform specific (e.g. DMI based) quirks
> - * overriding display_info.panel_orientation first, then if panel_orientation
> - * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
> - * "panel orientation" property to the connector.
> + * It is allowed to call this function with a panel_orientation of
> + * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
>   *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
> -int drm_connector_init_panel_orientation_property(
> -	struct drm_connector *connector, int width, int height)
> +int drm_connector_set_panel_orientation(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_display_info *info = &connector->display_info;
>  	struct drm_property *prop;
> -	int orientation_quirk;
>  
> -	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> -	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> -		info->panel_orientation = orientation_quirk;
> +	/* Already set? */
> +	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		return 0;

What happens on the scenario of ICL DSI here?
In case we had something badly set we just respect the bad choices?
Any way to at least have some kind of warn when we tried the dsi mode but
it had already been set?

>  
> -	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +	/* Don't attach the property if the orientation is unknown */
> +	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>  		return 0;
>  
> +	info->panel_orientation = panel_orientation;
> +
>  	prop = dev->mode_config.panel_orientation_property;
>  	if (!prop) {
>  		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> @@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
>  				   info->panel_orientation);
>  	return 0;
>  }
> -EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
> +EXPORT_SYMBOL(drm_connector_set_panel_orientation);
> +
> +/**
> + * drm_connector_set_panel_orientation_with_quirk -
> + *	set the connecter's panel_orientation after checking for quirks
> + * @connector: connector for which to init the panel-orientation property.
> + * @panel_orientation: drm_panel_orientation value to set
> + * @width: width in pixels of the panel, used for panel quirk detection
> + * @height: height in pixels of the panel, used for panel quirk detection
> + *
> + * Like drm_connector_set_panel_orientation(), but with a check for platform
> + * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_set_panel_orientation_with_quirk(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation,
> +	int width, int height)
> +{
> +	int orientation_quirk;
> +
> +	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> +	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		panel_orientation = orientation_quirk;
> +
> +	return drm_connector_set_panel_orientation(connector,
> +						   panel_orientation);
> +}
> +EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
>  
>  int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>  				    struct drm_property *property,
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..8cd51cf67d02 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1536,9 +1536,8 @@ static void icl_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);
> -	drm_connector_init_panel_orientation_property(&connector->base,
> +	drm_connector_set_panel_orientation_with_quirk(&connector->base,
> +				intel_dsi_get_panel_orientation(connector),
>  				connector->panel.fixed_mode->hdisplay,
>  				connector->panel.fixed_mode->vdisplay);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index aa515261cb9f..9f4425f8d0ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>  	intel_panel_setup_backlight(connector, pipe);
>  
> -	if (fixed_mode)
> -		drm_connector_init_panel_orientation_property(
> -			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
> +	if (fixed_mode) {
> +		/* We do not know the orientation, but their might be a quirk */
> +		drm_connector_set_panel_orientation_with_quirk(connector,
> +				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
> +				fixed_mode->hdisplay, fixed_mode->vdisplay);
> +	}
>  
>  	return true;
>  
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 50064cde0724..a0de8e70e426 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1632,10 +1632,9 @@ 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);
> -		drm_connector_init_panel_orientation_property(
> +		drm_connector_set_panel_orientation_with_quirk(
>  				&connector->base,
> +				vlv_dsi_get_panel_orientation(connector),
>  				connector->panel.fixed_mode->hdisplay,
>  				connector->panel.fixed_mode->vdisplay);
>  	}
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 221910948b37..2113500b4075 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>  					    uint64_t link_status);
>  void drm_connector_set_vrr_capable_property(
>  		struct drm_connector *connector, bool capable);
> -int drm_connector_init_panel_orientation_property(
> -	struct drm_connector *connector, int width, int height);
> +int drm_connector_set_panel_orientation(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation);
> +int drm_connector_set_panel_orientation_with_quirk(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation,
> +	int width, int height);
>  int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>  					  int min, int max);
>  
> -- 
> 2.23.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
@ 2020-01-02 18:17     ` Rodrigo Vivi
  0 siblings, 0 replies; 21+ messages in thread
From: Rodrigo Vivi @ 2020-01-02 18:17 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx, Derek Basehore, dri-devel

On Mon, Dec 16, 2019 at 12:51:57PM +0100, Hans de Goede wrote:
> From: Derek Basehore <dbasehore@chromium.org>
> 
> Not every platform needs quirk detection for panel orientation, so
> split the drm_connector_init_panel_orientation_property into two
> functions. One for platforms without the need for quirks, and the
> other for platforms that need quirks.
> 
> Hans de Goede (changes in v2):
> 
> Rename the function from drm_connector_init_panel_orientation_property
> to drm_connector_set_panel_orientation[_with_quirk] and pass in the
> panel-orientation to set.
> 
> Beside the rename, also make the function set the passed in value
> only once, if the value was set before (to a value other then
> DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
> 
> This change is preparation for allowing the user to override the
> panel-orientation for any connector from the kernel commandline.
> When the panel-orientation is overridden this way, then we must ignore
> the panel-orientation detection done by the driver.
> 
> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
>  drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
>  drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
>  drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
>  include/drm/drm_connector.h             |  9 ++-
>  5 files changed, 71 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 0965632008a9..f4fa5c59717d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>   *	coordinates, so if userspace rotates the picture to adjust for
>   *	the orientation it must also apply the same transformation to the
>   *	touchscreen input coordinates. This property is initialized by calling
> - *	drm_connector_init_panel_orientation_property().
> + *	drm_connector_set_panel_orientation() or
> + *	drm_connector_set_panel_orientation_with_quirk()

do we have a better name than quirks for these dsi modes?

>   *
>   * scaling mode:
>   *	This property defines how a non-native mode is upscaled to the native
> @@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
>  EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
>  
>  /**
> - * drm_connector_init_panel_orientation_property -
> - *	initialize the connecters panel_orientation property
> - * @connector: connector for which to init the panel-orientation property.
> - * @width: width in pixels of the panel, used for panel quirk detection
> - * @height: height in pixels of the panel, used for panel quirk detection
> + * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
> + * @connector: connector for which to set the panel-orientation property.
> + * @panel_orientation: drm_panel_orientation value to set
> + *
> + * This function sets the connector's panel_orientation and attaches
> + * a "panel orientation" property to the connector.
>   *
> - * This function should only be called for built-in panels, after setting
> - * connector->display_info.panel_orientation first (if known).
> + * Calling this function on a connector where the panel_orientation has
> + * already been set is a no-op (e.g. the orientation has been overridden with
> + * a kernel commandline option).
>   *
> - * This function will check for platform specific (e.g. DMI based) quirks
> - * overriding display_info.panel_orientation first, then if panel_orientation
> - * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
> - * "panel orientation" property to the connector.
> + * It is allowed to call this function with a panel_orientation of
> + * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
>   *
>   * Returns:
>   * Zero on success, negative errno on failure.
>   */
> -int drm_connector_init_panel_orientation_property(
> -	struct drm_connector *connector, int width, int height)
> +int drm_connector_set_panel_orientation(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_display_info *info = &connector->display_info;
>  	struct drm_property *prop;
> -	int orientation_quirk;
>  
> -	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> -	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> -		info->panel_orientation = orientation_quirk;
> +	/* Already set? */
> +	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		return 0;

What happens on the scenario of ICL DSI here?
In case we had something badly set we just respect the bad choices?
Any way to at least have some kind of warn when we tried the dsi mode but
it had already been set?

>  
> -	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +	/* Don't attach the property if the orientation is unknown */
> +	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>  		return 0;
>  
> +	info->panel_orientation = panel_orientation;
> +
>  	prop = dev->mode_config.panel_orientation_property;
>  	if (!prop) {
>  		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> @@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
>  				   info->panel_orientation);
>  	return 0;
>  }
> -EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
> +EXPORT_SYMBOL(drm_connector_set_panel_orientation);
> +
> +/**
> + * drm_connector_set_panel_orientation_with_quirk -
> + *	set the connecter's panel_orientation after checking for quirks
> + * @connector: connector for which to init the panel-orientation property.
> + * @panel_orientation: drm_panel_orientation value to set
> + * @width: width in pixels of the panel, used for panel quirk detection
> + * @height: height in pixels of the panel, used for panel quirk detection
> + *
> + * Like drm_connector_set_panel_orientation(), but with a check for platform
> + * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_set_panel_orientation_with_quirk(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation,
> +	int width, int height)
> +{
> +	int orientation_quirk;
> +
> +	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> +	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> +		panel_orientation = orientation_quirk;
> +
> +	return drm_connector_set_panel_orientation(connector,
> +						   panel_orientation);
> +}
> +EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
>  
>  int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>  				    struct drm_property *property,
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..8cd51cf67d02 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1536,9 +1536,8 @@ static void icl_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);
> -	drm_connector_init_panel_orientation_property(&connector->base,
> +	drm_connector_set_panel_orientation_with_quirk(&connector->base,
> +				intel_dsi_get_panel_orientation(connector),
>  				connector->panel.fixed_mode->hdisplay,
>  				connector->panel.fixed_mode->vdisplay);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index aa515261cb9f..9f4425f8d0ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>  	intel_panel_setup_backlight(connector, pipe);
>  
> -	if (fixed_mode)
> -		drm_connector_init_panel_orientation_property(
> -			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
> +	if (fixed_mode) {
> +		/* We do not know the orientation, but their might be a quirk */
> +		drm_connector_set_panel_orientation_with_quirk(connector,
> +				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
> +				fixed_mode->hdisplay, fixed_mode->vdisplay);
> +	}
>  
>  	return true;
>  
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 50064cde0724..a0de8e70e426 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1632,10 +1632,9 @@ 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);
> -		drm_connector_init_panel_orientation_property(
> +		drm_connector_set_panel_orientation_with_quirk(
>  				&connector->base,
> +				vlv_dsi_get_panel_orientation(connector),
>  				connector->panel.fixed_mode->hdisplay,
>  				connector->panel.fixed_mode->vdisplay);
>  	}
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 221910948b37..2113500b4075 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>  					    uint64_t link_status);
>  void drm_connector_set_vrr_capable_property(
>  		struct drm_connector *connector, bool capable);
> -int drm_connector_init_panel_orientation_property(
> -	struct drm_connector *connector, int width, int height);
> +int drm_connector_set_panel_orientation(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation);
> +int drm_connector_set_panel_orientation_with_quirk(
> +	struct drm_connector *connector,
> +	enum drm_panel_orientation panel_orientation,
> +	int width, int height);
>  int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>  					  int min, int max);
>  
> -- 
> 2.23.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
  2020-01-02 18:17     ` [Intel-gfx] " Rodrigo Vivi
@ 2020-01-02 21:35       ` Hans de Goede
  -1 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2020-01-02 21:35 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Derek Basehore, dri-devel

Hi Rodrigo,

Thank you for the review.

On 02-01-2020 19:17, Rodrigo Vivi wrote:
> On Mon, Dec 16, 2019 at 12:51:57PM +0100, Hans de Goede wrote:
>> From: Derek Basehore <dbasehore@chromium.org>
>>
>> Not every platform needs quirk detection for panel orientation, so
>> split the drm_connector_init_panel_orientation_property into two
>> functions. One for platforms without the need for quirks, and the
>> other for platforms that need quirks.
>>
>> Hans de Goede (changes in v2):
>>
>> Rename the function from drm_connector_init_panel_orientation_property
>> to drm_connector_set_panel_orientation[_with_quirk] and pass in the
>> panel-orientation to set.
>>
>> Beside the rename, also make the function set the passed in value
>> only once, if the value was set before (to a value other then
>> DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
>>
>> This change is preparation for allowing the user to override the
>> panel-orientation for any connector from the kernel commandline.
>> When the panel-orientation is overridden this way, then we must ignore
>> the panel-orientation detection done by the driver.
>>
>> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
>>   drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
>>   drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
>>   drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
>>   include/drm/drm_connector.h             |  9 ++-
>>   5 files changed, 71 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 0965632008a9..f4fa5c59717d 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>>    *	coordinates, so if userspace rotates the picture to adjust for
>>    *	the orientation it must also apply the same transformation to the
>>    *	touchscreen input coordinates. This property is initialized by calling
>> - *	drm_connector_init_panel_orientation_property().
>> + *	drm_connector_set_panel_orientation() or
>> + *	drm_connector_set_panel_orientation_with_quirk()
> 
> do we have a better name than quirks for these dsi modes?

The difference between the 2 functions is that the second one calls
drm_get_panel_orientation_quirk() and that if that returns a valid
orientation it overwrites the passed orientation with the return value
from drm_get_panel_orientation_quirk(), so the name seems correct.

As for drm_get_panel_orientation_quirk() itself that currently is only
defined on x86 (it is a static inline no-op elsewhere) and it used
DMI string matching to check for a model specific quirk. So again the
name seems correct.

>>    *
>>    * scaling mode:
>>    *	This property defines how a non-native mode is upscaled to the native
>> @@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
>>   EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
>>   
>>   /**
>> - * drm_connector_init_panel_orientation_property -
>> - *	initialize the connecters panel_orientation property
>> - * @connector: connector for which to init the panel-orientation property.
>> - * @width: width in pixels of the panel, used for panel quirk detection
>> - * @height: height in pixels of the panel, used for panel quirk detection
>> + * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
>> + * @connector: connector for which to set the panel-orientation property.
>> + * @panel_orientation: drm_panel_orientation value to set
>> + *
>> + * This function sets the connector's panel_orientation and attaches
>> + * a "panel orientation" property to the connector.
>>    *
>> - * This function should only be called for built-in panels, after setting
>> - * connector->display_info.panel_orientation first (if known).
>> + * Calling this function on a connector where the panel_orientation has
>> + * already been set is a no-op (e.g. the orientation has been overridden with
>> + * a kernel commandline option).
>>    *
>> - * This function will check for platform specific (e.g. DMI based) quirks
>> - * overriding display_info.panel_orientation first, then if panel_orientation
>> - * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
>> - * "panel orientation" property to the connector.
>> + * It is allowed to call this function with a panel_orientation of
>> + * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
>>    *
>>    * Returns:
>>    * Zero on success, negative errno on failure.
>>    */
>> -int drm_connector_init_panel_orientation_property(
>> -	struct drm_connector *connector, int width, int height)
>> +int drm_connector_set_panel_orientation(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation)
>>   {
>>   	struct drm_device *dev = connector->dev;
>>   	struct drm_display_info *info = &connector->display_info;
>>   	struct drm_property *prop;
>> -	int orientation_quirk;
>>   
>> -	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
>> -	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> -		info->panel_orientation = orientation_quirk;
>> +	/* Already set? */
>> +	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> +		return 0;
> 
> What happens on the scenario of ICL DSI here?
> In case we had something badly set we just respect the bad choices?

The only path where the value gets set twice is when it has been set
from the kernel commandline (which gets hooked up in the second patch),
and yes in that case we honor what the user passed in as value, honering
the user passed value (which gets processed first) is the whole reason
for the "Already set?" check.

> Any way to at least have some kind of warn when we tried the dsi mode but
> it had already been set?

The 2nd patch prints a message when a value is passed on the kernel commandline:

+	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
+		DRM_INFO("setting connector %s panel_orientation to %d\n",
+			 connector->name, mode->panel_orientation);


Regards,

Hans



>>   
>> -	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> +	/* Don't attach the property if the orientation is unknown */
>> +	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>>   		return 0;
>>   
>> +	info->panel_orientation = panel_orientation;
>> +
>>   	prop = dev->mode_config.panel_orientation_property;
>>   	if (!prop) {
>>   		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
>> @@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
>>   				   info->panel_orientation);
>>   	return 0;
>>   }
>> -EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
>> +EXPORT_SYMBOL(drm_connector_set_panel_orientation);
>> +
>> +/**
>> + * drm_connector_set_panel_orientation_with_quirk -
>> + *	set the connecter's panel_orientation after checking for quirks
>> + * @connector: connector for which to init the panel-orientation property.
>> + * @panel_orientation: drm_panel_orientation value to set
>> + * @width: width in pixels of the panel, used for panel quirk detection
>> + * @height: height in pixels of the panel, used for panel quirk detection
>> + *
>> + * Like drm_connector_set_panel_orientation(), but with a check for platform
>> + * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int drm_connector_set_panel_orientation_with_quirk(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation,
>> +	int width, int height)
>> +{
>> +	int orientation_quirk;
>> +
>> +	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
>> +	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> +		panel_orientation = orientation_quirk;
>> +
>> +	return drm_connector_set_panel_orientation(connector,
>> +						   panel_orientation);
>> +}
>> +EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
>>   
>>   int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>>   				    struct drm_property *property,
>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
>> index 6e398c33a524..8cd51cf67d02 100644
>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> @@ -1536,9 +1536,8 @@ static void icl_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);
>> -	drm_connector_init_panel_orientation_property(&connector->base,
>> +	drm_connector_set_panel_orientation_with_quirk(&connector->base,
>> +				intel_dsi_get_panel_orientation(connector),
>>   				connector->panel.fixed_mode->hdisplay,
>>   				connector->panel.fixed_mode->vdisplay);
>>   }
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index aa515261cb9f..9f4425f8d0ac 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>   	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>>   	intel_panel_setup_backlight(connector, pipe);
>>   
>> -	if (fixed_mode)
>> -		drm_connector_init_panel_orientation_property(
>> -			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
>> +	if (fixed_mode) {
>> +		/* We do not know the orientation, but their might be a quirk */
>> +		drm_connector_set_panel_orientation_with_quirk(connector,
>> +				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
>> +				fixed_mode->hdisplay, fixed_mode->vdisplay);
>> +	}
>>   
>>   	return true;
>>   
>> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> index 50064cde0724..a0de8e70e426 100644
>> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> @@ -1632,10 +1632,9 @@ 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);
>> -		drm_connector_init_panel_orientation_property(
>> +		drm_connector_set_panel_orientation_with_quirk(
>>   				&connector->base,
>> +				vlv_dsi_get_panel_orientation(connector),
>>   				connector->panel.fixed_mode->hdisplay,
>>   				connector->panel.fixed_mode->vdisplay);
>>   	}
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 221910948b37..2113500b4075 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>>   					    uint64_t link_status);
>>   void drm_connector_set_vrr_capable_property(
>>   		struct drm_connector *connector, bool capable);
>> -int drm_connector_init_panel_orientation_property(
>> -	struct drm_connector *connector, int width, int height);
>> +int drm_connector_set_panel_orientation(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation);
>> +int drm_connector_set_panel_orientation_with_quirk(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation,
>> +	int width, int height);
>>   int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>>   					  int min, int max);
>>   
>> -- 
>> 2.23.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
@ 2020-01-02 21:35       ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2020-01-02 21:35 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Derek Basehore, dri-devel

Hi Rodrigo,

Thank you for the review.

On 02-01-2020 19:17, Rodrigo Vivi wrote:
> On Mon, Dec 16, 2019 at 12:51:57PM +0100, Hans de Goede wrote:
>> From: Derek Basehore <dbasehore@chromium.org>
>>
>> Not every platform needs quirk detection for panel orientation, so
>> split the drm_connector_init_panel_orientation_property into two
>> functions. One for platforms without the need for quirks, and the
>> other for platforms that need quirks.
>>
>> Hans de Goede (changes in v2):
>>
>> Rename the function from drm_connector_init_panel_orientation_property
>> to drm_connector_set_panel_orientation[_with_quirk] and pass in the
>> panel-orientation to set.
>>
>> Beside the rename, also make the function set the passed in value
>> only once, if the value was set before (to a value other then
>> DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
>>
>> This change is preparation for allowing the user to override the
>> panel-orientation for any connector from the kernel commandline.
>> When the panel-orientation is overridden this way, then we must ignore
>> the panel-orientation detection done by the driver.
>>
>> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
>>   drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
>>   drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
>>   drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
>>   include/drm/drm_connector.h             |  9 ++-
>>   5 files changed, 71 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 0965632008a9..f4fa5c59717d 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>>    *	coordinates, so if userspace rotates the picture to adjust for
>>    *	the orientation it must also apply the same transformation to the
>>    *	touchscreen input coordinates. This property is initialized by calling
>> - *	drm_connector_init_panel_orientation_property().
>> + *	drm_connector_set_panel_orientation() or
>> + *	drm_connector_set_panel_orientation_with_quirk()
> 
> do we have a better name than quirks for these dsi modes?

The difference between the 2 functions is that the second one calls
drm_get_panel_orientation_quirk() and that if that returns a valid
orientation it overwrites the passed orientation with the return value
from drm_get_panel_orientation_quirk(), so the name seems correct.

As for drm_get_panel_orientation_quirk() itself that currently is only
defined on x86 (it is a static inline no-op elsewhere) and it used
DMI string matching to check for a model specific quirk. So again the
name seems correct.

>>    *
>>    * scaling mode:
>>    *	This property defines how a non-native mode is upscaled to the native
>> @@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
>>   EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
>>   
>>   /**
>> - * drm_connector_init_panel_orientation_property -
>> - *	initialize the connecters panel_orientation property
>> - * @connector: connector for which to init the panel-orientation property.
>> - * @width: width in pixels of the panel, used for panel quirk detection
>> - * @height: height in pixels of the panel, used for panel quirk detection
>> + * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
>> + * @connector: connector for which to set the panel-orientation property.
>> + * @panel_orientation: drm_panel_orientation value to set
>> + *
>> + * This function sets the connector's panel_orientation and attaches
>> + * a "panel orientation" property to the connector.
>>    *
>> - * This function should only be called for built-in panels, after setting
>> - * connector->display_info.panel_orientation first (if known).
>> + * Calling this function on a connector where the panel_orientation has
>> + * already been set is a no-op (e.g. the orientation has been overridden with
>> + * a kernel commandline option).
>>    *
>> - * This function will check for platform specific (e.g. DMI based) quirks
>> - * overriding display_info.panel_orientation first, then if panel_orientation
>> - * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
>> - * "panel orientation" property to the connector.
>> + * It is allowed to call this function with a panel_orientation of
>> + * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
>>    *
>>    * Returns:
>>    * Zero on success, negative errno on failure.
>>    */
>> -int drm_connector_init_panel_orientation_property(
>> -	struct drm_connector *connector, int width, int height)
>> +int drm_connector_set_panel_orientation(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation)
>>   {
>>   	struct drm_device *dev = connector->dev;
>>   	struct drm_display_info *info = &connector->display_info;
>>   	struct drm_property *prop;
>> -	int orientation_quirk;
>>   
>> -	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
>> -	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> -		info->panel_orientation = orientation_quirk;
>> +	/* Already set? */
>> +	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> +		return 0;
> 
> What happens on the scenario of ICL DSI here?
> In case we had something badly set we just respect the bad choices?

The only path where the value gets set twice is when it has been set
from the kernel commandline (which gets hooked up in the second patch),
and yes in that case we honor what the user passed in as value, honering
the user passed value (which gets processed first) is the whole reason
for the "Already set?" check.

> Any way to at least have some kind of warn when we tried the dsi mode but
> it had already been set?

The 2nd patch prints a message when a value is passed on the kernel commandline:

+	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
+		DRM_INFO("setting connector %s panel_orientation to %d\n",
+			 connector->name, mode->panel_orientation);


Regards,

Hans



>>   
>> -	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> +	/* Don't attach the property if the orientation is unknown */
>> +	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>>   		return 0;
>>   
>> +	info->panel_orientation = panel_orientation;
>> +
>>   	prop = dev->mode_config.panel_orientation_property;
>>   	if (!prop) {
>>   		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
>> @@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
>>   				   info->panel_orientation);
>>   	return 0;
>>   }
>> -EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
>> +EXPORT_SYMBOL(drm_connector_set_panel_orientation);
>> +
>> +/**
>> + * drm_connector_set_panel_orientation_with_quirk -
>> + *	set the connecter's panel_orientation after checking for quirks
>> + * @connector: connector for which to init the panel-orientation property.
>> + * @panel_orientation: drm_panel_orientation value to set
>> + * @width: width in pixels of the panel, used for panel quirk detection
>> + * @height: height in pixels of the panel, used for panel quirk detection
>> + *
>> + * Like drm_connector_set_panel_orientation(), but with a check for platform
>> + * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int drm_connector_set_panel_orientation_with_quirk(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation,
>> +	int width, int height)
>> +{
>> +	int orientation_quirk;
>> +
>> +	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
>> +	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
>> +		panel_orientation = orientation_quirk;
>> +
>> +	return drm_connector_set_panel_orientation(connector,
>> +						   panel_orientation);
>> +}
>> +EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
>>   
>>   int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>>   				    struct drm_property *property,
>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
>> index 6e398c33a524..8cd51cf67d02 100644
>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> @@ -1536,9 +1536,8 @@ static void icl_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);
>> -	drm_connector_init_panel_orientation_property(&connector->base,
>> +	drm_connector_set_panel_orientation_with_quirk(&connector->base,
>> +				intel_dsi_get_panel_orientation(connector),
>>   				connector->panel.fixed_mode->hdisplay,
>>   				connector->panel.fixed_mode->vdisplay);
>>   }
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index aa515261cb9f..9f4425f8d0ac 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>   	intel_connector->panel.backlight.power = intel_edp_backlight_power;
>>   	intel_panel_setup_backlight(connector, pipe);
>>   
>> -	if (fixed_mode)
>> -		drm_connector_init_panel_orientation_property(
>> -			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
>> +	if (fixed_mode) {
>> +		/* We do not know the orientation, but their might be a quirk */
>> +		drm_connector_set_panel_orientation_with_quirk(connector,
>> +				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
>> +				fixed_mode->hdisplay, fixed_mode->vdisplay);
>> +	}
>>   
>>   	return true;
>>   
>> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> index 50064cde0724..a0de8e70e426 100644
>> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
>> @@ -1632,10 +1632,9 @@ 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);
>> -		drm_connector_init_panel_orientation_property(
>> +		drm_connector_set_panel_orientation_with_quirk(
>>   				&connector->base,
>> +				vlv_dsi_get_panel_orientation(connector),
>>   				connector->panel.fixed_mode->hdisplay,
>>   				connector->panel.fixed_mode->vdisplay);
>>   	}
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 221910948b37..2113500b4075 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>>   					    uint64_t link_status);
>>   void drm_connector_set_vrr_capable_property(
>>   		struct drm_connector *connector, bool capable);
>> -int drm_connector_init_panel_orientation_property(
>> -	struct drm_connector *connector, int width, int height);
>> +int drm_connector_set_panel_orientation(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation);
>> +int drm_connector_set_panel_orientation_with_quirk(
>> +	struct drm_connector *connector,
>> +	enum drm_panel_orientation panel_orientation,
>> +	int width, int height);
>>   int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>>   					  int min, int max);
>>   
>> -- 
>> 2.23.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

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

* Re: [Intel-gfx] [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
  2020-01-02 21:35       ` [Intel-gfx] " Hans de Goede
@ 2020-01-03 19:37         ` Rodrigo Vivi
  -1 siblings, 0 replies; 21+ messages in thread
From: Rodrigo Vivi @ 2020-01-03 19:37 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx, Derek Basehore, dri-devel

On Thu, Jan 02, 2020 at 10:35:28PM +0100, Hans de Goede wrote:
> Hi Rodrigo,
> 
> Thank you for the review.
> 
> On 02-01-2020 19:17, Rodrigo Vivi wrote:
> > On Mon, Dec 16, 2019 at 12:51:57PM +0100, Hans de Goede wrote:
> > > From: Derek Basehore <dbasehore@chromium.org>
> > > 
> > > Not every platform needs quirk detection for panel orientation, so
> > > split the drm_connector_init_panel_orientation_property into two
> > > functions. One for platforms without the need for quirks, and the
> > > other for platforms that need quirks.
> > > 
> > > Hans de Goede (changes in v2):
> > > 
> > > Rename the function from drm_connector_init_panel_orientation_property
> > > to drm_connector_set_panel_orientation[_with_quirk] and pass in the
> > > panel-orientation to set.
> > > 
> > > Beside the rename, also make the function set the passed in value
> > > only once, if the value was set before (to a value other then
> > > DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
> > > 
> > > This change is preparation for allowing the user to override the
> > > panel-orientation for any connector from the kernel commandline.
> > > When the panel-orientation is overridden this way, then we must ignore
> > > the panel-orientation detection done by the driver.
> > > 
> > > Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >   drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
> > >   drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
> > >   drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
> > >   drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
> > >   include/drm/drm_connector.h             |  9 ++-
> > >   5 files changed, 71 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 0965632008a9..f4fa5c59717d 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> > >    *	coordinates, so if userspace rotates the picture to adjust for
> > >    *	the orientation it must also apply the same transformation to the
> > >    *	touchscreen input coordinates. This property is initialized by calling
> > > - *	drm_connector_init_panel_orientation_property().
> > > + *	drm_connector_set_panel_orientation() or
> > > + *	drm_connector_set_panel_orientation_with_quirk()
> > 
> > do we have a better name than quirks for these dsi modes?
> 
> The difference between the 2 functions is that the second one calls
> drm_get_panel_orientation_quirk() and that if that returns a valid
> orientation it overwrites the passed orientation with the return value
> from drm_get_panel_orientation_quirk(), so the name seems correct.
> 
> As for drm_get_panel_orientation_quirk() itself that currently is only
> defined on x86 (it is a static inline no-op elsewhere) and it used
> DMI string matching to check for a model specific quirk. So again the
> name seems correct.
> 
> > >    *
> > >    * scaling mode:
> > >    *	This property defines how a non-native mode is upscaled to the native
> > > @@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
> > >   EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
> > >   /**
> > > - * drm_connector_init_panel_orientation_property -
> > > - *	initialize the connecters panel_orientation property
> > > - * @connector: connector for which to init the panel-orientation property.
> > > - * @width: width in pixels of the panel, used for panel quirk detection
> > > - * @height: height in pixels of the panel, used for panel quirk detection
> > > + * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
> > > + * @connector: connector for which to set the panel-orientation property.
> > > + * @panel_orientation: drm_panel_orientation value to set
> > > + *
> > > + * This function sets the connector's panel_orientation and attaches
> > > + * a "panel orientation" property to the connector.
> > >    *
> > > - * This function should only be called for built-in panels, after setting
> > > - * connector->display_info.panel_orientation first (if known).
> > > + * Calling this function on a connector where the panel_orientation has
> > > + * already been set is a no-op (e.g. the orientation has been overridden with
> > > + * a kernel commandline option).
> > >    *
> > > - * This function will check for platform specific (e.g. DMI based) quirks
> > > - * overriding display_info.panel_orientation first, then if panel_orientation
> > > - * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
> > > - * "panel orientation" property to the connector.
> > > + * It is allowed to call this function with a panel_orientation of
> > > + * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
> > >    *
> > >    * Returns:
> > >    * Zero on success, negative errno on failure.
> > >    */
> > > -int drm_connector_init_panel_orientation_property(
> > > -	struct drm_connector *connector, int width, int height)
> > > +int drm_connector_set_panel_orientation(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation)
> > >   {
> > >   	struct drm_device *dev = connector->dev;
> > >   	struct drm_display_info *info = &connector->display_info;
> > >   	struct drm_property *prop;
> > > -	int orientation_quirk;
> > > -	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> > > -	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > -		info->panel_orientation = orientation_quirk;
> > > +	/* Already set? */
> > > +	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > +		return 0;
> > 
> > What happens on the scenario of ICL DSI here?
> > In case we had something badly set we just respect the bad choices?
> 
> The only path where the value gets set twice is when it has been set
> from the kernel commandline (which gets hooked up in the second patch),
> and yes in that case we honor what the user passed in as value, honering
> the user passed value (which gets processed first) is the whole reason
> for the "Already set?" check.
> 
> > Any way to at least have some kind of warn when we tried the dsi mode but
> > it had already been set?
> 
> The 2nd patch prints a message when a value is passed on the kernel commandline:
> 
> +	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
> +		DRM_INFO("setting connector %s panel_orientation to %d\n",
> +			 connector->name, mode->panel_orientation);

Maybe we could be more explicit there that we are "forcing" and orientation?

But anyway, that's for the other patch, for this one

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> 
> Regards,
> 
> Hans
> 
> 
> 
> > > -	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > +	/* Don't attach the property if the orientation is unknown */
> > > +	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > >   		return 0;
> > > +	info->panel_orientation = panel_orientation;
> > > +
> > >   	prop = dev->mode_config.panel_orientation_property;
> > >   	if (!prop) {
> > >   		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> > > @@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
> > >   				   info->panel_orientation);
> > >   	return 0;
> > >   }
> > > -EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
> > > +EXPORT_SYMBOL(drm_connector_set_panel_orientation);
> > > +
> > > +/**
> > > + * drm_connector_set_panel_orientation_with_quirk -
> > > + *	set the connecter's panel_orientation after checking for quirks
> > > + * @connector: connector for which to init the panel-orientation property.
> > > + * @panel_orientation: drm_panel_orientation value to set
> > > + * @width: width in pixels of the panel, used for panel quirk detection
> > > + * @height: height in pixels of the panel, used for panel quirk detection
> > > + *
> > > + * Like drm_connector_set_panel_orientation(), but with a check for platform
> > > + * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
> > > + *
> > > + * Returns:
> > > + * Zero on success, negative errno on failure.
> > > + */
> > > +int drm_connector_set_panel_orientation_with_quirk(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation,
> > > +	int width, int height)
> > > +{
> > > +	int orientation_quirk;
> > > +
> > > +	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> > > +	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > +		panel_orientation = orientation_quirk;
> > > +
> > > +	return drm_connector_set_panel_orientation(connector,
> > > +						   panel_orientation);
> > > +}
> > > +EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
> > >   int drm_connector_set_obj_prop(struct drm_mode_object *obj,
> > >   				    struct drm_property *property,
> > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> > > index 6e398c33a524..8cd51cf67d02 100644
> > > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > > @@ -1536,9 +1536,8 @@ static void icl_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);
> > > -	drm_connector_init_panel_orientation_property(&connector->base,
> > > +	drm_connector_set_panel_orientation_with_quirk(&connector->base,
> > > +				intel_dsi_get_panel_orientation(connector),
> > >   				connector->panel.fixed_mode->hdisplay,
> > >   				connector->panel.fixed_mode->vdisplay);
> > >   }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index aa515261cb9f..9f4425f8d0ac 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > >   	intel_connector->panel.backlight.power = intel_edp_backlight_power;
> > >   	intel_panel_setup_backlight(connector, pipe);
> > > -	if (fixed_mode)
> > > -		drm_connector_init_panel_orientation_property(
> > > -			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
> > > +	if (fixed_mode) {
> > > +		/* We do not know the orientation, but their might be a quirk */
> > > +		drm_connector_set_panel_orientation_with_quirk(connector,
> > > +				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
> > > +				fixed_mode->hdisplay, fixed_mode->vdisplay);
> > > +	}
> > >   	return true;
> > > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > > index 50064cde0724..a0de8e70e426 100644
> > > --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> > > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > > @@ -1632,10 +1632,9 @@ 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);
> > > -		drm_connector_init_panel_orientation_property(
> > > +		drm_connector_set_panel_orientation_with_quirk(
> > >   				&connector->base,
> > > +				vlv_dsi_get_panel_orientation(connector),
> > >   				connector->panel.fixed_mode->hdisplay,
> > >   				connector->panel.fixed_mode->vdisplay);
> > >   	}
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index 221910948b37..2113500b4075 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
> > >   					    uint64_t link_status);
> > >   void drm_connector_set_vrr_capable_property(
> > >   		struct drm_connector *connector, bool capable);
> > > -int drm_connector_init_panel_orientation_property(
> > > -	struct drm_connector *connector, int width, int height);
> > > +int drm_connector_set_panel_orientation(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation);
> > > +int drm_connector_set_panel_orientation_with_quirk(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation,
> > > +	int width, int height);
> > >   int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
> > >   					  int min, int max);
> > > -- 
> > > 2.23.0
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2)
@ 2020-01-03 19:37         ` Rodrigo Vivi
  0 siblings, 0 replies; 21+ messages in thread
From: Rodrigo Vivi @ 2020-01-03 19:37 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx, Derek Basehore, dri-devel

On Thu, Jan 02, 2020 at 10:35:28PM +0100, Hans de Goede wrote:
> Hi Rodrigo,
> 
> Thank you for the review.
> 
> On 02-01-2020 19:17, Rodrigo Vivi wrote:
> > On Mon, Dec 16, 2019 at 12:51:57PM +0100, Hans de Goede wrote:
> > > From: Derek Basehore <dbasehore@chromium.org>
> > > 
> > > Not every platform needs quirk detection for panel orientation, so
> > > split the drm_connector_init_panel_orientation_property into two
> > > functions. One for platforms without the need for quirks, and the
> > > other for platforms that need quirks.
> > > 
> > > Hans de Goede (changes in v2):
> > > 
> > > Rename the function from drm_connector_init_panel_orientation_property
> > > to drm_connector_set_panel_orientation[_with_quirk] and pass in the
> > > panel-orientation to set.
> > > 
> > > Beside the rename, also make the function set the passed in value
> > > only once, if the value was set before (to a value other then
> > > DRM_MODE_PANEL_ORIENTATION_UNKNOWN) make any further set calls a no-op.
> > > 
> > > This change is preparation for allowing the user to override the
> > > panel-orientation for any connector from the kernel commandline.
> > > When the panel-orientation is overridden this way, then we must ignore
> > > the panel-orientation detection done by the driver.
> > > 
> > > Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >   drivers/gpu/drm/drm_connector.c         | 74 ++++++++++++++++++-------
> > >   drivers/gpu/drm/i915/display/icl_dsi.c  |  5 +-
> > >   drivers/gpu/drm/i915/display/intel_dp.c |  9 ++-
> > >   drivers/gpu/drm/i915/display/vlv_dsi.c  |  5 +-
> > >   include/drm/drm_connector.h             |  9 ++-
> > >   5 files changed, 71 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 0965632008a9..f4fa5c59717d 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -1139,7 +1139,8 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> > >    *	coordinates, so if userspace rotates the picture to adjust for
> > >    *	the orientation it must also apply the same transformation to the
> > >    *	touchscreen input coordinates. This property is initialized by calling
> > > - *	drm_connector_init_panel_orientation_property().
> > > + *	drm_connector_set_panel_orientation() or
> > > + *	drm_connector_set_panel_orientation_with_quirk()
> > 
> > do we have a better name than quirks for these dsi modes?
> 
> The difference between the 2 functions is that the second one calls
> drm_get_panel_orientation_quirk() and that if that returns a valid
> orientation it overwrites the passed orientation with the return value
> from drm_get_panel_orientation_quirk(), so the name seems correct.
> 
> As for drm_get_panel_orientation_quirk() itself that currently is only
> defined on x86 (it is a static inline no-op elsewhere) and it used
> DMI string matching to check for a model specific quirk. So again the
> name seems correct.
> 
> > >    *
> > >    * scaling mode:
> > >    *	This property defines how a non-native mode is upscaled to the native
> > > @@ -2046,38 +2047,41 @@ void drm_connector_set_vrr_capable_property(
> > >   EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
> > >   /**
> > > - * drm_connector_init_panel_orientation_property -
> > > - *	initialize the connecters panel_orientation property
> > > - * @connector: connector for which to init the panel-orientation property.
> > > - * @width: width in pixels of the panel, used for panel quirk detection
> > > - * @height: height in pixels of the panel, used for panel quirk detection
> > > + * drm_connector_set_panel_orientation - sets the connecter's panel_orientation
> > > + * @connector: connector for which to set the panel-orientation property.
> > > + * @panel_orientation: drm_panel_orientation value to set
> > > + *
> > > + * This function sets the connector's panel_orientation and attaches
> > > + * a "panel orientation" property to the connector.
> > >    *
> > > - * This function should only be called for built-in panels, after setting
> > > - * connector->display_info.panel_orientation first (if known).
> > > + * Calling this function on a connector where the panel_orientation has
> > > + * already been set is a no-op (e.g. the orientation has been overridden with
> > > + * a kernel commandline option).
> > >    *
> > > - * This function will check for platform specific (e.g. DMI based) quirks
> > > - * overriding display_info.panel_orientation first, then if panel_orientation
> > > - * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
> > > - * "panel orientation" property to the connector.
> > > + * It is allowed to call this function with a panel_orientation of
> > > + * DRM_MODE_PANEL_ORIENTATION_UNKNOWN, in which case it is a no-op.
> > >    *
> > >    * Returns:
> > >    * Zero on success, negative errno on failure.
> > >    */
> > > -int drm_connector_init_panel_orientation_property(
> > > -	struct drm_connector *connector, int width, int height)
> > > +int drm_connector_set_panel_orientation(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation)
> > >   {
> > >   	struct drm_device *dev = connector->dev;
> > >   	struct drm_display_info *info = &connector->display_info;
> > >   	struct drm_property *prop;
> > > -	int orientation_quirk;
> > > -	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> > > -	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > -		info->panel_orientation = orientation_quirk;
> > > +	/* Already set? */
> > > +	if (info->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > +		return 0;
> > 
> > What happens on the scenario of ICL DSI here?
> > In case we had something badly set we just respect the bad choices?
> 
> The only path where the value gets set twice is when it has been set
> from the kernel commandline (which gets hooked up in the second patch),
> and yes in that case we honor what the user passed in as value, honering
> the user passed value (which gets processed first) is the whole reason
> for the "Already set?" check.
> 
> > Any way to at least have some kind of warn when we tried the dsi mode but
> > it had already been set?
> 
> The 2nd patch prints a message when a value is passed on the kernel commandline:
> 
> +	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
> +		DRM_INFO("setting connector %s panel_orientation to %d\n",
> +			 connector->name, mode->panel_orientation);

Maybe we could be more explicit there that we are "forcing" and orientation?

But anyway, that's for the other patch, for this one

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> 
> Regards,
> 
> Hans
> 
> 
> 
> > > -	if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > +	/* Don't attach the property if the orientation is unknown */
> > > +	if (panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > >   		return 0;
> > > +	info->panel_orientation = panel_orientation;
> > > +
> > >   	prop = dev->mode_config.panel_orientation_property;
> > >   	if (!prop) {
> > >   		prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
> > > @@ -2094,7 +2098,37 @@ int drm_connector_init_panel_orientation_property(
> > >   				   info->panel_orientation);
> > >   	return 0;
> > >   }
> > > -EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);
> > > +EXPORT_SYMBOL(drm_connector_set_panel_orientation);
> > > +
> > > +/**
> > > + * drm_connector_set_panel_orientation_with_quirk -
> > > + *	set the connecter's panel_orientation after checking for quirks
> > > + * @connector: connector for which to init the panel-orientation property.
> > > + * @panel_orientation: drm_panel_orientation value to set
> > > + * @width: width in pixels of the panel, used for panel quirk detection
> > > + * @height: height in pixels of the panel, used for panel quirk detection
> > > + *
> > > + * Like drm_connector_set_panel_orientation(), but with a check for platform
> > > + * specific (e.g. DMI based) quirks overriding the passed in panel_orientation.
> > > + *
> > > + * Returns:
> > > + * Zero on success, negative errno on failure.
> > > + */
> > > +int drm_connector_set_panel_orientation_with_quirk(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation,
> > > +	int width, int height)
> > > +{
> > > +	int orientation_quirk;
> > > +
> > > +	orientation_quirk = drm_get_panel_orientation_quirk(width, height);
> > > +	if (orientation_quirk != DRM_MODE_PANEL_ORIENTATION_UNKNOWN)
> > > +		panel_orientation = orientation_quirk;
> > > +
> > > +	return drm_connector_set_panel_orientation(connector,
> > > +						   panel_orientation);
> > > +}
> > > +EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
> > >   int drm_connector_set_obj_prop(struct drm_mode_object *obj,
> > >   				    struct drm_property *property,
> > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> > > index 6e398c33a524..8cd51cf67d02 100644
> > > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > > @@ -1536,9 +1536,8 @@ static void icl_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);
> > > -	drm_connector_init_panel_orientation_property(&connector->base,
> > > +	drm_connector_set_panel_orientation_with_quirk(&connector->base,
> > > +				intel_dsi_get_panel_orientation(connector),
> > >   				connector->panel.fixed_mode->hdisplay,
> > >   				connector->panel.fixed_mode->vdisplay);
> > >   }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index aa515261cb9f..9f4425f8d0ac 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -7340,9 +7340,12 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > >   	intel_connector->panel.backlight.power = intel_edp_backlight_power;
> > >   	intel_panel_setup_backlight(connector, pipe);
> > > -	if (fixed_mode)
> > > -		drm_connector_init_panel_orientation_property(
> > > -			connector, fixed_mode->hdisplay, fixed_mode->vdisplay);
> > > +	if (fixed_mode) {
> > > +		/* We do not know the orientation, but their might be a quirk */
> > > +		drm_connector_set_panel_orientation_with_quirk(connector,
> > > +				DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
> > > +				fixed_mode->hdisplay, fixed_mode->vdisplay);
> > > +	}
> > >   	return true;
> > > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > > index 50064cde0724..a0de8e70e426 100644
> > > --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> > > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > > @@ -1632,10 +1632,9 @@ 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);
> > > -		drm_connector_init_panel_orientation_property(
> > > +		drm_connector_set_panel_orientation_with_quirk(
> > >   				&connector->base,
> > > +				vlv_dsi_get_panel_orientation(connector),
> > >   				connector->panel.fixed_mode->hdisplay,
> > >   				connector->panel.fixed_mode->vdisplay);
> > >   	}
> > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > > index 221910948b37..2113500b4075 100644
> > > --- a/include/drm/drm_connector.h
> > > +++ b/include/drm/drm_connector.h
> > > @@ -1552,8 +1552,13 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
> > >   					    uint64_t link_status);
> > >   void drm_connector_set_vrr_capable_property(
> > >   		struct drm_connector *connector, bool capable);
> > > -int drm_connector_init_panel_orientation_property(
> > > -	struct drm_connector *connector, int width, int height);
> > > +int drm_connector_set_panel_orientation(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation);
> > > +int drm_connector_set_panel_orientation_with_quirk(
> > > +	struct drm_connector *connector,
> > > +	enum drm_panel_orientation panel_orientation,
> > > +	int width, int height);
> > >   int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
> > >   					  int min, int max);
> > > -- 
> > > 2.23.0
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH resend 2/2] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
  2019-12-16 11:51   ` [Intel-gfx] " Hans de Goede
@ 2020-01-03 19:38     ` Rodrigo Vivi
  -1 siblings, 0 replies; 21+ messages in thread
From: Rodrigo Vivi @ 2020-01-03 19:38 UTC (permalink / raw)
  To: Hans de Goede; +Cc: dri-devel, intel-gfx

On Mon, Dec 16, 2019 at 12:51:58PM +0100, Hans de Goede wrote:
> If the new video=... panel_orientation option is set for a connector, honor
> it and setup a matching "panel orientation" property on the connector.
> 
> BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
> Acked-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index f4fa5c59717d..d9d7fef26275 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
>  		connector->force = mode->force;
>  	}
>  
> +	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
> +		DRM_INFO("setting connector %s panel_orientation to %d\n",
> +			 connector->name, mode->panel_orientation);

Since this overrides the standard behavior should this deserve a debug that
is a bit more clear that we are forcing the mode with the cmdline?

> +		drm_connector_set_panel_orientation(connector,
> +						    mode->panel_orientation);
> +	}
> +
>  	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
>  		      connector->name, mode->name,
>  		      mode->xres, mode->yres,
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH resend 2/2] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
@ 2020-01-03 19:38     ` Rodrigo Vivi
  0 siblings, 0 replies; 21+ messages in thread
From: Rodrigo Vivi @ 2020-01-03 19:38 UTC (permalink / raw)
  To: Hans de Goede; +Cc: dri-devel, intel-gfx, Maxime Ripard

On Mon, Dec 16, 2019 at 12:51:58PM +0100, Hans de Goede wrote:
> If the new video=... panel_orientation option is set for a connector, honor
> it and setup a matching "panel orientation" property on the connector.
> 
> BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
> Acked-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index f4fa5c59717d..d9d7fef26275 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
>  		connector->force = mode->force;
>  	}
>  
> +	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
> +		DRM_INFO("setting connector %s panel_orientation to %d\n",
> +			 connector->name, mode->panel_orientation);

Since this overrides the standard behavior should this deserve a debug that
is a bit more clear that we are forcing the mode with the cmdline?

> +		drm_connector_set_panel_orientation(connector,
> +						    mode->panel_orientation);
> +	}
> +
>  	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
>  		      connector->name, mode->name,
>  		      mode->xres, mode->yres,
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH resend 2/2] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
  2020-01-03 19:38     ` Rodrigo Vivi
@ 2020-01-05 15:46       ` Hans de Goede
  -1 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2020-01-05 15:46 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: dri-devel, intel-gfx

Hi,

On 03-01-2020 20:38, Rodrigo Vivi wrote:
> On Mon, Dec 16, 2019 at 12:51:58PM +0100, Hans de Goede wrote:
>> If the new video=... panel_orientation option is set for a connector, honor
>> it and setup a matching "panel orientation" property on the connector.
>>
>> BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
>> Acked-by: Maxime Ripard <mripard@kernel.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index f4fa5c59717d..d9d7fef26275 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
>>   		connector->force = mode->force;
>>   	}
>>   
>> +	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
>> +		DRM_INFO("setting connector %s panel_orientation to %d\n",
>> +			 connector->name, mode->panel_orientation);
> 
> Since this overrides the standard behavior should this deserve a debug that
> is a bit more clear that we are forcing the mode with the cmdline?

Sure, I will send a v2 of this series with this fixed up right away.

Regards,

Hans


> 
>> +		drm_connector_set_panel_orientation(connector,
>> +						    mode->panel_orientation);
>> +	}
>> +
>>   	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
>>   		      connector->name, mode->name,
>>   		      mode->xres, mode->yres,
>> -- 
>> 2.23.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH resend 2/2] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member
@ 2020-01-05 15:46       ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2020-01-05 15:46 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: dri-devel, intel-gfx, Maxime Ripard

Hi,

On 03-01-2020 20:38, Rodrigo Vivi wrote:
> On Mon, Dec 16, 2019 at 12:51:58PM +0100, Hans de Goede wrote:
>> If the new video=... panel_orientation option is set for a connector, honor
>> it and setup a matching "panel orientation" property on the connector.
>>
>> BugLink: https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/83
>> Acked-by: Maxime Ripard <mripard@kernel.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index f4fa5c59717d..d9d7fef26275 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -140,6 +140,13 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
>>   		connector->force = mode->force;
>>   	}
>>   
>> +	if (mode->panel_orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) {
>> +		DRM_INFO("setting connector %s panel_orientation to %d\n",
>> +			 connector->name, mode->panel_orientation);
> 
> Since this overrides the standard behavior should this deserve a debug that
> is a bit more clear that we are forcing the mode with the cmdline?

Sure, I will send a v2 of this series with this fixed up right away.

Regards,

Hans


> 
>> +		drm_connector_set_panel_orientation(connector,
>> +						    mode->panel_orientation);
>> +	}
>> +
>>   	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
>>   		      connector->name, mode->name,
>>   		      mode->xres, mode->yres,
>> -- 
>> 2.23.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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

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

end of thread, other threads:[~2020-01-05 15:47 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16 11:51 [PATCH resend 0/2] drm/connector: Add support for specifying panel_orientation on the kernel cmdline Hans de Goede
2019-12-16 11:51 ` [Intel-gfx] " Hans de Goede
2019-12-16 11:51 ` [PATCH resend 1/2] drm/connector: Split out orientation quirk detection (v2) Hans de Goede
2019-12-16 11:51   ` [Intel-gfx] " Hans de Goede
2020-01-02 18:17   ` Rodrigo Vivi
2020-01-02 18:17     ` [Intel-gfx] " Rodrigo Vivi
2020-01-02 21:35     ` Hans de Goede
2020-01-02 21:35       ` [Intel-gfx] " Hans de Goede
2020-01-03 19:37       ` Rodrigo Vivi
2020-01-03 19:37         ` Rodrigo Vivi
2019-12-16 11:51 ` [PATCH resend 2/2] drm/connector: Hookup the new drm_cmdline_mode panel_orientation member Hans de Goede
2019-12-16 11:51   ` [Intel-gfx] " Hans de Goede
2020-01-03 19:38   ` Rodrigo Vivi
2020-01-03 19:38     ` Rodrigo Vivi
2020-01-05 15:46     ` Hans de Goede
2020-01-05 15:46       ` Hans de Goede
2019-12-16 16:45 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/connector: Add support for specifying panel_orientation on the kernel cmdline Patchwork
2019-12-16 17:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2019-12-16 21:28 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2020-01-01 17:04 ` [PATCH resend 0/2] " Hans de Goede
2020-01-01 17:04   ` [Intel-gfx] " Hans de Goede

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.