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 <[email protected]>
---
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(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 01638c51ce0a..86fb876efbe6 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -254,6 +254,11 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
+ if (plane_state->solid_fill_blob) {
+ drm_property_blob_put(plane_state->solid_fill_blob);
+ plane_state->solid_fill_blob = NULL;
+ }
+
if (plane->color_encoding_property) {
if (!drm_object_property_get_default_value(&plane->base,
plane->color_encoding_property,
@@ -336,6 +341,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
if (state->fb)
drm_framebuffer_get(state->fb);
+ if (state->solid_fill_blob)
+ drm_property_blob_get(state->solid_fill_blob);
+
state->fence = NULL;
state->commit = NULL;
state->fb_damage_clips = NULL;
@@ -385,6 +393,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
drm_crtc_commit_put(state->commit);
drm_property_blob_put(state->fb_damage_clips);
+ drm_property_blob_put(state->solid_fill_blob);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 454f980e16c9..039686c78c2a 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -316,6 +316,51 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
}
EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
+static int drm_atomic_set_solid_fill_prop(struct drm_plane_state *state,
+ struct drm_property_blob *blob)
+{
+ int blob_version;
+
+ if (blob == state->solid_fill_blob)
+ return 0;
+
+ if (blob) {
+ struct drm_mode_solid_fill *user_info = (struct drm_mode_solid_fill *)blob->data;
+
+ if (blob->length != sizeof(struct drm_mode_solid_fill)) {
+ drm_dbg_atomic(state->plane->dev,
+ "[PLANE:%d:%s] bad solid fill blob length: %zu\n",
+ state->plane->base.id, state->plane->name,
+ blob->length);
+ return -EINVAL;
+ }
+
+ blob_version = user_info->version;
+
+ /* Add more versions if necessary */
+ if (blob_version == 1) {
+ state->solid_fill.r = user_info->r;
+ state->solid_fill.g = user_info->g;
+ state->solid_fill.b = user_info->b;
+ } else {
+ drm_dbg_atomic(state->plane->dev,
+ "[PLANE:%d:%s] unsupported solid fill version (version=%d)\n",
+ state->plane->base.id, state->plane->name,
+ blob_version);
+ return -EINVAL;
+ }
+ }
+
+ drm_property_blob_put(state->solid_fill_blob);
+
+ if (blob)
+ state->solid_fill_blob = drm_property_blob_get(blob);
+ else
+ state->solid_fill_blob = NULL;
+
+ return 0;
+}
+
static void set_out_fence_for_crtc(struct drm_atomic_state *state,
struct drm_crtc *crtc, s32 __user *fence_ptr)
{
@@ -546,6 +591,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
state->src_h = val;
} else if (property == plane->pixel_source_property) {
state->pixel_source = val;
+ } else if (property == plane->solid_fill_property) {
+ struct drm_property_blob *solid_fill = drm_property_lookup_blob(dev, val);
+
+ ret = drm_atomic_set_solid_fill_prop(state, solid_fill);
+ drm_property_blob_put(solid_fill);
+
+ return ret;
} else if (property == plane->alpha_property) {
state->alpha = val;
} else if (property == plane->blend_mode_property) {
@@ -620,6 +672,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_h;
} else if (property == plane->pixel_source_property) {
*val = state->pixel_source;
+ } else if (property == plane->solid_fill_property) {
+ *val = state->solid_fill_blob ?
+ state->solid_fill_blob->base.id : 0;
} 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 c500310a3d09..c632dfcd8a9b 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -200,6 +200,10 @@
* "FB":
* Framebuffer source set by the "FB_ID" property.
*
+ * solid_fill:
+ * solid_fill is set up with drm_plane_create_solid_fill_property(). It
+ * contains pixel data that drivers can use to fill a plane.
+ *
* 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).
@@ -700,3 +704,29 @@ int drm_plane_create_pixel_source_property(struct drm_plane *plane,
return 0;
}
EXPORT_SYMBOL(drm_plane_create_pixel_source_property);
+
+/**
+ * drm_plane_create_solid_fill_property - create a new solid_fill property
+ * @plane: drm plane
+ *
+ * This creates a new property blob that holds pixel data for solid fill planes.
+ * The property is exposed to userspace as a property blob called "solid_fill".
+ *
+ * For information on what the blob contains, see `drm_mode_solid_fill`.
+ */
+int drm_plane_create_solid_fill_property(struct drm_plane *plane)
+{
+ struct drm_property *prop;
+
+ prop = drm_property_create(plane->dev,
+ DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
+ "solid_fill", 0);
+ if (!prop)
+ return -ENOMEM;
+
+ drm_object_attach_property(&plane->base, prop, 0);
+ plane->solid_fill_property = prop;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 122bbfbaae33..e7158fbee389 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -60,4 +60,5 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
unsigned int supported_modes);
int drm_plane_create_pixel_source_property(struct drm_plane *plane,
unsigned long extra_sources);
+int drm_plane_create_solid_fill_property(struct drm_plane *plane);
#endif
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 89508b4dea4a..abf1458fa099 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -46,6 +46,17 @@ enum drm_plane_pixel_source {
DRM_PLANE_PIXEL_SOURCE_MAX
};
+/**
+ * struct solid_fill_property - RGB values for solid fill plane
+ *
+ * Note: This is the V1 for this feature
+ */
+struct drm_solid_fill {
+ uint32_t r;
+ uint32_t g;
+ uint32_t b;
+};
+
/**
* struct drm_plane_state - mutable plane state
*
@@ -130,6 +141,23 @@ struct drm_plane_state {
*/
enum drm_plane_pixel_source pixel_source;
+ /**
+ * @solid_fill_blob:
+ *
+ * Blob containing relevant information for a solid fill plane
+ * including pixel format and data. See
+ * drm_plane_create_solid_fill_property() for more details.
+ */
+ struct drm_property_blob *solid_fill_blob;
+
+ /**
+ * @solid_fill:
+ *
+ * Pixel data for solid fill planes. See
+ * drm_plane_create_solid_fill_property() for more details.
+ */
+ struct drm_solid_fill solid_fill;
+
/**
* @alpha:
* Opacity of the plane with 0 as completely transparent and 0xffff as
@@ -720,6 +748,13 @@ struct drm_plane {
*/
struct drm_property *pixel_source_property;
+ /**
+ * @solid_fill_property:
+ * Optional solid_fill property for this plane. See
+ * drm_plane_create_solid_fill_property().
+ */
+ struct drm_property *solid_fill_property;
+
/**
* @alpha_property:
* Optional alpha property for this plane. See
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;
+};
+
+
struct drm_mode_card_res {
__u64 fb_id_ptr;
__u64 crtc_id_ptr;
--
2.41.0
On 28/07/2023 20:02, 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 <[email protected]>
> ---
> 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(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 01638c51ce0a..86fb876efbe6 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -254,6 +254,11 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
> plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
>
> + if (plane_state->solid_fill_blob) {
> + drm_property_blob_put(plane_state->solid_fill_blob);
> + plane_state->solid_fill_blob = NULL;
> + }
> +
> if (plane->color_encoding_property) {
> if (!drm_object_property_get_default_value(&plane->base,
> plane->color_encoding_property,
> @@ -336,6 +341,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
> if (state->fb)
> drm_framebuffer_get(state->fb);
>
> + if (state->solid_fill_blob)
> + drm_property_blob_get(state->solid_fill_blob);
> +
> state->fence = NULL;
> state->commit = NULL;
> state->fb_damage_clips = NULL;
> @@ -385,6 +393,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
> drm_crtc_commit_put(state->commit);
>
> drm_property_blob_put(state->fb_damage_clips);
> + drm_property_blob_put(state->solid_fill_blob);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 454f980e16c9..039686c78c2a 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -316,6 +316,51 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> }
> EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
>
> +static int drm_atomic_set_solid_fill_prop(struct drm_plane_state *state,
> + struct drm_property_blob *blob)
> +{
> + int blob_version;
> +
> + if (blob == state->solid_fill_blob)
> + return 0;
> +
> + if (blob) {
> + struct drm_mode_solid_fill *user_info = (struct drm_mode_solid_fill *)blob->data;
> +
> + if (blob->length != sizeof(struct drm_mode_solid_fill)) {
> + drm_dbg_atomic(state->plane->dev,
> + "[PLANE:%d:%s] bad solid fill blob length: %zu\n",
> + state->plane->base.id, state->plane->name,
> + blob->length);
> + return -EINVAL;
> + }
> +
> + blob_version = user_info->version;
I remember that we had versioning for quite some time. However it stroke
me while reviewing that we do not a way to negotiate supported
SOLID_FILL versions. However as we now have support for different
pixel_source properties, maybe we can drop version completely. If at
some point a driver needs to support different kind of SOLID_FILL
property (consider v2), we can add new pixel source to the enum.
> +
> + /* Add more versions if necessary */
> + if (blob_version == 1) {
> + state->solid_fill.r = user_info->r;
> + state->solid_fill.g = user_info->g;
> + state->solid_fill.b = user_info->b;
> + } else {
> + drm_dbg_atomic(state->plane->dev,
> + "[PLANE:%d:%s] unsupported solid fill version (version=%d)\n",
> + state->plane->base.id, state->plane->name,
> + blob_version);
> + return -EINVAL;
> + }
> + }
> +
> + drm_property_blob_put(state->solid_fill_blob);
> +
> + if (blob)
> + state->solid_fill_blob = drm_property_blob_get(blob);
> + else
> + state->solid_fill_blob = NULL;
> +
> + return 0;
> +}
> +
> static void set_out_fence_for_crtc(struct drm_atomic_state *state,
> struct drm_crtc *crtc, s32 __user *fence_ptr)
> {
> @@ -546,6 +591,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> state->src_h = val;
> } else if (property == plane->pixel_source_property) {
> state->pixel_source = val;
> + } else if (property == plane->solid_fill_property) {
> + struct drm_property_blob *solid_fill = drm_property_lookup_blob(dev, val);
> +
> + ret = drm_atomic_set_solid_fill_prop(state, solid_fill);
> + drm_property_blob_put(solid_fill);
> +
> + return ret;
Bonus point: dropping version from SOLID_FILL would allow us to use
drm_atomic_replace_property_blob_from_id() here.
> } else if (property == plane->alpha_property) {
> state->alpha = val;
> } else if (property == plane->blend_mode_property) {
> @@ -620,6 +672,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->src_h;
> } else if (property == plane->pixel_source_property) {
> *val = state->pixel_source;
> + } else if (property == plane->solid_fill_property) {
> + *val = state->solid_fill_blob ?
> + state->solid_fill_blob->base.id : 0;
> } 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 c500310a3d09..c632dfcd8a9b 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -200,6 +200,10 @@
> * "FB":
> * Framebuffer source set by the "FB_ID" property.
> *
> + * solid_fill:
> + * solid_fill is set up with drm_plane_create_solid_fill_property(). It
> + * contains pixel data that drivers can use to fill a plane.
> + *
> * 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).
> @@ -700,3 +704,29 @@ int drm_plane_create_pixel_source_property(struct drm_plane *plane,
> return 0;
> }
> EXPORT_SYMBOL(drm_plane_create_pixel_source_property);
> +
> +/**
> + * drm_plane_create_solid_fill_property - create a new solid_fill property
> + * @plane: drm plane
> + *
> + * This creates a new property blob that holds pixel data for solid fill planes.
> + * The property is exposed to userspace as a property blob called "solid_fill".
> + *
> + * For information on what the blob contains, see `drm_mode_solid_fill`.
> + */
> +int drm_plane_create_solid_fill_property(struct drm_plane *plane)
> +{
> + struct drm_property *prop;
> +
> + prop = drm_property_create(plane->dev,
> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
> + "solid_fill", 0);
> + if (!prop)
> + return -ENOMEM;
> +
> + drm_object_attach_property(&plane->base, prop, 0);
> + plane->solid_fill_property = prop;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 122bbfbaae33..e7158fbee389 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -60,4 +60,5 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
> unsigned int supported_modes);
> int drm_plane_create_pixel_source_property(struct drm_plane *plane,
> unsigned long extra_sources);
> +int drm_plane_create_solid_fill_property(struct drm_plane *plane);
> #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 89508b4dea4a..abf1458fa099 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -46,6 +46,17 @@ enum drm_plane_pixel_source {
> DRM_PLANE_PIXEL_SOURCE_MAX
> };
>
> +/**
> + * struct solid_fill_property - RGB values for solid fill plane
> + *
> + * Note: This is the V1 for this feature
> + */
> +struct drm_solid_fill {
> + uint32_t r;
> + uint32_t g;
> + uint32_t b;
> +};
> +
> /**
> * struct drm_plane_state - mutable plane state
> *
> @@ -130,6 +141,23 @@ struct drm_plane_state {
> */
> enum drm_plane_pixel_source pixel_source;
>
> + /**
> + * @solid_fill_blob:
> + *
> + * Blob containing relevant information for a solid fill plane
> + * including pixel format and data. See
> + * drm_plane_create_solid_fill_property() for more details.
> + */
> + struct drm_property_blob *solid_fill_blob;
> +
> + /**
> + * @solid_fill:
> + *
> + * Pixel data for solid fill planes. See
> + * drm_plane_create_solid_fill_property() for more details.
> + */
> + struct drm_solid_fill solid_fill;
> +
> /**
> * @alpha:
> * Opacity of the plane with 0 as completely transparent and 0xffff as
> @@ -720,6 +748,13 @@ struct drm_plane {
> */
> struct drm_property *pixel_source_property;
>
> + /**
> + * @solid_fill_property:
> + * Optional solid_fill property for this plane. See
> + * drm_plane_create_solid_fill_property().
> + */
> + struct drm_property *solid_fill_property;
> +
> /**
> * @alpha_property:
> * Optional alpha property for this plane. See
> 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;
> +};
> +
> +
> struct drm_mode_card_res {
> __u64 fb_id_ptr;
> __u64 crtc_id_ptr;
>
--
With best wishes
Dmitry
On Fri, Aug 4, 2023 at 3:27 PM Dmitry Baryshkov
<[email protected]> wrote:
>
> On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <[email protected]> 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 <[email protected]>
> > ---
> > 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. Maybe another COLOR_FILL enum value
with alpha might be better? Maybe just doing the alpha via the alpha
property is good enough.
>
> --
> With best wishes
> Dmitry
>
On Mon, Jul 31, 2023 at 6:01 AM Dmitry Baryshkov
<[email protected]> wrote:
>
> On 28/07/2023 20:02, 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 <[email protected]>
> > ---
> > 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(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index 01638c51ce0a..86fb876efbe6 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -254,6 +254,11 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
> > plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> > plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
> >
> > + if (plane_state->solid_fill_blob) {
> > + drm_property_blob_put(plane_state->solid_fill_blob);
> > + plane_state->solid_fill_blob = NULL;
> > + }
> > +
> > if (plane->color_encoding_property) {
> > if (!drm_object_property_get_default_value(&plane->base,
> > plane->color_encoding_property,
> > @@ -336,6 +341,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
> > if (state->fb)
> > drm_framebuffer_get(state->fb);
> >
> > + if (state->solid_fill_blob)
> > + drm_property_blob_get(state->solid_fill_blob);
> > +
> > state->fence = NULL;
> > state->commit = NULL;
> > state->fb_damage_clips = NULL;
> > @@ -385,6 +393,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
> > drm_crtc_commit_put(state->commit);
> >
> > drm_property_blob_put(state->fb_damage_clips);
> > + drm_property_blob_put(state->solid_fill_blob);
> > }
> > EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 454f980e16c9..039686c78c2a 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -316,6 +316,51 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
> > }
> > EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
> >
> > +static int drm_atomic_set_solid_fill_prop(struct drm_plane_state *state,
> > + struct drm_property_blob *blob)
> > +{
> > + int blob_version;
> > +
> > + if (blob == state->solid_fill_blob)
> > + return 0;
> > +
> > + if (blob) {
> > + struct drm_mode_solid_fill *user_info = (struct drm_mode_solid_fill *)blob->data;
> > +
> > + if (blob->length != sizeof(struct drm_mode_solid_fill)) {
> > + drm_dbg_atomic(state->plane->dev,
> > + "[PLANE:%d:%s] bad solid fill blob length: %zu\n",
> > + state->plane->base.id, state->plane->name,
> > + blob->length);
> > + return -EINVAL;
> > + }
> > +
> > + blob_version = user_info->version;
>
> I remember that we had versioning for quite some time. However it stroke
> me while reviewing that we do not a way to negotiate supported
> SOLID_FILL versions. However as we now have support for different
> pixel_source properties, maybe we can drop version completely. If at
> some point a driver needs to support different kind of SOLID_FILL
> property (consider v2), we can add new pixel source to the enum.
Agreed. Let's drop the versioning.
>
> > +
> > + /* Add more versions if necessary */
> > + if (blob_version == 1) {
> > + state->solid_fill.r = user_info->r;
> > + state->solid_fill.g = user_info->g;
> > + state->solid_fill.b = user_info->b;
> > + } else {
> > + drm_dbg_atomic(state->plane->dev,
> > + "[PLANE:%d:%s] unsupported solid fill version (version=%d)\n",
> > + state->plane->base.id, state->plane->name,
> > + blob_version);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + drm_property_blob_put(state->solid_fill_blob);
> > +
> > + if (blob)
> > + state->solid_fill_blob = drm_property_blob_get(blob);
> > + else
> > + state->solid_fill_blob = NULL;
> > +
> > + return 0;
> > +}
> > +
> > static void set_out_fence_for_crtc(struct drm_atomic_state *state,
> > struct drm_crtc *crtc, s32 __user *fence_ptr)
> > {
> > @@ -546,6 +591,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> > state->src_h = val;
> > } else if (property == plane->pixel_source_property) {
> > state->pixel_source = val;
> > + } else if (property == plane->solid_fill_property) {
> > + struct drm_property_blob *solid_fill = drm_property_lookup_blob(dev, val);
> > +
> > + ret = drm_atomic_set_solid_fill_prop(state, solid_fill);
> > + drm_property_blob_put(solid_fill);
> > +
> > + return ret;
>
> Bonus point: dropping version from SOLID_FILL would allow us to use
> drm_atomic_replace_property_blob_from_id() here.
>
> > } else if (property == plane->alpha_property) {
> > state->alpha = val;
> > } else if (property == plane->blend_mode_property) {
> > @@ -620,6 +672,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> > *val = state->src_h;
> > } else if (property == plane->pixel_source_property) {
> > *val = state->pixel_source;
> > + } else if (property == plane->solid_fill_property) {
> > + *val = state->solid_fill_blob ?
> > + state->solid_fill_blob->base.id : 0;
> > } 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 c500310a3d09..c632dfcd8a9b 100644
> > --- a/drivers/gpu/drm/drm_blend.c
> > +++ b/drivers/gpu/drm/drm_blend.c
> > @@ -200,6 +200,10 @@
> > * "FB":
> > * Framebuffer source set by the "FB_ID" property.
> > *
> > + * solid_fill:
> > + * solid_fill is set up with drm_plane_create_solid_fill_property(). It
> > + * contains pixel data that drivers can use to fill a plane.
> > + *
> > * 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).
> > @@ -700,3 +704,29 @@ int drm_plane_create_pixel_source_property(struct drm_plane *plane,
> > return 0;
> > }
> > EXPORT_SYMBOL(drm_plane_create_pixel_source_property);
> > +
> > +/**
> > + * drm_plane_create_solid_fill_property - create a new solid_fill property
> > + * @plane: drm plane
> > + *
> > + * This creates a new property blob that holds pixel data for solid fill planes.
> > + * The property is exposed to userspace as a property blob called "solid_fill".
> > + *
> > + * For information on what the blob contains, see `drm_mode_solid_fill`.
> > + */
> > +int drm_plane_create_solid_fill_property(struct drm_plane *plane)
> > +{
> > + struct drm_property *prop;
> > +
> > + prop = drm_property_create(plane->dev,
> > + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
> > + "solid_fill", 0);
> > + if (!prop)
> > + return -ENOMEM;
> > +
> > + drm_object_attach_property(&plane->base, prop, 0);
> > + plane->solid_fill_property = prop;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
> > diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> > index 122bbfbaae33..e7158fbee389 100644
> > --- a/include/drm/drm_blend.h
> > +++ b/include/drm/drm_blend.h
> > @@ -60,4 +60,5 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
> > unsigned int supported_modes);
> > int drm_plane_create_pixel_source_property(struct drm_plane *plane,
> > unsigned long extra_sources);
> > +int drm_plane_create_solid_fill_property(struct drm_plane *plane);
> > #endif
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > index 89508b4dea4a..abf1458fa099 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -46,6 +46,17 @@ enum drm_plane_pixel_source {
> > DRM_PLANE_PIXEL_SOURCE_MAX
> > };
> >
> > +/**
> > + * struct solid_fill_property - RGB values for solid fill plane
> > + *
> > + * Note: This is the V1 for this feature
> > + */
> > +struct drm_solid_fill {
> > + uint32_t r;
> > + uint32_t g;
> > + uint32_t b;
> > +};
> > +
> > /**
> > * struct drm_plane_state - mutable plane state
> > *
> > @@ -130,6 +141,23 @@ struct drm_plane_state {
> > */
> > enum drm_plane_pixel_source pixel_source;
> >
> > + /**
> > + * @solid_fill_blob:
> > + *
> > + * Blob containing relevant information for a solid fill plane
> > + * including pixel format and data. See
> > + * drm_plane_create_solid_fill_property() for more details.
> > + */
> > + struct drm_property_blob *solid_fill_blob;
> > +
> > + /**
> > + * @solid_fill:
> > + *
> > + * Pixel data for solid fill planes. See
> > + * drm_plane_create_solid_fill_property() for more details.
> > + */
> > + struct drm_solid_fill solid_fill;
> > +
> > /**
> > * @alpha:
> > * Opacity of the plane with 0 as completely transparent and 0xffff as
> > @@ -720,6 +748,13 @@ struct drm_plane {
> > */
> > struct drm_property *pixel_source_property;
> >
> > + /**
> > + * @solid_fill_property:
> > + * Optional solid_fill property for this plane. See
> > + * drm_plane_create_solid_fill_property().
> > + */
> > + struct drm_property *solid_fill_property;
> > +
> > /**
> > * @alpha_property:
> > * Optional alpha property for this plane. See
> > 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;
> > +};
> > +
> > +
> > struct drm_mode_card_res {
> > __u64 fb_id_ptr;
> > __u64 crtc_id_ptr;
> >
>
> --
> With best wishes
> Dmitry
>
On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <[email protected]> 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 <[email protected]>
> ---
> 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.
--
With best wishes
Dmitry
On Fri, 4 Aug 2023 at 16:44, Sebastian Wick <[email protected]> wrote:
>
> On Fri, Aug 4, 2023 at 3:27 PM Dmitry Baryshkov
> <[email protected]> wrote:
> >
> > On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <[email protected]> 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 <[email protected]>
> > > ---
> > > 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.
--
With best wishes
Dmitry
On 8/4/2023 6:40 AM, Sebastian Wick wrote:
> On Mon, Jul 31, 2023 at 6:01 AM Dmitry Baryshkov
> <[email protected]> wrote:
>>
>> On 28/07/2023 20:02, 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 <[email protected]>
>>> ---
>>> 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(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
>>> index 01638c51ce0a..86fb876efbe6 100644
>>> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
>>> @@ -254,6 +254,11 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
>>> plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>>> plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
>>>
>>> + if (plane_state->solid_fill_blob) {
>>> + drm_property_blob_put(plane_state->solid_fill_blob);
>>> + plane_state->solid_fill_blob = NULL;
>>> + }
>>> +
>>> if (plane->color_encoding_property) {
>>> if (!drm_object_property_get_default_value(&plane->base,
>>> plane->color_encoding_property,
>>> @@ -336,6 +341,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>>> if (state->fb)
>>> drm_framebuffer_get(state->fb);
>>>
>>> + if (state->solid_fill_blob)
>>> + drm_property_blob_get(state->solid_fill_blob);
>>> +
>>> state->fence = NULL;
>>> state->commit = NULL;
>>> state->fb_damage_clips = NULL;
>>> @@ -385,6 +393,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>>> drm_crtc_commit_put(state->commit);
>>>
>>> drm_property_blob_put(state->fb_damage_clips);
>>> + drm_property_blob_put(state->solid_fill_blob);
>>> }
>>> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>>> index 454f980e16c9..039686c78c2a 100644
>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>> @@ -316,6 +316,51 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
>>> }
>>> EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
>>>
>>> +static int drm_atomic_set_solid_fill_prop(struct drm_plane_state *state,
>>> + struct drm_property_blob *blob)
>>> +{
>>> + int blob_version;
>>> +
>>> + if (blob == state->solid_fill_blob)
>>> + return 0;
>>> +
>>> + if (blob) {
>>> + struct drm_mode_solid_fill *user_info = (struct drm_mode_solid_fill *)blob->data;
>>> +
>>> + if (blob->length != sizeof(struct drm_mode_solid_fill)) {
>>> + drm_dbg_atomic(state->plane->dev,
>>> + "[PLANE:%d:%s] bad solid fill blob length: %zu\n",
>>> + state->plane->base.id, state->plane->name,
>>> + blob->length);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + blob_version = user_info->version;
>>
>> I remember that we had versioning for quite some time. However it stroke
>> me while reviewing that we do not a way to negotiate supported
>> SOLID_FILL versions. However as we now have support for different
>> pixel_source properties, maybe we can drop version completely. If at
>> some point a driver needs to support different kind of SOLID_FILL
>> property (consider v2), we can add new pixel source to the enum.
>
> Agreed. Let's drop the versioning.
Acked.
Thanks,
Jessica Zhang
>
>>
>>> +
>>> + /* Add more versions if necessary */
>>> + if (blob_version == 1) {
>>> + state->solid_fill.r = user_info->r;
>>> + state->solid_fill.g = user_info->g;
>>> + state->solid_fill.b = user_info->b;
>>> + } else {
>>> + drm_dbg_atomic(state->plane->dev,
>>> + "[PLANE:%d:%s] unsupported solid fill version (version=%d)\n",
>>> + state->plane->base.id, state->plane->name,
>>> + blob_version);
>>> + return -EINVAL;
>>> + }
>>> + }
>>> +
>>> + drm_property_blob_put(state->solid_fill_blob);
>>> +
>>> + if (blob)
>>> + state->solid_fill_blob = drm_property_blob_get(blob);
>>> + else
>>> + state->solid_fill_blob = NULL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static void set_out_fence_for_crtc(struct drm_atomic_state *state,
>>> struct drm_crtc *crtc, s32 __user *fence_ptr)
>>> {
>>> @@ -546,6 +591,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>>> state->src_h = val;
>>> } else if (property == plane->pixel_source_property) {
>>> state->pixel_source = val;
>>> + } else if (property == plane->solid_fill_property) {
>>> + struct drm_property_blob *solid_fill = drm_property_lookup_blob(dev, val);
>>> +
>>> + ret = drm_atomic_set_solid_fill_prop(state, solid_fill);
>>> + drm_property_blob_put(solid_fill);
>>> +
>>> + return ret;
>>
>> Bonus point: dropping version from SOLID_FILL would allow us to use
>> drm_atomic_replace_property_blob_from_id() here.
>>
>>> } else if (property == plane->alpha_property) {
>>> state->alpha = val;
>>> } else if (property == plane->blend_mode_property) {
>>> @@ -620,6 +672,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>>> *val = state->src_h;
>>> } else if (property == plane->pixel_source_property) {
>>> *val = state->pixel_source;
>>> + } else if (property == plane->solid_fill_property) {
>>> + *val = state->solid_fill_blob ?
>>> + state->solid_fill_blob->base.id : 0;
>>> } 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 c500310a3d09..c632dfcd8a9b 100644
>>> --- a/drivers/gpu/drm/drm_blend.c
>>> +++ b/drivers/gpu/drm/drm_blend.c
>>> @@ -200,6 +200,10 @@
>>> * "FB":
>>> * Framebuffer source set by the "FB_ID" property.
>>> *
>>> + * solid_fill:
>>> + * solid_fill is set up with drm_plane_create_solid_fill_property(). It
>>> + * contains pixel data that drivers can use to fill a plane.
>>> + *
>>> * 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).
>>> @@ -700,3 +704,29 @@ int drm_plane_create_pixel_source_property(struct drm_plane *plane,
>>> return 0;
>>> }
>>> EXPORT_SYMBOL(drm_plane_create_pixel_source_property);
>>> +
>>> +/**
>>> + * drm_plane_create_solid_fill_property - create a new solid_fill property
>>> + * @plane: drm plane
>>> + *
>>> + * This creates a new property blob that holds pixel data for solid fill planes.
>>> + * The property is exposed to userspace as a property blob called "solid_fill".
>>> + *
>>> + * For information on what the blob contains, see `drm_mode_solid_fill`.
>>> + */
>>> +int drm_plane_create_solid_fill_property(struct drm_plane *plane)
>>> +{
>>> + struct drm_property *prop;
>>> +
>>> + prop = drm_property_create(plane->dev,
>>> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
>>> + "solid_fill", 0);
>>> + if (!prop)
>>> + return -ENOMEM;
>>> +
>>> + drm_object_attach_property(&plane->base, prop, 0);
>>> + plane->solid_fill_property = prop;
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
>>> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
>>> index 122bbfbaae33..e7158fbee389 100644
>>> --- a/include/drm/drm_blend.h
>>> +++ b/include/drm/drm_blend.h
>>> @@ -60,4 +60,5 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
>>> unsigned int supported_modes);
>>> int drm_plane_create_pixel_source_property(struct drm_plane *plane,
>>> unsigned long extra_sources);
>>> +int drm_plane_create_solid_fill_property(struct drm_plane *plane);
>>> #endif
>>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>>> index 89508b4dea4a..abf1458fa099 100644
>>> --- a/include/drm/drm_plane.h
>>> +++ b/include/drm/drm_plane.h
>>> @@ -46,6 +46,17 @@ enum drm_plane_pixel_source {
>>> DRM_PLANE_PIXEL_SOURCE_MAX
>>> };
>>>
>>> +/**
>>> + * struct solid_fill_property - RGB values for solid fill plane
>>> + *
>>> + * Note: This is the V1 for this feature
>>> + */
>>> +struct drm_solid_fill {
>>> + uint32_t r;
>>> + uint32_t g;
>>> + uint32_t b;
>>> +};
>>> +
>>> /**
>>> * struct drm_plane_state - mutable plane state
>>> *
>>> @@ -130,6 +141,23 @@ struct drm_plane_state {
>>> */
>>> enum drm_plane_pixel_source pixel_source;
>>>
>>> + /**
>>> + * @solid_fill_blob:
>>> + *
>>> + * Blob containing relevant information for a solid fill plane
>>> + * including pixel format and data. See
>>> + * drm_plane_create_solid_fill_property() for more details.
>>> + */
>>> + struct drm_property_blob *solid_fill_blob;
>>> +
>>> + /**
>>> + * @solid_fill:
>>> + *
>>> + * Pixel data for solid fill planes. See
>>> + * drm_plane_create_solid_fill_property() for more details.
>>> + */
>>> + struct drm_solid_fill solid_fill;
>>> +
>>> /**
>>> * @alpha:
>>> * Opacity of the plane with 0 as completely transparent and 0xffff as
>>> @@ -720,6 +748,13 @@ struct drm_plane {
>>> */
>>> struct drm_property *pixel_source_property;
>>>
>>> + /**
>>> + * @solid_fill_property:
>>> + * Optional solid_fill property for this plane. See
>>> + * drm_plane_create_solid_fill_property().
>>> + */
>>> + struct drm_property *solid_fill_property;
>>> +
>>> /**
>>> * @alpha_property:
>>> * Optional alpha property for this plane. See
>>> 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;
>>> +};
>>> +
>>> +
>>> struct drm_mode_card_res {
>>> __u64 fb_id_ptr;
>>> __u64 crtc_id_ptr;
>>>
>>
>> --
>> With best wishes
>> Dmitry
>>
>
On 8/4/2023 6:27 AM, Dmitry Baryshkov wrote:
> On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <[email protected]> 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 <[email protected]>
>> ---
>> 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.
Hi Dmitry,
I don't really agree that adding a solid fill alpha would be a good
idea. Since the intent behind solid fill is to have a single color for
the entire plane, I think it makes more sense to have solid fill rely on
the global plane alpha.
As stated in earlier discussions, I think having both a solid_fill.alpha
and a plane_state.alpha would be redundant and serve to confuse the user
as to which one to set.
Thanks,
Jessica Zhang
>
> --
> With best wishes
> Dmitry
On 8 August 2023 00:41:07 GMT+03:00, Jessica Zhang <[email protected]> wrote:
>
>
>On 8/4/2023 6:27 AM, Dmitry Baryshkov wrote:
>> On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <[email protected]> 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 <[email protected]>
>>> ---
>>> 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.
>
>Hi Dmitry,
>
>I don't really agree that adding a solid fill alpha would be a good idea. Since the intent behind solid fill is to have a single color for the entire plane, I think it makes more sense to have solid fill rely on the global plane alpha.
>
>As stated in earlier discussions, I think having both a solid_fill.alpha and a plane_state.alpha would be redundant and serve to confuse the user as to which one to set.
That depends on the blending mode: in Coverage mode one has independent plane and contents alpha values. And I consider alpha value to be a part of the colour in the rgba/bgra modes.
>
>Thanks,
>
>Jessica Zhang
>
>>
>> --
>> With best wishes
>> Dmitry
--
With best wishes
Dmitry
On 8/7/2023 6:07 PM, Dmitry Baryshkov wrote:
>
>
> On 8 August 2023 00:41:07 GMT+03:00, Jessica Zhang <[email protected]> wrote:
>>
>>
>> On 8/4/2023 6:27 AM, Dmitry Baryshkov wrote:
>>> On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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.
>>
>> Hi Dmitry,
>>
>> I don't really agree that adding a solid fill alpha would be a good idea. Since the intent behind solid fill is to have a single color for the entire plane, I think it makes more sense to have solid fill rely on the global plane alpha.
>>
>> As stated in earlier discussions, I think having both a solid_fill.alpha and a plane_state.alpha would be redundant and serve to confuse the user as to which one to set.
>
> That depends on the blending mode: in Coverage mode one has independent plane and contents alpha values. And I consider alpha value to be a part of the colour in the rgba/bgra modes.
Acked -- taking Sebastian's concern into consideration, I think I'll
have "PIXEL_SOURCE_SOLID_FILL_RGB" and add a separate
"PIXEL_SOURCE_SOLID_FILL_RGBA".
Thanks,
Jessica Zhang
>
>
>>
>> Thanks,
>>
>> Jessica Zhang
>>
>>>
>>> --
>>> With best wishes
>>> Dmitry
>
> --
> With best wishes
> Dmitry