All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Sebastian Wick <sebastian.wick@redhat.com>,
	dri-devel@lists.freedesktop.org,
	Uma Shankar <uma.shankar@intel.com>,
	amd-gfx@lists.freedesktop.org, Joshua Ashton <joshua@froggi.es>,
	Vitaly.Prosyak@amd.com
Subject: Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
Date: Wed, 15 Feb 2023 12:33:01 +0200	[thread overview]
Message-ID: <Y+y03W0GWUxSucBW@intel.com> (raw)
In-Reply-To: <20230215120125.59b5965c@eldfell>

On Wed, Feb 15, 2023 at 12:01:25PM +0200, Pekka Paalanen wrote:
> On Tue, 14 Feb 2023 22:10:35 +0200
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Tue, Feb 14, 2023 at 08:45:00PM +0100, Sebastian Wick wrote:
> 
> ...
> 
> > > We also have to figure out how a user space which doesn't
> > > know about the new property behaves when another client has set that
> > > property. If any property which currently might change the pixel
> > > values is used, we can't expose the entire color pipeline because the
> > > kernel might have to use some element in it to achieve its magic
> > > conversion. So essentially you already have this hard device between
> > > "old" and "new" and you can't use the new stuff incrementally.  
> > 
> > That problem exists with any new property. Old userspace and new
> > userspace may interact badly enought that nothing works right.
> > In that sense I think these props might even be pretty mundane
> > as the worst you might get from setting the infoframe wrong is
> > perhaps wrong colors on your display.
> > 
> > To solve that particular problem there has been talk (for years)
> > about some kind of "reset all" knob to make sure everything is
> > at a safe default value. I have a feeling there was even some
> > kind of semi-real proposal in recent times, but maybe I imgained
> > it?
> 
> I've been talking about that too, but I think it all collapsed into
> "let's just fix all KMS apps to always set all KMS properties" which
> results in patches like
> https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/952

That requires some knowledge about the property in question to
pick the value. I think for some prop types (enums at least)
we could guarantee that the first value is always the safe default,
but for eg. range properties there is no way to know. So doing
that fully blind is not possible atm.

I guess one option might be to include a "reset value" in the
props somehow, and just have everyclient set all unknown props
to that. But there are of course other options too (reset
flag to atomic ioctl, etc.).

> 
> It does not seem to be a serious enough problem for anyone to put in
> the work. And why would it be, when you can easily fix it in your own
> project like that Weston example. The Weston example is not even
> representative, because I did it before I saw any real problems.
> 
> Other musings have been in the direction that maybe logind (since it
> opens DRM devices for you) should save the full KMS state on the very
> first open after a reboot, and then KMS applications can ask logind
> what the boot-up state was. This is a variation of "save all KMS state
> from the moment you launch, and use that as the base if you ever let
> something else touch KMS in between".
> 
> You also never see the problem to begin with, if you never let
> something else touch KMS in between, so that already makes the problem
> rare outside of the tiny set of compositor developers.

Yeah, it's a pretty rare problem so not much interest I guess.

-- 
Ville Syrjälä
Intel

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Sebastian Wick <sebastian.wick@redhat.com>,
	dri-devel@lists.freedesktop.org,
	Uma Shankar <uma.shankar@intel.com>,
	amd-gfx@lists.freedesktop.org,
	Harry Wentland <harry.wentland@amd.com>,
	Joshua Ashton <joshua@froggi.es>,
	Vitaly.Prosyak@amd.com
Subject: Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
Date: Wed, 15 Feb 2023 12:33:01 +0200	[thread overview]
Message-ID: <Y+y03W0GWUxSucBW@intel.com> (raw)
In-Reply-To: <20230215120125.59b5965c@eldfell>

On Wed, Feb 15, 2023 at 12:01:25PM +0200, Pekka Paalanen wrote:
> On Tue, 14 Feb 2023 22:10:35 +0200
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Tue, Feb 14, 2023 at 08:45:00PM +0100, Sebastian Wick wrote:
> 
> ...
> 
> > > We also have to figure out how a user space which doesn't
> > > know about the new property behaves when another client has set that
> > > property. If any property which currently might change the pixel
> > > values is used, we can't expose the entire color pipeline because the
> > > kernel might have to use some element in it to achieve its magic
> > > conversion. So essentially you already have this hard device between
> > > "old" and "new" and you can't use the new stuff incrementally.  
> > 
> > That problem exists with any new property. Old userspace and new
> > userspace may interact badly enought that nothing works right.
> > In that sense I think these props might even be pretty mundane
> > as the worst you might get from setting the infoframe wrong is
> > perhaps wrong colors on your display.
> > 
> > To solve that particular problem there has been talk (for years)
> > about some kind of "reset all" knob to make sure everything is
> > at a safe default value. I have a feeling there was even some
> > kind of semi-real proposal in recent times, but maybe I imgained
> > it?
> 
> I've been talking about that too, but I think it all collapsed into
> "let's just fix all KMS apps to always set all KMS properties" which
> results in patches like
> https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/952

That requires some knowledge about the property in question to
pick the value. I think for some prop types (enums at least)
we could guarantee that the first value is always the safe default,
but for eg. range properties there is no way to know. So doing
that fully blind is not possible atm.

I guess one option might be to include a "reset value" in the
props somehow, and just have everyclient set all unknown props
to that. But there are of course other options too (reset
flag to atomic ioctl, etc.).

> 
> It does not seem to be a serious enough problem for anyone to put in
> the work. And why would it be, when you can easily fix it in your own
> project like that Weston example. The Weston example is not even
> representative, because I did it before I saw any real problems.
> 
> Other musings have been in the direction that maybe logind (since it
> opens DRM devices for you) should save the full KMS state on the very
> first open after a reboot, and then KMS applications can ask logind
> what the boot-up state was. This is a variation of "save all KMS state
> from the moment you launch, and use that as the base if you ever let
> something else touch KMS in between".
> 
> You also never see the problem to begin with, if you never let
> something else touch KMS in between, so that already makes the problem
> rare outside of the tiny set of compositor developers.

Yeah, it's a pretty rare problem so not much interest I guess.

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2023-02-15 10:35 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-03  2:07 [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum Joshua Ashton
2023-02-03  2:07 ` Joshua Ashton
2023-02-03  2:07 ` [PATCH 2/3] drm/connector: Add enum documentation to drm_colorspace Joshua Ashton
2023-02-03  2:07   ` Joshua Ashton
2023-02-08  8:56   ` Pekka Paalanen
2023-02-08  8:56     ` Pekka Paalanen
2023-02-16 21:22     ` Harry Wentland
2023-02-16 21:22       ` Harry Wentland
2023-02-03  2:07 ` [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum Joshua Ashton
2023-02-03  2:07   ` Joshua Ashton
2023-02-03 10:39   ` Ville Syrjälä
2023-02-03 12:59     ` Sebastian Wick
2023-02-03 13:35       ` Ville Syrjälä
2023-02-03 13:52         ` Sebastian Wick
2023-02-03 14:02           ` Ville Syrjälä
2023-02-08  9:18             ` Pekka Paalanen
2023-02-08 14:49               ` Ville Syrjälä
2023-02-09 10:05                 ` Pekka Paalanen
2023-02-09 10:05                   ` Pekka Paalanen
2023-02-03 14:39       ` Harry Wentland
2023-02-03 15:19         ` Ville Syrjälä
2023-02-03 15:24           ` Harry Wentland
2023-02-03 16:00             ` Ville Syrjälä
2023-02-03 18:28               ` Harry Wentland
2023-02-03 18:56                 ` Ville Syrjälä
2023-02-03 19:16                   ` Harry Wentland
2023-02-03 19:25                   ` Ville Syrjälä
2023-02-03 19:33                     ` Harry Wentland
2023-02-08 10:03                       ` Pekka Paalanen
2023-02-08 10:03                         ` Pekka Paalanen
2023-02-03 19:34                     ` Ville Syrjälä
2023-02-03 19:43                       ` Harry Wentland
2023-02-04  6:09                       ` Joshua Ashton
2023-02-06  9:47                         ` Ville Syrjälä
2023-02-06  9:47                           ` Ville Syrjälä
2023-02-06 17:16                           ` Harry Wentland
2023-02-08 10:32                             ` Pekka Paalanen
2023-02-08 10:32                               ` Pekka Paalanen
2023-02-08  9:30                   ` Pekka Paalanen
2023-02-08  9:30                     ` Pekka Paalanen
2023-02-08  9:25               ` Pekka Paalanen
2023-02-08  9:25                 ` Pekka Paalanen
2023-02-14 15:49               ` Sebastian Wick
2023-02-14 15:49                 ` Sebastian Wick
2023-02-14 16:56                 ` Harry Wentland
2023-02-14 19:45                   ` Sebastian Wick
2023-02-14 19:45                     ` Sebastian Wick
2023-02-14 20:04                     ` Harry Wentland
2023-02-14 20:04                       ` Harry Wentland
2023-02-15  9:40                       ` Pekka Paalanen
2023-02-15  9:40                         ` Pekka Paalanen
2023-02-15 20:45                         ` Harry Wentland
2023-02-15 20:45                           ` Harry Wentland
2023-02-14 20:10                     ` Ville Syrjälä
2023-02-14 20:10                       ` Ville Syrjälä
2023-02-14 21:18                       ` Sebastian Wick
2023-02-14 21:18                         ` Sebastian Wick
2023-02-14 21:27                         ` Ville Syrjälä
2023-02-14 21:27                           ` Ville Syrjälä
2023-02-15 10:01                       ` Pekka Paalanen
2023-02-15 10:01                         ` Pekka Paalanen
2023-02-15 10:33                         ` Ville Syrjälä [this message]
2023-02-15 10:33                           ` Ville Syrjälä
2023-02-15 11:46                   ` Daniel Stone
2023-02-15 11:46                     ` Daniel Stone
2023-02-15 20:54                     ` Harry Wentland
2023-02-15 20:54                       ` Harry Wentland
2023-02-15 22:07                       ` Daniel Stone
2023-02-15 22:07                         ` Daniel Stone
2023-02-03 14:52   ` Harry Wentland
2023-02-04 16:06   ` kernel test robot
2023-02-04 16:06     ` kernel test robot
2023-02-08  9:30   ` Pekka Paalanen
2023-02-08  9:30     ` Pekka Paalanen
2023-02-09 16:38     ` Joshua Ashton
2023-02-09 16:38       ` Joshua Ashton
2023-02-09 17:03       ` Simon Ser
2023-02-09 18:08         ` Ville Syrjälä
2023-02-10  9:37         ` Pekka Paalanen
2023-02-10  9:44           ` Simon Ser
2023-02-04  9:06 ` [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum kernel test robot
2023-02-04  9:06   ` kernel test robot
2023-02-04 10:28 ` kernel test robot
2023-02-04 10:28   ` kernel test robot
2023-02-08  8:29 ` Pekka Paalanen
2023-02-08  8:29   ` Pekka Paalanen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y+y03W0GWUxSucBW@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=Vitaly.Prosyak@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=joshua@froggi.es \
    --cc=ppaalanen@gmail.com \
    --cc=sebastian.wick@redhat.com \
    --cc=uma.shankar@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.