On Mon, May 10, 2021 at 09:49:38PM -0400, Tina Zhang wrote:
> Add a function to add modifiers to a plane.
>
> Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx>
For one, new functions for drivers needs kerneldoc.
But the real issue here is that you're suppoed to supply the modifiers
when creating the plane, not later on. So this function doesn't make
sense.
Please fix virtio code to use the existing functions
(drm_universal_plane_init() to be specific), or explain what that's not
possible.
-Daniel
> ---
> drivers/gpu/drm/drm_plane.c | 41 +++++++++++++++++++++++++++++++++++++
> include/drm/drm_plane.h | 3 +++
> 2 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index b570a480090a..793b16d84f86 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -288,6 +288,47 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
> }
> EXPORT_SYMBOL(drm_universal_plane_init);
>
> +int drm_plane_add_modifiers(struct drm_device *dev,
> + struct drm_plane *plane,
> + const uint64_t *format_modifiers)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + const uint64_t *temp_modifiers = format_modifiers;
> + unsigned int format_modifier_count = 0;
> +
> + /*
> + * Only considering adding modifiers when no modifier was
> + * added to that plane before.
> + */
> + if (!temp_modifiers || plane->modifier_count)
> + return -EINVAL;
> +
> + while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
> + format_modifier_count++;
> +
> + if (format_modifier_count)
> + config->allow_fb_modifiers = true;
> +
> + plane->modifier_count = format_modifier_count;
> + plane->modifiers = kmalloc_array(format_modifier_count,
> + sizeof(format_modifiers[0]),
> + GFP_KERNEL);
> +
> + if (format_modifier_count && !plane->modifiers) {
> + DRM_DEBUG_KMS("out of memory when allocating plane\n");
> + return -ENOMEM;
> + }
> +
> + memcpy(plane->modifiers, format_modifiers,
> + format_modifier_count * sizeof(format_modifiers[0]));
> + if (config->allow_fb_modifiers)
> + create_in_format_blob(dev, plane);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_add_modifiers);
> +
> +
> int drm_plane_register_all(struct drm_device *dev)
> {
> unsigned int num_planes = 0;
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 50c23eb432b7..0dacdeffc3bc 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -827,6 +827,9 @@ int drm_universal_plane_init(struct drm_device *dev,
> const uint64_t *format_modifiers,
> enum drm_plane_type type,
> const char *name, ...);
> +int drm_plane_add_modifiers(struct drm_device *dev,
> + struct drm_plane *plane,
> + const uint64_t *format_modifiers);
> int drm_plane_init(struct drm_device *dev,
> struct drm_plane *plane,
> uint32_t possible_crtcs,
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
