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

["Murthy, Arun R" <arun.r.murthy@intel.com>]

On 12-02-2025 10:23, Borah, Chaitanya Kumar wrote:
>> -----Original Message-----
>> From: Murthy, Arun R <arun.r.murthy@intel.com>
>> Sent: Wednesday, February 5, 2025 3:57 PM
>> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
>> xe@lists.freedesktop.org
>> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Syrjala,
>> Ville <ville.syrjala@intel.com>; Murthy, Arun R <arun.r.murthy@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@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.
>
>>   }
>>
>>   /**
>> @@ -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