Re: [PATCH 4/5] drm/radeon: rework page flip handling

["Michel Dänzer" <michel@daenzer.net>]

On 29.04.2014 23:29, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
> 
> Instead of trying to flip inside the vblank period when
> the buffer is idle, offload blocking for idle to a kernel
> thread and program the flip directly into the hardware.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
[...]
> +/**
> + * radeon_flip_work_func - page flip framebuffer
> + *
> + * @work - kernel work item
> + *
> + * Wait for the buffer object to become idle and do the actual page flip
> + */
> +static void radeon_flip_work_func(struct work_struct *__work)
>  {
[...]
> +	if (radeon_crtc->flip_work) {
> +		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
> +		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> +		goto pflip_cleanup1;
> +	}

I'm a little worried about this case. AFAICT this would drop the flip if
a previous one is still pending? I'm not sure current userspace can
actually hit this, but sooner or later we'll probably want to support
things like triple buffering, flips replacing previous ones still
pending, and asynchronous flips (not synchronized to vertical blank).
Those will probably require changes to the kernel/user interface, but it
might be good to at least keep them in mind already for the infrastructure.


Also, in patch 5, we could stop calling drm_vblank_get/put() when we're
not using the vblank interrupt for flipping?


The series looks good to me other than that.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel