Re: [PATCH v8 5/6] drm/dp_mst: Add new quirk for Synaptics MST hubs

[Harry Wentland <hwentlan@amd.com>]

On 2019-08-26 3:14 p.m., Francis, David 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@redhat.com>
>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> Signed-off-by: David Francis <David.Francis@amd.com>
>>> ---
>>>  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
> 

That's fair enough. Thanks for the explanation.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> 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@amd.com>
> 
>>
>>>  };
>>>
>>>  #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@amd.com>
> Sent: August 26, 2019 3:05 PM
> To: Francis, David; dri-devel@lists.freedesktop.org
> 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@redhat.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Signed-off-by: David Francis <David.Francis@amd.com>
>> ---
>>  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel