Quoting Bhadane, Dnyaneshwar (2025-12-31 08:54:21-03:00) > > >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 <[email protected]> >> --- >> 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);
Oops... Thanks for catching that! -- Gustavo Sousa > >BR, >Dnyaneshwar >> >> /* >> >> --- >> base-commit: f2a0e58c77845e1a5cb6c549dc02b2670042e588 >> change-id: 20251222-xe3_lpd-no-cd2x-divider-48f9f0972f98 >> >> Best regards, >> -- >> Gustavo Sousa <[email protected]> >> >
