On Mon, Nov 05, 2018 at 06:44:34PM -0800, Dhinakaran Pandiyan wrote:
> Allows drivers to pass a larger modifier array, thereby avoiding
> declarations of static modifier arrays that are only slight different
> for each plane.
>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Suggested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_plane.c | 35 +++++++++++++++++++++++++++--------
> 1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 1fa98bd12003..1546ffbf8e36 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -179,8 +179,8 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> const char *name, ...)
> {
> struct drm_mode_config *config = &dev->mode_config;
> - unsigned int format_modifier_count = 0;
> - int ret;
> + unsigned int format_modifier_count, in_modifier_count = 0;
> + int ret, i;
>
> /* plane index is used with 32bit bitmasks */
> if (WARN_ON(config->num_total_plane >= 32))
> @@ -216,22 +216,43 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>
> if (format_modifiers) {
> const uint64_t *temp_modifiers = format_modifiers;
> +
> while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
> - format_modifier_count++;
> + in_modifier_count++;
> }
>
> - plane->modifier_count = format_modifier_count;
> - plane->modifiers = kmalloc_array(format_modifier_count,
> + plane->modifiers = kmalloc_array(in_modifier_count,
> sizeof(format_modifiers[0]),
> GFP_KERNEL);
>
> - if (format_modifier_count && !plane->modifiers) {
> + if (in_modifier_count && !plane->modifiers) {
> DRM_DEBUG_KMS("out of memory when allocating plane\n");
> kfree(plane->format_types);
> drm_mode_object_unregister(dev, &plane->base);
> return -ENOMEM;
> }
>
> + for (i = 0, format_modifier_count = 0; i < in_modifier_count; i++) {
> + int j;
> +
> + for (j = 0; funcs->format_mod_supported && j < format_count; j++)
> + if (funcs->format_mod_supported(plane, formats[j],
> + format_modifiers[i]))
> + break;
> +
> + if (j < format_count)
> + plane->modifiers[format_modifier_count++] =
> + format_modifiers[i];
> + }
> +
> + if (format_modifier_count < in_modifier_count) {
> + size_t size;
> +
> + size = format_modifier_count * sizeof(format_modifiers[0]);
> + plane->modifiers = krealloc(plane->modifiers, size, GFP_KERNEL);
Should check that the realloc actually succeeded.
And I think we might want to give this same treatment to plane->formats[]
as well.
And perhaps we could even throw out plane->modifiers[] and just rely on
the IN_FORMATS blob exclusively? Hmm. Looks like that is not getting fully
populated unless the driver has provided .format_mod_supported(). Not
sure why that is, and not sure what userspace is supposed to do with a
partially filled blob like that. I'm thinking we shouldn't even attach
the property to the plane in that case.
> + }
> + plane->modifier_count = format_modifier_count;
> +
> if (name) {
> va_list ap;
>
> @@ -251,8 +272,6 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>
> memcpy(plane->format_types, formats, format_count * sizeof(uint32_t));
> plane->format_count = format_count;
> - memcpy(plane->modifiers, format_modifiers,
> - format_modifier_count * sizeof(format_modifiers[0]));
> plane->possible_crtcs = possible_crtcs;
> plane->type = type;
>
> --
> 2.14.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
