linux-kernel - Re: [PATCH 2/2] drm/msm/disp: Correct porch timing for SDM845

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <v4lpt45c7miwt45ld4sfinixnpje6tb73dhqmahl6kin4i7wyj@6ss563kvk63v>
Date: Wed, 12 Feb 2025 11:13:24 +0100
From: Marijn Suijten <marijn.suijten@...ainline.org>
To: "James A. MacInnes" <james.a.macinnes@...il.com>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	robdclark@...il.com, quic_abhinavk@...cinc.com, dmitry.baryshkov@...aro.org, 
	sean@...rly.run, airlied@...il.com, simona@...ll.ch
Subject: Re: [PATCH 2/2] drm/msm/disp: Correct porch timing for SDM845

On 2025-02-11 19:42:25, James A. MacInnes wrote:
> Type-C DisplayPort inop due to incorrect settings.
> 
> SDM845 (DPU 4.0) lacks wide_bus support; porch shift removed.

Same comment on "inop", elaborating the meaning of "incorrect settings" and
describing relevance to DPU 4.0 from patch 1/2.

> 
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")

This commit came long before wide bus support, are you sure this is the right
Fixes tag?

> 

Drop empty line between tags.

> Signed-off-by: James A. MacInnes <james.a.macinnes@...il.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index abd6600046cb..3e0fef0955ce 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -94,17 +94,17 @@ static void drm_mode_to_intf_timing_params(
>  		timing->vsync_polarity = 0;
>  	}
>  
> +	timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent);
> +	timing->compression_en = dpu_encoder_is_dsc_enabled(phys_enc->parent);
> +
>  	/* for DP/EDP, Shift timings to align it to bottom right */
> -	if (phys_enc->hw_intf->cap->type == INTF_DP) {
> +	if (phys_enc->hw_intf->cap->type == INTF_DP && timing->wide_bus_en) {

This code existed long before widebus: are you sure this is correct?

Note that an identical `if` condtion exists right below, under the "for DP,
divide the horizonal parameters by 2 when widebus is enabled" comment.  If
this "Shift timings to align it to bottom right" should really only happen when
widebus is enabled, move the code into that instead.

- Marijn

>  		timing->h_back_porch += timing->h_front_porch;
>  		timing->h_front_porch = 0;
>  		timing->v_back_porch += timing->v_front_porch;
>  		timing->v_front_porch = 0;
>  	}
>  
> -	timing->wide_bus_en = dpu_encoder_is_widebus_enabled(phys_enc->parent);
> -	timing->compression_en = dpu_encoder_is_dsc_enabled(phys_enc->parent);
> -
>  	/*
>  	 * for DP, divide the horizonal parameters by 2 when
>  	 * widebus is enabled
> -- 
> 2.43.0
>