On 2025-02-12 15:03:46, James A. MacInnes wrote:
> SDM845 DPU hardware is rev 4.0.0 per hardware documents.
> Original patch to enable wide_bus operation did not take into account
> the SDM845 and it got carried over by accident.
>
> - Incorrect setting caused inoperable DisplayPort.
> - Corrected by separating SDM845 into its own descriptor.
If anything I'd have appreciated to see our conversation in v1 pasted here
verbatim which is of the right verbosity to explain this. I can't do much with
a list of two items.
I don't have a clearer way of explaining what all I find confusing about this
description, so let me propose what I would have written if this was my patch
instead:
When widebus was enabled for DisplayPort in commit c7c412202623 ("drm/msm/dp:
enable widebus on all relevant chipsets") it was clarified that it is only
supported on DPU 5.0.0 onwards which includes SC7180 on DPU revision 6.2.
However, this patch missed that the description structure for SC7180 is also
reused for SDM845 (because of identical io_start address) which is only DPU
4.0.0, leading to a wrongly enbled widebus feature and corruption on that
platform.
Create a separate msm_dp_desc_sdm845 structure for this SoC compatible,
with the wide_bus_supported flag turned off.
Note that no other DisplayPort compatibles currently exist for SoCs older
than DPU 4.0.0 besides SDM845.
Hope I'm not considered being too picky. I first sketch **how** the original
patch created a problem, then explain how this patch is intending to fix it,
and finally describe that we went a step further and ensured no other SoCs
are suffering from a similar problem.
- Marijn
>
> Fixes: c7c412202623 ("drm/msm/dp: enable widebus on all relevant chipsets")
> 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..e30cccd63910 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 },
> + {}
> +};
> +
> 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
>
