Re: [RFC PATCH 0/3] Support for Solid Fill Planes
On Mon, 26 Jun 2023 16:02:50 -0700
Jessica Zhang wrote:
> On 11/7/2022 11:37 AM, Ville Syrjälä wrote:
> > On Fri, Oct 28, 2022 at 03:59:49PM -0700, Jessica Zhang wrote:
> >> Introduce and add support for COLOR_FILL and COLOR_FILL_FORMAT
> >> properties. When the color fill value is set, and the framebuffer is set
> >> to NULL, memory fetch will be disabled.
> >
> > Thinking a bit more universally I wonder if there should be
> > some kind of enum property:
> >
> > enum plane_pixel_source {
> > FB,
> > COLOR,
> > LIVE_FOO,
> > LIVE_BAR,
> > ...
> > }
>
> Reviving this thread as this was the initial comment suggesting to
> implement pixel_source as an enum.
>
> I think the issue with having pixel_source as an enum is how to decide
> what counts as a NULL commit.
>
> Currently, setting the FB to NULL will disable the plane. So I'm
> guessing we will extend that logic to "if there's no pixel_source set
> for the plane, then it will be a NULL commit and disable the plane".
>
> In that case, the question then becomes when to set the pixel_source to
> NONE. Because if we do that when setting a NULL FB (or NULL solid_fill
> blob), it then forces userspace to set one property before the other.
Right, that won't work.
There is no ordering between each property being set inside a single
atomic commit. They can all be applied to kernel-internal state
theoretically simultaneously, or any arbitrary random order, and the
end result must always be the same. Hence, setting one property cannot
change the state of another mutable property. I believe that doing
otherwise would make userspace fragile and hard to get right.
I guess there might be an exception to that rule when the same property
is set multiple times in a single atomic commit; the last setting in
the array prevails. That's universal and not a special-case between two
specific properties.
> Because of that, I'm thinking of having pixel_source be represented by a
> bitmask instead. That way, we will simply unset the corresponding
> pixel_source bit when passing in a NULL FB/solid_fill blob. Then, in
> order to detect whether a commit is NULL or has a valid pixel source, we
> can just check if pixel_source == 0.
Sounds fine to me at first hand, but isn't there the enum property that
says if the kernel must look at solid_fill blob *or* FB_ID?
If enum prop says "use solid_fill prop", the why would changes to FB_ID
do anything? Is it for backwards-compatibility with KMS clients that do
not know about the enum prop?
It seems like that kind of backwards-compatiblity will cause problems
in trying to reason about the atomic state, as explained above, leading
to very delicate and fragile conditions where things work intuitively.
Hence, I'm not sure backwards-compatibility is wanted. This won't be
the first or the last KMS property where an unexpected value left over
will make old atomic KMS clients silently malfunction up to showing no
recognisable picture at all. *If* that problem needs solving, there
have been ideas floating around about resetting everything to nice
values so that userspace can ignore what it does not understand. So far
there has been no real interest in solving that problem in the kernel
though.
Legacy non-atomic UAPI wrappers can do whatever they want, and program
any (new) properties they want in order to implement the legacy
expectations, so that does not seem to be a problem.
Thanks,
pq
>
> Would be interested in any feedback on this.
>
> Thanks,
>
> Jessica Zhang
>
> >
> >> In addition, loosen the NULL FB checks within the atomic commit callstack
> >> to allow a NULL FB when color_fill is nonzero and add FB checks in
> >> methods where the FB was previously assumed to be non-NULL.
> >>
> >> Finally, have the DPU driver use drm_plane_state.color_fill and
> >> drm_plane_state.color_fill_format instead of dpu_plane_state.color_fill,
> >> and add extra checks in the DPU atomic commit callstack to account for a
> >> NULL FB in cases where color_fill is set.
> >>
> >> Some drivers support hardware that have optimizations for solid fill
> >> planes. This series aims to expose these capabilities to userspace as
> >> some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
> >> hardware composer HAL) that can be set by apps like the Android Gears
> >> app.
> >>
> >> Userspace can set the color_fill value by setting COLOR_FILL_FORMAT to a
> >> DRM format, setting COLOR_FILL to a color fill value, and setting the
> >> framebuffer to NULL.
> >
> > Is there some real reason for the format property? Ie. why not
> > just do what was the plan for the crttc background color and
> > specify the color in full 16bpc format and just pick as many
> > msbs from that as the hw can use?
> >
> >>
> >> Jessica Zhang (3):
> >>drm: Introduce color fill properties for drm plane
> >>drm: Adjust atomic checks for solid fill color
> >>drm/msm/dpu: Use color_fill property for DPU planes
>
Re: XDC 2023: Registration & Call for Proposals now open!
This is a reminder that the XDC 2023 Registration and the Call for Proposals are still open and will be open for about 2 more weeks. Do not forget to submit your proposals before the deadline (July 17)! The conference will take place in A Coruña this year, from October 17 to 19. You can find more information about the conference in the links below and you can also follow us on Mastodon for the latest updates. https://floss.social/@XOrgDevConf Thanks for your attention, -Ricardo On Mon, 2023-04-17 at 13:41 +0200, Samuel Iglesias Gonsálvez wrote: > > Hello! > > Registration & Call for Proposals are now open for XDC 2023, which will > take place on October 17-19, 2023. > > https://xdc2023.x.org > > As usual, the conference is free of charge and open to the general > public. If you plan on attending, please make sure to register as early > as possible! > > In order to register as attendee, you will therefore need to register > via the XDC website. > > https://indico.freedesktop.org/event/4/registrations/ > > In addition to registration, the CfP is now open for talks, workshops > and demos at XDC 2023. While any serious proposal will be gratefully > considered, topics of interest to X.Org and freedesktop.org developers > are encouraged. The program focus is on new development, ongoing > challenges and anything else that will spark discussions among > attendees in the hallway track. > > We are open to talks across all layers of the graphics stack, from the > kernel to desktop environments / graphical applications and about how > to make things better for the developers who build them. Head to the > CfP page to learn more: > > https://indico.freedesktop.org/event/4/abstracts/ > > The deadline for submissions is Monday, 17 July 2023 (23:59 CEST) > > Check out our Reimbursement Policy to accept speaker expenses: > > https://www.x.org/wiki/XorgFoundation/Policies/Reimbursement/ > > If you have any questions, please send me an email to > siglesias AT igalia.com, adding on Cc the X.org board (board > at foundation.x.org). > > And please keep in mind, you can follow us on Twitter for all the latest > updates and to stay connected: > > https://twitter.com/XOrgDevConf > > Best, > > Sam > >
xwayland: Clean up drm lease when terminating
Hi, I think a fix for https://gitlab.freedesktop.org/xorg/xserver/-/issues/946 is https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1130. Thanks, Russell
Re: [RFC PATCH 0/3] Support for Solid Fill Planes
On 6/27/2023 12:58 AM, Pekka Paalanen wrote:
On Mon, 26 Jun 2023 16:02:50 -0700
Jessica Zhang wrote:
On 11/7/2022 11:37 AM, Ville Syrjälä wrote:
On Fri, Oct 28, 2022 at 03:59:49PM -0700, Jessica Zhang wrote:
Introduce and add support for COLOR_FILL and COLOR_FILL_FORMAT
properties. When the color fill value is set, and the framebuffer is set
to NULL, memory fetch will be disabled.
Thinking a bit more universally I wonder if there should be
some kind of enum property:
enum plane_pixel_source {
FB,
COLOR,
LIVE_FOO,
LIVE_BAR,
...
}
Reviving this thread as this was the initial comment suggesting to
implement pixel_source as an enum.
I think the issue with having pixel_source as an enum is how to decide
what counts as a NULL commit.
Currently, setting the FB to NULL will disable the plane. So I'm
guessing we will extend that logic to "if there's no pixel_source set
for the plane, then it will be a NULL commit and disable the plane".
In that case, the question then becomes when to set the pixel_source to
NONE. Because if we do that when setting a NULL FB (or NULL solid_fill
blob), it then forces userspace to set one property before the other.
Right, that won't work.
There is no ordering between each property being set inside a single
atomic commit. They can all be applied to kernel-internal state
theoretically simultaneously, or any arbitrary random order, and the
end result must always be the same. Hence, setting one property cannot
change the state of another mutable property. I believe that doing
otherwise would make userspace fragile and hard to get right.
I guess there might be an exception to that rule when the same property
is set multiple times in a single atomic commit; the last setting in
the array prevails. That's universal and not a special-case between two
specific properties.
Because of that, I'm thinking of having pixel_source be represented by a
bitmask instead. That way, we will simply unset the corresponding
pixel_source bit when passing in a NULL FB/solid_fill blob. Then, in
order to detect whether a commit is NULL or has a valid pixel source, we
can just check if pixel_source == 0.
Sounds fine to me at first hand, but isn't there the enum property that
says if the kernel must look at solid_fill blob *or* FB_ID?
If enum prop says "use solid_fill prop", the why would changes to FB_ID
do anything? Is it for backwards-compatibility with KMS clients that do
not know about the enum prop?
It seems like that kind of backwards-compatiblity will cause problems
in trying to reason about the atomic state, as explained above, leading
to very delicate and fragile conditions where things work intuitively.
Hence, I'm not sure backwards-compatibility is wanted. This won't be
the first or the last KMS property where an unexpected value left over
will make old atomic KMS clients silently malfunction up to showing no
recognisable picture at all. *If* that problem needs solving, there
have been ideas floating around about resetting everything to nice
values so that userspace can ignore what it does not understand. So far
there has been no real interest in solving that problem in the kernel
though.
Legacy non-atomic UAPI wrappers can do whatever they want, and program
any (new) properties they want in order to implement the legacy
expectations, so that does not seem to be a problem.
Hi Pekka and Dmitry,
After reading through both of your comments, I think I have a better
understanding of the pixel_source implementation now.
So to summarize, we want to expose another property called
"pixel_source" to userspace that will default to FB (as to not break
legacy userspace).
If userspace wants to use solid fill planes, it will set both the
solid_fill *and* pixel_source properties to a valid blob and COLOR
respectively. If it wants to use FB, it will set FB_ID and pixel_source
to a valid FB and FB.
Here's a table illustrating what I've described above:
+-+-+-+
| Use Case| Legacy Userspace| solid_fill-aware|
| | | Userspace |
+=+=+=+
| Valid FB| pixel_source = FB | pixel_source = FB |
| | FB_ID = valid FB| FB_ID = valid FB|
+-+-+-+
| Valid | pixel_source = COLOR| N/A |
| solid_fill blob | solid_fill = valid blob | |
+-+-+-+
| NULL commit | pixel_source = FB | pixel_source = FB |
| | FB_ID = NULL| FB_ID = NULL|
+-+-+-+
Is there anything I'm missing or needs to be clarified?
Thanks,
Je
Re: [RFC PATCH 0/3] Support for Solid Fill Planes
On 28/06/2023 00:27, Jessica Zhang wrote:
On 6/27/2023 12:58 AM, Pekka Paalanen wrote:
On Mon, 26 Jun 2023 16:02:50 -0700
Jessica Zhang wrote:
On 11/7/2022 11:37 AM, Ville Syrjälä wrote:
On Fri, Oct 28, 2022 at 03:59:49PM -0700, Jessica Zhang wrote:
Introduce and add support for COLOR_FILL and COLOR_FILL_FORMAT
properties. When the color fill value is set, and the framebuffer
is set
to NULL, memory fetch will be disabled.
Thinking a bit more universally I wonder if there should be
some kind of enum property:
enum plane_pixel_source {
FB,
COLOR,
LIVE_FOO,
LIVE_BAR,
...
}
Reviving this thread as this was the initial comment suggesting to
implement pixel_source as an enum.
I think the issue with having pixel_source as an enum is how to decide
what counts as a NULL commit.
Currently, setting the FB to NULL will disable the plane. So I'm
guessing we will extend that logic to "if there's no pixel_source set
for the plane, then it will be a NULL commit and disable the plane".
In that case, the question then becomes when to set the pixel_source to
NONE. Because if we do that when setting a NULL FB (or NULL solid_fill
blob), it then forces userspace to set one property before the other.
Right, that won't work.
There is no ordering between each property being set inside a single
atomic commit. They can all be applied to kernel-internal state
theoretically simultaneously, or any arbitrary random order, and the
end result must always be the same. Hence, setting one property cannot
change the state of another mutable property. I believe that doing
otherwise would make userspace fragile and hard to get right.
I guess there might be an exception to that rule when the same property
is set multiple times in a single atomic commit; the last setting in
the array prevails. That's universal and not a special-case between two
specific properties.
Because of that, I'm thinking of having pixel_source be represented by a
bitmask instead. That way, we will simply unset the corresponding
pixel_source bit when passing in a NULL FB/solid_fill blob. Then, in
order to detect whether a commit is NULL or has a valid pixel source, we
can just check if pixel_source == 0.
Sounds fine to me at first hand, but isn't there the enum property that
says if the kernel must look at solid_fill blob *or* FB_ID?
If enum prop says "use solid_fill prop", the why would changes to FB_ID
do anything? Is it for backwards-compatibility with KMS clients that do
not know about the enum prop?
It seems like that kind of backwards-compatiblity will cause problems
in trying to reason about the atomic state, as explained above, leading
to very delicate and fragile conditions where things work intuitively.
Hence, I'm not sure backwards-compatibility is wanted. This won't be
the first or the last KMS property where an unexpected value left over
will make old atomic KMS clients silently malfunction up to showing no
recognisable picture at all. *If* that problem needs solving, there
have been ideas floating around about resetting everything to nice
values so that userspace can ignore what it does not understand. So far
there has been no real interest in solving that problem in the kernel
though.
Legacy non-atomic UAPI wrappers can do whatever they want, and program
any (new) properties they want in order to implement the legacy
expectations, so that does not seem to be a problem.
Hi Pekka and Dmitry,
After reading through both of your comments, I think I have a better
understanding of the pixel_source implementation now.
So to summarize, we want to expose another property called
"pixel_source" to userspace that will default to FB (as to not break
legacy userspace).
If userspace wants to use solid fill planes, it will set both the
solid_fill *and* pixel_source properties to a valid blob and COLOR
respectively. If it wants to use FB, it will set FB_ID and pixel_source
to a valid FB and FB.
Here's a table illustrating what I've described above:
+-+-+-+
| Use Case | Legacy Userspace | solid_fill-aware |
| | | Userspace |
+=+=+=+
| Valid FB | pixel_source = FB | pixel_source = FB |
| | FB_ID = valid FB | FB_ID = valid FB |
+-+-+-+
| Valid | pixel_source = COLOR | N/A |
| solid_fill blob | solid_fill = valid blob | |
Probably these two cells were swapped.
+-+-+-+
| NULL commit | pixel_source = FB | pixel_source = FB |
| | FB_ID = NULL | FB_ID = NULL |
+-+-+-+
Re: [RFC PATCH 0/3] Support for Solid Fill Planes
On 6/27/2023 2:59 PM, Dmitry Baryshkov wrote:
On 28/06/2023 00:27, Jessica Zhang wrote:
On 6/27/2023 12:58 AM, Pekka Paalanen wrote:
On Mon, 26 Jun 2023 16:02:50 -0700
Jessica Zhang wrote:
On 11/7/2022 11:37 AM, Ville Syrjälä wrote:
On Fri, Oct 28, 2022 at 03:59:49PM -0700, Jessica Zhang wrote:
Introduce and add support for COLOR_FILL and COLOR_FILL_FORMAT
properties. When the color fill value is set, and the framebuffer
is set
to NULL, memory fetch will be disabled.
Thinking a bit more universally I wonder if there should be
some kind of enum property:
enum plane_pixel_source {
FB,
COLOR,
LIVE_FOO,
LIVE_BAR,
...
}
Reviving this thread as this was the initial comment suggesting to
implement pixel_source as an enum.
I think the issue with having pixel_source as an enum is how to decide
what counts as a NULL commit.
Currently, setting the FB to NULL will disable the plane. So I'm
guessing we will extend that logic to "if there's no pixel_source set
for the plane, then it will be a NULL commit and disable the plane".
In that case, the question then becomes when to set the pixel_source to
NONE. Because if we do that when setting a NULL FB (or NULL solid_fill
blob), it then forces userspace to set one property before the other.
Right, that won't work.
There is no ordering between each property being set inside a single
atomic commit. They can all be applied to kernel-internal state
theoretically simultaneously, or any arbitrary random order, and the
end result must always be the same. Hence, setting one property cannot
change the state of another mutable property. I believe that doing
otherwise would make userspace fragile and hard to get right.
I guess there might be an exception to that rule when the same property
is set multiple times in a single atomic commit; the last setting in
the array prevails. That's universal and not a special-case between two
specific properties.
Because of that, I'm thinking of having pixel_source be represented
by a
bitmask instead. That way, we will simply unset the corresponding
pixel_source bit when passing in a NULL FB/solid_fill blob. Then, in
order to detect whether a commit is NULL or has a valid pixel
source, we
can just check if pixel_source == 0.
Sounds fine to me at first hand, but isn't there the enum property that
says if the kernel must look at solid_fill blob *or* FB_ID?
If enum prop says "use solid_fill prop", the why would changes to FB_ID
do anything? Is it for backwards-compatibility with KMS clients that do
not know about the enum prop?
It seems like that kind of backwards-compatiblity will cause problems
in trying to reason about the atomic state, as explained above, leading
to very delicate and fragile conditions where things work intuitively.
Hence, I'm not sure backwards-compatibility is wanted. This won't be
the first or the last KMS property where an unexpected value left over
will make old atomic KMS clients silently malfunction up to showing no
recognisable picture at all. *If* that problem needs solving, there
have been ideas floating around about resetting everything to nice
values so that userspace can ignore what it does not understand. So far
there has been no real interest in solving that problem in the kernel
though.
Legacy non-atomic UAPI wrappers can do whatever they want, and program
any (new) properties they want in order to implement the legacy
expectations, so that does not seem to be a problem.
Hi Pekka and Dmitry,
After reading through both of your comments, I think I have a better
understanding of the pixel_source implementation now.
So to summarize, we want to expose another property called
"pixel_source" to userspace that will default to FB (as to not break
legacy userspace).
If userspace wants to use solid fill planes, it will set both the
solid_fill *and* pixel_source properties to a valid blob and COLOR
respectively. If it wants to use FB, it will set FB_ID and
pixel_source to a valid FB and FB.
Here's a table illustrating what I've described above:
+-+-+-+
| Use Case | Legacy Userspace | solid_fill-aware |
| | | Userspace |
+=+=+=+
| Valid FB | pixel_source = FB | pixel_source = FB |
| | FB_ID = valid FB | FB_ID = valid FB |
+-+-+-+
| Valid | pixel_source = COLOR | N/A |
| solid_fill blob | solid_fill = valid blob | |
Probably these two cells were swapped.
Ack, yes they were swapped.
+-+-+-+
| NULL commit | pixel_source = FB | pixel_source = FB |
| | FB_ID = NULL | FB_ID = NULL
