Re: pci_set_mwi() ... why isn't it used more?

[David Brownell <david-b@pacbell.net>]

Jeff Garzik wrote:
> On Mon, Jan 20, 2003 at 10:41:35AM -0800, David Brownell wrote:
> 
>>I was looking at some new hardware and noticed that it's
>>got explicit support for the PCI Memory Write and Invalidate
>>command ... enabled (in part) under Linux by pci_set_mwi().
>>
>>However, very few Linux drivers use that routine.  Given
>>that it can lead to improved performance, and that devices
>>don't have to implement that enable bit, I'm curious what
>>the story is...
> 
> You missed the reason entirely ;-)

What, with a "covers everything" choice like "something else"? ;)


But to confirm:  you're saying there's no particular reason not to
use it pretty generally?  (Or at least, no known reason?)

I'd mostly be concerned about potential bridge/cpu chipset problems,
since those are the class of problems I'd have very little chance
of noticing, with only a handful of test platforms.  If individual
devices have broken MWI it'd be easy for them not to enable it.
But if they have to cope with buggy platform implementations...

I suppose the potential for broken PCI devices is exactly why MWI
isn't automatically enabled when DMA mastering is enabled, though
I don't understand why the cacheline size doesn't get fixed then
(unless it's that same issue).  Devices can use the cacheline size
to get better Memory Read Line/Read Multiple" throughput; setting
it shouldn't be tied exclusively to enabling MWI.


> pci_set_mwi() is brand new, I just added it.  Hasn't filtered down to
> drivers yet.  The few drivers that cared prior to its addition, like
> drivers/net/acenic.c, just hand-coded the workarounds needed for proper
> MWI support on all chipsets.

Yep, I noticed that it grew from acenic.  Didn't check back too many
kernel revs though, I guess "new" is relative ... 2.4 and 2.5 both
have it today.


> pci_set_mwi() would not exist at all, were it not for the existing
> hardware quirks.  (if hardware were sane, drivers would just
> individually twiddle the _INVALIDATE bit in PCI_COMMAND, and never call
> functions other than pci_{read,write}_config_word.

Actually I sort of prefer having the extra logic (set cacheline size,
twiddle that bit) out of drivers; there's no reason to have two copies
of that, particularly given there's already one arch-specific tweak.

Not that it's complex code, but it's easier for driver writers to
just know "call pci_set_mwi() if you're using DMA, unless you know
the hardware is buggy in that way" than to replicate its logic.

- Dave