[PATCH v2 08/68] drm/connector: Introduce drmm_connector_init_with_ddc

[Thomas Zimmermann]

Hi

Am 22.06.22 um 16:31 schrieb Maxime Ripard:
> Let's create a DRM-managed variant of drm_connector_init_with_ddc that will
> take care of an action of the connector cleanup.
> 
> Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> ---
>   drivers/gpu/drm/drm_connector.c | 74 ++++++++++++++++++++++++++++-----
>   include/drm/drm_connector.h     |  5 +++
>   2 files changed, 69 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 0fec2d87178f..076ca247c6d0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -354,6 +354,30 @@ int drm_connector_init(struct drm_device *dev,
>   }
>   EXPORT_SYMBOL(drm_connector_init);
>   
> +typedef int (*connector_init_t)(struct drm_device *dev,
> +				struct drm_connector *connector,
> +				const struct drm_connector_funcs *funcs,
> +				int connector_type);
> +
> +static int __drm_connector_init_with_ddc(struct drm_device *dev,
> +					 struct drm_connector *connector,
> +					 connector_init_t init_func,
> +					 const struct drm_connector_funcs *funcs,
> +					 int connector_type,
> +					 struct i2c_adapter *ddc)

Back in the days when drm_connector_init_with_ddc() was added, there was 
a small controversy about whether we should simply extend the regular 
drm_connector_init() to support the extra ddc argument. That would have 
meant a lot of churn, but the idea was probably sound.

Maybe it would be better to provide a single function 
drmm_connector_init() that receives a ddc argument. If the argument is 
NULL, no DDC channel would be set. This would make 
drmm_connector_init_with_ddc() unnnecessary.

Best regards
Thomas

> +{
> +	int ret;
> +
> +	ret = init_func(dev, connector, funcs, connector_type);
> +	if (ret)
> +		return ret;
> +
> +	/* provide ddc symlink in sysfs */
> +	connector->ddc = ddc;
> +
> +	return ret;
> +}
> +
>   /**
>    * drm_connector_init_with_ddc - Init a preallocated connector
>    * @dev: DRM device
> @@ -371,6 +395,10 @@ EXPORT_SYMBOL(drm_connector_init);
>    *
>    * Ensures that the ddc field of the connector is correctly set.
>    *
> + * Note: consider using drmm_connector_init_with_ddc() instead of
> + * drm_connector_init_with_ddc() to let the DRM managed resource
> + * infrastructure take care of cleanup and deallocation.
> + *
>    * Returns:
>    * Zero on success, error code on failure.
>    */
> @@ -380,16 +408,9 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
>   				int connector_type,
>   				struct i2c_adapter *ddc)
>   {
> -	int ret;
> -
> -	ret = drm_connector_init(dev, connector, funcs, connector_type);
> -	if (ret)
> -		return ret;
> -
> -	/* provide ddc symlink in sysfs */
> -	connector->ddc = ddc;
> -
> -	return ret;
> +	return __drm_connector_init_with_ddc(dev, connector,
> +					     drm_connector_init,
> +					     funcs, connector_type, ddc);
>   }
>   EXPORT_SYMBOL(drm_connector_init_with_ddc);
>   
> @@ -443,6 +464,39 @@ int drmm_connector_init(struct drm_device *dev,
>   }
>   EXPORT_SYMBOL(drmm_connector_init);
>   
> +/**
> + * drmm_connector_init_with_ddc - Init a preallocated connector
> + * @dev: DRM device
> + * @connector: the connector to init
> + * @funcs: callbacks for this connector
> + * @connector_type: user visible type of the connector
> + * @ddc: pointer to the associated ddc adapter
> + *
> + * Initialises a preallocated connector. Connectors should be
> + * subclassed as part of driver connector objects.
> + *
> + * Cleanup is automatically handled with a call to
> + * drm_connector_cleanup() in a DRM-managed action.
> + *
> + * The connector structure should be allocated with drmm_kzalloc().
> + *
> + * Ensures that the ddc field of the connector is correctly set.
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drmm_connector_init_with_ddc(struct drm_device *dev,
> +				 struct drm_connector *connector,
> +				 const struct drm_connector_funcs *funcs,
> +				 int connector_type,
> +				 struct i2c_adapter *ddc)
> +{
> +	return __drm_connector_init_with_ddc(dev, connector,
> +					     drmm_connector_init,
> +					     funcs, connector_type, ddc);
> +}
> +EXPORT_SYMBOL(drmm_connector_init_with_ddc);
> +
>   /**
>    * drm_connector_attach_edid_property - attach edid property.
>    * @connector: the connector
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 35a6b6e944b7..2565541f2c10 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1676,6 +1676,11 @@ int drmm_connector_init(struct drm_device *dev,
>   			struct drm_connector *connector,
>   			const struct drm_connector_funcs *funcs,
>   			int connector_type);
> +int drmm_connector_init_with_ddc(struct drm_device *dev,
> +				 struct drm_connector *connector,
> +				 const struct drm_connector_funcs *funcs,
> +				 int connector_type,
> +				 struct i2c_adapter *ddc);
>   void drm_connector_attach_edid_property(struct drm_connector *connector);
>   int drm_connector_register(struct drm_connector *connector);
>   void drm_connector_unregister(struct drm_connector *connector);

-- 
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/20220628/a8fc56d4/attachment.sig>