drm/dp: device identification and quirks ~v3

Face the fact, there are Display Port sink and branch devices out there in the wild that don't follow the Display Port specifications, or they have bugs, or just otherwise require special treatment. Start a common quirk database the drivers can query based on the DP device identification. At least for now, we leave the workarounds for the drivers to implement as they see fit.

For starters, add a branch device that can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. Naturally, the workaround of reducing main link attributes for all devices ended up in regressions for other devices. So here we are.

v2: Rebase on DRM DP desc read helpers

v3: Fix the OUI memcmp blunder (Clint)

Cc Ville Syrjälä <[email protected]>

Cc Dhinakaran Pandiyan <[email protected]>

Cc Clint Taylor <[email protected]>

Cc Adam Jackson <[email protected]>

Cc Harry Wentland <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
  include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
  2 files changed, 82 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_dp_helper.c +50 -2

index 52e0ca9a5bb1..213fb837e1c4 100644
@@ -1209,6 +1209,51 @@ int drm_dp_stop_crc(struct drm_dp_aux *aux)
  }
  EXPORT_SYMBOL(drm_dp_stop_crc);
 
+struct dpcd_quirk {
+	u8 oui[3];
+	bool is_branch;
+	u32 quirks;
+};
+
+#define OUI(first, second, third) { (first), (second), (third) }
+
+static const struct dpcd_quirk dpcd_quirk_list[] = {
+	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
+	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+};
+
+#undef OUI
+
+/*
+ * Get a bit mask of DPCD quirks for the sink/branch device identified by
+ * ident. The quirk data is shared but it's up to the drivers to act on the
+ * data.
+ *
+ * For now, only the OUI (first three bytes) is used, but this may be extended
+ * to device identification string and hardware/firmware revisions later.
+ */
+static u32
+drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
+{
+	const struct dpcd_quirk *quirk;
+	u32 quirks = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
+		quirk = &dpcd_quirk_list[i];
+
+		if (quirk->is_branch != is_branch)
+			continue;
+
+		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
+			continue;
+
+		quirks |= quirk->quirks;
+	}
+
+	return quirks;
+}
+
  /**
   * drm_dp_read_desc - read sink/branch descriptor from DPCD
   * @aux: DisplayPort AUX channel

@@ -1231,14 +1276,17 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
  	if (ret < 0)
  		return ret;
 
+	desc->quirks = drm_dp_get_quirks(ident, is_branch);
+
  	dev_id_len = strnlen(ident->device_id, sizeof(ident->device_id));
 
-	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d\n",
+	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
  		      is_branch ? "branch" : "sink",
  		      (int)sizeof(ident->oui), ident->oui,
  		      dev_id_len, ident->device_id,
  		      ident->hw_rev >> 4, ident->hw_rev & 0xf,
-		      ident->sw_major_rev, ident->sw_minor_rev);
+		      ident->sw_major_rev, ident->sw_minor_rev,
+		      desc->quirks);
 
  	return 0;
  }

include/drm/drm_dp_helper.h +23 -0

index aee5b96b51d7..717cb8496725 100644
@@ -1090,12 +1090,44 @@ struct drm_dp_dpcd_ident {
  /**
   * struct drm_dp_desc - DP branch/sink device descriptor
   * @ident: DP device identification from DPCD 0x400 (sink) or 0x500 (branch).
+ * @quirks: Quirks; use drm_dp_has_quirk() to query for the quirks.
   */
  struct drm_dp_desc {
  	struct drm_dp_dpcd_ident ident;
+	u32 quirks;
  };
 
  int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
  		     bool is_branch);
 
