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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2017-05-11 at 12:57 +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 OUI (with the option of
> expanding to device identification string and hardware/firmware
> revisions later). 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

Please see nit below.

> main link attributes for all devices ended up in regressions for other
> devices. So here we are.
> 
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> Cc: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
> Cc: Adam Jackson <ajax@xxxxxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 61 +++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_dp_helper.h     |  7 +++++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 3e5f52110ea1..58b2ced33151 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1208,3 +1208,64 @@ int drm_dp_stop_crc(struct drm_dp_aux *aux)
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_dp_stop_crc);
> +
> +/*
> + * 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.
> + */
> +
> +/*
> + * FIXME: Add support for device identification and hardware/firmware revision.
> + */
> +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, DP_DPCD_QUIRK_LIMITED_M_N },
> +};
> +
> +#undef OUI
> +
> +/**
> + * drm_dp_get_quirks() - get quirks for the sink/branch device
> + * @oui: pointer to len bytes of DPCD at 0x400 (sink) or 0x500 (branch)
> + * @len: 3-12 bytes of DPCD data
> + * @is_branch: true for branch devices, false for sink devices
> + *
> + * Get a bit mask of DPCD quirks for the sink/branch device. 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.
> + */
> +u32 drm_dp_get_quirks(const u8 *oui, unsigned int len, bool is_branch)
> +{
> +	const struct dpcd_quirk *quirk;
> +	u32 quirks = 0;
> +	int i;
> +
> +	if (WARN_ON_ONCE(len < 3))
> +		return 0;
> +
> +	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, oui, 3) != 0)
> +			continue;
> +
> +		quirks |= quirk->quirks;
> +	}
> +
> +	return quirks;
> +}
> +EXPORT_SYMBOL(drm_dp_get_quirks);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index f7007e544f29..8abe9897fe59 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1079,4 +1079,11 @@ 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);
>  
> +/* Display Port sink and branch device quirks. */
> +
> +/* The sink is limited to small link M and N values. */


I believe these are called mvid and nvid timestamps in the DP spec.
There's no reference link m and n outside i915 afaict. 

I'd prefer "small" to be quantified here or documentation that 24 bits
of link m/n doesn't work. It'll clarify how to use this quirk a little
bit.


With or without these
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>

-DK

> +#define DP_DPCD_QUIRK_LIMITED_M_N			BIT(0)
> +
> +u32 drm_dp_get_quirks(const u8 *oui, unsigned int len, bool is_branch);
> +
>  #endif /* _DRM_DP_HELPER_H_ */

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux