[PATCH] RFC: drm/drm_plane: Expose the plane capability and interoperability

Tue Jul 30 04:07:32 UTC 2024

> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> Sent: Tuesday, July 30, 2024 4:21 AM
> To: Murthy, Arun R <arun.r.murthy at intel.com>
> Cc: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> 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 at intel.com>
> > > Sent: Tuesday, July 9, 2024 1:17 PM
> > > To: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> > > Cc: Murthy, Arun R <arun.r.murthy at intel.com>
> > > 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 at intel.com>
> > > ---
> > >  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