Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Am 06.07.21 um 09:09 schrieb Pekka Paalanen: On Mon, 5 Jul 2021 17:49:42 +0200 Werner Sembach wrote: Am 01.07.21 um 15:24 schrieb Pekka Paalanen: On Thu, 1 Jul 2021 14:50:13 +0200 Werner Sembach wrote: Am 01.07.21 um 10:07 schrieb Pekka Paalanen: On Wed, 30 Jun 2021 11:20:18 +0200 Werner Sembach wrote: Am 30.06.21 um 10:41 schrieb Pekka Paalanen: On Tue, 29 Jun 2021 13:39:18 +0200 Werner Sembach wrote: Am 29.06.21 um 13:17 schrieb Pekka Paalanen: On Tue, 29 Jun 2021 08:12:54 + Simon Ser wrote: On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen wrote: yes, I think this makes sense, even if it is a property that one can't tell for sure what it does before hand. Using a pair of properties, preference and active, to ask for something and then check what actually worked is good for reducing the combinatorial explosion caused by needing to "atomic TEST_ONLY commit" test different KMS configurations. Userspace has a better chance of finding a configuration that is possible. OTOH, this has the problem than in UI one cannot tell the user in advance which options are truly possible. Given that KMS properties are rarely completely independent, and in this case known to depend on several other KMS properties, I think it is good enough to know after the fact. If a driver does not use what userspace prefers, there is no way to understand why, or what else to change to make it happen. That problem exists anyway, because TEST_ONLY commits do not give useful feedback but only a yes/no. By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one property at a time), user-space can discover which property makes the atomic commit fail. That works if the properties are independent of each other. Color range, color format, bpc and more may all be interconnected, allowing only certain combinations to work. If all these properties have "auto" setting too, then it would be possible to probe each property individually, but that still does not tell which combinations are valid. If you probe towards a certain configuration by setting the properties one by one, then depending on the order you pick the properties, you may come to a different conclusion on which property breaks the configuration. My mind crossed another point that must be considered: When plugin in a Monitor a list of possible Resolutions+Framerate combinations is created for xrandr and other userspace (I guess by atomic checks? but I don't know). Hi, I would not think so, but I hope to be corrected if I'm wrong. My belief is that the driver collects a list of modes from EDID, some standard modes, and maybe some other hardcoded modes, and then validates each entry against all the known limitations like vertical and horizontal frequency limits, discarding modes that do not fit. Not all limitations are known during that phase, which is why KMS property "link-status" exists. When userspace actually programs a mode (not a TEST_ONLY commit), the link training may fail. The kernel prunes the mode from the list and sets the link status property to signal failure, and sends a hotplug uevent. Userspace needs to re-check the mode list and try again. That is a generic escape hatch for when TEST_ONLY commit succeeds, but in reality the hardware cannot do it, you just cannot know until you actually try for real. It causes end user visible flicker if it happens on an already running connector, but since it usually happens when turning a connector on to begin with, there is no flicker to be seen, just a small delay in finding a mode that works. During this drm properties are already considered, which is no problem atm because as far as i can tell there is currently no drm property that would make a certain Resolutions+Framerate combination unreachable that would be possible with everything on default. I would not expect KMS properties to be considered at all. It would reject modes that are actually possible if the some KMS properties were changed. So at least going forward, current KMS property values cannot factor in. At least the debugfs variable "force_yuv420_output" did change the available modes here: https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 before my patch https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf Hi, debugfs is not proper UAPI, so we can just ignore it. Display servers cannot be expected to poke in debugfs. Debugfs is not even supposed to exist in production systems, but I'm sure people use it for hacking stuff. But that's all it is for: developer testing and hacking. e.g. Ubuntu has it active by default, but only read (and writable) by root. Hi, that's normal, yes. Root can do damage anyway, and it's useful for debugging. KMS clients OTOH often do not run as root. Forcing a color format via a DRM property in this
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Mon, 5 Jul 2021 17:49:42 +0200 Werner Sembach wrote: > Am 01.07.21 um 15:24 schrieb Pekka Paalanen: > > On Thu, 1 Jul 2021 14:50:13 +0200 > > Werner Sembach wrote: > > > >> Am 01.07.21 um 10:07 schrieb Pekka Paalanen: > >> > >>> On Wed, 30 Jun 2021 11:20:18 +0200 > >>> Werner Sembach wrote: > >>> > Am 30.06.21 um 10:41 schrieb Pekka Paalanen: > > > On Tue, 29 Jun 2021 13:39:18 +0200 > > Werner Sembach wrote: > > > >> Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > >>> On Tue, 29 Jun 2021 08:12:54 + > >>> Simon Ser wrote: > >>> > On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen > wrote: > > > yes, I think this makes sense, even if it is a property that one > > can't > > tell for sure what it does before hand. > > > > Using a pair of properties, preference and active, to ask for > > something > > and then check what actually worked is good for reducing the > > combinatorial explosion caused by needing to "atomic TEST_ONLY > > commit" > > test different KMS configurations. Userspace has a better chance of > > finding a configuration that is possible. > > > > OTOH, this has the problem than in UI one cannot tell the user in > > advance which options are truly possible. Given that KMS properties > > are > > rarely completely independent, and in this case known to depend on > > several other KMS properties, I think it is good enough to know > > after > > the fact. > > > > If a driver does not use what userspace prefers, there is no way to > > understand why, or what else to change to make it happen. That > > problem > > exists anyway, because TEST_ONLY commits do not give useful feedback > > but only a yes/no. > By submitting incremental atomic reqs with TEST_ONLY (i.e. only > changing one > property at a time), user-space can discover which property makes > the atomic > commit fail. > >>> That works if the properties are independent of each other. Color > >>> range, color format, bpc and more may all be interconnected, > >>> allowing only certain combinations to work. > >>> > >>> If all these properties have "auto" setting too, then it would be > >>> possible to probe each property individually, but that still does not > >>> tell which combinations are valid. > >>> > >>> If you probe towards a certain configuration by setting the properties > >>> one by one, then depending on the order you pick the properties, you > >>> may come to a different conclusion on which property breaks the > >>> configuration. > >> My mind crossed another point that must be considered: When plugin in > >> a Monitor a list of possible Resolutions+Framerate combinations is > >> created for xrandr and other userspace (I guess by atomic checks? but > >> I don't know). > > Hi, > > > > I would not think so, but I hope to be corrected if I'm wrong. > > > > My belief is that the driver collects a list of modes from EDID, some > > standard modes, and maybe some other hardcoded modes, and then > > validates each entry against all the known limitations like vertical > > and horizontal frequency limits, discarding modes that do not fit. > > > > Not all limitations are known during that phase, which is why KMS > > property "link-status" exists. When userspace actually programs a mode > > (not a TEST_ONLY commit), the link training may fail. The kernel prunes > > the mode from the list and sets the link status property to signal > > failure, and sends a hotplug uevent. Userspace needs to re-check the > > mode list and try again. > > > > That is a generic escape hatch for when TEST_ONLY commit succeeds, but > > in reality the hardware cannot do it, you just cannot know until you > > actually try for real. It causes end user visible flicker if it happens > > on an already running connector, but since it usually happens when > > turning a connector on to begin with, there is no flicker to be seen, > > just a small delay in finding a mode that works. > > > >> During this drm > >> properties are already considered, which is no problem atm because as > >> far as i can tell there is currently no drm property that would make > >> a certain Resolutions+Framerate combination unreachable that would be > >> possible with everything on default. > > I would not expect KMS properties to be considered at all. It would > > reject modes that are actually possible if the some KMS properties were > > changed. So at least going forward, current KMS property values cannot >
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Am 01.07.21 um 15:24 schrieb Pekka Paalanen: > On Thu, 1 Jul 2021 14:50:13 +0200 > Werner Sembach wrote: > >> Am 01.07.21 um 10:07 schrieb Pekka Paalanen: >> >>> On Wed, 30 Jun 2021 11:20:18 +0200 >>> Werner Sembach wrote: >>> Am 30.06.21 um 10:41 schrieb Pekka Paalanen: > On Tue, 29 Jun 2021 13:39:18 +0200 > Werner Sembach wrote: > >> Am 29.06.21 um 13:17 schrieb Pekka Paalanen: >>> On Tue, 29 Jun 2021 08:12:54 + >>> Simon Ser wrote: >>> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen wrote: > yes, I think this makes sense, even if it is a property that one can't > tell for sure what it does before hand. > > Using a pair of properties, preference and active, to ask for > something > and then check what actually worked is good for reducing the > combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > test different KMS configurations. Userspace has a better chance of > finding a configuration that is possible. > > OTOH, this has the problem than in UI one cannot tell the user in > advance which options are truly possible. Given that KMS properties > are > rarely completely independent, and in this case known to depend on > several other KMS properties, I think it is good enough to know after > the fact. > > If a driver does not use what userspace prefers, there is no way to > understand why, or what else to change to make it happen. That problem > exists anyway, because TEST_ONLY commits do not give useful feedback > but only a yes/no. By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one property at a time), user-space can discover which property makes the atomic commit fail. >>> That works if the properties are independent of each other. Color >>> range, color format, bpc and more may all be interconnected, >>> allowing only certain combinations to work. >>> >>> If all these properties have "auto" setting too, then it would be >>> possible to probe each property individually, but that still does not >>> tell which combinations are valid. >>> >>> If you probe towards a certain configuration by setting the properties >>> one by one, then depending on the order you pick the properties, you >>> may come to a different conclusion on which property breaks the >>> configuration. >> My mind crossed another point that must be considered: When plugin in >> a Monitor a list of possible Resolutions+Framerate combinations is >> created for xrandr and other userspace (I guess by atomic checks? but >> I don't know). > Hi, > > I would not think so, but I hope to be corrected if I'm wrong. > > My belief is that the driver collects a list of modes from EDID, some > standard modes, and maybe some other hardcoded modes, and then > validates each entry against all the known limitations like vertical > and horizontal frequency limits, discarding modes that do not fit. > > Not all limitations are known during that phase, which is why KMS > property "link-status" exists. When userspace actually programs a mode > (not a TEST_ONLY commit), the link training may fail. The kernel prunes > the mode from the list and sets the link status property to signal > failure, and sends a hotplug uevent. Userspace needs to re-check the > mode list and try again. > > That is a generic escape hatch for when TEST_ONLY commit succeeds, but > in reality the hardware cannot do it, you just cannot know until you > actually try for real. It causes end user visible flicker if it happens > on an already running connector, but since it usually happens when > turning a connector on to begin with, there is no flicker to be seen, > just a small delay in finding a mode that works. > >> During this drm >> properties are already considered, which is no problem atm because as >> far as i can tell there is currently no drm property that would make >> a certain Resolutions+Framerate combination unreachable that would be >> possible with everything on default. > I would not expect KMS properties to be considered at all. It would > reject modes that are actually possible if the some KMS properties were > changed. So at least going forward, current KMS property values cannot > factor in. At least the debugfs variable "force_yuv420_output" did change the available modes here: https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 before my patch
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Thu, 1 Jul 2021 14:50:13 +0200 Werner Sembach wrote: > Am 01.07.21 um 10:07 schrieb Pekka Paalanen: > > > On Wed, 30 Jun 2021 11:20:18 +0200 > > Werner Sembach wrote: > > > >> Am 30.06.21 um 10:41 schrieb Pekka Paalanen: > >> > >>> On Tue, 29 Jun 2021 13:39:18 +0200 > >>> Werner Sembach wrote: > >>> > Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > > On Tue, 29 Jun 2021 08:12:54 + > > Simon Ser wrote: > > > >> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen > >> wrote: > >> > >>> yes, I think this makes sense, even if it is a property that one can't > >>> tell for sure what it does before hand. > >>> > >>> Using a pair of properties, preference and active, to ask for > >>> something > >>> and then check what actually worked is good for reducing the > >>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > >>> test different KMS configurations. Userspace has a better chance of > >>> finding a configuration that is possible. > >>> > >>> OTOH, this has the problem than in UI one cannot tell the user in > >>> advance which options are truly possible. Given that KMS properties > >>> are > >>> rarely completely independent, and in this case known to depend on > >>> several other KMS properties, I think it is good enough to know after > >>> the fact. > >>> > >>> If a driver does not use what userspace prefers, there is no way to > >>> understand why, or what else to change to make it happen. That problem > >>> exists anyway, because TEST_ONLY commits do not give useful feedback > >>> but only a yes/no. > >> By submitting incremental atomic reqs with TEST_ONLY (i.e. only > >> changing one > >> property at a time), user-space can discover which property makes the > >> atomic > >> commit fail. > > That works if the properties are independent of each other. Color > > range, color format, bpc and more may all be interconnected, > > allowing only certain combinations to work. > > > > If all these properties have "auto" setting too, then it would be > > possible to probe each property individually, but that still does not > > tell which combinations are valid. > > > > If you probe towards a certain configuration by setting the properties > > one by one, then depending on the order you pick the properties, you > > may come to a different conclusion on which property breaks the > > configuration. > My mind crossed another point that must be considered: When plugin in > a Monitor a list of possible Resolutions+Framerate combinations is > created for xrandr and other userspace (I guess by atomic checks? but > I don't know). > >>> Hi, > >>> > >>> I would not think so, but I hope to be corrected if I'm wrong. > >>> > >>> My belief is that the driver collects a list of modes from EDID, some > >>> standard modes, and maybe some other hardcoded modes, and then > >>> validates each entry against all the known limitations like vertical > >>> and horizontal frequency limits, discarding modes that do not fit. > >>> > >>> Not all limitations are known during that phase, which is why KMS > >>> property "link-status" exists. When userspace actually programs a mode > >>> (not a TEST_ONLY commit), the link training may fail. The kernel prunes > >>> the mode from the list and sets the link status property to signal > >>> failure, and sends a hotplug uevent. Userspace needs to re-check the > >>> mode list and try again. > >>> > >>> That is a generic escape hatch for when TEST_ONLY commit succeeds, but > >>> in reality the hardware cannot do it, you just cannot know until you > >>> actually try for real. It causes end user visible flicker if it happens > >>> on an already running connector, but since it usually happens when > >>> turning a connector on to begin with, there is no flicker to be seen, > >>> just a small delay in finding a mode that works. > >>> > During this drm > properties are already considered, which is no problem atm because as > far as i can tell there is currently no drm property that would make > a certain Resolutions+Framerate combination unreachable that would be > possible with everything on default. > >>> I would not expect KMS properties to be considered at all. It would > >>> reject modes that are actually possible if the some KMS properties were > >>> changed. So at least going forward, current KMS property values cannot > >>> factor in. > >> At least the debugfs variable "force_yuv420_output" did change the > >> available modes here: > >> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 > >> > >> before my patch > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf >
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Am 01.07.21 um 10:07 schrieb Pekka Paalanen: > On Wed, 30 Jun 2021 11:20:18 +0200 > Werner Sembach wrote: > >> Am 30.06.21 um 10:41 schrieb Pekka Paalanen: >> >>> On Tue, 29 Jun 2021 13:39:18 +0200 >>> Werner Sembach wrote: >>> Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > On Tue, 29 Jun 2021 08:12:54 + > Simon Ser wrote: > >> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen >> wrote: >> >>> yes, I think this makes sense, even if it is a property that one can't >>> tell for sure what it does before hand. >>> >>> Using a pair of properties, preference and active, to ask for something >>> and then check what actually worked is good for reducing the >>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" >>> test different KMS configurations. Userspace has a better chance of >>> finding a configuration that is possible. >>> >>> OTOH, this has the problem than in UI one cannot tell the user in >>> advance which options are truly possible. Given that KMS properties are >>> rarely completely independent, and in this case known to depend on >>> several other KMS properties, I think it is good enough to know after >>> the fact. >>> >>> If a driver does not use what userspace prefers, there is no way to >>> understand why, or what else to change to make it happen. That problem >>> exists anyway, because TEST_ONLY commits do not give useful feedback >>> but only a yes/no. >> By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing >> one >> property at a time), user-space can discover which property makes the >> atomic >> commit fail. > That works if the properties are independent of each other. Color > range, color format, bpc and more may all be interconnected, > allowing only certain combinations to work. > > If all these properties have "auto" setting too, then it would be > possible to probe each property individually, but that still does not > tell which combinations are valid. > > If you probe towards a certain configuration by setting the properties > one by one, then depending on the order you pick the properties, you > may come to a different conclusion on which property breaks the > configuration. My mind crossed another point that must be considered: When plugin in a Monitor a list of possible Resolutions+Framerate combinations is created for xrandr and other userspace (I guess by atomic checks? but I don't know). >>> Hi, >>> >>> I would not think so, but I hope to be corrected if I'm wrong. >>> >>> My belief is that the driver collects a list of modes from EDID, some >>> standard modes, and maybe some other hardcoded modes, and then >>> validates each entry against all the known limitations like vertical >>> and horizontal frequency limits, discarding modes that do not fit. >>> >>> Not all limitations are known during that phase, which is why KMS >>> property "link-status" exists. When userspace actually programs a mode >>> (not a TEST_ONLY commit), the link training may fail. The kernel prunes >>> the mode from the list and sets the link status property to signal >>> failure, and sends a hotplug uevent. Userspace needs to re-check the >>> mode list and try again. >>> >>> That is a generic escape hatch for when TEST_ONLY commit succeeds, but >>> in reality the hardware cannot do it, you just cannot know until you >>> actually try for real. It causes end user visible flicker if it happens >>> on an already running connector, but since it usually happens when >>> turning a connector on to begin with, there is no flicker to be seen, >>> just a small delay in finding a mode that works. >>> During this drm properties are already considered, which is no problem atm because as far as i can tell there is currently no drm property that would make a certain Resolutions+Framerate combination unreachable that would be possible with everything on default. >>> I would not expect KMS properties to be considered at all. It would >>> reject modes that are actually possible if the some KMS properties were >>> changed. So at least going forward, current KMS property values cannot >>> factor in. >> At least the debugfs variable "force_yuv420_output" did change the >> available modes here: >> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 >> >> before my patch >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf > Hi, > > debugfs is not proper UAPI, so we can just ignore it. Display servers > cannot be expected to poke in debugfs. Debugfs is not even supposed to > exist in production systems, but I'm sure people use it for hacking > stuff. But that's all it is for: developer testing and hacking. e.g.
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Wed, 30 Jun 2021 11:20:18 +0200 Werner Sembach wrote: > Am 30.06.21 um 10:41 schrieb Pekka Paalanen: > > > On Tue, 29 Jun 2021 13:39:18 +0200 > > Werner Sembach wrote: > > > >> Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > >>> On Tue, 29 Jun 2021 08:12:54 + > >>> Simon Ser wrote: > >>> > On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen > wrote: > > > yes, I think this makes sense, even if it is a property that one can't > > tell for sure what it does before hand. > > > > Using a pair of properties, preference and active, to ask for something > > and then check what actually worked is good for reducing the > > combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > > test different KMS configurations. Userspace has a better chance of > > finding a configuration that is possible. > > > > OTOH, this has the problem than in UI one cannot tell the user in > > advance which options are truly possible. Given that KMS properties are > > rarely completely independent, and in this case known to depend on > > several other KMS properties, I think it is good enough to know after > > the fact. > > > > If a driver does not use what userspace prefers, there is no way to > > understand why, or what else to change to make it happen. That problem > > exists anyway, because TEST_ONLY commits do not give useful feedback > > but only a yes/no. > By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing > one > property at a time), user-space can discover which property makes the > atomic > commit fail. > >>> That works if the properties are independent of each other. Color > >>> range, color format, bpc and more may all be interconnected, > >>> allowing only certain combinations to work. > >>> > >>> If all these properties have "auto" setting too, then it would be > >>> possible to probe each property individually, but that still does not > >>> tell which combinations are valid. > >>> > >>> If you probe towards a certain configuration by setting the properties > >>> one by one, then depending on the order you pick the properties, you > >>> may come to a different conclusion on which property breaks the > >>> configuration. > >> My mind crossed another point that must be considered: When plugin in > >> a Monitor a list of possible Resolutions+Framerate combinations is > >> created for xrandr and other userspace (I guess by atomic checks? but > >> I don't know). > > Hi, > > > > I would not think so, but I hope to be corrected if I'm wrong. > > > > My belief is that the driver collects a list of modes from EDID, some > > standard modes, and maybe some other hardcoded modes, and then > > validates each entry against all the known limitations like vertical > > and horizontal frequency limits, discarding modes that do not fit. > > > > Not all limitations are known during that phase, which is why KMS > > property "link-status" exists. When userspace actually programs a mode > > (not a TEST_ONLY commit), the link training may fail. The kernel prunes > > the mode from the list and sets the link status property to signal > > failure, and sends a hotplug uevent. Userspace needs to re-check the > > mode list and try again. > > > > That is a generic escape hatch for when TEST_ONLY commit succeeds, but > > in reality the hardware cannot do it, you just cannot know until you > > actually try for real. It causes end user visible flicker if it happens > > on an already running connector, but since it usually happens when > > turning a connector on to begin with, there is no flicker to be seen, > > just a small delay in finding a mode that works. > > > >> During this drm > >> properties are already considered, which is no problem atm because as > >> far as i can tell there is currently no drm property that would make > >> a certain Resolutions+Framerate combination unreachable that would be > >> possible with everything on default. > > I would not expect KMS properties to be considered at all. It would > > reject modes that are actually possible if the some KMS properties were > > changed. So at least going forward, current KMS property values cannot > > factor in. > > At least the debugfs variable "force_yuv420_output" did change the > available modes here: > https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 > > before my patch > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf Hi, debugfs is not proper UAPI, so we can just ignore it. Display servers cannot be expected to poke in debugfs. Debugfs is not even supposed to exist in production systems, but I'm sure people use it for hacking stuff. But that's all it is for: developer testing and hacking. > Forcing a color format via a DRM property in this function would >
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Am 30.06.21 um 10:41 schrieb Pekka Paalanen: On Tue, 29 Jun 2021 13:39:18 +0200 Werner Sembach wrote: Am 29.06.21 um 13:17 schrieb Pekka Paalanen: On Tue, 29 Jun 2021 08:12:54 + Simon Ser wrote: On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen wrote: yes, I think this makes sense, even if it is a property that one can't tell for sure what it does before hand. Using a pair of properties, preference and active, to ask for something and then check what actually worked is good for reducing the combinatorial explosion caused by needing to "atomic TEST_ONLY commit" test different KMS configurations. Userspace has a better chance of finding a configuration that is possible. OTOH, this has the problem than in UI one cannot tell the user in advance which options are truly possible. Given that KMS properties are rarely completely independent, and in this case known to depend on several other KMS properties, I think it is good enough to know after the fact. If a driver does not use what userspace prefers, there is no way to understand why, or what else to change to make it happen. That problem exists anyway, because TEST_ONLY commits do not give useful feedback but only a yes/no. By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one property at a time), user-space can discover which property makes the atomic commit fail. That works if the properties are independent of each other. Color range, color format, bpc and more may all be interconnected, allowing only certain combinations to work. If all these properties have "auto" setting too, then it would be possible to probe each property individually, but that still does not tell which combinations are valid. If you probe towards a certain configuration by setting the properties one by one, then depending on the order you pick the properties, you may come to a different conclusion on which property breaks the configuration. My mind crossed another point that must be considered: When plugin in a Monitor a list of possible Resolutions+Framerate combinations is created for xrandr and other userspace (I guess by atomic checks? but I don't know). Hi, I would not think so, but I hope to be corrected if I'm wrong. My belief is that the driver collects a list of modes from EDID, some standard modes, and maybe some other hardcoded modes, and then validates each entry against all the known limitations like vertical and horizontal frequency limits, discarding modes that do not fit. Not all limitations are known during that phase, which is why KMS property "link-status" exists. When userspace actually programs a mode (not a TEST_ONLY commit), the link training may fail. The kernel prunes the mode from the list and sets the link status property to signal failure, and sends a hotplug uevent. Userspace needs to re-check the mode list and try again. That is a generic escape hatch for when TEST_ONLY commit succeeds, but in reality the hardware cannot do it, you just cannot know until you actually try for real. It causes end user visible flicker if it happens on an already running connector, but since it usually happens when turning a connector on to begin with, there is no flicker to be seen, just a small delay in finding a mode that works. During this drm properties are already considered, which is no problem atm because as far as i can tell there is currently no drm property that would make a certain Resolutions+Framerate combination unreachable that would be possible with everything on default. I would not expect KMS properties to be considered at all. It would reject modes that are actually possible if the some KMS properties were changed. So at least going forward, current KMS property values cannot factor in. At least the debugfs variable "force_yuv420_output" did change the available modes here: https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 before my patch https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf Forcing a color format via a DRM property in this function would reintroduce the problem. And I think i915 driver works similar in this regard. However for example forcing YCbCr420 encoding would limit the available resolutions (my screen for example only supports YCbCr420 on 4k@60 and @50Hz and on no other resolution or frequency (native is 2560x1440@144Hz). So would a "force color format" that does not get resetted on repluging/reenabling a monitor break the output, for example, of an not updated xrandr, unaware of this new property? Yes, not because the mode list would be missing the mode, but because actually setting the mode would fail. Well, like described above, I think the mode would actually be missing, which is also an unexpected behavior from a user perspective. RandR in particular is problematic, because it does not actually understand any KMS properties, it
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Tue, 29 Jun 2021 13:39:18 +0200 Werner Sembach wrote: > Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > > On Tue, 29 Jun 2021 08:12:54 + > > Simon Ser wrote: > > > >> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen > >> wrote: > >> > >>> yes, I think this makes sense, even if it is a property that one can't > >>> tell for sure what it does before hand. > >>> > >>> Using a pair of properties, preference and active, to ask for something > >>> and then check what actually worked is good for reducing the > >>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > >>> test different KMS configurations. Userspace has a better chance of > >>> finding a configuration that is possible. > >>> > >>> OTOH, this has the problem than in UI one cannot tell the user in > >>> advance which options are truly possible. Given that KMS properties are > >>> rarely completely independent, and in this case known to depend on > >>> several other KMS properties, I think it is good enough to know after > >>> the fact. > >>> > >>> If a driver does not use what userspace prefers, there is no way to > >>> understand why, or what else to change to make it happen. That problem > >>> exists anyway, because TEST_ONLY commits do not give useful feedback > >>> but only a yes/no. > >> By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing > >> one > >> property at a time), user-space can discover which property makes the > >> atomic > >> commit fail. > > That works if the properties are independent of each other. Color > > range, color format, bpc and more may all be interconnected, > > allowing only certain combinations to work. > > > > If all these properties have "auto" setting too, then it would be > > possible to probe each property individually, but that still does not > > tell which combinations are valid. > > > > If you probe towards a certain configuration by setting the properties > > one by one, then depending on the order you pick the properties, you > > may come to a different conclusion on which property breaks the > > configuration. > > My mind crossed another point that must be considered: When plugin in > a Monitor a list of possible Resolutions+Framerate combinations is > created for xrandr and other userspace (I guess by atomic checks? but > I don't know). Hi, I would not think so, but I hope to be corrected if I'm wrong. My belief is that the driver collects a list of modes from EDID, some standard modes, and maybe some other hardcoded modes, and then validates each entry against all the known limitations like vertical and horizontal frequency limits, discarding modes that do not fit. Not all limitations are known during that phase, which is why KMS property "link-status" exists. When userspace actually programs a mode (not a TEST_ONLY commit), the link training may fail. The kernel prunes the mode from the list and sets the link status property to signal failure, and sends a hotplug uevent. Userspace needs to re-check the mode list and try again. That is a generic escape hatch for when TEST_ONLY commit succeeds, but in reality the hardware cannot do it, you just cannot know until you actually try for real. It causes end user visible flicker if it happens on an already running connector, but since it usually happens when turning a connector on to begin with, there is no flicker to be seen, just a small delay in finding a mode that works. > During this drm > properties are already considered, which is no problem atm because as > far as i can tell there is currently no drm property that would make > a certain Resolutions+Framerate combination unreachable that would be > possible with everything on default. I would not expect KMS properties to be considered at all. It would reject modes that are actually possible if the some KMS properties were changed. So at least going forward, current KMS property values cannot factor in. > However for example forcing YCbCr420 encoding would limit the > available resolutions (my screen for example only supports YCbCr420 > on 4k@60 and @50Hz and on no other resolution or frequency (native is > 2560x1440@144Hz). > > So would a "force color format" that does not get resetted on > repluging/reenabling a monitor break the output, for example, of an > not updated xrandr, unaware of this new property? Yes, not because the mode list would be missing the mode, but because actually setting the mode would fail. RandR in particular is problematic, because it does not actually understand any KMS properties, it is merely a relay. So anything that *uses* RandR protocol or xrandr command would also need to be patched to understand the new properties. The kernel automatically resetting *some* properties in *some* occasions seems really fragile and complicated to me, which is why I'm a lot more keen to see a "reset everything to sensible defaults" generic mechanism added to KMS. Thanks, pq pgpQK9N1g1OVZ.pgp Description:
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > On Tue, 29 Jun 2021 08:12:54 + > Simon Ser wrote: > >> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen >> wrote: >> >>> yes, I think this makes sense, even if it is a property that one can't >>> tell for sure what it does before hand. >>> >>> Using a pair of properties, preference and active, to ask for something >>> and then check what actually worked is good for reducing the >>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" >>> test different KMS configurations. Userspace has a better chance of >>> finding a configuration that is possible. >>> >>> OTOH, this has the problem than in UI one cannot tell the user in >>> advance which options are truly possible. Given that KMS properties are >>> rarely completely independent, and in this case known to depend on >>> several other KMS properties, I think it is good enough to know after >>> the fact. >>> >>> If a driver does not use what userspace prefers, there is no way to >>> understand why, or what else to change to make it happen. That problem >>> exists anyway, because TEST_ONLY commits do not give useful feedback >>> but only a yes/no. >> By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one >> property at a time), user-space can discover which property makes the atomic >> commit fail. > That works if the properties are independent of each other. Color > range, color format, bpc and more may all be interconnected, > allowing only certain combinations to work. > > If all these properties have "auto" setting too, then it would be > possible to probe each property individually, but that still does not > tell which combinations are valid. > > If you probe towards a certain configuration by setting the properties > one by one, then depending on the order you pick the properties, you > may come to a different conclusion on which property breaks the > configuration. My mind crossed another point that must be considered: When plugin in a Monitor a list of possible Resolutions+Framerate combinations is created for xrandr and other userspace (I guess by atomic checks? but I don't know). During this drm properties are already considered, which is no problem atm because as far as i can tell there is currently no drm property that would make a certain Resolutions+Framerate combination unreachable that would be possible with everything on default. However for example forcing YCbCr420 encoding would limit the available resolutions (my screen for example only supports YCbCr420 on 4k@60 and @50Hz and on no other resolution or frequency (native is 2560x1440@144Hz). So would a "force color format" that does not get resetted on repluging/reenabling a monitor break the output, for example, of an not updated xrandr, unaware of this new property? >> I'm not a fan of this "preference" property approach. The only way to find >> out >> whether it's possible to change the color format is to perform a user-visible >> change (with a regular atomic commit) and check whether it worked >> after-the-fact. This is unlike all other existing KMS properties. > I agree. FWIW, "max bpc" exists already. > >> I'd much rather see a more general approach to fix this combinatorial >> explosion >> than to add special-cases like this. > What would you suggest? > > Maybe all properties should have an "auto" value in addition to the > explicit no-negotiation values where at all possible? > > That might help probing each property individually with TEST_ONLY > commits, but it says nothing about combinations. > > A feedback list perhaps? TEST_ONLY commit somehow returning a list of > property/value tuples indicating what value the "auto" valued > properties actually get? > > What should a kernel driver optimize for when determining "auto" values? > > > Thanks, > pq > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > On Tue, 29 Jun 2021 08:12:54 + > Simon Ser wrote: > >> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen >> wrote: >> >>> yes, I think this makes sense, even if it is a property that one can't >>> tell for sure what it does before hand. >>> >>> Using a pair of properties, preference and active, to ask for something >>> and then check what actually worked is good for reducing the >>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" >>> test different KMS configurations. Userspace has a better chance of >>> finding a configuration that is possible. >>> >>> OTOH, this has the problem than in UI one cannot tell the user in >>> advance which options are truly possible. Given that KMS properties are >>> rarely completely independent, and in this case known to depend on >>> several other KMS properties, I think it is good enough to know after >>> the fact. >>> >>> If a driver does not use what userspace prefers, there is no way to >>> understand why, or what else to change to make it happen. That problem >>> exists anyway, because TEST_ONLY commits do not give useful feedback >>> but only a yes/no. >> By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one >> property at a time), user-space can discover which property makes the atomic >> commit fail. > That works if the properties are independent of each other. Color > range, color format, bpc and more may all be interconnected, > allowing only certain combinations to work. > > If all these properties have "auto" setting too, then it would be > possible to probe each property individually, but that still does not > tell which combinations are valid. > > If you probe towards a certain configuration by setting the properties > one by one, then depending on the order you pick the properties, you > may come to a different conclusion on which property breaks the > configuration. My mind crossed another point that must be considered: When plugin in a Monitor a list of possible Resolutions+Framerate combinations is created for xrandr and other userspace (I guess by atomic checks? but I don't know). During this drm properties are already considered, which is no problem atm because as far as i can tell there is currently no drm property that would make a certain Resolutions+Framerate combination unreachable that would be possible with everything on default. However for example forcing YCbCr420 encoding would limit the available resolutions (my screen for example only supports YCbCr420 on 4k@60 and @50Hz and on no other resolution or frequency (native is 2560x1440@144Hz). So would a "force color format" that does not get resetted on repluging/reenabling a monitor break the output, for example, of an not updated xrandr, unaware of this new property? > >> I'm not a fan of this "preference" property approach. The only way to find >> out >> whether it's possible to change the color format is to perform a user-visible >> change (with a regular atomic commit) and check whether it worked >> after-the-fact. This is unlike all other existing KMS properties. > I agree. FWIW, "max bpc" exists already. > >> I'd much rather see a more general approach to fix this combinatorial >> explosion >> than to add special-cases like this. > What would you suggest? > > Maybe all properties should have an "auto" value in addition to the > explicit no-negotiation values where at all possible? > > That might help probing each property individually with TEST_ONLY > commits, but it says nothing about combinations. > > A feedback list perhaps? TEST_ONLY commit somehow returning a list of > property/value tuples indicating what value the "auto" valued > properties actually get? > > What should a kernel driver optimize for when determining "auto" values? > > > Thanks, > pq > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Tue, 29 Jun 2021 08:12:54 + Simon Ser wrote: > On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen > wrote: > > > yes, I think this makes sense, even if it is a property that one can't > > tell for sure what it does before hand. > > > > Using a pair of properties, preference and active, to ask for something > > and then check what actually worked is good for reducing the > > combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > > test different KMS configurations. Userspace has a better chance of > > finding a configuration that is possible. > > > > OTOH, this has the problem than in UI one cannot tell the user in > > advance which options are truly possible. Given that KMS properties are > > rarely completely independent, and in this case known to depend on > > several other KMS properties, I think it is good enough to know after > > the fact. > > > > If a driver does not use what userspace prefers, there is no way to > > understand why, or what else to change to make it happen. That problem > > exists anyway, because TEST_ONLY commits do not give useful feedback > > but only a yes/no. > > By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one > property at a time), user-space can discover which property makes the atomic > commit fail. That works if the properties are independent of each other. Color range, color format, bpc and more may all be interconnected, allowing only certain combinations to work. If all these properties have "auto" setting too, then it would be possible to probe each property individually, but that still does not tell which combinations are valid. If you probe towards a certain configuration by setting the properties one by one, then depending on the order you pick the properties, you may come to a different conclusion on which property breaks the configuration. > I'm not a fan of this "preference" property approach. The only way to find out > whether it's possible to change the color format is to perform a user-visible > change (with a regular atomic commit) and check whether it worked > after-the-fact. This is unlike all other existing KMS properties. I agree. FWIW, "max bpc" exists already. > I'd much rather see a more general approach to fix this combinatorial > explosion > than to add special-cases like this. What would you suggest? Maybe all properties should have an "auto" value in addition to the explicit no-negotiation values where at all possible? That might help probing each property individually with TEST_ONLY commits, but it says nothing about combinations. A feedback list perhaps? TEST_ONLY commit somehow returning a list of property/value tuples indicating what value the "auto" valued properties actually get? What should a kernel driver optimize for when determining "auto" values? Thanks, pq pgpfbFvx4TeQP.pgp Description: OpenPGP digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen wrote: > yes, I think this makes sense, even if it is a property that one can't > tell for sure what it does before hand. > > Using a pair of properties, preference and active, to ask for something > and then check what actually worked is good for reducing the > combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > test different KMS configurations. Userspace has a better chance of > finding a configuration that is possible. > > OTOH, this has the problem than in UI one cannot tell the user in > advance which options are truly possible. Given that KMS properties are > rarely completely independent, and in this case known to depend on > several other KMS properties, I think it is good enough to know after > the fact. > > If a driver does not use what userspace prefers, there is no way to > understand why, or what else to change to make it happen. That problem > exists anyway, because TEST_ONLY commits do not give useful feedback > but only a yes/no. By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing one property at a time), user-space can discover which property makes the atomic commit fail. I'm not a fan of this "preference" property approach. The only way to find out whether it's possible to change the color format is to perform a user-visible change (with a regular atomic commit) and check whether it worked after-the-fact. This is unlike all other existing KMS properties. I'd much rather see a more general approach to fix this combinatorial explosion than to add special-cases like this. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Fri, 18 Jun 2021 11:11:11 +0200
Werner Sembach wrote:
> Add a new general drm property "preferred color format" which can be used
> by userspace to tell the graphic drivers to which color format to use.
>
> Possible options are:
> - auto (default/current behaviour)
> - rgb
> - ycbcr444
> - ycbcr422 (not supported by both amdgpu and i915)
> - ycbcr420
>
> In theory the auto option should choose the best available option for the
> current setup, but because of bad internal conversion some monitors look
> better with rgb and some with ycbcr444.
>
> Also, because of bad shielded connectors and/or cables, it might be
> preferable to use the less bandwidth heavy ycbcr422 and ycbcr420 formats
> for a signal that is less deceptible to interference.
>
> In the future, automatic color calibration for screens might also depend on
> this option being available.
>
> Signed-off-by: Werner Sembach
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 4 +++
> drivers/gpu/drm/drm_atomic_uapi.c | 4 +++
> drivers/gpu/drm/drm_connector.c | 48 -
> include/drm/drm_connector.h | 17 ++
> 4 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index bc3487964fb5..90d62f305257 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -687,6 +687,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> if (old_connector_state->max_requested_bpc !=
> new_connector_state->max_requested_bpc)
> new_crtc_state->connectors_changed = true;
> +
> + if (old_connector_state->preferred_color_format !=
> + new_connector_state->preferred_color_format)
> + new_crtc_state->connectors_changed = true;
> }
>
> if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 438e9585b225..c536f5e22016 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -796,6 +796,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> fence_ptr);
> } else if (property == connector->max_bpc_property) {
> state->max_requested_bpc = val;
> + } else if (property == connector->preferred_color_format_property) {
> + state->preferred_color_format = val;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -873,6 +875,8 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = 0;
> } else if (property == connector->max_bpc_property) {
> *val = state->max_requested_bpc;
> + } else if (property == connector->preferred_color_format_property) {
> + *val = state->preferred_color_format;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 818de58d972f..aea03dd02e33 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -889,6 +889,14 @@ static const struct drm_prop_enum_list
> drm_dp_subconnector_enum_list[] = {
> { DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */
> };
>
> +static const struct drm_prop_enum_list
> drm_preferred_color_format_enum_list[] = {
> + { 0, "auto" },
> + { DRM_COLOR_FORMAT_RGB444, "rgb" },
> + { DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
> + { DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
> + { DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
> +};
> +
> static const struct drm_prop_enum_list drm_active_color_format_enum_list[] =
> {
> { 0, "unknown" },
> { DRM_COLOR_FORMAT_RGB444, "rgb" },
> @@ -1219,11 +1227,19 @@ static const struct drm_prop_enum_list
> dp_colorspaces[] = {
> * Drivers shall use drm_connector_attach_active_bpc_property() to install
> * this property.
> *
> + * preferred color format:
> + * This property is used by userspace to change the used color format. When
> + * used the driver will use the selected format if valid for the hardware,
> + * sink, and current resolution and refresh rate combination. Drivers to
> + * use the function drm_connector_attach_preferred_color_format_property()
> + * to create and attach the property to the connector during
> + * initialization.
> + *
> * active color format:
> * This read-only property tells userspace the color format actually used
> * by the
[Intel-gfx] [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Add a new general drm property "preferred color format" which can be used
by userspace to tell the graphic drivers to which color format to use.
Possible options are:
- auto (default/current behaviour)
- rgb
- ycbcr444
- ycbcr422 (not supported by both amdgpu and i915)
- ycbcr420
In theory the auto option should choose the best available option for the
current setup, but because of bad internal conversion some monitors look
better with rgb and some with ycbcr444.
Also, because of bad shielded connectors and/or cables, it might be
preferable to use the less bandwidth heavy ycbcr422 and ycbcr420 formats
for a signal that is less deceptible to interference.
In the future, automatic color calibration for screens might also depend on
this option being available.
Signed-off-by: Werner Sembach
---
drivers/gpu/drm/drm_atomic_helper.c | 4 +++
drivers/gpu/drm/drm_atomic_uapi.c | 4 +++
drivers/gpu/drm/drm_connector.c | 48 -
include/drm/drm_connector.h | 17 ++
4 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index bc3487964fb5..90d62f305257 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -687,6 +687,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (old_connector_state->max_requested_bpc !=
new_connector_state->max_requested_bpc)
new_crtc_state->connectors_changed = true;
+
+ if (old_connector_state->preferred_color_format !=
+ new_connector_state->preferred_color_format)
+ new_crtc_state->connectors_changed = true;
}
if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 438e9585b225..c536f5e22016 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -796,6 +796,8 @@ static int drm_atomic_connector_set_property(struct
drm_connector *connector,
fence_ptr);
} else if (property == connector->max_bpc_property) {
state->max_requested_bpc = val;
+ } else if (property == connector->preferred_color_format_property) {
+ state->preferred_color_format = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -873,6 +875,8 @@ drm_atomic_connector_get_property(struct drm_connector
*connector,
*val = 0;
} else if (property == connector->max_bpc_property) {
*val = state->max_requested_bpc;
+ } else if (property == connector->preferred_color_format_property) {
+ *val = state->preferred_color_format;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 818de58d972f..aea03dd02e33 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -889,6 +889,14 @@ static const struct drm_prop_enum_list
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Native, "Native"}, /* DP */
};
+static const struct drm_prop_enum_list drm_preferred_color_format_enum_list[]
= {
+ { 0, "auto" },
+ { DRM_COLOR_FORMAT_RGB444, "rgb" },
+ { DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
+ { DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
+ { DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
+};
+
static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
{ 0, "unknown" },
{ DRM_COLOR_FORMAT_RGB444, "rgb" },
@@ -1219,11 +1227,19 @@ static const struct drm_prop_enum_list dp_colorspaces[]
= {
* Drivers shall use drm_connector_attach_active_bpc_property() to install
* this property.
*
+ * preferred color format:
+ * This property is used by userspace to change the used color format. When
+ * used the driver will use the selected format if valid for the hardware,
+ * sink, and current resolution and refresh rate combination. Drivers to
+ * use the function drm_connector_attach_preferred_color_format_property()
+ * to create and attach the property to the connector during
+ * initialization.
+ *
* active color format:
* This read-only property tells userspace the color format actually used
* by the hardware display engine on "the cable" on a connector. The chosen
* value depends on hardware capabilities, both display engine and
- * connected monitor. Drivers shall use
+ *
