[PATCH 1/5] drm: report dp downstream port type as a subconnector property

[PATCH 1/5] drm: report dp downstream port type as a subconnector property

[Jeevan B]

From: Oleg Vasilev <oleg.vasilev@intel.com>

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

v5: rebase

[1]: https://bugs.freedesktop.org/show_bug.cgi?id=104097

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Jeevan B <jeevan.b@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-3-oleg.vasilev@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

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

[PATCH 2/5] drm/i915: utilize subconnector property for DP

[Jeevan B]

From: Oleg Vasilev <oleg.vasilev@intel.com>

Since DP-specific information is stored in driver's structures, every
driver needs to implement subconnector property by itself.

v2: updates to match previous commit changes

v3: rebase

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Jeevan B <jeevan.b@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Tested-by: Oleg Vasilev <oleg.vasilev@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-4-oleg.vasilev@intel.com
---
 drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index db6ae8e..ba443e1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6155,6 +6155,11 @@ intel_dp_detect(struct drm_connector *connector,
 	 */
 	intel_display_power_flush_work(dev_priv);
 
+	if (!intel_dp_is_edp(intel_dp))
+		drm_dp_set_subconnector_property(connector,
+						 status,
+						 intel_dp->dpcd,
+						 intel_dp->downstream_ports);
 	return status;
 }
 
@@ -7211,6 +7216,9 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	enum port port = dp_to_dig_port(intel_dp)->base.port;
 
+	if (!intel_dp_is_edp(intel_dp))
+		drm_mode_add_dp_subconnector_property(connector);
+
 	if (!IS_G4X(dev_priv) && port != PORT_A)
 		intel_attach_force_audio_property(connector);
 
-- 
2.7.4

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

[PATCH 3/5] drm/nouveau: utilize subconnector property for DP

[Jeevan B]

From: Oleg Vasilev <oleg.vasilev@intel.com>

Since DP-specific information is stored in driver's structures, every
driver needs to implement subconnector property by itself.

v2: rebase

Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Jeevan B <jeevan.b@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-5-oleg.vasilev@intel.com
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 13 +++++++++++++
 drivers/gpu/drm/nouveau/nouveau_dp.c        |  9 +++++++++
 drivers/gpu/drm/nouveau/nouveau_encoder.h   |  1 +
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 9a9a7f5..6464e48 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -648,6 +648,17 @@ nouveau_connector_detect(struct drm_connector *connector, bool force)
 	pm_runtime_mark_last_busy(dev->dev);
 	pm_runtime_put_autosuspend(dev->dev);
 
+	if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+		enum drm_mode_subconnector subconnector = DRM_MODE_SUBCONNECTOR_Unknown;
+
+		if (conn_status == connector_status_connected && nv_encoder)
+			subconnector = nv_encoder->dp.subconnector;
+		drm_object_property_set_value(&connector->base,
+			connector->dev->mode_config.dp_subconnector_property,
+			subconnector);
+	}
+
 	return conn_status;
 }
 
@@ -1373,6 +1384,8 @@ nouveau_connector_create(struct drm_device *dev,
 			kfree(nv_connector);
 			return ERR_PTR(ret);
 		}
+
+		drm_mode_add_dp_subconnector_property(connector);
 		funcs = &nouveau_connector_funcs;
 		break;
 	default:
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 2674f15..85eac85 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -62,6 +62,7 @@ nouveau_dp_detect(struct nouveau_encoder *nv_encoder)
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nvkm_i2c_aux *aux;
 	u8 dpcd[8];
+	u8 port_cap[DP_MAX_DOWNSTREAM_PORTS] = {};
 	int ret;
 
 	aux = nv_encoder->aux;
@@ -72,6 +73,14 @@ nouveau_dp_detect(struct nouveau_encoder *nv_encoder)
 	if (ret)
 		return ret;
 
