Re: radeon_pm.c locking problem

[David Eger <eger@havoc.gtf.org>]

On Mon, Jul 05, 2004 at 06:41:04PM +0200, Kronos wrote:
> Il Mon, Jul 05, 2004 at 08:44:53AM -0500, Benjamin Herrenschmidt ha scritto: 
> > > is expanded in:
> > > 
> > > do {
> > >         unsigned long flags;
> > >         spin_lock_irqsave(&rinfo->reg_lock, flags);
> > >         __OUTPLL(pllPIXCLKS_CNTL, INPLL(pllPIXCLKS_CNTL) & ~PIXCLKS_CNTL__PIX2CLK_SRC_SEL_MASK);
> > >         spin_unlock_irqrestore(&rinfo->reg_lock, flags);
> > > } while(0)
> > > 
> > > and INPLL will try to reacquire the lock ->reg_lock. We could either use
> > > __INPLL (which does not take the lock) or make OUTPLL an inline function
> > > (in this way INPLL is called before entering the function).
> > 
> > Make it an inline function...
> 
> Here it is:

Your patch is much, much prettier than mine (which is in mainline),
and Linus is hoping for such a prettier solution.

On the other hand, is there any reason we're taking this
rinfo->reg_lock?   Would it be possible for the fb driver to be called
in two separate contexts at the same time?

Also, having this macro be pretty sort of obsccures the fact that, 
the way the code is now, we're doing

	OUTPLL(foo0, bar0);
	OUTPLL(foo1, bar1);
	OUTPLL(foo2, bar2);

which translates to taking this lock three times in quick succession.
Bleah.

I had half a mind to put a radeon_fifo_wait() call in the
OUTREG() macro (because there needs to be a 1-1 correspondence 
between empty fifo slots and OUTREG calls, but then you add a
chunk of a loop, udelay, and printk to every register write...
again lots of overhead for a simple register write.... 
better to sprinkle the code with the write counts and hope they stay
in line?  Maybe.  That's the route I took, anyways.

-dte


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com