Re: [PATCH v3 4/5] drm/i915/display: Populate list of async supported formats/modifiers

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jan 21, 2025 at 03:34:20AM +0000, Murthy, Arun R wrote:
> > On Wed, Jan 08, 2025 at 11:09:02AM +0530, Arun R Murthy wrote:
> > > Populate the list of formats/modifiers supported by async flip.
> > > Register a async property and expose the same to user through blob.
> > >
> > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/skl_universal_plane.c | 51
> > > ++++++++++++++++++++++
> > >  1 file changed, 51 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > > b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > > index
> > >
> > ff9764cac1e71959e56283f61b5192ea261cec7a..e5e47f2219dae62e76cbde2e
> > fb40
> > > 266b047ab2b2 100644
> > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > > @@ -170,6 +170,44 @@ static const u32 icl_hdr_plane_formats[] = {
> > >  	DRM_FORMAT_XVYU16161616,
> > >  };
> > >
> > > +static u64 tgl_asyn_modifiers[] = {
> > > +	DRM_FORMAT_MOD_LINEAR,
> > > +	I915_FORMAT_MOD_X_TILED,
> > > +	I915_FORMAT_MOD_Y_TILED,
> > > +	I915_FORMAT_MOD_4_TILED,
> > > +	I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS,
> > > +	I915_FORMAT_MOD_4_TILED_MTL_RC_CCS,
> > > +	I915_FORMAT_MOD_4_TILED_DG2_RC_CCS,
> > > +	I915_FORMAT_MOD_4_TILED_BMG_CCS,
> > > +	I915_FORMAT_MOD_4_TILED_LNL_CCS,
> > > +};
> > > +
> > > +static u64 icl_async_modifiers[] = {
> > > +	I915_FORMAT_MOD_X_TILED,
> > > +	I915_FORMAT_MOD_Y_TILED,
> > > +	I915_FORMAT_MOD_Yf_TILED,
> > > +	I915_FORMAT_MOD_Y_TILED_CCS,
> > > +	I915_FORMAT_MOD_Yf_TILED_CCS,
> > > +};
> > > +
> > > +static u64 skl_async_modifiers[] = {
> > > +	I915_FORMAT_MOD_X_TILED,
> > > +	I915_FORMAT_MOD_Y_TILED,
> > > +	I915_FORMAT_MOD_Yf_TILED,
> > > +};
> > > +
> > > +static u32 intel_async_formats[] = {
> > > +	DRM_FORMAT_RGB565,
> > > +	DRM_FORMAT_XRGB8888,
> > > +	DRM_FORMAT_XBGR8888,
> > > +	DRM_FORMAT_ARGB8888,
> > > +	DRM_FORMAT_ABGR8888,
> > > +	DRM_FORMAT_XRGB2101010,
> > > +	DRM_FORMAT_XBGR2101010,
> > > +	DRM_FORMAT_XRGB16161616F,
> > > +	DRM_FORMAT_XBGR16161616F,
> > > +};
> > 
> > I've just pushed my .can_async_flip() thing. I'm thinking with that all this can
> > just disappear and we can have a completely generic implementation. Eg
> > something like:
> > 
> Thanks, will rebase and push!
> 
> Thanks and Regards,
> Arun R Murthy
> --------------------
> > intel_plane_format_mod_supported_async()
> > {
> > 	// some generic checks here (eg. reject planar formats)
> > 
> > 	return plane->format_mod_supported() &&
> > 		plane->can_async_flip();
> > }

Actually for this one I think it'd probably make sense to call the 
normal format_mod_supported() before doing any other checks, just 
in case we ever get situations where the parameters come directly
from userspace. It's better to filter out completely bogus values
as early as possible, and the normal format_mod_supported() already
has to be prepared for garbage values.

> > 
> > > +
> > >  int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)  {
> > >  	switch (format) {
> > > @@ -2613,6 +2651,7 @@ skl_universal_plane_create(struct
> > drm_i915_private *dev_priv,
> > >  	unsigned int supported_rotations;
> > >  	unsigned int supported_csc;
> > >  	const u64 *modifiers;
> > > +	u64 *async_modifiers;
> > >  	const u32 *formats;
> > >  	int num_formats;
> > >  	int ret;
> > > @@ -2715,6 +2754,18 @@ skl_universal_plane_create(struct
> > drm_i915_private *dev_priv,
> > >  	if (ret)
> > >  		goto fail;
> > >
> > > +	if (DISPLAY_VER(dev_priv) >= 12)
> > > +		async_modifiers = tgl_asyn_modifiers;
> > > +	else if (DISPLAY_VER(dev_priv) == 11)
> > > +		async_modifiers = icl_async_modifiers;
> > > +	else
> > > +		async_modifiers = skl_async_modifiers;
> > > +
> > > +	drm_plane_create_format_blob(&dev_priv->drm, &plane->base,
> > > +				     async_modifiers, sizeof(async_modifiers),
> > > +				     intel_async_formats,
> > > +				     sizeof(intel_async_formats), true);
> > > +
> > >  	if (DISPLAY_VER(dev_priv) >= 13)
> > >  		supported_rotations = DRM_MODE_ROTATE_0 |
> > DRM_MODE_ROTATE_180;
> > >  	else
> > >
> > > --
> > > 2.25.1
> > 
> > --
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel