[Intel-gfx] [PATCH 1/5] drm: report dp downstream port type as a subconnector property
Shankar, Uma
uma.shankar at intel.com
Wed Apr 1 12:26:36 UTC 2020
> -----Original Message-----
> From: B, Jeevan <jeevan.b at intel.com>
> Sent: Wednesday, April 1, 2020 5:42 PM
> To: Shankar, Uma <uma.shankar at intel.com>
> Cc: B, Jeevan <jeevan.b at intel.com>; Ville Syrjälä <ville.syrjala at linux.intel.com>;
> intel-gfx at lists.freedesktop.org
> Subject: [PATCH 1/5] drm: report dp downstream port type as a subconnector
> property
>
> Currently, downstream port type is only reported in debugfs. This information should
> be considered important since it reflects the actual physical connector type. Some
> userspace (e.g. window compositors) may want to show this info to a user.
>
> The 'subconnector' property is already utilized for DVI-I and TV-out for reporting
> connector subtype.
>
> The initial motivation for this feature came from i2c test [1].
> It is supposed to be skipped on VGA connectors, but it cannot detect VGA over DP
> and fails instead.
>
> v2:
> - Ville: utilized drm_dp_is_branch()
> - Ville: implement DP 1.0 downstream type info
> - Replaced create_dp_properties with add_dp_subconnector_property
> - Added dp_set_subconnector_property helper
>
> v4:
> - Ville: add DP1.0 best assumption about subconnector
> - Ville: assume DVI is DVI-D
> - Ville: reuse Writeback enum value for Virtual subconnector
> - Renamed #defines: HDMI -> HDMIA, DP -> DisplayPort
>
> [1]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
>
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> Signed-off-by: Oleg Vasilev <oleg.vasilev at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Signed-off-by: Jeevan B <jeevan.b at intel.com>
You can update the version number for this series : use subject-prefix=v5 as git-format.
Also move your signed off close to Oleg's. Best is to put Cc: at the top, then all
Signed-off and Reviewed-by
Also in patch mention change in v5, like if it's just rebased
V5: rebase
> Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-3-
> oleg.vasilev at intel.com
> ---
> drivers/gpu/drm/drm_connector.c | 49 ++++++++++++++++++++++++--
> drivers/gpu/drm/drm_dp_helper.c | 77
> +++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_connector.h | 3 ++
> include/drm/drm_dp_helper.h | 8 +++++
> include/drm/drm_mode_config.h | 6 ++++
> include/uapi/drm/drm_mode.h | 21 ++++++-----
> 6 files changed, 154 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b1099e1..b6972d1 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -844,7 +844,7 @@ static const struct drm_prop_enum_list
> drm_dvi_i_select_enum_list[] = {
> DRM_ENUM_NAME_FN(drm_get_dvi_i_select_name, drm_dvi_i_select_enum_list)
>
> static const struct drm_prop_enum_list drm_dvi_i_subconnector_enum_list[] = {
> - { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I and
> TV-out */
> + { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I, TV-
> out and DP */
> { DRM_MODE_SUBCONNECTOR_DVID, "DVI-D" }, /* DVI-I */
> { DRM_MODE_SUBCONNECTOR_DVIA, "DVI-A" }, /* DVI-I */
> };
> @@ -861,7 +861,7 @@ static const struct drm_prop_enum_list
> drm_tv_select_enum_list[] = { DRM_ENUM_NAME_FN(drm_get_tv_select_name,
> drm_tv_select_enum_list)
>
> static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = {
> - { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I and
> TV-out */
> + { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I, TV-
> out and DP */
> { DRM_MODE_SUBCONNECTOR_Composite, "Composite" }, /* TV-out */
> { DRM_MODE_SUBCONNECTOR_SVIDEO, "SVIDEO" }, /* TV-out */
> { DRM_MODE_SUBCONNECTOR_Component, "Component" }, /* TV-out */
> @@ -870,6 +870,19 @@ static const struct drm_prop_enum_list
> drm_tv_subconnector_enum_list[] = {
> DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
> drm_tv_subconnector_enum_list)
>
> +static const struct drm_prop_enum_list drm_dp_subconnector_enum_list[] = {
> + { DRM_MODE_SUBCONNECTOR_Unknown, "Unknown" }, /* DVI-I, TV-
> out and DP */
> + { DRM_MODE_SUBCONNECTOR_VGA, "VGA" }, /* DP */
> + { DRM_MODE_SUBCONNECTOR_DVID, "DVI-D" }, /* DP */
> + { DRM_MODE_SUBCONNECTOR_HDMIA, "HDMI" }, /* DP */
> + { DRM_MODE_SUBCONNECTOR_DisplayPort, "DP" }, /* DP */
> + { DRM_MODE_SUBCONNECTOR_Wireless, "Wireless" }, /* DP */
> + { DRM_MODE_SUBCONNECTOR_Native, "Native" }, /* DP */
> +};
> +
> +DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
> + drm_dp_subconnector_enum_list)
> +
> static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> /* For Default case, driver will set the colorspace */
> { DRM_MODE_COLORIMETRY_DEFAULT, "Default" }, @@ -1186,6 +1199,14
> @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
> * can also expose this property to external outputs, in which case they
> * must support "None", which should be the default (since external screens
> * have a built-in scaler).
> + *
> + * subconnector:
> + * This property is used by DVI-I, TVout and DisplayPort to indicate different
> + * connector subtypes. Enum values more or less match with those from main
> + * connector types.
> + * For DVI-I and TVout there is also a matching property "select subconnector"
> + * allowing to switch between signal types.
> + * DP subconnector corresponds to a downstream port.
> */
>
> int drm_connector_create_standard_properties(struct drm_device *dev) @@ -
> 1275,6 +1296,30 @@ int drm_mode_create_dvi_i_properties(struct drm_device
> *dev) EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
>
> /**
> + * drm_mode_add_dp_subconnector_property - create subconnector property
> +for DP
> + * @connector: drm_connector to attach property
> + *
> + * Called by a driver when DP connector is created.
> + */
> +void drm_mode_add_dp_subconnector_property(struct drm_connector
> +*connector) {
> + struct drm_mode_config *mode_config = &connector->dev->mode_config;
> +
> + if (!mode_config->dp_subconnector_property)
> + mode_config->dp_subconnector_property =
> + drm_property_create_enum(connector->dev,
> + DRM_MODE_PROP_IMMUTABLE,
> + "subconnector",
> + drm_dp_subconnector_enum_list,
> + ARRAY_SIZE(drm_dp_subconnector_enum_list));
> +
> + drm_object_attach_property(&connector->base,
> + mode_config->dp_subconnector_property,
> + DRM_MODE_SUBCONNECTOR_Unknown);
> +}
> +EXPORT_SYMBOL(drm_mode_add_dp_subconnector_property);
> +
> +/**
> * DOC: HDMI connector properties
> *
> * content type (HDMI specific):
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 8ba4531..5d5b50f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -594,6 +594,83 @@ void drm_dp_downstream_debug(struct seq_file *m, }
> EXPORT_SYMBOL(drm_dp_downstream_debug);
>
> +/**
> + * drm_dp_subconnector_type() - get DP branch device type
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + *
> + */
> +enum drm_mode_subconnector
> +drm_dp_subconnector_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> + const u8 port_cap[4])
> +{
> + int type;
> + if (!drm_dp_is_branch(dpcd))
> + return DRM_MODE_SUBCONNECTOR_Native;
> + /* DP 1.0 approach */
> + if (dpcd[DP_DPCD_REV] == DP_DPCD_REV_10) {
> + type = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> + DP_DWN_STRM_PORT_TYPE_MASK;
> +
> + switch (type) {
> + case DP_DWN_STRM_PORT_TYPE_TMDS:
> + /* Can be HDMI or DVI-D, DVI-D is a safer option */
> + return DRM_MODE_SUBCONNECTOR_DVID;
> + case DP_DWN_STRM_PORT_TYPE_ANALOG:
> + /* Can be VGA or DVI-A, VGA is more popular */
> + return DRM_MODE_SUBCONNECTOR_VGA;
> + case DP_DWN_STRM_PORT_TYPE_DP:
> + return DRM_MODE_SUBCONNECTOR_DisplayPort;
> + case DP_DWN_STRM_PORT_TYPE_OTHER:
> + default:
> + return DRM_MODE_SUBCONNECTOR_Unknown;
> + }
> + }
> + type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> +
> + switch (type) {
> + case DP_DS_PORT_TYPE_DP:
> + case DP_DS_PORT_TYPE_DP_DUALMODE:
> + return DRM_MODE_SUBCONNECTOR_DisplayPort;
> + case DP_DS_PORT_TYPE_VGA:
> + return DRM_MODE_SUBCONNECTOR_VGA;
> + case DP_DS_PORT_TYPE_DVI:
> + return DRM_MODE_SUBCONNECTOR_DVID;
> + case DP_DS_PORT_TYPE_HDMI:
> + return DRM_MODE_SUBCONNECTOR_HDMIA;
> + case DP_DS_PORT_TYPE_WIRELESS:
> + return DRM_MODE_SUBCONNECTOR_Wireless;
> + case DP_DS_PORT_TYPE_NON_EDID:
> + default:
> + return DRM_MODE_SUBCONNECTOR_Unknown;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_subconnector_type);
> +
> +/**
> + * drm_mode_set_dp_subconnector_property - set subconnector for DP
> +connector
> + * @connector: dp connector to attach property
> + * @status: connector status which is about to be set
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + *
> + * Called by a driver on every detect event.
> + */
> +void drm_dp_set_subconnector_property(struct drm_connector *connector,
> + enum drm_connector_status status,
> + const u8 *dpcd,
> + const u8 port_cap[4])
> +{
> + enum drm_mode_subconnector subconnector =
> +DRM_MODE_SUBCONNECTOR_Unknown;
> +
> + if (status == connector_status_connected)
> + subconnector = drm_dp_subconnector_type(dpcd, port_cap);
> + drm_object_property_set_value(&connector->base,
> + connector->dev->mode_config.dp_subconnector_property,
> + subconnector);
> +}
> +EXPORT_SYMBOL(drm_dp_set_subconnector_property);
> +
> /*
> * I2C-over-AUX implementation
> */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index
> fd543d1..82797c8 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1556,10 +1556,13 @@ const char *drm_get_dvi_i_subconnector_name(int
> val); const char *drm_get_dvi_i_select_name(int val); const char
> *drm_get_tv_subconnector_name(int val); const char
> *drm_get_tv_select_name(int val);
> +const char *drm_get_dp_subconnector_name(int val);
> const char *drm_get_content_protection_name(int val); const char
> *drm_get_hdcp_content_type_name(int val);
>
> int drm_mode_create_dvi_i_properties(struct drm_device *dev);
> +void drm_mode_add_dp_subconnector_property(struct drm_connector
> +*connector);
> +
> int drm_mode_create_tv_margin_properties(struct drm_device *dev); int
> drm_mode_create_tv_properties(struct drm_device *dev,
> unsigned int num_modes,
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index
> 305533d..6408b31 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -26,6 +26,7 @@
> #include <linux/delay.h>
> #include <linux/i2c.h>
> #include <linux/types.h>
> +#include <drm/drm_connector.h>
>
> /*
> * Unless otherwise noted, all values are from the DP 1.1a spec. Note that @@ -
> 1599,6 +1600,13 @@ int drm_dp_downstream_max_bpc(const u8
> dpcd[DP_RECEIVER_CAP_SIZE], int drm_dp_downstream_id(struct drm_dp_aux
> *aux, char id[6]); void drm_dp_downstream_debug(struct seq_file *m, const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
> const u8 port_cap[4], struct drm_dp_aux *aux);
> +enum drm_mode_subconnector
> +drm_dp_subconnector_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> + const u8 port_cap[4]);
> +void drm_dp_set_subconnector_property(struct drm_connector *connector,
> + enum drm_connector_status status,
> + const u8 *dpcd,
> + const u8 port_cap[4]);
>
> void drm_dp_remote_aux_init(struct drm_dp_aux *aux); void
> drm_dp_aux_init(struct drm_dp_aux *aux); diff --git
> a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index
> 6c3ef49..085fb00 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -681,6 +681,12 @@ struct drm_mode_config {
> struct drm_property *dvi_i_select_subconnector_property;
>
> /**
> + * @dp_subconnector_property: Optional DP property to differentiate
> + * between different DP downstream port types.
> + */
> + struct drm_property *dp_subconnector_property;
> +
> + /**
> * @tv_subconnector_property: Optional TV property to differentiate
> * between different TV connector types.
> */
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index
> 735c8cf..3358c6b 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -332,14 +332,19 @@ struct drm_mode_get_encoder {
> /* This is for connectors with multiple signal types. */
> /* Try to match DRM_MODE_CONNECTOR_X as closely as possible. */ enum
> drm_mode_subconnector {
> - DRM_MODE_SUBCONNECTOR_Automatic = 0,
> - DRM_MODE_SUBCONNECTOR_Unknown = 0,
> - DRM_MODE_SUBCONNECTOR_DVID = 3,
> - DRM_MODE_SUBCONNECTOR_DVIA = 4,
> - DRM_MODE_SUBCONNECTOR_Composite = 5,
> - DRM_MODE_SUBCONNECTOR_SVIDEO = 6,
> - DRM_MODE_SUBCONNECTOR_Component = 8,
> - DRM_MODE_SUBCONNECTOR_SCART = 9,
> + DRM_MODE_SUBCONNECTOR_Automatic = 0, /* DVI-I, TV */
> + DRM_MODE_SUBCONNECTOR_Unknown = 0, /* DVI-I, TV, DP */
> + DRM_MODE_SUBCONNECTOR_VGA = 1, /* DP */
> + DRM_MODE_SUBCONNECTOR_DVID = 3, /* DVI-I DP */
> + DRM_MODE_SUBCONNECTOR_DVIA = 4, /* DVI-I */
> + DRM_MODE_SUBCONNECTOR_Composite = 5, /* TV */
> + DRM_MODE_SUBCONNECTOR_SVIDEO = 6, /* TV */
> + DRM_MODE_SUBCONNECTOR_Component = 8, /* TV */
> + DRM_MODE_SUBCONNECTOR_SCART = 9, /* TV */
> + DRM_MODE_SUBCONNECTOR_DisplayPort = 10, /* DP */
> + DRM_MODE_SUBCONNECTOR_HDMIA = 11, /* DP */
> + DRM_MODE_SUBCONNECTOR_Native = 15, /* DP */
> + DRM_MODE_SUBCONNECTOR_Wireless = 18, /* DP */
> };
>
> #define DRM_MODE_CONNECTOR_Unknown 0
> --
> 2.7.4
More information about the Intel-gfx
mailing list