Re: [PATCH 2/2] drm: mali-dp: Set the drm->irq_enabled flag to match driver's state.

[Daniel Vetter]

On Fri, Jul 29, 2016 at 03:44:16PM +0100, Liviu Dudau wrote:
> Mali DP driver does not use drm_irq_{un,}install() function so the
> drm->irq_enabled flag does not get managed automatically. Among other
> DRM framework functions, drm_wait_vblank() checks the value of the
> flag and return -EINVAL if not set.
> 
> Cc: Brian Starkey <Brian.Starkey@xxxxxxx>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@xxxxxxx>
> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 87c9249..a1c01e4 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -377,6 +377,8 @@ static int malidp_bind(struct device *dev)
>  	if (ret < 0)
>  		goto irq_init_fail;
>  
> +	drm->irq_enabled = true;

I wonder whether we should have a cocci script or something with catching
some of these issues that every new driver seems to hit. Another one is
filling out legacy helpers for all the legacy entry points using atomic
functions. Or still using ->load/unload. Documentation is great, but it
seems like most folks don't read it until they hit a snag ;-)

Just idle musings ...
-Daniel

> +
>  	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
>  	if (ret < 0) {
>  		DRM_ERROR("failed to initialise vblank\n");
> @@ -402,6 +404,7 @@ fbdev_fail:
>  vblank_fail:
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
> +	drm->irq_enabled = false;
>  irq_init_fail:
>  	component_unbind_all(dev, drm);
>  bind_fail:
> @@ -439,6 +442,7 @@ static void malidp_unbind(struct device *dev)
>  	drm_kms_helper_poll_fini(drm);
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
> +	drm->irq_enabled = false;
>  	drm_vblank_cleanup(drm);
>  	component_unbind_all(dev, drm);
>  	of_node_put(malidp->crtc.port);
> -- 
> 2.9.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch