El Dom 28 Oct 2001 00:10, Leif Delgass escribió:
> On Sat, 27 Oct 2001, Manuel Teira wrote:
> > El Sáb 27 Oct 2001 21:40, Leif Delgass escribió:
> > > On Sat, 27 Oct 2001, Manuel Teira wrote:
> > > > El Sáb 27 Oct 2001 19:49, Leif Delgass escribió:
> > > > > Well, I just got my box to hang hard (like with the vt switching)
> > > > > when running tuxracer and switching modes with Ctrl-Alt-+ (I have 3
> > > > > modes defined in my config and the hang happened when looping back
> > > > > to the original mode, i.e. the third switch), so I think the
> > > > > answer is yes, it needs locking. I really should use a journalling
> > > > > filesystem, all this fsck-ing is getting a bit tedious. ;)
> > > >
> > > > OK. I have added the DRILock/Unlock to the AtiModeSet function in the
> > > > atimode.c file. I've added another condition (also to the locks in
> > > > the aticonsole.c file for vt changing) in this way:
> > >
> > > It looks like ATIEnter/LeaveVT calls ATIEnter/LeaveGraphics, which in
> > > turn calls ATIModeSet. Won't this lead to trying to obtain a lock when
> > > the lock is already secured? It might be better to put the lock in
> > > aticonsole.c in the ATIModeSwitch function.
> >
> > You are right, I was deadlocking the server. Anyway, the idea is that
> > ATIModeSet has to Lock and Unlock, but ATIEnterVT(LeaveVT) has only to
> > Unlock(Lock) the DRM. We could Lock/Unlock in ATISwitchMode, but I
> > think that we are locking more that needed, because the ATIModeCalculate
> > that is also called from ATISwitchMode doesn't need to be locked.
> >
> > I think that the better way to do this is:
> >
> > ATIEnterVT: Unlock at the end of the Function (to avoid interlocks)
Well, here I wanted to say 'at the beginning of the Function'. Sorry.
> > ATILeaveVT: Lock at the end of the Function (to avoid interlocks)
> > ATIModeSet: Lock at the beginning and Unlock at the end.
> >
> > What do you thing about this?
>
> Well, let's look at the sequence assuming what you suggest. This is as
> much for me to get it straight in my head as anything else...
>
> Xserver is running. We switch to text mode:
>
> 1. ATILeaveVT is called, which calls ATILeaveGraphics
> 2. ATIModeSave
> 3. ATIModeSet (DRILock,DRIUnlock)
> 4. ATILock, return from ATILeaveGraphics
> 5. DRILock
>
> I'm not sure it's worth unlocking just to lock again here.
>
> Text mode is running. We switch to graphics mode:
>
> 1. ATIEnterVT is called, which calls ATIEnterGraphics
> 2. ATIUnlock
> 3. ATIModeCalculate
> 4. ATIModeSave
> 5. ATIModeSet (DRILock,DRIUnlock), return from ATIEnterGraphics
> Oops!, at this point we still hold the DRILock, so again we'll be trying
> to lock twice, right?
> 6. DRIUnlock
> Here we've already unlocked.
Yes, you're right. Too late to think yesterday.
>
> Well, we could unlock at the _beginning_ of ATIEnterVT, but here's what I
> would suggest...
>
> Don't do locking/unlocking in ATIModeSet, but do the locking in the three
> functions in aticonsole.c:
>
> ATILeaveVT()
> 1. DRILock, then call ATILeaveGraphics
> 2. ATIModeSave
> 3. ATIModeSet (ok, we've got the lock already)
> 4. ATILock, return from ATILeaveGraphics
> 5. return
>
> ATIEnterVT()
> 1. call ATIEnterGraphics
> 2. ATIUnlock
> 3. ATIModeCalculate
> 4. ATIModeSave
> 5. ATIModeSet (we still hold the DRILock), return
> 6. DRIUnlock, return
>
> ATISwitchMode()
> 1. ATIModeCalculate
> 2. DRILock
> 3. ATIModeSet
> 4. DRIUnlock, return
>
At a first glance, it looks that this aproximation is the best. The idea is
use for the locking the upper functions to avoid interlock problems. We do
not want to use our locks from 2 levels of functions. In this way, I think
there is no problem.
> Here you can still avoid locking until after the mode calculate. Also
> ATIModeSet is called by ATIClockPreInit, and I don't think DRI locking is
> necessary at that point (in fact, I think it happens before the DRI
> initialization?). Of course, this is my high-level view (I haven't
> followed all the code in the sequence in detail) and I could be missing
> something. I'm also not sure exactly _where_ the lockup happens and so
> I'm not sure how fine grained the locking can get. What do you think?
My idea is that we don't need more detail. We just need to lock when a mode
change is made (both for going to text mode or changing the screen mode) and
that changes are made with the API defined in aticonsole.c.
<snip src="from your other mail">
OK, I actually looked at the definition of DRILock/DRIUnlock
(programs/Xserver/GL/dri/dri.c), and it does reference counting, so
locking twice might not be an issue, it just increments the reference
count. What I haven't found yet is where the DRM_LOCK/DRM_UNLOCK macros
used in DRILock/DRIUnlock are defined, and I'm not sure how contention is
handled either. So I'm going to try and get a better idea of how the
locking is actually implemented, and hopefully I know what I'm talking
about next time.
</snip>
I still think that we should avoid more than one consecutive locks. It has
nonsense from my point of view, at least in this case. The
DRM_LOCK/DRM_UNLOCK macros are defined in:
xc/programs/Xserver/hw/xfree86/os-support/xf86drm.h
and a lot of other things I don't feel strong enough to look at now. ;-)
And to finish, I think that your last aproximation is the good one. I will
look again at it this night, touch the code in that way and commit these and
your other changes to the CVS.
BTW, your yesterday question:
"do you know how to get etags to index K&R style functions like the ones in
aticonsole.c"
I thought that K&R style was this way:
int foo(a,b)
int a;
int b;
{
/* Function body */
}
Or something so, isn't it?
But the functions in aticonsole.c are ANSI-C defined, arent't they, but with
a little unusual indenting:
int
foo(
int a,
int b
)
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
