diff mbox series

[2/2] platform: generic: allwinner: fix OV process for T-HEAD c9xx pmu

Message ID IA1PR20MB4953B0129CC1E042708AC6B0BB11A@IA1PR20MB4953.namprd20.prod.outlook.com
State Superseded
Headers show
Series fix T-HEAD c9xx cpu softlock | expand

Commit Message

Inochi Amaoto Aug. 12, 2023, 5:06 a.m. UTC 
From: "haijiao.liu@sophgo.com" <haijiao.liu@sophgo.com>

T-HEAD c9xx pmu only needs to clear OV bits of MCOUNTEROF when MOIP bit
of MIP is set, so correct the MIP value check to avoid race conditions
during processing interrupt.

In addition, the S-mode scounterof only have OV bit set when the related
bits of MCOUNTERWEN is set, so also configure MCOUNTERWEN to allow kernel
to access valid SCOUNTEROF.

Signed-off-by: haijiao.liu@sophgo.com <haijiao.liu@sophgo.com>
---
 platform/generic/allwinner/sun20i-d1.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Guo Ren Aug. 12, 2023, 12:01 p.m. UTC | #1 
Reviewed-by: Guo Ren <guoren@kernel.org>

Hi Heiko,

Could you help test this patch? I think it could fix your perf record problem.

On Sat, Aug 12, 2023 at 1:06 PM Inochi Amaoto <inochiama@outlook.com> wrote:
>
> From: "haijiao.liu@sophgo.com" <haijiao.liu@sophgo.com>
>
> T-HEAD c9xx pmu only needs to clear OV bits of MCOUNTEROF when MOIP bit
> of MIP is set, so correct the MIP value check to avoid race conditions
> during processing interrupt.
>
> In addition, the S-mode scounterof only have OV bit set when the related
> bits of MCOUNTERWEN is set, so also configure MCOUNTERWEN to allow kernel
> to access valid SCOUNTEROF.
>
> Signed-off-by: haijiao.liu@sophgo.com <haijiao.liu@sophgo.com>
> ---
>  platform/generic/allwinner/sun20i-d1.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> index 0f0a9f3..d0f556e 100644
> --- a/platform/generic/allwinner/sun20i-d1.c
> +++ b/platform/generic/allwinner/sun20i-d1.c
> @@ -239,9 +239,17 @@ static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
>          * Otherwise, there will be race conditions where we may clear the bit
>          * the software is yet to handle the interrupt.
>          */
> -       if (!(mip_val & THEAD_C9XX_MIP_MOIP))
> +       if (mip_val & THEAD_C9XX_MIP_MOIP)
>                 csr_clear(THEAD_C9XX_CSR_MCOUNTEROF, BIT(ctr_idx));
>
> +       /**
> +        * This register is described in C920 document as the control register
> +        * for enabling writes to the superuser state counter. However, if the
> +        * corresponding bit is not set to 1, scounterof will always read as 0
> +        * when the counter register overflows.
> +        */
> +       csr_set(THEAD_C9XX_CSR_MCOUNTERWEN, BIT(ctr_idx));
> +
>         /**
>          * SSCOFPMF uses the OF bit for enabling/disabling the interrupt,
>          * while the C9XX has designated enable bits.
> @@ -252,6 +260,7 @@ static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
>
>  static void thead_c9xx_pmu_ctr_disable_irq(uint32_t ctr_idx)
>  {
> +       csr_clear(THEAD_C9XX_CSR_MCOUNTERWEN, BIT(ctr_idx));
>         csr_clear(THEAD_C9XX_CSR_MCOUNTERINTEN, BIT(ctr_idx));
>  }
>
> --
> 2.34.1
>
Inochi Amaoto Aug. 20, 2023, 4:54 a.m. UTC | #2 
Thanks. This is good for me. And the patch does not need any further changes.

>On Sun, Aug 20, 2023 at 8:53 AM Inochi Amaoto <inochiama@outlook.com> wrote:
>>
>> Thanks, but now I have a small problem: wheather the bit cycles and bit
>> instructions should be set in soc extensions init? Or these two bits should
>> be handled in sbi pmu init?
>
>The mphm_mask only represents the available HPM counters. The fixed
>counters are assumed to be available.
>
>This is already taken care by ib/sbi/sbi_pmu.c in the pmu_add_hw_event_map().
>
>Regards,
>Anup
diff mbox series

Patch

diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
index 0f0a9f3..d0f556e 100644
--- a/platform/generic/allwinner/sun20i-d1.c
+++ b/platform/generic/allwinner/sun20i-d1.c
@@ -239,9 +239,17 @@  static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
 	 * Otherwise, there will be race conditions where we may clear the bit
 	 * the software is yet to handle the interrupt.
 	 */
-	if (!(mip_val & THEAD_C9XX_MIP_MOIP))
+	if (mip_val & THEAD_C9XX_MIP_MOIP)
 		csr_clear(THEAD_C9XX_CSR_MCOUNTEROF, BIT(ctr_idx));
 
+	/**
+	 * This register is described in C920 document as the control register
+	 * for enabling writes to the superuser state counter. However, if the
+	 * corresponding bit is not set to 1, scounterof will always read as 0
+	 * when the counter register overflows.
+	 */
+	csr_set(THEAD_C9XX_CSR_MCOUNTERWEN, BIT(ctr_idx));
+
 	/**
 	 * SSCOFPMF uses the OF bit for enabling/disabling the interrupt,
 	 * while the C9XX has designated enable bits.
@@ -252,6 +260,7 @@  static void thead_c9xx_pmu_ctr_enable_irq(uint32_t ctr_idx)
 
 static void thead_c9xx_pmu_ctr_disable_irq(uint32_t ctr_idx)
 {
+	csr_clear(THEAD_C9XX_CSR_MCOUNTERWEN, BIT(ctr_idx));
 	csr_clear(THEAD_C9XX_CSR_MCOUNTERINTEN, BIT(ctr_idx));
 }