From: Andri Yngvason <andri@yngvason.is>
To: Sebastian Wick <sebastian.wick@redhat.com>
Cc: dri-devel@lists.freedesktop.org,
"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"Werner Sembach" <wse@tuxedocomputers.com>,
"Leo Li" <sunpeng.li@amd.com>, "David Airlie" <airlied@gmail.com>,
intel-gfx@lists.freedesktop.org, "Pan,
Xinhui" <Xinhui.Pan@amd.com>,
"Rodrigo Siqueira" <Rodrigo.Siqueira@amd.com>,
linux-kernel@vger.kernel.org,
"Maxime Ripard" <mripard@kernel.org>,
"Jani Nikula" <jani.nikula@linux.intel.com>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
"Alex Deucher" <alexander.deucher@amd.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
amd-gfx@lists.freedesktop.org,
"Harry Wentland" <harry.wentland@amd.com>,
"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH v2 2/4] drm/uAPI: Add "force color format" drm property as setting for userspace
Date: Tue, 16 Jan 2024 14:11:43 +0000 [thread overview]
Message-ID: <CAFNQBQyfWmfu5T7bgZDZFGfyhsxQi7YXmY_wPc9Y+mm5iSspXQ@mail.gmail.com> (raw)
In-Reply-To: <20240116132918.GB311990@toolbox>
þri., 16. jan. 2024 kl. 13:29 skrifaði Sebastian Wick
<sebastian.wick@redhat.com>:
>
> On Tue, Jan 16, 2024 at 01:13:13PM +0000, Andri Yngvason wrote:
[...]
> > şri., 16. jan. 2024 kl. 11:42 skrifaği Sebastian Wick
> > <sebastian.wick@redhat.com>:
> > >
> > > On Mon, Jan 15, 2024 at 04:05:52PM +0000, Andri Yngvason wrote:
> > > > From: Werner Sembach <wse@tuxedocomputers.com>
> > > >
> > > > Add a new general drm property "force color format" which can be used
> > > > by userspace to tell the graphics driver which color format to use.
> > >
> > > I don't like the "force" in the name. This just selects the color
> > > format, let's just call it "color format" then.
> > >
> >
> > In previous revisions, this was "preferred color format" and "actual
> > color format", of which the latter has been dropped. I recommend
> > reading the discussion for previous revisions.
>
> Please don't imply that I didn't read the thread I'm answering to.
>
> > There are arguments for adding "actual color format" later and if it
> > is added later, we'd end up with "color format" and "actual color
> > format", which might be confusing, and it is why I chose to call it
> > "force color format" because it clearly communicates intent and
> > disambiguates it from "actual color format".
>
> There is no such thing as "actual color format" in upstream though.
> Basing your naming on discarded ideas is not useful. The thing that sets
> the color space for example is called "Colorspace", not "force
> colorspace".
>
Sure, I'm happy with calling it whatever people want. Maybe we can
have a vote on it?
> > [...]
> > > > @@ -1396,6 +1404,15 @@ static const u32 dp_colorspaces =
> > > > * drm_connector_attach_max_bpc_property() to create and attach the
> > > > * property to the connector during initialization.
> > > > *
> > > > + * force 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,
> > >
> > > All properties are always "used", they just can have different values.
> > > You probably want to talk about the auto mode here.
> >
> > Maybe we can say something like: If userspace does not set the
> > property or if it is explicitly set to zero, the driver will select
> > the appropriate color format based on other constraints.
>
> The property can be in any state without involvement from user space.
> Don't talk about setting it, talk about the state it is in:
>
> When the color format is auto, the driver will select a format.
>
Ok.
> > >
> > > > + * sink, and current resolution and refresh rate combination. Drivers to
> > >
> > > If valid? So when a value is not actually supported user space can still
> > > set it? What happens then? How should user space figure out if the
> > > driver and the sink support the format?
> >
> > The kernel does not expose this property unless it's implemented in the driver.
>
> If the driver simply doesn't support *one format*, the enum value for
> that format should not be exposed, period. This isn't about the property
> on its own.
Right, understood. You mean that enum should only contain values that
are supported by the driver.
>
> > This was originally "preferred color format". Perhaps the
> > documentation should better reflect that it is now a mandatory
> > constraint which fails the modeset if not satisfied.
>
> That would definitely help.
>
> > >
> > > For the Colorspace prop, the kernel just exposes all formats it supports
> > > (independent of the sink) and then makes it the job of user space to
> > > figure out if the sink supports it.
> > >
> > > The same could be done here. Property value is exposed if the driver
> > > supports it in general, commits can fail if the driver can't support it
> > > for a specific commit because e.g. the resolution or refresh rate. User
> > > space must look at the EDID/DisplayID/mode to figure out the supported
> > > format for the sink.
> >
> > Yes, we can make it possible for userspace to discover which modes are
> > supported by the monitor, but there are other constraints that need to
> > be satisfied. This was discussed in the previous revision.
>
> I mean, yes, that's what I said. User space would then only be
> responsible for checking the sink capabilities and the atomic check
> would take into account other (non-sink) constraints.
Since we need to probe using TEST_ONLY anyway, we'll end up with two
mechanisms to do the same thing where one of them depends on the other
for completeness.
>
> > In any case, these things can be added later and need not be a part of
> > this change set.
>
> No, this is the contract between the kernel and user space and has to be
> figured out before we can merge new uAPI.
>
> >
> > [...]
> >
Thanks,
Andri
next prev parent reply other threads:[~2024-01-17 9:31 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-15 16:05 [PATCH v2 0/4] New DRM properties for output color format Andri Yngvason
2024-01-15 16:05 ` [PATCH v2 1/4] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check Andri Yngvason
2024-04-17 19:57 ` Harry Wentland
2024-01-15 16:05 ` [PATCH v2 2/4] drm/uAPI: Add "force color format" drm property as setting for userspace Andri Yngvason
2024-01-16 11:42 ` Sebastian Wick
2024-01-16 13:13 ` Andri Yngvason
2024-01-16 13:29 ` Sebastian Wick
2024-01-16 14:11 ` Andri Yngvason [this message]
2024-01-17 9:21 ` Pekka Paalanen
2024-01-17 12:58 ` Andri Yngvason
2024-01-18 21:40 ` Sebastian Wick
2024-01-19 8:42 ` Pekka Paalanen
2024-04-17 19:57 ` Harry Wentland
2024-01-15 16:05 ` [PATCH v2 3/4] drm/amd/display: Add handling for new "force color format" property Andri Yngvason
2024-01-15 16:05 ` [PATCH v2 4/4] drm/i915/display: " Andri Yngvason
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=CAFNQBQyfWmfu5T7bgZDZFGfyhsxQi7YXmY_wPc9Y+mm5iSspXQ@mail.gmail.com \
--to=andri@yngvason.is \
--cc=Rodrigo.Siqueira@amd.com \
--cc=Xinhui.Pan@amd.com \
--cc=airlied@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=harry.wentland@amd.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=joonas.lahtinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=rodrigo.vivi@intel.com \
--cc=sebastian.wick@redhat.com \
--cc=sunpeng.li@amd.com \
--cc=tvrtko.ursulin@linux.intel.com \
--cc=tzimmermann@suse.de \
--cc=wse@tuxedocomputers.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;
as well as URLs for NNTP newsgroup(s).