| Message ID | 20230214173145.2482651-11-konrad.dybcio@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,01/14] drm/msm/a6xx: De-staticize sptprac en/disable functions | expand |
On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > One of the protected ranges was too small (compared to the data we > have downstream). Fix it. > > Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization") > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 503c750216e6..d6b38bfdb3b4 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = { > A6XX_PROTECT_NORDWR(0x00800, 0x0082), > A6XX_PROTECT_NORDWR(0x008a0, 0x0008), > A6XX_PROTECT_NORDWR(0x008ab, 0x0024), > - A6XX_PROTECT_RDONLY(0x008de, 0x00ae), > + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc), Nak, this is intentional, we need userspace to be able to configure the CP counters. Otherwise this would break fdperf, perfetto, etc (although maybe we should comment where we diverge from downstream) BR, -R > A6XX_PROTECT_NORDWR(0x00900, 0x004d), > A6XX_PROTECT_NORDWR(0x0098d, 0x0272), > A6XX_PROTECT_NORDWR(0x00e00, 0x0001), > -- > 2.39.1 >
On 14/02/2023 23:56, Rob Clark wrote: > On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> One of the protected ranges was too small (compared to the data we >> have downstream). Fix it. >> >> Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization") >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> index 503c750216e6..d6b38bfdb3b4 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = { >> A6XX_PROTECT_NORDWR(0x00800, 0x0082), >> A6XX_PROTECT_NORDWR(0x008a0, 0x0008), >> A6XX_PROTECT_NORDWR(0x008ab, 0x0024), >> - A6XX_PROTECT_RDONLY(0x008de, 0x00ae), >> + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc), > > Nak, this is intentional, we need userspace to be able to configure > the CP counters. Otherwise this would break fdperf, perfetto, etc > > (although maybe we should comment where we diverge from downstream) Yes, please. Otherwise it is extremely hard to understand the reason for diversion between the vendor driver and our one. > > BR, > -R > >> A6XX_PROTECT_NORDWR(0x00900, 0x004d), >> A6XX_PROTECT_NORDWR(0x0098d, 0x0272), >> A6XX_PROTECT_NORDWR(0x00e00, 0x0001), >> -- >> 2.39.1 >>
On 15.02.2023 01:10, Dmitry Baryshkov wrote: > On 14/02/2023 23:56, Rob Clark wrote: >> On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >>> >>> One of the protected ranges was too small (compared to the data we >>> have downstream). Fix it. >>> >>> Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization") >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> --- >>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>> index 503c750216e6..d6b38bfdb3b4 100644 >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>> @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = { >>> A6XX_PROTECT_NORDWR(0x00800, 0x0082), >>> A6XX_PROTECT_NORDWR(0x008a0, 0x0008), >>> A6XX_PROTECT_NORDWR(0x008ab, 0x0024), >>> - A6XX_PROTECT_RDONLY(0x008de, 0x00ae), >>> + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc), >> >> Nak, this is intentional, we need userspace to be able to configure >> the CP counters. Otherwise this would break fdperf, perfetto, etc >> >> (although maybe we should comment where we diverge from downstream) > > Yes, please. Otherwise it is extremely hard to understand the reason for diversion between the vendor driver and our one. +1 I am content with dropping this patch from this series, so long as you leave a clue for others to not scratch their heads on this! Konrad > >> >> BR, >> -R >> >>> A6XX_PROTECT_NORDWR(0x00900, 0x004d), >>> A6XX_PROTECT_NORDWR(0x0098d, 0x0272), >>> A6XX_PROTECT_NORDWR(0x00e00, 0x0001), >>> -- >>> 2.39.1 >>> >
On Tue, Feb 14, 2023 at 4:38 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 15.02.2023 01:10, Dmitry Baryshkov wrote: > > On 14/02/2023 23:56, Rob Clark wrote: > >> On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > >>> > >>> One of the protected ranges was too small (compared to the data we > >>> have downstream). Fix it. > >>> > >>> Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization") > >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > >>> --- > >>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>> index 503c750216e6..d6b38bfdb3b4 100644 > >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>> @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = { > >>> A6XX_PROTECT_NORDWR(0x00800, 0x0082), > >>> A6XX_PROTECT_NORDWR(0x008a0, 0x0008), > >>> A6XX_PROTECT_NORDWR(0x008ab, 0x0024), > >>> - A6XX_PROTECT_RDONLY(0x008de, 0x00ae), > >>> + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc), > >> > >> Nak, this is intentional, we need userspace to be able to configure > >> the CP counters. Otherwise this would break fdperf, perfetto, etc > >> > >> (although maybe we should comment where we diverge from downstream) > > > > Yes, please. Otherwise it is extremely hard to understand the reason for diversion between the vendor driver and our one. > +1 > > I am content with dropping this patch from this series, so long > as you leave a clue for others to not scratch their heads on this! Yeah, I admit it is kinda a trap as-is. And makes things less obvious what to do when porting from downstream. When I get a few minutes I'll double check that there weren't any other exceptions (I don't think they were but it has been a while) and add some comments. BR, -R > Konrad > > > >> > >> BR, > >> -R > >> > >>> A6XX_PROTECT_NORDWR(0x00900, 0x004d), > >>> A6XX_PROTECT_NORDWR(0x0098d, 0x0272), > >>> A6XX_PROTECT_NORDWR(0x00e00, 0x0001), > >>> -- > >>> 2.39.1 > >>> > >
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index 503c750216e6..d6b38bfdb3b4 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = { A6XX_PROTECT_NORDWR(0x00800, 0x0082), A6XX_PROTECT_NORDWR(0x008a0, 0x0008), A6XX_PROTECT_NORDWR(0x008ab, 0x0024), - A6XX_PROTECT_RDONLY(0x008de, 0x00ae), + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc), A6XX_PROTECT_NORDWR(0x00900, 0x004d), A6XX_PROTECT_NORDWR(0x0098d, 0x0272), A6XX_PROTECT_NORDWR(0x00e00, 0x0001),
One of the protected ranges was too small (compared to the data we have downstream). Fix it. Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization") Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)