Re: [Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property

2023-10-19 Thread Simon Ser
For the uAPI:

Acked-by: Simon Ser 


Re: [Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property

2023-10-19 Thread Sebastian Wick
On Tue, Aug 29, 2023 at 10:48:16AM +0300, Pekka Paalanen wrote:
> On Mon, 28 Aug 2023 17:05:07 -0700
> Jessica Zhang  wrote:
> 
> > Add support for pixel_source property to drm_plane and related
> > documentation. In addition, force pixel_source to
> > DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break
> > legacy userspace.
> > 
> > This enum property will allow user to specify a pixel source for the
> > plane. Possible pixel sources will be defined in the
> > drm_plane_pixel_source enum.
> > 
> > Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the
> > default value) and DRM_PLANE_PIXEL_SOURCE_NONE.
> > 
> > Signed-off-by: Jessica Zhang 
> > ---
> >  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
> >  drivers/gpu/drm/drm_blend.c   | 90 
> > +++
> >  drivers/gpu/drm/drm_plane.c   | 19 +--
> >  include/drm/drm_blend.h   |  2 +
> >  include/drm/drm_plane.h   | 21 
> >  6 files changed, 133 insertions(+), 4 deletions(-)
> 
> ...
> 
> > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> > index 6e74de833466..c3c57bae06b7 100644
> > --- a/drivers/gpu/drm/drm_blend.c
> > +++ b/drivers/gpu/drm/drm_blend.c
> > @@ -185,6 +185,21 @@
> >   *  plane does not expose the "alpha" property, then this is
> >   *  assumed to be 1.0
> >   *
> > + * pixel_source:
> > + * pixel_source is set up with drm_plane_create_pixel_source_property().
> > + * It is used to toggle the active source of pixel data for the plane.
> > + * The plane will only display data from the set pixel_source -- any
> > + * data from other sources will be ignored.
> > + *
> > + * Possible values:
> > + *
> > + * "NONE":
> > + * No active pixel source.
> > + * Committing with a NONE pixel source will disable the plane.
> > + *
> > + * "FB":
> > + * Framebuffer source set by the "FB_ID" property.
> > + *
> >   * Note that all the property extensions described here apply either to the
> >   * plane or the CRTC (e.g. for the background color, which currently is not
> >   * exposed and assumed to be black).
> 
> This UAPI:
> Acked-by: Pekka Paalanen 

Thanks Jessica, same for me

Acked-by: Sebastian Wick 

> 
> 
> Thanks,
> pq




Re: [Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property

2023-10-18 Thread Jessica Zhang




On 9/24/2023 3:06 AM, Dmitry Baryshkov wrote:

On 29/08/2023 03:05, Jessica Zhang wrote:

Add support for pixel_source property to drm_plane and related
documentation. In addition, force pixel_source to
DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break
legacy userspace.

This enum property will allow user to specify a pixel source for the
plane. Possible pixel sources will be defined in the
drm_plane_pixel_source enum.

Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the
default value) and DRM_PLANE_PIXEL_SOURCE_NONE.

Signed-off-by: Jessica Zhang 


Acked-by: Dmitry Baryshkov 

Minor question below


---
  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
  drivers/gpu/drm/drm_blend.c   | 90 
+++

  drivers/gpu/drm/drm_plane.c   | 19 +--
  include/drm/drm_blend.h   |  2 +
  include/drm/drm_plane.h   | 21 
  6 files changed, 133 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c

index 784e63d70a42..01638c51ce0a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct 
drm_plane_state *plane_state,

  plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
  plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+    plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
  if (plane->color_encoding_property) {
  if (!drm_object_property_get_default_value(&plane->base,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c

index d867e7f9f2cd..454f980e16c9 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -544,6 +544,8 @@ static int drm_atomic_plane_set_property(struct 
drm_plane *plane,

  state->src_w = val;
  } else if (property == config->prop_src_h) {
  state->src_h = val;
+    } else if (property == plane->pixel_source_property) {
+    state->pixel_source = val;
  } else if (property == plane->alpha_property) {
  state->alpha = val;
  } else if (property == plane->blend_mode_property) {
@@ -616,6 +618,8 @@ drm_atomic_plane_get_property(struct drm_plane 
*plane,

  *val = state->src_w;
  } else if (property == config->prop_src_h) {
  *val = state->src_h;
+    } else if (property == plane->pixel_source_property) {
+    *val = state->pixel_source;
  } else if (property == plane->alpha_property) {
  *val = state->alpha;
  } else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 6e74de833466..c3c57bae06b7 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -185,6 +185,21 @@
   * plane does not expose the "alpha" property, then this is
   * assumed to be 1.0
   *
+ * pixel_source:
+ *    pixel_source is set up with 
drm_plane_create_pixel_source_property().
+ *    It is used to toggle the active source of pixel data for the 
plane.

+ *    The plane will only display data from the set pixel_source -- any
+ *    data from other sources will be ignored.
+ *
+ *    Possible values:
+ *
+ *    "NONE":
+ *    No active pixel source.
+ *    Committing with a NONE pixel source will disable the plane.
+ *
+ *    "FB":
+ *    Framebuffer source set by the "FB_ID" property.
+ *
   * Note that all the property extensions described here apply either 
to the
   * plane or the CRTC (e.g. for the background color, which currently 
is not

   * exposed and assumed to be black).
@@ -615,3 +630,78 @@ int drm_plane_create_blend_mode_property(struct 
drm_plane *plane,

  return 0;
  }
  EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+static const struct drm_prop_enum_list drm_pixel_source_enum_list[] = {
+    { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
+    { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
+};
+
+/**
+ * drm_plane_create_pixel_source_property - create a new pixel source 
property

+ * @plane: DRM plane
+ * @extra_sources: Bitmask of additional supported pixel_sources for 
the driver.
+ *   DRM_PLANE_PIXEL_SOURCE_FB and 
DRM_PLANE_PIXEL_SOURCE_NONE will

+ *   always be enabled as supported sources.
+ *
+ * This creates a new property describing the current source of pixel 
data for the
+ * plane. The pixel_source will be initialized as 
DRM_PLANE_PIXEL_SOURCE_FB by default.

+ *
+ * Drivers can set a custom default source by overriding the 
pixel_source value in

+ * drm_plane_funcs.reset()
+ *
+ * The property is exposed to userspace as an enumeration property 
called

+ * "pixel_source" and has the following enumeration values:
+ *
+ * "NONE":
+ * No active pixel source
+ *
+ * "FB":
+ *    Framebuffer pixel source
+ *
+ * Returns:
+ *

Re: [Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property

2023-09-24 Thread Dmitry Baryshkov

On 29/08/2023 03:05, Jessica Zhang wrote:

Add support for pixel_source property to drm_plane and related
documentation. In addition, force pixel_source to
DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break
legacy userspace.

This enum property will allow user to specify a pixel source for the
plane. Possible pixel sources will be defined in the
drm_plane_pixel_source enum.

Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the
default value) and DRM_PLANE_PIXEL_SOURCE_NONE.

Signed-off-by: Jessica Zhang 


Acked-by: Dmitry Baryshkov 

Minor question below


---
  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
  drivers/gpu/drm/drm_blend.c   | 90 +++
  drivers/gpu/drm/drm_plane.c   | 19 +--
  include/drm/drm_blend.h   |  2 +
  include/drm/drm_plane.h   | 21 
  6 files changed, 133 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..01638c51ce0a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct 
drm_plane_state *plane_state,
  
  	plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;

plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+   plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
  
  	if (plane->color_encoding_property) {

if (!drm_object_property_get_default_value(&plane->base,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index d867e7f9f2cd..454f980e16c9 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -544,6 +544,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
+   } else if (property == plane->pixel_source_property) {
+   state->pixel_source = val;
} else if (property == plane->alpha_property) {
state->alpha = val;
} else if (property == plane->blend_mode_property) {
@@ -616,6 +618,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
+   } else if (property == plane->pixel_source_property) {
+   *val = state->pixel_source;
} else if (property == plane->alpha_property) {
*val = state->alpha;
} else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 6e74de833466..c3c57bae06b7 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -185,6 +185,21 @@
   * plane does not expose the "alpha" property, then this is
   * assumed to be 1.0
   *
+ * pixel_source:
+ * pixel_source is set up with drm_plane_create_pixel_source_property().
+ * It is used to toggle the active source of pixel data for the plane.
+ * The plane will only display data from the set pixel_source -- any
+ * data from other sources will be ignored.
+ *
+ * Possible values:
+ *
+ * "NONE":
+ * No active pixel source.
+ * Committing with a NONE pixel source will disable the plane.
+ *
+ * "FB":
+ * Framebuffer source set by the "FB_ID" property.
+ *
   * Note that all the property extensions described here apply either to the
   * plane or the CRTC (e.g. for the background color, which currently is not
   * exposed and assumed to be black).
@@ -615,3 +630,78 @@ int drm_plane_create_blend_mode_property(struct drm_plane 
*plane,
return 0;
  }
  EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+static const struct drm_prop_enum_list drm_pixel_source_enum_list[] = {
+   { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
+   { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
+};
+
+/**
+ * drm_plane_create_pixel_source_property - create a new pixel source property
+ * @plane: DRM plane
+ * @extra_sources: Bitmask of additional supported pixel_sources for the 
driver.
+ *DRM_PLANE_PIXEL_SOURCE_FB and DRM_PLANE_PIXEL_SOURCE_NONE 
will
+ *always be enabled as supported sources.
+ *
+ * This creates a new property describing the current source of pixel data for 
the
+ * plane. The pixel_source will be initialized as DRM_PLANE_PIXEL_SOURCE_FB by 
default.
+ *
+ * Drivers can set a custom default source by overriding the pixel_source 
value in
+ * drm_plane_funcs.reset()
+ *
+ * The property is exposed to userspace as an enumeration property called
+ * "pixel_source" and has the following enumeration values:
+ *
+ * "NONE":
+ *  No active pixel source
+ *
+ * 

Re: [Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property

2023-08-29 Thread Pekka Paalanen
On Mon, 28 Aug 2023 17:05:07 -0700
Jessica Zhang  wrote:

> Add support for pixel_source property to drm_plane and related
> documentation. In addition, force pixel_source to
> DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break
> legacy userspace.
> 
> This enum property will allow user to specify a pixel source for the
> plane. Possible pixel sources will be defined in the
> drm_plane_pixel_source enum.
> 
> Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the
> default value) and DRM_PLANE_PIXEL_SOURCE_NONE.
> 
> Signed-off-by: Jessica Zhang 
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
>  drivers/gpu/drm/drm_blend.c   | 90 
> +++
>  drivers/gpu/drm/drm_plane.c   | 19 +--
>  include/drm/drm_blend.h   |  2 +
>  include/drm/drm_plane.h   | 21 
>  6 files changed, 133 insertions(+), 4 deletions(-)

...

> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 6e74de833466..c3c57bae06b7 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -185,6 +185,21 @@
>   *plane does not expose the "alpha" property, then this is
>   *assumed to be 1.0
>   *
> + * pixel_source:
> + *   pixel_source is set up with drm_plane_create_pixel_source_property().
> + *   It is used to toggle the active source of pixel data for the plane.
> + *   The plane will only display data from the set pixel_source -- any
> + *   data from other sources will be ignored.
> + *
> + *   Possible values:
> + *
> + *   "NONE":
> + *   No active pixel source.
> + *   Committing with a NONE pixel source will disable the plane.
> + *
> + *   "FB":
> + *   Framebuffer source set by the "FB_ID" property.
> + *
>   * Note that all the property extensions described here apply either to the
>   * plane or the CRTC (e.g. for the background color, which currently is not
>   * exposed and assumed to be black).

This UAPI:
Acked-by: Pekka Paalanen 


Thanks,
pq


pgpIXA2uzDNp7.pgp
Description: OpenPGP digital signature


[Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property

2023-08-28 Thread Jessica Zhang
Add support for pixel_source property to drm_plane and related
documentation. In addition, force pixel_source to
DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break
legacy userspace.

This enum property will allow user to specify a pixel source for the
plane. Possible pixel sources will be defined in the
drm_plane_pixel_source enum.

Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the
default value) and DRM_PLANE_PIXEL_SOURCE_NONE.

Signed-off-by: Jessica Zhang 
---
 drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
 drivers/gpu/drm/drm_blend.c   | 90 +++
 drivers/gpu/drm/drm_plane.c   | 19 +--
 include/drm/drm_blend.h   |  2 +
 include/drm/drm_plane.h   | 21 
 6 files changed, 133 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..01638c51ce0a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct 
drm_plane_state *plane_state,
 
plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+   plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
 
if (plane->color_encoding_property) {
if (!drm_object_property_get_default_value(&plane->base,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index d867e7f9f2cd..454f980e16c9 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -544,6 +544,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
+   } else if (property == plane->pixel_source_property) {
+   state->pixel_source = val;
} else if (property == plane->alpha_property) {
state->alpha = val;
} else if (property == plane->blend_mode_property) {
@@ -616,6 +618,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
+   } else if (property == plane->pixel_source_property) {
+   *val = state->pixel_source;
} else if (property == plane->alpha_property) {
*val = state->alpha;
} else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 6e74de833466..c3c57bae06b7 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -185,6 +185,21 @@
  *  plane does not expose the "alpha" property, then this is
  *  assumed to be 1.0
  *
+ * pixel_source:
+ * pixel_source is set up with drm_plane_create_pixel_source_property().
+ * It is used to toggle the active source of pixel data for the plane.
+ * The plane will only display data from the set pixel_source -- any
+ * data from other sources will be ignored.
+ *
+ * Possible values:
+ *
+ * "NONE":
+ * No active pixel source.
+ * Committing with a NONE pixel source will disable the plane.
+ *
+ * "FB":
+ * Framebuffer source set by the "FB_ID" property.
+ *
  * Note that all the property extensions described here apply either to the
  * plane or the CRTC (e.g. for the background color, which currently is not
  * exposed and assumed to be black).
@@ -615,3 +630,78 @@ int drm_plane_create_blend_mode_property(struct drm_plane 
*plane,
return 0;
 }
 EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+static const struct drm_prop_enum_list drm_pixel_source_enum_list[] = {
+   { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
+   { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
+};
+
+/**
+ * drm_plane_create_pixel_source_property - create a new pixel source property
+ * @plane: DRM plane
+ * @extra_sources: Bitmask of additional supported pixel_sources for the 
driver.
+ *DRM_PLANE_PIXEL_SOURCE_FB and DRM_PLANE_PIXEL_SOURCE_NONE 
will
+ *always be enabled as supported sources.
+ *
+ * This creates a new property describing the current source of pixel data for 
the
+ * plane. The pixel_source will be initialized as DRM_PLANE_PIXEL_SOURCE_FB by 
default.
+ *
+ * Drivers can set a custom default source by overriding the pixel_source 
value in
+ * drm_plane_funcs.reset()
+ *
+ * The property is exposed to userspace as an enumeration property called
+ * "pixel_source" and has the following enumeration values:
+ *
+ * "NONE":
+ *  No active pixel source
+ *
+ * "FB":
+ * Framebuffer pixel source
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+