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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxx>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
