[Intel-gfx] [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 at 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 at intel.com>
> Reviewed-by: Clinton Taylor <clinton.a.taylor at 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 at linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>> Cc: Clint Taylor <clinton.a.taylor at intel.com>
>> Cc: Adam Jackson <ajax at redhat.com>
>> Cc: Harry Wentland <harry.wentland at amd.com>
>> Signed-off-by: Jani Nikula <jani.nikula at 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