diff mbox series

platform: generic: allwinner: correct mhpmevent count

Message ID IA1PR20MB495331314E453C7E9822C88ABB08A@IA1PR20MB4953.namprd20.prod.outlook.com
State Accepted
Headers show
Series platform: generic: allwinner: correct mhpmevent count | expand

Commit Message

Inochi Amaoto Aug. 3, 2023, 1:50 a.m. UTC 
Only the CSR mhpmevent 3-9,13-17 of D1 have valid function, so reduce
the mhpm_count to 15 to avoid invalid usage.

Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
---
 platform/generic/allwinner/sun20i-d1.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
2.41.0

Comments

Anup Patel Aug. 6, 2023, 5:03 a.m. UTC | #1 
On Thu, Aug 3, 2023 at 7:20 AM Inochi Amaoto <inochiama@outlook.com> wrote:
>
> Only the CSR mhpmevent 3-9,13-17 of D1 have valid function, so reduce
> the mhpm_count to 15 to avoid invalid usage.
>
> Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> ---
>  platform/generic/allwinner/sun20i-d1.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> index 0f0a9f3..94a3a5e 100644
> --- a/platform/generic/allwinner/sun20i-d1.c
> +++ b/platform/generic/allwinner/sun20i-d1.c
> @@ -272,7 +272,8 @@ static int sun20i_d1_extensions_init(const struct fdt_match *match,
>         sbi_pmu_set_device(&thead_c9xx_pmu_device);
>
>         /* auto-detection doesn't work on t-head c9xx cores */
> -       hfeatures->mhpm_count = 29;
> +       /* D1 has 29 mhpmevent csrs, but only 3-9,13-17 have valid value */
> +       hfeatures->mhpm_count = 15;

With the latest non-contiguous counter support, we need to set
mhpm_mask as well.
I have taken care of this at the time of merging this patch.

Reviewed-by: Anup Patel <anup@brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

>         hfeatures->mhpm_bits = 64;
>
>         return 0;
> --
> 2.41.0
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
Guo Ren Aug. 6, 2023, 5:32 a.m. UTC | #2 
On Sun, Aug 6, 2023 at 1:04 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Thu, Aug 3, 2023 at 7:20 AM Inochi Amaoto <inochiama@outlook.com> wrote:
> >
> > Only the CSR mhpmevent 3-9,13-17 of D1 have valid function, so reduce
> > the mhpm_count to 15 to avoid invalid usage.
> >
> > Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> > ---
> >  platform/generic/allwinner/sun20i-d1.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> > index 0f0a9f3..94a3a5e 100644
> > --- a/platform/generic/allwinner/sun20i-d1.c
> > +++ b/platform/generic/allwinner/sun20i-d1.c
> > @@ -272,7 +272,8 @@ static int sun20i_d1_extensions_init(const struct fdt_match *match,
> >         sbi_pmu_set_device(&thead_c9xx_pmu_device);
> >
> >         /* auto-detection doesn't work on t-head c9xx cores */
> > -       hfeatures->mhpm_count = 29;
> > +       /* D1 has 29 mhpmevent csrs, but only 3-9,13-17 have valid value */
> > +       hfeatures->mhpm_count = 15;
>
> With the latest non-contiguous counter support, we need to set
> mhpm_mask as well.
> I have taken care of this at the time of merging this patch.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Applied this patch to the riscv/opensbi repo.
Sorry for the delayed reply. I think we need more tests on this patch.
Now my colleagues are cooperating with Inochi to solve the problem.

We are also solving the perf record problem on D1, so I'm unsure about
the final solution. I recommend you hold this patch for a while. Maybe
we would adjust it.

ps:
Include Heiko email

>
> Thanks,
> Anup
>
> >         hfeatures->mhpm_bits = 64;
> >
> >         return 0;
> > --
> > 2.41.0
> >
> >
> > --
> > opensbi mailing list
> > opensbi@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
Anup Patel Aug. 6, 2023, 5:35 a.m. UTC | #3 
On Sun, Aug 6, 2023 at 11:02 AM Guo Ren <guoren@kernel.org> wrote:
>
> On Sun, Aug 6, 2023 at 1:04 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Thu, Aug 3, 2023 at 7:20 AM Inochi Amaoto <inochiama@outlook.com> wrote:
> > >
> > > Only the CSR mhpmevent 3-9,13-17 of D1 have valid function, so reduce
> > > the mhpm_count to 15 to avoid invalid usage.
> > >
> > > Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> > > ---
> > >  platform/generic/allwinner/sun20i-d1.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> > > index 0f0a9f3..94a3a5e 100644
> > > --- a/platform/generic/allwinner/sun20i-d1.c
> > > +++ b/platform/generic/allwinner/sun20i-d1.c
> > > @@ -272,7 +272,8 @@ static int sun20i_d1_extensions_init(const struct fdt_match *match,
> > >         sbi_pmu_set_device(&thead_c9xx_pmu_device);
> > >
> > >         /* auto-detection doesn't work on t-head c9xx cores */
> > > -       hfeatures->mhpm_count = 29;
> > > +       /* D1 has 29 mhpmevent csrs, but only 3-9,13-17 have valid value */
> > > +       hfeatures->mhpm_count = 15;
> >
> > With the latest non-contiguous counter support, we need to set
> > mhpm_mask as well.
> > I have taken care of this at the time of merging this patch.
> >
> > Reviewed-by: Anup Patel <anup@brainfault.org>
> >
> > Applied this patch to the riscv/opensbi repo.
> Sorry for the delayed reply. I think we need more tests on this patch.
> Now my colleagues are cooperating with Inochi to solve the problem.
>
> We are also solving the perf record problem on D1, so I'm unsure about
> the final solution. I recommend you hold this patch for a while. Maybe
> we would adjust it.

Okay, I have dropped it from my queue.

Regards,
Anup

>
> ps:
> Include Heiko email
>
> >
> > Thanks,
> > Anup
> >
> > >         hfeatures->mhpm_bits = 64;
> > >
> > >         return 0;
> > > --
> > > 2.41.0
> > >
> > >
> > > --
> > > opensbi mailing list
> > > opensbi@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/opensbi
>
>
>
> --
> Best Regards
>  Guo Ren
Atish Patra Aug. 8, 2023, 7 a.m. UTC | #4 
On Sat, Aug 5, 2023 at 10:33 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Sun, Aug 6, 2023 at 1:04 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Thu, Aug 3, 2023 at 7:20 AM Inochi Amaoto <inochiama@outlook.com> wrote:
> > >
> > > Only the CSR mhpmevent 3-9,13-17 of D1 have valid function, so reduce
> > > the mhpm_count to 15 to avoid invalid usage.
> > >
> > > Signed-off-by: Inochi Amaoto <inochiama@outlook.com>
> > > ---
> > >  platform/generic/allwinner/sun20i-d1.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
> > > index 0f0a9f3..94a3a5e 100644
> > > --- a/platform/generic/allwinner/sun20i-d1.c
> > > +++ b/platform/generic/allwinner/sun20i-d1.c
> > > @@ -272,7 +272,8 @@ static int sun20i_d1_extensions_init(const struct fdt_match *match,
> > >         sbi_pmu_set_device(&thead_c9xx_pmu_device);
> > >
> > >         /* auto-detection doesn't work on t-head c9xx cores */
> > > -       hfeatures->mhpm_count = 29;
> > > +       /* D1 has 29 mhpmevent csrs, but only 3-9,13-17 have valid value */
> > > +       hfeatures->mhpm_count = 15;
> >
> > With the latest non-contiguous counter support, we need to set
> > mhpm_mask as well.
> > I have taken care of this at the time of merging this patch.
> >
> > Reviewed-by: Anup Patel <anup@brainfault.org>
> >
> > Applied this patch to the riscv/opensbi repo.
> Sorry for the delayed reply. I think we need more tests on this patch.
> Now my colleagues are cooperating with Inochi to solve the problem.
>
> We are also solving the perf record problem on D1, so I'm unsure about
> the final solution. I recommend you hold this patch for a while. Maybe
> we would adjust it.
>

What is the problem with perf record on D1 ? Is it hardware related or
any kernel issue ?

> ps:
> Include Heiko email
>
> >
> > Thanks,
> > Anup
> >
> > >         hfeatures->mhpm_bits = 64;
> > >
> > >         return 0;
> > > --
> > > 2.41.0
> > >
> > >
> > > --
> > > opensbi mailing list
> > > opensbi@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/opensbi
>
>
>
> --
> Best Regards
>  Guo Ren
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
diff mbox series

Patch

diff --git a/platform/generic/allwinner/sun20i-d1.c b/platform/generic/allwinner/sun20i-d1.c
index 0f0a9f3..94a3a5e 100644
--- a/platform/generic/allwinner/sun20i-d1.c
+++ b/platform/generic/allwinner/sun20i-d1.c
@@ -272,7 +272,8 @@  static int sun20i_d1_extensions_init(const struct fdt_match *match,
 	sbi_pmu_set_device(&thead_c9xx_pmu_device);

 	/* auto-detection doesn't work on t-head c9xx cores */
-	hfeatures->mhpm_count = 29;
+	/* D1 has 29 mhpmevent csrs, but only 3-9,13-17 have valid value */
+	hfeatures->mhpm_count = 15;
 	hfeatures->mhpm_bits = 64;

 	return 0;