[Intel-gfx] [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 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..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_ */