[PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[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 M and N 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

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Cc: Adam Jackson <ajax@redhat.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 52e0ca9a5bb1..8c3797283c3b 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -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;
 }
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index aee5b96b51d7..717cb8496725 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -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_ */
-- 
2.11.0

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

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Clint Taylor]

On 05/17/2017 07:25 AM, Jani Nikula wrote:
> 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 M and N 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
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>   include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>   2 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 52e0ca9a5bb1..8c3797283c3b 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -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;

Oops, All branch devices appear to have the quirk applied. The memcmp 
should be closed before the !=0

if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)


[  659.496861] [drm:drm_dp_read_desc] DP branch: OUI 00-1c-f8 dev-ID 
175IB0 HW-rev 1.0 SW-rev 7.32 quirks 0x0001
[  659.549017] [drm:drm_dp_read_desc] DP branch: OUI 00-22-b9 dev-ID 
7737 HW-rev 10.12 SW-rev 6.4 quirks 0x0001
[  659.630449] [drm:drm_dp_read_desc] DP sink: OUI ee-ff-c0 dev-ID \001 
HW-rev 0.0 SW-rev 0.0 quirks 0x0000

This was actually good to find out that LSPCON didn't like the reduced M 
and N from the quirk at HBR2.

-Clint

> +
> +		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;
>   }
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index aee5b96b51d7..717cb8496725 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -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_ */

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

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Jani Nikula]

On Wed, 17 May 2017, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> On 05/17/2017 07:25 AM, Jani Nikula wrote:
>> 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 M and N 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
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>> Cc: Adam Jackson <ajax@redhat.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>>   include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>>   2 files changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 52e0ca9a5bb1..8c3797283c3b 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -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;
>
> Oops, All branch devices appear to have the quirk applied. The memcmp 
> should be closed before the !=0

Now that's embarrassing. Thanks for catching this.

BR,
Jani.


>
> if (memcmp(quirk->oui, ident->oui, sizeof(ident->oui)) != 0)
>
>
> [  659.496861] [drm:drm_dp_read_desc] DP branch: OUI 00-1c-f8 dev-ID 
> 175IB0 HW-rev 1.0 SW-rev 7.32 quirks 0x0001
> [  659.549017] [drm:drm_dp_read_desc] DP branch: OUI 00-22-b9 dev-ID 
> 7737 HW-rev 10.12 SW-rev 6.4 quirks 0x0001
> [  659.630449] [drm:drm_dp_read_desc] DP sink: OUI ee-ff-c0 dev-ID \001 
> HW-rev 0.0 SW-rev 0.0 quirks 0x0000
>
> This was actually good to find out that LSPCON didn't like the reduced M 
> and N from the quirk at HBR2.
>
> -Clint
>
>> +
>> +		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;
>>   }
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index aee5b96b51d7..717cb8496725 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -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_ */
>

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

[PATCH 0/4] drm/dp: device identification and quirks ~v3

[Jani Nikula]

Update on [1].

BR,
Jani.


[1] http://mid.mail-archive.com/cover.1495030890.git.jani.nikula@intel.com


Jani Nikula (4):
  drm/dp: add helper for reading DP sink/branch device desc from DPCD
  drm/i915: use drm DP helper to read DPCD desc
  drm/dp: start a DPCD based DP sink/branch device quirk database
  drm/i915: Detect USB-C specific dongles before reducing M and N

 drivers/gpu/drm/drm_dp_helper.c      | 83 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h      |  3 +-
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++----
 drivers/gpu/drm/i915/intel_dp.c      | 45 +++++--------------
 drivers/gpu/drm/i915/intel_dp_mst.c  |  5 ++-
 drivers/gpu/drm/i915/intel_drv.h     | 13 +-----
 drivers/gpu/drm/i915/intel_lspcon.c  |  2 +-
 include/drm/drm_dp_helper.h          | 51 ++++++++++++++++++++++
 8 files changed, 166 insertions(+), 58 deletions(-)

-- 
2.11.0

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

[PATCH 1/4] drm/dp: add helper for reading DP sink/branch device desc from DPCD

