diff mbox series

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

Message ID 20250212-asyn-v5-2-dc182281dca3@intel.com (mailing list archive)
State New, archived
Headers show
Series Expose modifiers/formats supported by async flips | expand

Commit Message

Arun R Murthy Feb. 12, 2025, 4:18 p.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.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/drm_plane.c | 46 +++++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 12 deletions(-)

Comments

Borah, Chaitanya Kumar Feb. 13, 2025, 9:55 a.m. UTC | #1
> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Wednesday, February 12, 2025 9:48 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 v5 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.
> 
> 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.
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/drm_plane.c | 46 +++++++++++++++++++++++++++++++++--
> ----------
>  1 file changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index
> c9d8871417722186d2b6f87197c9e15a70924b4f..01f67f1f5f29e37b8d0e0793c
> 58bbe1bba337eb2 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -190,9 +190,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; @@ -234,24 +237,26
> @@ 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;
>  			}
>  		}
> 
> +		if (!mod->formats) {
> +			mod->modifier = 0;
> +			blob_data->count_modifiers--;
> +			continue;
> +		}

I don't think change works. 0 represents DRM_FORMAT_MOD_LINEAR. 

>  		mod->modifier = plane->modifiers[i];
>  		mod->offset = 0;
>  		mod->pad = 0;
>  		mod++;
>  	}
> 
> -	drm_object_attach_property(&plane->base, config-
> >modifiers_property,
> -				   blob->base.id);
> -
> -	return 0;
> +	return blob->base.id;
>  }
> 
>  /**
> @@ -368,6 +373,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)) @@ -474,8 +480,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
Borah, Chaitanya Kumar Feb. 14, 2025, 6:32 p.m. UTC | #2
> -----Original Message-----
> From: Borah, Chaitanya Kumar
> Sent: Thursday, February 13, 2025 3:26 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; dri-
> devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; intel-
> xe@lists.freedesktop.org
> Cc: Syrjala, Ville <ville.syrjala@intel.com>
> Subject: RE: [PATCH v5 2/3] drm/plane: modify create_in_formats to
> accommodate async
> 
> > -----Original Message-----
> > From: Murthy, Arun R <arun.r.murthy@intel.com>
> > Sent: Wednesday, February 12, 2025 9:48 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 v5 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.
> >
> > 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.
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > ---
> >  drivers/gpu/drm/drm_plane.c | 46
> +++++++++++++++++++++++++++++++++--
> > ----------
> >  1 file changed, 34 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index
> >
> c9d8871417722186d2b6f87197c9e15a70924b4f..01f67f1f5f29e37b8d0e0793c
> > 58bbe1bba337eb2 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -190,9 +190,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; @@ -234,24 +237,26
> > @@ 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;
> >  			}
> >  		}
> >
> > +		if (!mod->formats) {
> > +			mod->modifier = 0;
> > +			blob_data->count_modifiers--;
> > +			continue;
> > +		}
> 
> I don't think change works. 0 represents DRM_FORMAT_MOD_LINEAR.

Relooked into the code, this change will prevent  adding modifiers which do not have any formats supported into the modifier list.
So it is correct in that. Smart idea!
However, the line mod->modifier = 0 is redundant. It will just get overwritten by the next valid modifier.

Regards

Chaitanya

> 
> >  		mod->modifier = plane->modifiers[i];
> >  		mod->offset = 0;
> >  		mod->pad = 0;
> >  		mod++;
> >  	}
> >
> > -	drm_object_attach_property(&plane->base, config-
> > >modifiers_property,
> > -				   blob->base.id);
> > -
> > -	return 0;
> > +	return blob->base.id;
> >  }
> >
> >  /**
> > @@ -368,6 +373,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)) @@ -474,8 +480,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
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index c9d8871417722186d2b6f87197c9e15a70924b4f..01f67f1f5f29e37b8d0e0793c58bbe1bba337eb2 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -190,9 +190,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;
@@ -234,24 +237,26 @@  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;
 			}
 		}
 
+		if (!mod->formats) {
+			mod->modifier = 0;
+			blob_data->count_modifiers--;
+			continue;
+		}
 		mod->modifier = plane->modifiers[i];
 		mod->offset = 0;
 		mod->pad = 0;
 		mod++;
 	}
 
-	drm_object_attach_property(&plane->base, config->modifiers_property,
-				   blob->base.id);
-
-	return 0;
+	return blob->base.id;
 }
 
 /**
@@ -368,6 +373,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))
@@ -474,8 +480,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;
 }