Re: [Freedreno] [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property
On Tue, 29 Aug 2023 at 02:45, Jessica Zhang wrote:
>
>
>
> On 8/8/2023 3:57 PM, Jessica Zhang wrote:
> >
> >
> > On 8/7/2023 6:07 PM, Dmitry Baryshkov wrote:
> >>
> >>
> >> 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.
> >
> > 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".
>
> Hi Dmitry,
>
> Since it looks like there's still some ongoing discussion with Pekka
> about whether to support an RGBA solid fill source, I'll just leave a
> note to add an RGBA source in the future.
Sure! Let's get the basic framework landed. After that we can extend
it with RGBA, if that seems sensible.
>
> Thanks,
>
> Jessica Zhang
>
> >
> > Thanks,
> >
> > Jessica Zhang
> >
> >>
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Jessica Zhang
> >>>
>
> --
> With best wishes
> Dmitry
> >>
> >> --
> >> With best wishes
> >> Dmitry
--
With best wishes
Dmitry
[Freedreno] [PATCH RFC v6 08/10] drm/msm/dpu: Allow NULL FBs in atomic commit
Since solid fill planes allow for a NULL framebuffer in a valid commit,
add NULL framebuffer checks to atomic commit calls within DPU.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 41 ---
2 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 8ce7586e2ddf..5e845510e8c1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -451,6 +451,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
struct drm_plane_state *state;
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
struct dpu_plane_state *pstate = NULL;
+ const struct msm_format *fmt;
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
@@ -470,7 +471,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
- format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
+ if (drm_plane_solid_fill_enabled(state))
+ fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base,
+ DRM_FORMAT_ABGR, 0);
+ else
+ fmt = msm_framebuffer_format(pstate->base.fb);
+
+ format = to_dpu_format(fmt);
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..114c803ff99b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -837,8 +837,13 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
pipe_cfg->dst_rect = new_plane_state->dst;
- fb_rect.x2 = new_plane_state->fb->width;
- fb_rect.y2 = new_plane_state->fb->height;
+ if (drm_plane_solid_fill_enabled(new_plane_state))
+ return 0;
+
+ if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB &&
new_plane_state->fb) {
+ fb_rect.x2 = new_plane_state->fb->width;
+ fb_rect.y2 = new_plane_state->fb->height;
+ }
/* Ensure fb size is supported */
if (drm_rect_width(&fb_rect) > MAX_IMG_WIDTH ||
@@ -1082,21 +1087,32 @@ static void dpu_plane_sspp_atomic_update(struct
drm_plane *plane)
struct drm_crtc *crtc = state->crtc;
struct drm_framebuffer *fb = state->fb;
bool is_rt_pipe;
- const struct dpu_format *fmt =
- to_dpu_format(msm_framebuffer_format(fb));
+ const struct dpu_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
struct msm_gem_address_space *aspace = kms->base.aspace;
struct dpu_hw_fmt_layout layout;
bool layout_valid = false;
- int ret;
- ret = dpu_format_populate_layout(aspace, fb, &layout);
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else
- layout_valid = true;
+ if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) {
+ int ret;
+
+ fmt = to_dpu_format(msm_framebuffer_format(fb));
+
+ ret = dpu_format_populate_layout(aspace, fb, &layout);
+ if (ret)
+ DPU_ERROR_PLANE(pdpu, "failed to get format layout,
%d\n", ret);
+ else
+ layout_valid = true;
+
+ DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
DRM_RECT_FMT
+ ", %4.4s ubwc %d\n", fb->base.id,
DRM_RECT_FP_ARG(&state->src),
+ crtc->base.id, DRM_RECT_ARG(&state->dst),
+ (char *)&fmt->base.pixel_format,
DPU_FORMAT_IS_UBWC(fmt));
+ } else {
+ fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+ }
pstate->pending = true;
@@ -1104,11 +1120,6 @@ static void dpu_plane_sspp_atomic_update(struct
drm_plane *plane)
pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
pdpu->is_rt_pipe = is_rt_pipe;
- DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
- ", %4.4s ubwc %d\n", fb->base.id,
DRM_RECT_FP_ARG(&state->src),
- crtc->base.id, DRM_RECT_ARG(&state->dst),
- (char *)&fmt->base.pixel_format,
DPU_FORMAT_IS_UBWC(fmt));
-
dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
[Freedreno] [PATCH RFC v6 01/10] drm: Introduce pixel_source DRM plane property
Add support for pixel_source property to drm_plane and related
documentation. In addition, force pixel_source to
DRM_PLANE_PIXEL_SOURCE_FB in DRM_IOCTL_MODE_SETPLANE as to not break
legacy userspace.
This enum property will allow user to specify a pixel source for the
plane. Possible pixel sources will be defined in the
drm_plane_pixel_source enum.
Currently, the only pixel sources are DRM_PLANE_PIXEL_SOURCE_FB (the
default value) and DRM_PLANE_PIXEL_SOURCE_NONE.
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
drivers/gpu/drm/drm_blend.c | 90 +++
drivers/gpu/drm/drm_plane.c | 19 +--
include/drm/drm_blend.h | 2 +
include/drm/drm_plane.h | 21
6 files changed, 133 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..01638c51ce0a 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct
drm_plane_state *plane_state,
plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+ plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
if (plane->color_encoding_property) {
if (!drm_object_property_get_default_value(&plane->base,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index d867e7f9f2cd..454f980e16c9 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -544,6 +544,8 @@ static int drm_atomic_plane_set_property(struct drm_plane
*plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
+ } else if (property == plane->pixel_source_property) {
+ state->pixel_source = val;
} else if (property == plane->alpha_property) {
state->alpha = val;
} else if (property == plane->blend_mode_property) {
@@ -616,6 +618,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
+ } else if (property == plane->pixel_source_property) {
+ *val = state->pixel_source;
} else if (property == plane->alpha_property) {
*val = state->alpha;
} else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 6e74de833466..c3c57bae06b7 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -185,6 +185,21 @@
* plane does not expose the "alpha" property, then this is
* assumed to be 1.0
*
+ * pixel_source:
+ * pixel_source is set up with drm_plane_create_pixel_source_property().
+ * It is used to toggle the active source of pixel data for the plane.
+ * The plane will only display data from the set pixel_source -- any
+ * data from other sources will be ignored.
+ *
+ * Possible values:
+ *
+ * "NONE":
+ * No active pixel source.
+ * Committing with a NONE pixel source will disable the plane.
+ *
+ * "FB":
+ * Framebuffer source set by the "FB_ID" property.
+ *
* Note that all the property extensions described here apply either to the
* plane or the CRTC (e.g. for the background color, which currently is not
* exposed and assumed to be black).
@@ -615,3 +630,78 @@ int drm_plane_create_blend_mode_property(struct drm_plane
*plane,
return 0;
}
EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+static const struct drm_prop_enum_list drm_pixel_source_enum_list[] = {
+ { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
+ { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
+};
+
+/**
+ * drm_plane_create_pixel_source_property - create a new pixel source property
+ * @plane: DRM plane
+ * @extra_sources: Bitmask of additional supported pixel_sources for the
driver.
+ *DRM_PLANE_PIXEL_SOURCE_FB and DRM_PLANE_PIXEL_SOURCE_NONE
will
+ *always be enabled as supported sources.
+ *
+ * This creates a new property describing the current source of pixel data for
the
+ * plane. The pixel_source will be initialized as DRM_PLANE_PIXEL_SOURCE_FB by
default.
+ *
+ * Drivers can set a custom default source by overriding the pixel_source
value in
+ * drm_plane_funcs.reset()
+ *
+ * The property is exposed to userspace as an enumeration property called
+ * "pixel_source" and has the following enumeration values:
+ *
+ * "NONE":
+ * No active pixel source
+ *
+ * "FB":
+ * Framebuffer pixel source
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+
[Freedreno] [PATCH RFC v6 10/10] drm/msm/dpu: Add solid fill and pixel source properties
Add solid_fill and pixel_source properties to DPU plane
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 639ecbeeacf8..fda20c5e43b6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1453,6 +1453,8 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
DPU_ERROR("failed to install zpos property, rc = %d\n", ret);
drm_plane_create_alpha_property(plane);
+ drm_plane_create_solid_fill_property(plane);
+ drm_plane_create_pixel_source_property(plane,
BIT(DRM_PLANE_PIXEL_SOURCE_SOLID_FILL));
drm_plane_create_blend_mode_property(plane,
BIT(DRM_MODE_BLEND_PIXEL_NONE) |
BIT(DRM_MODE_BLEND_PREMULTI) |
--
2.42.0
[Freedreno] [PATCH RFC v6 06/10] drm/atomic: Move framebuffer checks to helper
Currently framebuffer checks happen directly in
drm_atomic_plane_check(). Move these checks into their own helper
method.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/drm_atomic.c | 130 ---
1 file changed, 73 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3cb599b3304a..cc0e93d19e15 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -580,6 +580,76 @@ plane_switching_crtc(const struct drm_plane_state
*old_plane_state,
return true;
}
+static int drm_atomic_plane_check_fb(const struct drm_plane_state *state)
+{
+ struct drm_plane *plane = state->plane;
+ const struct drm_framebuffer *fb = state->fb;
+ struct drm_mode_rect *clips;
+
+ uint32_t num_clips;
+ unsigned int fb_width, fb_height;
+ int ret;
+
+ /* Check whether this plane supports the fb pixel format. */
+ ret = drm_plane_check_pixel_format(plane, fb->format->format,
+ fb->modifier);
+
+ if (ret) {
+ drm_dbg_atomic(plane->dev,
+ "[PLANE:%d:%s] invalid pixel format %p4cc,
modifier 0x%llx\n",
+ plane->base.id, plane->name,
+ &fb->format->format, fb->modifier);
+ return ret;
+ }
+
+ fb_width = fb->width << 16;
+ fb_height = fb->height << 16;
+
+ /* Make sure source coordinates are inside the fb. */
+ if (state->src_w > fb_width ||
+ state->src_x > fb_width - state->src_w ||
+ state->src_h > fb_height ||
+ state->src_y > fb_height - state->src_h) {
+ drm_dbg_atomic(plane->dev,
+ "[PLANE:%d:%s] invalid source coordinates "
+ "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
+ plane->base.id, plane->name,
+ state->src_w >> 16,
+ ((state->src_w & 0x) * 15625) >> 10,
+ state->src_h >> 16,
+ ((state->src_h & 0x) * 15625) >> 10,
+ state->src_x >> 16,
+ ((state->src_x & 0x) * 15625) >> 10,
+ state->src_y >> 16,
+ ((state->src_y & 0x) * 15625) >> 10,
+ fb->width, fb->height);
+ return -ENOSPC;
+ }
+
+ clips = __drm_plane_get_damage_clips(state);
+ num_clips = drm_plane_get_damage_clips_count(state);
+
+ /* Make sure damage clips are valid and inside the fb. */
+ while (num_clips > 0) {
+ if (clips->x1 >= clips->x2 ||
+ clips->y1 >= clips->y2 ||
+ clips->x1 < 0 ||
+ clips->y1 < 0 ||
+ clips->x2 > fb_width ||
+ clips->y2 > fb_height) {
+ drm_dbg_atomic(plane->dev,
+ "[PLANE:%d:%s] invalid damage clip %d %d
%d %d\n",
+ plane->base.id, plane->name, clips->x1,
+ clips->y1, clips->x2, clips->y2);
+ return -EINVAL;
+ }
+ clips++;
+ num_clips--;
+ }
+
+ return 0;
+}
+
/**
* drm_atomic_plane_check - check plane state
* @old_plane_state: old plane state to check
@@ -596,9 +666,6 @@ 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;
- unsigned int fb_width, fb_height;
- struct drm_mode_rect *clips;
- uint32_t num_clips;
int ret;
/* either *both* CRTC and FB must be set, or neither */
@@ -625,17 +692,6 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
return -EINVAL;
}
- /* Check whether this plane supports the fb pixel format. */
- ret = drm_plane_check_pixel_format(plane, fb->format->format,
- fb->modifier);
- if (ret) {
- drm_dbg_atomic(plane->dev,
- "[PLANE:%d:%s] invalid pixel format %p4cc,
modifier 0x%llx\n",
- plane->base.id, plane->name,
- &fb->format->format, fb->modifier);
- return ret;
- }
-
/* Give drivers some help against integer overflows */
if (new_plane_state->crtc_w > INT_MAX ||
new_plane_state->crtc_x > INT_MAX - (int32_t)
new_plane_state->crtc_w ||
@@ -649,50 +705,10 @@ static in
[Freedreno] [PATCH RFC v6 04/10] drm/atomic: Add pixel source to plane state dump
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_blend.c | 1 +
drivers/gpu/drm/drm_crtc_internal.h | 1 +
3 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index b4c6ffc438da..bcecb64ccfad 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_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_blend.c b/drivers/gpu/drm/drm_blend.c
index 1016a206ca0c..3d484aa1e029 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -643,6 +643,7 @@ static const struct drm_prop_enum_list
drm_pixel_source_enum_list[] = {
{ DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
{ DRM_PLANE_PIXEL_SOURCE_SOLID_FILL, "SOLID_FILL" },
};
+DRM_ENUM_NAME_FN(drm_get_pixel_source_name, drm_pixel_source_enum_list);
/**
* drm_plane_create_pixel_source_property - create a new pixel source property
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 501a10edd0e1..7bc93ba449d5 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -267,6 +267,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_get_pixel_source_name(int val);
/* drm_bridge.c */
void drm_bridge_detach(struct drm_bridge *bridge);
--
2.42.0
[Freedreno] [PATCH RFC v6 09/10] drm/msm/dpu: Use DRM solid_fill property
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 | 37 +--
1 file changed, 25 insertions(+), 12 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..639ecbeeacf8 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_DEFAULT2
-#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,19 +604,35 @@ 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_abgr_fill_color(struct drm_plane_state *state)
+{
+ struct drm_solid_fill solid_fill = state->solid_fill;
+
+ uint32_t ret = 0;
+ uint8_t a = state->alpha & 0xFF;
+ uint8_t b = solid_fill.b >> 24;
+ uint8_t g = solid_fill.g >> 24;
+ uint8_t r = solid_fill.r >> 24;
+
+ ret |= a << 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
* @color: RGB fill color value, [23..16] Blue, [15..8] Green, [7..0] Red
* @alpha: 8-bit fill alpha value, 255 selects 100% alpha
*/
-static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
- uint32_t color, uint32_t alpha)
+static void _dpu_plane_color_fill(struct dpu_plane *pdpu, uint32_t color)
{
const struct dpu_format *fmt;
const struct drm_plane *plane = &pdpu->base;
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
- u32 fill_color = (color & 0xFF) | ((alpha & 0xFF) << 24);
DPU_DEBUG_PLANE(pdpu, "\n");
@@ -633,11 +647,11 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
/* update sspp */
_dpu_plane_color_fill_pipe(pstate, &pstate->pipe,
&pstate->pipe_cfg.dst_rect,
- fill_color, fmt);
+ color, fmt);
if (pstate->r_pipe.sspp)
_dpu_plane_color_fill_pipe(pstate, &pstate->r_pipe,
&pstate->r_pipe_cfg.dst_rect,
- fill_color, fmt);
+ color, fmt);
}
static int dpu_plane_prepare_fb(struct drm_plane *plane,
@@ -976,10 +990,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);
+ _dpu_plane_color_fill(pdpu, 0x);
+ else if (drm_plane_solid_fill_enabled(plane->state))
+ _dpu_plane_color_fill(pdpu,
_dpu_plane_get_abgr_fill_color(plane->state));
else {
dpu_plane_flush_csc(pdpu, &pstate->pipe);
dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
@@ -1024,7 +1037,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 */
--
2.42.0
[Freedreno] [PATCH RFC v6 03/10] drm: Add solid fill pixel source
Add "SOLID_FILL" as a valid pixel source. If the pixel_source property is
set to "SOLID_FILL", it will display data from the drm_plane "solid_fill"
blob property.
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/drm_blend.c | 10 +-
include/drm/drm_plane.h | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 273021cc21c8..1016a206ca0c 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -200,6 +200,9 @@
* "FB":
* Framebuffer source set by the "FB_ID" property.
*
+ * "SOLID_FILL":
+ * Solid fill color source set by the "solid_fill" 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.
@@ -638,6 +641,7 @@ 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_PIXEL_SOURCE_SOLID_FILL, "SOLID_FILL" },
};
/**
@@ -662,6 +666,9 @@ static const struct drm_prop_enum_list
drm_pixel_source_enum_list[] = {
* "FB":
* Framebuffer pixel source
*
+ * "SOLID_FILL":
+ * Solid fill color pixel source
+ *
* Returns:
* Zero on success, negative errno on failure.
*/
@@ -671,7 +678,8 @@ int drm_plane_create_pixel_source_property(struct drm_plane
*plane,
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);
+
BIT(DRM_PLANE_PIXEL_SOURCE_NONE) |
+
BIT(DRM_PLANE_PIXEL_SOURCE_SOLID_FILL);
int i;
/* FB is supported by default */
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index a38e18bfb43e..49995c4be2ab 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -43,6 +43,7 @@ enum drm_scaling_filter {
enum drm_plane_pixel_source {
DRM_PLANE_PIXEL_SOURCE_NONE,
DRM_PLANE_PIXEL_SOURCE_FB,
+ DRM_PLANE_PIXEL_SOURCE_SOLID_FILL,
DRM_PLANE_PIXEL_SOURCE_MAX
};
--
2.42.0
[Freedreno] [PATCH RFC v6 00/10] Support for Solid Fill Planes
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 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 appropriate version number and solid fill color (in RGB323232 format) and and setting the pixel_source property to DRM_PLANE_PIXEL_SOURCE_COLOR. This will disable memory fetch and the resulting plane will display the color specified by the solid_fill blob. 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 creating additional versions of the drm_solid_fill struct. 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 v6: - Have _dpu_plane_color_fill() take in a single ABGR 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/20230728-solid-fill-v5-0-053dbefa9...@quicinc.com 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 ABGR/RGBA 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/20230404-solid-fill-v4-0-f4ec0caa7...@quicinc.com 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/20230104234036.636-1-quic_jessz...@quicinc.com 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_CO
[Freedreno] [PATCH RFC v6 05/10] drm/atomic: Add solid fill data to plane state dump
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 | 8
include/drm/drm_plane.h | 3 +++
3 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index bcecb64ccfad..3cb599b3304a 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 559d101162ba..6244b622a21a 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1495,6 +1495,14 @@ __drm_plane_get_damage_clips(const struct
drm_plane_state *state)
state->fb_damage_clips->data : NULL);
}
+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 49995c4be2ab..a58f84b6bd5e 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -1001,6 +1001,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);
--
2.42.0
[Freedreno] [PATCH RFC v6 02/10] drm: Introduce solid fill DRM plane property
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 r, g, b;
};
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/drm_atomic_state_helper.c | 9
drivers/gpu/drm/drm_atomic_uapi.c | 26 ++
drivers/gpu/drm/drm_blend.c | 30 ++
include/drm/drm_blend.h | 1 +
include/drm/drm_plane.h | 36 +++
include/uapi/drm/drm_mode.h | 24 +
6 files changed, 126 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..1cae596ab693 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -316,6 +316,20 @@ drm_atomic_set_crtc_for_connector(struct
drm_connector_state *conn_state,
}
EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
+static void drm_atomic_set_solid_fill_prop(struct drm_plane_state *state)
+{
+ struct drm_mode_solid_fill *user_info;
+
+ if (!state->solid_fill_blob)
+ return;
+
+ user_info = (struct drm_mode_solid_fill *)state->solid_fill_blob->data;
+
+ state->solid_fill.r = user_info->r;
+ state->solid_fill.g = user_info->g;
+ state->solid_fill.b = user_info->b;
+}
+
static void set_out_fence_for_crtc(struct drm_atomic_state *state,
struct drm_crtc *crtc, s32 __user *fence_ptr)
{
@@ -546,6 +560,15 @@ 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) {
+ ret = drm_atomic_replace_property_blob_from_id(dev,
+ &state->solid_fill_blob,
+ val, sizeof(struct drm_mode_solid_fill),
+ -1, &replaced);
+ if (ret)
+ return ret;
+
+ drm_atomic_set_solid_fill_prop(state);
} else if (property == plane->alpha_property) {
state->alpha = val;
} else if (property == plane->blend_mode_property) {
@@ -620,6 +643,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 c3c57bae06b7..273021cc21c8 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
+
[Freedreno] [PATCH RFC v6 07/10] drm/atomic: Loosen FB atomic checks
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
---
drivers/gpu/drm/drm_atomic.c| 20 +++-
drivers/gpu/drm/drm_atomic_helper.c | 36
include/drm/drm_atomic_helper.h | 4 ++--
include/drm/drm_plane.h | 29 +
4 files changed, 62 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index cc0e93d19e15..cdc6cfedd433 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -668,14 +668,14 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
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;
}
@@ -706,9 +706,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 && 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 41b8066f61ff..a176064ee27e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -864,7 +864,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 +881,29 @@ 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);
+ if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && 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_
Re: [Freedreno] [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property
On 8/8/2023 3:57 PM, Jessica Zhang wrote:
On 8/7/2023 6:07 PM, Dmitry Baryshkov wrote:
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.
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".
Hi Dmitry,
Since it looks like there's still some ongoing discussion with Pekka
about whether to support an RGBA solid fill source, I'll just leave a
note to add an RGBA source in the future.
Thanks,
Jessica Zhang
Thanks,
Jessica Zhang
Thanks,
Jessica Zhang
--
With best wishes
Dmitry
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v4 0/3] drm: simplify support for transparent DRM bridges
On Tue, 22 Aug 2023 at 17:19, Laurent Pinchart wrote: > > On Tue, Aug 22, 2023 at 05:17:37PM +0300, Laurent Pinchart wrote: > > Hi Dmitry, > > > > Thank you for the patches. > > > > On Thu, Aug 17, 2023 at 05:55:13PM +0300, Dmitry Baryshkov wrote: > > > Supporting DP/USB-C can result in a chain of several transparent > > > bridges (PHY, redrivers, mux, etc). This results in drivers having > > > similar boilerplate code for such bridges. > > > > What do you mean by transparent bridge here ? Bridges are a DRM concept, > > and as far as I can tell, a PHY isn't a bridge. Why does it need to be > > handled as one, especially if it's completely transparent ? > > > > > Next, these drivers are susceptible to -EPROBE_DEFER loops: the next > > > bridge can either be probed from the bridge->attach callback, when it is > > > too late to return -EPROBE_DEFER, or from the probe() callback, when the > > > next bridge might not yet be available, because it depends on the > > > resources provided by the probing device. > > > > Can't device links help avoiding defer probing in those cases ? > > > > > Last, but not least, this results in the the internal knowledge of DRM > > > subsystem slowly diffusing into other subsystems, like PHY or USB/TYPEC. > > > > Why so ? The PHY subsystem should provide a PHY, without considering > > what subsystem it will be used by. This patch series seems to me to > > actually create this DRM dependency in other subsystems, > > I was wrong on this one, there are indeed existing drm_bridge instances > in drivers/usb/ and drivers/phy/. That's certainly not nice. Why do we > even need drm_bridge there, why can't the PHYs be acquired by their > consumers in DRM (and anywhere else) using the PHY API ? During the design of the DT bindings for DisplayPort on these platforms we have evaluated different approaches. First approach was to add a special 'displayport' property to the USB-C connector, pointing to DP. This approach was declined by DT maintainers. Then we tried different approaches towards building connection graphs between different parties. Finally it was determined that the easiest way is to describe all USB-C signal paths properly. SS lines start at the PHY, then they pass through different muxes and retimers and then end up at the usb-c-connector. SS lines are muxed by the USB+DP PHY and switched between USB-3 (SuperSpeed) and DP. Then comes the question of binding everything together from the DRM point of view. The usb-c-connector is the logical place for the last drm_bridge, unfortunately. We need to send HPD events from the TypeC port driver (either directly or from the altmode driver). Then either we have to point the connector->fwnode to the DP port or build the full drm_bridge chain. Second path was selected, as it fits better into the overall framework. > > > which I don't > > think is a very good idea. Resources should be registered in their own > > subsystem with the appropriate API, not in a way that is tied to a > > particular consumer. > > > > > To solve all these issues, define a separate DRM helper, which creates > > > separate aux device just for the bridge. During probe such aux device > > > doesn't result in the EPROBE_DEFER loops. Instead it allows the device > > > drivers to probe properly, according to the actual resource > > > dependencies. The bridge auxdevs are then probed when the next bridge > > > becomes available, sparing drivers from drm_bridge_attach() returning > > > -EPROBE_DEFER. > > > > I'm not thrilled :-( Let's discuss the questions above first. > > > > > Proposed merge strategy: immutable branch with the drm commit, which is > > > then merged into PHY and USB subsystems together with the corresponding > > > patch. > > > > > > Changes since v3: > > > - Moved bridge driver to gpu/drm/bridge (Neil Armstrong) > > > - Renamed it to aux-bridge (since there is already a simple_bridge > > > driver) > > > - Made CONFIG_OF mandatory for this driver (Neil Armstrong) > > > - Added missing kfree and ida_free (Dan Carpenter) > > > > > > Changes since v2: > > > - ifdef'ed bridge->of_node access (LKP) > > > > > > Changes since v1: > > > - Added EXPORT_SYMBOL_GPL / MODULE_LICENSE / etc. to drm_simple_bridge > > > > > > Dmitry Baryshkov (3): > > > drm/bridge: add transparent bridge helper > > > phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE > > > usb: typec: nb7vpq904m: switch to DRM_AUX_BRIDGE > > > > > > drivers/gpu/drm/bridge/Kconfig| 9 ++ > > > drivers/gpu/drm/bridge/Makefile | 1 + > > > drivers/gpu/drm/bridge/aux-bridge.c | 132 ++ > > > drivers/phy/qualcomm/Kconfig | 2 +- > > > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 44 +--- > > > drivers/usb/typec/mux/Kconfig | 2 +- > > > drivers/usb/typec/mux/nb7vpq904m.c| 44 +--- > > > include/drm/bridge/aux-bridge.h | 19 > > > 8 files changed, 167 insertions(+), 86 deletions(-) > > > c
Re: [Freedreno] [PATCH v2 6/6] drm/drm-file: Allow size unit selection in drm_show_memory_stats
On Wed, Aug 23, 2023 at 6:36 PM Adrián Larumbe
wrote:
>
> The current implementation will try to pick the highest available
> unit. This is rather unflexible, and allowing drivers to display BO size
> statistics through fdinfo in units of their choice might be desirable.
>
> The new argument to drm_show_memory_stats is to be interpreted as the
> integer multiplier of a 10-power of 2, so 1 would give us size in Kib and 2
> in Mib. If we want drm-file functions to pick the highest unit, then 0
> should be passed.
>
> Signed-off-by: Adrián Larumbe
> ---
> drivers/gpu/drm/drm_file.c | 22 +-
> drivers/gpu/drm/msm/msm_drv.c | 2 +-
> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +-
> include/drm/drm_file.h | 5 +++--
> 4 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 762965e3d503..517e1fb8072a 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -873,7 +873,7 @@ void drm_send_event(struct drm_device *dev, struct
> drm_pending_event *e)
> EXPORT_SYMBOL(drm_send_event);
>
> static void print_size(struct drm_printer *p, const char *stat,
> - const char *region, u64 sz)
> + const char *region, u64 sz, unsigned int unit)
> {
> const char *units[] = {"", " KiB", " MiB"};
> unsigned u;
> @@ -881,6 +881,8 @@ static void print_size(struct drm_printer *p, const char
> *stat,
> for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
> if (sz < SZ_1K)
> break;
> + if (unit > 0 && unit == u)
> + break;
> sz = div_u64(sz, SZ_1K);
> }
>
> @@ -898,17 +900,18 @@ static void print_size(struct drm_printer *p, const
> char *stat,
> void drm_print_memory_stats(struct drm_printer *p,
> const struct drm_memory_stats *stats,
> enum drm_gem_object_status supported_status,
> - const char *region)
> + const char *region,
> + unsigned int unit)
I'm not really adverse to changing what units we use.. or perhaps
changing the threshold to go to higher units to be 1x or 10x
of the previous unit. But I'm less excited about having different
drivers using different units.
BR,
-R
> {
> - print_size(p, "total", region, stats->private + stats->shared);
> - print_size(p, "shared", region, stats->shared);
> - print_size(p, "active", region, stats->active);
> + print_size(p, "total", region, stats->private + stats->shared, unit);
> + print_size(p, "shared", region, stats->shared, unit);
> + print_size(p, "active", region, stats->active, unit);
>
> if (supported_status & DRM_GEM_OBJECT_RESIDENT)
> - print_size(p, "resident", region, stats->resident);
> + print_size(p, "resident", region, stats->resident, unit);
>
> if (supported_status & DRM_GEM_OBJECT_PURGEABLE)
> - print_size(p, "purgeable", region, stats->purgeable);
> + print_size(p, "purgeable", region, stats->purgeable, unit);
> }
> EXPORT_SYMBOL(drm_print_memory_stats);
>
> @@ -916,11 +919,12 @@ EXPORT_SYMBOL(drm_print_memory_stats);
> * drm_show_memory_stats - Helper to collect and show standard fdinfo memory
> stats
> * @p: the printer to print output to
> * @file: the DRM file
> + * @unit: multipliyer of power of two exponent of desired unit
> *
> * Helper to iterate over GEM objects with a handle allocated in the
> specified
> * file.
> */
> -void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file)
> +void drm_show_memory_stats(struct drm_printer *p, struct drm_file *file,
> unsigned int unit)
> {
> struct drm_gem_object *obj;
> struct drm_memory_stats status = {};
> @@ -967,7 +971,7 @@ void drm_show_memory_stats(struct drm_printer *p, struct
> drm_file *file)
> }
> spin_unlock(&file->table_lock);
>
> - drm_print_memory_stats(p, &status, supported_status, "memory");
> + drm_print_memory_stats(p, &status, supported_status, "memory", unit);
> }
> EXPORT_SYMBOL(drm_show_memory_stats);
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 2a0e3529598b..cd1198151744 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -1067,7 +1067,7 @@ static void msm_show_fdinfo(struct drm_printer *p,
> struct drm_file *file)
>
> msm_gpu_show_fdinfo(priv->gpu, file->driver_priv, p);
>
> - drm_show_memory_stats(p, file);
> + drm_show_memory_stats(p, file, 0);
> }
>
> static const struct file_operations fops = {
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 93d5f5538c0b..79c08cee3e9d 100644
> --- a
