Re: [PATCH v4 12/12] drm/mediatek: mtk_dp: Clarify XTAL freq and Debounce registers

[CK Hu (胡俊光)]

On Thu, 2026-07-09 at 13:31 +0200, AngeloGioacchino Del Regno wrote:
> Clarify the meaning of the sub-fields in the crystal frequency and
> transmitter interrupt debounce registers, with the only purpose of
> increasing human readability.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/mediatek/mtk_dp.c     | 35 ++++++++++++++++-----------
>  drivers/gpu/drm/mediatek/mtk_dp_reg.h |  6 +++--
>  2 files changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index fe7e05e27f6f..1183738ab903 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1216,9 +1216,15 @@ static void mtk_dp_initialize_settings_v2p1(struct mtk_dp *mtk_dp)
>  
>  static void mtk_dp_initialize_settings(struct mtk_dp *mtk_dp)
>  {
> +	/*
> +	 * Set DP XTAL freq to 26MHz and accumulator to 1.
> +	 * Valid values for XTAL freq are 12 and 26.
> +	 */
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_342C,
> -			   XTAL_FREQ_DP_TRANS_P0_DEFAULT,
> -			   XTAL_FREQ_DP_TRANS_P0_MASK);
> +			   FIELD_PREP_CONST(XTAL_FREQ_DP_ACCUM_NUM_MASK, 1) |
> +			   FIELD_PREP_CONST(XTAL_FREQ_DP_CLOCK_MHZ_MASK, 26),
> +			   XTAL_FREQ_DP_ACCUM_NUM_MASK | XTAL_FREQ_DP_CLOCK_MHZ_MASK);
> +
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3540,
>  			   FEC_CLOCK_EN_MODE_DP_TRANS_P0,
>  			   FEC_CLOCK_EN_MODE_DP_TRANS_P0);
> @@ -1260,28 +1266,29 @@ static void mtk_dp_initialize_aux_hpd_detect_settings(struct mtk_dp *mtk_dp)
>  
>  static void mtk_dp_initialize_hpd_detect_settings(struct mtk_dp *mtk_dp)
>  {
> -	u32 val;
> -
>  	/* Mask AUX TOP interrupt, as this uses transmitter for HPD */
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_IRQ_MASK,
>  			   AUX_TOP_IRQ_MSK, AUX_TOP_IRQ_MSK);
>  
> -	/* Debounce threshold */
> +	/* Set Hotplug debounce threshold time: xtal_period * deb_thd */
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3410,
>  			   8, HPD_DEB_THD_DP_TRANS_P0_MASK);
>  
> -	val = (HPD_INT_THD_DP_TRANS_P0_LOWER_500US |
> -	       HPD_INT_THD_DP_TRANS_P0_UPPER_1100US) << 4;

It seems the original code has bug here.
Fix the bug first (it could be backport), and then clarify register.

> -	mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3410,
> -			   val, HPD_INT_THD_DP_TRANS_P0_MASK);
> -
>  	/*
> -	 * Connect threshold 1.5ms + 5 x 0.1ms = 2ms
> -	 * Disconnect threshold 1.5ms + 5 x 0.1ms = 2ms
> +	 * Set interrupt debounce threshold time
> +	 * VAL  |  0     1     2     3  INT_THD
> +	 * Low  | 100   300   500   700 uS
> +	 * High | 700   900  1100  1300 uS
>  	 */
> -	val = (5 << 8) | (5 << 12);
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3410,
> -			   val,
> +			   FIELD_PREP_CONST(HPD_INT_LOW_THD_DP_TRANS_P0_MASK, 2) |
> +			   FIELD_PREP_CONST(HPD_INT_HIGH_THD_DP_TRANS_P0_MASK, 2),
> +			   HPD_INT_THD_DP_TRANS_P0_MASK);
> +
> +	/* Connection and Disconnection thresholds: 1.5ms + (5 x 0.1) ms = 2ms */
> +	mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3410,
> +			   FIELD_PREP_CONST(HPD_DISC_THD_DP_TRANS_P0_MASK, 5) |
> +			   FIELD_PREP_CONST(HPD_CONN_THD_DP_TRANS_P0_MASK, 5),
>  			   HPD_DISC_THD_DP_TRANS_P0_MASK |
>  			   HPD_CONN_THD_DP_TRANS_P0_MASK);
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3430,
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> index 9caf743f4fe6..ce47b99dddc4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> @@ -253,6 +253,8 @@
>  #define MTK_DP_TRANS_P0_3410				0x1410
>  #define HPD_DEB_THD_DP_TRANS_P0_MASK				GENMASK(3, 0)
>  #define HPD_INT_THD_DP_TRANS_P0_MASK				GENMASK(7, 4)
> +#define HPD_INT_HIGH_THD_DP_TRANS_P0_MASK			GENMASK(7, 6)
> +#define HPD_INT_LOW_THD_DP_TRANS_P0_MASK			GENMASK(5, 4)
>  #define HPD_INT_THD_DP_TRANS_P0_LOWER_500US			(2 << 4)
>  #define HPD_INT_THD_DP_TRANS_P0_UPPER_1100US			(2 << 6)

Useless, remove it.

Regards,
CK

>  #define HPD_DISC_THD_DP_TRANS_P0_MASK				GENMASK(11, 8)
> @@ -267,8 +269,8 @@
>  #define IRQ_MASK_DP_TRANS_P0_INT_IRQ				(BIT(3) << 4)
>  #define IRQ_STATUS_DP_TRANS_P0_MASK				GENMASK(15, 12)
>  #define MTK_DP_TRANS_P0_342C				0x142c
> -#define XTAL_FREQ_DP_TRANS_P0_DEFAULT				(BIT(0) | BIT(3) | BIT(5) | BIT(6))
> -#define XTAL_FREQ_DP_TRANS_P0_MASK				GENMASK(7, 0)
> +#define XTAL_FREQ_DP_ACCUM_NUM_MASK				GENMASK(1, 0)
> +#define XTAL_FREQ_DP_CLOCK_MHZ_MASK				GENMASK(7, 2)
>  #define MTK_DP_TRANS_P0_3430				0x1430
>  #define HPD_INT_THD_ECO_DP_TRANS_P0_MASK			GENMASK(1, 0)
>  #define HPD_INT_THD_ECO_DP_TRANS_P0_HIGH_BOUND_EXT		BIT(1)