Hi
Am 10.06.22 um 11:28 schrieb Maxime Ripard:
Let's create a DRM-managed variant of drmm_writeback_connector_init that will take care of an action of the encoder and connector cleanup.
Signed-off-by: Maxime Ripard maxime@cerno.tech
drivers/gpu/drm/drm_writeback.c | 136 ++++++++++++++++++++++++-------- include/drm/drm_writeback.h | 5 ++ 2 files changed, 108 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index dccf4504f1bb..0b00921f3379 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -149,32 +149,27 @@ static const struct drm_encoder_funcs drm_writeback_encoder_funcs = { .destroy = drm_encoder_cleanup, };
-/**
- drm_writeback_connector_init - Initialize a writeback connector and its properties
- @dev: DRM device
- @wb_connector: Writeback connector to initialize
- @con_funcs: Connector funcs vtable
- @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
- @formats: Array of supported pixel formats for the writeback engine
- @n_formats: Length of the formats array
- This function creates the writeback-connector-specific properties if they
- have not been already created, initializes the connector as
- type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
- values. It will also create an internal encoder associated with the
- drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
- the encoder helper.
- Drivers should always use this function instead of drm_connector_init() to
- set up writeback connectors.
- Returns: 0 on success, or a negative error code
- */
-int drm_writeback_connector_init(struct drm_device *dev,
struct drm_writeback_connector *wb_connector,const struct drm_connector_funcs *con_funcs,const struct drm_encoder_helper_funcs *enc_helper_funcs,const u32 *formats, int n_formats)+typedef int (*encoder_init_t)(struct drm_device *dev,
struct drm_encoder *encoder,const struct drm_encoder_funcs *funcs,int encoder_type,const char *name, ...);+typedef int (*connector_init_t)(struct drm_device *dev,
struct drm_connector *connector,const struct drm_connector_funcs *funcs,int connector_type);
This code has menawhile changed in drm-tip, which makes it harder to give a good review for such refactoring.
But generally, I'd do the connector and encoder initialization in drmm_writeback_connector_init() and give the initialized encoders to an internal helper that does the common init steps. That avoids such indirections with functions pointers.
Best regards Thomas
+static int writeback_init(struct drm_device *dev,
struct drm_writeback_connector *wb_connector,connector_init_t conn_init,void (*conn_destroy)(struct drm_connector *connector),encoder_init_t enc_init,void (*enc_destroy)(struct drm_encoder *encoder),const struct drm_connector_funcs *con_funcs,const struct drm_encoder_funcs *enc_funcs,const struct drm_encoder_helper_funcs *enc_helper_funcs,const u32 *formats, int n_formats)@@ -190,16 +185,16 @@ int drm_writeback_connector_init(struct drm_device *dev, return PTR_ERR(blob);
drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
- ret = drm_encoder_init(dev, &wb_connector->encoder,
&drm_writeback_encoder_funcs,DRM_MODE_ENCODER_VIRTUAL, NULL);
ret = enc_init(dev, &wb_connector->encoder,
enc_funcs,DRM_MODE_ENCODER_VIRTUAL, NULL);if (ret) goto fail;
connector->interlace_allowed = 0;
- ret = drm_connector_init(dev, connector, con_funcs,
DRM_MODE_CONNECTOR_WRITEBACK);
- ret = conn_init(dev, connector, con_funcs,
DRM_MODE_CONNECTOR_WRITEBACK);@@ -231,15 +226,90 @@ int drm_writeback_connector_init(struct drm_device *dev, return 0;
attach_fail:
- drm_connector_cleanup(connector);
- if (conn_destroy)
conn_destroy(connector);
- drm_encoder_cleanup(&wb_connector->encoder);
- if (enc_destroy)
enc_destroy(&wb_connector->encoder);+/**
- drm_writeback_connector_init - Initialize a writeback connector and its properties
- @dev: DRM device
- @wb_connector: Writeback connector to initialize
- @con_funcs: Connector funcs vtable
- @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
- @formats: Array of supported pixel formats for the writeback engine
- @n_formats: Length of the formats array
- This function creates the writeback-connector-specific properties if they
- have not been already created, initializes the connector as
- type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
- values. It will also create an internal encoder associated with the
- drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
- the encoder helper.
- Drivers should always use this function instead of drm_connector_init() to
- set up writeback connectors.
- Returns: 0 on success, or a negative error code
- */
+int drm_writeback_connector_init(struct drm_device *dev,
struct drm_writeback_connector *wb_connector,const struct drm_connector_funcs *con_funcs,const struct drm_encoder_helper_funcs *enc_helper_funcs,const u32 *formats, int n_formats)+{
- return writeback_init(dev, wb_connector,
drm_connector_init, drm_connector_cleanup,drm_encoder_init, drm_encoder_cleanup,con_funcs,&drm_writeback_encoder_funcs, enc_helper_funcs,formats, n_formats);+} EXPORT_SYMBOL(drm_writeback_connector_init);
+/**
- drmm_writeback_connector_init - Initialize a writeback connector and its properties
- @dev: DRM device
- @wb_connector: Writeback connector to initialize
- @con_funcs: Connector funcs vtable
- @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
- @formats: Array of supported pixel formats for the writeback engine
- @n_formats: Length of the formats array
- This function creates the writeback-connector-specific properties if they
- have not been already created, initializes the connector as
- type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
- values. It will also create an internal encoder associated with the
- drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
- the encoder helper.
- The writeback connector is DRM-managed and won't need any cleanup.
- Drivers should always use this function instead of drm_connector_init() to
- set up writeback connectors.
- Returns: 0 on success, or a negative error code
- */
+int drmm_writeback_connector_init(struct drm_device *dev,
struct drm_writeback_connector *wb_connector,const struct drm_connector_funcs *con_funcs,const struct drm_encoder_helper_funcs *enc_helper_funcs,const u32 *formats, int n_formats)+{
- return writeback_init(dev, wb_connector,
drmm_connector_init, NULL,drmm_encoder_init, NULL,con_funcs,NULL, enc_helper_funcs,formats, n_formats);+} +EXPORT_SYMBOL(drmm_writeback_connector_init);
- int drm_writeback_set_fb(struct drm_connector_state *conn_state, struct drm_framebuffer *fb) {
diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index 9697d2714d2a..cc259ae44734 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -151,6 +151,11 @@ int drm_writeback_connector_init(struct drm_device *dev, const struct drm_connector_funcs *con_funcs, const struct drm_encoder_helper_funcs *enc_helper_funcs, const u32 *formats, int n_formats); +int drmm_writeback_connector_init(struct drm_device *dev,
struct drm_writeback_connector *wb_connector,const struct drm_connector_funcs *con_funcs,const struct drm_encoder_helper_funcs *enc_helper_funcs,const u32 *formats, int n_formats);int drm_writeback_set_fb(struct drm_connector_state *conn_state, struct drm_framebuffer *fb);