+/**
+ * enum drm_dp_quirk - Display Port sink/branch device specific quirks
+ *
+ * Display Port sink and branch devices in the wild have a variety of bugs, try
+ * to collect them here. The quirks are shared, but it's up to the drivers to
+ * implement workarounds for them.
+ */
+enum drm_dp_quirk {
+	/**
+	 * @DP_DPCD_QUIRK_LIMITED_M_N:
+	 *
+	 * The device requires main link attributes Mdiv and Ndiv to be limited
+	 * to 16 bits.
+	 */
+	DP_DPCD_QUIRK_LIMITED_M_N,
+};
+
+/**
+ * drm_dp_has_quirk() - does the DP device have a specific quirk
+ * @desc: Device decriptor filled by drm_dp_read_desc()
+ * @quirk: Quirk to query for

Face the fact, there are Display Port sink and branch devices out there in the wild that don't follow the Display Port specifications, or they have bugs, or just otherwise require special treatment. Start a common quirk database the drivers can query based on the DP device identification. At least for now, we leave the workarounds for the drivers to implement as they see fit.

For starters, add a branch device that can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. Naturally, the workaround of reducing main link attributes for all devices ended up in regressions for other devices. So here we are.

v2: Rebase on DRM DP desc read helpers

v3: Fix the OUI memcmp blunder (Clint)

Cc Ville Syrjälä <[email protected]>

Cc Dhinakaran Pandiyan <[email protected]>

Cc Clint Taylor <[email protected]>

Cc Adam Jackson <[email protected]>

Cc Harry Wentland <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_dp_helper.c +0 -1

index 52e0ca9a5bb1..213fb837e1c4 100644
--- a/drivers/gpu/drm/drm_dp_helper.c

Face the fact, there are Display Port sink and branch devices out there in the wild that don't follow the Display Port specifications, or they have bugs, or just otherwise require special treatment. Start a common quirk database the drivers can query based on the DP device identification. At least for now, we leave the workarounds for the drivers to implement as they see fit.

For starters, add a branch device that can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. Naturally, the workaround of reducing main link attributes for all devices ended up in regressions for other devices. So here we are.

v2: Rebase on DRM DP desc read helpers

v3: Fix the OUI memcmp blunder (Clint)

Cc Ville Syrjälä <[email protected]>

Cc Dhinakaran Pandiyan <[email protected]>

Cc Clint Taylor <[email protected]>

Cc Adam Jackson <[email protected]>

Cc Harry Wentland <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_dp_helper.c +5 -0

index 52e0ca9a5bb1..213fb837e1c4 100644
@@ -1209,6 +1209,51 @@ int drm_dp_stop_crc(struct drm_dp_aux *aux)
 }
 EXPORT_SYMBOL(drm_dp_stop_crc);
 
+struct dpcd_quirk {
+	u8 oui[3];
+	bool is_branch;
+	u32 quirks;
+};

drivers/gpu/drm/drm_dp_helper.c +35 -2

+/*
+ * Get a bit mask of DPCD quirks for the sink/branch device identified by
+ * ident. The quirk data is shared but it's up to the drivers to act on the
+ * data.
+ *
+ * For now, only the OUI (first three bytes) is used, but this may be extended
+ * to device identification string and hardware/firmware revisions later.
+ */
+static u32
+drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
+{
+	const struct dpcd_quirk *quirk;
+	u32 quirks = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
+		quirk = &dpcd_quirk_list[i];
+
+		if (quirk->is_branch != is_branch)
+			continue;
+
+		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
+			continue;
+
+		quirks |= quirk->quirks;
+	}
+
+	return quirks;
+}
+
 /**
  * drm_dp_read_desc - read sink/branch descriptor from DPCD
  * @aux: DisplayPort AUX channel

@@ -1231,14 +1276,17 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
 	if (ret < 0)
 		return ret;
 
+	desc->quirks = drm_dp_get_quirks(ident, is_branch);
+
 	dev_id_len = strnlen(ident->device_id, sizeof(ident->device_id));
 
-	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d\n",
+	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
 		      is_branch ? "branch" : "sink",
 		      (int)sizeof(ident->oui), ident->oui,
 		      dev_id_len, ident->device_id,
 		      ident->hw_rev >> 4, ident->hw_rev & 0xf,
-		      ident->sw_major_rev, ident->sw_minor_rev);
+		      ident->sw_major_rev, ident->sw_minor_rev,
+		      desc->quirks);
 
 	return 0;
 }

include/drm/drm_dp_helper.h +23 -0

index aee5b96b51d7..717cb8496725 100644
@@ -1090,12 +1090,44 @@ struct drm_dp_dpcd_ident {
 /**
  * struct drm_dp_desc - DP branch/sink device descriptor
  * @ident: DP device identification from DPCD 0x400 (sink) or 0x500 (branch).
+ * @quirks: Quirks; use drm_dp_has_quirk() to query for the quirks.
  */
 struct drm_dp_desc {
 	struct drm_dp_dpcd_ident ident;
+	u32 quirks;
 };
 
 int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
 		     bool is_branch);
 
