[PATCH] drm/msm/dpu: correct LM pairing for SM6150

[PATCH] drm/msm/dpu: correct LM pairing for SM6150

[Dmitry Baryshkov]

According to the vendor devicetree on SM6150 LM_0 is paired with LM_2
rather than LM_1. Correct pairing indices.

Fixes: cb2f9144693b ("drm/msm/dpu: Add SM6150 support")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
index 621a2140f675fa28b3a7fcd8573e59b306cd6832..81eb274cc7000a3b70b0f6650088ddcd24648eab 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
@@ -116,20 +116,20 @@ static const struct dpu_lm_cfg sm6150_lm[] = {
 		.sblk = &sdm845_lm_sblk,
 		.pingpong = PINGPONG_0,
 		.dspp = DSPP_0,
-		.lm_pair = LM_1,
+		.lm_pair = LM_2,
 	}, {
 		.name = "lm_1", .id = LM_1,
 		.base = 0x45000, .len = 0x320,
 		.features = MIXER_QCM2290_MASK,
 		.sblk = &sdm845_lm_sblk,
 		.pingpong = PINGPONG_1,
-		.lm_pair = LM_0,
 	}, {
 		.name = "lm_2", .id = LM_2,
 		.base = 0x46000, .len = 0x320,
 		.features = MIXER_QCM2290_MASK,
 		.sblk = &sdm845_lm_sblk,
 		.pingpong = PINGPONG_2,
+		.lm_pair = LM_0,
 	},
 };
 

---
base-commit: a3d570eace66b4016f2692a6f1045742ee70c6b1
change-id: 20241216-dpu-fix-sm6150-17f0739f8fe0

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Re: [PATCH] drm/msm/dpu: correct LM pairing for SM6150

[Abhinav Kumar]

On 12/16/2024 12:20 AM, Dmitry Baryshkov wrote:
> According to the vendor devicetree on SM6150 LM_0 is paired with LM_2
> rather than LM_1. Correct pairing indices.
> 
> Fixes: cb2f9144693b ("drm/msm/dpu: Add SM6150 support")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
> index 621a2140f675fa28b3a7fcd8573e59b306cd6832..81eb274cc7000a3b70b0f6650088ddcd24648eab 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
> @@ -116,20 +116,20 @@ static const struct dpu_lm_cfg sm6150_lm[] = {
>   		.sblk = &sdm845_lm_sblk,
>   		.pingpong = PINGPONG_0,
>   		.dspp = DSPP_0,
> -		.lm_pair = LM_1,
> +		.lm_pair = LM_2,
>   	}, {
>   		.name = "lm_1", .id = LM_1,
>   		.base = 0x45000, .len = 0x320,
>   		.features = MIXER_QCM2290_MASK,
>   		.sblk = &sdm845_lm_sblk,
>   		.pingpong = PINGPONG_1,
> -		.lm_pair = LM_0,
>   	}, {
>   		.name = "lm_2", .id = LM_2,
>   		.base = 0x46000, .len = 0x320,
>   		.features = MIXER_QCM2290_MASK,
>   		.sblk = &sdm845_lm_sblk,
>   		.pingpong = PINGPONG_2,
> +		.lm_pair = LM_0,
>   	},
>   };

Have a basic question here. We check the peer only if we will have more 
than one LM needed in the topology but sm6150 does not have 3dmux, so 
the number of LMs will not go beyond one.

318 		/* Valid primary mixer found, find matching peers */
319 		if (lm_count < reqs->topology.num_lm) {

It seems like this fix will be unused or does not really matter.

Downstream has a different implementation for lm_pair, its used even to 
decide the LM pair for CWB mux. Upstream has a simpler implementation of 
just doing that in the code of using ODD LMs for ODD CWB muxes and even 
LMs for even CWB muxes. So fix is okay but not needed.

>   
> 
> ---
> base-commit: a3d570eace66b4016f2692a6f1045742ee70c6b1
> change-id: 20241216-dpu-fix-sm6150-17f0739f8fe0
> 
> Best regards,

Re: [PATCH] drm/msm/dpu: correct LM pairing for SM6150

[Dmitry Baryshkov]

On Mon, Dec 16, 2024 at 11:26:37AM -0800, Abhinav Kumar wrote:
> 
> 
> On 12/16/2024 12:20 AM, Dmitry Baryshkov wrote:
> > According to the vendor devicetree on SM6150 LM_0 is paired with LM_2
> > rather than LM_1. Correct pairing indices.
> > 
> > Fixes: cb2f9144693b ("drm/msm/dpu: Add SM6150 support")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
> > index 621a2140f675fa28b3a7fcd8573e59b306cd6832..81eb274cc7000a3b70b0f6650088ddcd24648eab 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
> > @@ -116,20 +116,20 @@ static const struct dpu_lm_cfg sm6150_lm[] = {
> >   		.sblk = &sdm845_lm_sblk,
> >   		.pingpong = PINGPONG_0,
> >   		.dspp = DSPP_0,
> > -		.lm_pair = LM_1,
> > +		.lm_pair = LM_2,
> >   	}, {
> >   		.name = "lm_1", .id = LM_1,
> >   		.base = 0x45000, .len = 0x320,
> >   		.features = MIXER_QCM2290_MASK,
> >   		.sblk = &sdm845_lm_sblk,
> >   		.pingpong = PINGPONG_1,
> > -		.lm_pair = LM_0,
> >   	}, {
> >   		.name = "lm_2", .id = LM_2,
> >   		.base = 0x46000, .len = 0x320,
> >   		.features = MIXER_QCM2290_MASK,
> >   		.sblk = &sdm845_lm_sblk,
> >   		.pingpong = PINGPONG_2,
> > +		.lm_pair = LM_0,
> >   	},
> >   };
> 
> Have a basic question here. We check the peer only if we will have more than
> one LM needed in the topology but sm6150 does not have 3dmux, so the number
> of LMs will not go beyond one.
> 
> 318 		/* Valid primary mixer found, find matching peers */
> 319 		if (lm_count < reqs->topology.num_lm) {
> 
> It seems like this fix will be unused or does not really matter.
> 
> Downstream has a different implementation for lm_pair, its used even to
> decide the LM pair for CWB mux. Upstream has a simpler implementation of
> just doing that in the code of using ODD LMs for ODD CWB muxes and even LMs
> for even CWB muxes. So fix is okay but not needed.

So which topology is supposed to work with LM_0 / LM_2 pair?

I'd still prefer to land the fix for the sake of catalog having the
correct data.

> 
> > 
> > ---
> > base-commit: a3d570eace66b4016f2692a6f1045742ee70c6b1
> > change-id: 20241216-dpu-fix-sm6150-17f0739f8fe0
> > 
> > Best regards,

-- 
With best wishes
Dmitry

Re: [PATCH] drm/msm/dpu: correct LM pairing for SM6150

[Abhinav Kumar]

On 12/16/2024 2:23 PM, Dmitry Baryshkov wrote:
> On Mon, Dec 16, 2024 at 11:26:37AM -0800, Abhinav Kumar wrote:
>>
>>
>> On 12/16/2024 12:20 AM, Dmitry Baryshkov wrote:
>>> According to the vendor devicetree on SM6150 LM_0 is paired with LM_2
>>> rather than LM_1. Correct pairing indices.
>>>
>>> Fixes: cb2f9144693b ("drm/msm/dpu: Add SM6150 support")
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
>>> index 621a2140f675fa28b3a7fcd8573e59b306cd6832..81eb274cc7000a3b70b0f6650088ddcd24648eab 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_3_sm6150.h
>>> @@ -116,20 +116,20 @@ static const struct dpu_lm_cfg sm6150_lm[] = {
>>>    		.sblk = &sdm845_lm_sblk,
>>>    		.pingpong = PINGPONG_0,
>>>    		.dspp = DSPP_0,
>>> -		.lm_pair = LM_1,
>>> +		.lm_pair = LM_2,
>>>    	}, {
>>>    		.name = "lm_1", .id = LM_1,
>>>    		.base = 0x45000, .len = 0x320,
>>>    		.features = MIXER_QCM2290_MASK,
>>>    		.sblk = &sdm845_lm_sblk,
>>>    		.pingpong = PINGPONG_1,
>>> -		.lm_pair = LM_0,
>>>    	}, {
>>>    		.name = "lm_2", .id = LM_2,
>>>    		.base = 0x46000, .len = 0x320,
>>>    		.features = MIXER_QCM2290_MASK,
>>>    		.sblk = &sdm845_lm_sblk,
>>>    		.pingpong = PINGPONG_2,
>>> +		.lm_pair = LM_0,
>>>    	},
>>>    };
>>
>> Have a basic question here. We check the peer only if we will have more than
>> one LM needed in the topology but sm6150 does not have 3dmux, so the number
>> of LMs will not go beyond one.
>>
>> 318 		/* Valid primary mixer found, find matching peers */
>> 319 		if (lm_count < reqs->topology.num_lm) {
>>
>> It seems like this fix will be unused or does not really matter.
>>
>> Downstream has a different implementation for lm_pair, its used even to
>> decide the LM pair for CWB mux. Upstream has a simpler implementation of
>> just doing that in the code of using ODD LMs for ODD CWB muxes and even LMs
>> for even CWB muxes. So fix is okay but not needed.
> 
> So which topology is supposed to work with LM_0 / LM_2 pair?
> 

Since there is no 3dmux, none of the LMs can really have a "pair" in 
this chipset.

This chipset has one DSI, one DP controller (which supports MST). So I 
think the only possibility is single LM case for DSI and single LM case 
for DP SST OR single LM case for DSI and 2 stream DP MST (which will use 
2 LMs - one for each stream) . But even that can only do low resolutions 
as even the MAX mdp clk is low on these.

> I'd still prefer to land the fix for the sake of catalog having the
> correct data.
> 

the lm_pair left over in the downstream DT is for CWB mux because even 
LMs can goto even CWBs. So only LM_0 and LM_2 can goto CWB_0 and are 
hence a pair for CWB_0. But this has no relevance to upstream code.

Dropping lm_pair would be more accurate for this chipset since it will 
always goto single lm case.

         else if (!dpu_kms->catalog->caps->has_3d_merge)
                 topology.num_lm = 1;

>>
>>>
>>> ---
>>> base-commit: a3d570eace66b4016f2692a6f1045742ee70c6b1
>>> change-id: 20241216-dpu-fix-sm6150-17f0739f8fe0
>>>
>>> Best regards,
>