[PATCH 2/4] drm/plane: Allocate planes with drm_universal_plane_alloc()

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Sep 16 11:06:54 UTC 2022


Hi Thomas,

Thank you for the patch.

On Fri, Sep 09, 2022 at 12:59:45PM +0200, Thomas Zimmermann wrote:
> Provide drm_univeral_plane_alloc(), which allocated an initializes a
> plane. Code for non-atomic drivers uses this pattern. Convert it to
> the new function. The modeset helpers contain a quirk for handling their
> color formats differently. Set the flag outside plane allocation.
> 
> The new function is already deprecated to some extend. Drivers should
> rather use drmm_univeral_plane_alloc() or drm_universal_plane_init().

If this is already deprecated and used by a single driver, what is the
point ?

> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
>  drivers/gpu/drm/drm_modeset_helper.c    | 61 +++++++++++--------------
>  drivers/gpu/drm/drm_plane.c             | 38 +++++++++++++++
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 41 ++++++-----------
>  include/drm/drm_plane.h                 | 44 ++++++++++++++++++
>  4 files changed, 121 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 611dd01fb604..38040eebfa16 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -113,38 +113,6 @@ static const struct drm_plane_funcs primary_plane_funcs = {
>  	.destroy = drm_plane_helper_destroy,
>  };
>  
> -static struct drm_plane *create_primary_plane(struct drm_device *dev)
> -{
> -	struct drm_plane *primary;
> -	int ret;
> -
> -	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
> -	if (primary == NULL) {
> -		DRM_DEBUG_KMS("Failed to allocate primary plane\n");
> -		return NULL;
> -	}
> -
> -	/*
> -	 * Remove the format_default field from drm_plane when dropping
> -	 * this helper.
> -	 */
> -	primary->format_default = true;
> -
> -	/* possible_crtc's will be filled in later by crtc_init */
> -	ret = drm_universal_plane_init(dev, primary, 0,
> -				       &primary_plane_funcs,
> -				       safe_modeset_formats,
> -				       ARRAY_SIZE(safe_modeset_formats),
> -				       NULL,
> -				       DRM_PLANE_TYPE_PRIMARY, NULL);
> -	if (ret) {
> -		kfree(primary);
> -		primary = NULL;
> -	}
> -
> -	return primary;
> -}
> -
>  /**
>   * drm_crtc_init - Legacy CRTC initialization function
>   * @dev: DRM device
> @@ -176,10 +144,33 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  		  const struct drm_crtc_funcs *funcs)
>  {
>  	struct drm_plane *primary;
> +	int ret;
> +
> +	/* possible_crtc's will be filled in later by crtc_init */
> +	primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
> +					      &primary_plane_funcs,
> +					      safe_modeset_formats,
> +					      ARRAY_SIZE(safe_modeset_formats),
> +					      NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (IS_ERR(primary))
> +		return PTR_ERR(primary);
>  
> -	primary = create_primary_plane(dev);
> -	return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs,
> -					 NULL);
> +	/*
> +	 * Remove the format_default field from drm_plane when dropping
> +	 * this helper.
> +	 */
> +	primary->format_default = true;
> +
> +	ret = drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs, NULL);
> +	if (ret)
> +		goto err_drm_plane_cleanup;
> +
> +	return 0;
> +
> +err_drm_plane_cleanup:
> +	drm_plane_cleanup(primary);
> +	kfree(primary);
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_crtc_init);
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 0f14b4d3bb10..33357629a7f5 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -448,6 +448,44 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev, size_t size,
>  }
>  EXPORT_SYMBOL(__drmm_universal_plane_alloc);
>  
> +void *__drm_universal_plane_alloc(struct drm_device *dev, size_t size,
> +				  size_t offset, uint32_t possible_crtcs,
> +				  const struct drm_plane_funcs *funcs,
> +				  const uint32_t *formats, unsigned int format_count,
> +				  const uint64_t *format_modifiers,
> +				  enum drm_plane_type type,
> +				  const char *name, ...)
> +{
> +	void *container;
> +	struct drm_plane *plane;
> +	va_list ap;
> +	int ret;
> +
> +	if (drm_WARN_ON(dev, !funcs))
> +		return ERR_PTR(-EINVAL);
> +
> +	container = kzalloc(size, GFP_KERNEL);
> +	if (!container)
> +		return ERR_PTR(-ENOMEM);
> +
> +	plane = container + offset;
> +
> +	va_start(ap, name);
> +	ret = __drm_universal_plane_init(dev, plane, possible_crtcs, funcs,
> +					 formats, format_count, format_modifiers,
> +					 type, name, ap);
> +	va_end(ap);
> +	if (ret)
> +		goto err_kfree;
> +
> +	return container;
> +
> +err_kfree:
> +	kfree(container);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(__drm_universal_plane_alloc);
> +
>  int drm_plane_register_all(struct drm_device *dev)
>  {
>  	unsigned int num_planes = 0;
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 660c4cbc0b3d..6b8a014b5e97 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -1281,32 +1281,6 @@ static const struct drm_plane_funcs nv04_primary_plane_funcs = {
>  	.destroy = drm_plane_helper_destroy,
>  };
>  
> -static struct drm_plane *
> -create_primary_plane(struct drm_device *dev)
> -{
> -        struct drm_plane *primary;
> -        int ret;
> -
> -        primary = kzalloc(sizeof(*primary), GFP_KERNEL);
> -        if (primary == NULL) {
> -                DRM_DEBUG_KMS("Failed to allocate primary plane\n");
> -                return NULL;
> -        }
> -
> -        /* possible_crtc's will be filled in later by crtc_init */
> -        ret = drm_universal_plane_init(dev, primary, 0,
> -				       &nv04_primary_plane_funcs,
> -                                       modeset_formats,
> -                                       ARRAY_SIZE(modeset_formats), NULL,
> -                                       DRM_PLANE_TYPE_PRIMARY, NULL);
> -        if (ret) {
> -                kfree(primary);
> -                primary = NULL;
> -        }
> -
> -        return primary;
> -}
> -
>  static int nv04_crtc_vblank_handler(struct nvif_notify *notify)
>  {
>  	struct nouveau_crtc *nv_crtc =
> @@ -1321,6 +1295,7 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
>  {
>  	struct nouveau_display *disp = nouveau_display(dev);
>  	struct nouveau_crtc *nv_crtc;
> +	struct drm_plane *primary;
>  	int ret;
>  
>  	nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
> @@ -1335,8 +1310,18 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
>  	nv_crtc->save = nv_crtc_save;
>  	nv_crtc->restore = nv_crtc_restore;
>  
> -	drm_crtc_init_with_planes(dev, &nv_crtc->base,
> -                                  create_primary_plane(dev), NULL,
> +	primary = __drm_universal_plane_alloc(dev, sizeof(*primary), 0, 0,
> +					      &nv04_primary_plane_funcs,
> +					      modeset_formats,
> +					      ARRAY_SIZE(modeset_formats), NULL,
> +					      DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (IS_ERR(primary)) {
> +		ret = PTR_ERR(primary);
> +		kfree(nv_crtc);
> +		return ret;
> +	}
> +
> +	drm_crtc_init_with_planes(dev, &nv_crtc->base, primary, NULL,
>                                    &nv04_crtc_funcs, NULL);
>  	drm_crtc_helper_add(&nv_crtc->base, &nv04_crtc_helper_funcs);
>  	drm_mode_crtc_set_gamma_size(&nv_crtc->base, 256);
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 910cb941f3d5..21dfa7f97948 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -809,6 +809,50 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev,
>  					      format_count, format_modifiers, \
>  					      plane_type, name, ##__VA_ARGS__))
>  
> +__printf(10, 11)
> +void *__drm_universal_plane_alloc(struct drm_device *dev,
> +				  size_t size, size_t offset,
> +				  uint32_t possible_crtcs,
> +				  const struct drm_plane_funcs *funcs,
> +				  const uint32_t *formats,
> +				  unsigned int format_count,
> +				  const uint64_t *format_modifiers,
> +				  enum drm_plane_type plane_type,
> +				  const char *name, ...);
> +
> +/**
> + * drm_universal_plane_alloc - Allocate and initialize an universal plane object
> + * @dev: DRM device
> + * @type: the type of the struct which contains struct &drm_plane
> + * @member: the name of the &drm_plane within @type
> + * @possible_crtcs: bitmask of possible CRTCs
> + * @funcs: callbacks for the new plane
> + * @formats: array of supported formats (DRM_FORMAT\_\*)
> + * @format_count: number of elements in @formats
> + * @format_modifiers: array of struct drm_format modifiers terminated by
> + *                    DRM_FORMAT_MOD_INVALID
> + * @plane_type: type of plane (overlay, primary, cursor)
> + * @name: printf style format string for the plane name, or NULL for default name
> + *
> + * Allocates and initializes a plane object of type @type. The caller
> + * is responsible for releasing the allocated memory with kfree().
> + *
> + * Drivers are encouraged to use drmm_universal_plane_alloc() instead.
> + *
> + * Drivers that only support the DRM_FORMAT_MOD_LINEAR modifier support may set
> + * @format_modifiers to NULL. The plane will advertise the linear modifier.
> + *
> + * Returns:
> + * Pointer to new plane, or ERR_PTR on failure.
> + */
> +#define drm_universal_plane_alloc(dev, type, member, possible_crtcs, funcs, formats, \
> +				   format_count, format_modifiers, plane_type, name, ...) \
> +	((type *)__drm_universal_plane_alloc(dev, sizeof(type), \
> +					     offsetof(type, member), \
> +					     possible_crtcs, funcs, formats, \
> +					     format_count, format_modifiers, \
> +					     plane_type, name, ##__VA_ARGS__))
> +
>  /**
>   * drm_plane_index - find the index of a registered plane
>   * @plane: plane to find index for
> -- 
> 2.37.2
> 

-- 
Regards,

Laurent Pinchart


More information about the dri-devel mailing list