+	if (dpcd[DP_DPCD_REV] > 0x10) {
+		ret = nvkm_rdaux(aux, DP_DOWNSTREAM_PORT_0,
+				 port_cap, DP_MAX_DOWNSTREAM_PORTS);
+		if (ret)
+			memset(port_cap, 0, DP_MAX_DOWNSTREAM_PORTS);
+	}
+	nv_encoder->dp.subconnector = drm_dp_subconnector_type(dpcd, port_cap);
+
 	nv_encoder->dp.link_bw = 27000 * dpcd[1];
 	nv_encoder->dp.link_nr = dpcd[2] & DP_MAX_LANE_COUNT_MASK;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h b/drivers/gpu/drm/nouveau/nouveau_encoder.h
index 3517f92..e17971a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
+++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
@@ -63,6 +63,7 @@ struct nouveau_encoder {
 			struct nv50_mstm *mstm;
 			int link_nr;
 			int link_bw;
+			enum drm_mode_subconnector subconnector;
 		} dp;
 	};
 
-- 
2.7.4

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

[PATCH 4/5] drm/amdgpu: utilize subconnector property for DP through atombios

[Jeevan B]

From: Oleg Vasilev <oleg.vasilev@intel.com>

Since DP-specific information is stored in driver's structures, every
driver needs to implement subconnector property by itself.

v2: rebase

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Jeevan B <jeevan.b@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-6-oleg.vasilev@intel.com
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 10 ++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h       |  1 +
 drivers/gpu/drm/amd/amdgpu/atombios_dp.c       | 18 +++++++++++++++++-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index f355d9a..71aade0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -26,6 +26,7 @@
 
 #include <drm/drm_edid.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_dp_helper.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/amdgpu_drm.h>
 #include "amdgpu.h"
@@ -1405,6 +1406,10 @@ amdgpu_connector_dp_detect(struct drm_connector *connector, bool force)
 		pm_runtime_put_autosuspend(connector->dev->dev);
 	}
 
+	drm_dp_set_subconnector_property(&amdgpu_connector->base,
+					 ret,
+					 amdgpu_dig_connector->dpcd,
+					 amdgpu_dig_connector->downstream_ports);
 	return ret;
 }
 
@@ -1951,6 +1956,11 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 	if (has_aux)
 		amdgpu_atombios_dp_aux_init(amdgpu_connector);
 
+	if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	    connector_type == DRM_MODE_CONNECTOR_eDP) {
+		drm_mode_add_dp_subconnector_property(&amdgpu_connector->base);
+	}
+
 	return;
 
 failed:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 37ba07e..04a430e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -469,6 +469,7 @@ struct amdgpu_encoder {
 struct amdgpu_connector_atom_dig {
 	/* displayport */
 	u8 dpcd[DP_RECEIVER_CAP_SIZE];
+	u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	u8 dp_sink_type;
 	int dp_clock;
 	int dp_lane_count;
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
index 9b74cfd..900b272 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
@@ -328,6 +328,22 @@ static void amdgpu_atombios_dp_probe_oui(struct amdgpu_connector *amdgpu_connect
 			      buf[0], buf[1], buf[2]);
 }
 
+static void amdgpu_atombios_dp_ds_ports(struct amdgpu_connector *amdgpu_connector)
+{
+	struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
+	int ret;
+
+	if (dig_connector->dpcd[DP_DPCD_REV] > 0x10) {
+		ret = drm_dp_dpcd_read(&amdgpu_connector->ddc_bus->aux,
+				       DP_DOWNSTREAM_PORT_0,
+				       dig_connector->downstream_ports,
+				       DP_MAX_DOWNSTREAM_PORTS);
+		if (ret)
+			memset(dig_connector->downstream_ports, 0,
+			       DP_MAX_DOWNSTREAM_PORTS);
+	}
+}
+
 int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
 {
 	struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
@@ -343,7 +359,7 @@ int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
 			      dig_connector->dpcd);
 
 		amdgpu_atombios_dp_probe_oui(amdgpu_connector);
-
+		amdgpu_atombios_dp_ds_ports(amdgpu_connector);
 		return 0;
 	}
 
-- 
2.7.4

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

[PATCH 5/5] drm/amdgpu: utilize subconnector property for DP through DisplayManager

[Jeevan B]

From: Oleg Vasilev <oleg.vasilev@intel.com>

Since DP-specific information is stored in driver's structures, every
driver needs to implement subconnector property by itself. Display
Core already has the subconnector information, we only need to
expose it through DRM property.

v2:rebase

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Jeevan B <jeevan.b@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
Tested-by: Oleg Vasilev <oleg.vasilev@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-7-oleg.vasilev@intel.com
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 41 +++++++++++++++++++++-
 .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c    |  3 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d3674d8..91c0ef2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -121,6 +121,42 @@ MODULE_FIRMWARE(FIRMWARE_NAVI12_DMCU);
 static int amdgpu_dm_init(struct amdgpu_device *adev);
 static void amdgpu_dm_fini(struct amdgpu_device *adev);
 
+static enum drm_mode_subconnector get_subconnector_type(struct dc_link *link)
+{
+	switch (link->dpcd_caps.dongle_type) {
+	case DISPLAY_DONGLE_NONE:
+		return DRM_MODE_SUBCONNECTOR_Native;
+	case DISPLAY_DONGLE_DP_VGA_CONVERTER:
+		return DRM_MODE_SUBCONNECTOR_VGA;
+	case DISPLAY_DONGLE_DP_DVI_CONVERTER:
+	case DISPLAY_DONGLE_DP_DVI_DONGLE:
+		return DRM_MODE_SUBCONNECTOR_DVID;
+	case DISPLAY_DONGLE_DP_HDMI_CONVERTER:
+	case DISPLAY_DONGLE_DP_HDMI_DONGLE:
+		return DRM_MODE_SUBCONNECTOR_HDMIA;
+	case DISPLAY_DONGLE_DP_HDMI_MISMATCHED_DONGLE:
+	default:
+		return DRM_MODE_SUBCONNECTOR_Unknown;
+	}
+}
+
+static void update_subconnector_property(struct amdgpu_dm_connector *aconnector)
+{
+	struct dc_link *link = aconnector->dc_link;
+	struct drm_connector *connector = &aconnector->base;
+	enum drm_mode_subconnector subconnector = DRM_MODE_SUBCONNECTOR_Unknown;
+
+	if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
+		return;
+
+	if (aconnector->dc_sink)
+		subconnector = get_subconnector_type(link);
+
+	drm_object_property_set_value(&connector->base,
+			connector->dev->mode_config.dp_subconnector_property,
+			subconnector);
+}
+
 /*
  * initializes drm_device display related structures, based on the information
  * provided by DAL. The drm strcutures are: drm_crtc, drm_connector,
@@ -1917,7 +1953,6 @@ void amdgpu_dm_update_connector_after_detect(
 	if (aconnector->mst_mgr.mst_state == true)
 		return;
 
-
 	sink = aconnector->dc_link->local_sink;
 	if (sink)
 		dc_sink_retain(sink);
@@ -2038,6 +2073,8 @@ void amdgpu_dm_update_connector_after_detect(
 
 	mutex_unlock(&dev->mode_config.mutex);
 
+	update_subconnector_property(aconnector);
+
 	if (sink)
 		dc_sink_release(sink);
 }
@@ -4518,6 +4555,8 @@ amdgpu_dm_connector_detect(struct drm_connector *connector, bool force)
 	else
 		connected = (aconnector->base.force == DRM_FORCE_ON);
 
+	update_subconnector_property(aconnector);
+
 	return (connected ? connector_status_connected :
 			connector_status_disconnected);
 }
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index d56b758..267c82c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -26,6 +26,7 @@
 #include <linux/version.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_dp_mst_helper.h>
+#include <drm/drm_dp_helper.h>
 #include "dm_services.h"
 #include "amdgpu.h"
 #include "amdgpu_dm.h"
@@ -464,6 +465,8 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
 		16,
 		4,
 		aconnector->connector_id);
+
+	drm_mode_add_dp_subconnector_property(&aconnector->base);
 }
 
 int dm_mst_get_pbn_divider(struct dc_link *link)
-- 
2.7.4

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

Re: [PATCH 1/5] drm: report dp downstream port type as a subconnector property

[Jani Nikula]

On Tue, 07 Apr 2020, Jeevan B <jeevan.b@intel.com> wrote:
> From: Oleg Vasilev <oleg.vasilev@intel.com>
>
> 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
>
> v5: rebase
>
> [1]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-3-oleg.vasilev@intel.com

High level looks fine to me, please see some nitpicks inline.

BR,
Jani.


> ---
>  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)
> +{

So far most functions dealing with properties are named:

- drm_mode_create_*
- drm_connector_attach_*

Sometimes the latter calling the former, sometimes creating inline.

drm_mode_add_* that attaches a property on a connector is confusing.

Perhaps call it drm_connector_attach_dp_subconnector_property().

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

This does not help at all. What should this point at?

> + *
> + */
> +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

Unhelpful documentation. Sure port_cap is port capabilities, but what
should this point at? (I can tell looking at the code, but what's the
point of documentation if you have to look at the code?)

> + *
> + * 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])

Inconsistent parameters.

> +{
> +	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>

Forward declarations instead of includes if possible, please.

>  
>  /*
>   * 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]);

Inconsistent parameters.

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

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

Re: [PATCH 2/5] drm/i915: utilize subconnector property for DP

[Jani Nikula]

On Tue, 07 Apr 2020, Jeevan B <jeevan.b@intel.com> wrote:
> From: Oleg Vasilev <oleg.vasilev@intel.com>
>
> Since DP-specific information is stored in driver's structures, every
> driver needs to implement subconnector property by itself.
>
> v2: updates to match previous commit changes
>
> v3: rebase
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Tested-by: Oleg Vasilev <oleg.vasilev@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-4-oleg.vasilev@intel.com

You're not supposed to add the Link: tag yourself.

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


> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index db6ae8e..ba443e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6155,6 +6155,11 @@ intel_dp_detect(struct drm_connector *connector,
>  	 */
>  	intel_display_power_flush_work(dev_priv);
>  
> +	if (!intel_dp_is_edp(intel_dp))
> +		drm_dp_set_subconnector_property(connector,
> +						 status,
> +						 intel_dp->dpcd,
> +						 intel_dp->downstream_ports);
>  	return status;
>  }
>  
> @@ -7211,6 +7216,9 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	enum port port = dp_to_dig_port(intel_dp)->base.port;
>  
> +	if (!intel_dp_is_edp(intel_dp))
> +		drm_mode_add_dp_subconnector_property(connector);
> +
>  	if (!IS_G4X(dev_priv) && port != PORT_A)
>  		intel_attach_force_audio_property(connector);

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

Re: [Intel-gfx] [PATCH 4/5] drm/amdgpu: utilize subconnector property for DP through atombios

[Jani Nikula]

Alex, Harry, Christian, can you please eyeball this series and see if it
makes sense for you?

Thanks,
Jani.


On Tue, 07 Apr 2020, Jeevan B <jeevan.b@intel.com> wrote:
> From: Oleg Vasilev <oleg.vasilev@intel.com>
>
> Since DP-specific information is stored in driver's structures, every
> driver needs to implement subconnector property by itself.
>
> v2: rebase
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-6-oleg.vasilev@intel.com
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 10 ++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h       |  1 +
>  drivers/gpu/drm/amd/amdgpu/atombios_dp.c       | 18 +++++++++++++++++-
>  3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index f355d9a..71aade0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -26,6 +26,7 @@
>  
>  #include <drm/drm_edid.h>
>  #include <drm/drm_fb_helper.h>
> +#include <drm/drm_dp_helper.h>
>  #include <drm/drm_probe_helper.h>
>  #include <drm/amdgpu_drm.h>
>  #include "amdgpu.h"
> @@ -1405,6 +1406,10 @@ amdgpu_connector_dp_detect(struct drm_connector *connector, bool force)
>  		pm_runtime_put_autosuspend(connector->dev->dev);
>  	}
>  
> +	drm_dp_set_subconnector_property(&amdgpu_connector->base,
> +					 ret,
> +					 amdgpu_dig_connector->dpcd,
> +					 amdgpu_dig_connector->downstream_ports);
>  	return ret;
>  }
>  
> @@ -1951,6 +1956,11 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>  	if (has_aux)
>  		amdgpu_atombios_dp_aux_init(amdgpu_connector);
>  
> +	if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	    connector_type == DRM_MODE_CONNECTOR_eDP) {
> +		drm_mode_add_dp_subconnector_property(&amdgpu_connector->base);
> +	}
> +
>  	return;
>  
>  failed:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 37ba07e..04a430e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -469,6 +469,7 @@ struct amdgpu_encoder {
>  struct amdgpu_connector_atom_dig {
>  	/* displayport */
>  	u8 dpcd[DP_RECEIVER_CAP_SIZE];
> +	u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>  	u8 dp_sink_type;
>  	int dp_clock;
>  	int dp_lane_count;
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> index 9b74cfd..900b272 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> @@ -328,6 +328,22 @@ static void amdgpu_atombios_dp_probe_oui(struct amdgpu_connector *amdgpu_connect
>  			      buf[0], buf[1], buf[2]);
>  }
>  
> +static void amdgpu_atombios_dp_ds_ports(struct amdgpu_connector *amdgpu_connector)
> +{
> +	struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
> +	int ret;
> +
> +	if (dig_connector->dpcd[DP_DPCD_REV] > 0x10) {
> +		ret = drm_dp_dpcd_read(&amdgpu_connector->ddc_bus->aux,
> +				       DP_DOWNSTREAM_PORT_0,
> +				       dig_connector->downstream_ports,
> +				       DP_MAX_DOWNSTREAM_PORTS);
> +		if (ret)
> +			memset(dig_connector->downstream_ports, 0,
> +			       DP_MAX_DOWNSTREAM_PORTS);
> +	}
> +}
> +
>  int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
>  {
>  	struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
> @@ -343,7 +359,7 @@ int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
>  			      dig_connector->dpcd);
>  
>  		amdgpu_atombios_dp_probe_oui(amdgpu_connector);
> -
> +		amdgpu_atombios_dp_ds_ports(amdgpu_connector);
>  		return 0;
>  	}

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

Re: [Intel-gfx] [PATCH 4/5] drm/amdgpu: utilize subconnector property for DP through atombios

[Alex Deucher]

On Wed, Apr 15, 2020 at 6:05 AM Jani Nikula <jani.nikula@intel.com> wrote:
>
>
> Alex, Harry, Christian, can you please eyeball this series and see if it
> makes sense for you?
>

Patches 4, 5 are:
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Feel free to take them through whichever tree you want.

Alex


> Thanks,
> Jani.
>
>
> On Tue, 07 Apr 2020, Jeevan B <jeevan.b@intel.com> wrote:
> > From: Oleg Vasilev <oleg.vasilev@intel.com>
> >
> > Since DP-specific information is stored in driver's structures, every
> > driver needs to implement subconnector property by itself.
> >
> > v2: rebase
> >
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org
> > Signed-off-by: Jeevan B <jeevan.b@intel.com>
> > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-6-oleg.vasilev@intel.com
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 10 ++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h       |  1 +
> >  drivers/gpu/drm/amd/amdgpu/atombios_dp.c       | 18 +++++++++++++++++-
> >  3 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> > index f355d9a..71aade0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> > @@ -26,6 +26,7 @@
> >
> >  #include <drm/drm_edid.h>
> >  #include <drm/drm_fb_helper.h>
> > +#include <drm/drm_dp_helper.h>
> >  #include <drm/drm_probe_helper.h>
> >  #include <drm/amdgpu_drm.h>
> >  #include "amdgpu.h"
> > @@ -1405,6 +1406,10 @@ amdgpu_connector_dp_detect(struct drm_connector *connector, bool force)
> >               pm_runtime_put_autosuspend(connector->dev->dev);
> >       }
> >
> > +     drm_dp_set_subconnector_property(&amdgpu_connector->base,
> > +                                      ret,
> > +                                      amdgpu_dig_connector->dpcd,
> > +                                      amdgpu_dig_connector->downstream_ports);
> >       return ret;
> >  }
> >
> > @@ -1951,6 +1956,11 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> >       if (has_aux)
> >               amdgpu_atombios_dp_aux_init(amdgpu_connector);
> >
> > +     if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > +         connector_type == DRM_MODE_CONNECTOR_eDP) {
> > +             drm_mode_add_dp_subconnector_property(&amdgpu_connector->base);
> > +     }
> > +
> >       return;
> >
> >  failed:
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > index 37ba07e..04a430e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > @@ -469,6 +469,7 @@ struct amdgpu_encoder {
> >  struct amdgpu_connector_atom_dig {
> >       /* displayport */
> >       u8 dpcd[DP_RECEIVER_CAP_SIZE];
> > +     u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> >       u8 dp_sink_type;
> >       int dp_clock;
> >       int dp_lane_count;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> > index 9b74cfd..900b272 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> > @@ -328,6 +328,22 @@ static void amdgpu_atombios_dp_probe_oui(struct amdgpu_connector *amdgpu_connect
> >                             buf[0], buf[1], buf[2]);
> >  }
> >
> > +static void amdgpu_atombios_dp_ds_ports(struct amdgpu_connector *amdgpu_connector)
> > +{
> > +     struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
> > +     int ret;
> > +
> > +     if (dig_connector->dpcd[DP_DPCD_REV] > 0x10) {
> > +             ret = drm_dp_dpcd_read(&amdgpu_connector->ddc_bus->aux,
> > +                                    DP_DOWNSTREAM_PORT_0,
> > +                                    dig_connector->downstream_ports,
> > +                                    DP_MAX_DOWNSTREAM_PORTS);
> > +             if (ret)
> > +                     memset(dig_connector->downstream_ports, 0,
> > +                            DP_MAX_DOWNSTREAM_PORTS);
> > +     }
> > +}
> > +
> >  int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
> >  {
> >       struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
> > @@ -343,7 +359,7 @@ int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
> >                             dig_connector->dpcd);
> >
> >               amdgpu_atombios_dp_probe_oui(amdgpu_connector);
> > -
> > +             amdgpu_atombios_dp_ds_ports(amdgpu_connector);
> >               return 0;
> >       }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> 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

Re: [Intel-gfx] [PATCH 4/5] drm/amdgpu: utilize subconnector property for DP through atombios

[Jani Nikula]

On Thu, 16 Apr 2020, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, Apr 15, 2020 at 6:05 AM Jani Nikula <jani.nikula@intel.com> wrote:
>>
>>
>> Alex, Harry, Christian, can you please eyeball this series and see if it
>> makes sense for you?
>>
>
> Patches 4, 5 are:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Feel free to take them through whichever tree you want.

Thanks a bunch! I'll let you know.

BR,
Jani.

