Re: [Dri-devel] Mach64 locking on mode switches
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
Re: [Dri-devel] Mach64 locking on mode switches
On Sun, 28 Oct 2001, Manuel Teira wrote:
[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. ;-)
Yes, if we can avoid multiple locks, it's better. I did finally track
down the macros and re-read the locking doc on the PI site. The
LOCK/UNLOCK_HARDWARE macro in the mesa code looks like it is using the drm
locking scheme. I have a way to go yet to understand all the mesa stuff,
but it looks like there's a good deal of work to do there. There are a
lot of disabled bits of code and everything is being flushed with
mach64EmitHwStateLocked. We'll need to get the AGP stuff done (with
fallback for PCI) and finish the drm ioctls before we can get mesa using
DMA buffers, though. BTW, I noticed one other place where the
LOCK_TEST_WITH_RETURN can be used in mach64_dma.c in mach64_dma_idle().
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.
OK. I did test it out doing it in the way I suggested, but I'm still
getting a lockup. I was thinking of compiling with the debugging macros
turned on to get a better idea of where the problem is. Since the problem
doesn't seem to happen with gears, I wonder if there's something in the
mesa code that isn't locking properly.
BTW, your yesterday question:
do you know how to get etags to index KR style functions like the ones in
aticonsole.c
I thought that KR 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
)
Yes, you're right. It turns out that etags indexes these fine, but I'm
not able to expand the function list in the emacs speedbar, so it may be
that the speedbar code is using a regex that's looking for the function
names and args on the same line. I'll have to learn a little more lisp, I
think. ;)
--Leif
--
Leif Delgass
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Mach64 locking on mode switches
El Dom 28 Oct 2001 19:17, Leif Delgass escribió: snip OK. I did test it out doing it in the way I suggested, but I'm still getting a lockup. I was thinking of compiling with the debugging macros turned on to get a better idea of where the problem is. Since the problem doesn't seem to happen with gears, I wonder if there's something in the mesa code that isn't locking properly. Have in mind that the Mesa layer is now writing directly to the registers. Are this direct writes locked in all the Mesa lib? For example, and at a first glance, the function: mach64UploadHwStateLocked @ mach64_ioctl.c is making WRITE access to the card without calling LOCK_HARDWARE. Perhaps this funcion is called from another place were the lock takes place, I don't know. Anyway, when the DMA work takes shape on the Mesa layer, the places where the lock is needed will be less than now (I think), only when the DMA buffers are flushed via a BusMaster operation. So, I think that we must not worry about the interlock while the DMA work is not finished. Frank, how is it going? Any news? Well, I've just commited the changes to the CVS. The Sunday is finishing and a new week of hard work is beginning :-( . At least here in Spain the week is shorter (it's only 4 work days). ;-) Best regards. -- Manuel Teira ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Mach64 locking on mode switches
On Sun, 28 Oct 2001, Manuel Teira wrote: El Dom 28 Oct 2001 19:17, Leif Delgass escribió: snip OK. I did test it out doing it in the way I suggested, but I'm still getting a lockup. I was thinking of compiling with the debugging macros turned on to get a better idea of where the problem is. Since the problem doesn't seem to happen with gears, I wonder if there's something in the mesa code that isn't locking properly. Have in mind that the Mesa layer is now writing directly to the registers. Are this direct writes locked in all the Mesa lib? For example, and at a first glance, the function: mach64UploadHwStateLocked @ mach64_ioctl.c is making WRITE access to the card without calling LOCK_HARDWARE. Perhaps this funcion is called from another place were the lock takes place, I don't know. In theory, it should only be called after locking, which is why Locked is in the function name, I think (and from what I can tell from a quick scan of the code, it is being called after locking). I tried testing the changes again and remembered to look at the X log when I rebooted before starting X again. I got a series (19) of DRIUnlock called when not locked messages. I'm going to try to trace all the DRILock/Unlock calls and see if I can track this down. Anyway, when the DMA work takes shape on the Mesa layer, the places where the lock is needed will be less than now (I think), only when the DMA buffers are flushed via a BusMaster operation. So, I think that we must not worry about the interlock while the DMA work is not finished. Frank, how is it going? Any news? Well, I've just commited the changes to the CVS. The Sunday is finishing and a new week of hard work is beginning :-( . At least here in Spain the week is shorter (it's only 4 work days). ;-) Nice. Of course, I'm currently not working so I have lots of spare time to waste on this. ;) -- Leif Delgass ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Mach64 locking on mode switches
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:
#ifdef XF86DRI
if (pATI-Chip = ATI_CHIP_264GT
pATI-directRenderingEnabled)
{
DRILock(pScreen,0);
}
#endif
...
#ifdef XF86DRI
if (pATI-Chip = ATI_CHIP_264GT
pATI-directRenderingEnabled)
{
DRIUnlock(pScreen);
}
#endif
I think this is a better way because the code in aticonsole.c and atimode.c
is generic for all the ATI adapters, so we are restricting the checking to
the Mach64 chips greater than the GT that is the first one (if I'm not
mistaken) supporting 3D.
I'm also changing the related macros for locking the 2D accelerated functions.
--Leif
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Mach64 locking on mode switches
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.
#ifdef XF86DRI
if (pATI-Chip = ATI_CHIP_264GT
pATI-directRenderingEnabled)
{
DRILock(pScreen,0);
}
#endif
...
#ifdef XF86DRI
if (pATI-Chip = ATI_CHIP_264GT
pATI-directRenderingEnabled)
{
DRIUnlock(pScreen);
}
#endif
I think this is a better way because the code in aticonsole.c and atimode.c
is generic for all the ATI adapters, so we are restricting the checking to
the Mach64 chips greater than the GT that is the first one (if I'm not
mistaken) supporting 3D.
Yep, that looks better.
I'm also changing the related macros for locking the 2D accelerated functions.
OK. I'm attaching a small patch that defines and uses a convenience macro
used by the other drm drivers (LOCK_TEST_WITH_RETURN). We can use this
when we add more ioctls. The patch also includes the FIFO size defines,
but you can just say no to that and delete the .rej file.
Another thing I noticed is that all the other 2D/dri drivers notify the
xf86 module loader about DRI/DRM symbols using [xf86]LoaderRefSymLists, in
addition to other symbols (see R128PreInit in r128_driver.c for an
example). I can't find anywhere that the ati/mach64 driver does this, but
it doesn't seem to be causing any problems.
On a totally unrelated note, do you know how to get etags to index KR
style functions like the ones in aticonsole.c?
--
Leif Delgass
m64-locking-macro.patch.gz
Re: [Dri-devel] Mach64 locking on mode switches
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) 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? OK. I'm attaching a small patch that defines and uses a convenience macro used by the other drm drivers (LOCK_TEST_WITH_RETURN). We can use this when we add more ioctls. The patch also includes the FIFO size defines, but you can just say no to that and delete the .rej file. All right. I've commited the changes to the CVS branch yet, but I have to fix the deadlock you've found, so, I'll also send the changes you've sent me. On a totally unrelated note, do you know how to get etags to index KR style functions like the ones in aticonsole.c? No. Sorry. ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] Mach64 locking on mode switches
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) 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. 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 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? --Leif -- Leif Delgass ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
