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/ > > 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)? 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/ > 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 > + {} > +}; > + > 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 >