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

[Alex Deucher]

On Thu, Dec 15, 2022 at 4:07 AM Michel Dänzer
<michel.daenzer at mailbox.org> wrote:
>
> On 12/14/22 16:46, Alex Deucher 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.
>
> 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.
>

Yeah, maybe that has been fixed now.  IIRC, the max bpc hack was added
a long time ago.

Alex

> 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.
>
>
> > I guess the driver can select a lower bpc at its discretion to produce
> > what it thinks is the best default, but what about users that don't want
> > the default?
>
> They can choose the lower refresh rate?
>
>
> --
> Earthling Michel Dänzer            |                  https://redhat.com
> Libre software enthusiast          |         Mesa and Xwayland developer
>