| Message ID | 20250212-asyn-v5-2-dc182281dca3@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Expose modifiers/formats supported by async flips | expand |
> -----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
> -----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 --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; }
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(-)