Re: [PATCH] drm/simple_kms_helper: add drmm_simple_encoder_init()

Hi

Am 22.07.20 um 15:25 schrieb Philipp Zabel:

Add a drm_simple_encoder_init() variant that registers drm_encoder_cleanup() with drmm_add_action().

Now drivers can store encoders in memory allocated with drmm_kmalloc() after the call to drmm_mode_config_init(), without having to manually make sure that drm_encoder_cleanup() is called before the memory is freed.

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

drivers/gpu/drm/drm_simple_kms_helper.c | 42 +++++++++++++++++++++++++ include/drm/drm_simple_kms_helper.h | 4 +++ 2 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 74946690aba4..a243f00cf63d 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -9,6 +9,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> +#include <drm/drm_managed.h> #include <drm/drm_plane_helper.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> @@ -71,6 +72,47 @@ int drm_simple_encoder_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_simple_encoder_init);

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

• struct drm_encoder *encoder = ptr;
• drm_encoder_cleanup(encoder);

+}

This doesn't work. DRM cleans up the encoder by invoking the destroy callback from the encoder functions. This additional helper would cleanup the encoder a second time.

You can already embed the encoder in another structure and things should work as expected.

Best regards Thomas

+/**

• • drmm_simple_encoder_init - Initialize a preallocated encoder with

• •                        basic functionality.
    

• • @dev: drm device
• • @encoder: the encoder to initialize
• • @encoder_type: user visible type of the encoder
• • Initialises a preallocated encoder that has no further functionality.
• • Settings for possible CRTC and clones are left to their initial values.
• • Cleanup is automatically handled through registering drm_encoder_cleanup()
• • with drmm_add_action().
• • The caller of drmm_simple_encoder_init() is responsible for allocating
• • the encoder's memory with drmm_kzalloc() to ensure it is automatically
• • freed after the encoder has been cleaned up.
• • Returns:
• • Zero on success, error code on failure.
• */

+int drmm_simple_encoder_init(struct drm_device *dev,

• 	     struct drm_encoder *encoder,
  

• 	     int encoder_type)
  

+{

• int ret;
• ret = drm_encoder_init(dev, encoder, &drm_simple_encoder_funcs_cleanup,

• 	       encoder_type, NULL);
  

• if (ret)

• return ret;
  

• return drmm_add_action_or_reset(dev, drmm_encoder_cleanup, encoder);

+} +EXPORT_SYMBOL(drmm_simple_encoder_init);

static enum drm_mode_status drm_simple_kms_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode) diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h index a026375464ff..27f0915599c8 100644 --- a/include/drm/drm_simple_kms_helper.h +++ b/include/drm/drm_simple_kms_helper.h @@ -185,4 +185,8 @@ int drm_simple_encoder_init(struct drm_device *dev, struct drm_encoder *encoder, int encoder_type);

+int drmm_simple_encoder_init(struct drm_device *dev,

• 	     struct drm_encoder *encoder,
  

• 	     int encoder_type);
  

#endif /* __LINUX_DRM_SIMPLE_KMS_HELPER_H */