[PATCH 16/16] drm/amd/display: Don't restrict bpc to 8 bpc

[Michel Dänzer]

On 12/15/22 18:42, Michel Dänzer wrote:
> On 12/15/22 18:33, Alex Deucher wrote:
>> On Thu, Dec 15, 2022 at 4:17 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>>
>>> On Wed, 14 Dec 2022 10:46:55 -0500
>>> Alex Deucher <alexdeucher at gmail.com> wrote:
>>>
>>>> On Wed, Dec 14, 2022 at 4:01 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>>>>
>>>>> On Tue, 13 Dec 2022 18:20:59 +0100
>>>>> Michel Dänzer <michel.daenzer at mailbox.org> wrote:
>>>>>
>>>>>> On 12/12/22 19:21, Harry Wentland wrote:
>>>>>>> This will let us pass kms_hdr.bpc_switch.
>>>>>>>
>>>>>>> I don't see any good reasons why we still need to
>>>>>>> limit bpc to 8 bpc and doing so is problematic when
>>>>>>> we enable HDR.
>>>>>>>
>>>>>>> If I remember correctly there might have been some
>>>>>>> displays out there where the advertised link bandwidth
>>>>>>> was not large enough to drive the default timing at
>>>>>>> max bpc. This would leave to an atomic commit/check
>>>>>>> failure which should really be handled in compositors
>>>>>>> with some sort of fallback mechanism.
>>>>>>>
>>>>>>> If this somehow turns out to still be an issue I
>>>>>>> suggest we add a module parameter to allow users to
>>>>>>> limit the max_bpc to a desired value.
>>>>>>
>>>>>> While leaving the fallback for user space to handle makes some sense
>>>>>> in theory, in practice most KMS display servers likely won't handle
>>>>>> it.
>>>>>>
>>>>>> Another issue is that if mode validation is based on the maximum bpc
>>>>>> value, it may reject modes which would work with lower bpc.
>>>>>>
>>>>>>
>>>>>> What Ville (CC'd) suggested before instead (and what i915 seems to be
>>>>>> doing already) is that the driver should do mode validation based on
>>>>>> the *minimum* bpc, and automatically make the effective bpc lower
>>>>>> than the maximum as needed to make the rest of the atomic state work.
>>>>>
>>>>> A driver is always allowed to choose a bpc lower than max_bpc, so it
>>>>> very well should do so when necessary due to *known* hardware etc.
>>>>> limitations.
>>>>>
>>>>
>>>> In the amdgpu case, it's more of a preference thing.  The driver would
>>>> enable higher bpcs at the expense of refresh rate and it seemed most
>>>> users want higher refresh rates than higher bpc.
>>>
>>> Hi Alex,
>>>
>>> we already have userspace in explicit control of the refresh rate.
>>> Userspace picks the refresh rate first, then the driver silently checks
>>> what bpc is possible. Userspace preference wins, so bpc is chosen to
>>> produce the desired refresh rate.
>>>
>>> In what cases does the driver really pick a refresh rate on its own?
>>
>> It didn't, but IIRC, at the time the driver filtered out modes that it
>> could not support with the max bpc.  It looks like that has been fixed
>> now, so maybe this whole discussion is moot.
> 
> That depends on the answer to the follow-up question in my previous post.

Never mind, looks like cbd14ae7ea93 ("drm/amd/display: Fix incorrectly pruned modes with deep color") does lower bpc as needed both for mode validation and atomic commit (check).


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer