AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Harry Wentland <harry.wentland@amd.com>
Cc: Sebastian Wick <sebastian.wick@redhat.com>,
	dri-devel@lists.freedesktop.org,
	Pekka Paalanen <ppaalanen@gmail.com>,
	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: Fri, 3 Feb 2023 18:00:44 +0200	[thread overview]
Message-ID: <Y90vrEa3/1RbaGOV@intel.com> (raw)
In-Reply-To: <758e5cf6-53e0-567c-c760-5b773bc7a11c@amd.com>

On Fri, Feb 03, 2023 at 10:24:52AM -0500, Harry Wentland wrote:
> 
> 
> On 2/3/23 10:19, Ville Syrjälä wrote:
> > On Fri, Feb 03, 2023 at 09:39:42AM -0500, Harry Wentland wrote:
> >>
> >>
> >> On 2/3/23 07:59, Sebastian Wick wrote:
> >>> On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä
> >>> <ville.syrjala@linux.intel.com> wrote:
> >>>>
> >>>> On Fri, Feb 03, 2023 at 02:07:44AM +0000, Joshua Ashton wrote:
> >>>>> Userspace has no way of controlling or knowing the pixel encoding
> >>>>> currently, so there is no way for it to ever get the right values here.
> >>>>
> >>>> That applies to a lot of the other values as well (they are
> >>>> explicitly RGB or YCC). The idea was that this property sets the
> >>>> infoframe/MSA/SDP value exactly, and other properties should be
> >>>> added to for use userspace to control the pixel encoding/colorspace
> >>>> conversion(if desired, or userspace just makes sure to
> >>>> directly feed in correct kind of data).
> >>>
> >>> I'm all for getting userspace control over pixel encoding but even
> >>> then the kernel always knows which pixel encoding is selected and
> >>> which InfoFrame has to be sent. Is there a reason why userspace would
> >>> want to control the variant explicitly to the wrong value?
> >>>
> >>
> >> I've asked this before but haven't seen an answer: Is there an existing
> >> upstream userspace project that makes use of this property (other than
> >> what Joshua is working on in gamescope right now)? That would help us
> >> understand the intent better.
> > 
> > The intent was to control the infoframe colorimetry bits,
> > nothing more. No idea what real userspace there was, if any.
> > 
> >>
> >> I don't think giving userspace explicit control over the exact infoframe
> >> values is the right thing to do.
> > 
> > Only userspace knows what kind of data it's stuffing into
> > the pixels (and/or how it configures the csc units/etc.) to
> > generate them.
> > 
> 
> Yes, but userspace doesn't control or know whether we drive
> RGB or YCbCr on the wire. In fact, in some cases our driver
> needs to fallback to YCbCr420 for bandwidth reasons. There
> is currently no way for userspace to know that and I don't
> think it makes sense.

People want that control as well for whatever reason. We've
been asked to allow YCbCr 4:4:4 output many times.

The automagic 4:2:0 fallback I think is rather fundementally
incompatible with fancy color management. How would we even
know whether to use eg. BT.2020 vs. BT.709 matrix? In i915
that stuff is just always BT.709 limited range, no questions
asked.

So I think if userspace wants real color management it's
going to have to set up the whole pipeline. And for that
we need at least one new property to control the RGB->YCbCr
conversion (or to explicitly avoid it).

And given that the proposed patch just swept all the
non-BT.2020 issues under the rug makes me think no
one has actually come up with any kind of consistent
plan for anything else really.

> 
> Userspace needs full control of framebuffer pixel formats,
> as well as control over DEGAMMA, GAMMA, CTM color operations.
> It also needs to be able to select whether to drive the panel
> as sRGB or BT.2020/PQ but it doesn't make sense for it to
> control the pixel encoding on the wire (RGB vs YCbCr).
> 
> > I really don't want a repeat of the disaster of the
> > 'Broadcast RGB' which has coupled together the infoframe 
> > and automagic conversion stuff. And I think this one would
> > be about 100x worse given this property has something
> > to do with actual colorspaces as well.
> >  
> 
> I'm unaware of this disaster. Could you elaborate?

The property now controls both the infoframe stuff (and
whatever super vague stuff DP has for it in MSA) and 
full->limited range compression in the display pipeline. 
And as a result  there is no way to eg. allow already 
limited range input, which is what some people wanted.

And naturally it's all made a lot more terrible by all
the displays that fail to implement the spec correctly,
but that's another topic.

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2023-02-03 16:00 UTC|newest]

Thread overview: 56+ 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 ` [PATCH 2/3] drm/connector: Add enum documentation to drm_colorspace Joshua Ashton
2023-02-08  8:56   ` Pekka Paalanen
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 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-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ä [this message]
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-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 17:16                           ` Harry Wentland
2023-02-08 10:32                             ` Pekka Paalanen
2023-02-08  9:30                   ` Pekka Paalanen
2023-02-08  9:25               ` Pekka Paalanen
2023-02-14 15:49               ` Sebastian Wick
2023-02-14 16:56                 ` Harry Wentland
2023-02-14 19:45                   ` Sebastian Wick
2023-02-14 20:04                     ` Harry Wentland
2023-02-15  9:40                       ` Pekka Paalanen
2023-02-15 20:45                         ` Harry Wentland
2023-02-14 20:10                     ` Ville Syrjälä
2023-02-14 21:18                       ` Sebastian Wick
2023-02-14 21:27                         ` Ville Syrjälä
2023-02-15 10:01                       ` Pekka Paalanen
2023-02-15 10:33                         ` Ville Syrjälä
2023-02-15 11:46                   ` Daniel Stone
2023-02-15 20:54                     ` Harry Wentland
2023-02-15 22:07                       ` Daniel Stone
2023-02-03 14:52   ` Harry Wentland
2023-02-04 16:06   ` kernel test robot
2023-02-08  9:30   ` Pekka Paalanen
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 10:28 ` kernel test robot
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=Y90vrEa3/1RbaGOV@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=harry.wentland@amd.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox