lkml.org 
[lkml]   [2023]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH RFC v5 01/10] drm: Introduce pixel_source DRM plane property
    On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <quic_jesszhan@quicinc.com> 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 <quic_jesszhan@quicinc.com>
    > ---
    > 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 *plane,
    > + unsigned long extra_sources)
    > +{
    > + struct drm_device *dev = plane->dev;
    > + struct drm_property *prop;
    > + static const struct drm_prop_enum_list enum_list[] = {
    > + { DRM_PLANE_PIXEL_SOURCE_NONE, "NONE" },
    > + { DRM_PLANE_PIXEL_SOURCE_FB, "FB" },
    > + };
    > + static const unsigned int valid_source_mask = BIT(DRM_PLANE_PIXEL_SOURCE_FB);
    > + int i;
    > +
    > + /* FB is supported by default */
    > + unsigned long supported_sources = extra_sources | BIT(DRM_PLANE_PIXEL_SOURCE_FB);
    > +
    > + 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(enum_list); i++) {
    > + int ret;
    > +
    > + if (!test_bit(enum_list[i].type, &supported_sources))
    > + continue;
    > +
    > + ret = drm_property_add_enum(prop, enum_list[i].type, 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..f342cf15412b 100644
    > --- a/drivers/gpu/drm/drm_plane.c
    > +++ b/drivers/gpu/drm/drm_plane.c
    > @@ -987,6 +987,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)
    > + plane->state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;

    Nit: you are going to set the value anyway. So the second condition
    can be dropped.

    LGTM otherwise.

    > +
    > 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 51291983ea44..89508b4dea4a 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.41.0
    >


    --
    With best wishes
    Dmitry

    \
     
     \ /
      Last update: 2023-07-28 19:24    [W:5.351 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site