diff mbox series

[v9,2/3] drm/plane: modify create_in_formats to accommodate async

Message ID 20250319-asyn-v9-2-629d1ece63e7@intel.com (mailing list archive)
State New
Headers show
Series Expose modifiers/formats supported by async flips | expand

Commit Message

Arun R Murthy March 19, 2025, 10:32 a.m. UTC 
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)
v7: Retain the unsupported modifiers, userspace should have no
impact, return pointer to blob instead of blob_id(Ville)

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Tested-by: Naveen Kumar <naveen1.kumar@intel.com>
---
 drivers/gpu/drm/drm_plane.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

Comments

Ville Syrjälä March 26, 2025, 5:23 p.m. UTC | #1 
On Wed, Mar 19, 2025 at 04:02:16PM +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)
> v7: Retain the unsupported modifiers, userspace should have no
> impact, return pointer to blob instead of blob_id(Ville)
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> Tested-by: Naveen Kumar <naveen1.kumar@intel.com>
> ---
>  drivers/gpu/drm/drm_plane.c | 44 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 31 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index fe181c1002171acc68d3054c2d178f9b9f501fe2..5cd3956caf414fa72432f9d23fa289ec266408ce 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -193,9 +193,13 @@ 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 struct drm_property_blob *create_in_format_blob(struct drm_device *dev,
> +						       struct drm_plane *plane,
> +						       bool (*format_mod_supported)
> +						       (struct drm_plane *plane,
> +							u32 format,
> +							u64 modifier))

Indentation looks wonky here.

Apart from that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


>  {
> -	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;
> @@ -221,7 +225,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
>  
>  	blob = drm_property_create_blob(dev, blob_size, NULL);
>  	if (IS_ERR(blob))
> -		return -1;
> +		return PTR_ERR(blob);
>  
>  	blob_data = blob->data;
>  	blob_data->version = FORMAT_BLOB_CURRENT;
> @@ -237,10 +241,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;
>  			}
>  		}
> @@ -251,10 +255,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;
>  }
>  
>  /**
> @@ -366,6 +367,7 @@ static int __drm_universal_plane_init(struct drm_device *dev,
>  				      const char *name, va_list ap)
>  {
>  	struct drm_mode_config *config = &dev->mode_config;
> +	struct drm_property_blob *blob;
>  	static const uint64_t default_modifiers[] = {
>  		DRM_FORMAT_MOD_LINEAR,
>  	};
> @@ -477,8 +479,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 = create_in_format_blob(dev, plane,
> +					     plane->funcs->format_mod_supported);
> +		if (!IS_ERR(blob))
> +			drm_object_attach_property(&plane->base,
> +						   config->modifiers_property,
> +						   blob->base.id);
> +	}
> +
> +	if (plane->funcs->format_mod_supported_async) {
> +		blob = create_in_format_blob(dev, plane,
> +					     plane->funcs->format_mod_supported_async);
> +		if (!IS_ERR(blob))
> +			drm_object_attach_property(&plane->base,
> +						   config->async_modifiers_property,
> +						   blob->base.id);
> +	}
> +
>  
>  	return 0;
>  }
> 
> -- 
> 2.25.1
Arun R Murthy March 27, 2025, 5:03 a.m. UTC | #2 
> On Wed, Mar 19, 2025 at 04:02:16PM +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)
> > v7: Retain the unsupported modifiers, userspace should have no impact,
> > return pointer to blob instead of blob_id(Ville)
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> > Tested-by: Naveen Kumar <naveen1.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/drm_plane.c | 44
> > +++++++++++++++++++++++++++++++-------------
> >  1 file changed, 31 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index
> >
> fe181c1002171acc68d3054c2d178f9b9f501fe2..5cd3956caf414fa72432f9d23f
> a2
> > 89ec266408ce 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -193,9 +193,13 @@ 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 struct drm_property_blob *create_in_format_blob(struct drm_device
> *dev,
> > +						       struct drm_plane *plane,
> > +						       bool
> (*format_mod_supported)
> > +						       (struct drm_plane *plane,
> > +							u32 format,
> > +							u64 modifier))
> 
> Indentation looks wonky here.
> 
> Apart from that
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
Not sure why patchwork is showing like this. On code the indentation looks proper.

Thanks and Regards,
Arun R Murthy
--------------------
> 
> >  {
> > -	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; @@ -221,7 +225,7 @@
> > static int create_in_format_blob(struct drm_device *dev, struct
> > drm_plane *plane
> >
> >  	blob = drm_property_create_blob(dev, blob_size, NULL);
> >  	if (IS_ERR(blob))
> > -		return -1;
> > +		return PTR_ERR(blob);
> >
> >  	blob_data = blob->data;
> >  	blob_data->version = FORMAT_BLOB_CURRENT; @@ -237,10 +241,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;
> >  			}
> >  		}
> > @@ -251,10 +255,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;
> >  }
> >
> >  /**
> > @@ -366,6 +367,7 @@ static int __drm_universal_plane_init(struct
> drm_device *dev,
> >  				      const char *name, va_list ap)  {
> >  	struct drm_mode_config *config = &dev->mode_config;
> > +	struct drm_property_blob *blob;
> >  	static const uint64_t default_modifiers[] = {
> >  		DRM_FORMAT_MOD_LINEAR,
> >  	};
> > @@ -477,8 +479,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 = create_in_format_blob(dev, plane,
> > +					     plane->funcs-
> >format_mod_supported);
> > +		if (!IS_ERR(blob))
> > +			drm_object_attach_property(&plane->base,
> > +						   config->modifiers_property,
> > +						   blob->base.id);
> > +	}
> > +
> > +	if (plane->funcs->format_mod_supported_async) {
> > +		blob = create_in_format_blob(dev, plane,
> > +					     plane->funcs-
> >format_mod_supported_async);
> > +		if (!IS_ERR(blob))
> > +			drm_object_attach_property(&plane->base,
> > +						   config-
> >async_modifiers_property,
> > +						   blob->base.id);
> > +	}
> > +
> >
> >  	return 0;
> >  }
> >
> > --
> > 2.25.1
> 
> --
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index fe181c1002171acc68d3054c2d178f9b9f501fe2..5cd3956caf414fa72432f9d23fa289ec266408ce 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -193,9 +193,13 @@  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 struct drm_property_blob *create_in_format_blob(struct drm_device *dev,
+						       struct drm_plane *plane,
+						       bool (*format_mod_supported)
+						       (struct drm_plane *plane,
+							u32 format,
+							u64 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;
@@ -221,7 +225,7 @@  static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 
 	blob = drm_property_create_blob(dev, blob_size, NULL);
 	if (IS_ERR(blob))
-		return -1;
+		return PTR_ERR(blob);
 
 	blob_data = blob->data;
 	blob_data->version = FORMAT_BLOB_CURRENT;
@@ -237,10 +241,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;
 			}
 		}
@@ -251,10 +255,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;
 }
 
 /**
@@ -366,6 +367,7 @@  static int __drm_universal_plane_init(struct drm_device *dev,
 				      const char *name, va_list ap)
 {
 	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_property_blob *blob;
 	static const uint64_t default_modifiers[] = {
 		DRM_FORMAT_MOD_LINEAR,
 	};
@@ -477,8 +479,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 = create_in_format_blob(dev, plane,
+					     plane->funcs->format_mod_supported);
+		if (!IS_ERR(blob))
+			drm_object_attach_property(&plane->base,
+						   config->modifiers_property,
+						   blob->base.id);
+	}
+
+	if (plane->funcs->format_mod_supported_async) {
+		blob = create_in_format_blob(dev, plane,
+					     plane->funcs->format_mod_supported_async);
+		if (!IS_ERR(blob))
+			drm_object_attach_property(&plane->base,
+						   config->async_modifiers_property,
+						   blob->base.id);
+	}
+
 
 	return 0;
 }