How should "max bpc" KMS property work?

How should "max bpc" KMS property work?

[Pekka Paalanen]

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

Hi all,

I'm working on setting HDR & WCG video modes in Weston, and I thought
setting "max bpc" KMS property on the connector would be a good idea.
I'm confused about how it works though.

I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612

Summary:

- Apparently the property was originally added as a manual workaround
  for sink hardware behaving badly with high depth. A simple end user
  setting for "max bpc" would suffice for this use.

- Drivers will sometimes automatically choose a lower bpc than the "max
  bpc" value, but never bigger.

- amdgpu seems to (did?) default "max bpc" to 8, meaning that I
  definitely want to raise it.

If I always slam "max bpc" to the highest supported value for that
property, do I lose more than workarounds for bad sink hardware?

Do I lose the ability to set video modes that take too much bandwidth
at uncapped driver-selected bpc while capping the bpc lower would allow
me to use those video modes?

Or, are drivers required to choose a lower-than-usual but highest
usable bpc to make the requested video mode squeeze through the
connector and link?

Do I need to implement a fallback strategy in a display server,
starting from the highest possible "max bpc" value, and if my modeset
is rejected, repeatedly try with lower "max bpc" setting until it works
or I'm out of bpc options?


Thanks,
pq

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

Re: How should "max bpc" KMS property work?

[Ville Syrjälä]

On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote:
> Hi all,
> 
> I'm working on setting HDR & WCG video modes in Weston, and I thought
> setting "max bpc" KMS property on the connector would be a good idea.
> I'm confused about how it works though.
> 
> I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
> 
> Summary:
> 
> - Apparently the property was originally added as a manual workaround
>   for sink hardware behaving badly with high depth. A simple end user
>   setting for "max bpc" would suffice for this use.
> 
> - Drivers will sometimes automatically choose a lower bpc than the "max
>   bpc" value, but never bigger.
> 
> - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
>   definitely want to raise it.

I've occasionally pondered about doing the same for i915, just to have
the safest default possible. But I'd hate to lose the deep color testing
coverage knowing very few people would in practice raise the limit.
Also the number of systems where deep color doesn't work reliably
(or can't be made to work by not using a crap cable) seems to be quite
low.

> 
> If I always slam "max bpc" to the highest supported value for that
> property, do I lose more than workarounds for bad sink hardware?

We don't have any workarounds implemented like this in the kernel.
Or should not have at least. "max bpc" exists purely for the user
to have a say in the matter in addition to whatever the EDID/quirks
say. Ie. if the kernel knows for sure that deep color won't work on
a particular setup then it should just not allow deep color at all
despite what the prop value says.

So the only danger is fighting with the user's wishes which I guess
you can overcome with some kind of user visible knob.

> 
> Do I lose the ability to set video modes that take too much bandwidth
> at uncapped driver-selected bpc while capping the bpc lower would allow
> me to use those video modes?
> 
> Or, are drivers required to choose a lower-than-usual but highest
> usable bpc to make the requested video mode squeeze through the
> connector and link?

IMO drivers should implement the "reduce bpc until it fits"
fallback. We have that in i915, except for MST where we'd need
to potentially involve multiple streams in the fallback. That
is something we intend to remedy eventually but it's not an
entirely trivial thing to implement so will take some actual
work. ATM we just cap MST to <=8bpc to avoid users getting into
this situation so often.

> 
> Do I need to implement a fallback strategy in a display server,
> starting from the highest possible "max bpc" value, and if my modeset
> is rejected, repeatedly try with lower "max bpc" setting until it works
> or I'm out of bpc options?

IMO the bpc part should be handled by the kernel since we already
had this behaviour even before the "max bpc" prop was introduced
and we didn't add an explicit "use this bpc or fail" prop. But of
course you should have some kind of sensible fallback strategy for
things that just fail for other reasons.

The one problem we have in the kernel is that we have no way to
ask the user if the display we tried to light up is actually
working. So our policy decisions can't really involve user input.
Userspace should not generally have that problem.

-- 
Ville Syrjälä
Intel

Re: How should "max bpc" KMS property work?

[Alex Deucher]

On Tue, Apr 26, 2022 at 4:35 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> Hi all,
>
> I'm working on setting HDR & WCG video modes in Weston, and I thought
> setting "max bpc" KMS property on the connector would be a good idea.
> I'm confused about how it works though.
>
> I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
>
> Summary:
>
> - Apparently the property was originally added as a manual workaround
>   for sink hardware behaving badly with high depth. A simple end user
>   setting for "max bpc" would suffice for this use.
>
> - Drivers will sometimes automatically choose a lower bpc than the "max
>   bpc" value, but never bigger.
>
> - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
>   definitely want to raise it.
>
> If I always slam "max bpc" to the highest supported value for that
> property, do I lose more than workarounds for bad sink hardware?
>
> Do I lose the ability to set video modes that take too much bandwidth
> at uncapped driver-selected bpc while capping the bpc lower would allow
> me to use those video modes?

You wouldn't lose workarounds for amdgpu, you'd just lose potential
modes.  The reason we added this feature in the first place was
because users bought new 4K monitors and the driver capped them at
30Hz because we always defaulted to the highest supported bpc.  We got
tons of bug reports about 4k@60 not being available and that was due
to the fact that the bpc was set to something greater than 8.  I'm not
sure what the right answer is.  It really depends on whether the user
wants higher bpc or faster refresh rates and possibly additional
higher res modes.

Alex

>
> Or, are drivers required to choose a lower-than-usual but highest
> usable bpc to make the requested video mode squeeze through the
> connector and link?
>
> Do I need to implement a fallback strategy in a display server,
> starting from the highest possible "max bpc" value, and if my modeset
> is rejected, repeatedly try with lower "max bpc" setting until it works
> or I'm out of bpc options?
>
>
> Thanks,
> pq

Re: How should "max bpc" KMS property work?

[Ville Syrjälä]

On Tue, Apr 26, 2022 at 03:38:25PM -0400, Alex Deucher wrote:
> On Tue, Apr 26, 2022 at 4:35 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > Hi all,
> >
> > I'm working on setting HDR & WCG video modes in Weston, and I thought
> > setting "max bpc" KMS property on the connector would be a good idea.
> > I'm confused about how it works though.
> >
> > I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
> >
> > Summary:
> >
> > - Apparently the property was originally added as a manual workaround
> >   for sink hardware behaving badly with high depth. A simple end user
> >   setting for "max bpc" would suffice for this use.
> >
> > - Drivers will sometimes automatically choose a lower bpc than the "max
> >   bpc" value, but never bigger.
> >
> > - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
> >   definitely want to raise it.
> >
> > If I always slam "max bpc" to the highest supported value for that
> > property, do I lose more than workarounds for bad sink hardware?
> >
> > Do I lose the ability to set video modes that take too much bandwidth
> > at uncapped driver-selected bpc while capping the bpc lower would allow
> > me to use those video modes?
> 
> You wouldn't lose workarounds for amdgpu, you'd just lose potential
> modes.  The reason we added this feature in the first place was
> because users bought new 4K monitors and the driver capped them at
> 30Hz because we always defaulted to the highest supported bpc.  We got
> tons of bug reports about 4k@60 not being available and that was due
> to the fact that the bpc was set to something greater than 8.  I'm not
> sure what the right answer is.  It really depends on whether the user
> wants higher bpc or faster refresh rates and possibly additional
> higher res modes.

IMO the right answer is to do mode filtering based on the min bpc.

-- 
Ville Syrjälä
Intel

Re: How should "max bpc" KMS property work?

[Pekka Paalanen]

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

Hi Ville and Alex,

thanks for the replies. More below.

TL;DR:

My take-away from this is that I should slam 'max bpc' to the max by
default, and offer a knob for the user in case they want to lower it.


On Tue, 26 Apr 2022 20:55:14 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote:
> > Hi all,
> > 
> > I'm working on setting HDR & WCG video modes in Weston, and I thought
> > setting "max bpc" KMS property on the connector would be a good idea.
> > I'm confused about how it works though.
> > 
> > I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
> > 
> > Summary:
> > 
> > - Apparently the property was originally added as a manual workaround
> >   for sink hardware behaving badly with high depth. A simple end user
> >   setting for "max bpc" would suffice for this use.
> > 
> > - Drivers will sometimes automatically choose a lower bpc than the "max
> >   bpc" value, but never bigger.
> > 
> > - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
> >   definitely want to raise it.  
> 
> I've occasionally pondered about doing the same for i915, just to have
> the safest default possible. But I'd hate to lose the deep color testing
> coverage knowing very few people would in practice raise the limit.
> Also the number of systems where deep color doesn't work reliably
> (or can't be made to work by not using a crap cable) seems to be quite
> low.

I think when HDR and WCG get into display servers, setting 'max bpc'
will become a standard action.

It's bit moot to e.g. render everything in electrical 10 bit RGB, if
the link is just going to squash that into electrical 8 bit RGB, right?

So even 10 bit color would require setting 'max bpc' to at least 10 to
be able to actually see it, source-side dithering aside.

> > 
> > If I always slam "max bpc" to the highest supported value for that
> > property, do I lose more than workarounds for bad sink hardware?  
> 
> We don't have any workarounds implemented like this in the kernel.
> Or should not have at least. "max bpc" exists purely for the user
> to have a say in the matter in addition to whatever the EDID/quirks
> say. Ie. if the kernel knows for sure that deep color won't work on
> a particular setup then it should just not allow deep color at all
> despite what the prop value says.
> 
> So the only danger is fighting with the user's wishes which I guess
> you can overcome with some kind of user visible knob.

Right, good.

Furthermore, as a KMS client cannot make much assumptions about the KMS
state it inherits from some other KMS client, it should know and
program all possible KMS properties according to its own desires
anyway. That, and the DRM master concept make sure that there cannot be
any "third party" KMS configuration programs, like V4L2 has.

> > Do I lose the ability to set video modes that take too much bandwidth
> > at uncapped driver-selected bpc while capping the bpc lower would allow
> > me to use those video modes?
> > 
> > Or, are drivers required to choose a lower-than-usual but highest
> > usable bpc to make the requested video mode squeeze through the
> > connector and link?  
> 
> IMO drivers should implement the "reduce bpc until it fits"
> fallback. We have that in i915, except for MST where we'd need
> to potentially involve multiple streams in the fallback. That
> is something we intend to remedy eventually but it's not an
> entirely trivial thing to implement so will take some actual
> work. ATM we just cap MST to <=8bpc to avoid users getting into
> this situation so often.

Excellent, but judging from what Alex said, this is also not what
amdgpu does. We have two drivers doing different things then?

So with Weston I probably have to document, that if you can't get the
video mode you want working, try turning the 'max bpc' knob down and
try again.

Or, I could cap 'max bpc' based on my framebuffer depth. If I have an
electrical 8 bit FB (default in Weston), then there is not much use for
having 'max bpc' > 8. This ignores the KMS color pipeline a bit. Does
that make sense?

Does KMS use dithering automatically, btw?

The only mention of dithering in KMS docs is some nouveau-specific KMS
properties and another for radeon.

> > 
> > Do I need to implement a fallback strategy in a display server,
> > starting from the highest possible "max bpc" value, and if my modeset
> > is rejected, repeatedly try with lower "max bpc" setting until it works
> > or I'm out of bpc options?  
> 
> IMO the bpc part should be handled by the kernel since we already
> had this behaviour even before the "max bpc" prop was introduced
> and we didn't add an explicit "use this bpc or fail" prop. But of
> course you should have some kind of sensible fallback strategy for
> things that just fail for other reasons.

Right, but this means that I don't have to add 'max bpc' as yet another
dimension in the combinatorial explosion of KMS parameters I would need
to search to find a working setup. That's really good.

The one thing missing is seeing what format and bpc we actually got on
the link.

> The one problem we have in the kernel is that we have no way to
> ask the user if the display we tried to light up is actually
> working. So our policy decisions can't really involve user input.
> Userspace should not generally have that problem.

Indeed.

Also like Alex said, the kernel does not know if the user prefers high
color depth or high refresh rate either. One way to solve that is to
light up the requested video mode any way the kernel can, and then
report what the resulting color depth is. Another way is to have
explicit "use this bpc or fail" KMS property, maybe in the form of 'min
bpc' as I recall being discussed in the past, and let userspace guess
what might work. The former is easier to light up, but probing requires
actually setting the modes. The latter may require a lot of
trial-and-error from userspace to find anything that works, but it
takes only time and not blinking - as far as things can be detected
with TEST_ONLY commits. Then one still has to ask the user if it
actually worked.


Thanks,
pq

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

Re: How should "max bpc" KMS property work?

[Michel Dänzer]

On 2022-04-27 12:52, Pekka Paalanen wrote:
> On Tue, 26 Apr 2022 20:55:14 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote:
>>>
>>> Do I lose the ability to set video modes that take too much bandwidth
>>> at uncapped driver-selected bpc while capping the bpc lower would allow
>>> me to use those video modes?
>>>
>>> Or, are drivers required to choose a lower-than-usual but highest
>>> usable bpc to make the requested video mode squeeze through the
>>> connector and link?  
>>
>> IMO drivers should implement the "reduce bpc until it fits"
>> fallback. We have that in i915, except for MST where we'd need
>> to potentially involve multiple streams in the fallback. That
>> is something we intend to remedy eventually but it's not an
>> entirely trivial thing to implement so will take some actual
>> work. ATM we just cap MST to <=8bpc to avoid users getting into
>> this situation so often.
> 
> Excellent, but judging from what Alex said, this is also not what
> amdgpu does. We have two drivers doing different things then?
> 
> So with Weston I probably have to document, that if you can't get the
> video mode you want working, try turning the 'max bpc' knob down and
> try again.
> 
> Or, I could cap 'max bpc' based on my framebuffer depth. If I have an
> electrical 8 bit FB (default in Weston), then there is not much use for
> having 'max bpc' > 8. This ignores the KMS color pipeline a bit. Does
> that make sense?

I don't think so. The output of LUTs in current HW has at least 10 bpc, regardless of the FB format.


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

Re: How should "max bpc" KMS property work?

[Harry Wentland]

On 2022-04-27 06:52, Pekka Paalanen wrote:
> Hi Ville and Alex,
> 
> thanks for the replies. More below.
> 
> TL;DR:
> 
> My take-away from this is that I should slam 'max bpc' to the max by
> default, and offer a knob for the user in case they want to lower it.
> 
> 
> On Tue, 26 Apr 2022 20:55:14 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
>> On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote:
>>> Hi all,
>>>
>>> I'm working on setting HDR & WCG video modes in Weston, and I thought
>>> setting "max bpc" KMS property on the connector would be a good idea.
>>> I'm confused about how it works though.
>>>
>>> I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
>>>
>>> Summary:
>>>
>>> - Apparently the property was originally added as a manual workaround
>>>   for sink hardware behaving badly with high depth. A simple end user
>>>   setting for "max bpc" would suffice for this use.
>>>
>>> - Drivers will sometimes automatically choose a lower bpc than the "max
>>>   bpc" value, but never bigger.
>>>
>>> - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
>>>   definitely want to raise it.  
>>

I've wanted to remove the 8 bpc limitations for a while now but it
looks like we never did for anything other than eDP.

The original problem we solved was that some monitors default timing
couldn't be driven at a high bpc. Therefore users were faced with black
displays. On some displays you also can't drive high refresh rate modes
with a higher bpc.

>> I've occasionally pondered about doing the same for i915, just to have
>> the safest default possible. But I'd hate to lose the deep color testing
>> coverage knowing very few people would in practice raise the limit.
>> Also the number of systems where deep color doesn't work reliably
>> (or can't be made to work by not using a crap cable) seems to be quite
>> low.
> 
> I think when HDR and WCG get into display servers, setting 'max bpc'
> will become a standard action.
> 
> It's bit moot to e.g. render everything in electrical 10 bit RGB, if
> the link is just going to squash that into electrical 8 bit RGB, right?
> 
> So even 10 bit color would require setting 'max bpc' to at least 10 to
> be able to actually see it, source-side dithering aside.
> 
>>>
>>> If I always slam "max bpc" to the highest supported value for that
>>> property, do I lose more than workarounds for bad sink hardware?  
>>
>> We don't have any workarounds implemented like this in the kernel.
>> Or should not have at least. "max bpc" exists purely for the user
>> to have a say in the matter in addition to whatever the EDID/quirks
>> say. Ie. if the kernel knows for sure that deep color won't work on
>> a particular setup then it should just not allow deep color at all
>> despite what the prop value says.
>>
>> So the only danger is fighting with the user's wishes which I guess
>> you can overcome with some kind of user visible knob.
> 
> Right, good.
> 
> Furthermore, as a KMS client cannot make much assumptions about the KMS
> state it inherits from some other KMS client, it should know and
> program all possible KMS properties according to its own desires
> anyway. That, and the DRM master concept make sure that there cannot be
> any "third party" KMS configuration programs, like V4L2 has.
> 
>>> Do I lose the ability to set video modes that take too much bandwidth
>>> at uncapped driver-selected bpc while capping the bpc lower would allow
>>> me to use those video modes?
>>>
>>> Or, are drivers required to choose a lower-than-usual but highest
>>> usable bpc to make the requested video mode squeeze through the
>>> connector and link?  
>>
>> IMO drivers should implement the "reduce bpc until it fits"
>> fallback. We have that in i915, except for MST where we'd need
>> to potentially involve multiple streams in the fallback. That
>> is something we intend to remedy eventually but it's not an
>> entirely trivial thing to implement so will take some actual
>> work. ATM we just cap MST to <=8bpc to avoid users getting into
>> this situation so often.
> 
> Excellent, but judging from what Alex said, this is also not what
> amdgpu does. We have two drivers doing different things then?
> > So with Weston I probably have to document, that if you can't get the
> video mode you want working, try turning the 'max bpc' knob down and
> try again.
> 
> Or, I could cap 'max bpc' based on my framebuffer depth. If I have an
> electrical 8 bit FB (default in Weston), then there is not much use for
> having 'max bpc' > 8. This ignores the KMS color pipeline a bit. Does
> that make sense?
> 

I think both of those options make sense. I'll need to think about the
automatic fallback if we don't have enough bandwidth for max_bpc.

If a KMS driver falls back automatically we probably want some way
for a (color managed) compositor to know if the output bpc is reduced.

> Does KMS use dithering automatically, btw?
> 

amdgpu's display driver does.

> The only mention of dithering in KMS docs is some nouveau-specific KMS
> properties and another for radeon.
> 
>>>
>>> Do I need to implement a fallback strategy in a display server,
>>> starting from the highest possible "max bpc" value, and if my modeset
>>> is rejected, repeatedly try with lower "max bpc" setting until it works
>>> or I'm out of bpc options?  
>>
>> IMO the bpc part should be handled by the kernel since we already
>> had this behaviour even before the "max bpc" prop was introduced
>> and we didn't add an explicit "use this bpc or fail" prop. But of
>> course you should have some kind of sensible fallback strategy for
>> things that just fail for other reasons.
> 
> Right, but this means that I don't have to add 'max bpc' as yet another
> dimension in the combinatorial explosion of KMS parameters I would need
> to search to find a working setup. That's really good.
> 
> The one thing missing is seeing what format and bpc we actually got on
> the link.
> 
>> The one problem we have in the kernel is that we have no way to
>> ask the user if the display we tried to light up is actually
>> working. So our policy decisions can't really involve user input.
>> Userspace should not generally have that problem.
> 
> Indeed.
> 
> Also like Alex said, the kernel does not know if the user prefers high
> color depth or high refresh rate either. One way to solve that is to
> light up the requested video mode any way the kernel can, and then
> report what the resulting color depth is. Another way is to have
> explicit "use this bpc or fail" KMS property, maybe in the form of 'min
> bpc' as I recall being discussed in the past, and let userspace guess
> what might work. The former is easier to light up, but probing requires
> actually setting the modes. The latter may require a lot of
> trial-and-error from userspace to find anything that works, but it
> takes only time and not blinking - as far as things can be detected
> with TEST_ONLY commits. Then one still has to ask the user if it
> actually worked.
> 

min_bpc sounds like something we might want for HDR use-cases, unless
the compositor has a way to confirm the output box (and format). min_bpc
would allow the KMS driver to pick the lowest required bpc so the
compositor always has a guarantee of quality.

Harry

> 
> Thanks,
> pq

Re: How should "max bpc" KMS property work?

[Sebastian Wick]

On Wed, Apr 27, 2022 at 5:41 PM Harry Wentland <harry.wentland@amd.com> wrote:
>
>
>
> On 2022-04-27 06:52, Pekka Paalanen wrote:
> > Hi Ville and Alex,
> >
> > thanks for the replies. More below.
> >
> > TL;DR:
> >
> > My take-away from this is that I should slam 'max bpc' to the max by
> > default, and offer a knob for the user in case they want to lower it.
> >
> >
> > On Tue, 26 Apr 2022 20:55:14 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >
> >> On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote:
> >>> Hi all,
> >>>
> >>> I'm working on setting HDR & WCG video modes in Weston, and I thought
> >>> setting "max bpc" KMS property on the connector would be a good idea.
> >>> I'm confused about how it works though.
> >>>
> >>> I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
> >>>
> >>> Summary:
> >>>
> >>> - Apparently the property was originally added as a manual workaround
> >>>   for sink hardware behaving badly with high depth. A simple end user
> >>>   setting for "max bpc" would suffice for this use.
> >>>
> >>> - Drivers will sometimes automatically choose a lower bpc than the "max
> >>>   bpc" value, but never bigger.
> >>>
> >>> - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
> >>>   definitely want to raise it.
> >>
>
> I've wanted to remove the 8 bpc limitations for a while now but it
> looks like we never did for anything other than eDP.
>
> The original problem we solved was that some monitors default timing
> couldn't be driven at a high bpc. Therefore users were faced with black
> displays. On some displays you also can't drive high refresh rate modes
> with a higher bpc.
>
> >> I've occasionally pondered about doing the same for i915, just to have
> >> the safest default possible. But I'd hate to lose the deep color testing
> >> coverage knowing very few people would in practice raise the limit.
> >> Also the number of systems where deep color doesn't work reliably
> >> (or can't be made to work by not using a crap cable) seems to be quite
> >> low.
> >
> > I think when HDR and WCG get into display servers, setting 'max bpc'
> > will become a standard action.
> >
> > It's bit moot to e.g. render everything in electrical 10 bit RGB, if
> > the link is just going to squash that into electrical 8 bit RGB, right?
> >
> > So even 10 bit color would require setting 'max bpc' to at least 10 to
> > be able to actually see it, source-side dithering aside.
> >
> >>>
> >>> If I always slam "max bpc" to the highest supported value for that
> >>> property, do I lose more than workarounds for bad sink hardware?
> >>
> >> We don't have any workarounds implemented like this in the kernel.
> >> Or should not have at least. "max bpc" exists purely for the user
> >> to have a say in the matter in addition to whatever the EDID/quirks
> >> say. Ie. if the kernel knows for sure that deep color won't work on
> >> a particular setup then it should just not allow deep color at all
> >> despite what the prop value says.
> >>
> >> So the only danger is fighting with the user's wishes which I guess
> >> you can overcome with some kind of user visible knob.
> >
> > Right, good.
> >
> > Furthermore, as a KMS client cannot make much assumptions about the KMS
> > state it inherits from some other KMS client, it should know and
> > program all possible KMS properties according to its own desires
> > anyway. That, and the DRM master concept make sure that there cannot be
> > any "third party" KMS configuration programs, like V4L2 has.
> >
> >>> Do I lose the ability to set video modes that take too much bandwidth
> >>> at uncapped driver-selected bpc while capping the bpc lower would allow
> >>> me to use those video modes?
> >>>
> >>> Or, are drivers required to choose a lower-than-usual but highest
> >>> usable bpc to make the requested video mode squeeze through the
> >>> connector and link?
> >>
> >> IMO drivers should implement the "reduce bpc until it fits"
> >> fallback. We have that in i915, except for MST where we'd need
> >> to potentially involve multiple streams in the fallback. That
> >> is something we intend to remedy eventually but it's not an
> >> entirely trivial thing to implement so will take some actual
> >> work. ATM we just cap MST to <=8bpc to avoid users getting into
> >> this situation so often.
> >
> > Excellent, but judging from what Alex said, this is also not what
> > amdgpu does. We have two drivers doing different things then?
> > > So with Weston I probably have to document, that if you can't get the
> > video mode you want working, try turning the 'max bpc' knob down and
> > try again.
> >
> > Or, I could cap 'max bpc' based on my framebuffer depth. If I have an
> > electrical 8 bit FB (default in Weston), then there is not much use for
> > having 'max bpc' > 8. This ignores the KMS color pipeline a bit. Does
> > that make sense?
> >
>
> I think both of those options make sense. I'll need to think about the
> automatic fallback if we don't have enough bandwidth for max_bpc.
>
> If a KMS driver falls back automatically we probably want some way
> for a (color managed) compositor to know if the output bpc is reduced.
>
> > Does KMS use dithering automatically, btw?
> >
>
> amdgpu's display driver does.
>
> > The only mention of dithering in KMS docs is some nouveau-specific KMS
> > properties and another for radeon.
> >
> >>>
> >>> Do I need to implement a fallback strategy in a display server,
> >>> starting from the highest possible "max bpc" value, and if my modeset
> >>> is rejected, repeatedly try with lower "max bpc" setting until it works
> >>> or I'm out of bpc options?
> >>
> >> IMO the bpc part should be handled by the kernel since we already
> >> had this behaviour even before the "max bpc" prop was introduced
> >> and we didn't add an explicit "use this bpc or fail" prop. But of
> >> course you should have some kind of sensible fallback strategy for
> >> things that just fail for other reasons.
> >
> > Right, but this means that I don't have to add 'max bpc' as yet another
> > dimension in the combinatorial explosion of KMS parameters I would need
> > to search to find a working setup. That's really good.
> >
> > The one thing missing is seeing what format and bpc we actually got on
> > the link.
> >
> >> The one problem we have in the kernel is that we have no way to
> >> ask the user if the display we tried to light up is actually
> >> working. So our policy decisions can't really involve user input.
> >> Userspace should not generally have that problem.
> >
> > Indeed.
> >
> > Also like Alex said, the kernel does not know if the user prefers high
> > color depth or high refresh rate either. One way to solve that is to
> > light up the requested video mode any way the kernel can, and then
> > report what the resulting color depth is. Another way is to have
> > explicit "use this bpc or fail" KMS property, maybe in the form of 'min
> > bpc' as I recall being discussed in the past, and let userspace guess
> > what might work. The former is easier to light up, but probing requires
> > actually setting the modes. The latter may require a lot of
> > trial-and-error from userspace to find anything that works, but it
> > takes only time and not blinking - as far as things can be detected
> > with TEST_ONLY commits. Then one still has to ask the user if it
> > actually worked.
> >
>
> min_bpc sounds like something we might want for HDR use-cases, unless
> the compositor has a way to confirm the output box (and format). min_bpc
> would allow the KMS driver to pick the lowest required bpc so the
> compositor always has a guarantee of quality.

IMO that would be ideal. The driver should try to reduce bandwidth by lowering
the bpc down to the min_bpc if the hardware can't drive the selected mode at a
higher bpc. User space usually knows which bpc is sufficient for the use case
but will never complain about too much bpc. Drivers which don't support
lowering the bpc dynamically can then still go with the min_bpc and user space
still gets all the modes which work with the minimum required bpc.

>
> Harry
>
> >
> > Thanks,
> > pq
>

Re: How should "max bpc" KMS property work?

[Pekka Paalanen]

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

On Wed, 27 Apr 2022 23:29:02 +0200
Sebastian Wick <sebastian.wick@redhat.com> wrote:

> On Wed, Apr 27, 2022 at 5:41 PM Harry Wentland <harry.wentland@amd.com> wrote:
> >
> >
> >
> > On 2022-04-27 06:52, Pekka Paalanen wrote:  
> > > Hi Ville and Alex,
> > >
> > > thanks for the replies. More below.
> > >
> > > TL;DR:
> > >
> > > My take-away from this is that I should slam 'max bpc' to the max by
> > > default, and offer a knob for the user in case they want to lower it.
> > >
> > >
> > > On Tue, 26 Apr 2022 20:55:14 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > >  
> > >> On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote:  
> > >>> Hi all,
> > >>>
> > >>> I'm working on setting HDR & WCG video modes in Weston, and I thought
> > >>> setting "max bpc" KMS property on the connector would be a good idea.
> > >>> I'm confused about how it works though.
> > >>>
> > >>> I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
> > >>>
> > >>> Summary:
> > >>>
> > >>> - Apparently the property was originally added as a manual workaround
> > >>>   for sink hardware behaving badly with high depth. A simple end user
> > >>>   setting for "max bpc" would suffice for this use.
> > >>>
> > >>> - Drivers will sometimes automatically choose a lower bpc than the "max
> > >>>   bpc" value, but never bigger.
> > >>>
> > >>> - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
> > >>>   definitely want to raise it.  
> > >>  
> >
> > I've wanted to remove the 8 bpc limitations for a while now but it
> > looks like we never did for anything other than eDP.
> >
> > The original problem we solved was that some monitors default timing
> > couldn't be driven at a high bpc. Therefore users were faced with black
> > displays. On some displays you also can't drive high refresh rate modes
> > with a higher bpc.

Aha, so there was nothing that the driver could have checked in order
to avoid those practical failures with high bpc. That makes sense.

I mean, link training succeeded fine and all looked good from the
source-side, but the sink just malfunctioned?

Rather than display servers not handling 'link-status' changes at all.

> > >> I've occasionally pondered about doing the same for i915, just to have
> > >> the safest default possible. But I'd hate to lose the deep color testing
> > >> coverage knowing very few people would in practice raise the limit.
> > >> Also the number of systems where deep color doesn't work reliably
> > >> (or can't be made to work by not using a crap cable) seems to be quite
> > >> low.  
> > >
> > > I think when HDR and WCG get into display servers, setting 'max bpc'
> > > will become a standard action.
> > >
> > > It's bit moot to e.g. render everything in electrical 10 bit RGB, if
> > > the link is just going to squash that into electrical 8 bit RGB, right?
> > >
> > > So even 10 bit color would require setting 'max bpc' to at least 10 to
> > > be able to actually see it, source-side dithering aside.
> > >  
> > >>>
> > >>> If I always slam "max bpc" to the highest supported value for that
> > >>> property, do I lose more than workarounds for bad sink hardware?  
> > >>
> > >> We don't have any workarounds implemented like this in the kernel.
> > >> Or should not have at least. "max bpc" exists purely for the user
> > >> to have a say in the matter in addition to whatever the EDID/quirks
> > >> say. Ie. if the kernel knows for sure that deep color won't work on
> > >> a particular setup then it should just not allow deep color at all
> > >> despite what the prop value says.
> > >>
> > >> So the only danger is fighting with the user's wishes which I guess
> > >> you can overcome with some kind of user visible knob.  
> > >
> > > Right, good.
> > >
> > > Furthermore, as a KMS client cannot make much assumptions about the KMS
> > > state it inherits from some other KMS client, it should know and
> > > program all possible KMS properties according to its own desires
> > > anyway. That, and the DRM master concept make sure that there cannot be
> > > any "third party" KMS configuration programs, like V4L2 has.
> > >  
> > >>> Do I lose the ability to set video modes that take too much bandwidth
> > >>> at uncapped driver-selected bpc while capping the bpc lower would allow
> > >>> me to use those video modes?
> > >>>
> > >>> Or, are drivers required to choose a lower-than-usual but highest
> > >>> usable bpc to make the requested video mode squeeze through the
> > >>> connector and link?  
> > >>
> > >> IMO drivers should implement the "reduce bpc until it fits"
> > >> fallback. We have that in i915, except for MST where we'd need
> > >> to potentially involve multiple streams in the fallback. That
> > >> is something we intend to remedy eventually but it's not an
> > >> entirely trivial thing to implement so will take some actual
> > >> work. ATM we just cap MST to <=8bpc to avoid users getting into
> > >> this situation so often.  
> > >
> > > Excellent, but judging from what Alex said, this is also not what
> > > amdgpu does. We have two drivers doing different things then?  
> > > > So with Weston I probably have to document, that if you can't get the  
> > > video mode you want working, try turning the 'max bpc' knob down and
> > > try again.
> > >
> > > Or, I could cap 'max bpc' based on my framebuffer depth. If I have an
> > > electrical 8 bit FB (default in Weston), then there is not much use for
> > > having 'max bpc' > 8. This ignores the KMS color pipeline a bit. Does
> > > that make sense?
> > >  
> >
> > I think both of those options make sense. I'll need to think about the
> > automatic fallback if we don't have enough bandwidth for max_bpc.
> >
> > If a KMS driver falls back automatically we probably want some way
> > for a (color managed) compositor to know if the output bpc is reduced.

The notion of 'max bpc' already implies that KMS drivers will
automatically fall back to lower bpc under some conditions, or at least
that the link bpc is chosen by the driver.


> >  
> > > Does KMS use dithering automatically, btw?
> > >  
> >
> > amdgpu's display driver does.
> >  
> > > The only mention of dithering in KMS docs is some nouveau-specific KMS
> > > properties and another for radeon.
> > >  
> > >>>
> > >>> Do I need to implement a fallback strategy in a display server,
> > >>> starting from the highest possible "max bpc" value, and if my modeset
> > >>> is rejected, repeatedly try with lower "max bpc" setting until it works
> > >>> or I'm out of bpc options?  
> > >>
> > >> IMO the bpc part should be handled by the kernel since we already
> > >> had this behaviour even before the "max bpc" prop was introduced
> > >> and we didn't add an explicit "use this bpc or fail" prop. But of
> > >> course you should have some kind of sensible fallback strategy for
> > >> things that just fail for other reasons.  
> > >
> > > Right, but this means that I don't have to add 'max bpc' as yet another
> > > dimension in the combinatorial explosion of KMS parameters I would need
> > > to search to find a working setup. That's really good.
> > >
> > > The one thing missing is seeing what format and bpc we actually got on
> > > the link.
> > >  
> > >> The one problem we have in the kernel is that we have no way to
> > >> ask the user if the display we tried to light up is actually
> > >> working. So our policy decisions can't really involve user input.
> > >> Userspace should not generally have that problem.  
> > >
> > > Indeed.
> > >
> > > Also like Alex said, the kernel does not know if the user prefers high
> > > color depth or high refresh rate either. One way to solve that is to
> > > light up the requested video mode any way the kernel can, and then
> > > report what the resulting color depth is. Another way is to have
> > > explicit "use this bpc or fail" KMS property, maybe in the form of 'min
> > > bpc' as I recall being discussed in the past, and let userspace guess
> > > what might work. The former is easier to light up, but probing requires
> > > actually setting the modes. The latter may require a lot of
> > > trial-and-error from userspace to find anything that works, but it
> > > takes only time and not blinking - as far as things can be detected
> > > with TEST_ONLY commits. Then one still has to ask the user if it
> > > actually worked.
> > >  
> >
> > min_bpc sounds like something we might want for HDR use-cases, unless
> > the compositor has a way to confirm the output box (and format). min_bpc
> > would allow the KMS driver to pick the lowest required bpc so the
> > compositor always has a guarantee of quality.  
> 
> IMO that would be ideal. The driver should try to reduce bandwidth by lowering
> the bpc down to the min_bpc if the hardware can't drive the selected mode at a
> higher bpc. User space usually knows which bpc is sufficient for the use case
> but will never complain about too much bpc. Drivers which don't support
> lowering the bpc dynamically can then still go with the min_bpc and user space
> still gets all the modes which work with the minimum required bpc.

This would be nice, yes.

I'm fairly sure 'min bpc' was discussed here on the dri-devel mailing
list in the past, but I don't remember when or by whom.


Thanks,
pq

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

Re: How should "max bpc" KMS property work?

[Simon Ser]

On Thursday, April 28th, 2022 at 09:50, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> > > > Also like Alex said, the kernel does not know if the user prefers high
> > > > color depth or high refresh rate either. One way to solve that is to
> > > > light up the requested video mode any way the kernel can, and then
> > > > report what the resulting color depth is. Another way is to have
> > > > explicit "use this bpc or fail" KMS property, maybe in the form of 'min
> > > > bpc' as I recall being discussed in the past, and let userspace guess
> > > > what might work. The former is easier to light up, but probing requires
> > > > actually setting the modes. The latter may require a lot of
> > > > trial-and-error from userspace to find anything that works, but it
> > > > takes only time and not blinking - as far as things can be detected
> > > > with TEST_ONLY commits. Then one still has to ask the user if it
> > > > actually worked.
> > >
> > > min_bpc sounds like something we might want for HDR use-cases, unless
> > > the compositor has a way to confirm the output box (and format). min_bpc
> > > would allow the KMS driver to pick the lowest required bpc so the
> > > compositor always has a guarantee of quality.
> >
> > IMO that would be ideal. The driver should try to reduce bandwidth by lowering
> > the bpc down to the min_bpc if the hardware can't drive the selected mode at a
> > higher bpc. User space usually knows which bpc is sufficient for the use case
> > but will never complain about too much bpc. Drivers which don't support
> > lowering the bpc dynamically can then still go with the min_bpc and user space
> > still gets all the modes which work with the minimum required bpc.
>
>
> This would be nice, yes.
>
> I'm fairly sure 'min bpc' was discussed here on the dri-devel mailing
> list in the past, but I don't remember when or by whom.

Yup. I explained there that I'd prefer "current bpc" + "user bpc" props
and let user-space deal with the fallback logic just like it does for
modes, modifiers, etc.

Re: How should "max bpc" KMS property work?

[Ville Syrjälä]

On Thu, Apr 28, 2022 at 07:52:58AM +0000, Simon Ser wrote:
> On Thursday, April 28th, 2022 at 09:50, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > > > > Also like Alex said, the kernel does not know if the user prefers high
> > > > > color depth or high refresh rate either. One way to solve that is to
> > > > > light up the requested video mode any way the kernel can, and then
> > > > > report what the resulting color depth is. Another way is to have
> > > > > explicit "use this bpc or fail" KMS property, maybe in the form of 'min
> > > > > bpc' as I recall being discussed in the past, and let userspace guess
> > > > > what might work. The former is easier to light up, but probing requires
> > > > > actually setting the modes. The latter may require a lot of
> > > > > trial-and-error from userspace to find anything that works, but it
> > > > > takes only time and not blinking - as far as things can be detected
> > > > > with TEST_ONLY commits. Then one still has to ask the user if it
> > > > > actually worked.
> > > >
> > > > min_bpc sounds like something we might want for HDR use-cases, unless
> > > > the compositor has a way to confirm the output box (and format). min_bpc
> > > > would allow the KMS driver to pick the lowest required bpc so the
> > > > compositor always has a guarantee of quality.
> > >
> > > IMO that would be ideal. The driver should try to reduce bandwidth by lowering
> > > the bpc down to the min_bpc if the hardware can't drive the selected mode at a
> > > higher bpc. User space usually knows which bpc is sufficient for the use case
> > > but will never complain about too much bpc. Drivers which don't support
> > > lowering the bpc dynamically can then still go with the min_bpc and user space
> > > still gets all the modes which work with the minimum required bpc.
> >
> >
> > This would be nice, yes.
> >
> > I'm fairly sure 'min bpc' was discussed here on the dri-devel mailing
> > list in the past, but I don't remember when or by whom.
> 
> Yup. I explained there that I'd prefer "current bpc" + "user bpc" props
> and let user-space deal with the fallback logic just like it does for
> modes, modifiers, etc.

The main problem is that the bpc is not really al that well defined.
We have stuff like 4:2:0 subsampling, DSC (compression), etc. muddying
the waters. Of course max_bpc already suffers a bit from those issues,
but at least it can still claim to do what it says on the tin.
Guaranteeing any kind of minimum bpc is not possible without first
defining what that actually means.

Oh and the various processing blocks in the pipeline might also have
varying input/output precision. So those can also degrade the quality
regardless of how many bits are coming out the end of the pipe.

I suspect trying to exose all that explicitly would result in an API
that just has too many knobs and interactions between the knobs. So
likely no one would be able to succesfully use it.

-- 
Ville Syrjälä
Intel

Re: How should "max bpc" KMS property work?

[Sebastian Wick]

On Thu, Apr 28, 2022 at 4:50 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Thu, Apr 28, 2022 at 07:52:58AM +0000, Simon Ser wrote:
> > On Thursday, April 28th, 2022 at 09:50, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > > > > > Also like Alex said, the kernel does not know if the user prefers high
> > > > > > color depth or high refresh rate either. One way to solve that is to
> > > > > > light up the requested video mode any way the kernel can, and then
> > > > > > report what the resulting color depth is. Another way is to have
> > > > > > explicit "use this bpc or fail" KMS property, maybe in the form of 'min
> > > > > > bpc' as I recall being discussed in the past, and let userspace guess
> > > > > > what might work. The former is easier to light up, but probing requires
> > > > > > actually setting the modes. The latter may require a lot of
> > > > > > trial-and-error from userspace to find anything that works, but it
> > > > > > takes only time and not blinking - as far as things can be detected
> > > > > > with TEST_ONLY commits. Then one still has to ask the user if it
> > > > > > actually worked.
> > > > >
> > > > > min_bpc sounds like something we might want for HDR use-cases, unless
> > > > > the compositor has a way to confirm the output box (and format). min_bpc
> > > > > would allow the KMS driver to pick the lowest required bpc so the
> > > > > compositor always has a guarantee of quality.
> > > >
> > > > IMO that would be ideal. The driver should try to reduce bandwidth by lowering
> > > > the bpc down to the min_bpc if the hardware can't drive the selected mode at a
> > > > higher bpc. User space usually knows which bpc is sufficient for the use case
> > > > but will never complain about too much bpc. Drivers which don't support
> > > > lowering the bpc dynamically can then still go with the min_bpc and user space
> > > > still gets all the modes which work with the minimum required bpc.
> > >
> > >
> > > This would be nice, yes.
> > >
> > > I'm fairly sure 'min bpc' was discussed here on the dri-devel mailing
> > > list in the past, but I don't remember when or by whom.
> >
> > Yup. I explained there that I'd prefer "current bpc" + "user bpc" props
> > and let user-space deal with the fallback logic just like it does for
> > modes, modifiers, etc.
>
> The main problem is that the bpc is not really al that well defined.
> We have stuff like 4:2:0 subsampling, DSC (compression), etc. muddying
> the waters. Of course max_bpc already suffers a bit from those issues,
> but at least it can still claim to do what it says on the tin.
> Guaranteeing any kind of minimum bpc is not possible without first
> defining what that actually means.
>
> Oh and the various processing blocks in the pipeline might also have
> varying input/output precision. So those can also degrade the quality
> regardless of how many bits are coming out the end of the pipe.
>
> I suspect trying to exose all that explicitly would result in an API
> that just has too many knobs and interactions between the knobs. So
> likely no one would be able to succesfully use it.

I said something similar on IRC earlier today and I agree.

We have 3 options here:

1. Don't give user space any kind of information
2. Give user space all the information so that it can figure out the
effective bpc
3. Let the kernel figure out the effective bpc

We *need* the information in user space so 1. is not going to work.
Exposing all hardware details in the current KMS API just won't work
well. So that leaves the kernel having to estimate the effective bpc.

I also agree that we have to define what the effective bpc is and IMO
it should contain not only connector properties but also all the color
processing blocks, pipe limitations and dithering in between the
framebuffer and the connector.

(or we can burn KMS down and create a libcamera kind of thing :>)

>
> --
> Ville Syrjälä
> Intel
>

Re: How should "max bpc" KMS property work?

[Hans de Goede]

Hi,

On 4/26/22 19:55, Ville Syrjälä wrote:
> On Tue, Apr 26, 2022 at 11:35:02AM +0300, Pekka Paalanen wrote:
>> Hi all,
>>
>> I'm working on setting HDR & WCG video modes in Weston, and I thought
>> setting "max bpc" KMS property on the connector would be a good idea.
>> I'm confused about how it works though.
>>
>> I did some digging in https://gitlab.freedesktop.org/wayland/weston/-/issues/612
>>
>> Summary:
>>
>> - Apparently the property was originally added as a manual workaround
>>   for sink hardware behaving badly with high depth. A simple end user
>>   setting for "max bpc" would suffice for this use.
>>
>> - Drivers will sometimes automatically choose a lower bpc than the "max
>>   bpc" value, but never bigger.
>>
>> - amdgpu seems to (did?) default "max bpc" to 8, meaning that I
>>   definitely want to raise it.
> 
> I've occasionally pondered about doing the same for i915, just to have
> the safest default possible. But I'd hate to lose the deep color testing
> coverage knowing very few people would in practice raise the limit.
> Also the number of systems where deep color doesn't work reliably
> (or can't be made to work by not using a crap cable) seems to be quite
> low.

I got pointed to this thread by Jonas Ådahl while asking some questions
the "max bpc" property related to:

https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102#note_1382328

The current i915 behavior which you describe here, which if I understand
things correctly is for "max bpc" to default to as high as possible is
causing problems with flickerfree boot in plymouth. Plymouth does a modeset
on the monitor's native resolution in case the BIOS/GOP setup the monitor
in a non native mode. Plymouth does not touch the "max bpc" property when
doing this modeset. Normally this works fine and when the BIOS/GOP has
already configured the monitor at the native resolution the i915 driver
will do a fastset and all is well.

Still the modeset is causing the screen to go black for multiple seconds,
despite the resolution being unchanged. What is happening according to
the on screen mode info from the monitor is that on plymouth's modeset
the link is being configured changes from 8 bpc to 10 bpc.

Is there anyway to avoid this without hardcoding "max bpc" to 8 in
plymouth (which would cause the same problem in the other direction
if the firmware sets up the link for 10bpc I believe) ?

Regards,

Hans

Re: How should "max bpc" KMS property work?

[Pekka Paalanen]

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

On Fri, 20 May 2022 17:20:50 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> I got pointed to this thread by Jonas Ådahl while asking some questions
> the "max bpc" property related to:
> 
> https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102#note_1382328
> 
> The current i915 behavior which you describe here, which if I understand
> things correctly is for "max bpc" to default to as high as possible is
> causing problems with flickerfree boot in plymouth. Plymouth does a modeset
> on the monitor's native resolution in case the BIOS/GOP setup the monitor
> in a non native mode. Plymouth does not touch the "max bpc" property when
> doing this modeset. Normally this works fine and when the BIOS/GOP has
> already configured the monitor at the native resolution the i915 driver
> will do a fastset and all is well.
> 
> Still the modeset is causing the screen to go black for multiple seconds,
> despite the resolution being unchanged. What is happening according to
> the on screen mode info from the monitor is that on plymouth's modeset
> the link is being configured changes from 8 bpc to 10 bpc.
> 
> Is there anyway to avoid this without hardcoding "max bpc" to 8 in
> plymouth (which would cause the same problem in the other direction
> if the firmware sets up the link for 10bpc I believe) ?

Hi Hans,

there was an attempt to get much of the current link state information
delivered to userspace, but I've forgot most about it.
I did find https://lkml.org/lkml/2021/6/18/294 linked from
https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_963469 .
I said the same in the Plymouth Gitlab issue you linked to.

Personally, I would need to know all current link details for
(professional) color management: am I still driving the monitor with
the same signal as I did when I measured the monitor one reboot ago?
If not, I cannot trust the color output and need to measure again.

Nice to see there would be other uses for knowing which might be higher
priority to the larger community.

Would it be proper to initialize 'max bpc' to the link depth used by
boot-up firmware? I guess it could make things more reliable and solve
the Plymouth blanking issue, but not the professional color management
use cases.


Thanks,
pq

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

Re: How should "max bpc" KMS property work?

[Sebastian Wick]

On Mon, May 23, 2022 at 10:23 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Fri, 20 May 2022 17:20:50 +0200
> Hans de Goede <hdegoede@redhat.com> wrote:
>
> > I got pointed to this thread by Jonas Ådahl while asking some questions
> > the "max bpc" property related to:
> >
> > https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102#note_1382328
> >
> > The current i915 behavior which you describe here, which if I understand
> > things correctly is for "max bpc" to default to as high as possible is
> > causing problems with flickerfree boot in plymouth. Plymouth does a modeset
> > on the monitor's native resolution in case the BIOS/GOP setup the monitor
> > in a non native mode. Plymouth does not touch the "max bpc" property when
> > doing this modeset. Normally this works fine and when the BIOS/GOP has
> > already configured the monitor at the native resolution the i915 driver
> > will do a fastset and all is well.
> >
> > Still the modeset is causing the screen to go black for multiple seconds,
> > despite the resolution being unchanged. What is happening according to
> > the on screen mode info from the monitor is that on plymouth's modeset
> > the link is being configured changes from 8 bpc to 10 bpc.
> >
> > Is there anyway to avoid this without hardcoding "max bpc" to 8 in
> > plymouth (which would cause the same problem in the other direction
> > if the firmware sets up the link for 10bpc I believe) ?
>
> Hi Hans,
>
> there was an attempt to get much of the current link state information
> delivered to userspace, but I've forgot most about it.
> I did find https://lkml.org/lkml/2021/6/18/294 linked from
> https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_963469 .
> I said the same in the Plymouth Gitlab issue you linked to.
>
> Personally, I would need to know all current link details for
> (professional) color management: am I still driving the monitor with
> the same signal as I did when I measured the monitor one reboot ago?
> If not, I cannot trust the color output and need to measure again.

I would go further and say that not being in control of all the link
details is an issue for user space.

Max bpc doesn't give us a minimum guarantee.
Bpc doesn't really make sense for YUV.
We don't know if the link is using RGB or YUV.
The Colorspace property requires user space to know if the link is RGB
or YUV (or does the link change depending on the Colorspace property?
What about the default Colorspace?).
Link compression can mess up colors.

There simply is no way to write a proper user space with the current KMS API.

>
> Nice to see there would be other uses for knowing which might be higher
> priority to the larger community.
>
> Would it be proper to initialize 'max bpc' to the link depth used by
> boot-up firmware? I guess it could make things more reliable and solve
> the Plymouth blanking issue, but not the professional color management
> use cases.

I was always under the impression that if you do an atomic commit
without changing any properties that it won't result in a mode set
which would clearly make the current behavior a bug.

>
>
> Thanks,
> pq

Re: How should "max bpc" KMS property work?

[Hans de Goede]

Hi,

On 5/23/22 13:54, Sebastian Wick wrote:
> On Mon, May 23, 2022 at 10:23 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>>
>> On Fri, 20 May 2022 17:20:50 +0200
>> Hans de Goede <hdegoede@redhat.com> wrote:
>>
>>> I got pointed to this thread by Jonas Ådahl while asking some questions
>>> the "max bpc" property related to:
>>>
>>> https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102#note_1382328
>>>
>>> The current i915 behavior which you describe here, which if I understand
>>> things correctly is for "max bpc" to default to as high as possible is
>>> causing problems with flickerfree boot in plymouth. Plymouth does a modeset
>>> on the monitor's native resolution in case the BIOS/GOP setup the monitor
>>> in a non native mode. Plymouth does not touch the "max bpc" property when
>>> doing this modeset. Normally this works fine and when the BIOS/GOP has
>>> already configured the monitor at the native resolution the i915 driver
>>> will do a fastset and all is well.
>>>
>>> Still the modeset is causing the screen to go black for multiple seconds,
>>> despite the resolution being unchanged. What is happening according to
>>> the on screen mode info from the monitor is that on plymouth's modeset
>>> the link is being configured changes from 8 bpc to 10 bpc.
>>>
>>> Is there anyway to avoid this without hardcoding "max bpc" to 8 in
>>> plymouth (which would cause the same problem in the other direction
>>> if the firmware sets up the link for 10bpc I believe) ?
>>
>> Hi Hans,
>>
>> there was an attempt to get much of the current link state information
>> delivered to userspace, but I've forgot most about it.
>> I did find https://lkml.org/lkml/2021/6/18/294 linked from
>> https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_963469 .
>> I said the same in the Plymouth Gitlab issue you linked to.
>>
>> Personally, I would need to know all current link details for
>> (professional) color management: am I still driving the monitor with
>> the same signal as I did when I measured the monitor one reboot ago?
>> If not, I cannot trust the color output and need to measure again.
> 
> I would go further and say that not being in control of all the link
> details is an issue for user space.
> 
> Max bpc doesn't give us a minimum guarantee.
> Bpc doesn't really make sense for YUV.
> We don't know if the link is using RGB or YUV.
> The Colorspace property requires user space to know if the link is RGB
> or YUV (or does the link change depending on the Colorspace property?
> What about the default Colorspace?).
> Link compression can mess up colors.
> 
> There simply is no way to write a proper user space with the current KMS API.
> 
>>
>> Nice to see there would be other uses for knowing which might be higher
>> priority to the larger community.
>>
>> Would it be proper to initialize 'max bpc' to the link depth used by
>> boot-up firmware? I guess it could make things more reliable and solve
>> the Plymouth blanking issue, but not the professional color management
>> use cases.
> 
> I was always under the impression that if you do an atomic commit
> without changing any properties that it won't result in a mode set
> which would clearly make the current behavior a bug.

I agree, IMHO the i915 driver currently setting max-bpc to 12 by default,
causing an unrequested link re-negotation on the first modeset is
a bug in the i195 driver and is also the root cause of this
plymouth bug-report:

https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102

Regards,

Hans

Re: How should "max bpc" KMS property work?

[Ville Syrjälä]

On Tue, May 24, 2022 at 11:36:22AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 5/23/22 13:54, Sebastian Wick wrote:
> > On Mon, May 23, 2022 at 10:23 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >>
> >> On Fri, 20 May 2022 17:20:50 +0200
> >> Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >>> I got pointed to this thread by Jonas Ådahl while asking some questions
> >>> the "max bpc" property related to:
> >>>
> >>> https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102#note_1382328
> >>>
> >>> The current i915 behavior which you describe here, which if I understand
> >>> things correctly is for "max bpc" to default to as high as possible is
> >>> causing problems with flickerfree boot in plymouth. Plymouth does a modeset
> >>> on the monitor's native resolution in case the BIOS/GOP setup the monitor
> >>> in a non native mode. Plymouth does not touch the "max bpc" property when
> >>> doing this modeset. Normally this works fine and when the BIOS/GOP has
> >>> already configured the monitor at the native resolution the i915 driver
> >>> will do a fastset and all is well.
> >>>
> >>> Still the modeset is causing the screen to go black for multiple seconds,
> >>> despite the resolution being unchanged. What is happening according to
> >>> the on screen mode info from the monitor is that on plymouth's modeset
> >>> the link is being configured changes from 8 bpc to 10 bpc.
> >>>
> >>> Is there anyway to avoid this without hardcoding "max bpc" to 8 in
> >>> plymouth (which would cause the same problem in the other direction
> >>> if the firmware sets up the link for 10bpc I believe) ?
> >>
> >> Hi Hans,
> >>
> >> there was an attempt to get much of the current link state information
> >> delivered to userspace, but I've forgot most about it.
> >> I did find https://lkml.org/lkml/2021/6/18/294 linked from
> >> https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_963469 .
> >> I said the same in the Plymouth Gitlab issue you linked to.
> >>
> >> Personally, I would need to know all current link details for
> >> (professional) color management: am I still driving the monitor with
> >> the same signal as I did when I measured the monitor one reboot ago?
> >> If not, I cannot trust the color output and need to measure again.
> > 
> > I would go further and say that not being in control of all the link
> > details is an issue for user space.
> > 
> > Max bpc doesn't give us a minimum guarantee.
> > Bpc doesn't really make sense for YUV.
> > We don't know if the link is using RGB or YUV.
> > The Colorspace property requires user space to know if the link is RGB
> > or YUV (or does the link change depending on the Colorspace property?
> > What about the default Colorspace?).
> > Link compression can mess up colors.
> > 
> > There simply is no way to write a proper user space with the current KMS API.
> > 
> >>
> >> Nice to see there would be other uses for knowing which might be higher
> >> priority to the larger community.
> >>
> >> Would it be proper to initialize 'max bpc' to the link depth used by
> >> boot-up firmware? I guess it could make things more reliable and solve
> >> the Plymouth blanking issue, but not the professional color management
> >> use cases.
> > 
> > I was always under the impression that if you do an atomic commit
> > without changing any properties that it won't result in a mode set
> > which would clearly make the current behavior a bug.
> 
> I agree, IMHO the i915 driver currently setting max-bpc to 12 by default,
> causing an unrequested link re-negotation on the first modeset is
> a bug in the i195 driver and is also the root cause of this
> plymouth bug-report:
> 
> https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102

Why would anyone want to run at 8bpc when they have a panel with
higher color depth? So I think someone is going to be doing that
modeset eventually anyway.

-- 
Ville Syrjälä
Intel

Re: How should "max bpc" KMS property work?

[Alex Deucher]

On Tue, May 24, 2022 at 11:43 AM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Tue, May 24, 2022 at 11:36:22AM +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 5/23/22 13:54, Sebastian Wick wrote:
> > > On Mon, May 23, 2022 at 10:23 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> > >>
> > >> On Fri, 20 May 2022 17:20:50 +0200
> > >> Hans de Goede <hdegoede@redhat.com> wrote:
> > >>
> > >>> I got pointed to this thread by Jonas Ådahl while asking some questions
> > >>> the "max bpc" property related to:
> > >>>
> > >>> https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102#note_1382328
> > >>>
> > >>> The current i915 behavior which you describe here, which if I understand
> > >>> things correctly is for "max bpc" to default to as high as possible is
> > >>> causing problems with flickerfree boot in plymouth. Plymouth does a modeset
> > >>> on the monitor's native resolution in case the BIOS/GOP setup the monitor
> > >>> in a non native mode. Plymouth does not touch the "max bpc" property when
> > >>> doing this modeset. Normally this works fine and when the BIOS/GOP has
> > >>> already configured the monitor at the native resolution the i915 driver
> > >>> will do a fastset and all is well.
> > >>>
> > >>> Still the modeset is causing the screen to go black for multiple seconds,
> > >>> despite the resolution being unchanged. What is happening according to
> > >>> the on screen mode info from the monitor is that on plymouth's modeset
> > >>> the link is being configured changes from 8 bpc to 10 bpc.
> > >>>
> > >>> Is there anyway to avoid this without hardcoding "max bpc" to 8 in
> > >>> plymouth (which would cause the same problem in the other direction
> > >>> if the firmware sets up the link for 10bpc I believe) ?
> > >>
> > >> Hi Hans,
> > >>
> > >> there was an attempt to get much of the current link state information
> > >> delivered to userspace, but I've forgot most about it.
> > >> I did find https://lkml.org/lkml/2021/6/18/294 linked from
> > >> https://gitlab.freedesktop.org/drm/amd/-/issues/476#note_963469 .
> > >> I said the same in the Plymouth Gitlab issue you linked to.
> > >>
> > >> Personally, I would need to know all current link details for
> > >> (professional) color management: am I still driving the monitor with
> > >> the same signal as I did when I measured the monitor one reboot ago?
> > >> If not, I cannot trust the color output and need to measure again.
> > >
> > > I would go further and say that not being in control of all the link
> > > details is an issue for user space.
> > >
> > > Max bpc doesn't give us a minimum guarantee.
> > > Bpc doesn't really make sense for YUV.
> > > We don't know if the link is using RGB or YUV.
> > > The Colorspace property requires user space to know if the link is RGB
> > > or YUV (or does the link change depending on the Colorspace property?
> > > What about the default Colorspace?).
> > > Link compression can mess up colors.
> > >
> > > There simply is no way to write a proper user space with the current KMS API.
> > >
> > >>
> > >> Nice to see there would be other uses for knowing which might be higher
> > >> priority to the larger community.
> > >>
> > >> Would it be proper to initialize 'max bpc' to the link depth used by
> > >> boot-up firmware? I guess it could make things more reliable and solve
> > >> the Plymouth blanking issue, but not the professional color management
> > >> use cases.
> > >
> > > I was always under the impression that if you do an atomic commit
> > > without changing any properties that it won't result in a mode set
> > > which would clearly make the current behavior a bug.
> >
> > I agree, IMHO the i915 driver currently setting max-bpc to 12 by default,
> > causing an unrequested link re-negotation on the first modeset is
> > a bug in the i195 driver and is also the root cause of this
> > plymouth bug-report:
> >
> > https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102
>
> Why would anyone want to run at 8bpc when they have a panel with
> higher color depth? So I think someone is going to be doing that
> modeset eventually anyway.

We used to do something similar, but then got piles of bug reports
about the displays running at 30Hz rather than 60 so we changed it to
8.  It's hard to say what a user will prefer.

Alex

Re: How should "max bpc" KMS property work?

[Simon Ser]

On Wednesday, May 25th, 2022 at 00:03, Alex Deucher <alexdeucher@gmail.com> wrote:

> > Why would anyone want to run at 8bpc when they have a panel with
> > higher color depth? So I think someone is going to be doing that
> > modeset eventually anyway.
>
> We used to do something similar, but then got piles of bug reports
> about the displays running at 30Hz rather than 60 so we changed it to
> 8. It's hard to say what a user will prefer.

"max bpc" is a maximum, ie. it shouldn't make a modeset fail, the kernel
should automatically reduce the bpc if the link bandwidth is insufficient.
So that sounds like there's an amdgpu bug here?

Re: How should "max bpc" KMS property work?

[Pekka Paalanen]

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

On Wed, 25 May 2022 06:04:44 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, May 25th, 2022 at 00:03, Alex Deucher <alexdeucher@gmail.com> wrote:
> 
> > > Why would anyone want to run at 8bpc when they have a panel with
> > > higher color depth? So I think someone is going to be doing that
> > > modeset eventually anyway.  
> >
> > We used to do something similar, but then got piles of bug reports
> > about the displays running at 30Hz rather than 60 so we changed it to
> > 8. It's hard to say what a user will prefer.  
> 
> "max bpc" is a maximum, ie. it shouldn't make a modeset fail, the kernel
> should automatically reduce the bpc if the link bandwidth is insufficient.
> So that sounds like there's an amdgpu bug here?

Yes, that's an amdgpu bug in my opinion too. A video mode is always
explicit about timings, while "max bpc" is just a max and not "use this
exactly", so I believe the explicit setting should win.

There is no "use this bpc exactly" KMS UAPI yet.


Thanks,
pq

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

Re: How should "max bpc" KMS property work?

[Pekka Paalanen]

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

On Mon, 23 May 2022 13:54:50 +0200
Sebastian Wick <sebastian.wick@redhat.com> wrote:

> I was always under the impression that if you do an atomic commit
> without changing any properties that it won't result in a mode set
> which would clearly make the current behavior a bug.

This is a very good point.

If one does an atomic commit (even with ALLOW_MODESET) but has changed
no KMS property at all, there should be no change in KMS hardware
state. That would be logical to me, but I also understand why KMS
doesn't work like that yet: the existing KMS properties are not enough.

Also KMS properties with "auto" value are probably favoring "the
best/highest possible" instead of "keep current state", which raises the
question of how such KMS properties should be initialized when the
firmware has chosen a different value from what "auto" in the driver
would. At the same time, this should not regress old userspace that
never sets a KMS property because the userspace was written before the
kernel exposed the property and the only thing happening was the driver
automatically choosing the value. Actually, the definition of "auto"
therefore is neither; it is "whatever the driver happened to be doing
before exposing this property".

Another question is how userspace can tell the kernel that it wants to
keep the current hardware state. That's the Plymouth problem.

Mind that "max bpc" is *always* in auto. It's only an upper limit, with
the assumption that the driver can choose less.


Thanks,
pq

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

Re: How should "max bpc" KMS property work?

[Michel Dänzer]

On 2022-05-25 09:28, Pekka Paalanen wrote:
> On Mon, 23 May 2022 13:54:50 +0200
> Sebastian Wick <sebastian.wick@redhat.com> wrote:
> 
>> I was always under the impression that if you do an atomic commit
>> without changing any properties that it won't result in a mode set
>> which would clearly make the current behavior a bug.
> 
> This is a very good point.
> 
> If one does an atomic commit (even with ALLOW_MODESET) but has changed
> no KMS property at all, there should be no change in KMS hardware
> state.

The problem then becomes how to change the effective bpc to the maximum value?


> Also KMS properties with "auto" value are probably favoring "the
> best/highest possible" instead of "keep current state", which raises the
> question of how such KMS properties should be initialized when the
> firmware has chosen a different value from what "auto" in the driver
> would. At the same time, this should not regress old userspace that
> never sets a KMS property because the userspace was written before the
> kernel exposed the property and the only thing happening was the driver
> automatically choosing the value. Actually, the definition of "auto"
> therefore is neither; it is "whatever the driver happened to be doing
> before exposing this property".
> 
> Another question is how userspace can tell the kernel that it wants to
> keep the current hardware state. That's the Plymouth problem.
> 
> Mind that "max bpc" is *always* in auto. It's only an upper limit, with
> the assumption that the driver can choose less.

It seems to me like the "max bpc" property is just kind of bad API, and trying to tweak it to cater to more use cases as we discover them will take us down a rabbit hole. It seems better to replace it with new properties which allow user space to determine the current effective bpc and to explicitly control it.


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

Re: How should "max bpc" KMS property work?

[Michel Dänzer]

On 2022-05-25 00:03, Alex Deucher wrote:
> On Tue, May 24, 2022 at 11:43 AM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
>> On Tue, May 24, 2022 at 11:36:22AM +0200, Hans de Goede wrote:
>>> Hi,
>>> On 5/23/22 13:54, Sebastian Wick wrote:
>>>> On Mon, May 23, 2022 at 10:23 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>>>>>
>>>>> Nice to see there would be other uses for knowing which might be higher
>>>>> priority to the larger community.
>>>>>
>>>>> Would it be proper to initialize 'max bpc' to the link depth used by
>>>>> boot-up firmware? I guess it could make things more reliable and solve
>>>>> the Plymouth blanking issue, but not the professional color management
>>>>> use cases.
>>>>
>>>> I was always under the impression that if you do an atomic commit
>>>> without changing any properties that it won't result in a mode set
>>>> which would clearly make the current behavior a bug.
>>>
>>> I agree, IMHO the i915 driver currently setting max-bpc to 12 by default,
>>> causing an unrequested link re-negotation on the first modeset is
>>> a bug in the i195 driver and is also the root cause of this
>>> plymouth bug-report:
>>>
>>> https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/102
>>
>> Why would anyone want to run at 8bpc when they have a panel with
>> higher color depth? So I think someone is going to be doing that
>> modeset eventually anyway.
> 
> We used to do something similar, but then got piles of bug reports
> about the displays running at 30Hz rather than 60 so we changed it to
> 8.  It's hard to say what a user will prefer.

Ville's suggestion elsewhere to filter modes based on minimum bpc (and lower effective bpc as needed for the selected mode, while there's only the "max bpc" property) should take care of that particular issue?


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

Re: How should "max bpc" KMS property work?

[Simon Ser]

On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:

> > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > the assumption that the driver can choose less.
>
> It seems to me like the "max bpc" property is just kind of bad API,
> and trying to tweak it to cater to more use cases as we discover them
> will take us down a rabbit hole. It seems better to replace it with
> new properties which allow user space to determine the current
> effective bpc and to explicitly control it.

+1, this sounds much more robust, and allows better control by user-space.
User-space needs to have fallback logic for other state as well anyways.
If the combinatorial explosion is too much, we should think about optimizing
the KMS implementation, or improve the uAPI.

Re: How should "max bpc" KMS property work?

[Pekka Paalanen]

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

On Wed, 25 May 2022 09:23:51 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> 
> > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > the assumption that the driver can choose less.  
> >
> > It seems to me like the "max bpc" property is just kind of bad API,
> > and trying to tweak it to cater to more use cases as we discover them
> > will take us down a rabbit hole. It seems better to replace it with
> > new properties which allow user space to determine the current
> > effective bpc and to explicitly control it.  
> 
> +1, this sounds much more robust, and allows better control by user-space.
> User-space needs to have fallback logic for other state as well anyways.
> If the combinatorial explosion is too much, we should think about optimizing
> the KMS implementation, or improve the uAPI.

+1 as well, with some recommendations added and the running off to the
sunset:

Use two separate KMS properties for reporting current vs.
programming desired. The KMS property reporting the current value
must be read-only (immutable). This preserves the difference between
what userspace wanted and what it got, making it possible to read
back both without confusing them. It also allows preserving driver behaviour

It allows the desired value to include "auto" meaning the driver should
pick best/highest value that works. That helps with the combinatorial
explosion as userspace can leave details for the driver to choose when
it doesn't care. Then userspace can read back from "current" property
to see what actually happened.

Plymouth could read the "current" property first and explicitly set
that to keep the current setting instead of hitting "auto" or making
assumptions about firmware set state.

There could also be another special value "driver default", different
from "auto". While "auto" gets the best/highest possible, "driver
default" would be the default value and mean whatever the driver did
before it exposed this property. This should avoid regressions in
behaviour.

All this won't fix the "empty commit should not change anything"
problem, because userspace needs to explicitly set the properties it
does *not* want to change. That's backwards, but fixing that would mean
changing what existing userspace experiences - which might be ok or
not, I don't know.

Thinking even further, about the problem of TEST_ONLY commits not
telling you what "auto" settings would actually give you; there could
be a new/extended KMS ioctl that would be the same as commit now, but
allow the kernel to return another set of KMS state back with
TEST_ONLY. Like reading back all KMS state after commit was done for
real. The "current" KMS properties would be part of that set, and tell
userspace what would happen in a real commit.


Thanks,
pq

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

Re: How should "max bpc" KMS property work?

[Jani Nikula]

On Wed, 25 May 2022, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> Use two separate KMS properties for reporting current vs.
> programming desired. The KMS property reporting the current value
> must be read-only (immutable). This preserves the difference between
> what userspace wanted and what it got, making it possible to read
> back both without confusing them. It also allows preserving driver behaviour

This seems like a common theme. Wondering if we should evolve helpers to
create a property pair like this in one go.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: How should "max bpc" KMS property work?

[Ville Syrjälä]

On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:
> On Wed, 25 May 2022 09:23:51 +0000
> Simon Ser <contact@emersion.fr> wrote:
> 
> > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > 
> > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > the assumption that the driver can choose less.  
> > >
> > > It seems to me like the "max bpc" property is just kind of bad API,
> > > and trying to tweak it to cater to more use cases as we discover them
> > > will take us down a rabbit hole. It seems better to replace it with
> > > new properties which allow user space to determine the current
> > > effective bpc and to explicitly control it.  
> > 
> > +1, this sounds much more robust, and allows better control by user-space.
> > User-space needs to have fallback logic for other state as well anyways.
> > If the combinatorial explosion is too much, we should think about optimizing
> > the KMS implementation, or improve the uAPI.
> 
> +1 as well, with some recommendations added and the running off to the
> sunset:
> 
> Use two separate KMS properties for reporting current vs.
> programming desired. The KMS property reporting the current value
> must be read-only (immutable). This preserves the difference between
> what userspace wanted and what it got, making it possible to read
> back both without confusing them. It also allows preserving driver behaviour

I don't see much real point in a property to report the current bpc.
That can't be used to do anything atomically. So I suppose plymouth
would be the only user.

So IMO if someone really need explicit control over this then we 
should just expose properties to set things explicitly. So we'd
need at least props for the actual bpc and some kind of color 
encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
would really need to figure out how DSC would interact with those.

For just the plymouth case I guess the easiest thing would be to
just clamp "max bpc" to the current value. The problem with that
is that we'd potentially break existing userspace. Eg. I don't think
the modesetting ddx or perhaps even most of the wayland compositors
set "max bpc" at all. So we'd need to roll out a bunch of userspace
updates first. And the "current bpc" prop idea would also have this
same problem since plymouth would just clamp "max bpc" based on the
current bpc before the real userspace starts.

-- 
Ville Syrjälä
Intel

How should "max bpc" and "Colorspace" KMS property work?

[Pekka Paalanen]

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

On Tue, 31 May 2022 20:37:31 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:
> > On Wed, 25 May 2022 09:23:51 +0000
> > Simon Ser <contact@emersion.fr> wrote:
> >   
> > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > >   
> > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > > the assumption that the driver can choose less.    
> > > >
> > > > It seems to me like the "max bpc" property is just kind of bad API,
> > > > and trying to tweak it to cater to more use cases as we discover them
> > > > will take us down a rabbit hole. It seems better to replace it with
> > > > new properties which allow user space to determine the current
> > > > effective bpc and to explicitly control it.    
> > > 
> > > +1, this sounds much more robust, and allows better control by user-space.
> > > User-space needs to have fallback logic for other state as well anyways.
> > > If the combinatorial explosion is too much, we should think about optimizing
> > > the KMS implementation, or improve the uAPI.  
> > 
> > +1 as well, with some recommendations added and the running off to the
> > sunset:
> > 
> > Use two separate KMS properties for reporting current vs.
> > programming desired. The KMS property reporting the current value
> > must be read-only (immutable). This preserves the difference between
> > what userspace wanted and what it got, making it possible to read
> > back both without confusing them. It also allows preserving driver behaviour  
> 
> I don't see much real point in a property to report the current bpc.
> That can't be used to do anything atomically. So I suppose plymouth
> would be the only user.

Hi Ville,

I think also professional color managed display servers would need it.

If they detect that the link bpc is no longer the same as it was when
the monitor was profiled, the profile will need to be re-verified by
measuring the monitor again.

See "Color calibration auditing system" notes in
https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description.

> So IMO if someone really need explicit control over this then we 
> should just expose properties to set things explicitly. So we'd
> need at least props for the actual bpc and some kind of color 
> encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
> would really need to figure out how DSC would interact with those.

I believe there still must be "auto" setting for bpc, and a separate
feedback property, so that userspace can use "auto" to see what it can
get without doing thousands of TEST_ONLY commits plus a few "link
status" failure handlings in order to find a working configuration (I'm
assuming we have many more properties than just "max bpc" to figure
out). After "auto" has told userspace what actually works without blind
trial and error, then userspace can program than value explicitly to
make sure it doesn't change accidentally in the future.

What's DSC?

Now that you mentioned some kind of color encoding property (I assume
you referred mostly to the sub-sampling aspect), how does the connector
property "Colorspace" factor in?

The enum values (which are not documented in KMS docs, btw.) are tuples
of color space + color model, e.g. on Intel:

"Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601,
XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB,
BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater}

Reading the KMS docs from
https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
they say:

> Basically the expectation from userspace is:
> 
>         Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> colorspace
> 
>         Set this new property to let the sink know what it converted
> the CRTC output to.
> 
>         This property is just to inform sink what colorspace source
> is trying to drive. 

However, where does userspace program the actual conversion from RGB to
NNN? Is it expected to be embedded in CTM?

Or does setting "Colorspace" imply some additional automatic
conversion? If so, where does it happen and how is it chosen?

> For just the plymouth case I guess the easiest thing would be to
> just clamp "max bpc" to the current value. The problem with that
> is that we'd potentially break existing userspace. Eg. I don't think
> the modesetting ddx or perhaps even most of the wayland compositors
> set "max bpc" at all. So we'd need to roll out a bunch of userspace
> updates first. And the "current bpc" prop idea would also have this
> same problem since plymouth would just clamp "max bpc" based on the
> current bpc before the real userspace starts.

True, but I believe once color management spreads through Wayland, so
will KMS clients also learn to set it.

Besides, any KMS client that does not set absolutely all KMS properties
will be at the mercy of any other KMS client they temporarily switch
with. So every KMS client should learn to program all possible KMS
properties anyway.


Thanks,
pq

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

Re: How should "max bpc" and "Colorspace" KMS property work?

[Simon Ser]

On Wednesday, June 1st, 2022 at 09:21, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> What's DSC?

Display Stream Compression. A "visually" lossless compression format
used between the GPU and the sink to reduce bandwidth usage.

https://vesa.org/vesa-display-compression-codecs/

Re: How should "max bpc" and "Colorspace" KMS property work?

[Ville Syrjälä]

On Wed, Jun 01, 2022 at 10:21:26AM +0300, Pekka Paalanen wrote:
> On Tue, 31 May 2022 20:37:31 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:
> > > On Wed, 25 May 2022 09:23:51 +0000
> > > Simon Ser <contact@emersion.fr> wrote:
> > >   
> > > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > > >   
> > > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > > > the assumption that the driver can choose less.    
> > > > >
> > > > > It seems to me like the "max bpc" property is just kind of bad API,
> > > > > and trying to tweak it to cater to more use cases as we discover them
> > > > > will take us down a rabbit hole. It seems better to replace it with
> > > > > new properties which allow user space to determine the current
> > > > > effective bpc and to explicitly control it.    
> > > > 
> > > > +1, this sounds much more robust, and allows better control by user-space.
> > > > User-space needs to have fallback logic for other state as well anyways.
> > > > If the combinatorial explosion is too much, we should think about optimizing
> > > > the KMS implementation, or improve the uAPI.  
> > > 
> > > +1 as well, with some recommendations added and the running off to the
> > > sunset:
> > > 
> > > Use two separate KMS properties for reporting current vs.
> > > programming desired. The KMS property reporting the current value
> > > must be read-only (immutable). This preserves the difference between
> > > what userspace wanted and what it got, making it possible to read
> > > back both without confusing them. It also allows preserving driver behaviour  
> > 
> > I don't see much real point in a property to report the current bpc.
> > That can't be used to do anything atomically. So I suppose plymouth
> > would be the only user.
> 
> Hi Ville,
> 
> I think also professional color managed display servers would need it.
> 
> If they detect that the link bpc is no longer the same as it was when
> the monitor was profiled, the profile will need to be re-verified by
> measuring the monitor again.
> 
> See "Color calibration auditing system" notes in
> https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description.
> 
> > So IMO if someone really need explicit control over this then we 
> > should just expose properties to set things explicitly. So we'd
> > need at least props for the actual bpc and some kind of color 
> > encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
> > would really need to figure out how DSC would interact with those.
> 
> I believe there still must be "auto" setting for bpc, and a separate
> feedback property, so that userspace can use "auto" to see what it can
> get without doing thousands of TEST_ONLY commits plus a few "link
> status" failure handlings in order to find a working configuration (I'm
> assuming we have many more properties than just "max bpc" to figure
> out). After "auto" has told userspace what actually works without blind
> trial and error, then userspace can program than value explicitly to
> make sure it doesn't change accidentally in the future.

Yeah we need "auto", but IMO mainly just to keep the current userspace
working. Using that to probe what's possible doesn't sound particularly
workable since you can't use it with TEST_ONLY commits. Also change to
some other property could still cause the whole thing to fail after the
max bpc has been probed so not sure it really buys you anything.

> 
> What's DSC?

Compression.

> 
> Now that you mentioned some kind of color encoding property (I assume
> you referred mostly to the sub-sampling aspect), how does the connector
> property "Colorspace" factor in?

The "Colorspace" property just changes what we report to the display
via infoframes/MSA/SDP. It does not cause the display hardware to do
any colorspace conversion/etc.

To actually force the display hardware to do the desired conversion
we need some new properties. ATM the only automagic conversion that
can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
which is needed on some displays to get 4k+ modes to work at all.

> 
> The enum values (which are not documented in KMS docs, btw.) are tuples
> of color space + color model, e.g. on Intel:
> 
> "Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601,
> XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB,
> BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater}

The accepted values are just what the CTA-861/DP specs
allow us to transmit in he infoframe/SDP/MSA.

> 
> Reading the KMS docs from
> https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
> they say:
> 
> > Basically the expectation from userspace is:
> > 
> >         Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > colorspace
> > 
> >         Set this new property to let the sink know what it converted
> > the CRTC output to.
> > 
> >         This property is just to inform sink what colorspace source
> > is trying to drive. 
> 
> However, where does userspace program the actual conversion from RGB to
> NNN? Is it expected to be embedded in CTM?
> 
> Or does setting "Colorspace" imply some additional automatic
> conversion? If so, where does it happen and how is it chosen?
> 
> > For just the plymouth case I guess the easiest thing would be to
> > just clamp "max bpc" to the current value. The problem with that
> > is that we'd potentially break existing userspace. Eg. I don't think
> > the modesetting ddx or perhaps even most of the wayland compositors
> > set "max bpc" at all. So we'd need to roll out a bunch of userspace
> > updates first. And the "current bpc" prop idea would also have this
> > same problem since plymouth would just clamp "max bpc" based on the
> > current bpc before the real userspace starts.
> 
> True, but I believe once color management spreads through Wayland, so
> will KMS clients also learn to set it.

Sure. But my point is that if we want to change how the "max bpc"
works I think we need to roll out the userspace stuff first so that
we at least can tell the user "please update you userspace to release x"
when they hit the regression.

-- 
Ville Syrjälä
Intel

Re: How should "max bpc" and "Colorspace" KMS property work?

[Sebastian Wick]

On Wed, Jun 1, 2022 at 4:06 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Wed, Jun 01, 2022 at 10:21:26AM +0300, Pekka Paalanen wrote:
> > On Tue, 31 May 2022 20:37:31 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >
> > > On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:
> > > > On Wed, 25 May 2022 09:23:51 +0000
> > > > Simon Ser <contact@emersion.fr> wrote:
> > > >
> > > > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > > > >
> > > > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > > > > the assumption that the driver can choose less.
> > > > > >
> > > > > > It seems to me like the "max bpc" property is just kind of bad API,
> > > > > > and trying to tweak it to cater to more use cases as we discover them
> > > > > > will take us down a rabbit hole. It seems better to replace it with
> > > > > > new properties which allow user space to determine the current
> > > > > > effective bpc and to explicitly control it.
> > > > >
> > > > > +1, this sounds much more robust, and allows better control by user-space.
> > > > > User-space needs to have fallback logic for other state as well anyways.
> > > > > If the combinatorial explosion is too much, we should think about optimizing
> > > > > the KMS implementation, or improve the uAPI.
> > > >
> > > > +1 as well, with some recommendations added and the running off to the
> > > > sunset:
> > > >
> > > > Use two separate KMS properties for reporting current vs.
> > > > programming desired. The KMS property reporting the current value
> > > > must be read-only (immutable). This preserves the difference between
> > > > what userspace wanted and what it got, making it possible to read
> > > > back both without confusing them. It also allows preserving driver behaviour
> > >
> > > I don't see much real point in a property to report the current bpc.
> > > That can't be used to do anything atomically. So I suppose plymouth
> > > would be the only user.
> >
> > Hi Ville,
> >
> > I think also professional color managed display servers would need it.
> >
> > If they detect that the link bpc is no longer the same as it was when
> > the monitor was profiled, the profile will need to be re-verified by
> > measuring the monitor again.
> >
> > See "Color calibration auditing system" notes in
> > https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description.
> >
> > > So IMO if someone really need explicit control over this then we
> > > should just expose properties to set things explicitly. So we'd
> > > need at least props for the actual bpc and some kind of color
> > > encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
> > > would really need to figure out how DSC would interact with those.
> >
> > I believe there still must be "auto" setting for bpc, and a separate
> > feedback property, so that userspace can use "auto" to see what it can
> > get without doing thousands of TEST_ONLY commits plus a few "link
> > status" failure handlings in order to find a working configuration (I'm
> > assuming we have many more properties than just "max bpc" to figure
> > out). After "auto" has told userspace what actually works without blind
> > trial and error, then userspace can program than value explicitly to
> > make sure it doesn't change accidentally in the future.
>
> Yeah we need "auto", but IMO mainly just to keep the current userspace
> working. Using that to probe what's possible doesn't sound particularly
> workable since you can't use it with TEST_ONLY commits. Also change to
> some other property could still cause the whole thing to fail after the
> max bpc has been probed so not sure it really buys you anything.