+/**
+ * enum drm_dp_quirk - Display Port sink/branch device specific quirks
+ *
+ * Display Port sink and branch devices in the wild have a variety of bugs, try
+ * to collect them here. The quirks are shared, but it's up to the drivers to
+ * implement workarounds for them.
+ */
+enum drm_dp_quirk {
+	/**
+	 * @DP_DPCD_QUIRK_LIMITED_M_N:
+	 *
+	 * The device requires main link attributes Mdiv and Ndiv to be limited
+	 * to 16 bits.
+	 */
+	DP_DPCD_QUIRK_LIMITED_M_N,
+};
+
+/**
+ * drm_dp_has_quirk() - does the DP device have a specific quirk
+ * @desc: Device decriptor filled by drm_dp_read_desc()
+ * @quirk: Quirk to query for

Quoted 40 lines · Jani Nikula

Face the fact, there are Display Port sink and branch devices out there in the wild that don't follow the Display Port specifications, or they have bugs, or just otherwise require special treatment. Start a common quirk database the drivers can query based on the DP device identification. At least for now, we leave the workarounds for the drivers to implement as they see fit.

For starters, add a branch device that can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. Naturally, the workaround of reducing main link attributes for all devices ended up in regressions for other devices. So here we are.

v2: Rebase on DRM DP desc read helpers

v3: Fix the OUI memcmp blunder (Clint)

Cc Ville Syrjälä <[email protected]>

Cc Dhinakaran Pandiyan <[email protected]>

Cc Clint Taylor <[email protected]>

Cc Adam Jackson <[email protected]>

Cc Harry Wentland <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_dp_helper.c +5 -0

index 52e0ca9a5bb1..213fb837e1c4 100644
@@ -1209,6 +1209,51 @@ int drm_dp_stop_crc(struct drm_dp_aux *aux)
 }
 EXPORT_SYMBOL(drm_dp_stop_crc);
 
+struct dpcd_quirk {
+	u8 oui[3];
+	bool is_branch;
+	u32 quirks;
+};

drivers/gpu/drm/drm_dp_helper.c int drm_dp_stop_crc(struct drm_dp_aux *aux) · line 1226

Quoted 10 lines

drivers/gpu/drm/drm_dp_helper.c +10 -0

+
+#define OUI(first, second, third) { (first), (second), (third) }
+
+static const struct dpcd_quirk dpcd_quirk_list[] = {
+	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
+	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+};
+
+#undef OUI
+

Wouldn't be more clear this way:

#define OUI_ANALOGIX {0x00, 0x22, 0xb9}

static const struct dpcd_quirk dpcd_quirk_list[] = {
{ OUI_ANALOGIX, true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
};

Anyway, it seems the quirk is for all analogix branch devs, not only ANX7737, is it correct?

Regards
Andrzej

Quoted 102 lines

drivers/gpu/drm/drm_dp_helper.c +35 -2

+/*
+ * Get a bit mask of DPCD quirks for the sink/branch device identified by
+ * ident. The quirk data is shared but it's up to the drivers to act on the
+ * data.
+ *
+ * For now, only the OUI (first three bytes) is used, but this may be extended
+ * to device identification string and hardware/firmware revisions later.
+ */
+static u32
+drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
+{
+	const struct dpcd_quirk *quirk;
+	u32 quirks = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
+		quirk = &dpcd_quirk_list[i];
+
+		if (quirk->is_branch != is_branch)
+			continue;
+
+		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
+			continue;
+
+		quirks |= quirk->quirks;
+	}
+
+	return quirks;
+}
+
 /**
  * drm_dp_read_desc - read sink/branch descriptor from DPCD
  * @aux: DisplayPort AUX channel

@@ -1231,14 +1276,17 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
 	if (ret < 0)
 		return ret;
 
+	desc->quirks = drm_dp_get_quirks(ident, is_branch);
+
 	dev_id_len = strnlen(ident->device_id, sizeof(ident->device_id));
 
-	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d\n",
+	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
 		      is_branch ? "branch" : "sink",
 		      (int)sizeof(ident->oui), ident->oui,
 		      dev_id_len, ident->device_id,
 		      ident->hw_rev >> 4, ident->hw_rev & 0xf,
-		      ident->sw_major_rev, ident->sw_minor_rev);
+		      ident->sw_major_rev, ident->sw_minor_rev,
+		      desc->quirks);
 
 	return 0;
 }

include/drm/drm_dp_helper.h +32 -0

index aee5b96b51d7..717cb8496725 100644
@@ -1090,12 +1090,44 @@ struct drm_dp_dpcd_ident {
 /**
  * struct drm_dp_desc - DP branch/sink device descriptor
  * @ident: DP device identification from DPCD 0x400 (sink) or 0x500 (branch).
+ * @quirks: Quirks; use drm_dp_has_quirk() to query for the quirks.
  */
 struct drm_dp_desc {
 	struct drm_dp_dpcd_ident ident;
+	u32 quirks;
 };
 
 int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
 		     bool is_branch);
 