>
> Alex
>
>
>> Thanks,
>> Jani.
>>
>>
>> On Tue, 07 Apr 2020, Jeevan B <jeevan.b@intel.com> wrote:
>> > From: Oleg Vasilev <oleg.vasilev@intel.com>
>> >
>> > Since DP-specific information is stored in driver's structures, every
>> > driver needs to implement subconnector property by itself.
>> >
>> > v2: rebase
>> >
>> > Cc: Alex Deucher <alexander.deucher@amd.com>
>> > Cc: Christian König <christian.koenig@amd.com>
>> > Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
>> > Cc: amd-gfx@lists.freedesktop.org
>> > Signed-off-by: Jeevan B <jeevan.b@intel.com>
>> > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
>> > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
>> > Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-6-oleg.vasilev@intel.com
>> > ---
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 10 ++++++++++
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h       |  1 +
>> >  drivers/gpu/drm/amd/amdgpu/atombios_dp.c       | 18 +++++++++++++++++-
>> >  3 files changed, 28 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> > index f355d9a..71aade0 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> > @@ -26,6 +26,7 @@
>> >
>> >  #include <drm/drm_edid.h>
>> >  #include <drm/drm_fb_helper.h>
>> > +#include <drm/drm_dp_helper.h>
>> >  #include <drm/drm_probe_helper.h>
>> >  #include <drm/amdgpu_drm.h>
>> >  #include "amdgpu.h"
>> > @@ -1405,6 +1406,10 @@ amdgpu_connector_dp_detect(struct drm_connector *connector, bool force)
>> >               pm_runtime_put_autosuspend(connector->dev->dev);
>> >       }
>> >
>> > +     drm_dp_set_subconnector_property(&amdgpu_connector->base,
>> > +                                      ret,
>> > +                                      amdgpu_dig_connector->dpcd,
>> > +                                      amdgpu_dig_connector->downstream_ports);
>> >       return ret;
>> >  }
>> >
>> > @@ -1951,6 +1956,11 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>> >       if (has_aux)
>> >               amdgpu_atombios_dp_aux_init(amdgpu_connector);
>> >
>> > +     if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>> > +         connector_type == DRM_MODE_CONNECTOR_eDP) {
>> > +             drm_mode_add_dp_subconnector_property(&amdgpu_connector->base);
>> > +     }
>> > +
>> >       return;
>> >
>> >  failed:
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> > index 37ba07e..04a430e 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> > @@ -469,6 +469,7 @@ struct amdgpu_encoder {
>> >  struct amdgpu_connector_atom_dig {
>> >       /* displayport */
>> >       u8 dpcd[DP_RECEIVER_CAP_SIZE];
>> > +     u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>> >       u8 dp_sink_type;
>> >       int dp_clock;
>> >       int dp_lane_count;
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
>> > index 9b74cfd..900b272 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
>> > @@ -328,6 +328,22 @@ static void amdgpu_atombios_dp_probe_oui(struct amdgpu_connector *amdgpu_connect
>> >                             buf[0], buf[1], buf[2]);
>> >  }
>> >
>> > +static void amdgpu_atombios_dp_ds_ports(struct amdgpu_connector *amdgpu_connector)
>> > +{
>> > +     struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
>> > +     int ret;
>> > +
>> > +     if (dig_connector->dpcd[DP_DPCD_REV] > 0x10) {
>> > +             ret = drm_dp_dpcd_read(&amdgpu_connector->ddc_bus->aux,
>> > +                                    DP_DOWNSTREAM_PORT_0,
>> > +                                    dig_connector->downstream_ports,
>> > +                                    DP_MAX_DOWNSTREAM_PORTS);
>> > +             if (ret)
>> > +                     memset(dig_connector->downstream_ports, 0,
>> > +                            DP_MAX_DOWNSTREAM_PORTS);
>> > +     }
>> > +}
>> > +
>> >  int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
>> >  {
>> >       struct amdgpu_connector_atom_dig *dig_connector = amdgpu_connector->con_priv;
>> > @@ -343,7 +359,7 @@ int amdgpu_atombios_dp_get_dpcd(struct amdgpu_connector *amdgpu_connector)
>> >                             dig_connector->dpcd);
>> >
>> >               amdgpu_atombios_dp_probe_oui(amdgpu_connector);
>> > -
>> > +             amdgpu_atombios_dp_ds_ports(amdgpu_connector);
>> >               return 0;
>> >       }
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 1/5] drm: report dp downstream port type as a subconnector property

[Jeevan B]

