Re: [PATCH 06/27] drm/sun4i: Implement some semblance of vblank event handling

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Wednesday 08 Jun 2016 14:18:58 Daniel Vetter wrote:
> atomic_flush seems to be the right place, right after we commit the
> plane updates. Again use the fullproof version, since the pipe might
> be off.
> 
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_crtc.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c
> b/drivers/gpu/drm/sun4i/sun4i_crtc.c index 4182a21f5923..f628b6d8f23f
> 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> @@ -51,10 +51,22 @@ static void sun4i_crtc_atomic_flush(struct drm_crtc
> *crtc, {
>  	struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
>  	struct sun4i_drv *drv = scrtc->drv;
> +	struct drm_pending_vblank_event *event = crtc->state->event;
> 
>  	DRM_DEBUG_DRIVER("Committing plane changes\n");
> 
>  	sun4i_backend_commit(drv->backend);
> +
> +	if (event) {
> +		crtc->state->event = NULL;
> +
> +		spin_lock_irq(&crtc->dev->event_lock);
> +		if (drm_crtc_vblank_get(crtc) == 0)
> +			drm_crtc_arm_vblank_event(crtc, event);
> +		else
> +			drm_crtc_send_vblank_event(crtc, event);
> +		spin_unlock_irq(&crtc->dev->event_lock);
> +	}

Is it me, or is it entirely useless given the code in 
sun4i_crtc_atomic_flush() that sets crtc->state->event to NULL ?

AuthorDate: Wed Jun 8 14:18:58 2016 +0200
CommitDate: Fri Jun 10 16:55:48 2016 +0200

It would have been worth it waiting for the maintainers to review the patch...

You've added similar code to a bunch of drivers. How do you protect against 
the race conditions documented in the drm_crtc_arm_vblank_event() function ?

>  }
> 
>  static void sun4i_crtc_disable(struct drm_crtc *crtc)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel