On Fri, Jun 05, 2026 at 09:23:59PM +0530, Dibin Moolakadan Subrahmanian wrote: > The DC_STATE_EN register has read-only status bits that are set by > hardware on some platforms. These bits may cause the read-back > verification loop in gen9_write_dc_state() to spuriously retry. > > Mask the RO bits from the read-back comparison to prevent > unnecessary retries. > > Changes in v2: > - Rename patch from > "drm/i915/display: Use rmw in gen9_write_dc_state() to preserve non-DC > bits" > to > "drm/i915/display: Mask RO bits in gen9_write_dc_state()" > - Mask only RO bits rather than masking all non DC state bits > in DC_STATE_EN. As the register has also some clear-on-write flags, > like 'Display DC*CO State Status DSI'(Imre Deak) > > Changes in v3: > - Limit ro mask to read-back comparison. > > Changes in v4: > - Add bit definitions (Jani Nikula) > > BSpec: 49437,69115 > Signed-off-by: Dibin Moolakadan Subrahmanian > <[email protected]> > --- > .../i915/display/intel_display_power_well.c | 24 +++++++++++++++---- > .../gpu/drm/i915/display/intel_display_regs.h | 4 ++++ > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c > b/drivers/gpu/drm/i915/display/intel_display_power_well.c > index 04bd0dde5bed..bb0272f59d97 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c > @@ -726,12 +726,28 @@ static void assert_can_disable_dc9(struct intel_display > *display) > */ > } > > +static u32 dc_state_ro_mask(struct intel_display *display) > +{ > + if (DISPLAY_VER(display) >= 20) > + return DC_STATE_EN_CSR_MASK_CMTG_1 | > DC_STATE_EN_CSR_MASK_CMTG_0; > + else if (DISPLAY_VER(display) >= 13 && !display->platform.dg2) > + return DC_STATE_EN_CSR_MASK_CMTG_0; > + > + return 0; > +} > + > static void gen9_write_dc_state(struct intel_display *display, > u32 state) > { > int rewrites = 0; > int rereads = 0; > u32 v; > + /* > + * Mask out RO status bits from read-back comparison. > + * HW may set these bits independently, so exclude them > + * to prevent the verify loop from retrying due to RO bits mismatch. > + */ > + u32 ro_mask = dc_state_ro_mask(display); > > intel_de_write(display, DC_STATE_EN, state); > > @@ -743,7 +759,7 @@ static void gen9_write_dc_state(struct intel_display > *display, > do { > v = intel_de_read(display, DC_STATE_EN); > > - if (v != state) { > + if ((v & ~ro_mask) != (state & ~ro_mask)) { > intel_de_write(display, DC_STATE_EN, state); > rewrites++; > rereads = 0; > @@ -753,10 +769,10 @@ static void gen9_write_dc_state(struct intel_display > *display, > > } while (rewrites < 100); > > - if (v != state) > + if ((v & ~ro_mask) != (state & ~ro_mask)) > drm_err(display->drm, > - "Writing dc state to 0x%x failed, now 0x%x\n", > - state, v); > + "Writing dc state to 0x%x failed, now 0x%x > (ro_mask=0x%x)\n", > + state, v, ro_mask); > > /* Most of the times we need one retry, avoid spam */ > if (rewrites > 1) > diff --git a/drivers/gpu/drm/i915/display/intel_display_regs.h > b/drivers/gpu/drm/i915/display/intel_display_regs.h > index 4321f8b529da..061bff0ee911 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_display_regs.h > @@ -3078,6 +3078,10 @@ enum skl_power_gate { > #define DC_STATE_EN_DC9 (1 << 3) > #define DC_STATE_EN_UPTO_DC6 (2 << 0) > #define DC_STATE_EN_UPTO_DC5_DC6_MASK 0x3 > +/* display version 20+ */ > +#define DC_STATE_EN_CSR_MASK_CMTG_1 REG_BIT(11) > +/* display version 13+, except dg2 */ > +#define DC_STATE_EN_CSR_MASK_CMTG_0 REG_BIT(10)
The register spec for pre-LNL platforms (bspec/49437) doesn't mention CMTG, but I guess we can assume the flag is used the same way there as on newer platforms. Patch looks ok to me: Reviewed-by: Imre Deak <[email protected]> > > #define DC_STATE_DEBUG _MMIO(0x45520) > #define DC_STATE_DEBUG_MASK_CORES (1 << 0) > -- > 2.43.0 >
