On Wed, 12 Feb 2025 11:03:39 +0100 Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> wrote: > On 2025-02-11 19:42:24, James A. MacInnes wrote: > > SDM845 DPU hardware is rev 4.0.0 per hardware document. > > Just checking: version 4.0.0 is not named in the code that you're > changing: are you mentioning this because the patch you're fixing > here [1] says that widebus is "recommended" on 5.x.x which includes > sc7180, yet didn't account for that sc7180_dp_descs also being used > in the SDM845 compatible which is 4.0.0? That is something worth > mentioning in the patch description. > > [1]: > https://lore.kernel.org/linux-arm-msm/20240730195012.2595980-1-quic_abhinavk@xxxxxxxxxxx/ That is correct. All the 'modern' Qualcomm dpus got wide-bus turned on and as the SDM845 has not had a working Type-C port it was never tested. Happy to add that to my commit message. This is my second submission to the kernel forum. > > > > > Incorrect setting caused inop displayport. > > Inop doesn't seem to be a common abbreviation, there's enough space > to spell out "inoperative". And spend some more words on _why_ this > is an "incorrect setting" in the first place (based on the > suggestion above)? > Happy to spend many more words. Just looking to meet the requirements for the boards and try to not ruffle any feathers. It seems to easy to be far too verbose. > I am trying to remember the details from the original widebus series: > we discussed that the INTF_CFG2_DATABUS_WIDEN flag was available > starting with DPU 4.0.0 (IIRC, cannot find the source), yet the DSI > host only supports it from 6G v2.5 onwards (SC7280 and up?) [2]. > Seems a similar limitation applies to DP hosts. > > [2]: > https://lore.kernel.org/linux-arm-msm/20230822-add-widebus-support-v4-4-9dc86083d6ea@xxxxxxxxxxx/ > That would reflect the testing I have performed. With the wide_bus system enabled, The MIPI display functions fine, but the Altmode DisplayPort (type-c to DP) does not turn on a standard monitor and the type-c to HDMI connection has either a system that does not sync (horrific flashing) or just a single solid line. At other resolutions I was getting vblank errors from deeper into the system. > > Corrected by separating SDM845 to own descriptor. > > its own* > > > > > Fixes: c7c412202623 ("drm/msm/dp: enable widebus on all relevant > > chipsets") > > No need for empty lines between trailing tags. > > > Signed-off-by: James A. MacInnes <james.a.macinnes@xxxxxxxxx> > > --- > > drivers/gpu/drm/msm/dp/dp_display.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > b/drivers/gpu/drm/msm/dp/dp_display.c index > > aff51bb973eb..2cbdbf85a85c 100644 --- > > a/drivers/gpu/drm/msm/dp/dp_display.c +++ > > b/drivers/gpu/drm/msm/dp/dp_display.c @@ -126,6 +126,11 @@ static > > const struct msm_dp_desc msm_dp_desc_sa8775p[] = { {} > > }; > > > > +static const struct msm_dp_desc msm_dp_desc_sdm845[] = { > > + { .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, > > .wide_bus_supported = false }, > > We can probably drop the assignment, it'll be false/0 by default. > > - Marijn > Thank you, I will get that cleaned up. > > + {} > > +}; > > + > > static const struct msm_dp_desc msm_dp_desc_sc7180[] = { > > { .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, > > .wide_bus_supported = true }, {} > > @@ -178,7 +183,7 @@ static const struct of_device_id > > msm_dp_dt_match[] = { { .compatible = "qcom,sc8180x-edp", .data = > > &msm_dp_desc_sc8180x }, { .compatible = "qcom,sc8280xp-dp", .data = > > &msm_dp_desc_sc8280xp }, { .compatible = "qcom,sc8280xp-edp", .data > > = &msm_dp_desc_sc8280xp }, > > - { .compatible = "qcom,sdm845-dp", .data = > > &msm_dp_desc_sc7180 }, > > + { .compatible = "qcom,sdm845-dp", .data = > > &msm_dp_desc_sdm845 }, { .compatible = "qcom,sm8350-dp", .data = > > &msm_dp_desc_sc7180 }, { .compatible = "qcom,sm8650-dp", .data = > > &msm_dp_desc_sm8650 }, { .compatible = "qcom,x1e80100-dp", .data = > > &msm_dp_desc_x1e80100 }, -- > > 2.43.0 > >
