diff mbox series

drm/msm: fix the highest_bank_bit for sc7180

Message ID 20240808235227.2701479-1-quic_abhinavk@quicinc.com
State Accepted
Commit 3e30296b374af33cb4c12ff93df0b1e5b2d0f80b
Headers show
Series drm/msm: fix the highest_bank_bit for sc7180 | expand

Commit Message

Abhinav Kumar Aug. 8, 2024, 11:52 p.m. UTC
sc7180 programs the ubwc settings as 0x1e as that would mean a
highest bank bit of 14 which matches what the GPU sets as well.

However, the highest_bank_bit field of the msm_mdss_data which is
being used to program the SSPP's fetch configuration is programmed
to a highest bank bit of 16 as 0x3 translates to 16 and not 14.

Fix the highest bank bit field used for the SSPP to match the mdss
and gpu settings.

Fixes: 6f410b246209 ("drm/msm/mdss: populate missing data")
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
---
 drivers/gpu/drm/msm/msm_mdss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rob Clark Aug. 12, 2024, 6:12 p.m. UTC | #1
On Thu, Aug 8, 2024 at 4:52 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
> sc7180 programs the ubwc settings as 0x1e as that would mean a
> highest bank bit of 14 which matches what the GPU sets as well.
>
> However, the highest_bank_bit field of the msm_mdss_data which is
> being used to program the SSPP's fetch configuration is programmed
> to a highest bank bit of 16 as 0x3 translates to 16 and not 14.
>
> Fix the highest bank bit field used for the SSPP to match the mdss
> and gpu settings.
>
> Fixes: 6f410b246209 ("drm/msm/mdss: populate missing data")
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
>  drivers/gpu/drm/msm/msm_mdss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index d90b9471ba6f..faa88fd6eb4d 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -577,7 +577,7 @@ static const struct msm_mdss_data sc7180_data = {
>         .ubwc_enc_version = UBWC_2_0,
>         .ubwc_dec_version = UBWC_2_0,
>         .ubwc_static = 0x1e,
> -       .highest_bank_bit = 0x3,
> +       .highest_bank_bit = 0x1,
>         .reg_bus_bw = 76800,
>  };
>
> --
> 2.44.0
>
Stephen Boyd Aug. 12, 2024, 6:36 p.m. UTC | #2
Quoting Abhinav Kumar (2024-08-08 16:52:27)
> sc7180 programs the ubwc settings as 0x1e as that would mean a
> highest bank bit of 14 which matches what the GPU sets as well.
>
> However, the highest_bank_bit field of the msm_mdss_data which is
> being used to program the SSPP's fetch configuration is programmed
> to a highest bank bit of 16 as 0x3 translates to 16 and not 14.
>
> Fix the highest bank bit field used for the SSPP to match the mdss
> and gpu settings.
>
> Fixes: 6f410b246209 ("drm/msm/mdss: populate missing data")
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---

Tested-by: Stephen Boyd <swboyd@chromium.org> # Trogdor.Lazor
Stephen Boyd Aug. 12, 2024, 6:40 p.m. UTC | #3
Quoting Abhinav Kumar (2024-08-08 16:52:27)
> sc7180 programs the ubwc settings as 0x1e as that would mean a
> highest bank bit of 14 which matches what the GPU sets as well.
>
> However, the highest_bank_bit field of the msm_mdss_data which is
> being used to program the SSPP's fetch configuration is programmed
> to a highest bank bit of 16 as 0x3 translates to 16 and not 14.
>
> Fix the highest bank bit field used for the SSPP to match the mdss
> and gpu settings.
>
> Fixes: 6f410b246209 ("drm/msm/mdss: populate missing data")
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  drivers/gpu/drm/msm/msm_mdss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index d90b9471ba6f..faa88fd6eb4d 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -577,7 +577,7 @@ static const struct msm_mdss_data sc7180_data = {
>         .ubwc_enc_version = UBWC_2_0,
>         .ubwc_dec_version = UBWC_2_0,
>         .ubwc_static = 0x1e,
> -       .highest_bank_bit = 0x3,
> +       .highest_bank_bit = 0x1,

Usually when I see hex it's because there's a mask. This isn't a mask
though? Can it just be '1'?
Abhinav Kumar Aug. 12, 2024, 7:41 p.m. UTC | #4
On 8/12/2024 11:40 AM, Stephen Boyd wrote:
> Quoting Abhinav Kumar (2024-08-08 16:52:27)
>> sc7180 programs the ubwc settings as 0x1e as that would mean a
>> highest bank bit of 14 which matches what the GPU sets as well.
>>
>> However, the highest_bank_bit field of the msm_mdss_data which is
>> being used to program the SSPP's fetch configuration is programmed
>> to a highest bank bit of 16 as 0x3 translates to 16 and not 14.
>>
>> Fix the highest bank bit field used for the SSPP to match the mdss
>> and gpu settings.
>>
>> Fixes: 6f410b246209 ("drm/msm/mdss: populate missing data")
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/msm_mdss.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
>> index d90b9471ba6f..faa88fd6eb4d 100644
>> --- a/drivers/gpu/drm/msm/msm_mdss.c
>> +++ b/drivers/gpu/drm/msm/msm_mdss.c
>> @@ -577,7 +577,7 @@ static const struct msm_mdss_data sc7180_data = {
>>          .ubwc_enc_version = UBWC_2_0,
>>          .ubwc_dec_version = UBWC_2_0,
>>          .ubwc_static = 0x1e,
>> -       .highest_bank_bit = 0x3,
>> +       .highest_bank_bit = 0x1,
> 
> Usually when I see hex it's because there's a mask. This isn't a mask
> though? Can it just be '1'?

I just retained the same convention that was used earlier. It seems like 
a mix and match right now. sc7180, sm6115 and qcm2290 were using 0x.

I can post a separate change to change all of them.
Stephen Boyd Aug. 12, 2024, 8:40 p.m. UTC | #5
Quoting Abhinav Kumar (2024-08-12 12:41:40)
>
> I just retained the same convention that was used earlier. It seems like
> a mix and match right now. sc7180, sm6115 and qcm2290 were using 0x.
>
> I can post a separate change to change all of them.

Sounds good.
Dmitry Baryshkov Aug. 28, 2024, 7:59 p.m. UTC | #6
On Mon, Aug 12, 2024 at 12:41:40PM GMT, Abhinav Kumar wrote:
> 
> 
> On 8/12/2024 11:40 AM, Stephen Boyd wrote:
> > Quoting Abhinav Kumar (2024-08-08 16:52:27)
> > > sc7180 programs the ubwc settings as 0x1e as that would mean a
> > > highest bank bit of 14 which matches what the GPU sets as well.
> > > 
> > > However, the highest_bank_bit field of the msm_mdss_data which is
> > > being used to program the SSPP's fetch configuration is programmed
> > > to a highest bank bit of 16 as 0x3 translates to 16 and not 14.
> > > 
> > > Fix the highest bank bit field used for the SSPP to match the mdss
> > > and gpu settings.
> > > 
> > > Fixes: 6f410b246209 ("drm/msm/mdss: populate missing data")
> > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> > > ---
> > >   drivers/gpu/drm/msm/msm_mdss.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> > > index d90b9471ba6f..faa88fd6eb4d 100644
> > > --- a/drivers/gpu/drm/msm/msm_mdss.c
> > > +++ b/drivers/gpu/drm/msm/msm_mdss.c
> > > @@ -577,7 +577,7 @@ static const struct msm_mdss_data sc7180_data = {
> > >          .ubwc_enc_version = UBWC_2_0,
> > >          .ubwc_dec_version = UBWC_2_0,
> > >          .ubwc_static = 0x1e,
> > > -       .highest_bank_bit = 0x3,
> > > +       .highest_bank_bit = 0x1,
> > 
> > Usually when I see hex it's because there's a mask. This isn't a mask
> > though? Can it just be '1'?
> 
> I just retained the same convention that was used earlier. It seems like a
> mix and match right now. sc7180, sm6115 and qcm2290 were using 0x.
> 
> I can post a separate change to change all of them.

We probably need to do a +13 to all of them to follow the approach of
the a6xx code.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index d90b9471ba6f..faa88fd6eb4d 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -577,7 +577,7 @@  static const struct msm_mdss_data sc7180_data = {
 	.ubwc_enc_version = UBWC_2_0,
 	.ubwc_dec_version = UBWC_2_0,
 	.ubwc_static = 0x1e,
-	.highest_bank_bit = 0x3,
+	.highest_bank_bit = 0x1,
 	.reg_bus_bw = 76800,
 };