+/**
+ * enum drm_dp_quirk - Display Port sink/branch device specific quirks
+ *
+ * Display Port sink and branch devices in the wild have a variety of bugs, try
+ * to collect them here. The quirks are shared, but it's up to the drivers to
+ * implement workarounds for them.
+ */
+enum drm_dp_quirk {
+	/**
+	 * @DP_DPCD_QUIRK_LIMITED_M_N:
+	 *
+	 * The device requires main link attributes Mdiv and Ndiv to be limited
+	 * to 16 bits.
+	 */
+	DP_DPCD_QUIRK_LIMITED_M_N,
+};
+
+/**
+ * drm_dp_has_quirk() - does the DP device have a specific quirk
+ * @desc: Device decriptor filled by drm_dp_read_desc()
+ * @quirk: Quirk to query for
+ *
+ * Return true if DP device identified by @desc has @quirk.
+ */
+static inline bool
+drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
+{
+	return desc->quirks & BIT(quirk);
+}
+
 #endif /* _DRM_DP_HELPER_H_ */

Quoted 12 lines · Jani Nikula

Face the fact, there are Display Port sink and branch devices out there in the wild that don't follow the Display Port specifications, or they have bugs, or just otherwise require special treatment. Start a common quirk database the drivers can query based on the DP device identification. At least for now, we leave the workarounds for the drivers to implement as they see fit.

For starters, add a branch device that can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. Naturally, the workaround of reducing main link attributes for all devices ended up in regressions for other devices. So here we are.

Quoted 3 lines

v2: Rebase on DRM DP desc read helpers

v3: Fix the OUI memcmp blunder (Clint)

Tested-by Clinton Taylor <[email protected]>

Reviewed-by Clinton Taylor <[email protected]>

Quoted 138 lines

Cc Ville Syrjälä <[email protected]>

Cc Dhinakaran Pandiyan <[email protected]>

Cc Clint Taylor <[email protected]>

