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
