[PATCH v3 1/3] drm: Add callbacks for late registering

[Chris Wilson]

On Tue, Jun 21, 2016 at 03:09:38PM +0200, Benjamin Gaignard wrote:
> Like what has been done for connectors add callbacks on encoder,
> crtc and plane to let driver do actions after drm device registration.
> 
> Correspondingly, add callbacks called before unregister drm device.
> 
> version 2:
> add drm_modeset_register_all() and drm_modeset_unregister_all()
> to centralize all calls
> 
> version 3:
> in error case unwind registers in drm_modeset_register_all
> fix uninitialed return value
> inverse order of unregistration in drm_modeset_unregister_all
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> ---
>  drivers/gpu/drm/drm_crtc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_drv.c  |   4 +-
>  include/drm/drm_crtc.h     |  79 +++++++++++++++++++++++++++
>  3 files changed, 213 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e7c862b..c078bc4 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev)
>  	return num;
>  }
>  
> +static int drm_crtc_register_all(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	int ret = 0;
> +
> +	drm_for_each_crtc(crtc, dev) {
> +		if (crtc->funcs->late_register)
> +			ret = crtc->funcs->late_register(crtc);
> +		if (ret)

What happened to the unwind here?

> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +/**
> + * drm_modeset_register_all - do late registration
> + * @dev: drm device
> + *
> + * This function call late_register callback for all planes,
> + * crcts, encoders and connectors
> + *
> + * Returns:
> + * Zero on success, erro code on failure
> + */
> +int drm_modeset_register_all(struct drm_device *dev)
> +{
> +	int ret;
> +
> +	ret = drm_plane_register_all(dev);
> +	if (ret)
> +		goto err_plane;
> +
> +	ret = drm_crtc_register_all(dev);
> +	if  (ret)
> +		goto err_crtc;
> +
> +	ret = drm_encoder_register_all(dev);
> +	if (ret)
> +		goto err_encoder;
> +
> +	ret = drm_connector_register_all(dev);
> +	if (ret)
> +		goto err_connector;
> +
> +	return 0;
> +
> +err_connector:
> +	drm_encoder_unregister_all(dev);

The name here should be what we are about to free. That just makes it a
bit easier to change the sequence later (if adding new stags).

Looks good enough though,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris
 

-- 
Chris Wilson, Intel Open Source Technology Centre