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

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2799 bytes --]

On Tue, 13 Dec 2022 18:20:59 +0100
Michel Dänzer <michel.daenzer@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.

So things like mode validation cannot just look at a single max or min
bpc, but it needs to figure out if there is any usable bpc value that
makes the mode work.

The max_bpc knob exists only for the cases where the sink undetectably
malfunctions unless the bpc is artificially limited more than seems
necessary. That malfunction requires a human to detect, and reconfigure
their system as we don't have a quirk database for this I think.

The question of userspace wanting a specific bpc is a different matter
and an unsolved one. It also ties to userspace wanting to use the
current mode to avoid a mode switch between e.g. hand-off from firmware
boot splash to proper userspace. That's also unsolved AFAIK.

OTOH, we have the discussion that concluded as
https://gitlab.freedesktop.org/wayland/weston/-/issues/612#note_1359898
which really puts userspace in charge of max_bpc, so the driver-chosen
default value does not have much impact as long as it makes the
firmware-chosen video mode to continue, as requested in
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/995
given that userspace cannot know what the actual bpc currently is nor
set the exact bpc to keep it the same.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]