Re: [PATCH] drm/i915/cdclk: Incorporate Xe3_LPD changes for CD2X divider

["Bhadane, Dnyaneshwar" <dnyaneshwar.bhadane@intel.com>]

On 23-Dec-25 3:43 AM, Gustavo Sousa wrote:
> On Xe3_LPD, there is no instruction to program the CD2X divider anymore
> and the hardware is expected to always use the default value of 0b00,
> meaning "divide by 1".
> 
> With that, the CDCLK_CTL register was changed so that:
> 
>    (1) The field "CD2X Divider Select" became a debug-only field.
>        Because we are programming CDCLK_CTL with a direct write instead
>        of read-modify-write operation, we still need to program "CD2X
>        Divider Select" in order to keep the field from deviating from its
>        default value.  Let's, however, throw a warning if we encounter a
>        CDCLK value that would result in an unexpected value for that
>        field.
> 
>    (2) The field "CD2X Pipe Select" has been removed. In fact, some
>        debugging in a PTL machine showed that such field comes back as
>        zero after writing a non-zero value to it.  As such, do not
>        program it starting with Xe3_LPD.
> 
> Bspec: 68864, 69090
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_cdclk.c | 27 ++++++++++++++++++++++-----
>   1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 0aa59d624095..684b8437951b 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1933,6 +1933,8 @@ static u32 bxt_cdclk_cd2x_pipe(struct intel_display *display, enum pipe pipe)
>   static u32 bxt_cdclk_cd2x_div_sel(struct intel_display *display,
>   				  int cdclk, int vco, u16 waveform)
>   {
> +	u32 ret;
> +
>   	/* cdclk = vco / 2 / div{1,1.5,2,4} */
>   	switch (cdclk_divider(cdclk, vco, waveform)) {
>   	default:
> @@ -1941,14 +1943,27 @@ static u32 bxt_cdclk_cd2x_div_sel(struct intel_display *display,
>   		drm_WARN_ON(display->drm, vco != 0);
>   		fallthrough;
>   	case 2:
> -		return BXT_CDCLK_CD2X_DIV_SEL_1;
> +		ret = BXT_CDCLK_CD2X_DIV_SEL_1;
> +		break;
>   	case 3:
> -		return BXT_CDCLK_CD2X_DIV_SEL_1_5;
> +		ret = BXT_CDCLK_CD2X_DIV_SEL_1_5;
> +		break;
>   	case 4:
> -		return BXT_CDCLK_CD2X_DIV_SEL_2;
> +		ret = BXT_CDCLK_CD2X_DIV_SEL_2;
> +		break;
>   	case 8:
> -		return BXT_CDCLK_CD2X_DIV_SEL_4;
> +		ret = BXT_CDCLK_CD2X_DIV_SEL_4;
> +		break;
>   	}
> +
> +	/*
> +	 * On Xe3_LPD onward, the expectation is to always have
> +	 * BXT_CDCLK_CD2X_DIV_SEL_1 as the default.
> +	 */
> +	if (DISPLAY_VER(display) >= 30)
> +		drm_WARN_ON(display->drm, ret != BXT_CDCLK_CD2X_DIV_SEL_1);
> +
> +	return ret;
>   }
>   
>   static u16 cdclk_squash_waveform(struct intel_display *display,
> @@ -2136,7 +2151,9 @@ static u32 bxt_cdclk_ctl(struct intel_display *display,
>   
>   	waveform = cdclk_squash_waveform(display, cdclk);
>   
> -	val = bxt_cdclk_cd2x_div_sel(display, cdclk, vco, waveform) |
> +	val = bxt_cdclk_cd2x_div_sel(display, cdclk, vco, waveform);
> +
> +	if (DISPLAY_VER(display) < 30)
>   		bxt_cdclk_cd2x_pipe(display, pipe);
Hey Gustavo,
Please update this line, function’s return value should be ORed with val.

i.e.	val |= bxt_cdclk_cd2x_pipe(display, pipe);

BR,
Dnyaneshwar
>   
>   	/*
> 
> ---
> base-commit: f2a0e58c77845e1a5cb6c549dc02b2670042e588
> change-id: 20251222-xe3_lpd-no-cd2x-divider-48f9f0972f98
> 
> Best regards,
> --
> Gustavo Sousa <gustavo.sousa@intel.com>
>