[PATCH 3/3] drm: do not sleep on vblank while holding a mutex

Ilija Hadzic ihadzic at research.bell-labs.com
Fri Oct 28 05:10:51 PDT 2011



On Fri, 28 Oct 2011, Daniel Vetter wrote:

> On Fri, Oct 28, 2011 at 08:59:04AM +0200, Michel Dänzer wrote:
>> On Don, 2011-10-27 at 22:19 -0500, Ilija Hadzic wrote:
>>> On Thu, 27 Oct 2011, Daniel Vetter wrote:
>>>
>>>> So I think the right thing to do is
>>>> - Kill dev->last_vblank_wait (in a prep patch).
>>>
>>> Agreed. Also drm_vblank_info function can go away
>>
>> Actually, I was rather going to submit a patch to hook it up again —
>> AFAICT it was unhooked without any justification. It could be useful for
>> debugging vblank related hangs. Any issues with it, such as
>> last_vblank_wait not being guaranteed to really be the last one, can
>> always be improved later on.
>
> I've thought a bit about the usefulness of it for debugging before
> proposing to kill it and I think it can die: It's only really useful for a
> complete hangs, if we have an issue we just missing a wakeup somewhere,
> that's not gonna catch things. Hence I think something that allows you to
> watch things while it's running is much better, i.e. either a drm debug
> prinkt or a tracecall.
>
> But if you're firmly attached to that debug file it should be pretty easy
> to shove that under the protection of one of the vblank spinlocks.

I'll keep it then and figure out the best mutex/spinlock to use. It can be 
anything that exists on one-per-CRTC basis (vblank waits on different CTCs 
are not contending). The critical section is from that switch in which 
vblwait->request.sequence is incremented until it is assigned to 
dev->last_vblank_wait[crtc] (and we are only protecting that section 
against itself, executed in contexts of different PIDs).

I guess we settled now on this patch (other comments will be 
addressed in a different set of patches).

-- Ilija


More information about the dri-devel mailing list