[Jani Nikula]

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 35 +++++++++++++++++++++++++++++++++++
 include/drm/drm_dp_helper.h     | 19 +++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 3e5f52110ea1..52e0ca9a5bb1 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1208,3 +1208,38 @@ int drm_dp_stop_crc(struct drm_dp_aux *aux)
 	return 0;
 }
 EXPORT_SYMBOL(drm_dp_stop_crc);
+
+/**
+ * drm_dp_read_desc - read sink/branch descriptor from DPCD
+ * @aux: DisplayPort AUX channel
+ * @desc: Device decriptor to fill from DPCD
+ * @is_branch: true for branch devices, false for sink devices
+ *
+ * Read DPCD 0x400 (sink) or 0x500 (branch) into @desc. Also debug log the
+ * identification.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
+		     bool is_branch)
+{
+	struct drm_dp_dpcd_ident *ident = &desc->ident;
+	unsigned int offset = is_branch ? DP_BRANCH_OUI : DP_SINK_OUI;
+	int ret, dev_id_len;
+
+	ret = drm_dp_dpcd_read(aux, offset, ident, sizeof(*ident));
+	if (ret < 0)
+		return ret;
+
+	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",
+		      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);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_read_desc);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index f7007e544f29..aee5b96b51d7 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1079,4 +1079,23 @@ void drm_dp_aux_unregister(struct drm_dp_aux *aux);
 int drm_dp_start_crc(struct drm_dp_aux *aux, struct drm_crtc *crtc);
 int drm_dp_stop_crc(struct drm_dp_aux *aux);
 
+struct drm_dp_dpcd_ident {
+	u8 oui[3];
+	u8 device_id[6];
+	u8 hw_rev;
+	u8 sw_major_rev;
+	u8 sw_minor_rev;
+} __packed;
+
+/**
+ * struct drm_dp_desc - DP branch/sink device descriptor
+ * @ident: DP device identification from DPCD 0x400 (sink) or 0x500 (branch).
+ */
+struct drm_dp_desc {
+	struct drm_dp_dpcd_ident ident;
+};
+
+int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc,
+		     bool is_branch);
+
 #endif /* _DRM_DP_HELPER_H_ */
-- 
2.11.0

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

[PATCH 2/4] drm/i915: use drm DP helper to read DPCD desc

[Jani Nikula]

Switch to using the common DP helpers instead of using our own.

v2: also remove leftover struct intel_dp_desc (Daniel)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c     | 37 ++++---------------------------------
 drivers/gpu/drm/i915/intel_drv.h    | 13 +------------
 drivers/gpu/drm/i915/intel_lspcon.c |  2 +-
 3 files changed, 6 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4a6feb6a69bd..2a5f385e8f44 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1548,37 +1548,6 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
 	DRM_DEBUG_KMS("common rates: %s\n", str);
 }
 
-bool
-__intel_dp_read_desc(struct intel_dp *intel_dp, struct intel_dp_desc *desc)
-{
-	u32 base = drm_dp_is_branch(intel_dp->dpcd) ? DP_BRANCH_OUI :
-						      DP_SINK_OUI;
-
-	return drm_dp_dpcd_read(&intel_dp->aux, base, desc, sizeof(*desc)) ==
-	       sizeof(*desc);
-}
-
-bool intel_dp_read_desc(struct intel_dp *intel_dp)
-{
-	struct intel_dp_desc *desc = &intel_dp->desc;
-	bool oui_sup = intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] &
-		       DP_OUI_SUPPORT;
-	int dev_id_len;
-
-	if (!__intel_dp_read_desc(intel_dp, desc))
-		return false;
-
-	dev_id_len = strnlen(desc->device_id, sizeof(desc->device_id));
-	DRM_DEBUG_KMS("DP %s: OUI %*phD%s dev-ID %*pE HW-rev %d.%d SW-rev %d.%d\n",
-		      drm_dp_is_branch(intel_dp->dpcd) ? "branch" : "sink",
-		      (int)sizeof(desc->oui), desc->oui, oui_sup ? "" : "(NS)",
-		      dev_id_len, desc->device_id,
-		      desc->hw_rev >> 4, desc->hw_rev & 0xf,
-		      desc->sw_major_rev, desc->sw_minor_rev);
-
-	return true;
-}
-
 int
 intel_dp_max_link_rate(struct intel_dp *intel_dp)
 {
@@ -3662,7 +3631,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
 	if (!intel_dp_read_dpcd(intel_dp))
 		return false;
 
-	intel_dp_read_desc(intel_dp);
+	drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc,
+			 drm_dp_is_branch(intel_dp->dpcd));
 
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
 		dev_priv->no_aux_handshake = intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
@@ -4677,7 +4647,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 
 	intel_dp_print_rates(intel_dp);
 
-	intel_dp_read_desc(intel_dp);
+	drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc,
+			 drm_dp_is_branch(intel_dp->dpcd));
 
 	intel_dp_configure_mst(intel_dp);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bd500977b3fc..71f94a01aedd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -938,14 +938,6 @@ enum link_m_n_set {
 	M2_N2
 };
 
-struct intel_dp_desc {
-	u8 oui[3];
-	u8 device_id[6];
-	u8 hw_rev;
-	u8 sw_major_rev;
-	u8 sw_minor_rev;
-} __packed;
-
 struct intel_dp_compliance_data {
 	unsigned long edid;
 	uint8_t video_pattern;
@@ -996,7 +988,7 @@ struct intel_dp {
 	/* Max rate for the current link */
 	int max_link_rate;
 	/* sink or branch descriptor */
-	struct intel_dp_desc desc;
+	struct drm_dp_desc desc;
 	struct drm_dp_aux aux;
 	enum intel_display_power_domain aux_power_domain;
 	uint8_t train_set[4];
@@ -1571,9 +1563,6 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 }
 
 bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
-bool __intel_dp_read_desc(struct intel_dp *intel_dp,
-			  struct intel_dp_desc *desc);
-bool intel_dp_read_desc(struct intel_dp *intel_dp);
 int intel_dp_link_required(int pixel_clock, int bpp);
 int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
 bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 71cbe9c08932..5abef482eacf 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -240,7 +240,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
 		return false;
 	}
 
-	intel_dp_read_desc(dp);
+	drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd));
 
 	DRM_DEBUG_KMS("Success: LSPCON init\n");
 	return true;
-- 
2.11.0

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

[PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[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ä <ville.syrjala@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Cc: Adam Jackson <ajax@redhat.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
 include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 52e0ca9a5bb1..213fb837e1c4 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -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;
 }
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index aee5b96b51d7..717cb8496725 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -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_ */
-- 
2.11.0

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

[PATCH 4/4] drm/i915: Detect USB-C specific dongles before reducing M and N

[Jani Nikula]

The Analogix 7737 DP to HDMI converter requires reduced M and N values
when to operate correctly at HBR2. We tried to reduce the M/N values for
all devices in commit 9a86cda07af2 ("drm/i915/dp: reduce link M/N
parameters"), but that regressed some other sinks. Detect this IC by its
OUI value of 0x0022B9 via the DPCD quirk list, and only reduce the M/N
values for that.

v2 by Jani: Rebased on the DP quirk database

v3 by Jani: Rebased on the reworked DP quirk database

v4 by Jani: Improve commit message (Daniel)

Fixes: 9a86cda07af2 ("drm/i915/dp: reduce link M/N parameters")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93578
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100755
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  3 ++-
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++--------
 drivers/gpu/drm/i915/intel_dp.c      |  8 ++++++--
 drivers/gpu/drm/i915/intel_dp_mst.c  |  5 ++++-
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 17883a84b8c1..31ca580e47ae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -611,7 +611,8 @@ struct intel_link_m_n {
 
 void intel_link_compute_m_n(int bpp, int nlanes,
 			    int pixel_clock, int link_clock,
-			    struct intel_link_m_n *m_n);
+			    struct intel_link_m_n *m_n,
+			    bool reduce_m_n);
 
 /* Interface history:
  *
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8217ed0e7132..ea4f116bd410 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6116,7 +6116,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 	pipe_config->fdi_lanes = lane;
 
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
-			       link_bw, &pipe_config->fdi_m_n);
+			       link_bw, &pipe_config->fdi_m_n, false);
 
 	ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config);
 	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
@@ -6292,7 +6292,8 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
 }
 
 static void compute_m_n(unsigned int m, unsigned int n,
-			uint32_t *ret_m, uint32_t *ret_n)
+			uint32_t *ret_m, uint32_t *ret_n,
+			bool reduce_m_n)
 {
 	/*
 	 * Reduce M/N as much as possible without loss in precision. Several DP
@@ -6300,9 +6301,11 @@ static void compute_m_n(unsigned int m, unsigned int n,
 	 * values. The passed in values are more likely to have the least
 	 * significant bits zero than M after rounding below, so do this first.
 	 */
-	while ((m & 1) == 0 && (n & 1) == 0) {
-		m >>= 1;
-		n >>= 1;
+	if (reduce_m_n) {
+		while ((m & 1) == 0 && (n & 1) == 0) {
+			m >>= 1;
+			n >>= 1;
+		}
 	}
 
 	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
@@ -6313,16 +6316,19 @@ static void compute_m_n(unsigned int m, unsigned int n,
 void
 intel_link_compute_m_n(int bits_per_pixel, int nlanes,
 		       int pixel_clock, int link_clock,
-		       struct intel_link_m_n *m_n)
+		       struct intel_link_m_n *m_n,
+		       bool reduce_m_n)
 {
 	m_n->tu = 64;
 
 	compute_m_n(bits_per_pixel * pixel_clock,
 		    link_clock * nlanes * 8,
-		    &m_n->gmch_m, &m_n->gmch_n);
+		    &m_n->gmch_m, &m_n->gmch_n,
+		    reduce_m_n);
 
 	compute_m_n(pixel_clock, link_clock,
-		    &m_n->link_m, &m_n->link_n);
+		    &m_n->link_m, &m_n->link_n,
+		    reduce_m_n);
 }
 
 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2a5f385e8f44..1ae9de5cf39c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1627,6 +1627,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int link_avail, link_clock;
 	int common_len;
 	uint8_t link_bw, rate_select;
+	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
+					   DP_DPCD_QUIRK_LIMITED_M_N);
 
 	common_len = intel_dp_common_len_rate_limit(intel_dp,
 						    intel_dp->max_link_rate);
