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

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

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

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.

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

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

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.

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?

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?

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.

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

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.

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/

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-p... 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

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-p... 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

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-p... 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

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.

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.