On Wed, 17 May 2017, Clint Taylor <clinton.a.tay...@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.syrj...@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
>> Cc: Clint Taylor <clinton.a.tay...@intel.com>
>> Cc: Adam Jackson <a...@redhat.com>
>> Cc: Harry Wentland <harry.wentl...@amd.com>
>> Signed-off-by: Jani Nikula <jani.nik...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
