On Wed, Feb 19, 2025 at 02:47:24PM +0530, Arun R Murthy wrote:
> 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.
> 
> v5: create_in_formats can return -ve value in failure case, correct the
> if condition to check the creation of blob <Chaitanya>
> Dont add the modifier for which none of the formats is not supported.
> v6: Remove the code for masking the unsupported modifiers as UMD can
> leave with it. (Naveen/Chaitanya)
> 
> Signed-off-by: Arun R Murthy <arun.r.mur...@intel.com>
> Tested-by: Naveen Kumar <naveen1.ku...@intel.com>
> ---
>  drivers/gpu/drm/drm_plane.c | 45 
> +++++++++++++++++++++++++++++++++------------
>  1 file changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 
> fe181c1002171acc68d3054c2d178f9b9f501fe2..fc26ef8a6cab346c9491495b57f88f1eb205d22c
>  100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -193,9 +193,12 @@ 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;
>       struct drm_format_modifier *mod;
>       size_t blob_size, formats_size, modifiers_size;
> @@ -237,13 +240,17 @@ 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])) {

Weird line breaks.

>                               mod->formats |= 1ULL << j;
>                       }
>               }
> +             if (!mod->formats) {
> +                     blob_data->count_modifiers--;
> +                     continue;
> +             }

I hope no userspace rejects the blob if there
is extra data at the end...

Since this can actually change the behaviour 
I think this one needs to be a separate patch
in case we need a revert later.

>  
>               mod->modifier = plane->modifiers[i];
>               mod->offset = 0;
> @@ -251,10 +258,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;

The return type is wrong for this.

I think it'd probably be better to return
struct drm_property_blob* instead so we can
return errors properly as well.

>  }
>  
>  /**
> @@ -371,6 +375,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))
> @@ -477,8 +482,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 > 0)
> +                     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 > 0)
> +                     drm_object_attach_property(&plane->base,
> +                                                
> config->async_modifiers_property,
> +                                                blob_id);
> +     }
> +
>  
>       return 0;
>  }
> 
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Reply via email to