@@ -1759,7 +1761,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	intel_link_compute_m_n(bpp, lane_count,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
-			       &pipe_config->dp_m_n);
+			       &pipe_config->dp_m_n,
+			       reduce_m_n);
 
 	if (intel_connector->panel.downclock_mode != NULL &&
 		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
@@ -1767,7 +1770,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			intel_link_compute_m_n(bpp, lane_count,
 				intel_connector->panel.downclock_mode->clock,
 				pipe_config->port_clock,
-				&pipe_config->dp_m2_n2);
+				&pipe_config->dp_m2_n2,
+				reduce_m_n);
 	}
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 68c788eb0b95..125917cebc60 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -44,6 +44,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	int lane_count, slots;
 	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	int mst_pbn;
+	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
+					   DP_DPCD_QUIRK_LIMITED_M_N);
 
 	pipe_config->has_pch_encoder = false;
 	bpp = 24;
@@ -79,7 +81,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	intel_link_compute_m_n(bpp, lane_count,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
-			       &pipe_config->dp_m_n);
+			       &pipe_config->dp_m_n,
+			       reduce_m_n);
 
 	pipe_config->dp_m_n.tu = slots;
 
-- 
2.11.0

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

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Clint Taylor]

On 05/18/2017 04:10 AM, Jani Nikula wrote:
> 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)

Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>

>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>   include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>   2 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 52e0ca9a5bb1..213fb837e1c4 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -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;
>   }
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index aee5b96b51d7..717cb8496725 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -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_ */

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

✓ Fi.CI.BAT: success for drm/dp: device identification and quirks ~v3

[Patchwork]

== Series Details ==

Series: drm/dp: device identification and quirks ~v3
URL   : https://patchwork.freedesktop.org/series/24619/
State : success

== Summary ==

Series 24619v1 drm/dp: device identification and quirks ~v3
https://patchwork.freedesktop.org/api/1.0/series/24619/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:445s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:439s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:588s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:513s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:495s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:487s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:418s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:416s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:425s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:497s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7500u     total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18  time:463s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:465s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:572s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:470s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:503s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:445s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:408s

