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

From: David Brownell (david-b@pacbell.net)
Date: Mon Jan 20 2003 - 14:37:30 EST


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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Jan 23 2003 - 22:00:24 EST