Why would we need "auto" to not break user space?

I agree though that a single property which accurately reflects the
"bpc" should be sufficient. If you want to set bpc to a certain value
you probably also want to do the same for RGB/YUV subsampling and DSC
which makes selecting a working mode without TEST_ONLY commits
impractical.

The combinatorial explosion is a real issue but to me it seems
possible to aggregate some properties into a group without
dependencies on other properties. For example we could have a "mode"
group which contains "MODE_ID", "bpc", pixel format/subsampling and
DSC properties in the future. If the atomic test commit then returns a
bitmask of the groups which failed you can considerably cut down on
the combinatorial problem.

>
> >
> > What's DSC?
>
> Compression.
>
> >
> > Now that you mentioned some kind of color encoding property (I assume
> > you referred mostly to the sub-sampling aspect), how does the connector
> > property "Colorspace" factor in?
>
> The "Colorspace" property just changes what we report to the display
> via infoframes/MSA/SDP. It does not cause the display hardware to do
> any colorspace conversion/etc.
>
> To actually force the display hardware to do the desired conversion
> we need some new properties. ATM the only automagic conversion that
> can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
> which is needed on some displays to get 4k+ modes to work at all.

Having control over this and compression would also help with defining
bpc. We could say bpc refers to the minimum bpc of all components for
RGB and the bpc of the luminance channel for YUV, all with compression
disabled. If user space turns on subsampling and compression it
understands the impacts on the effective bit depth.

Anyway, the Colorspace property is literally impossible to set
correctly because the kernel can just send either RGB or YUV without
any way for user space to figure out which it is but it's responsible
for sending the corresponding Colorspace. If I set the colorspace with
the wrong pixel format the colors are, as expected, completely broken
and there is nothing I can do but hope that it works by accident.

>
> >
> > The enum values (which are not documented in KMS docs, btw.) are tuples
> > of color space + color model, e.g. on Intel:
> >
> > "Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601,
> > XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB,
> > BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater}
>
> The accepted values are just what the CTA-861/DP specs
> allow us to transmit in he infoframe/SDP/MSA.
>
> >
> > Reading the KMS docs from
> > https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
> > they say:
> >
> > > Basically the expectation from userspace is:
> > >
> > >         Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > colorspace
> > >
> > >         Set this new property to let the sink know what it converted
> > > the CRTC output to.
> > >
> > >         This property is just to inform sink what colorspace source
> > > is trying to drive.
> >
> > However, where does userspace program the actual conversion from RGB to
> > NNN? Is it expected to be embedded in CTM?
> >
> > Or does setting "Colorspace" imply some additional automatic
> > conversion? If so, where does it happen and how is it chosen?
> >
> > > For just the plymouth case I guess the easiest thing would be to
> > > just clamp "max bpc" to the current value. The problem with that
> > > is that we'd potentially break existing userspace. Eg. I don't think
> > > the modesetting ddx or perhaps even most of the wayland compositors
> > > set "max bpc" at all. So we'd need to roll out a bunch of userspace
> > > updates first. And the "current bpc" prop idea would also have this
> > > same problem since plymouth would just clamp "max bpc" based on the
> > > current bpc before the real userspace starts.
> >
> > True, but I believe once color management spreads through Wayland, so
> > will KMS clients also learn to set it.
>
> Sure. But my point is that if we want to change how the "max bpc"
> works I think we need to roll out the userspace stuff first so that
> we at least can tell the user "please update you userspace to release x"
> when they hit the regression.

Shouldn't we just change "max bpc" to always be the maximum by
default? This doesn't break any user space but also doesn't solve the
plymouth issue. For that we would then use the "bpc" property which
always reflects the actual value. Plymouth would then set "bpc" to
max(current bpc, 8) and leave "max bpc" at maximum.

>
> --
> Ville Syrjälä
> Intel
>

Re: How should "max bpc" and "Colorspace" KMS property work?

[Pekka Paalanen]

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

On Wed, 1 Jun 2022 17:06:25 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Wed, Jun 01, 2022 at 10:21:26AM +0300, Pekka Paalanen wrote:
> > On Tue, 31 May 2022 20:37:31 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >   
> > > On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:  
> > > > On Wed, 25 May 2022 09:23:51 +0000
> > > > Simon Ser <contact@emersion.fr> wrote:
> > > >     
> > > > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > > > >     
> > > > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > > > > the assumption that the driver can choose less.      
> > > > > >
> > > > > > It seems to me like the "max bpc" property is just kind of bad API,
> > > > > > and trying to tweak it to cater to more use cases as we discover them
> > > > > > will take us down a rabbit hole. It seems better to replace it with
> > > > > > new properties which allow user space to determine the current
> > > > > > effective bpc and to explicitly control it.      
> > > > > 
> > > > > +1, this sounds much more robust, and allows better control by user-space.
> > > > > User-space needs to have fallback logic for other state as well anyways.
> > > > > If the combinatorial explosion is too much, we should think about optimizing
> > > > > the KMS implementation, or improve the uAPI.    
> > > > 
> > > > +1 as well, with some recommendations added and the running off to the
> > > > sunset:
> > > > 
> > > > Use two separate KMS properties for reporting current vs.
> > > > programming desired. The KMS property reporting the current value
> > > > must be read-only (immutable). This preserves the difference between
> > > > what userspace wanted and what it got, making it possible to read
> > > > back both without confusing them. It also allows preserving driver behaviour    
> > > 
> > > I don't see much real point in a property to report the current bpc.
> > > That can't be used to do anything atomically. So I suppose plymouth
> > > would be the only user.  
> > 
> > Hi Ville,
> > 
> > I think also professional color managed display servers would need it.
> > 
> > If they detect that the link bpc is no longer the same as it was when
> > the monitor was profiled, the profile will need to be re-verified by
> > measuring the monitor again.
> > 
> > See "Color calibration auditing system" notes in
> > https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description.
> >   
> > > So IMO if someone really need explicit control over this then we 
> > > should just expose properties to set things explicitly. So we'd
> > > need at least props for the actual bpc and some kind of color 
> > > encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
> > > would really need to figure out how DSC would interact with those.  
> > 
> > I believe there still must be "auto" setting for bpc, and a separate
> > feedback property, so that userspace can use "auto" to see what it can
> > get without doing thousands of TEST_ONLY commits plus a few "link
> > status" failure handlings in order to find a working configuration (I'm
> > assuming we have many more properties than just "max bpc" to figure
> > out). After "auto" has told userspace what actually works without blind
> > trial and error, then userspace can program than value explicitly to
> > make sure it doesn't change accidentally in the future.  
> 
> Yeah we need "auto", but IMO mainly just to keep the current userspace
> working. Using that to probe what's possible doesn't sound particularly
> workable since you can't use it with TEST_ONLY commits. Also change to
> some other property could still cause the whole thing to fail after the
> max bpc has been probed so not sure it really buys you anything.

