Re: [PATCH 2/4] drm: adp: Handle drm_crtc_vblank_get() errors

[Alyssa Rosenzweig]

> -	spin_lock_irqsave(&crtc->dev->event_lock, flags);
>  	if (crtc->state->event) {
> -		drm_crtc_vblank_get(crtc);
> -		adp->event = crtc->state->event;
> +		spin_lock_irqsave(&crtc->dev->event_lock, flags);
> +
> +		if (drm_crtc_vblank_get(crtc) != 0)
> +			drm_crtc_send_vblank_event(crtc, crtc->state->event);
> +		else
> +			adp->event = crtc->state->event;
> +
> +		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>  	}
>  	crtc->state->event = NULL;
> -	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);

Kind of confused about

>  	crtc->state->event = NULL;

now being out of the lock. Should we set to NULL in the if, since
if we don't take the if, we know event is already NULL? Or should we
hold the lock for the whole time, the way the code did before your
change? I'm not sure between the two, but the in-between here smells
wrong.