> -----Original Message----- > From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > Sent: Tuesday, July 30, 2024 4:21 AM > To: Murthy, Arun R <arun.r.murthy@xxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] RFC: drm/drm_plane: Expose the plane capability and > interoperability > > On Mon, Jul 29, 2024 at 04:59:14AM GMT, Murthy, Arun R wrote: > > Gentle Reminder! > > Any comments? > > First of all, the format is underdocumented. Second, there is a usual > requirement for new uAPI: please provide a pointer to IGT patch and to the > userspace utilising the property. There are some discussions on using this in UMD. https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/29618#note_2487123 Thanks and Regards, Arun R Murthy -------------------- > > > > > Thanks and Regards, > > Arun R Murthy > > -------------------- > > > > > -----Original Message----- > > > From: Murthy, Arun R <arun.r.murthy@xxxxxxxxx> > > > Sent: Tuesday, July 9, 2024 1:17 PM > > > To: dri-devel@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Cc: Murthy, Arun R <arun.r.murthy@xxxxxxxxx> > > > Subject: [PATCH] RFC: drm/drm_plane: Expose the plane capability and > > > interoperability > > > > > > Each plane has its own capability or interoperability based on the > > > harware restrictions. If this is exposed to the user, then user can > > > read it once on boot and store this. Later can be used as a lookup > > > table to check a corresponding capability is supported by plane then > > > only go ahead with the framebuffer creation/calling atomic_ioctl. > > > > > > For Ex: There are few restiction as to async flip doesnt support all > > > the formats/modifiers. Similar restrictions on scaling. With the > > > availabililty of this info to user, failures in atomic_check can be > > > avoided as these are more the hardware capabilities. > > > > > > There are two options on how this can be acheived. > > > Option 1: > > > > > > Add a new element to struct drm_mode_get_plane that holds the addr > > > to the array of a new struct. This new struct consists of the > > > formats supported and the corresponding plane capabilities. > > > > > > Option 2: > > > > > > These can be exposed to user as a plane property so that the user > > > can get to know the limitation ahead and avoid failures in atomic_check. > > > > > > Usually atomic_get_property is controlled over the state struct for > > > the parameters/elements that can change. But here these capabilities > > > or the interoperabilities are rather hardware restrictions and wont change > over flips. > > > Hence having as a plane_property may not make much sense. > > > On the other hand, Option 1 include changes in the uapi struct > > > drm_mode_get_plane. Shouldnt have impact on backward compatibility, > > > but if userspace has some implementation so as to check the size of > > > the struct, then it might a challenge. > > > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/drm_atomic_uapi.c | 3 +++ > > > include/drm/drm_plane.h | 8 ++++++++ > > > include/uapi/drm/drm_mode.h | 20 ++++++++++++++++++++ > > > 3 files changed, 31 insertions(+) > > > > > > =============Option 2======================== > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > > b/drivers/gpu/drm/drm_atomic_uapi.c > > > index 22bbb2d83e30..b46177d5fc8c 100644 > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > > @@ -631,6 +631,9 @@ drm_atomic_plane_get_property(struct drm_plane > > > *plane, > > > *val = state->hotspot_x; > > > } else if (property == plane->hotspot_y_property) { > > > *val = state->hotspot_y; > > > + } else if (property == config->prop_plane_caps) { > > > + *val = (state->plane_caps) ? > > > + state->plane_caps->base.id : 0; > > > } else { > > > drm_dbg_atomic(dev, > > > "[PLANE:%d:%s] unknown property > [PROP:%d:%s]\n", diff > > > --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index > > > dd718c62ac31..dfe931677d0a 100644 > > > --- a/include/drm/drm_plane.h > > > +++ b/include/drm/drm_plane.h > > > @@ -260,6 +260,14 @@ struct drm_plane_state { > > > * flow. > > > */ > > > bool color_mgmt_changed : 1; > > > + > > > + /** > > > + * @plane_caps: > > > + * > > > + * Blob representing plane capcabilites and interoperability. > > > + * This element is a pointer to the array of struct drm_format_blob. > > > + */ > > > + struct drm_property_blob *plane_caps; > > > }; > > > > > > static inline struct drm_rect > > > > > > =============Option 1======================== > > > > > > diff --git a/include/uapi/drm/drm_mode.h > > > b/include/uapi/drm/drm_mode.h index d390011b89b4..0b5c1b65ef63 > > > 100644 > > > --- a/include/uapi/drm/drm_mode.h > > > +++ b/include/uapi/drm/drm_mode.h > > > @@ -312,6 +312,20 @@ struct drm_mode_set_plane { > > > __u32 src_w; > > > }; > > > > > > +#define DRM_FORMAT_PLANE_CAP_LINEAR_TILE BIT(0) > > > +#define DRM_FORMAT_PLANE_CAP_X_TILE BIT(1) > > > +#define DRM_FORMAT_PLANE_CAP_Y_TILE BIT(2) > > > +#define DRM_FORMAT_PLANE_CAP_Yf_TILE BIT(3) > > > +#define DRM_FORMAT_PLANE_CAP_ASYNC_FLIP BIT(4) > > > +#define DRM_FORMAT_PLANE_CAP_FBC BIT(5) > > > +#define DRM_FORMAT_PLANE_CAP_RC BIT(6) > > > + > > > +struct drm_format_blob { > > > + __u64 modifier; > > > + __u32 plane_caps; > > > + > > > +}; > > > + > > > /** > > > * struct drm_mode_get_plane - Get plane metadata. > > > * > > > @@ -355,6 +369,12 @@ struct drm_mode_get_plane { > > > * supported by the plane. These formats do not require modifiers. > > > */ > > > __u64 format_type_ptr; > > > + /** > > > + * @ format_blob_ptr: Pointer to the array of struct drm_format_blob. > > > + * Specify the plane capabilites/restrictions w.r.t tiling/sync-async > > > + * flips etc > > > + */ > > > + __u64 format_blob_ptr; > > > }; > > > > > > struct drm_mode_get_plane_res { > > > -- > > > 2.25.1 > > > > -- > With best wishes > Dmitry
