| Message ID | 20240503135455.966-7-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mips: bmips: improve handling of RAC and CBR addr | expand |
On Fri, May 03, 2024 at 03:54:06PM +0200, Christian Marangi wrote: > Drop redundant boot_cpu_type in arch_sync_dma_for_cpu_all. These needs > to be parsed only once and we can make use of bmips_rac_flush_disable to > disable RAC flush on unsupported CPU. > > Set this value in bmips_cpu_setup for unsupported CPU to skip this > redundant check every time DMA needs to be synced. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Sent by mistake please ignore and use the other PATCH 6/6 patch. (wrong commit title)
On 5/3/24 06:54, Christian Marangi wrote: > Drop redundant boot_cpu_type in arch_sync_dma_for_cpu_all. These needs > to be parsed only once and we can make use of bmips_rac_flush_disable to > disable RAC flush on unsupported CPU. > > Set this value in bmips_cpu_setup for unsupported CPU to skip this > redundant check every time DMA needs to be synced. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> You are taking a shortcut that is reasonable in premise, but keying off the bmips_rac_flush_disable is IMHO misleading. The RAC is enabled in the BMIPS5000 and BMIPS5200 cores, just it does not need SW management unlike earlier cores. If you renamed it to bmips_rac_flush_needed that might be more compelling. Also, the other reason is that on a kernel that was configured for supporting only BMIPS5000 and BMIPS5200 CPUs, I think we could get some decent dead code elimination of the boot_cpu_type() check, which would not be the case.
On Fri, May 03, 2024 at 12:07:45PM -0700, Florian Fainelli wrote: > On 5/3/24 06:54, Christian Marangi wrote: > > Drop redundant boot_cpu_type in arch_sync_dma_for_cpu_all. These needs > > to be parsed only once and we can make use of bmips_rac_flush_disable to > > disable RAC flush on unsupported CPU. > > > > Set this value in bmips_cpu_setup for unsupported CPU to skip this > > redundant check every time DMA needs to be synced. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > You are taking a shortcut that is reasonable in premise, but keying off the > bmips_rac_flush_disable is IMHO misleading. The RAC is enabled in the > BMIPS5000 and BMIPS5200 cores, just it does not need SW management unlike > earlier cores. > > If you renamed it to bmips_rac_flush_needed that might be more compelling. > Also, the other reason is that on a kernel that was configured for > supporting only BMIPS5000 and BMIPS5200 CPUs, I think we could get some > decent dead code elimination of the boot_cpu_type() check, which would not > be the case. I was a bit confused by the last part, should I drop this or just rename the variable? Cause I think for kernel that support ONLY those CPU I guess the DMA function will be optimized anyway since the bool will always be false I guess?
On 5/3/24 12:39, Christian Marangi wrote: > On Fri, May 03, 2024 at 12:07:45PM -0700, Florian Fainelli wrote: >> On 5/3/24 06:54, Christian Marangi wrote: >>> Drop redundant boot_cpu_type in arch_sync_dma_for_cpu_all. These needs >>> to be parsed only once and we can make use of bmips_rac_flush_disable to >>> disable RAC flush on unsupported CPU. >>> >>> Set this value in bmips_cpu_setup for unsupported CPU to skip this >>> redundant check every time DMA needs to be synced. >>> >>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> >> >> You are taking a shortcut that is reasonable in premise, but keying off the >> bmips_rac_flush_disable is IMHO misleading. The RAC is enabled in the >> BMIPS5000 and BMIPS5200 cores, just it does not need SW management unlike >> earlier cores. >> >> If you renamed it to bmips_rac_flush_needed that might be more compelling. >> Also, the other reason is that on a kernel that was configured for >> supporting only BMIPS5000 and BMIPS5200 CPUs, I think we could get some >> decent dead code elimination of the boot_cpu_type() check, which would not >> be the case. > > I was a bit confused by the last part, should I drop this or just rename > the variable? Cause I think for kernel that support ONLY those CPU I > guess the DMA function will be optimized anyway since the bool will > always be false I guess? I don't think it can be optimized, I would drop that patch. This is a hot-path and so any optimization is welcome.
diff --git a/arch/mips/bmips/dma.c b/arch/mips/bmips/dma.c index 799cc3e12fc3..e9af34f82dcd 100644 --- a/arch/mips/bmips/dma.c +++ b/arch/mips/bmips/dma.c @@ -11,11 +11,6 @@ void arch_sync_dma_for_cpu_all(void) { u32 cfg; - if (boot_cpu_type() != CPU_BMIPS3300 && - boot_cpu_type() != CPU_BMIPS4350 && - boot_cpu_type() != CPU_BMIPS4380) - return; - if (unlikely(bmips_rac_flush_disable)) return; diff --git a/arch/mips/bmips/setup.c b/arch/mips/bmips/setup.c index bef84677248e..d27043b10405 100644 --- a/arch/mips/bmips/setup.c +++ b/arch/mips/bmips/setup.c @@ -40,7 +40,6 @@ * with "mips-cbr-reg" in the "cpus" node. */ void __iomem *bmips_cbr_addr; -extern bool bmips_rac_flush_disable; static const unsigned long kbase = VMLINUX_LOAD_ADDRESS & 0xfff00000; diff --git a/arch/mips/include/asm/bmips.h b/arch/mips/include/asm/bmips.h index 3a1cdfddb987..4a48c8f1077e 100644 --- a/arch/mips/include/asm/bmips.h +++ b/arch/mips/include/asm/bmips.h @@ -82,6 +82,7 @@ extern char bmips_smp_int_vec[]; extern char bmips_smp_int_vec_end[]; extern void __iomem *bmips_cbr_addr; +extern bool bmips_rac_flush_disable; extern int bmips_smp_enabled; extern int bmips_cpu_offset; extern cpumask_t bmips_booted_mask; diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c index 7bde6bbaa41f..63534af367c7 100644 --- a/arch/mips/kernel/smp-bmips.c +++ b/arch/mips/kernel/smp-bmips.c @@ -681,6 +681,13 @@ void bmips_cpu_setup(void) " or $8, $9\n" " .word 0x4088b008\n" /* mtc0 $8, $22, 8 */ : : : "$8", "$9"); + + /* Disable RAC flush as not supported */ + bmips_rac_flush_disable = true; break; + + default: + /* Disable RAC flush as not supported */ + bmips_rac_flush_disable = true; } }
Drop redundant boot_cpu_type in arch_sync_dma_for_cpu_all. These needs to be parsed only once and we can make use of bmips_rac_flush_disable to disable RAC flush on unsupported CPU. Set this value in bmips_cpu_setup for unsupported CPU to skip this redundant check every time DMA needs to be synced. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- arch/mips/bmips/dma.c | 5 ----- arch/mips/bmips/setup.c | 1 - arch/mips/include/asm/bmips.h | 1 + arch/mips/kernel/smp-bmips.c | 7 +++++++ 4 files changed, 8 insertions(+), 6 deletions(-)