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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
