| Message ID | 20250205-asyn-v4-2-9a5b018e359b@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| 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 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 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. > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > --- > drivers/gpu/drm/drm_plane.c | 40 +++++++++++++++++++++++++++++-------- > --- > 1 file changed, 29 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index > 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ce > b64613bd5e0a663 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -191,7 +191,11 @@ 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; > @@ -235,10 +239,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; > } > } > @@ -249,10 +253,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; I think we can return the blob instead of the id, more on this later. > } > > /** > @@ -369,6 +370,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)) @@ -475,8 +477,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) The function create_in_format_blob() can return negative value. This check does not work if that is the case. > 0 can be used as the blob id's start range is from 1 to INT_MAX Regards Chaitanya > + 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) > + drm_object_attach_property(&plane->base, > + config- > >async_modifiers_property, > + blob_id); > + } > + > > return 0; > } > > -- > 2.25.1
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 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. >> >> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> >> --- >> drivers/gpu/drm/drm_plane.c | 40 +++++++++++++++++++++++++++++-------- >> --- >> 1 file changed, 29 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c >> index >> 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ce >> b64613bd5e0a663 100644 >> --- a/drivers/gpu/drm/drm_plane.c >> +++ b/drivers/gpu/drm/drm_plane.c >> @@ -191,7 +191,11 @@ 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; >> @@ -235,10 +239,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; >> } >> } >> @@ -249,10 +253,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; > I think we can return the blob instead of the id, more on this later. Do you mean pointer to the blob. That can also be done. But any advantage of doing so, because from blob_id we can get the blob. > >> } >> >> /** >> @@ -369,6 +370,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)) @@ -475,8 +477,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) > The function create_in_format_blob() can return negative value. This check does not work if that is the case. corrected! Thanks and Regards, Arun R Murthy ------------------- >> 0 can be used as the blob id's start range is from 1 to INT_MAX > Regards > > Chaitanya > >> + 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) >> + drm_object_attach_property(&plane->base, >> + config- >>> async_modifiers_property, >> + blob_id); >> + } >> + >> >> return 0; >> } >> >> -- >> 2.25.1
> -----Original Message----- > From: Murthy, Arun R <arun.r.murthy@intel.com> > Sent: Wednesday, February 12, 2025 10:55 AM > To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@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 v4 2/3] drm/plane: modify create_in_formats to > accommodate async > > 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 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. > >> > >> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > >> --- > >> drivers/gpu/drm/drm_plane.c | 40 > >> +++++++++++++++++++++++++++++-------- > >> --- > >> 1 file changed, 29 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_plane.c > >> b/drivers/gpu/drm/drm_plane.c index > >> > 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ce > >> b64613bd5e0a663 100644 > >> --- a/drivers/gpu/drm/drm_plane.c > >> +++ b/drivers/gpu/drm/drm_plane.c > >> @@ -191,7 +191,11 @@ 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; > >> @@ -235,10 +239,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; > >> } > >> } > >> @@ -249,10 +253,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; > > I think we can return the blob instead of the id, more on this later. > Do you mean pointer to the blob. That can also be done. > But any advantage of doing so, because from blob_id we can get the blob. Forgot to modify this comment. This was before I found out a valid blob id is always greater than 0. So you can go ahead with using the check of >0 as discussed below. Regards Chaitanya > > > >> } > >> > >> /** > >> @@ -369,6 +370,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)) @@ -475,8 +477,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) > > The function create_in_format_blob() can return negative value. This check > does not work if that is the case. > > corrected! > > Thanks and Regards, > Arun R Murthy > ------------------- > > >> 0 can be used as the blob id's start range is from 1 to INT_MAX > > Regards > > > > Chaitanya > > > >> + 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) > >> + 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 416818e54ccffcf3d3aada2723e96ce8ccf1dd97..3819fdde985576bd6ba6a08ceb64613bd5e0a663 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -191,7 +191,11 @@ 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; @@ -235,10 +239,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; } } @@ -249,10 +253,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; } /** @@ -369,6 +370,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)) @@ -475,8 +477,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) + 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) + 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. Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> --- drivers/gpu/drm/drm_plane.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-)