Add an alternative to drm_universal_plane_init() that allocates and initializes a plane and registers drm_plane_cleanup() with drmm_add_action_or_reset().

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- Changes since v1: - add __printf annotation to __drm_universal_plane_init() and make it static --- drivers/gpu/drm/drm_plane.c | 130 ++++++++++++++++++++++++++++-------- include/drm/drm_plane.h | 42 ++++++++++++ 2 files changed, 143 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index b7b90b3a2e38..ddf7f0724260 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -30,6 +30,7 @@ #include <drm/drm_file.h> #include <drm/drm_crtc.h> #include <drm/drm_fourcc.h> +#include <drm/drm_managed.h> #include <drm/drm_vblank.h>

#include "drm_crtc_internal.h" @@ -152,31 +153,16 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane return 0; }

-/** - * drm_universal_plane_init - Initialize a new universal plane object - * @dev: DRM device - * @plane: plane object to init - * @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 - * @type: type of plane (overlay, primary, cursor) - * @name: printf style format string for the plane name, or NULL for default name - * - * Initializes a plane object of type @type. - * - * Returns: - * Zero on success, error code on failure. - */ -int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, - 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, ...) +__printf(9, 0) +static int __drm_universal_plane_init(struct drm_device *dev, + struct drm_plane *plane, + 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, va_list ap) { struct drm_mode_config *config = &dev->mode_config; unsigned int format_modifier_count = 0; @@ -237,11 +223,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, }

if (name) { - va_list ap; - - va_start(ap, name); plane->name = kvasprintf(GFP_KERNEL, name, ap); - va_end(ap); } else { plane->name = kasprintf(GFP_KERNEL, "plane-%d", drm_num_planes(dev)); @@ -286,8 +268,98 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,

return 0; } + +/** + * drm_universal_plane_init - Initialize a new universal plane object + * @dev: DRM device + * @plane: plane object to init + * @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 + * @type: type of plane (overlay, primary, cursor) + * @name: printf style format string for the plane name, or NULL for default name + * + * Initializes a plane object of type @type. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, + 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, ...) +{ + va_list ap; + int ret; + + if (name) + va_start(ap, name); + ret = __drm_universal_plane_init(dev, plane, possible_crtcs, funcs, + formats, format_count, format_modifiers, + type, name, ap); + if (name) + va_end(ap); + return ret; +} EXPORT_SYMBOL(drm_universal_plane_init);

+static void drmm_universal_plane_alloc_release(struct drm_device *dev, void *ptr) +{ + struct drm_plane *plane = ptr; + + if (WARN_ON(!plane->dev)) + return; + + drm_plane_cleanup(plane); +} + +void *__drmm_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 (!funcs || funcs->destroy) + return ERR_PTR(-EINVAL); + + container = drmm_kzalloc(dev, size, GFP_KERNEL); + if (!container) + return ERR_PTR(-ENOMEM); + + plane = container + offset; + + if (name) + va_start(ap, name); + ret = __drm_universal_plane_init(dev, plane, possible_crtcs, funcs, + formats, format_count, format_modifiers, + type, name, ap); + if (name) + va_end(ap); + if (ret) + return ERR_PTR(ret); + + ret = drmm_add_action_or_reset(dev, drmm_universal_plane_alloc_release, + plane); + if (ret) + return ERR_PTR(ret); + + return container; +} +EXPORT_SYMBOL(__drmm_universal_plane_alloc); + 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 3f396d94afe4..82bd63710a39 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -746,6 +746,48 @@ int drm_plane_init(struct drm_device *dev, bool is_primary); void drm_plane_cleanup(struct drm_plane *plane);

+__printf(10, 11) +void *__drmm_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, ...); + +/** + * drmm_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. Cleanup is + * automatically handled through registering drm_plane_cleanup() with + * drmm_add_action(). + * + * The @drm_plane_funcs.destroy hook must be NULL. + * + * Returns: + * Pointer to new plane, or ERR_PTR on failure. + */ +#define drmm_universal_plane_alloc(dev, type, member, possible_crtcs, funcs, formats, \ + format_count, format_modifiers, plane_type, name, ...) \ + ((type *)__drmm_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

Add an alternative to drm_crtc_init_with_planes() that allocates and initializes a crtc and registers drm_crtc_cleanup() with drmm_add_action_or_reset().

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de --- Changes since v1: - fix drm_crtc_init_with_planes() to actually call __drm_crtc_init_with_planes() instead of itself - add __printf annotation to __drm_crtc_init_with_planes() --- drivers/gpu/drm/drm_crtc.c | 120 ++++++++++++++++++++++++++++--------- include/drm/drm_crtc.h | 33 ++++++++++ 2 files changed, 125 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 283bcc4362ca..c245746ccc46 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -38,6 +38,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_edid.h> #include <drm/drm_fourcc.h> +#include <drm/drm_managed.h> #include <drm/drm_modeset_lock.h> #include <drm/drm_atomic.h> #include <drm/drm_auth.h> @@ -231,30 +232,12 @@ struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc) * Setting MODE_ID to 0 will release reserved resources for the CRTC. */

-/** - * drm_crtc_init_with_planes - Initialise a new CRTC object with - * specified primary and cursor planes. - * @dev: DRM device - * @crtc: CRTC object to init - * @primary: Primary plane for CRTC - * @cursor: Cursor plane for CRTC - * @funcs: callbacks for the new CRTC - * @name: printf style format string for the CRTC name, or NULL for default name - * - * Inits a new object created as base part of a driver crtc object. Drivers - * should use this function instead of drm_crtc_init(), which is only provided - * for backwards compatibility with drivers which do not yet support universal - * planes). For really simple hardware which has only 1 plane look at - * drm_simple_display_pipe_init() instead. - * - * Returns: - * Zero on success, error code on failure. - */ -int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, - struct drm_plane *primary, - struct drm_plane *cursor, - const struct drm_crtc_funcs *funcs, - const char *name, ...) +__printf(6, 0) +static int __drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, + struct drm_plane *primary, + struct drm_plane *cursor, + const struct drm_crtc_funcs *funcs, + const char *name, va_list ap) { struct drm_mode_config *config = &dev->mode_config; int ret; @@ -282,11 +265,7 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, return ret;

if (name) { - va_list ap; - - va_start(ap, name); crtc->name = kvasprintf(GFP_KERNEL, name, ap); - va_end(ap); } else { crtc->name = kasprintf(GFP_KERNEL, "crtc-%d", drm_num_crtcs(dev)); @@ -330,8 +309,93 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,

return 0; } + +/** + * drm_crtc_init_with_planes - Initialise a new CRTC object with + * specified primary and cursor planes. + * @dev: DRM device + * @crtc: CRTC object to init + * @primary: Primary plane for CRTC + * @cursor: Cursor plane for CRTC + * @funcs: callbacks for the new CRTC + * @name: printf style format string for the CRTC name, or NULL for default name + * + * Inits a new object created as base part of a driver crtc object. Drivers + * should use this function instead of drm_crtc_init(), which is only provided + * for backwards compatibility with drivers which do not yet support universal + * planes). For really simple hardware which has only 1 plane look at + * drm_simple_display_pipe_init() instead. + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, + struct drm_plane *primary, + struct drm_plane *cursor, + const struct drm_crtc_funcs *funcs, + const char *name, ...) +{ + va_list ap; + int ret; + + if (name) + va_start(ap, name); + ret = __drm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs, + name, ap); + if (name) + va_end(ap); + + return ret; +} EXPORT_SYMBOL(drm_crtc_init_with_planes);

+static void drmm_crtc_alloc_with_planes_cleanup(struct drm_device *dev, + void *ptr) +{ + struct drm_crtc *crtc = ptr; + + drm_crtc_cleanup(crtc); +} + +void *__drmm_crtc_alloc_with_planes(struct drm_device *dev, + size_t size, size_t offset, + struct drm_plane *primary, + struct drm_plane *cursor, + const struct drm_crtc_funcs *funcs, + const char *name, ...) +{ + void *container; + struct drm_crtc *crtc; + va_list ap; + int ret; + + if (!funcs || funcs->destroy) + return ERR_PTR(-EINVAL); + + container = drmm_kzalloc(dev, size, GFP_KERNEL); + if (!container) + return ERR_PTR(-ENOMEM); + + crtc = container + offset; + + if (name) + va_start(ap, name); + ret = __drm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs, + name, ap); + if (name) + va_end(ap); + if (ret) + return ERR_PTR(ret); + + ret = drmm_add_action_or_reset(dev, drmm_crtc_alloc_with_planes_cleanup, + crtc); + if (ret) + return ERR_PTR(ret); + + return container; +} +EXPORT_SYMBOL(__drmm_crtc_alloc_with_planes); + /** * drm_crtc_cleanup - Clean up the core crtc usage * @crtc: CRTC to cleanup diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 59b51a09cae6..b71c0a3d4126 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1210,6 +1210,39 @@ int drm_crtc_init_with_planes(struct drm_device *dev, const char *name, ...); void drm_crtc_cleanup(struct drm_crtc *crtc);

+__printf(7, 8) +void *__drmm_crtc_alloc_with_planes(struct drm_device *dev, + size_t size, size_t offset, + struct drm_plane *primary, + struct drm_plane *cursor, + const struct drm_crtc_funcs *funcs, + const char *name, ...); + +/** + * drm_crtc_alloc_with_planes - Allocate and initialize a new CRTC object with + * specified primary and cursor planes. + * @dev: DRM device + * @type: the type of the struct which contains struct &drm_crtc + * @member: the name of the &drm_crtc within @type. + * @primary: Primary plane for CRTC + * @cursor: Cursor plane for CRTC + * @funcs: callbacks for the new CRTC + * @name: printf style format string for the CRTC name, or NULL for default name + * + * Allocates and initializes a new crtc object. Cleanup is automatically + * handled through registering drmm_crtc_cleanup() with drmm_add_action(). + * + * The @drm_crtc_funcs.destroy hook must be NULL. + * + * Returns: + * Pointer to new crtc, or ERR_PTR on failure. + */ +#define drmm_crtc_alloc_with_planes(dev, type, member, primary, cursor, funcs, name, ...) \ + ((type *)__drmm_crtc_alloc_with_planes(dev, sizeof(type), \ + offsetof(type, member), \ + primary, cursor, funcs, \ + name, ##__VA_ARGS__)) + /** * drm_crtc_index - find the index of a registered CRTC * @crtc: CRTC to find index for

On Thu, Aug 27, 2020 at 06:05:42PM +0200, Philipp Zabel wrote:

Add an alternative to drm_encoder_init() that allocates and initializes an encoder and registers drm_encoder_cleanup() with drmm_add_action_or_reset().

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

Changes since v1:

• add __printf annotation to __drm_encoder_init()

I think these are the ones we want, but much easier to judge if the series comes with an example conversion and corresponding cleanup of driver load/unload code. With that we'd see whether anything silly is left sticking behind, or whether we have some cleanup inversion issues that break it all.

Cheers, Daniel

---

drivers/gpu/drm/drm_encoder.c | 105 ++++++++++++++++++++++++++-------- include/drm/drm_encoder.h | 30 ++++++++++ 2 files changed, 112 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index e555281f43d4..de8c59087721 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -26,6 +26,7 @@ #include <drm/drm_device.h> #include <drm/drm_drv.h> #include <drm/drm_encoder.h> +#include <drm/drm_managed.h>

#include "drm_crtc_internal.h"

@@ -91,25 +92,11 @@ void drm_encoder_unregister_all(struct drm_device *dev) } }

-/**

• • drm_encoder_init - Init a preallocated encoder
• • @dev: drm device
• • @encoder: the encoder to init
• • @funcs: callbacks for this encoder
• • @encoder_type: user visible type of the encoder
• • @name: printf style format string for the encoder name, or NULL for default name
• • Initialises a preallocated encoder. Encoder should be subclassed as part of
• • driver encoder objects. At driver unload time drm_encoder_cleanup() should be
• • called from the driver's &drm_encoder_funcs.destroy hook.
• • Returns:
• • Zero on success, error code on failure.
• */

-int drm_encoder_init(struct drm_device *dev,

•      struct drm_encoder *encoder,
  

•      const struct drm_encoder_funcs *funcs,
  

•      int encoder_type, const char *name, ...)
  

+__printf(5, 0) +static int __drm_encoder_init(struct drm_device *dev,

• 	      struct drm_encoder *encoder,
  

• 	      const struct drm_encoder_funcs *funcs,
  

• 	      int encoder_type, const char *name, va_list ap)
  

{ int ret;

@@ -125,11 +112,7 @@ int drm_encoder_init(struct drm_device *dev, encoder->encoder_type = encoder_type; encoder->funcs = funcs; if (name) {

• va_list ap;
  

• va_start(ap, name);
  

  encoder->name = kvasprintf(GFP_KERNEL, name, ap);

• va_end(ap);
  

  } else { encoder->name = kasprintf(GFP_KERNEL, "%s-%d", drm_encoder_enum_list[encoder_type].name,

@@ -150,6 +133,38 @@ int drm_encoder_init(struct drm_device *dev,

return ret; }

+/**

• • drm_encoder_init - Init a preallocated encoder
• • @dev: drm device
• • @encoder: the encoder to init
• • @funcs: callbacks for this encoder
• • @encoder_type: user visible type of the encoder
• • @name: printf style format string for the encoder name, or NULL for default name
• • Initializes a preallocated encoder. Encoder should be subclassed as part of
• • driver encoder objects. At driver unload time drm_encoder_cleanup() should be
• • called from the driver's &drm_encoder_funcs.destroy hook.
• • Returns:
• • Zero on success, error code on failure.
• */

+int drm_encoder_init(struct drm_device *dev,

•      struct drm_encoder *encoder,
  

•      const struct drm_encoder_funcs *funcs,
  

•      int encoder_type, const char *name, ...)
  

+{

• va_list ap;
• int ret;
• if (name)

• va_start(ap, name);
  

• ret = __drm_encoder_init(dev, encoder, funcs, encoder_type, name, ap);
• if (name)

• va_end(ap);
  

• return ret;

+} EXPORT_SYMBOL(drm_encoder_init);

/** @@ -181,6 +196,50 @@ void drm_encoder_cleanup(struct drm_encoder *encoder) } EXPORT_SYMBOL(drm_encoder_cleanup);

+static void drmm_encoder_alloc_release(struct drm_device *dev, void *ptr) +{

• struct drm_encoder *encoder = ptr;
• if (WARN_ON(!encoder->dev))

• return;
  

• drm_encoder_cleanup(encoder);

+}

+void *__drmm_encoder_alloc(struct drm_device *dev, size_t size, size_t offset,

• 	   const struct drm_encoder_funcs *funcs,
  

• 	   int encoder_type, const char *name, ...)
  

+{

• void *container;
• struct drm_encoder *encoder;
• va_list ap;
• int ret;
• if (WARN_ON(!funcs || funcs->destroy))

• return ERR_PTR(-EINVAL);
  

• container = drmm_kzalloc(dev, size, GFP_KERNEL);
• if (!container)

• return ERR_PTR(-EINVAL);
  

• encoder = container + offset;
• if (name)

• va_start(ap, name);
  

• ret = __drm_encoder_init(dev, encoder, funcs, encoder_type, name, ap);
• if (name)

• va_end(ap);
  

• if (ret)

• return ERR_PTR(ret);
  

• ret = drmm_add_action_or_reset(dev, drmm_encoder_alloc_release, encoder);
• if (ret)

• return ERR_PTR(ret);
  

• return container;

+} +EXPORT_SYMBOL(__drmm_encoder_alloc);

static struct drm_crtc *drm_encoder_get_crtc(struct drm_encoder *encoder) { struct drm_connector *connector; diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h index a60f5f1555ac..4ecad1260ff7 100644 --- a/include/drm/drm_encoder.h +++ b/include/drm/drm_encoder.h @@ -195,6 +195,36 @@ int drm_encoder_init(struct drm_device *dev, const struct drm_encoder_funcs *funcs, int encoder_type, const char *name, ...);

+__printf(6, 7) +void *__drmm_encoder_alloc(struct drm_device *dev,

• 	   size_t size, size_t offset,
  

• 	   const struct drm_encoder_funcs *funcs,
  

• 	   int encoder_type,
  

• 	   const char *name, ...);
  

+/**

• • drmm_encoder_alloc - Allocate and initialize an encoder
• • @dev: drm device
• • @type: the type of the struct which contains struct &drm_encoder
• • @member: the name of the &drm_encoder within @type.
• • @funcs: callbacks for this encoder
• • @encoder_type: user visible type of the encoder
• • @name: printf style format string for the encoder name, or NULL for default name
• • Allocates and initializes an encoder. Encoder should be subclassed as part of
• • driver encoder objects. Cleanup is automatically handled through registering
• • drm_encoder_cleanup() with drmm_add_action().
• • The @drm_encoder_funcs.destroy hook must be NULL.
• • Returns:
• • Pointer to new encoder, or ERR_PTR on failure.
• */

+#define drmm_encoder_alloc(dev, type, member, funcs, encoder_type, name, ...) \

• ((type *)__drmm_encoder_alloc(dev, sizeof(type), \

• 		      offsetof(type, member), funcs, \
  

• 		      encoder_type, name, ##__VA_ARGS__))
  

/**

• drm_encoder_index - find the index of a registered encoder
• @encoder: encoder to find index for

-- 2.20.1