Re: [Intel-gfx] [PATCH v3 3/7] drm: Add support for a panel-orientation connector property

On Mon, Oct 30, 2017 at 11:57:10AM +0100, Hans de Goede wrote:

Hi,

On 30-10-17 10:43, Daniel Vetter wrote:

...

On Mon, Oct 23, 2017 at 09:14:21AM +0200, Hans de Goede wrote:

...

On some devices the LCD panel is mounted in the casing in such a way that the up/top side of the panel does not match with the top side of the device (e.g. it is mounted upside-down).

This commit adds the necessary infra for lcd-panel drm_connector-s to have a "panel orientation" property to communicate how the panel is orientated vs the casing.

Userspace can use this property to check for non-normal orientation and then adjust the displayed image accordingly by rotating it to compensate.

Signed-off-by: Hans de Goede hdegoede@redhat.com

Changes in v2: -Rebased on 4.14-rc1 -Store panel_orientation in drm_display_info, so that drm_fb_helper.c can access it easily -Have a single drm_connector_init_panel_orientation_property rather then create and attach functions. The caller is expected to set drm_display_info.panel_orientation before calling this, then this will check for platform specific quirks overriding the panel_orientation and if the panel_orientation is set after this then it will attach the property.

---

drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_connector.c | 73 +++++++++++++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 11 +++++++ include/drm/drm_mode_config.h | 7 ++++ include/uapi/drm/drm_mode.h | 7 ++++ 5 files changed, 99 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9d005ac98c2b..0b166e626eb6 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -7,6 +7,7 @@ menuconfig DRM tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)" depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA

• select DRM_PANEL_ORIENTATION_QUIRKS select HDMI select FB_CMDLINE select I2C

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 704fc8934616..129c83a84320 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -24,6 +24,7 @@ #include <drm/drm_connector.h> #include <drm/drm_edid.h> #include <drm/drm_encoder.h> +#include <drm/drm_utils.h> #include "drm_crtc_internal.h" #include "drm_internal.h" @@ -212,6 +213,8 @@ int drm_connector_init(struct drm_device *dev, mutex_init(&connector->mutex); connector->edid_blob_ptr = NULL; connector->status = connector_status_unknown;

• connector->display_info.panel_orientation =

• DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
  

  drm_connector_get_cmdline_mode(connector);

@@ -664,6 +667,13 @@ static const struct drm_prop_enum_list drm_aspect_ratio_enum_list[] = { { DRM_MODE_PICTURE_ASPECT_16_9, "16:9" }, }; +static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {

• { DRM_MODE_PANEL_ORIENTATION_NORMAL, "Normal" },
• { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down" },
• { DRM_MODE_PANEL_ORIENTATION_LEFT_UP, "Left Side Up" },
• { DRM_MODE_PANEL_ORIENTATION_RIGHT_UP, "Right Side Up" },

+};

• static const struct drm_prop_enum_list drm_dvi_i_select_enum_list[] = { { DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */ { DRM_MODE_SUBCONNECTOR_DVID, "DVI-D" }, /* DVI-I */

@@ -768,6 +778,18 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,

• CRTC_ID:
• Mode object ID of the &drm_crtc this connector should be connected to.

• • Connectors for LCD panels may also have one standardized property:
• • panel orientation:
• • On some devices the LCD panel is mounted in the casing in such a way
• • that the up/top side of the panel does not match with the top side of
• • the device. Userspace can use this property to check for this.
• • Note that input coordinates from touchscreens (input devices with
• • INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
• • coordinates, so if userspace rotates the picture to adjust for
• • the orientation it must also apply the same transformation to the
• • touchscreen input coordinates.
  */ int drm_connector_create_standard_properties(struct drm_device *dev)

@@ -1234,6 +1256,57 @@ void drm_mode_connector_set_link_status_property(struct drm_connector *connector } EXPORT_SYMBOL(drm_mode_connector_set_link_status_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
• • This function should only be called for built-in panels, after setting
• • connector->display_info.panel_orientation first (if known).
• • 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.
• • Returns:
• • Zero on success, negative errno on failure.
• */

Hm, I think our more usual way is to set the prop up first, and then the parsing mode updates the property (in case it's not quite as stable as we thought). Not the property init function calling the parsing code.

I know that the panel rotation will probably not change, but I think it'd be good to be consistent here. Or at least look into whether that makes sense ...

I'm not calling any parsing code here, what I'm calling is the code checking for quirks, so that that is done in one central place just like how drm_add_edid_modes() calls edid_get_quirks().

I could create the property in drm_connector_create_standard_properties() instead of in drm_connector_init_panel_orientation_property and rename the latter to drm_connector_attach_panel_orientation_property if you prefer having things split that way.

Auto attaching the property is tricky since we only want it on panels.

I just looked a bit backwards, and I hoped we could auto-update the property when parsing the edid (like we do in a bunch of other places). But sounds like that's not possible, so please disregard my suggestions.

If it later on turns out we need 2 steps, or have this also updated in the edid parsing code, we can fix that when there's a real need. -Daniel

Regards,

Hans

...

Besides this bikeshed color question makes all sense. -Daniel

...

+int drm_connector_init_panel_orientation_property(

• struct drm_connector *connector, int width, int height)

+{

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

• if (info->panel_orientation == DRM_MODE_PANEL_ORIENTATION_UNKNOWN)

• return 0;
  

• prop = dev->mode_config.panel_orientation_property;
• if (!prop) {

• prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
  

• 		"panel orientation",
  

• 		drm_panel_orientation_enum_list,
  

• 		ARRAY_SIZE(drm_panel_orientation_enum_list));
  

• if (!prop)
  

• 	return -ENOMEM;
  

• dev->mode_config.panel_orientation_property = prop;
  

• }
• drm_object_attach_property(&connector->base, prop,

• 		   info->panel_orientation);
  

• return 0;

+} +EXPORT_SYMBOL(drm_connector_init_panel_orientation_property);

• int drm_mode_connector_set_obj_prop(struct drm_mode_object *obj, struct drm_property *property, uint64_t value)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index b4285c40e1e4..e6883065a461 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -222,6 +222,15 @@ struct drm_display_info { #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) #define DRM_COLOR_FORMAT_YCRCB420 (1<<3)

• /**

• * @panel_orientation: Read only connector property for built-in panels,
  

• * indicating the orientation of the panel vs the device's casing.
  

• * drm_connector_init() sets this to DRM_MODE_PANEL_ORIENTATION_UNKNOWN.
  

• * When not UNKNOWN this gets used by the drm_fb_helpers to rotate the
  

• * fb to compensate and gets exported as prop to userspace.
  

• */
  

• int panel_orientation;
• /**
  • @color_formats: HDMI Color formats, selects between RGB and YCrCb
  • modes. Used DRM_COLOR_FORMAT_ defines, which are _not_ the same ones

@@ -1019,6 +1028,8 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector, const struct edid *edid); void drm_mode_connector_set_link_status_property(struct drm_connector *connector, uint64_t link_status); +int drm_connector_init_panel_orientation_property(

• struct drm_connector *connector, int width, int height); /**
  • struct drm_tile_group - Tile group metadata

diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 0b4ac2ebc610..7d4ee1726e0a 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -728,6 +728,13 @@ struct drm_mode_config { */ struct drm_property *suggested_y_property;

• /**

• * @panel_orientation_property: Optional connector property indicating
  

• * how the lcd-panel is mounted inside the casing (e.g. normal or
  

• * upside-down).
  

• */
  

• struct drm_property *panel_orientation_property;
• /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 34b6bb34b002..f60fae67bc1f 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -127,6 +127,13 @@ extern "C" { #define DRM_MODE_LINK_STATUS_GOOD 0 #define DRM_MODE_LINK_STATUS_BAD 1 +/* Panel Orientation options */ +#define DRM_MODE_PANEL_ORIENTATION_UNKNOWN -1 +#define DRM_MODE_PANEL_ORIENTATION_NORMAL 0 +#define DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP 1 +#define DRM_MODE_PANEL_ORIENTATION_LEFT_UP 2 +#define DRM_MODE_PANEL_ORIENTATION_RIGHT_UP 3

• /*
  • DRM_MODE_ROTATE_<degrees>

-- 2.14.2

---

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