Hi Ville,

earlier in this thread I drafted how the property-pair with "auto"
could be made useful also with TEST_ONLY commits:

> Thinking even further, about the problem of TEST_ONLY commits not
> telling you what "auto" settings would actually give you; there could
> be a new/extended KMS ioctl that would be the same as commit now, but
> allow the kernel to return another set of KMS state back with
> TEST_ONLY. Like reading back all KMS state after commit was done for
> real. The "current" KMS properties would be part of that set, and tell
> userspace what would happen in a real commit.

I do believe the combinatorial explosion of the KMS state search space
to find a working configuration is going to be a very real problem
sooner or later.


> > Now that you mentioned some kind of color encoding property (I assume
> > you referred mostly to the sub-sampling aspect), how does the connector
> > property "Colorspace" factor in?  
> 
> The "Colorspace" property just changes what we report to the display
> via infoframes/MSA/SDP. It does not cause the display hardware to do
> any colorspace conversion/etc.

Good.

> To actually force the display hardware to do the desired conversion
> we need some new properties. ATM the only automagic conversion that
> can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
> which is needed on some displays to get 4k+ modes to work at all.

When "Colorspace" says RGB, and the automatic fallback would kick in to
create a conflict, what happens?

> > 
> > The enum values (which are not documented in KMS docs, btw.) are tuples
> > of color space + color model, e.g. on Intel:
> > 
> > "Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601,
> > XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB,
> > BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater}  
> 
> The accepted values are just what the CTA-861/DP specs
> allow us to transmit in he infoframe/SDP/MSA.

Sure, but I mean the KMS doc a) does not refer to any standard, and b)
does not even list what the possible values could be.


> > 
> > Reading the KMS docs from
> > https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
> > they say:
> >   
> > > Basically the expectation from userspace is:
> > > 
> > >         Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > colorspace
> > > 
> > >         Set this new property to let the sink know what it converted
> > > the CRTC output to.
> > > 
> > >         This property is just to inform sink what colorspace source
> > > is trying to drive.   
> > 
> > However, where does userspace program the actual conversion from RGB to
> > NNN? Is it expected to be embedded in CTM?
> > 
> > Or does setting "Colorspace" imply some additional automatic
> > conversion? If so, where does it happen and how is it chosen?
> >   
> > > For just the plymouth case I guess the easiest thing would be to
> > > just clamp "max bpc" to the current value. The problem with that
> > > is that we'd potentially break existing userspace. Eg. I don't think
> > > the modesetting ddx or perhaps even most of the wayland compositors
> > > set "max bpc" at all. So we'd need to roll out a bunch of userspace
> > > updates first. And the "current bpc" prop idea would also have this
> > > same problem since plymouth would just clamp "max bpc" based on the
> > > current bpc before the real userspace starts.  
> > 
> > True, but I believe once color management spreads through Wayland, so
> > will KMS clients also learn to set it.  
> 
> Sure. But my point is that if we want to change how the "max bpc"
> works I think we need to roll out the userspace stuff first so that
> we at least can tell the user "please update you userspace to release x"
> when they hit the regression.

Sorry, I lost track on who is suggesting to change what.

I thought we agreed that "max bpc" means limiting link bpc to at most
that value, but the driver will automatically pick a lower value if
that makes the requested video mode work (and in lack of new KMS
properties).

I have no desire that change that. I also think the Plymouth issue is
not fully fixable without some new KMS property, and until then the
only thing Plymouth could do is to smash "max bpc" always to 8 - which
mostly stops being a problem once display servers learn to handle "max
bpc".

However, when new KMS properties are introduced, I very much like to
promote the two property setup for anything that could be a) set to
"auto", or b) be changed by the kernel *and* userspace.


Thanks,
pq

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

Re: How should "max bpc" and "Colorspace" KMS property work?

