From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
swizzling) is what we want on this platform (and others with a UBWC
1.0 encoder).
Fix it to make mesa happy (the hardware doesn't care about the 2 higher
bits, as they weren't consumed on this platform).
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/soc/qcom/ubwc_config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
--- a/drivers/soc/qcom/ubwc_config.c
+++ b/drivers/soc/qcom/ubwc_config.c
@@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
static const struct qcom_ubwc_cfg_data sm6125_data = {
.ubwc_enc_version = UBWC_1_0,
.ubwc_dec_version = UBWC_3_0,
- .ubwc_swizzle = 1,
+ .ubwc_swizzle = 7,
.highest_bank_bit = 14,
};
--
2.49.0On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
> swizzling) is what we want on this platform (and others with a UBWC
> 1.0 encoder).
>
> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
> bits, as they weren't consumed on this platform).
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/soc/qcom/ubwc_config.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
> --- a/drivers/soc/qcom/ubwc_config.c
> +++ b/drivers/soc/qcom/ubwc_config.c
> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
> static const struct qcom_ubwc_cfg_data sm6125_data = {
> .ubwc_enc_version = UBWC_1_0,
> .ubwc_dec_version = UBWC_3_0,
> - .ubwc_swizzle = 1,
> + .ubwc_swizzle = 7,
> .highest_bank_bit = 14,
> };
Add a comment and squash into the patch 1.
--
With best wishes
Dmitry
On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>> swizzling) is what we want on this platform (and others with a UBWC
>> 1.0 encoder).
>>
>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>> bits, as they weren't consumed on this platform).
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>> drivers/soc/qcom/ubwc_config.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>> --- a/drivers/soc/qcom/ubwc_config.c
>> +++ b/drivers/soc/qcom/ubwc_config.c
>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>> static const struct qcom_ubwc_cfg_data sm6125_data = {
>> .ubwc_enc_version = UBWC_1_0,
>> .ubwc_dec_version = UBWC_3_0,
>> - .ubwc_swizzle = 1,
>> + .ubwc_swizzle = 7,
>> .highest_bank_bit = 14,
>> };
>
> Add a comment and squash into the patch 1.
I don't think that's a good idea, plus this series should be merged
together anyway
Konrad
On 14/05/2025 23:05, Konrad Dybcio wrote:
> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>
>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>>> swizzling) is what we want on this platform (and others with a UBWC
>>> 1.0 encoder).
>>>
>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>>> bits, as they weren't consumed on this platform).
>>>
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> ---
>>> drivers/soc/qcom/ubwc_config.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>>> --- a/drivers/soc/qcom/ubwc_config.c
>>> +++ b/drivers/soc/qcom/ubwc_config.c
>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>>> static const struct qcom_ubwc_cfg_data sm6125_data = {
>>> .ubwc_enc_version = UBWC_1_0,
>>> .ubwc_dec_version = UBWC_3_0,
>>> - .ubwc_swizzle = 1,
>>> + .ubwc_swizzle = 7,
>>> .highest_bank_bit = 14,
>>> };
>>
>> Add a comment and squash into the patch 1.
>
> I don't think that's a good idea, plus this series should be merged
> together anyway
Well... Granted Rob's comment, I really think the patches should be
reordered a bit:
- MDSS: offset HBB by 13 (patch 2)
- switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
- get a handle (patch 4)
- resolve / simplify (patches 5-10, not squashed)
- fix sm6125 (patch 13)
- WARN_ON (swizzle != swizzle) or (HBB != HBB)
- switch to common R/O config, keeping WARN_ON for the calculated values
(with the hope to drop them after testing)
WDYT?
--
With best wishes
Dmitry
On 5/14/25 10:33 PM, Dmitry Baryshkov wrote:
> On 14/05/2025 23:05, Konrad Dybcio wrote:
>> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
>>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>
>>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>>>> swizzling) is what we want on this platform (and others with a UBWC
>>>> 1.0 encoder).
>>>>
>>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>>>> bits, as they weren't consumed on this platform).
>>>>
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>> ---
>>>> drivers/soc/qcom/ubwc_config.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>>>> --- a/drivers/soc/qcom/ubwc_config.c
>>>> +++ b/drivers/soc/qcom/ubwc_config.c
>>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>>>> static const struct qcom_ubwc_cfg_data sm6125_data = {
>>>> .ubwc_enc_version = UBWC_1_0,
>>>> .ubwc_dec_version = UBWC_3_0,
>>>> - .ubwc_swizzle = 1,
>>>> + .ubwc_swizzle = 7,
>>>> .highest_bank_bit = 14,
>>>> };
>>>
>>> Add a comment and squash into the patch 1.
>>
>> I don't think that's a good idea, plus this series should be merged
>> together anyway
>
> Well... Granted Rob's comment, I really think the patches should be reordered a bit:
>
> - MDSS: offset HBB by 13 (patch 2)
> - switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
> - get a handle (patch 4)
> - resolve / simplify (patches 5-10, not squashed)
> - fix sm6125 (patch 13)
> - WARN_ON (swizzle != swizzle) or (HBB != HBB)
> - switch to common R/O config, keeping WARN_ON for the calculated values (with the hope to drop them after testing)
Does this bring any functional benefit? This series is unfun to remix
Konrad
On 15/05/2025 19:18, Konrad Dybcio wrote:
> On 5/14/25 10:33 PM, Dmitry Baryshkov wrote:
>> On 14/05/2025 23:05, Konrad Dybcio wrote:
>>> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
>>>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>
>>>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>>>>> swizzling) is what we want on this platform (and others with a UBWC
>>>>> 1.0 encoder).
>>>>>
>>>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>>>>> bits, as they weren't consumed on this platform).
>>>>>
>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>> ---
>>>>> drivers/soc/qcom/ubwc_config.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>>>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>>>>> --- a/drivers/soc/qcom/ubwc_config.c
>>>>> +++ b/drivers/soc/qcom/ubwc_config.c
>>>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>>>>> static const struct qcom_ubwc_cfg_data sm6125_data = {
>>>>> .ubwc_enc_version = UBWC_1_0,
>>>>> .ubwc_dec_version = UBWC_3_0,
>>>>> - .ubwc_swizzle = 1,
>>>>> + .ubwc_swizzle = 7,
>>>>> .highest_bank_bit = 14,
>>>>> };
>>>>
>>>> Add a comment and squash into the patch 1.
>>>
>>> I don't think that's a good idea, plus this series should be merged
>>> together anyway
>>
>> Well... Granted Rob's comment, I really think the patches should be reordered a bit:
>>
>> - MDSS: offset HBB by 13 (patch 2)
>> - switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
>> - get a handle (patch 4)
>> - resolve / simplify (patches 5-10, not squashed)
>> - fix sm6125 (patch 13)
>> - WARN_ON (swizzle != swizzle) or (HBB != HBB)
>> - switch to common R/O config, keeping WARN_ON for the calculated values (with the hope to drop them after testing)
>
> Does this bring any functional benefit? This series is unfun to remix
I know the pain.
The functional benefit is to have the WARN_ON and side-by-side
comparison of common_ubwc_config vs computed ubwc_config for HBB and
swizzle.
You can say that I dislike the idea of copying & modifying config as
this is the code that we will drop later (hopefully).
--
With best wishes
Dmitry
On 5/15/25 6:21 PM, Dmitry Baryshkov wrote:
> On 15/05/2025 19:18, Konrad Dybcio wrote:
>> On 5/14/25 10:33 PM, Dmitry Baryshkov wrote:
>>> On 14/05/2025 23:05, Konrad Dybcio wrote:
>>>> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>
>>>>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>>>>>> swizzling) is what we want on this platform (and others with a UBWC
>>>>>> 1.0 encoder).
>>>>>>
>>>>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>>>>>> bits, as they weren't consumed on this platform).
>>>>>>
>>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>> ---
>>>>>> drivers/soc/qcom/ubwc_config.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>>>>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>>>>>> --- a/drivers/soc/qcom/ubwc_config.c
>>>>>> +++ b/drivers/soc/qcom/ubwc_config.c
>>>>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>>>>>> static const struct qcom_ubwc_cfg_data sm6125_data = {
>>>>>> .ubwc_enc_version = UBWC_1_0,
>>>>>> .ubwc_dec_version = UBWC_3_0,
>>>>>> - .ubwc_swizzle = 1,
>>>>>> + .ubwc_swizzle = 7,
>>>>>> .highest_bank_bit = 14,
>>>>>> };
>>>>>
>>>>> Add a comment and squash into the patch 1.
>>>>
>>>> I don't think that's a good idea, plus this series should be merged
>>>> together anyway
>>>
>>> Well... Granted Rob's comment, I really think the patches should be reordered a bit:
>>>
>>> - MDSS: offset HBB by 13 (patch 2)
>>> - switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
>>> - get a handle (patch 4)
>>> - resolve / simplify (patches 5-10, not squashed)
>>> - fix sm6125 (patch 13)
>>> - WARN_ON (swizzle != swizzle) or (HBB != HBB)
>>> - switch to common R/O config, keeping WARN_ON for the calculated values (with the hope to drop them after testing)
>>
>> Does this bring any functional benefit? This series is unfun to remix
>
> I know the pain.
>
> The functional benefit is to have the WARN_ON and side-by-side comparison of common_ubwc_config vs computed ubwc_config for HBB and swizzle.
HBB I agree, since we'll be outsourcing it to yet another driver, swizzle
should be good enough (tm) - I scanned through the values in the driver
and couldn't find anything wrong just by eye
I realize this sounds funny, but all in all I don't think it's worth the
effort just for that one
Konrad
On Thu, 15 May 2025 at 19:36, Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 5/15/25 6:21 PM, Dmitry Baryshkov wrote:
> > On 15/05/2025 19:18, Konrad Dybcio wrote:
> >> On 5/14/25 10:33 PM, Dmitry Baryshkov wrote:
> >>> On 14/05/2025 23:05, Konrad Dybcio wrote:
> >>>> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
> >>>>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
> >>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>>>>>
> >>>>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
> >>>>>> swizzling) is what we want on this platform (and others with a UBWC
> >>>>>> 1.0 encoder).
> >>>>>>
> >>>>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
> >>>>>> bits, as they weren't consumed on this platform).
> >>>>>>
> >>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>>>>> ---
> >>>>>> drivers/soc/qcom/ubwc_config.c | 2 +-
> >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
> >>>>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
> >>>>>> --- a/drivers/soc/qcom/ubwc_config.c
> >>>>>> +++ b/drivers/soc/qcom/ubwc_config.c
> >>>>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
> >>>>>> static const struct qcom_ubwc_cfg_data sm6125_data = {
> >>>>>> .ubwc_enc_version = UBWC_1_0,
> >>>>>> .ubwc_dec_version = UBWC_3_0,
> >>>>>> - .ubwc_swizzle = 1,
> >>>>>> + .ubwc_swizzle = 7,
> >>>>>> .highest_bank_bit = 14,
> >>>>>> };
> >>>>>
> >>>>> Add a comment and squash into the patch 1.
> >>>>
> >>>> I don't think that's a good idea, plus this series should be merged
> >>>> together anyway
> >>>
> >>> Well... Granted Rob's comment, I really think the patches should be reordered a bit:
> >>>
> >>> - MDSS: offset HBB by 13 (patch 2)
> >>> - switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
> >>> - get a handle (patch 4)
> >>> - resolve / simplify (patches 5-10, not squashed)
> >>> - fix sm6125 (patch 13)
> >>> - WARN_ON (swizzle != swizzle) or (HBB != HBB)
> >>> - switch to common R/O config, keeping WARN_ON for the calculated values (with the hope to drop them after testing)
> >>
> >> Does this bring any functional benefit? This series is unfun to remix
> >
> > I know the pain.
> >
> > The functional benefit is to have the WARN_ON and side-by-side comparison of common_ubwc_config vs computed ubwc_config for HBB and swizzle.
>
> HBB I agree, since we'll be outsourcing it to yet another driver, swizzle
> should be good enough (tm) - I scanned through the values in the driver
> and couldn't find anything wrong just by eye
Well. What is the ubwc_swizzle value used for SDM845? I think it
should be 6 according to a6xx_gpu.c and 0 according to msm_mdss.c.
Yes, higher bits are most likely ignored. Still, we'd better have one
correct value.
>
> I realize this sounds funny, but all in all I don't think it's worth the
> effort just for that one
>
> Konrad
--
With best wishes
Dmitry
On 5/15/25 7:15 PM, Dmitry Baryshkov wrote:
> On Thu, 15 May 2025 at 19:36, Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 5/15/25 6:21 PM, Dmitry Baryshkov wrote:
>>> On 15/05/2025 19:18, Konrad Dybcio wrote:
>>>> On 5/14/25 10:33 PM, Dmitry Baryshkov wrote:
>>>>> On 14/05/2025 23:05, Konrad Dybcio wrote:
>>>>>> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
>>>>>>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>>>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>>>
>>>>>>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>>>>>>>> swizzling) is what we want on this platform (and others with a UBWC
>>>>>>>> 1.0 encoder).
>>>>>>>>
>>>>>>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>>>>>>>> bits, as they weren't consumed on this platform).
>>>>>>>>
>>>>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>>> ---
>>>>>>>> drivers/soc/qcom/ubwc_config.c | 2 +-
>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>>>>>>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>>>>>>>> --- a/drivers/soc/qcom/ubwc_config.c
>>>>>>>> +++ b/drivers/soc/qcom/ubwc_config.c
>>>>>>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>>>>>>>> static const struct qcom_ubwc_cfg_data sm6125_data = {
>>>>>>>> .ubwc_enc_version = UBWC_1_0,
>>>>>>>> .ubwc_dec_version = UBWC_3_0,
>>>>>>>> - .ubwc_swizzle = 1,
>>>>>>>> + .ubwc_swizzle = 7,
>>>>>>>> .highest_bank_bit = 14,
>>>>>>>> };
>>>>>>>
>>>>>>> Add a comment and squash into the patch 1.
>>>>>>
>>>>>> I don't think that's a good idea, plus this series should be merged
>>>>>> together anyway
>>>>>
>>>>> Well... Granted Rob's comment, I really think the patches should be reordered a bit:
>>>>>
>>>>> - MDSS: offset HBB by 13 (patch 2)
>>>>> - switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
>>>>> - get a handle (patch 4)
>>>>> - resolve / simplify (patches 5-10, not squashed)
>>>>> - fix sm6125 (patch 13)
>>>>> - WARN_ON (swizzle != swizzle) or (HBB != HBB)
>>>>> - switch to common R/O config, keeping WARN_ON for the calculated values (with the hope to drop them after testing)
>>>>
>>>> Does this bring any functional benefit? This series is unfun to remix
>>>
>>> I know the pain.
>>>
>>> The functional benefit is to have the WARN_ON and side-by-side comparison of common_ubwc_config vs computed ubwc_config for HBB and swizzle.
>>
>> HBB I agree, since we'll be outsourcing it to yet another driver, swizzle
>> should be good enough (tm) - I scanned through the values in the driver
>> and couldn't find anything wrong just by eye
>
> Well. What is the ubwc_swizzle value used for SDM845? I think it
> should be 6 according to a6xx_gpu.c and 0 according to msm_mdss.c.
> Yes, higher bits are most likely ignored. Still, we'd better have one
> correct value.
Ehh, so laziness bites after all..
Unfortunately it seems like I don't have a good answer for you
- although I can infer a technically valid config for these
at the very least:
msm8937
msm8998
sc8180x
sdm670
sdm845
sm6150
sm7150
sm8150
with the ubwc1.0 platforms receiving all 3 levels and ubwc 2.0/
3.0 enabling 2/3
this however I'm not sure matches what downstream does..
Konrad
On 15/05/2025 20:56, Konrad Dybcio wrote:
> On 5/15/25 7:15 PM, Dmitry Baryshkov wrote:
>> On Thu, 15 May 2025 at 19:36, Konrad Dybcio
>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>
>>> On 5/15/25 6:21 PM, Dmitry Baryshkov wrote:
>>>> On 15/05/2025 19:18, Konrad Dybcio wrote:
>>>>> On 5/14/25 10:33 PM, Dmitry Baryshkov wrote:
>>>>>> On 14/05/2025 23:05, Konrad Dybcio wrote:
>>>>>>> On 5/14/25 9:23 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Wed, May 14, 2025 at 05:10:33PM +0200, Konrad Dybcio wrote:
>>>>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>>>>
>>>>>>>>> The value of 7 (a.k.a. GENMASK(2, 0), a.k.a. disabling levels 1-3 of
>>>>>>>>> swizzling) is what we want on this platform (and others with a UBWC
>>>>>>>>> 1.0 encoder).
>>>>>>>>>
>>>>>>>>> Fix it to make mesa happy (the hardware doesn't care about the 2 higher
>>>>>>>>> bits, as they weren't consumed on this platform).
>>>>>>>>>
>>>>>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>>>> ---
>>>>>>>>> drivers/soc/qcom/ubwc_config.c | 2 +-
>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
>>>>>>>>> index 9caecd071035ccb03f14464e9b7129ba34a7f862..96b94cf01218cce2dacdba22c7573ba6148fcdd1 100644
>>>>>>>>> --- a/drivers/soc/qcom/ubwc_config.c
>>>>>>>>> +++ b/drivers/soc/qcom/ubwc_config.c
>>>>>>>>> @@ -103,7 +103,7 @@ static const struct qcom_ubwc_cfg_data sm6115_data = {
>>>>>>>>> static const struct qcom_ubwc_cfg_data sm6125_data = {
>>>>>>>>> .ubwc_enc_version = UBWC_1_0,
>>>>>>>>> .ubwc_dec_version = UBWC_3_0,
>>>>>>>>> - .ubwc_swizzle = 1,
>>>>>>>>> + .ubwc_swizzle = 7,
>>>>>>>>> .highest_bank_bit = 14,
>>>>>>>>> };
>>>>>>>>
>>>>>>>> Add a comment and squash into the patch 1.
>>>>>>>
>>>>>>> I don't think that's a good idea, plus this series should be merged
>>>>>>> together anyway
>>>>>>
>>>>>> Well... Granted Rob's comment, I really think the patches should be reordered a bit:
>>>>>>
>>>>>> - MDSS: offset HBB by 13 (patch 2)
>>>>>> - switch drm/msm/mdss and display to common DB (patches 1+3 squashed)
>>>>>> - get a handle (patch 4)
>>>>>> - resolve / simplify (patches 5-10, not squashed)
>>>>>> - fix sm6125 (patch 13)
>>>>>> - WARN_ON (swizzle != swizzle) or (HBB != HBB)
>>>>>> - switch to common R/O config, keeping WARN_ON for the calculated values (with the hope to drop them after testing)
>>>>>
>>>>> Does this bring any functional benefit? This series is unfun to remix
>>>>
>>>> I know the pain.
>>>>
>>>> The functional benefit is to have the WARN_ON and side-by-side comparison of common_ubwc_config vs computed ubwc_config for HBB and swizzle.
>>>
>>> HBB I agree, since we'll be outsourcing it to yet another driver, swizzle
>>> should be good enough (tm) - I scanned through the values in the driver
>>> and couldn't find anything wrong just by eye
>>
>> Well. What is the ubwc_swizzle value used for SDM845? I think it
>> should be 6 according to a6xx_gpu.c and 0 according to msm_mdss.c.
>> Yes, higher bits are most likely ignored. Still, we'd better have one
>> correct value.
>
> Ehh, so laziness bites after all..
>
> Unfortunately it seems like I don't have a good answer for you
> - although I can infer a technically valid config for these
> at the very least:
>
> msm8937
> msm8998
> sc8180x
> sdm670
> sdm845
> sm6150
> sm7150
> sm8150
WARN_ON would be a good thing in the end
>
> with the ubwc1.0 platforms receiving all 3 levels and ubwc 2.0/
> 3.0 enabling 2/3
>
> this however I'm not sure matches what downstream does..
Well, let's match previous GPU config, so 6 whenever we don't define
anything special.
>
> Konrad
--
With best wishes
Dmitry
© 2016 - 2026 Red Hat, Inc.