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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8c5aaa04-3209-40f2-9ba4-a1d6ae00fbf7@quicinc.com>
Date: Mon, 16 Dec 2024 15:55:24 -0800
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        "Marijn
 Suijten" <marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Li Liu <quic_lliu6@...cinc.com>,
        Fange Zhang
	<quic_fangez@...cinc.com>,
        <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/msm/dpu: correct LM pairing for SM6150



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@...aro.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,
>