On 12/15/22 10:07, Michel Dänzer wrote:
> On 12/14/22 16:46, Alex Deucher wrote:
>> On Wed, Dec 14, 2022 at 4:01 AM Pekka Paalanen <ppaala...@gmail.com> wrote:
>>> On Tue, 13 Dec 2022 18:20:59 +0100
>>> Michel Dänzer <michel.daen...@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.
>
> I wrote the above because I thought that this patch might result in some
> modes getting pruned because they can't work with the max bpc. However, I see
> now that cbd14ae7ea93 ("drm/amd/display: Fix incorrectly pruned modes with
> deep color") should prevent that AFAICT.
>
> The question then is: What happens if user space tries to use a mode which
> doesn't work with the max bpc? Does the driver automatically lower the
> effective bpc as needed, or does the atomic commit (check) fail? The latter
> would seem bad.
Per my previous post in the other sub-thread, cbd14ae7ea93 ("drm/amd/display:
Fix incorrectly pruned modes with deep color") seems to do the former. The
commit log of this patch should probably be changed to reflect that.
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
