[PATCH 08/64] drm/writeback: Introduce drmm_writeback_connector_init
Thomas Zimmermann
tzimmermann at suse.de
Mon Jun 20 10:33:32 UTC 2022
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 at 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)
> {
> struct drm_property_blob *blob;
> struct drm_connector *connector = &wb_connector->base;
> @@ -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);
> if (ret)
> goto connector_fail;
>
> @@ -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);
> connector_fail:
> - drm_encoder_cleanup(&wb_connector->encoder);
> + if (enc_destroy)
> + enc_destroy(&wb_connector->encoder);
> fail:
> drm_property_blob_put(blob);
> return ret;
> }
> +
> +/**
> + * 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);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220620/ae07e8aa/attachment-0001.sig>
More information about the dri-devel
mailing list