Re: [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property

2023-08-18 Thread Pekka Paalanen
On Fri, 4 Aug 2023 16:59:00 +0300
Dmitry Baryshkov  wrote:

> On Fri, 4 Aug 2023 at 16:44, Sebastian Wick  wrote:
> >
> > On Fri, Aug 4, 2023 at 3:27 PM Dmitry Baryshkov
> >  wrote:  
> > >
> > > On Fri, 28 Jul 2023 at 20:03, Jessica Zhang  
> > > wrote:  
> > > >
> > > > Document and add support for solid_fill property to drm_plane. In
> > > > addition, add support for setting and getting the values for solid_fill.
> > > >
> > > > To enable solid fill planes, userspace must assign a property blob to
> > > > the "solid_fill" plane property containing the following information:
> > > >
> > > > struct drm_mode_solid_fill {
> > > > u32 version;
> > > > u32 r, g, b;
> > > > };
> > > >
> > > > Signed-off-by: Jessica Zhang 
> > > > ---
> > > >  drivers/gpu/drm/drm_atomic_state_helper.c |  9 +
> > > >  drivers/gpu/drm/drm_atomic_uapi.c | 55 
> > > > +++
> > > >  drivers/gpu/drm/drm_blend.c   | 30 +
> > > >  include/drm/drm_blend.h   |  1 +
> > > >  include/drm/drm_plane.h   | 35 
> > > >  include/uapi/drm/drm_mode.h   | 24 ++
> > > >  6 files changed, 154 insertions(+)
> > > >  
> > >
> > > [skipped most of the patch]
> > >  
> > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > > index 43691058d28f..53c8efa5ad7f 100644
> > > > --- a/include/uapi/drm/drm_mode.h
> > > > +++ b/include/uapi/drm/drm_mode.h
> > > > @@ -259,6 +259,30 @@ struct drm_mode_modeinfo {
> > > > char name[DRM_DISPLAY_MODE_LEN];
> > > >  };
> > > >
> > > > +/**
> > > > + * struct drm_mode_solid_fill - User info for solid fill planes
> > > > + *
> > > > + * This is the userspace API solid fill information structure.
> > > > + *
> > > > + * Userspace can enable solid fill planes by assigning the plane 
> > > > "solid_fill"
> > > > + * property to a blob containing a single drm_mode_solid_fill struct 
> > > > populated with an RGB323232
> > > > + * color and setting the pixel source to "SOLID_FILL".
> > > > + *
> > > > + * For information on the plane property, see 
> > > > drm_plane_create_solid_fill_property()
> > > > + *
> > > > + * @version: Version of the blob. Currently, there is only support for 
> > > > version == 1
> > > > + * @r: Red color value of single pixel
> > > > + * @g: Green color value of single pixel
> > > > + * @b: Blue color value of single pixel
> > > > + */
> > > > +struct drm_mode_solid_fill {
> > > > +   __u32 version;
> > > > +   __u32 r;
> > > > +   __u32 g;
> > > > +   __u32 b;  
> > >
> > > Another thought about the drm_mode_solid_fill uABI. I still think we
> > > should add alpha here. The reason is the following:
> > >
> > > It is true that we have  drm_plane_state::alpha and the plane's
> > > "alpha" property. However it is documented as "the plane-wide opacity
> > > [...] It can be combined with pixel alpha. The pixel values in the
> > > framebuffers are expected to not be pre-multiplied by the global alpha
> > > associated to the plane.".
> > >
> > > I can imagine a use case, when a user might want to enable plane-wide
> > > opacity, set "pixel blend mode" to "Coverage" and then switch between
> > > partially opaque framebuffer and partially opaque solid-fill without
> > > touching the plane's alpha value.  
> >
> > The only reason I see against this is that there might be some
> > hardware which supports only RGB but not alpha on planes and they
> > could then not use this property.  
> 
> Fair enough.
> 
> > Maybe another COLOR_FILL enum value
> > with alpha might be better? Maybe just doing the alpha via the alpha
> > property is good enough.  
> 
> One of our customers has a use case for setting the opaque solid fill,
> while keeping the plane's alpha intact.

Could you explain more about why they must keep plane alpha intact
instead of reprogramming everything with atomic? Is there some
combination that just cannot reach the same end result via userspace
manipulation of the solid fill values with plane alpha?

Or is it a matter of userspace architecture where you have independent
components responsible for different KMS property values?


Thanks,
pq


pgpgMZKMHODko.pgp
Description: OpenPGP digital signature


Re: [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property

2023-08-18 Thread Dmitry Baryshkov

On 18/08/2023 13:51, Pekka Paalanen wrote:

On Fri, 4 Aug 2023 16:59:00 +0300
Dmitry Baryshkov  wrote:


On Fri, 4 Aug 2023 at 16:44, Sebastian Wick  wrote:


On Fri, Aug 4, 2023 at 3:27 PM Dmitry Baryshkov
 wrote:


On Fri, 28 Jul 2023 at 20:03, Jessica Zhang  wrote:


Document and add support for solid_fill property to drm_plane. In
addition, add support for setting and getting the values for solid_fill.

To enable solid fill planes, userspace must assign a property blob to
the "solid_fill" plane property containing the following information:

struct drm_mode_solid_fill {
 u32 version;
 u32 r, g, b;
};

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic_state_helper.c |  9 +
  drivers/gpu/drm/drm_atomic_uapi.c | 55 +++
  drivers/gpu/drm/drm_blend.c   | 30 +
  include/drm/drm_blend.h   |  1 +
  include/drm/drm_plane.h   | 35 
  include/uapi/drm/drm_mode.h   | 24 ++
  6 files changed, 154 insertions(+)
  


[skipped most of the patch]
  

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 43691058d28f..53c8efa5ad7f 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -259,6 +259,30 @@ struct drm_mode_modeinfo {
 char name[DRM_DISPLAY_MODE_LEN];
  };

+/**
+ * struct drm_mode_solid_fill - User info for solid fill planes
+ *
+ * This is the userspace API solid fill information structure.
+ *
+ * Userspace can enable solid fill planes by assigning the plane "solid_fill"
+ * property to a blob containing a single drm_mode_solid_fill struct populated 
with an RGB323232
+ * color and setting the pixel source to "SOLID_FILL".
+ *
+ * For information on the plane property, see 
drm_plane_create_solid_fill_property()
+ *
+ * @version: Version of the blob. Currently, there is only support for version 
== 1
+ * @r: Red color value of single pixel
+ * @g: Green color value of single pixel
+ * @b: Blue color value of single pixel
+ */
+struct drm_mode_solid_fill {
+   __u32 version;
+   __u32 r;
+   __u32 g;
+   __u32 b;


Another thought about the drm_mode_solid_fill uABI. I still think we
should add alpha here. The reason is the following:

It is true that we have  drm_plane_state::alpha and the plane's
"alpha" property. However it is documented as "the plane-wide opacity
[...] It can be combined with pixel alpha. The pixel values in the
framebuffers are expected to not be pre-multiplied by the global alpha
associated to the plane.".

I can imagine a use case, when a user might want to enable plane-wide
opacity, set "pixel blend mode" to "Coverage" and then switch between
partially opaque framebuffer and partially opaque solid-fill without
touching the plane's alpha value.


The only reason I see against this is that there might be some
hardware which supports only RGB but not alpha on planes and they
could then not use this property.


Fair enough.


Maybe another COLOR_FILL enum value
with alpha might be better? Maybe just doing the alpha via the alpha
property is good enough.


One of our customers has a use case for setting the opaque solid fill,
while keeping the plane's alpha intact.


Could you explain more about why they must keep plane alpha intact
instead of reprogramming everything with atomic? Is there some
combination that just cannot reach the same end result via userspace
manipulation of the solid fill values with plane alpha?

Or is it a matter of userspace architecture where you have independent
components responsible for different KMS property values?
The latter one. The goal is to be able to switch between pixel sources 
without touching any additional properties (including plane's alpha value).


--
With best wishes
Dmitry



Re: Wayland governance meeting - 22-25 August

2023-08-18 Thread Xaver Hugl
Hi,

Victoria wanted to take part in this meeting, but she's on vacation during
that week. Would you mind having the meeting the week afterwards instead?

Best regards,
Xaver

Am Do., 17. Aug. 2023 um 21:44 Uhr schrieb Simon Zeni :

> Hello there!
>
> Apologies again for the duplicated governance meeting last week. Here's the
> doodle [1] for next week's meeting about the direct input access protocol
> [2].
> The poll will close and the date for the meeting will be announced on
> monday
> the 21st.
>
> The meeting will be held on KDE's platform here [3]. Everybody is free to
> join
> to listen if they want to, anonymously or not.
>
> The notes for this meeting, along with the ones from the previous meetings
> can
> be found on the wayland-protocols wiki page [4].
>
> Cheers,
>
> Simon
>
> [1]: https://nuudel.digitalcourage.de/FM7RK4ZeZGoEIJXf
> [2]:
> https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/110
> [3]: https://meet.kde.org/b/dav-v5j-p0q-6iz
> [4]:
> https://gitlab.freedesktop.org/wayland/wayland-protocols/-/wikis/meetings
>


Re: [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property

2023-08-18 Thread Pekka Paalanen
On Fri, 18 Aug 2023 14:03:14 +0300
Dmitry Baryshkov  wrote:

> On 18/08/2023 13:51, Pekka Paalanen wrote:
> > On Fri, 4 Aug 2023 16:59:00 +0300
> > Dmitry Baryshkov  wrote:
> >   
> >> On Fri, 4 Aug 2023 at 16:44, Sebastian Wick  
> >> wrote:  
> >>>
> >>> On Fri, Aug 4, 2023 at 3:27 PM Dmitry Baryshkov
> >>>  wrote:  
> 
>  On Fri, 28 Jul 2023 at 20:03, Jessica Zhang  
>  wrote:  
> >
> > Document and add support for solid_fill property to drm_plane. In
> > addition, add support for setting and getting the values for solid_fill.
> >
> > To enable solid fill planes, userspace must assign a property blob to
> > the "solid_fill" plane property containing the following information:
> >
> > struct drm_mode_solid_fill {
> >  u32 version;
> >  u32 r, g, b;
> > };
> >
> > Signed-off-by: Jessica Zhang 
> > ---
> >   drivers/gpu/drm/drm_atomic_state_helper.c |  9 +
> >   drivers/gpu/drm/drm_atomic_uapi.c | 55 
> > +++
> >   drivers/gpu/drm/drm_blend.c   | 30 +
> >   include/drm/drm_blend.h   |  1 +
> >   include/drm/drm_plane.h   | 35 
> >   include/uapi/drm/drm_mode.h   | 24 ++
> >   6 files changed, 154 insertions(+)
> > 
> 
>  [skipped most of the patch]

...

> >>> Maybe another COLOR_FILL enum value
> >>> with alpha might be better? Maybe just doing the alpha via the alpha
> >>> property is good enough.  
> >>
> >> One of our customers has a use case for setting the opaque solid fill,
> >> while keeping the plane's alpha intact.  
> > 
> > Could you explain more about why they must keep plane alpha intact
> > instead of reprogramming everything with atomic? Is there some
> > combination that just cannot reach the same end result via userspace
> > manipulation of the solid fill values with plane alpha?
> > 
> > Or is it a matter of userspace architecture where you have independent
> > components responsible for different KMS property values?  

> The latter one. The goal is to be able to switch between pixel sources 
> without touching any additional properties (including plane's alpha value).

Sorry, but that does not seem like a good justification for KMS UAPI
design.

It is even in conflict with how atomic KMS UAPI was designed to work:
collect all your changes into a single commit, and push it at once.
Here we are talking about separate components changing the different
properties of the same KMS plane even. If you want to change both plane
opacity and contents, does it mean you need two refresh cycles, one at
a time? Could the two components be even racing with each other,
stalling each other randomly?


Thanks,
pq


pgpo8hTx6xtJw.pgp
Description: OpenPGP digital signature