Cc Adam Jackson <[email protected]>

Cc Harry Wentland <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
  drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
  include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
  2 files changed, 82 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_dp_helper.c +50 -2

index 52e0ca9a5bb1..213fb837e1c4 100644
@@ -1209,6 +1209,51 @@ int drm_dp_stop_crc(struct drm_dp_aux *aux)
  }
  EXPORT_SYMBOL(drm_dp_stop_crc);
 
+struct dpcd_quirk {
+	u8 oui[3];
+	bool is_branch;
+	u32 quirks;
+};
+
+#define OUI(first, second, third) { (first), (second), (third) }
+
+static const struct dpcd_quirk dpcd_quirk_list[] = {
+	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
+	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+};
+
+#undef OUI
+
+/*
+ * Get a bit mask of DPCD quirks for the sink/branch device identified by
+ * ident. The quirk data is shared but it's up to the drivers to act on the
+ * data.
+ *
+ * For now, only the OUI (first three bytes) is used, but this may be extended
+ * to device identification string and hardware/firmware revisions later.
+ */
+static u32
+drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
+{
+	const struct dpcd_quirk *quirk;
+	u32 quirks = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
+		quirk = &dpcd_quirk_list[i];
+
+		if (quirk->is_branch != is_branch)
+			continue;
+
+		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
+			continue;
+
+		quirks |= quirk->quirks;
+	}
+
+	return quirks;
+}
+
  /**
   * drm_dp_read_desc - read sink/branch descriptor from DPCD
   * @aux: DisplayPort AUX channel

@@ -1231,14 +1276,17 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
  	if (ret < 0)
  		return ret;
 
+	desc->quirks = drm_dp_get_quirks(ident, is_branch);
+
  	dev_id_len = strnlen(ident->device_id, sizeof(ident->device_id));
 
-	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d\n",
+	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
  		      is_branch ? "branch" : "sink",
  		      (int)sizeof(ident->oui), ident->oui,
  		      dev_id_len, ident->device_id,
  		      ident->hw_rev >> 4, ident->hw_rev & 0xf,
-		      ident->sw_major_rev, ident->sw_minor_rev);
+		      ident->sw_major_rev, ident->sw_minor_rev,
+		      desc->quirks);
 
  	return 0;
  }

include/drm/drm_dp_helper.h +32 -0

index aee5b96b51d7..717cb8496725 100644
@@ -1090,12 +1090,44 @@ struct drm_dp_dpcd_ident {
  /**
   * struct drm_dp_desc - DP branch/sink device descriptor
   * @ident: DP device identification from DPCD 0x400 (sink) or 0x500 (branch).
+ * @quirks: Quirks; use drm_dp_has_quirk() to query for the quirks.
   */
  struct drm_dp_desc {
  	struct drm_dp_dpcd_ident ident;
+	u32 quirks;
  };
 
  int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
  		     bool is_branch);
 
+/**
+ * enum drm_dp_quirk - Display Port sink/branch device specific quirks
+ *
+ * Display Port sink and branch devices in the wild have a variety of bugs, try
+ * to collect them here. The quirks are shared, but it's up to the drivers to
+ * implement workarounds for them.
+ */
+enum drm_dp_quirk {
+	/**
+	 * @DP_DPCD_QUIRK_LIMITED_M_N:
+	 *
+	 * The device requires main link attributes Mdiv and Ndiv to be limited
+	 * to 16 bits.
+	 */
+	DP_DPCD_QUIRK_LIMITED_M_N,
+};
+
+/**
+ * drm_dp_has_quirk() - does the DP device have a specific quirk
+ * @desc: Device decriptor filled by drm_dp_read_desc()
+ * @quirk: Quirk to query for
+ *
+ * Return true if DP device identified by @desc has @quirk.
+ */
+static inline bool
+drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
+{
+	return desc->quirks & BIT(quirk);
+}
+
  #endif /* _DRM_DP_HELPER_H_ */

Quoted 30 lines · Jani Nikula

Face the fact, there are Display Port sink and branch devices out there in the wild that don't follow the Display Port specifications, or they have bugs, or just otherwise require special treatment. Start a common quirk database the drivers can query based on the DP device identification. At least for now, we leave the workarounds for the drivers to implement as they see fit.

For starters, add a branch device that can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. Naturally, the workaround of reducing main link attributes for all devices ended up in regressions for other devices. So here we are.

v2: Rebase on DRM DP desc read helpers

v3: Fix the OUI memcmp blunder (Clint)

Cc Ville Syrjälä <[email protected]>

Cc Dhinakaran Pandiyan <[email protected]>

Cc Clint Taylor <[email protected]>

Cc Adam Jackson <[email protected]>

Cc Harry Wentland <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_dp_helper.c +0 -1

index 52e0ca9a5bb1..213fb837e1c4 100644
--- a/drivers/gpu/drm/drm_dp_helper.c

<snip>

drivers/gpu/drm/drm_dp_helper.c

Quoted 11 lines

+ * enum drm_dp_quirk - Display Port sink/branch device specific quirks
+ *
+ * Display Port sink and branch devices in the wild have a variety of bugs, try
+ * to collect them here. The quirks are shared, but it's up to the drivers to
+ * implement workarounds for them.
+ */

drivers/gpu/drm/drm_dp_helper.c +5 -0

+enum drm_dp_quirk {
+	/**
+	 * @DP_DPCD_QUIRK_LIMITED_M_N:
+	 *
+	 * The device requires main link attributes Mdiv and Ndiv to be limited

s/Mdiv/Mvid
s/Ndiv/Nvid

Quoted 5 lines

drivers/gpu/drm/drm_dp_helper.c +4 -0

+	 * to 16 bits.
+	 */
+	DP_DPCD_QUIRK_LIMITED_M_N,
+};

Quoted 15 lines · Clint Taylor

Quoted 14 lines · Jani Nikula

Face the fact, there are Display Port sink and branch devices out there in the wild that don't follow the Display Port specifications, or they have bugs, or just otherwise require special treatment. Start a common quirk database the drivers can query based on the DP device identification. At least for now, we leave the workarounds for the drivers to implement as they see fit.

For starters, add a branch device that can't handle full 24-bit main link Mdiv and Ndiv main link attributes properly. Naturally, the workaround of reducing main link attributes for all devices ended up in regressions for other devices. So here we are.

v2: Rebase on DRM DP desc read helpers

Quoted 3 lines

Quoted 1 line

v3: Fix the OUI memcmp blunder (Clint)

Tested-by Clinton Taylor <[email protected]>

Reviewed-by Clinton Taylor <[email protected]>

I pushed the series to drm-intel topic/dp-quirks branch based on v4.12-rc3, with the goal of merging this to v4.12. Thanks for the review and testing so far; would you mind giving that branch a go too, to ensure I didn't screw anything up while applying?

BR,
Jani.

Quoted 128 lines

Quoted 128 lines

Cc Ville Syrjälä <[email protected]>

Cc Dhinakaran Pandiyan <[email protected]>

Cc Clint Taylor <[email protected]>

Cc Adam Jackson <[email protected]>

Cc Harry Wentland <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---
   drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
   include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
   2 files changed, 82 insertions(+), 2 deletions(-)

drivers/gpu/drm/drm_dp_helper.c +50 -2

index 52e0ca9a5bb1..213fb837e1c4 100644
@@ -1209,6 +1209,51 @@ int drm_dp_stop_crc(struct drm_dp_aux *aux)
   }
   EXPORT_SYMBOL(drm_dp_stop_crc);
 
