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

Chris Wilson chris at chris-wilson.co.uk
Tue Jun 21 10:31:43 UTC 2016


On Tue, Jun 21, 2016 at 11:31:34AM +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
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> ---
>  drivers/gpu/drm/drm_crtc.c | 122 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_drv.c  |   4 +-
>  include/drm/drm_crtc.h     |  79 +++++++++++++++++++++++++++++
>  3 files changed, 203 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e7c862b..b393feb 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;
> +
> +	drm_for_each_crtc(crtc, dev) {
> +		if (crtc->funcs->late_register)
> +			ret = crtc->funcs->late_register(crtc);
> +		if (ret)

ret is uninitialised.

It is a good question (probably for another series) on what exactly to
do on registration failure? At the very least we need to unwind on the
error path, or we just ignore errors (other than a DRM_ERROR to
userspace explaining why one object is not available, but otherwise let
the driver load).

> +int drm_modeset_register_all(struct drm_device *dev)
> +{
> +	int ret;
> +
> +	ret = drm_plane_register_all(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_crtc_register_all(dev);
> +	if  (ret)
> +		return ret;
> +
> +	ret = drm_encoder_register_all(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_connector_register_all(dev);

Might as well continue on with 

ret = <>
if (ret)
	return ret;

for a consistent style. Along with the comment about how should we be
handling errors? If we report an error, everything up to that point
should be unwound.

> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_modeset_register_all);
> +
> +/**
> + * drm_modeset_unregister_all - do early unregistration
> + * @dev: drm device
> + *
> + * This function call early_unregister callbakc for all planes,
> + * crtcs, encoders and connectors
> + */
> +void drm_modeset_unregister_all(struct drm_device *dev)
> +{
> +	drm_plane_unregister_all(dev);
> +	drm_crtc_unregister_all(dev);
> +	drm_encoder_unregister_all(dev);
> +	drm_connector_unregister_all(dev);

Unregister should be in the opposite order.

> +}
> +EXPORT_SYMBOL(drm_modeset_unregister_all);

I think the plan is not to export these symbols,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the dri-devel mailing list