On Mon, 26 Aug 2019, "Francis, David" <David.Francis@xxxxxxx> wrote:
> On 2019-08-26 2:05 p.m., David Francis wrote:
>>> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not
>>> support virtual DPCD registers, but do support DSC.
>>> The DSC caps can be read from the physical aux,
>>> like in SST DSC. These hubs have many different
>>> DEVICE_IDs. Add a new quirk to detect this case.
>>>
>>> Cc: Lyude Paul <lyude@xxxxxxxxxx>
>>> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
>>> Signed-off-by: David Francis <David.Francis@xxxxxxx>
>>> ---
>>> drivers/gpu/drm/drm_dp_helper.c | 2 ++
>>> include/drm/drm_dp_helper.h | 7 +++++++
>>> 2 files changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>>> index 2cc21eff4cf3..fc39323e7d52 100644
>>> --- a/drivers/gpu/drm/drm_dp_helper.c
>>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>>> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
>>> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) },
>>> /* CH7511 seems to leave SINK_COUNT zeroed */
>>> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
>>> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
>>> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
>>
>>This seems to be generic OUI for Synaptics [1]. Could this cause us to
>>cast the net too wide?
>>
>>Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance
>>Synaptics is fixing this in the future and won't require the quirk.
>>
>>[1] https://aruljohn.com/mac/vendor/Synaptics
>>
>>
>>Harry
>
> It's not pretty, but
> - Currently the net of "all Synaptics devices with rev>DP1.4" catches every device we care about and none we don't
> - If a future Synaptics device supports virtual DPCD properly, we will check for that first and never resort to the workaround (see patch 6/6)
> - We don't know any of the properties of any hypothetical future Synaptics hardware, so we can't create cases for them now
One consideration is that if this causes certain behaviour in future
hardware, and, by coincidence, that happens to be desired behaviour, and
objectively fixing it (removing the quirk) causes a regression in that
desired behaviour, it's a regression.
I'm not sure what the right approach here is, but you really do need to
be aware of the kind of corners you might paint yourself into.
BR,
Jani.
>
> Also, received offline review from AMD MST DSC (Non-Linux) expert Wenjing Liu, giving me permission o mark this patch
> Reviewed-by: Wenjing Liu <Wenjing.Liu@xxxxxxx>
>
>>
>>> };
>>>
>>> #undef OUI
>>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>>> index 8364502f92cf..a1331b08705f 100644
>>> --- a/include/drm/drm_dp_helper.h
>>> +++ b/include/drm/drm_dp_helper.h
>>> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk {
>>> * The driver should ignore SINK_COUNT during detection.
>>> */
>>> DP_DPCD_QUIRK_NO_SINK_COUNT,
>>> + /**
>>> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD:
>>> + *
>>> + * The device supports MST DSC despite not supporting Virtual DPCD.
>>> + * The DSC caps can be read from the physical aux instead.
>>> + */
>>> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
>>> };
>>>
>>> /**
>
> ________________________________________
> From: Wentland, Harry <Harry.Wentland@xxxxxxx>
> Sent: August 26, 2019 3:05 PM
> To: Francis, David; dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v8 5/6] drm/dp_mst: Add new quirk for Synaptics MST hubs
>
> On 2019-08-26 2:05 p.m., David Francis wrote:
>> Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not
>> support virtual DPCD registers, but do support DSC.
>> The DSC caps can be read from the physical aux,
>> like in SST DSC. These hubs have many different
>> DEVICE_IDs. Add a new quirk to detect this case.
>>
>> Cc: Lyude Paul <lyude@xxxxxxxxxx>
>> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
>> Signed-off-by: David Francis <David.Francis@xxxxxxx>
>> ---
>> drivers/gpu/drm/drm_dp_helper.c | 2 ++
>> include/drm/drm_dp_helper.h | 7 +++++++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 2cc21eff4cf3..fc39323e7d52 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -1270,6 +1270,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
>> { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) },
>> /* CH7511 seems to leave SINK_COUNT zeroed */
>> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
>> + /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
>> + { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
>
> This seems to be generic OUI for Synaptics [1]. Could this cause us to
> cast the net too wide?
>
> Even if we check that it's DP_DPCD_REV >= 1.4 there's a good chance
> Synaptics is fixing this in the future and won't require the quirk.
>
> [1] https://aruljohn.com/mac/vendor/Synaptics
>
> Harry
>
>> };
>>
>> #undef OUI
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index 8364502f92cf..a1331b08705f 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -1434,6 +1434,13 @@ enum drm_dp_quirk {
>> * The driver should ignore SINK_COUNT during detection.
>> */
>> DP_DPCD_QUIRK_NO_SINK_COUNT,
>> + /**
>> + * @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD:
>> + *
>> + * The device supports MST DSC despite not supporting Virtual DPCD.
>> + * The DSC caps can be read from the physical aux instead.
>> + */
>> + DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
>> };
>>
>> /**
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
