On 05/01/2023 01:40, Jessica Zhang wrote:
Add support for solid_fill property to drm_plane. In addition, add
support for setting and getting the values for solid_fill.

solid_fill holds data for supporting solid fill planes. The property
accepts an RGB323232 value and the driver data is formatted as such:

struct drm_solid_fill {
        u32 r;
        u32 g;
        u32 b;
};

This description is unnecessary (and confusing), since you describe drm_solid_fill_info below.


To enable solid fill planes, userspace must assigned solid_fill to a

must assign. Also the phrasing seems strange. The blob is assigned to the 'solid_fill' property, not the other way around.

property blob containing the following information:

This should clearly describe if solid_fill overrides FB or if FB overrides solid_fill. Also please extend properties documentation in Documentation/gpu/drm-kms.rst.


struct drm_solid_fill_info {
        u8 version;
        u32 r, g, b;
};

Changes in V2:
- Changed solid_fill property to a property blob (Simon, Dmitry)
- Added drm_solid_fill struct (Simon)
- Added drm_solid_fill_info struct (Simon)

Changes in V3:
- Corrected typo in drm_solid_fill struct documentation

Signed-off-by: Jessica Zhang <quic_jessz...@quicinc.com>
---
  drivers/gpu/drm/drm_atomic_state_helper.c |  9 ++++
  drivers/gpu/drm/drm_atomic_uapi.c         | 59 +++++++++++++++++++++++
  drivers/gpu/drm/drm_blend.c               | 17 +++++++
  include/drm/drm_blend.h                   |  1 +
  include/drm/drm_plane.h                   | 43 +++++++++++++++++
  5 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index dfb57217253b..c96fd1f2ad99 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -253,6 +253,11 @@ 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;
+ 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,
@@ -335,6 +340,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;
@@ -384,6 +392,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 c06d0639d552..8a1d2fb7a757 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -316,6 +316,55 @@ drm_atomic_set_crtc_for_connector(struct 
drm_connector_state *conn_state,
  }
  EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
+static void drm_atomic_convert_solid_fill_info(struct drm_solid_fill *out,
+               struct drm_solid_fill_info *in)

No need for a separate function, you can inline this.

+{
+       out->r = in->r;
+       out->g = in->g;
+       out->b = in->b;
+}
+
+static int drm_atomic_set_solid_fill_prop(struct drm_plane_state *state,
+               struct drm_property_blob *blob)
+{
+       int ret = 0;
+       int blob_version;
+
+       if (blob == state->solid_fill_blob)
+               return 0;
+
+       drm_property_blob_put(state->solid_fill_blob);
+       state->solid_fill_blob = NULL;
+
+       memset(&state->solid_fill, 0, sizeof(state->solid_fill));
+
+       if (blob) {
+               if (blob->length != sizeof(struct drm_solid_fill_info)) {
+                       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 = ((struct drm_solid_fill_info 
*)blob->data)->version;

Please assign blob->data to temporary var.

+
+               /* Append with more versions if necessary */

s/Append/Add/

+               if (blob_version == 1) {
+                       drm_atomic_convert_solid_fill_info(&state->solid_fill, 
blob->data);
+               } else {
+                       drm_dbg_atomic(state->plane->dev,
+                                       "[PLANE:%d:%s] failed to set solid fill 
(ret=%d)\n",
+                                       state->plane->base.id, 
state->plane->name,
+                                       ret);
+                       return -EINVAL;
+               }
+               state->solid_fill_blob = drm_property_blob_get(blob);
+       }
+
+       return ret;
+}
+
  static void set_out_fence_for_crtc(struct drm_atomic_state *state,
                                   struct drm_crtc *crtc, s32 __user *fence_ptr)
  {
@@ -544,6 +593,13 @@ 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->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) {
@@ -616,6 +672,9 @@ 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->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 b4c8cab7158c..17ab645c8309 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -616,3 +616,20 @@ int drm_plane_create_blend_mode_property(struct drm_plane 
*plane,
        return 0;
  }
  EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+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 88bdfec3bd88..0338a860b9c8 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
                              struct drm_atomic_state *state);
  int drm_plane_create_blend_mode_property(struct drm_plane *plane,
                                         unsigned int supported_modes);
+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 447e664e49d5..3b9da06f358b 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -40,6 +40,25 @@ enum drm_scaling_filter {
        DRM_SCALING_FILTER_NEAREST_NEIGHBOR,
  };
+/**
+ * struct drm_solid_fill_info - User info for solid fill planes
+ */
+struct drm_solid_fill_info {
+       __u8 version;
+       __u32 r, g, b;
+};
+
+/**
+ * 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
   *
@@ -116,6 +135,23 @@ struct drm_plane_state {
        /** @src_h: height of visible portion of plane (in 16.16) */
        uint32_t src_h, src_w;
+ /**
+        * @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
@@ -699,6 +735,13 @@ struct drm_plane {
         */
        struct drm_plane_state *state;
+ /*
+        * @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

--
With best wishes
Dmitry

Reply via email to