+struct dpcd_quirk {
+	u8 oui[3];
+	bool is_branch;
+	u32 quirks;
+};
+
+#define OUI(first, second, third) { (first), (second), (third) }
+
+static const struct dpcd_quirk dpcd_quirk_list[] = {
+	/* Analogix 7737 needs reduced M and N at HBR2 link rates */
+	{ OUI(0x00, 0x22, 0xb9), true, BIT(DP_DPCD_QUIRK_LIMITED_M_N) },
+};
+
+#undef OUI
+
+/*
+ * Get a bit mask of DPCD quirks for the sink/branch device identified by
+ * ident. The quirk data is shared but it's up to the drivers to act on the
+ * data.
+ *
+ * For now, only the OUI (first three bytes) is used, but this may be extended
+ * to device identification string and hardware/firmware revisions later.
+ */
+static u32
+drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch)
+{
+	const struct dpcd_quirk *quirk;
+	u32 quirks = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dpcd_quirk_list); i++) {
+		quirk = &dpcd_quirk_list[i];
+
+		if (quirk->is_branch != is_branch)
+			continue;
+
+		if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
+			continue;
+
+		quirks |= quirk->quirks;
+	}
+
+	return quirks;
+}
+
   /**
    * drm_dp_read_desc - read sink/branch descriptor from DPCD
    * @aux: DisplayPort AUX channel

@@ -1231,14 +1276,17 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
   	if (ret < 0)
   		return ret;
 
+	desc->quirks = drm_dp_get_quirks(ident, is_branch);
+
   	dev_id_len = strnlen(ident->device_id, sizeof(ident->device_id));
 
-	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d\n",
+	DRM_DEBUG_KMS("DP %s: OUI %*phD dev-ID %*pE HW-rev %d.%d SW-rev %d.%d quirks 0x%04x\n",
   		      is_branch ? "branch" : "sink",
   		      (int)sizeof(ident->oui), ident->oui,
   		      dev_id_len, ident->device_id,
   		      ident->hw_rev >> 4, ident->hw_rev & 0xf,
-		      ident->sw_major_rev, ident->sw_minor_rev);
+		      ident->sw_major_rev, ident->sw_minor_rev,
+		      desc->quirks);
 
   	return 0;
   }

include/drm/drm_dp_helper.h +25 -0

index aee5b96b51d7..717cb8496725 100644
@@ -1090,12 +1090,44 @@ struct drm_dp_dpcd_ident {
   /**
    * struct drm_dp_desc - DP branch/sink device descriptor
    * @ident: DP device identification from DPCD 0x400 (sink) or 0x500 (branch).
+ * @quirks: Quirks; use drm_dp_has_quirk() to query for the quirks.
    */
   struct drm_dp_desc {
   	struct drm_dp_dpcd_ident ident;
+	u32 quirks;
   };
 
   int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
   		     bool is_branch);
 
+/**
+ * enum drm_dp_quirk - Display Port sink/branch device specific quirks
+ *
+ * Display Port sink and branch devices in the wild have a variety of bugs, try
+ * to collect them here. The quirks are shared, but it's up to the drivers to
+ * implement workarounds for them.
+ */
+enum drm_dp_quirk {
+	/**
+	 * @DP_DPCD_QUIRK_LIMITED_M_N:
+	 *
+	 * The device requires main link attributes Mdiv and Ndiv to be limited
+	 * to 16 bits.
+	 */
+	DP_DPCD_QUIRK_LIMITED_M_N,
+};
+
+/**
+ * drm_dp_has_quirk() - does the DP device have a specific quirk
+ * @desc: Device decriptor filled by drm_dp_read_desc()
+ * @quirk: Quirk to query for
+ *
+ * Return true if DP device identified by @desc has @quirk.

Quoted 8 lines

include/drm/drm_dp_helper.h struct drm_dp_dpcd_ident { · line 1133

Quoted 8 lines

include/drm/drm_dp_helper.h +7 -0

+ */
+static inline bool
+drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
+{
+	return desc->quirks & BIT(quirk);
+}
+
   #endif /* _DRM_DP_HELPER_H_ */