NACK[L/K/J]/Cmnt: [PATCH 2/2][SRU][U/L/K/J] drm/i915: Explain the magic numbers for AUX SYNC/precharge length

[Stefan Bader]

On 06.06.23 10:10, Koba Ko wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/2023004
> 
> Replace the hardcoded final numbers in the AUX SYNC/precharge
> setup, and derive those from numbers from the (e)DP specs.
> 
> The new functions can serve as the single point of truth for
> the number of SYNC pulses we use.
> 
> Cc: Jouni Högander <jouni.hogander at intel.com>
> Change-Id: I6393cb8875c3dc8ce4f505fdb468acfa49b7fd4d
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20230329172434.18744-2-ville.syrjala@linux.intel.com
> Reviewed-by: Jouni Högander <jouni.hogander at intel.com>
> (cherry picked from commit 26bfc3f36f2104c174dfc72415547d5c28ef3f1c)
> Signed-off-by: Koba Ko <koba.ko at canonical.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dp_aux.c | 32 +++++++++++++++++++--
>   1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> index 2bc1193745557..faec2b8be5f46 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c
> @@ -119,6 +119,32 @@ static u32 skl_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
>   	return index ? 0 : 1;
>   }
>   
> +static int intel_dp_aux_sync_len(void)
> +{
> +	int precharge = 16; /* 10-16 */
> +	int preamble = 16;
> +
> +	return precharge + preamble;
> +}
> +
> +static int intel_dp_aux_fw_sync_len(void)
> +{
> +	int precharge = 16; /* 10-16 */
> +	int preamble = 8;
> +
> +	return precharge + preamble;
> +}
> +
> +static int g4x_dp_aux_precharge_len(void)
> +{
> +	int precharge_min = 10;
> +	int preamble = 16;
> +
> +	/* HW wants the length of the extra precharge in 2us units */
> +	return (intel_dp_aux_sync_len() -
> +		precharge_min - preamble) / 2;
> +}
> +
>   static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>   				int send_bytes,
>   				u32 aux_clock_divider)
> @@ -141,7 +167,7 @@ static u32 g4x_get_aux_send_ctl(struct intel_dp *intel_dp,
>   	       timeout |
>   	       DP_AUX_CH_CTL_RECEIVE_ERROR |
>   	       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> -	       (3 << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> +	       (g4x_dp_aux_precharge_len() << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>   	       (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT);
>   }
>   
> @@ -164,8 +190,8 @@ static u32 skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>   	      DP_AUX_CH_CTL_TIME_OUT_MAX |
>   	      DP_AUX_CH_CTL_RECEIVE_ERROR |
>   	      (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> -	      DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
> -	      DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> +	      DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(intel_dp_aux_fw_sync_len()) |
> +	      DP_AUX_CH_CTL_SYNC_PULSE_SKL(intel_dp_aux_sync_len());
>   
>   	if (intel_tc_port_in_tbt_alt_mode(dig_port))
>   		ret |= DP_AUX_CH_CTL_TBT_IO;

Rejected for the following reasons:
* Lunar already has this via https://bugs.launchpad.net/bugs/2018655
* Kinetic and Jammy  are missing the change this set is supposed to fix

-Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 44613 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230615/50222d64/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230615/50222d64/attachment-0001.sig>