Re: [PATCH v4 1/3] drm/plane: Add new plane property IN_FORMATS_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 1/3] drm/plane: Add new plane property
>> IN_FORMATS_ASYNC
>>
>> There exists a property IN_FORMATS which exposes the plane supported
>> modifiers/formats to the user. In some platforms when asynchronous flips are
>> used all of modifiers/formats mentioned in IN_FORMATS are not supported.
>> This patch adds a new plane property IN_FORMATS_ASYNC to expose the
>> async flips supported modifiers/formats so that user can use this information
>> ahead and done flips with unsupported formats/modifiers. This will save flips
> s/done/do
> s/flips/flip
Done!
>> failures.
>> Add a new function pointer similar to format_mod_supported specifically for
>> asynchronous flips.
>>
>> v2: Remove async variable from drm_plane (Ville)
>> v3: Add new function pointer for async (Ville)
>>
>> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>> ---
>>   drivers/gpu/drm/drm_mode_config.c |  7 +++++++
>>   drivers/gpu/drm/drm_plane.c       |  6 ++++++
>>   include/drm/drm_mode_config.h     |  6 ++++++
>>   include/drm/drm_plane.h           | 20 ++++++++++++++++++++
>>   4 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_mode_config.c
>> b/drivers/gpu/drm/drm_mode_config.c
>> index
>> 8642a2fb25a90116dab975aa0ab6b51deafb4b96..dbbef20753f834a85ae9ded
>> 748cff2b3f0e85043 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -388,6 +388,13 @@ static int
>> drm_mode_create_standard_properties(struct drm_device *dev)
>>   		return -ENOMEM;
>>   	dev->mode_config.size_hints_property = prop;
>>
>> +	prop = drm_property_create(dev,
>> +				   DRM_MODE_PROP_IMMUTABLE |
>> DRM_MODE_PROP_BLOB,
>> +				   "IN_FORMATS_ASYNC", 0);
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	dev->mode_config.async_modifiers_property = prop;
>> +
>>   	return 0;
>>   }
>>
>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> index
>> a28b22fdd7a41aca82d097d42237851da9a0a79b..416818e54ccffcf3d3aada27
>> 23e96ce8ccf1dd97 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -141,6 +141,12 @@
>>    *     various bugs in this area with inconsistencies between the capability
>>    *     flag and per-plane properties.
>>    *
>> + * IN_FORMATS_ASYNC:
>> + *     Blob property which contains the set of buffer format and modifier
>> + *     pairs supported by this plane for asynchronous flips. The blob is a struct
>> + *     drm_format_modifier_blob. Without this property the plane doesn't
>> + *     support buffers with modifiers.
> As pointed out in my previous comment we should remove this line or replace it with something like this
>
> Without this property the plane supports all the format modifier pair listed by IN_FORMAT for asynchronous flips.

Done!

>> Userspace cannot change this property.
>> + *
>>    * SIZE_HINTS:
>>    *     Blob property which contains the set of recommended plane size
>>    *     which can used for simple "cursor like" use cases (eg. no scaling).
>> diff --git a/include/drm/drm_mode_config.h
>> b/include/drm/drm_mode_config.h index
>> 271765e2e9f2da62aaf0d258828ef4196e14822e..0c116d6dfd277262b1a4c0f0
>> 97fce2d719f43844 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -936,6 +936,12 @@ struct drm_mode_config {
>>   	 */
>>   	struct drm_property *modifiers_property;
>>
>> +	/**
>> +	 * @async_modifiers_property: Plane property to list support
>> modifier/format
>> +	 * combination for asynchronous flips.
>> +	 */
>> +	struct drm_property *async_modifiers_property;
>> +
>>   	/**
>>   	 * @size_hints_property: Plane SIZE_HINTS property.
>>   	 */
>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
>> dd718c62ac31bf16606f3ee9f025a5b171cd1e67..179da19d602ac94cb6eeea3e
>> 6522ae5c7738bb46 100644
>> --- a/include/drm/drm_plane.h
>> +++ b/include/drm/drm_plane.h
>> @@ -549,6 +549,26 @@ struct drm_plane_funcs {
>>   	 */
>>   	bool (*format_mod_supported)(struct drm_plane *plane, uint32_t
>> format,
>>   				     uint64_t modifier);
>> +	/**
>> +	 * @format_mod_supported_async:
>> +	 *
>> +	 * This optional hook is used for the DRM to determine if for
>> +	 * asynchronous flip the given format/modifier combination is valid
>> for
>> +	 * the plane. This allows the DRM to generate the correct format
>> +	 * bitmask (which formats apply to which modifier), and to validate
>> +	 * modifiers at atomic_check time.
>> +	 *
>> +	 * If not present, then any modifier in the plane's modifier
>> +	 * list is allowed with any of the plane's formats.
>> +	 *
> This is a bit misleading because userspace still needs to consider IN_FORMATS.
> It cannot randomly pair any format with any modifier.

Done

Thanks and Regards,
Arun R Murthy
--------------------

> Regards
>
> Chaitanya
>
>> +	 * Returns:
>> +	 *
>> +	 * True if the given modifier is valid for that format on the plane.
>> +	 * False otherwise.
>> +	 */
>> +	bool (*format_mod_supported_async)(struct drm_plane *plane,
>> +					   uint32_t format, uint64_t modifier);
>> +
>>   };
>>
>>   /**
>>
>> --
>> 2.25.1