Re: [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property
On 8/4/2023 6:40 AM, Sebastian Wick wrote: On Mon, Jul 31, 2023 at 6:01 AM Dmitry Baryshkov 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 --- 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_ou
Re: [PATCH RFC v5 04/10] drm/atomic: Add pixel source to plane state dump
On 7/28/2023 5:04 PM, Dmitry Baryshkov wrote: On 28/07/2023 20:02, Jessica Zhang wrote: Add pixel source to the atomic plane state dump Signed-off-by: Jessica Zhang --- drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_crtc_internal.h | 2 ++ drivers/gpu/drm/drm_plane.c | 12 3 files changed, 15 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index b4c6ffc438da..c38014abc590 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -713,6 +713,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p, drm_printf(p, "plane[%u]: %s\n", plane->base.id, plane->name); drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); + drm_printf(p, "\tpixel-source=%s\n", drm_plane_get_pixel_source_name(state->pixel_source)); drm_printf(p, "\tfb=%u\n", state->fb ? state->fb->base.id : 0); if (state->fb) drm_framebuffer_print_info(p, 2, state->fb); diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index 501a10edd0e1..75b59ec9f1be 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -38,6 +38,7 @@ enum drm_color_encoding; enum drm_color_range; enum drm_connector_force; enum drm_mode_status; +enum drm_plane_pixel_source; struct drm_atomic_state; struct drm_bridge; @@ -267,6 +268,7 @@ int drm_plane_check_pixel_format(struct drm_plane *plane, u32 format, u64 modifier); struct drm_mode_rect * __drm_plane_get_damage_clips(const struct drm_plane_state *state); +const char *drm_plane_get_pixel_source_name(enum drm_plane_pixel_source pixel_source); /* drm_bridge.c */ void drm_bridge_detach(struct drm_bridge *bridge); diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index f342cf15412b..4188b3491625 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1487,6 +1487,18 @@ __drm_plane_get_damage_clips(const struct drm_plane_state *state) state->fb_damage_clips->data : NULL); } +const char *drm_plane_get_pixel_source_name(enum drm_plane_pixel_source pixel_source) +{ + switch(pixel_source) { + case DRM_PLANE_PIXEL_SOURCE_NONE: + return "NONE"; + case DRM_PLANE_PIXEL_SOURCE_FB: + return "fb"; + default: + return ""; + } +} Please use DRM_ENUM_NAME_FN instead. Hi Dmitry, Acked. Thanks, Jessica Zhang + /** * drm_plane_get_damage_clips - Returns damage clips. * @state: Plane state. -- With best wishes Dmitry
Re: [PATCH RFC v5 05/10] drm/atomic: Add solid fill data to plane state dump
On 7/28/2023 5:05 PM, Dmitry Baryshkov wrote: On 28/07/2023 20:02, Jessica Zhang wrote: Add solid_fill property data to the atomic plane state dump. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/drm_atomic.c | 4 drivers/gpu/drm/drm_plane.c | 10 ++ include/drm/drm_plane.h | 3 +++ 3 files changed, 17 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index c38014abc590..1ee7d08041bc 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -717,6 +717,10 @@ static void drm_atomic_plane_print_state(struct drm_printer *p, drm_printf(p, "\tfb=%u\n", state->fb ? state->fb->base.id : 0); if (state->fb) drm_framebuffer_print_info(p, 2, state->fb); + drm_printf(p, "\tsolid_fill=%u\n", + state->solid_fill_blob ? state->solid_fill_blob->base.id : 0); + if (state->solid_fill_blob) + drm_plane_solid_fill_print_info(p, 2, state); drm_printf(p, "\tcrtc-pos=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&dest)); drm_printf(p, "\tsrc-pos=" DRM_RECT_FP_FMT "\n", DRM_RECT_FP_ARG(&src)); drm_printf(p, "\trotation=%x\n", state->rotation); diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 4188b3491625..009d3ebd9b39 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -1494,11 +1494,21 @@ const char *drm_plane_get_pixel_source_name(enum drm_plane_pixel_source pixel_so return "NONE"; case DRM_PLANE_PIXEL_SOURCE_FB: return "fb"; + case DRM_PLANE_PIXEL_SOURCE_SOLID_FILL: + return "solid_fill"; default: return ""; } } This chunk should be a part of the previous commit. Or dropped completely once DRM_ENUM_NAME_FN is used. The rest LGTM. Hi Dmitry, Sounds good -- will drop this. Thanks, Jessica Zhang +void drm_plane_solid_fill_print_info(struct drm_printer *p, unsigned int indent, + const struct drm_plane_state *state) +{ + drm_printf_indent(p, indent, "r=0x%x\n", state->solid_fill.r); + drm_printf_indent(p, indent, "g=0x%x\n", state->solid_fill.g); + drm_printf_indent(p, indent, "b=0x%x\n", state->solid_fill.b); +} + /** * drm_plane_get_damage_clips - Returns damage clips. * @state: Plane state. diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 234fee3d5a95..303f01f0588c 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -1000,6 +1000,9 @@ drm_plane_get_damage_clips_count(const struct drm_plane_state *state); struct drm_mode_rect * drm_plane_get_damage_clips(const struct drm_plane_state *state); +void drm_plane_solid_fill_print_info(struct drm_printer *p, unsigned int indent, + const struct drm_plane_state *state); + int drm_plane_create_scaling_filter_property(struct drm_plane *plane, unsigned int supported_filters); -- With best wishes Dmitry
Re: [PATCH RFC v5 09/10] drm/msm/dpu: Use DRM solid_fill property
On 7/31/2023 5:52 PM, Dmitry Baryshkov wrote: On 01/08/2023 03:39, Jessica Zhang wrote: On 7/30/2023 9:15 PM, Dmitry Baryshkov wrote: On 28/07/2023 20:02, Jessica Zhang wrote: Drop DPU_PLANE_COLOR_FILL_FLAG and check the DRM solid_fill property to determine if the plane is solid fill. In addition drop the DPU plane color_fill field as we can now use drm_plane_state.solid_fill instead, and pass in drm_plane_state.alpha to _dpu_plane_color_fill_pipe() to allow userspace to configure the alpha value for the solid fill color. Reviewed-by: Dmitry Baryshkov Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24 ++-- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 114c803ff99b..95fc0394d13e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -42,7 +42,6 @@ #define SHARP_SMOOTH_THR_DEFAULT 8 #define SHARP_NOISE_THR_DEFAULT 2 -#define DPU_PLANE_COLOR_FILL_FLAG BIT(31) #define DPU_ZPOS_MAX 255 /* @@ -82,7 +81,6 @@ struct dpu_plane { enum dpu_sspp pipe; - uint32_t color_fill; bool is_error; bool is_rt_pipe; const struct dpu_mdss_cfg *catalog; @@ -606,6 +604,20 @@ static void _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate, _dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, pstate->rotation); } +static uint32_t _dpu_plane_get_bgr_fill_color(struct drm_solid_fill solid_fill) As I commented for v4 (please excuse me for not responding to your email at thattime), we can return abgr here, taking plane->state->alpha into account. Hi Dmitry, Since it seems that this comment wasn't resolved, I can drop your R-B tag in the next revision. It's a minor issue, so no need to drop the tag. From my previous response, I pointed out that the color parameter expects an RGB value [1]. So is the intention here to refactor _dpu_plane_color_fill() to accept an ABGR color? That's what I'm suggesting. Hi Dmitry, Got it, sounds good to me. Thanks, Jessica Zhang Thanks, Jessica Zhang [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L676 +{ + uint32_t ret = 0; + uint8_t b = solid_fill.b >> 24; + uint8_t g = solid_fill.g >> 24; + uint8_t r = solid_fill.r >> 24; + + ret |= b << 16; + ret |= g << 8; + ret |= r; + + return ret; +} + /** * _dpu_plane_color_fill - enables color fill on plane * @pdpu: Pointer to DPU plane object @@ -977,9 +989,9 @@ void dpu_plane_flush(struct drm_plane *plane) if (pdpu->is_error) /* force white frame with 100% alpha pipe output on error */ _dpu_plane_color_fill(pdpu, 0xFF, 0xFF); - else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) - /* force 100% alpha */ - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF); + else if (drm_plane_solid_fill_enabled(plane->state)) + _dpu_plane_color_fill(pdpu, _dpu_plane_get_bgr_fill_color(plane->state->solid_fill), + plane->state->alpha); else { dpu_plane_flush_csc(pdpu, &pstate->pipe); dpu_plane_flush_csc(pdpu, &pstate->r_pipe); @@ -1024,7 +1036,7 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane, } /* override for color fill */ - if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) { + if (drm_plane_solid_fill_enabled(plane->state)) { _dpu_plane_set_qos_ctrl(plane, pipe, false); /* skip remaining processing on color fill */ -- With best wishes Dmitry -- With best wishes Dmitry
Re: [PATCH RFC v5 01/10] drm: Introduce pixel_source DRM plane property
On 8/4/2023 6:15 AM, Sebastian Wick wrote: On Fri, Jul 28, 2023 at 7:03 PM 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. The current possible pixel sources are DRM_PLANE_PIXEL_SOURCE_NONE and DRM_PLANE_PIXEL_SOURCE_FB with *_PIXEL_SOURCE_FB being the default value. 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 | 85 +++ drivers/gpu/drm/drm_plane.c | 3 ++ include/drm/drm_blend.h | 2 + include/drm/drm_plane.h | 21 6 files changed, 116 insertions(+) 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..c500310a3d09 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,73 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_blend_mode_property); + +/** + * 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 always be enabled as a supported + *source. + * + * 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. + */ +int drm_plane_create_pixel_source_property(struct drm_plane *pla
Re: [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property
On 8/4/2023 6:27 AM, 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. 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
Re: [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property
On 8 August 2023 00:41:07 GMT+03:00, Jessica Zhang wrote: > > >On 8/4/2023 6:27 AM, 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. > >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