On Fri, 19 May 2017, Andrzej Hajda <a.ha...@samsung.com> 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ä <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..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 = &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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