[Ville Syrjälä]

On Thu, Jun 02, 2022 at 10:47:59AM +0300, Pekka Paalanen wrote:
> On Wed, 1 Jun 2022 17:06:25 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Wed, Jun 01, 2022 at 10:21:26AM +0300, Pekka Paalanen wrote:
> > > On Tue, 31 May 2022 20:37:31 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > >   
> > > > On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:  
> > > > > On Wed, 25 May 2022 09:23:51 +0000
> > > > > Simon Ser <contact@emersion.fr> wrote:
> > > > >     
> > > > > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > > > > >     
> > > > > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > > > > > the assumption that the driver can choose less.      
> > > > > > >
> > > > > > > It seems to me like the "max bpc" property is just kind of bad API,
> > > > > > > and trying to tweak it to cater to more use cases as we discover them
> > > > > > > will take us down a rabbit hole. It seems better to replace it with
> > > > > > > new properties which allow user space to determine the current
> > > > > > > effective bpc and to explicitly control it.      
> > > > > > 
> > > > > > +1, this sounds much more robust, and allows better control by user-space.
> > > > > > User-space needs to have fallback logic for other state as well anyways.
> > > > > > If the combinatorial explosion is too much, we should think about optimizing
> > > > > > the KMS implementation, or improve the uAPI.    
> > > > > 
> > > > > +1 as well, with some recommendations added and the running off to the
> > > > > sunset:
> > > > > 
> > > > > Use two separate KMS properties for reporting current vs.
> > > > > programming desired. The KMS property reporting the current value
> > > > > must be read-only (immutable). This preserves the difference between
> > > > > what userspace wanted and what it got, making it possible to read
> > > > > back both without confusing them. It also allows preserving driver behaviour    
> > > > 
> > > > I don't see much real point in a property to report the current bpc.
> > > > That can't be used to do anything atomically. So I suppose plymouth
> > > > would be the only user.  
> > > 
> > > Hi Ville,
> > > 
> > > I think also professional color managed display servers would need it.
> > > 
> > > If they detect that the link bpc is no longer the same as it was when
> > > the monitor was profiled, the profile will need to be re-verified by
> > > measuring the monitor again.
> > > 
> > > See "Color calibration auditing system" notes in
> > > https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description.
> > >   
> > > > So IMO if someone really need explicit control over this then we 
> > > > should just expose properties to set things explicitly. So we'd
> > > > need at least props for the actual bpc and some kind of color 
> > > > encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
> > > > would really need to figure out how DSC would interact with those.  
> > > 
> > > I believe there still must be "auto" setting for bpc, and a separate
> > > feedback property, so that userspace can use "auto" to see what it can
> > > get without doing thousands of TEST_ONLY commits plus a few "link
> > > status" failure handlings in order to find a working configuration (I'm
> > > assuming we have many more properties than just "max bpc" to figure
> > > out). After "auto" has told userspace what actually works without blind
> > > trial and error, then userspace can program than value explicitly to
> > > make sure it doesn't change accidentally in the future.  
> > 
> > Yeah we need "auto", but IMO mainly just to keep the current userspace
> > working. Using that to probe what's possible doesn't sound particularly
> > workable since you can't use it with TEST_ONLY commits. Also change to
> > some other property could still cause the whole thing to fail after the
> > max bpc has been probed so not sure it really buys you anything.
> 
> Hi Ville,
> 
> earlier in this thread I drafted how the property-pair with "auto"
> could be made useful also with TEST_ONLY commits:
> 
> > Thinking even further, about the problem of TEST_ONLY commits not
> > telling you what "auto" settings would actually give you; there could
> > be a new/extended KMS ioctl that would be the same as commit now, but
> > allow the kernel to return another set of KMS state back with
> > TEST_ONLY. Like reading back all KMS state after commit was done for
> > real. The "current" KMS properties would be part of that set, and tell
> > userspace what would happen in a real commit.
> 
> I do believe the combinatorial explosion of the KMS state search space
> to find a working configuration is going to be a very real problem
> sooner or later.

That's seems like an orthogonal issue that would need some kind of
new uapi approach that let's you get some state back out from
TEST_ONLY commits.

> > > Now that you mentioned some kind of color encoding property (I assume
> > > you referred mostly to the sub-sampling aspect), how does the connector
> > > property "Colorspace" factor in?  
> > 
> > The "Colorspace" property just changes what we report to the display
> > via infoframes/MSA/SDP. It does not cause the display hardware to do
> > any colorspace conversion/etc.
> 
> Good.
> 
> > To actually force the display hardware to do the desired conversion
> > we need some new properties. ATM the only automagic conversion that
> > can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
> > which is needed on some displays to get 4k+ modes to work at all.
> 
> When "Colorspace" says RGB, and the automatic fallback would kick in to
> create a conflict, what happens?

I would put that in the "Doctor, it hurts when I..." category.

> 
> > > 
> > > The enum values (which are not documented in KMS docs, btw.) are tuples
> > > of color space + color model, e.g. on Intel:
> > > 
> > > "Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601,
> > > XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB,
> > > BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater}  
> > 
> > The accepted values are just what the CTA-861/DP specs
> > allow us to transmit in he infoframe/SDP/MSA.
> 
> Sure, but I mean the KMS doc a) does not refer to any standard, and b)
> does not even list what the possible values could be.

Seems like something that can be remedied with a patch.

> 
> 
> > > 
> > > Reading the KMS docs from
> > > https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
> > > they say:
> > >   
> > > > Basically the expectation from userspace is:
> > > > 
> > > >         Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > > colorspace
> > > > 
> > > >         Set this new property to let the sink know what it converted
> > > > the CRTC output to.
> > > > 
> > > >         This property is just to inform sink what colorspace source
> > > > is trying to drive.   
> > > 
> > > However, where does userspace program the actual conversion from RGB to
> > > NNN? Is it expected to be embedded in CTM?
> > > 
> > > Or does setting "Colorspace" imply some additional automatic
> > > conversion? If so, where does it happen and how is it chosen?
> > >   
> > > > For just the plymouth case I guess the easiest thing would be to
> > > > just clamp "max bpc" to the current value. The problem with that
> > > > is that we'd potentially break existing userspace. Eg. I don't think
> > > > the modesetting ddx or perhaps even most of the wayland compositors
> > > > set "max bpc" at all. So we'd need to roll out a bunch of userspace
> > > > updates first. And the "current bpc" prop idea would also have this
> > > > same problem since plymouth would just clamp "max bpc" based on the
> > > > current bpc before the real userspace starts.  
> > > 
> > > True, but I believe once color management spreads through Wayland, so
> > > will KMS clients also learn to set it.  
> > 
> > Sure. But my point is that if we want to change how the "max bpc"
> > works I think we need to roll out the userspace stuff first so that
> > we at least can tell the user "please update you userspace to release x"
> > when they hit the regression.
> 
> Sorry, I lost track on who is suggesting to change what.
> 
> I thought we agreed that "max bpc" means limiting link bpc to at most
> that value, but the driver will automatically pick a lower value if
> that makes the requested video mode work (and in lack of new KMS
> properties).
> 
> I have no desire that change that. I also think the Plymouth issue is
> not fully fixable without some new KMS property, and until then the
> only thing Plymouth could do is to smash "max bpc" always to 8 - which
> mostly stops being a problem once display servers learn to handle "max
> bpc".

There's no real differene between the kernel automagically clamping
"max bpc" to the current BIOS programmed value vs. plymouth doing it.
Either approach will break deep color support for current userspace
which doesn't reset "max bpc" back to the max.

> 
> However, when new KMS properties are introduced, I very much like to
> promote the two property setup for anything that could be a) set to
> "auto", or b) be changed by the kernel *and* userspace.
> 
> 
> Thanks,
> pq



-- 
Ville Syrjälä
Intel

Re: How should "max bpc" and "Colorspace" KMS property work?

[Sebastian Wick]

On Thu, Jun 2, 2022 at 6:40 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Thu, Jun 02, 2022 at 10:47:59AM +0300, Pekka Paalanen wrote:
> > On Wed, 1 Jun 2022 17:06:25 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >
> > > On Wed, Jun 01, 2022 at 10:21:26AM +0300, Pekka Paalanen wrote:
> > > > On Tue, 31 May 2022 20:37:31 +0300
> > > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > >
> > > > > On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:
> > > > > > On Wed, 25 May 2022 09:23:51 +0000
> > > > > > Simon Ser <contact@emersion.fr> wrote:
> > > > > >
> > > > > > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > > > > > >
> > > > > > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > > > > > > the assumption that the driver can choose less.
> > > > > > > >
> > > > > > > > It seems to me like the "max bpc" property is just kind of bad API,
> > > > > > > > and trying to tweak it to cater to more use cases as we discover them
> > > > > > > > will take us down a rabbit hole. It seems better to replace it with
> > > > > > > > new properties which allow user space to determine the current
> > > > > > > > effective bpc and to explicitly control it.
> > > > > > >
> > > > > > > +1, this sounds much more robust, and allows better control by user-space.
> > > > > > > User-space needs to have fallback logic for other state as well anyways.
> > > > > > > If the combinatorial explosion is too much, we should think about optimizing
> > > > > > > the KMS implementation, or improve the uAPI.
> > > > > >
> > > > > > +1 as well, with some recommendations added and the running off to the
> > > > > > sunset:
> > > > > >
> > > > > > Use two separate KMS properties for reporting current vs.
> > > > > > programming desired. The KMS property reporting the current value
> > > > > > must be read-only (immutable). This preserves the difference between
> > > > > > what userspace wanted and what it got, making it possible to read
> > > > > > back both without confusing them. It also allows preserving driver behaviour
> > > > >
> > > > > I don't see much real point in a property to report the current bpc.
> > > > > That can't be used to do anything atomically. So I suppose plymouth
> > > > > would be the only user.
> > > >
> > > > Hi Ville,
> > > >
> > > > I think also professional color managed display servers would need it.
> > > >
> > > > If they detect that the link bpc is no longer the same as it was when
> > > > the monitor was profiled, the profile will need to be re-verified by
> > > > measuring the monitor again.
> > > >
> > > > See "Color calibration auditing system" notes in
> > > > https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description.
> > > >
> > > > > So IMO if someone really need explicit control over this then we
> > > > > should just expose properties to set things explicitly. So we'd
> > > > > need at least props for the actual bpc and some kind of color
> > > > > encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
> > > > > would really need to figure out how DSC would interact with those.
> > > >
> > > > I believe there still must be "auto" setting for bpc, and a separate
> > > > feedback property, so that userspace can use "auto" to see what it can
> > > > get without doing thousands of TEST_ONLY commits plus a few "link
> > > > status" failure handlings in order to find a working configuration (I'm
> > > > assuming we have many more properties than just "max bpc" to figure
> > > > out). After "auto" has told userspace what actually works without blind
> > > > trial and error, then userspace can program than value explicitly to
> > > > make sure it doesn't change accidentally in the future.
> > >
> > > Yeah we need "auto", but IMO mainly just to keep the current userspace
> > > working. Using that to probe what's possible doesn't sound particularly
> > > workable since you can't use it with TEST_ONLY commits. Also change to
> > > some other property could still cause the whole thing to fail after the
> > > max bpc has been probed so not sure it really buys you anything.
> >
> > Hi Ville,
> >
> > earlier in this thread I drafted how the property-pair with "auto"
> > could be made useful also with TEST_ONLY commits:
> >
> > > Thinking even further, about the problem of TEST_ONLY commits not
> > > telling you what "auto" settings would actually give you; there could
> > > be a new/extended KMS ioctl that would be the same as commit now, but
> > > allow the kernel to return another set of KMS state back with
> > > TEST_ONLY. Like reading back all KMS state after commit was done for
> > > real. The "current" KMS properties would be part of that set, and tell
> > > userspace what would happen in a real commit.
> >
> > I do believe the combinatorial explosion of the KMS state search space
> > to find a working configuration is going to be a very real problem
> > sooner or later.
>
> That's seems like an orthogonal issue that would need some kind of
> new uapi approach that let's you get some state back out from
> TEST_ONLY commits.
>
> > > > Now that you mentioned some kind of color encoding property (I assume
> > > > you referred mostly to the sub-sampling aspect), how does the connector
> > > > property "Colorspace" factor in?
> > >
> > > The "Colorspace" property just changes what we report to the display
> > > via infoframes/MSA/SDP. It does not cause the display hardware to do
> > > any colorspace conversion/etc.
> >
> > Good.
> >
> > > To actually force the display hardware to do the desired conversion
> > > we need some new properties. ATM the only automagic conversion that
> > > can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
> > > which is needed on some displays to get 4k+ modes to work at all.
> >
> > When "Colorspace" says RGB, and the automatic fallback would kick in to
> > create a conflict, what happens?
>
> I would put that in the "Doctor, it hurts when I..." category.

So what are we supposed to do with the Colorspace property other than
setting it to default?

>
> >
> > > >
> > > > The enum values (which are not documented in KMS docs, btw.) are tuples
> > > > of color space + color model, e.g. on Intel:
> > > >
> > > > "Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601,
> > > > XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB,
> > > > BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater}
> > >
> > > The accepted values are just what the CTA-861/DP specs
> > > allow us to transmit in he infoframe/SDP/MSA.
> >
> > Sure, but I mean the KMS doc a) does not refer to any standard, and b)
> > does not even list what the possible values could be.
>
> Seems like something that can be remedied with a patch.
>
> >
> >
> > > >
> > > > Reading the KMS docs from
> > > > https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
> > > > they say:
> > > >
> > > > > Basically the expectation from userspace is:
> > > > >
> > > > >         Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > > > colorspace
> > > > >
> > > > >         Set this new property to let the sink know what it converted
> > > > > the CRTC output to.
> > > > >
> > > > >         This property is just to inform sink what colorspace source
> > > > > is trying to drive.
> > > >
> > > > However, where does userspace program the actual conversion from RGB to
> > > > NNN? Is it expected to be embedded in CTM?
> > > >
> > > > Or does setting "Colorspace" imply some additional automatic
> > > > conversion? If so, where does it happen and how is it chosen?
> > > >
> > > > > For just the plymouth case I guess the easiest thing would be to
> > > > > just clamp "max bpc" to the current value. The problem with that
> > > > > is that we'd potentially break existing userspace. Eg. I don't think
> > > > > the modesetting ddx or perhaps even most of the wayland compositors
> > > > > set "max bpc" at all. So we'd need to roll out a bunch of userspace
> > > > > updates first. And the "current bpc" prop idea would also have this
> > > > > same problem since plymouth would just clamp "max bpc" based on the
> > > > > current bpc before the real userspace starts.
> > > >
> > > > True, but I believe once color management spreads through Wayland, so
> > > > will KMS clients also learn to set it.
> > >
> > > Sure. But my point is that if we want to change how the "max bpc"
> > > works I think we need to roll out the userspace stuff first so that
> > > we at least can tell the user "please update you userspace to release x"
> > > when they hit the regression.
> >
> > Sorry, I lost track on who is suggesting to change what.
> >
> > I thought we agreed that "max bpc" means limiting link bpc to at most
> > that value, but the driver will automatically pick a lower value if
> > that makes the requested video mode work (and in lack of new KMS
> > properties).
> >
> > I have no desire that change that. I also think the Plymouth issue is
> > not fully fixable without some new KMS property, and until then the
> > only thing Plymouth could do is to smash "max bpc" always to 8 - which
> > mostly stops being a problem once display servers learn to handle "max
> > bpc".
>
> There's no real differene between the kernel automagically clamping
> "max bpc" to the current BIOS programmed value vs. plymouth doing it.
> Either approach will break deep color support for current userspace
> which doesn't reset "max bpc" back to the max.
>
> >
> > However, when new KMS properties are introduced, I very much like to
> > promote the two property setup for anything that could be a) set to
> > "auto", or b) be changed by the kernel *and* userspace.
> >
> >
> > Thanks,
> > pq
>
>
>
> --
> Ville Syrjälä
> Intel
>

Re: How should "max bpc" and "Colorspace" KMS property work?

[Ville Syrjälä]

On Thu, Jun 02, 2022 at 07:08:33PM +0200, Sebastian Wick wrote:
> On Thu, Jun 2, 2022 at 6:40 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Thu, Jun 02, 2022 at 10:47:59AM +0300, Pekka Paalanen wrote:
> > > On Wed, 1 Jun 2022 17:06:25 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > >
> > > > On Wed, Jun 01, 2022 at 10:21:26AM +0300, Pekka Paalanen wrote:
> > > > > On Tue, 31 May 2022 20:37:31 +0300
> > > > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > > >
> > > > > > On Wed, May 25, 2022 at 01:36:47PM +0300, Pekka Paalanen wrote:
> > > > > > > On Wed, 25 May 2022 09:23:51 +0000
> > > > > > > Simon Ser <contact@emersion.fr> wrote:
> > > > > > >
> > > > > > > > On Wednesday, May 25th, 2022 at 10:35, Michel Dänzer <michel.daenzer@mailbox.org> wrote:
> > > > > > > >
> > > > > > > > > > Mind that "max bpc" is always in auto. It's only an upper limit, with
> > > > > > > > > > the assumption that the driver can choose less.
> > > > > > > > >
> > > > > > > > > It seems to me like the "max bpc" property is just kind of bad API,
> > > > > > > > > and trying to tweak it to cater to more use cases as we discover them
> > > > > > > > > will take us down a rabbit hole. It seems better to replace it with
> > > > > > > > > new properties which allow user space to determine the current
> > > > > > > > > effective bpc and to explicitly control it.
> > > > > > > >
> > > > > > > > +1, this sounds much more robust, and allows better control by user-space.
> > > > > > > > User-space needs to have fallback logic for other state as well anyways.
> > > > > > > > If the combinatorial explosion is too much, we should think about optimizing
> > > > > > > > the KMS implementation, or improve the uAPI.
> > > > > > >
> > > > > > > +1 as well, with some recommendations added and the running off to the
> > > > > > > sunset:
> > > > > > >
> > > > > > > Use two separate KMS properties for reporting current vs.
> > > > > > > programming desired. The KMS property reporting the current value
> > > > > > > must be read-only (immutable). This preserves the difference between
> > > > > > > what userspace wanted and what it got, making it possible to read
> > > > > > > back both without confusing them. It also allows preserving driver behaviour
> > > > > >
> > > > > > I don't see much real point in a property to report the current bpc.
> > > > > > That can't be used to do anything atomically. So I suppose plymouth
> > > > > > would be the only user.
> > > > >
> > > > > Hi Ville,
> > > > >
> > > > > I think also professional color managed display servers would need it.
> > > > >
> > > > > If they detect that the link bpc is no longer the same as it was when
> > > > > the monitor was profiled, the profile will need to be re-verified by
> > > > > measuring the monitor again.
> > > > >
> > > > > See "Color calibration auditing system" notes in
> > > > > https://gitlab.freedesktop.org/wayland/weston/-/issues/467 description.
> > > > >
> > > > > > So IMO if someone really need explicit control over this then we
> > > > > > should just expose properties to set things explicitly. So we'd
> > > > > > need at least props for the actual bpc and some kind of color
> > > > > > encoding property (RGB/YCbCr 4:4:4/4:2:2:/4:2:0, etc.). And someone
> > > > > > would really need to figure out how DSC would interact with those.
> > > > >
> > > > > I believe there still must be "auto" setting for bpc, and a separate
> > > > > feedback property, so that userspace can use "auto" to see what it can
> > > > > get without doing thousands of TEST_ONLY commits plus a few "link
> > > > > status" failure handlings in order to find a working configuration (I'm
> > > > > assuming we have many more properties than just "max bpc" to figure
> > > > > out). After "auto" has told userspace what actually works without blind
> > > > > trial and error, then userspace can program than value explicitly to
> > > > > make sure it doesn't change accidentally in the future.
> > > >
> > > > Yeah we need "auto", but IMO mainly just to keep the current userspace
> > > > working. Using that to probe what's possible doesn't sound particularly
> > > > workable since you can't use it with TEST_ONLY commits. Also change to
> > > > some other property could still cause the whole thing to fail after the
> > > > max bpc has been probed so not sure it really buys you anything.
> > >
> > > Hi Ville,
> > >
> > > earlier in this thread I drafted how the property-pair with "auto"
> > > could be made useful also with TEST_ONLY commits:
> > >
> > > > Thinking even further, about the problem of TEST_ONLY commits not
> > > > telling you what "auto" settings would actually give you; there could
> > > > be a new/extended KMS ioctl that would be the same as commit now, but
> > > > allow the kernel to return another set of KMS state back with
> > > > TEST_ONLY. Like reading back all KMS state after commit was done for
> > > > real. The "current" KMS properties would be part of that set, and tell
> > > > userspace what would happen in a real commit.
> > >
> > > I do believe the combinatorial explosion of the KMS state search space
> > > to find a working configuration is going to be a very real problem
> > > sooner or later.
> >
> > That's seems like an orthogonal issue that would need some kind of
> > new uapi approach that let's you get some state back out from
> > TEST_ONLY commits.
> >
> > > > > Now that you mentioned some kind of color encoding property (I assume
> > > > > you referred mostly to the sub-sampling aspect), how does the connector
> > > > > property "Colorspace" factor in?
> > > >
> > > > The "Colorspace" property just changes what we report to the display
> > > > via infoframes/MSA/SDP. It does not cause the display hardware to do
> > > > any colorspace conversion/etc.
> > >
> > > Good.
> > >
> > > > To actually force the display hardware to do the desired conversion
> > > > we need some new properties. ATM the only automagic conversion that
> > > > can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
> > > > which is needed on some displays to get 4k+ modes to work at all.
> > >
> > > When "Colorspace" says RGB, and the automatic fallback would kick in to
> > > create a conflict, what happens?
> >
> > I would put that in the "Doctor, it hurts when I..." category.
> 
> So what are we supposed to do with the Colorspace property other than
> setting it to default?

You set it to the correct value based on what you have in the
framebuffers(s). Granted the automagic YCbCr 4:2:0 fallback can screw
you over right now, and that is why we'll need explicit properties to
control the output encoding/etc.

> 
> >
> > >
> > > > >
> > > > > The enum values (which are not documented in KMS docs, btw.) are tuples
> > > > > of color space + color model, e.g. on Intel:
> > > > >
> > > > > "Colorspace": enum {Default, SMPTE_170M_YCC, BT709_YCC, XVYCC_601,
> > > > > XVYCC_709, SYCC_601, opYCC_601, opRGB, BT2020_CYCC, BT2020_RGB,
> > > > > BT2020_YCC, DCI-P3_RGB_D65, DCI-P3_RGB_Theater}
> > > >
> > > > The accepted values are just what the CTA-861/DP specs
> > > > allow us to transmit in he infoframe/SDP/MSA.
> > >
> > > Sure, but I mean the KMS doc a) does not refer to any standard, and b)
> > > does not even list what the possible values could be.
> >
> > Seems like something that can be remedied with a patch.
> >
> > >
> > >
> > > > >
> > > > > Reading the KMS docs from
> > > > > https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
> > > > > they say:
> > > > >
> > > > > > Basically the expectation from userspace is:
> > > > > >
> > > > > >         Set up CRTC DEGAMMA/CTM/GAMMA to convert to some sink
> > > > > > colorspace
> > > > > >
> > > > > >         Set this new property to let the sink know what it converted
> > > > > > the CRTC output to.
> > > > > >
> > > > > >         This property is just to inform sink what colorspace source
> > > > > > is trying to drive.
> > > > >
> > > > > However, where does userspace program the actual conversion from RGB to
> > > > > NNN? Is it expected to be embedded in CTM?
> > > > >
> > > > > Or does setting "Colorspace" imply some additional automatic
> > > > > conversion? If so, where does it happen and how is it chosen?
> > > > >
> > > > > > For just the plymouth case I guess the easiest thing would be to
> > > > > > just clamp "max bpc" to the current value. The problem with that
> > > > > > is that we'd potentially break existing userspace. Eg. I don't think
> > > > > > the modesetting ddx or perhaps even most of the wayland compositors
> > > > > > set "max bpc" at all. So we'd need to roll out a bunch of userspace
> > > > > > updates first. And the "current bpc" prop idea would also have this
> > > > > > same problem since plymouth would just clamp "max bpc" based on the
> > > > > > current bpc before the real userspace starts.
> > > > >
> > > > > True, but I believe once color management spreads through Wayland, so
> > > > > will KMS clients also learn to set it.
> > > >
> > > > Sure. But my point is that if we want to change how the "max bpc"
> > > > works I think we need to roll out the userspace stuff first so that
> > > > we at least can tell the user "please update you userspace to release x"
> > > > when they hit the regression.
> > >
> > > Sorry, I lost track on who is suggesting to change what.
> > >
> > > I thought we agreed that "max bpc" means limiting link bpc to at most
> > > that value, but the driver will automatically pick a lower value if
> > > that makes the requested video mode work (and in lack of new KMS
> > > properties).
> > >
> > > I have no desire that change that. I also think the Plymouth issue is
> > > not fully fixable without some new KMS property, and until then the
> > > only thing Plymouth could do is to smash "max bpc" always to 8 - which
> > > mostly stops being a problem once display servers learn to handle "max
> > > bpc".
> >
> > There's no real differene between the kernel automagically clamping
> > "max bpc" to the current BIOS programmed value vs. plymouth doing it.
> > Either approach will break deep color support for current userspace
> > which doesn't reset "max bpc" back to the max.
> >
> > >
> > > However, when new KMS properties are introduced, I very much like to
> > > promote the two property setup for anything that could be a) set to
> > > "auto", or b) be changed by the kernel *and* userspace.
> > >
> > >
> > > Thanks,
> > > pq
> >
> >
> >
> > --
> > Ville Syrjälä
> > Intel
> >

-- 
Ville Syrjälä
Intel

Re: How should "max bpc" and "Colorspace" KMS property work?

[Pekka Paalanen]

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

On Thu, 2 Jun 2022 19:40:01 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Thu, Jun 02, 2022 at 10:47:59AM +0300, Pekka Paalanen wrote:
> > On Wed, 1 Jun 2022 17:06:25 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >   

...

> > > The "Colorspace" property just changes what we report to the display
> > > via infoframes/MSA/SDP. It does not cause the display hardware to do
> > > any colorspace conversion/etc.  
> > 
> > Good.
> >   
> > > To actually force the display hardware to do the desired conversion
> > > we need some new properties. ATM the only automagic conversion that
> > > can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
> > > which is needed on some displays to get 4k+ modes to work at all.  
> > 
> > When "Colorspace" says RGB, and the automatic fallback would kick in to
> > create a conflict, what happens?  
> 
> I would put that in the "Doctor, it hurts when I..." category.

Hi Ville,

I meant specifically the case where the driver chooses to use YCbCr
4:2:0 even though userspace is setting absolutely everything to RGB.

So yeah, that is a problem, like you and Sebastian agreed later.

Am I safe from that automatic driver fallback if I don't use 4k or
bigger video modes? What about e.g. 1080p@120 or something? Can I
calculate when the fallback will happen and choose my "Colorspace"
accordingly? Which also requires me to set up the RGB->YCbCR color
model conversion myself?

What about failing the modeset instead if userspace sets "Colorspace"
explicitly to RGB, and the driver would need to fall back to YCbCR
4:2:0?

That would make the most sense to me, as then the driver would not
silently do a buggy thing.


> > I thought we agreed that "max bpc" means limiting link bpc to at most
> > that value, but the driver will automatically pick a lower value if
> > that makes the requested video mode work (and in lack of new KMS
> > properties).
> > 
> > I have no desire that change that. I also think the Plymouth issue is
> > not fully fixable without some new KMS property, and until then the
> > only thing Plymouth could do is to smash "max bpc" always to 8 - which
> > mostly stops being a problem once display servers learn to handle "max
> > bpc".  
> 
> There's no real differene between the kernel automagically clamping
> "max bpc" to the current BIOS programmed value vs. plymouth doing it.
> Either approach will break deep color support for current userspace
> which doesn't reset "max bpc" back to the max.

There is one big difference: if Plymouth does it, then people cannot
scream "kernel regression". People can point fingers at Plymouth, but I
would imagine the actual fixes will come as patches to other KMS clients
and not Plymouth.


Thanks,
pq

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

Re: How should "max bpc" and "Colorspace" KMS property work?

[Ville Syrjälä]

On Fri, Jun 03, 2022 at 10:19:09AM +0300, Pekka Paalanen wrote:
> On Thu, 2 Jun 2022 19:40:01 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Thu, Jun 02, 2022 at 10:47:59AM +0300, Pekka Paalanen wrote:
> > > On Wed, 1 Jun 2022 17:06:25 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > >   
> 
> ...
> 
> > > > The "Colorspace" property just changes what we report to the display
> > > > via infoframes/MSA/SDP. It does not cause the display hardware to do
> > > > any colorspace conversion/etc.  
> > > 
> > > Good.
> > >   
> > > > To actually force the display hardware to do the desired conversion
> > > > we need some new properties. ATM the only automagic conversion that
> > > > can happen (at least with i915) is the RGB 4:4:4->YCbCr 4:2:0 fallback,
> > > > which is needed on some displays to get 4k+ modes to work at all.  
> > > 
> > > When "Colorspace" says RGB, and the automatic fallback would kick in to
> > > create a conflict, what happens?  
> > 
> > I would put that in the "Doctor, it hurts when I..." category.
> 
> Hi Ville,
> 
> I meant specifically the case where the driver chooses to use YCbCr
> 4:2:0 even though userspace is setting absolutely everything to RGB.
> 
> So yeah, that is a problem, like you and Sebastian agreed later.
> 
> Am I safe from that automatic driver fallback if I don't use 4k or
> bigger video modes? What about e.g. 1080p@120 or something? Can I
> calculate when the fallback will happen and choose my "Colorspace"
> accordingly? Which also requires me to set up the RGB->YCbCR color
> model conversion myself?

Porbably not something you can trivially compute unless you
replicate the exact logic from the driver.

> 
> What about failing the modeset instead if userspace sets "Colorspace"
> explicitly to RGB, and the driver would need to fall back to YCbCR
> 4:2:0?
> 
> That would make the most sense to me, as then the driver would not
> silently do a buggy thing.

I'm not sure there's much point in polishing that turd too much.
Should just add the explicit props and make userspace that actually
cares about color management set them exactly as it likes.

> > > I thought we agreed that "max bpc" means limiting link bpc to at most
> > > that value, but the driver will automatically pick a lower value if
> > > that makes the requested video mode work (and in lack of new KMS
> > > properties).
> > > 
> > > I have no desire that change that. I also think the Plymouth issue is
> > > not fully fixable without some new KMS property, and until then the
> > > only thing Plymouth could do is to smash "max bpc" always to 8 - which
> > > mostly stops being a problem once display servers learn to handle "max
> > > bpc".  
> > 
> > There's no real differene between the kernel automagically clamping
> > "max bpc" to the current BIOS programmed value vs. plymouth doing it.
> > Either approach will break deep color support for current userspace
> > which doesn't reset "max bpc" back to the max.
> 
> There is one big difference: if Plymouth does it, then people cannot
> scream "kernel regression".

But they'll just report the bugs against i915 anyway. I'd rather
not have to deal with those without at least being able to point
them at an existing fix, at least for the more common wayland
compositors+Xorg.

> People can point fingers at Plymouth, but I
> would imagine the actual fixes will come as patches to other KMS clients
> and not Plymouth.

I'm worried that we'll be stuck herding these bug reports
for quite some time.

-- 
Ville Syrjälä
Intel