Messages in this thread | | | Subject | Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support | From | Robin Murphy <> | Date | Fri, 5 Jun 2020 15:41:27 +0100 |
| |
On 2020-06-05 14:46, Robin Murphy wrote: > On 2020-06-05 14:20, Mark Brown wrote: >> On Fri, Jun 05, 2020 at 12:34:36PM +0100, Robin Murphy wrote: >>> On 2020-06-04 22:28, Florian Fainelli wrote: >> >>>> For the BCM2835 case which is deemed performance critical, we would >>>> like >>>> to continue using an interrupt handler which does not have the extra >>>> comparison on BCM2835_SPI_CS_INTR. >> >>> FWIW, if I'm reading the patch correctly, then with sensible codegen >>> that >>> "overhead" should amount to a bit test on a live register plus a >>> not-taken >>> conditional branch - according to the 1176 TRM that should add up to a >>> whopping 2 cycles. If that's really significant then I'd have to wonder >>> whether you want to be at the mercy of the whole generic IRQ stack at >>> all, >>> and should perhaps consider using FIQ instead. >> >> Yes, and indeed the compiler does seem to manage that. It *is* non-zero >> overhead though. > > True, but so's the existing level of pointer-chasing indirection that > with some straightforward refactoring could be taken right out of the > critical path and confined to just the conditional complete() call. > That's the kind of thing leaving me unconvinced that this is code where > every single cycle counts ;)
Ha, and in fact having checked a build out of curiosity, this patch as-is actually stands to make things considerably worse. At least with GCC 8.3 and bcm2835_defconfig, bcm2835_spi_interrupt_common() doesn't get inlined, which means bcm2835_spi_interrupt() pushes/pops a stack frame and makes an out-of-line call to bcm2835_spi_interrupt_common(), resulting in massively *more* work than the extra two instructions of simply inlining the test.
So yes, the overhead of inlining the test vs. the alternative is indeed non-zero. It's just also negative :D
Robin.
| |