| Message ID | 20241216-dpu-fix-sm6150-v1-1-9fd7ce2ff606@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm/dpu: correct LM pairing for SM6150 | expand |
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,
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,
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, >
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, }, };
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(-) --- base-commit: a3d570eace66b4016f2692a6f1045742ee70c6b1 change-id: 20241216-dpu-fix-sm6150-17f0739f8fe0 Best regards,