On 2020-04-15 at 13:01:21 +0300, Jani Nikula wrote:
> On Tue, 07 Apr 2020, Jeevan B <jeevan.b@intel.com> wrote:
> > From: Oleg Vasilev <oleg.vasilev@intel.com>
> >
> > 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
> >
> > v5: rebase
> >
> > [1]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Jeevan B <jeevan.b@intel.com>
> > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-3-oleg.vasilev@intel.com
> 
> High level looks fine to me, please see some nitpicks inline.
> 
> BR,
> Jani.
> 
> 
> > ---
> >  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)
> > +{
> 
> So far most functions dealing with properties are named:
> 
> - drm_mode_create_*
> - drm_connector_attach_*
> 
> Sometimes the latter calling the former, sometimes creating inline.
> 
> drm_mode_add_* that attaches a property on a connector is confusing.
> 
> Perhaps call it drm_connector_attach_dp_subconnector_property().
>
Sure, I will change this.  
> > +	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
> 
> This does not help at all. What should this point at?
> 
> > + *
> > + */
> > +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
> 
> Unhelpful documentation. Sure port_cap is port capabilities, but what
> should this point at? (I can tell looking at the code, but what's the
> point of documentation if you have to look at the code?)
> 
I will correct this. 
Oleg might have added this just like in other functions as below
/**
 * drm_dp_downstream_debug() - debug DP branch devices
 * @m: pointer for debugfs file
 * @dpcd: DisplayPort configuration data
 * @port_cap: port capabilities
 * @aux: DisplayPort AUX channel
 *
 */

> > + *
> > + * 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])
> 
> Inconsistent parameters.
> 
> > +{
> > +	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>
> 
> Forward declarations instead of includes if possible, please.
>
I will change it.  
> >  
> >  /*
> >   * 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]);
> 
> Inconsistent parameters.
> 
> >  
> >  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
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/i915: utilize subconnector property for DP

[Jeevan B]

On 2020-04-15 at 13:01:59 +0300, Jani Nikula wrote:
> On Tue, 07 Apr 2020, Jeevan B <jeevan.b@intel.com> wrote:
> > From: Oleg Vasilev <oleg.vasilev@intel.com>
> >
> > Since DP-specific information is stored in driver's structures, every
> > driver needs to implement subconnector property by itself.
> >
> > v2: updates to match previous commit changes
> >
> > v3: rebase
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Jeevan B <jeevan.b@intel.com>
> > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> > Tested-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20190829114854.1539-4-oleg.vasilev@intel.com
> 
> You're not supposed to add the Link: tag yourself.
I will do the necessary change. 
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Thanks
Jeevan B
> 
>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index db6ae8e..ba443e1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -6155,6 +6155,11 @@ intel_dp_detect(struct drm_connector *connector,
> >  	 */
> >  	intel_display_power_flush_work(dev_priv);
> >  
> > +	if (!intel_dp_is_edp(intel_dp))
> > +		drm_dp_set_subconnector_property(connector,
> > +						 status,
> > +						 intel_dp->dpcd,
> > +						 intel_dp->downstream_ports);
> >  	return status;
> >  }
> >  
> > @@ -7211,6 +7216,9 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> >  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >  	enum port port = dp_to_dig_port(intel_dp)->base.port;
> >  
> > +	if (!intel_dp_is_edp(intel_dp))
> > +		drm_mode_add_dp_subconnector_property(connector);
> > +
> >  	if (!IS_G4X(dev_priv) && port != PORT_A)
> >  		intel_attach_force_audio_property(connector);
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/5] drm: report dp downstream port type as a subconnector property

[Jeevan B]

From: Oleg Vasilev <oleg.vasilev@intel.com>

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

v5: rebase

v6:
 - Jani Nikula: renamed a function name
 - Jani Nikula: addressed the issues with documentation

[1]: https://bugs.freedesktop.org/show_bug.cgi?id=104097

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Jeevan B <jeevan.b@intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/drm_connector.c | 49 ++++++++++++++++++++++++++--
 drivers/gpu/drm/drm_dp_helper.c | 71 +++++++++++++++++++++++++++++++++++++++++
 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, 148 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b1099e1..58f78c6 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_connector_attach_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_connector_attach_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_connector_attach_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 612a59e..f34339ee 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -594,6 +594,77 @@ void drm_dp_downstream_debug(struct seq_file *m,
 }
 EXPORT_SYMBOL(drm_dp_downstream_debug);
 
+/**
+ * drm_dp_subconnector_type() - get DP branch device type
+ *
+ */
+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
+ *
+ * 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..d4d3bed 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_connector_attach_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 3beb2aa..0f5432c 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
@@ -1608,6 +1609,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

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