ab08cb2750e769d074b2f147c8298ccd0cd08340 drm-tip: 2017y-05m-18d-15h-36m-17s UTC integration manifest
e82f32d drm/i915: Detect USB-C specific dongles before reducing M and N
8d6cff3 drm/dp: start a DPCD based DP sink/branch device quirk database
843956e drm/i915: use drm DP helper to read DPCD desc
1e88742 drm/dp: add helper for reading DP sink/branch device desc from DPCD

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4738/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Pandiyan, Dhinakaran]

On Thu, 2017-05-18 at 14:10 +0300, Jani Nikula wrote:
> 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ä <ville.syrjala@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>  include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>  2 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 52e0ca9a5bb1..213fb837e1c4 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c

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

s/Mdiv/Mvid
s/Ndiv/Nvid

> +	 * to 16 bits.
> +	 */
> +	DP_DPCD_QUIRK_LIMITED_M_N,
> +};
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Andrzej Hajda]

On 18.05.2017 13:10, Jani Nikula wrote:
> 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ä <ville.syrjala@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>  include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>  2 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 52e0ca9a5bb1..213fb837e1c4 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -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
> +

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

> +/*
> + * 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;
>  }
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index aee5b96b51d7..717cb8496725 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -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_ */


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

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Jani Nikula]

On Fri, 19 May 2017, Andrzej Hajda <a.hajda@samsung.com> wrote:
> On 18.05.2017 13:10, Jani Nikula wrote:
>> 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ä <ville.syrjala@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>> Cc: Adam Jackson <ajax@redhat.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>>  include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>>  2 files changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 52e0ca9a5bb1..213fb837e1c4 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -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
>> +
>
> 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) },
> };

Assuming we add more quirks like this later on, I prefer keeping my
approach.

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

True. I'm not sure if we have enough information to accurately limit
this to just the affected devices. The check can be narrowed later on as
needed.

BR,
Jani.


>
> Regards
> Andrzej
>
>> +/*
>> + * 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;
>>  }
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index aee5b96b51d7..717cb8496725 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -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_ */
>
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Jani Nikula]

On Thu, 18 May 2017, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> On 05/18/2017 04:10 AM, Jani Nikula wrote:
>> 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)
>
> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>

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.


>
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>> Cc: Adam Jackson <ajax@redhat.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>>   include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>>   2 files changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 52e0ca9a5bb1..213fb837e1c4 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -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;
>>   }
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index aee5b96b51d7..717cb8496725 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -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_ */
>

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

Re: [Intel-gfx] [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Jani Nikula]

On Thu, 18 May 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote:
> On Thu, 2017-05-18 at 14:10 +0300, Jani Nikula wrote:
>> 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ä <ville.syrjala@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>> Cc: Adam Jackson <ajax@redhat.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>>  include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>>  2 files changed, 82 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 52e0ca9a5bb1..213fb837e1c4 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>
> <snip>
>> + * 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
>
> s/Mdiv/Mvid
> s/Ndiv/Nvid

Thanks, I took the liberty of fixing this while applying.

BR,
Jani.

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

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

Re: [PATCH 3/4] drm/dp: start a DPCD based DP sink/branch device quirk database

[Clint Taylor]

On 05/29/2017 04:06 AM, Jani Nikula wrote:
> On Thu, 18 May 2017, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>> On 05/18/2017 04:10 AM, Jani Nikula wrote:
>>> 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)
>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> 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?
Topic branch appears to work as expected. Quirk is applied only to OUI 
0x0022B9 and the n and m values are reduced.

Topic branch is:
Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>

-Clint


> BR,
> Jani.
>
>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>> Cc: Clint Taylor <clinton.a.taylor@intel.com>
>>> Cc: Adam Jackson <ajax@redhat.com>
>>> Cc: Harry Wentland <harry.wentland@amd.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>    drivers/gpu/drm/drm_dp_helper.c | 52 +++++++++++++++++++++++++++++++++++++++--
>>>    include/drm/drm_dp_helper.h     | 32 +++++++++++++++++++++++++
>>>    2 files changed, 82 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>>> index 52e0ca9a5bb1..213fb837e1c4 100644
>>> --- a/drivers/gpu/drm/drm_dp_helper.c
>>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>>> @@ -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;
>>>    }
>>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>>> index aee5b96b51d7..717cb8496725 100644
>>> --- a/include/drm/drm_dp_helper.h
>>> +++ b/include/drm/drm_dp_helper.h
>>> @@ -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_ */

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