[PATCH v4 2/3] drm/plane: modify create_in_formats to accommodate async

[Borah, Chaitanya Kumar]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy at intel.com>
> Sent: Wednesday, February 12, 2025 10:55 AM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>; dri-
> devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; intel-
> xe at lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala at intel.com>
> Subject: Re: [PATCH v4 2/3] drm/plane: modify create_in_formats to
> accommodate async
> 
> On 12-02-2025 10:23, Borah, Chaitanya Kumar wrote:
> >> -----Original Message-----
> >> From: Murthy, Arun R <arun.r.murthy at intel.com>
> >> Sent: Wednesday, February 5, 2025 3:57 PM
> >> To: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org;
> >> intel- xe at lists.freedesktop.org
> >> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>;
> >> Syrjala, Ville <ville.syrjala at intel.com>; Murthy, Arun R
> >> <arun.r.murthy at intel.com>
> >> Subject: [PATCH v4 2/3] drm/plane: modify create_in_formats to
> >> accommodate async
> >>
> >> create_in_formats creates the list of supported format/modifiers for
> >> synchronous flips, modify the same function so as to take the
> >> format_mod_supported as argument and create list of format/modifier
> >> for async as well.
> >>
> >> Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_plane.c | 40
> >> +++++++++++++++++++++++++++++--------
> >> ---
> >>   1 file changed, 29 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_plane.c
> >> b/drivers/gpu/drm/drm_plane.c index
> >>
> 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ce
> >> b64613bd5e0a663 100644
> >> --- a/drivers/gpu/drm/drm_plane.c
> >> +++ b/drivers/gpu/drm/drm_plane.c
> >> @@ -191,7 +191,11 @@ modifiers_ptr(struct drm_format_modifier_blob
> >> *blob)
> >>   	return (struct drm_format_modifier *)(((char *)blob) + blob-
> >>> modifiers_offset);  }
> >> -static int create_in_format_blob(struct drm_device *dev, struct
> >> drm_plane
> >> *plane)
> >> +static int create_in_format_blob(struct drm_device *dev, struct
> >> +drm_plane
> >> *plane,
> >> +				 bool (*format_mod_supported)
> >> +						(struct drm_plane *plane,
> >> +						 uint32_t format,
> >> +						 uint64_t modifier))
> >>   {
> >>   	const struct drm_mode_config *config = &dev->mode_config;
> >>   	struct drm_property_blob *blob;
> >> @@ -235,10 +239,10 @@ static int create_in_format_blob(struct
> >> drm_device *dev, struct drm_plane *plane
> >>   	mod = modifiers_ptr(blob_data);
> >>   	for (i = 0; i < plane->modifier_count; i++) {
> >>   		for (j = 0; j < plane->format_count; j++) {
> >> -			if (!plane->funcs->format_mod_supported ||
> >> -			    plane->funcs->format_mod_supported(plane,
> >> -							       plane-
> >>> format_types[j],
> >> -							       plane-
> >>> modifiers[i])) {
> >> +			if (!format_mod_supported ||
> >> format_mod_supported
> >> +							(plane,
> >> +							 plane-
> >>> format_types[j],
> >> +							 plane->modifiers[i]))
> >> {
> >>   				mod->formats |= 1ULL << j;
> >>   			}
> >>   		}
> >> @@ -249,10 +253,7 @@ static int create_in_format_blob(struct
> >> drm_device *dev, struct drm_plane *plane
> >>   		mod++;
> >>   	}
> >>
> >> -	drm_object_attach_property(&plane->base, config-
> >>> modifiers_property,
> >> -				   blob->base.id);
> >> -
> >> -	return 0;
> >> +	return blob->base.id;
> > I think we can return the blob instead of the id, more on this later.
> Do you mean pointer to the blob. That can also be done.
> But any advantage of doing so, because from blob_id we can get the blob.

Forgot to modify this comment. This was before I found out a valid  blob id is always greater than 0.

So you can go ahead with using the check of >0 as discussed below.

Regards

Chaitanya


> >
> >>   }
> >>
> >>   /**
> >> @@ -369,6 +370,7 @@ static int __drm_universal_plane_init(struct
> >> drm_device *dev,
> >>   	};
> >>   	unsigned int format_modifier_count = 0;
> >>   	int ret;
> >> +	int blob_id;
> >>
> >>   	/* plane index is used with 32bit bitmasks */
> >>   	if (WARN_ON(config->num_total_plane >= 32)) @@ -475,8 +477,24
> @@
> >> static int __drm_universal_plane_init(struct drm_device *dev,
> >>   		drm_plane_create_hotspot_properties(plane);
> >>   	}
> >>
> >> -	if (format_modifier_count)
> >> -		create_in_format_blob(dev, plane);
> >> +	if (format_modifier_count) {
> >> +		blob_id = create_in_format_blob(dev, plane,
> >> +						plane->funcs-
> >>> format_mod_supported);
> >> +		if (blob_id)
> > The function create_in_format_blob() can return negative value. This check
> does not work if that is the case.
> 
> corrected!
> 
> Thanks and Regards,
> Arun R Murthy
> -------------------
> 
> >> 0 can be used as the blob id's start range is from 1 to INT_MAX
> > Regards
> >
> > Chaitanya
> >
> >> +			drm_object_attach_property(&plane->base,
> >> +						   config->modifiers_property,
> >> +						   blob_id);
> >> +	}
> >> +
> >> +	if (plane->funcs->format_mod_supported_async) {
> >> +		blob_id = create_in_format_blob(dev, plane,
> >> +						plane->funcs-
> >>> format_mod_supported_async);
> >> +		if (blob_id)
> >> +			drm_object_attach_property(&plane->base,
> >> +						   config-
> >>> async_modifiers_property,
> >> +						   blob_id);
> >> +	}
> >> +
> >>
> >>   	return 0;
> >>   }
> >>
> >> --
> >> 2.25.1