Some drivers support hardware that have optimizations for solid fill
planes. This series aims to expose these capabilities to userspace as
some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
hardware composer HAL) that can be set by apps like the Android Gears
test app.
In order to expose this capability to userspace, this series will:
- Introduce solid_fill and pixel_source properties to allow userspace to
toggle between FB and solid fill sources
- Loosen NULL FB checks within the DRM atomic commit callstack to allow
for NULL FB when solid fill is enabled.
- Add NULL FB checks in methods where FB was previously assumed to be
non-NULL
- Have MSM DPU driver use drm_plane_state.solid_fill instead of
dpu_plane_state.color_fill
Note: The solid fill planes feature depends on both the solid_fill *and*
pixel_source properties.
To use this feature, userspace can set the solid_fill property to a blob
containing the solid fill color (in RGB323232 format) and and setting the
pixel_source property to DRM_PLANE_PIXEL_SOURCE_SOLID_FILL. This will
disable memory fetch and the resulting plane will display the color
specified by the solid_fill blob.
In order to disable a solid fill plane, the user must set the pixel_source
to NONE. A plane with a pixel_source of "solid_fill" and a NULL solid_fill
blob will cause an error on commit.
Currently, there's only one version of the solid_fill blob property.
However if other drivers want to support a similar feature, but require
more than just the solid fill color, they can extend this feature by
extending the pixel_source enum and adding another solid fill blob property.
This 2 property approach was chosen because passing in a special 1x1 FB
with the necessary color information would have unecessary overhead that
does not reflect the behavior of the solid fill feature. In addition,
assigning the solid fill blob to FB_ID would require loosening some core
drm_property checks that might cause unwanted side effects elsewhere.
---
Changes in v7:
- Updated cover letter (Sebastian)
- Updated the uAPI documentation (Sebastian, Pekka)
- Specify that padding must be set to zero in drm_mode_solid_fill
documentation (Pekka)
- Use %08x format when printing solid fill info in plane state dump (Pekka)
- Use new_plane_state->fb directly in drm_atomic_plane_check() (Dmitry)
- Dropped documentation for alpha for _dpu_plane_color_fill() (Dmitry)
- Defined DPU_SOLID_FILL_FORMAT macro (Dmitry)
- Fixed some necessary checks being skipped in the DPU atomic
commit path (Dmitry)
- Rebased to tip of msm-next
- Picked up Acked-by and Reviewed-by tags
Changes in v6:
- Have _dpu_plane_color_fill() take in a single ABGR8888 color instead
of having separate alpha and BGR color parameters (Dmitry)
- Drop plane->state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB check
in SetPlane ioctl (Dmitry)
- Add DRM_PLANE_PIXEL_SOURCE_NONE as a default pixel source (Sebastian)
- Dropped versioning from solid fill property blob (Dmitry)
- Use DRM_ENUM_NAME_FN (Dmitry)
- Use drm_atomic_replace_property_blob_from_id() (Dmitry)
- drm_atomic_check_fb -> drm_atomic_plane_check_fb (Dmitry)
- Group redundant NULL FB checks (Dmitry)
- Squashed drm_plane_needs_disable() implementation with
DRM_PLANE_PIXEL_SOURCE_NONE declaration (Sebastian)
- Add comment to support RGBA solid fill color in the future (Dmitry)
- Link to v5: https://lore.kernel.org/r/[email protected]
Changes in v5:
- Added support for PIXEL_SOURCE_NONE (Sebastian)
- Added WARN_ON() in drm_plane_has_visible_data() if pixel_source isn't
set (Dmitry)
- Added debugfs support for both properties (Dmitry)
- Corrected u32 to u8 conversion (Pekka)
- Moved drm_solid_fill_info struct and related documentation to
include/uapi (Pekka)
- Changed drm_solid_fill_info.version to __u32 for data alignment (Pekka)
- Added more detailed UAPI and kernel documentation (Pekka)
- Reordered patch series so that the pixel_source property is introduced
before solid_fill (Dmitry)
- Fixed inconsistent ABGR8888/RGBA8888 format declaration (Pekka)
- Reset pixel_source to FB in drm_mode_setplane() (Dmitry)
- Rename supported_sources to extra_sources (Dmitry)
- Only destroy old solid_fill blob state if new state is valid (Pekka)
- Link to v4: https://lore.kernel.org/r/[email protected]
Changes in v4:
- Rebased onto latest kernel
- Reworded cover letter for clarity (Dmitry)
- Reworded commit messages for clarity
- Split existing changes into smaller commits
- Added pixel_source enum property (Dmitry, Pekka, Ville)
- Updated drm-kms comment docs with pixel_source and solid_fill
properties (Dmitry)
- Inlined drm_atomic_convert_solid_fill_info() (Dmitry)
- Passed in plane state alpha value to _dpu_plane_color_fill_pipe()
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- Fixed some logic errors in atomic checks (Dmitry)
- Introduced drm_plane_has_visible_data() and drm_atomic_check_fb() helper
methods (Dmitry)
- Fixed typo in drm_solid_fill struct documentation
- Created drm_plane_has_visible_data() helper and corrected CRTC and FB
NULL-check logic (Dmitry)
- Merged `if (fb)` blocks in drm_atomic_plane_check() and abstracted
them into helper method (Dmitry)
- Inverted `if (solid_fill_enabled) else if (fb)` check order (Dmitry)
- Fixed indentation (Dmitry)
Changes in v2:
- Dropped SOLID_FILL_FORMAT property (Simon)
- Switched to implementing solid_fill property as a blob (Simon, Dmitry)
- Added drm_solid_fill and drm_solid_fill_info structs (Simon)
- Changed to checks for if solid_fill_blob is set (Dmitry)
- Abstracted (plane_state && !solid_fill_blob) checks to helper method
(Dmitry)
- Removed DPU_PLANE_COLOR_FILL_FLAG
- Fixed whitespace and indentation issues (Dmitry)
- Changed to checks for if solid_fill_blob is set (Dmitry)
- Abstracted (plane_state && !solid_fill_blob) checks to helper method
(Dmitry)
- Fixed dropped 'const' warning
- Added helper to convert color fill to BGR888 (Rob)
- Fixed indentation issue (Dmitry)
- Added support for solid fill on planes of varying sizes
---
Jessica Zhang (10):
drm: Introduce pixel_source DRM plane property
drm: Introduce solid fill DRM plane property
drm: Add solid fill pixel source
drm/atomic: Add pixel source to plane state dump
drm/atomic: Add solid fill data to plane state dump
drm/atomic: Move framebuffer checks to helper
drm/atomic: Loosen FB atomic checks
drm/msm/dpu: Allow NULL FBs in atomic commit
drm/msm/dpu: Use DRM solid_fill property
drm/msm/dpu: Add solid fill and pixel source properties
drivers/gpu/drm/drm_atomic.c | 148 +++++++++++++++++-------------
drivers/gpu/drm/drm_atomic_helper.c | 39 ++++----
drivers/gpu/drm/drm_atomic_state_helper.c | 10 ++
drivers/gpu/drm/drm_atomic_uapi.c | 30 ++++++
drivers/gpu/drm/drm_blend.c | 133 +++++++++++++++++++++++++++
drivers/gpu/drm/drm_crtc_internal.h | 1 +
drivers/gpu/drm/drm_plane.c | 27 +++++-
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 75 ++++++++++-----
include/drm/drm_atomic_helper.h | 4 +-
include/drm/drm_blend.h | 3 +
include/drm/drm_plane.h | 90 ++++++++++++++++++
include/uapi/drm/drm_mode.h | 24 +++++
13 files changed, 481 insertions(+), 112 deletions(-)
---
base-commit: b08d26dac1a1075c874f40ee02ec8ddc39e20146
change-id: 20230404-solid-fill-05016175db36
Best regards,
--
Jessica Zhang <[email protected]>
Loosen the requirements for atomic and legacy commit so that, in cases
where pixel_source != FB, the commit can still go through.
This includes adding framebuffer NULL checks in other areas to account for
FB being NULL when non-FB pixel sources are enabled.
To disable a plane, the pixel_source must be NONE or the FB must be NULL
if pixel_source == FB.
Signed-off-by: Jessica Zhang <[email protected]>
---
drivers/gpu/drm/drm_atomic.c | 21 ++++++++++----------
drivers/gpu/drm/drm_atomic_helper.c | 39 +++++++++++++++++++++----------------
include/drm/drm_atomic_helper.h | 4 ++--
include/drm/drm_plane.h | 29 +++++++++++++++++++++++++++
4 files changed, 64 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 48a2df4e3d27..66e49c06aced 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -674,17 +674,16 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
{
struct drm_plane *plane = new_plane_state->plane;
struct drm_crtc *crtc = new_plane_state->crtc;
- const struct drm_framebuffer *fb = new_plane_state->fb;
int ret;
- /* either *both* CRTC and FB must be set, or neither */
- if (crtc && !fb) {
- drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no FB\n",
+ /* either *both* CRTC and pixel source must be set, or neither */
+ if (crtc && !drm_plane_has_visible_data(new_plane_state)) {
+ drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no visible data\n",
plane->base.id, plane->name);
return -EINVAL;
- } else if (fb && !crtc) {
- drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] FB set but no CRTC\n",
- plane->base.id, plane->name);
+ } else if (drm_plane_has_visible_data(new_plane_state) && !crtc) {
+ drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] Source %d has visible data but no CRTC\n",
+ plane->base.id, plane->name, new_plane_state->pixel_source);
return -EINVAL;
}
@@ -715,9 +714,11 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
}
- ret = drm_atomic_plane_check_fb(new_plane_state);
- if (ret)
- return ret;
+ if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && new_plane_state->fb) {
+ ret = drm_atomic_plane_check_fb(new_plane_state);
+ if (ret)
+ return ret;
+ }
if (plane_switching_crtc(old_plane_state, new_plane_state)) {
drm_dbg_atomic(plane->dev,
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 292e38eb6218..27e8c8dd9324 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -852,7 +852,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
bool can_position,
bool can_update_disabled)
{
- struct drm_framebuffer *fb = plane_state->fb;
struct drm_rect *src = &plane_state->src;
struct drm_rect *dst = &plane_state->dst;
unsigned int rotation = plane_state->rotation;
@@ -864,7 +863,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
*src = drm_plane_state_src(plane_state);
*dst = drm_plane_state_dest(plane_state);
- if (!fb) {
+ if (!drm_plane_has_visible_data(plane_state)) {
plane_state->visible = false;
return 0;
}
@@ -881,25 +880,31 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
return -EINVAL;
}
- drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
+ /* Check that selected pixel source is valid */
+ if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && plane_state->fb) {
+ struct drm_framebuffer *fb = plane_state->fb;
+ drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
- /* Check scaling */
- hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
- vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
- if (hscale < 0 || vscale < 0) {
- drm_dbg_kms(plane_state->plane->dev,
- "Invalid scaling of plane\n");
- drm_rect_debug_print("src: ", &plane_state->src, true);
- drm_rect_debug_print("dst: ", &plane_state->dst, false);
- return -ERANGE;
- }
+ /* Check scaling */
+ hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+ vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
- if (crtc_state->enable)
- drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
+ if (hscale < 0 || vscale < 0) {
+ drm_dbg_kms(plane_state->plane->dev,
+ "Invalid scaling of plane\n");
+ drm_rect_debug_print("src: ", &plane_state->src, true);
+ drm_rect_debug_print("dst: ", &plane_state->dst, false);
+ return -ERANGE;
+ }
- plane_state->visible = drm_rect_clip_scaled(src, dst, &clip);
+ if (crtc_state->enable)
+ drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
- drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+ plane_state->visible = drm_rect_clip_scaled(src, dst, &clip);
+ drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+ } else if (drm_plane_solid_fill_enabled(plane_state)) {
+ plane_state->visible = true;
+ }
if (!plane_state->visible)
/*
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 536a0b0091c3..6d97f38ac1f6 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -256,8 +256,8 @@ drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state,
* Anything else should be considered a bug in the atomic core, so we
* gently warn about it.
*/
- WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) ||
- (new_plane_state->crtc != NULL && new_plane_state->fb == NULL));
+ WARN_ON((new_plane_state->crtc == NULL && drm_plane_has_visible_data(new_plane_state)) ||
+ (new_plane_state->crtc != NULL && !drm_plane_has_visible_data(new_plane_state)));
return old_plane_state->crtc && !new_plane_state->crtc;
}
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 6171fb1a0b47..56b7a0acce74 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -992,6 +992,35 @@ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
#define drm_for_each_plane(plane, dev) \
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
+/**
+ * drm_plane_solid_fill_enabled - Check if solid fill is enabled on plane
+ * @state: plane state
+ *
+ * Returns:
+ * Whether the plane has been assigned a solid_fill_blob
+ */
+static inline bool drm_plane_solid_fill_enabled(struct drm_plane_state *state)
+{
+ if (!state)
+ return false;
+ return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_SOLID_FILL && state->solid_fill_blob;
+}
+
+static inline bool drm_plane_has_visible_data(const struct drm_plane_state *state)
+{
+ switch (state->pixel_source) {
+ case DRM_PLANE_PIXEL_SOURCE_NONE:
+ return false;
+ case DRM_PLANE_PIXEL_SOURCE_SOLID_FILL:
+ return state->solid_fill_blob != NULL;
+ case DRM_PLANE_PIXEL_SOURCE_FB:
+ default:
+ WARN_ON(state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB);
+ }
+
+ return state->fb != NULL;
+}
+
bool drm_any_plane_has_format(struct drm_device *dev,
u32 format, u64 modifier);
--
2.42.0
Add support for pixel_source property to drm_plane and related
documentation. In addition, force pixel_source to
DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break
legacy userspace.
This enum property will allow user to specify a pixel source for the
plane. Possible pixel sources will be defined in the
drm_plane_pixel_source enum.
Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the
default value) and DRM_PLANE_PIXEL_SOURCE_NONE.
Acked-by: Dmitry Baryshkov <[email protected]>
Acked-by: Pekka Paalanen <[email protected]>
Acked-by: Harry Wentland <[email protected]>
Acked-by: Sebastian Wick <[email protected]>
Acked-by: Simon Ser <[email protected]>
Signed-off-by: Jessica Zhang <[email protected]>
---
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
drivers/gpu/drm/drm_blend.c | 94 +++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_plane.c | 19 +++++--
include/drm/drm_blend.h | 2 +
include/drm/drm_plane.h | 21 +++++++
6 files changed, 137 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..01638c51ce0a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+ plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
if (plane->color_encoding_property) {
if (!drm_object_property_get_default_value(&plane->base,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 98d3b10c08ae..46c78b87803d 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -562,6 +562,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) {
@@ -634,6 +636,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..fce734cdb85b 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -185,6 +185,25 @@
* 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.
+ *
+ * For non-framebuffer sources, if pixel_source is set to a non-framebuffer
+ * and non-NONE source, and the corresponding source property is NULL, then
+ * the atomic commit should return an error.
+ *
+ * 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 +634,78 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
return 0;
}
EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+static const struct drm_prop_enum_list drm_pixel_source_enum_list[] = {
+ { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
+ { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
+};
+
+/**
+ * drm_plane_create_pixel_source_property - create a new pixel source property
+ * @plane: DRM plane
+ * @extra_sources: Bitmask of additional supported pixel_sources for the driver.
+ * DRM_PLANE_PIXEL_SOURCE_FB and DRM_PLANE_PIXEL_SOURCE_NONE will
+ * always be enabled as supported sources.
+ *
+ * This creates a new property describing the current source of pixel data for the
+ * plane. The pixel_source will be initialized as DRM_PLANE_PIXEL_SOURCE_FB by default.
+ *
+ * Drivers can set a custom default source by overriding the pixel_source value in
+ * drm_plane_funcs.reset()
+ *
+ * The property is exposed to userspace as an enumeration property called
+ * "pixel_source" and has the following enumeration values:
+ *
+ * "NONE":
+ * No active pixel source
+ *
+ * "FB":
+ * Framebuffer pixel source
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_plane_create_pixel_source_property(struct drm_plane *plane,
+ unsigned long extra_sources)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_property *prop;
+ static const unsigned int valid_source_mask = BIT(DRM_PLANE_PIXEL_SOURCE_FB) |
+ BIT(DRM_PLANE_PIXEL_SOURCE_NONE);
+ int i;
+
+ /* FB is supported by default */
+ unsigned long supported_sources = extra_sources |
+ BIT(DRM_PLANE_PIXEL_SOURCE_FB) |
+ BIT(DRM_PLANE_PIXEL_SOURCE_NONE);
+
+ if (WARN_ON(supported_sources & ~valid_source_mask))
+ return -EINVAL;
+
+ prop = drm_property_create(dev, DRM_MODE_PROP_ENUM | DRM_MODE_PROP_ATOMIC, "pixel_source",
+ hweight32(supported_sources));
+
+ if (!prop)
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(drm_pixel_source_enum_list); i++) {
+ int ret;
+
+ if (!test_bit(drm_pixel_source_enum_list[i].type, &supported_sources))
+ continue;
+
+ ret = drm_property_add_enum(prop, drm_pixel_source_enum_list[i].type,
+ drm_pixel_source_enum_list[i].name);
+ if (ret) {
+ drm_property_destroy(dev, prop);
+
+ return ret;
+ }
+ }
+
+ drm_object_attach_property(&plane->base, prop, DRM_PLANE_PIXEL_SOURCE_FB);
+ plane->pixel_source_property = prop;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_pixel_source_property);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 24e7998d1731..559d101162ba 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -839,6 +839,14 @@ bool drm_any_plane_has_format(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_any_plane_has_format);
+static bool drm_plane_needs_disable(struct drm_plane_state *state, struct drm_framebuffer *fb)
+{
+ if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_NONE)
+ return true;
+
+ return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb == NULL;
+}
+
/*
* __setplane_internal - setplane handler for internal callers
*
@@ -861,8 +869,8 @@ static int __setplane_internal(struct drm_plane *plane,
WARN_ON(drm_drv_uses_atomic_modeset(plane->dev));
- /* No fb means shut it down */
- if (!fb) {
+ /* No visible data means shut it down */
+ if (drm_plane_needs_disable(plane->state, fb)) {
plane->old_fb = plane->fb;
ret = plane->funcs->disable_plane(plane, ctx);
if (!ret) {
@@ -913,8 +921,8 @@ static int __setplane_atomic(struct drm_plane *plane,
WARN_ON(!drm_drv_uses_atomic_modeset(plane->dev));
- /* No fb means shut it down */
- if (!fb)
+ /* No visible data means shut it down */
+ if (drm_plane_needs_disable(plane->state, fb))
return plane->funcs->disable_plane(plane, ctx);
/*
@@ -987,6 +995,9 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
return -ENOENT;
}
+ if (plane->state)
+ plane->state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
+
if (plane_req->fb_id) {
fb = drm_framebuffer_lookup(dev, file_priv, plane_req->fb_id);
if (!fb) {
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 88bdfec3bd88..122bbfbaae33 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -58,4 +58,6 @@ 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_pixel_source_property(struct drm_plane *plane,
+ unsigned long extra_sources);
#endif
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 79d62856defb..0b24fae36304 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -40,6 +40,12 @@ enum drm_scaling_filter {
DRM_SCALING_FILTER_NEAREST_NEIGHBOR,
};
+enum drm_plane_pixel_source {
+ DRM_PLANE_PIXEL_SOURCE_NONE,
+ DRM_PLANE_PIXEL_SOURCE_FB,
+ DRM_PLANE_PIXEL_SOURCE_MAX
+};
+
/**
* struct drm_plane_state - mutable plane state
*
@@ -116,6 +122,14 @@ struct drm_plane_state {
/** @src_h: height of visible portion of plane (in 16.16) */
uint32_t src_h, src_w;
+ /**
+ * @pixel_source:
+ *
+ * Source of pixel information for the plane. See
+ * drm_plane_create_pixel_source_property() for more details.
+ */
+ enum drm_plane_pixel_source pixel_source;
+
/**
* @alpha:
* Opacity of the plane with 0 as completely transparent and 0xffff as
@@ -699,6 +713,13 @@ struct drm_plane {
*/
struct drm_plane_state *state;
+ /*
+ * @pixel_source_property:
+ * Optional pixel_source property for this plane. See
+ * drm_plane_create_pixel_source_property().
+ */
+ struct drm_property *pixel_source_property;
+
/**
* @alpha_property:
* Optional alpha property for this plane. See
--
2.42.0
On Sat, 28 Oct 2023 at 01:33, Jessica Zhang <[email protected]> wrote:
>
> Loosen the requirements for atomic and legacy commit so that, in cases
> where pixel_source != FB, the commit can still go through.
>
> This includes adding framebuffer NULL checks in other areas to account for
> FB being NULL when non-FB pixel sources are enabled.
>
> To disable a plane, the pixel_source must be NONE or the FB must be NULL
> if pixel_source == FB.
>
> Signed-off-by: Jessica Zhang <[email protected]>
> ---
> drivers/gpu/drm/drm_atomic.c | 21 ++++++++++----------
> drivers/gpu/drm/drm_atomic_helper.c | 39 +++++++++++++++++++++----------------
> include/drm/drm_atomic_helper.h | 4 ++--
> include/drm/drm_plane.h | 29 +++++++++++++++++++++++++++
> 4 files changed, 64 insertions(+), 29 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote:
> Some drivers support hardware that have optimizations for solid fill
> planes. This series aims to expose these capabilities to userspace as
> some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
> hardware composer HAL) that can be set by apps like the Android Gears
> test app.
>
> In order to expose this capability to userspace, this series will:
>
> [...]
Applied to drm-misc-next, thanks!
[01/10] drm: Introduce pixel_source DRM plane property
commit: e50e5fed41c7eed2db4119645bf3480ec43fec11
[02/10] drm: Introduce solid fill DRM plane property
commit: 85863a4e16e77079ee14865905ddc3ef9483a640
[03/10] drm: Add solid fill pixel source
commit: 4b64167042927531f4cfaf035b8f88c2f7a05f06
[04/10] drm/atomic: Add pixel source to plane state dump
commit: 8283ac7871a959848e09fc6593b8c12b8febfee6
[05/10] drm/atomic: Add solid fill data to plane state dump
commit: e86413f5442ee094e66b3e75f2d3419ed0df9520
[06/10] drm/atomic: Move framebuffer checks to helper
commit: 4ba6b7a646321e740c7f2d80c90505019c4e8fce
[07/10] drm/atomic: Loosen FB atomic checks
commit: f1e75da5364e780905d9cd6043f9c74cdcf84073
Best regards,
--
Dmitry Baryshkov <[email protected]>
On Saturday, December 2nd, 2023 at 22:41, Dmitry Baryshkov <[email protected]> wrote:
> On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote:
>
> > Some drivers support hardware that have optimizations for solid fill
> > planes. This series aims to expose these capabilities to userspace as
> > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
> > hardware composer HAL) that can be set by apps like the Android Gears
> > test app.
> >
> > In order to expose this capability to userspace, this series will:
> >
> > [...]
>
>
> Applied to drm-misc-next, thanks!
Where are the IGT and userspace for this uAPI addition?
On Sun, 3 Dec 2023 at 14:15, Simon Ser <[email protected]> wrote:
>
> On Saturday, December 2nd, 2023 at 22:41, Dmitry Baryshkov <[email protected]> wrote:
>
> > On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote:
> >
> > > Some drivers support hardware that have optimizations for solid fill
> > > planes. This series aims to expose these capabilities to userspace as
> > > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
> > > hardware composer HAL) that can be set by apps like the Android Gears
> > > test app.
> > >
> > > In order to expose this capability to userspace, this series will:
> > >
> > > [...]
> >
> >
> > Applied to drm-misc-next, thanks!
>
> Where are the IGT and userspace for this uAPI addition?
Indeed. I checked that there are uABI acks/reviews, but I didn't check
these requirements. Frankly speaking, I thought that they were handled
already, before giving the ack. How should we proceed? Should I land a
revert?
--
With best wishes
Dmitry
On Sun, Dec 03, 2023 at 08:10:31PM +0200, Dmitry Baryshkov wrote:
> On Sun, 3 Dec 2023 at 14:15, Simon Ser <[email protected]> wrote:
> >
> > On Saturday, December 2nd, 2023 at 22:41, Dmitry Baryshkov <[email protected]> wrote:
> >
> > > On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote:
> > >
> > > > Some drivers support hardware that have optimizations for solid fill
> > > > planes. This series aims to expose these capabilities to userspace as
> > > > some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
> > > > hardware composer HAL) that can be set by apps like the Android Gears
> > > > test app.
> > > >
> > > > In order to expose this capability to userspace, this series will:
> > > >
> > > > [...]
> > >
> > >
> > > Applied to drm-misc-next, thanks!
> >
> > Where are the IGT and userspace for this uAPI addition?
>
> Indeed. I checked that there are uABI acks/reviews, but I didn't check
> these requirements. Frankly speaking, I thought that they were handled
> already, before giving the ack. How should we proceed? Should I land a
> revert?
Yes
Maxime
Hi Simon
On 12/3/2023 4:15 AM, Simon Ser wrote:
> On Saturday, December 2nd, 2023 at 22:41, Dmitry Baryshkov <[email protected]> wrote:
>
>> On Fri, 27 Oct 2023 15:32:50 -0700, Jessica Zhang wrote:
>>
>>> Some drivers support hardware that have optimizations for solid fill
>>> planes. This series aims to expose these capabilities to userspace as
>>> some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
>>> hardware composer HAL) that can be set by apps like the Android Gears
>>> test app.
>>>
>>> In order to expose this capability to userspace, this series will:
>>>
>>> [...]
>>
>>
>> Applied to drm-misc-next, thanks!
>
> Where are the IGT and userspace for this uAPI addition?
Yes, we made IGT changes to test and validate this. We will post them on
the IGT dev list shortly and CC you.
We do not have a compositor change yet for this as we primarily used IGT
to validate this.
Can we re-try to land this once IGT changes are accepted?
Thanks
Abhinav
On Monday, December 4th, 2023 at 18:51, Abhinav Kumar <[email protected]> wrote:
> > Where are the IGT and userspace for this uAPI addition?
>
> Yes, we made IGT changes to test and validate this. We will post them on
> the IGT dev list shortly and CC you.
>
> We do not have a compositor change yet for this as we primarily used IGT
> to validate this.
Yes, please post the IGT.
> Can we re-try to land this once IGT changes are accepted?
There will need to be a user-space implementation as well, since this is
a hard requirement for new uAPI [1]. Maybe I'll give this a go if I have
time.
[1]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
On 12/4/2023 9:57 AM, Simon Ser wrote:
> On Monday, December 4th, 2023 at 18:51, Abhinav Kumar <[email protected]> wrote:
>
>>> Where are the IGT and userspace for this uAPI addition?
>>
>> Yes, we made IGT changes to test and validate this. We will post them on
>> the IGT dev list shortly and CC you.
>>
>> We do not have a compositor change yet for this as we primarily used IGT
>> to validate this.
>
> Yes, please post the IGT.
>
>> Can we re-try to land this once IGT changes are accepted?
>
> There will need to be a user-space implementation as well, since this is
> a hard requirement for new uAPI [1]. Maybe I'll give this a go if I have
> time.
>
Much appreciated.
> [1]: https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
Hi,
On Fri, Oct 27, 2023 at 03:32:57PM -0700, Jessica Zhang wrote:
> Loosen the requirements for atomic and legacy commit so that, in cases
> where pixel_source != FB, the commit can still go through.
>
> This includes adding framebuffer NULL checks in other areas to account for
> FB being NULL when non-FB pixel sources are enabled.
>
> To disable a plane, the pixel_source must be NONE or the FB must be NULL
> if pixel_source == FB.
>
> Signed-off-by: Jessica Zhang <[email protected]>
This breaks some plane kunit tests we have:
See https://lore.kernel.org/dri-devel/[email protected]/
Maxime