Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <700e7f0b-a535-545b-1c83-26ce2d4e8be1@gmail.com>
Date:   Wed, 15 Jun 2022 10:43:28 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH] ARM: bcmbca: add VFP and NEON fixup for bcm6846 SoC
Content-Language: en-US
To:     William Zhang <william.zhang@broadcom.com>,
        Linux ARM List <linux-arm-kernel@lists.infradead.org>,
        Russell King <linux@armlinux.org.uk>
Cc:     joel.peshkin@broadcom.com, dan.beygelman@broadcom.com,
        anand.gore@broadcom.com, kursad.oney@broadcom.com,
        samyon.furman@broadcom.com, tomer.yacoby@broadcom.com,
        philippe.reynes@softathome.com,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Ray Jui <rjui@broadcom.com>,
        Scott Branden <sbranden@broadcom.com>,
        linux-kernel@vger.kernel.org
References: <20220525003509.2812-1-william.zhang@broadcom.com>
From:   Florian Fainelli <f.fainelli@gmail.com>
In-Reply-To: <20220525003509.2812-1-william.zhang@broadcom.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/24/22 17:35, William Zhang wrote:
> BCM6846 SoC only has VFP and NEON support on core 0. So kernel VPF/NEON
> support is disabled in this chip. Add this fixup to manually turn on
> VFP/NEON in case userspace app need to access them on core 0.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>

We have had this conversation internally already, but I do not think 
this is sufficient in order to have a workable solution, you indicated 
that there is all sorts of user-space involvement in your SDK to ensure 
that only VFP tasks are scheduled on core 0, but if we were to seek a 
proper solution we would have to modify the ARM Linux kernel to forcibly 
migrate VFP tasks onto a core that can support executing them, or accept 
emulating them with the implied slow down.

Russell, what are your thoughts on this?

> 
> ---
> 
>   arch/arm/mach-bcm/Makefile |  5 ++++
>   arch/arm/mach-bcm/bcmbca.c | 54 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 59 insertions(+)
>   create mode 100644 arch/arm/mach-bcm/bcmbca.c
> 
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index b2394ddb0558..137b24b52139 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -68,3 +68,8 @@ CFLAGS_platsmp-brcmstb.o	+= -march=armv7-a
>   obj-y				+= brcmstb.o
>   obj-$(CONFIG_SMP)		+= platsmp-brcmstb.o
>   endif
> +
> +# BCMBCA
> +ifeq ($(CONFIG_ARCH_BCMBCA),y)
> +obj-y				+= bcmbca.o
> +endif
> diff --git a/arch/arm/mach-bcm/bcmbca.c b/arch/arm/mach-bcm/bcmbca.c
> new file mode 100644
> index 000000000000..6fcdfe0b94c1
> --- /dev/null
> +++ b/arch/arm/mach-bcm/bcmbca.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Broadcom Ltd
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/smp.h>
> +#include <asm/mach/arch.h>
> +#include <asm/vfp.h>
> +#include <asm/cp15.h>
> +
> +#include "../vfp/vfpinstr.h"
> +
> +/*
> + * In some low end BCA chips, only core 0 has VPF/NEON support. Kernel VPF
> + * NEON are disabled on these platforms. Add this fixup to enable VFP/NEON
> + * on core 0 manually in case some user app need to access vfp/neon on core 0
> + */
> +static void __init neon_enable(void *data)
> +{
> +	u32 access, fpexc;
> +	(void)data;
> +
> +	/* Enable full access to VFP (cp10 and cp11) */
> +	access = get_copro_access();
> +	set_copro_access(access | CPACC_FULL(10) | CPACC_FULL(11));
> +
> +	/* enable NEON and VFP extension */
> +	fpexc = fmrx(FPEXC);
> +	fmxr(FPEXC, fpexc | FPEXC_EN);
> +}
> +
> +static void __init bcmbca_neon_fixup(void)
> +{
> +	smp_call_function_single(0, neon_enable, NULL, 1);
> +}
> +
> +static const char *const bcmbca_match[] __initconst = {
> +	"brcm,bcm6846",
> +	NULL
> +};
> +
> +DT_MACHINE_START(BCMBCA, "Broadcom BCMBCA SoC")
> +	.dt_compat		= bcmbca_match,
> +	.init_late		= bcmbca_neon_fixup,
> +MACHINE_END


-- 
Florian