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

2017-05-30 Thread Clint Taylor 



On 05/29/2017 04:06 AM, Jani Nikula wrote:

On Thu, 18 May 2017, Clint Taylor  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 
Reviewed-by: Clinton Taylor 

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 

-Clint



BR,
Jani.



Cc: Ville Syrjälä 
Cc: Dhinakaran Pandiyan 
Cc: Clint Taylor 
Cc: Adam Jackson 
Cc: Harry Wentland 
Signed-off-by: Jani Nikula 
---
   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 = _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

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

2017-05-29 Thread Jani Nikula 
On Thu, 18 May 2017, Clint Taylor  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 
> Reviewed-by: Clinton Taylor 

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ä 
>> Cc: Dhinakaran Pandiyan 
>> Cc: Clint Taylor 
>> Cc: Adam Jackson 
>> Cc: Harry Wentland 
>> Signed-off-by: Jani Nikula 
>> ---
>>   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 = _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 {
>>

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

2017-05-19 Thread Jani Nikula 
On Fri, 19 May 2017, Andrzej Hajda  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ä 
>> Cc: Dhinakaran Pandiyan 
>> Cc: Clint Taylor 
>> Cc: Adam Jackson 
>> Cc: Harry Wentland 
>> Signed-off-by: Jani Nikula 
>> ---
>>  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 = _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

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

2017-05-18 Thread 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ä 
> Cc: Dhinakaran Pandiyan 
> Cc: Clint Taylor 
> Cc: Adam Jackson 
> Cc: Harry Wentland 
> Signed-off-by: Jani Nikula 
> ---
>  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 = _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
> + *
>

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

2017-05-18 Thread 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 
Reviewed-by: Clinton Taylor 



Cc: Ville Syrjälä 
Cc: Dhinakaran Pandiyan 
Cc: Clint Taylor 
Cc: Adam Jackson 
Cc: Harry Wentland 
Signed-off-by: Jani Nikula 
---
  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 = _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

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

2017-05-18 Thread Jani Nikula 
On Wed, 17 May 2017, Clint Taylor  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ä 
>> Cc: Dhinakaran Pandiyan 
>> Cc: Clint Taylor 
>> Cc: Adam Jackson 
>> Cc: Harry Wentland 
>> Signed-off-by: Jani Nikula 
>> ---
>>   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 = _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 0x
>
> 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 {
>>

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

2017-05-17 Thread 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ä 
Cc: Dhinakaran Pandiyan 
Cc: Clint Taylor 
Cc: Adam Jackson 
Cc: Harry Wentland 
Signed-off-by: Jani Nikula 
---
  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 = _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 0x


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