[PATCH v2] radeon: Use mdelay() instead of msleep() when we can't sleep in atom_op_delay().

Alan Cox alan at lxorguk.ukuu.org.uk
Wed Jan 4 04:09:36 PST 2012


> I think Alan's simply wrong.

In what way ? You stated this, then go on below to say what I did ?

> Splattering the entire driver for
> these two corner cases which don't happen at all under normal
> circumstances is imho utter nonsense.

Which is what I said

| > > Is this only special cases like a panic - if so can it not be called
in a
| > > way that distinguishes between normality and nasty cases.  
| > 
| > No idea, to be honest. It's an ATOM BIOS interpreter opcode, so in
| > theory it could be indirectly called from anywhere that uses ATOM
BIOS.  

| So lets stick to practice, and the real world. Screwing up everything
| else because of a crappy problem in your Atom BIOS code sucks but hey it
| happens. screwing up everything because of a theoretical concern is just
| dumb.


We don't want to do mdelays when they are not needed just because it is
theoretically possible they are not needed. There are several ways to fix
it, one is to stuff in_atomic() etc in the awkward spots. However if
you've got something where you have lots of call points to an interface
this is actually a bad idea, because you grow more insidiously, or a
change elsewhere in the kernel silently turns all your sleeps into delays
and things like live music work stop being doable with radeon graphics.

So the better way to fix it is to actually have an interface

	atom_execute_table_atomic()

so that such situations are called out clearly, and any change elsewhere
that messes it all up causes kernel spewage that can be dealt with
properly - either by using _atomic if its something obscure like a panic,
fixing the issue or reverting the problematic change further up the tree.

Another way to approach it would be to have

	radeon_atom_atomic(rdev);
	radeon_atom_whateverfoo(rdev, ...)
	radeon_atom_atomic_end(rdev);

which set and cleared a bitflag that the mdelay/msleep opcode tested.

That's quicker to implement but a spot less clean. Again it calls out any
such cases and makes them explicit. It also means any accidental changes
that affect this will result in spewage and debugging not silent trashing
of performance for stuff like machine control, music and some gaming.

It should only be for panic, although accel calls for printk spewage can
hit in atomic context in some situations but I don't think that becomes
an atombios path ?

Alan


More information about the dri-devel mailing list