> Subject: Re: [PATCH v2] drm/i915/power_well: Enable workaround for DSS clock > gating issue > > On Fri, Jan 30, 2026 at 01:45:34PM +0000, Mika Kahola wrote: > > Prevent display corruption observed after restart, hotplug, or unplug > > operations on Meteor Lake and newer platforms. The issue is caused by > > DSS clock gating affecting DSC logic when pipe power wells are disabled. > > > > Apply this WA by disabling DSS clock gating for the affected pipes > > before turning off their power wells. This avoids DSC corruption on > > external displays. > > > > v2: Use single intel_de_rmw() (Jani) > > Switch to use drm_dbg_kms() instead of drm_printf() (Jani) > > Remove WA number from commit message (Suraj) > > rename dss_clock_gating_enable_disable() to > > dss_pipe_gating_enable_disable(); > > > > WA: 22021048059 > > BSpec: 690991, 666241 > > Signed-off-by: Mika Kahola <[email protected]> > > --- > > .../i915/display/intel_display_power_well.c | 55 ++++++++++++++++++- > > .../gpu/drm/i915/display/intel_display_regs.h | 7 +++ > > .../gpu/drm/i915/display/intel_display_wa.c | 2 + > > .../gpu/drm/i915/display/intel_display_wa.h | 1 + > > 4 files changed, 63 insertions(+), 2 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 6f9bc6f9615e..78f707b00550 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c > > @@ -18,6 +18,7 @@ > > #include "intel_display_regs.h" > > #include "intel_display_rpm.h" > > #include "intel_display_types.h" > > +#include "intel_display_wa.h" > > #include "intel_dkl_phy.h" > > #include "intel_dkl_phy_regs.h" > > #include "intel_dmc.h" > > @@ -194,6 +195,48 @@ int intel_power_well_refcount(struct > i915_power_well *power_well) > > return power_well->count; > > } > > > > +static u32 dss_pipe_gating_bits(u8 irq_pipe_mask) { > > + u32 bits = 0; > > + > > + if (irq_pipe_mask & BIT(PIPE_A)) > > + bits |= DSS_PIPE_A_GATING_DISABLED; > > + if (irq_pipe_mask & BIT(PIPE_B)) > > + bits |= DSS_PIPE_B_GATING_DISABLED; > > + if (irq_pipe_mask & BIT(PIPE_C)) > > + bits |= DSS_PIPE_C_GATING_DISABLED; > > + if (irq_pipe_mask & BIT(PIPE_D)) > > + bits |= DSS_PIPE_D_GATING_DISABLED; > > + > > + return bits; > > +} > > + > > +static void dss_pipe_gating_enable_disable(struct intel_display *display, > > + u8 irq_pipe_mask, > > + bool disable) > > +{ > > + u32 bits = dss_pipe_gating_bits(irq_pipe_mask); > > + u32 clear, set; > > + > > + if (!bits) > > + return; > > + > > + /* > > + * Single intel_de_rmw() for both enable/disable: > > + * - disable == true, set bits (disable clock gating) > > + * - disable == false, clear bits (re-enable clock gating) > > + */ > > + set = disable ? bits : 0; > > + clear = disable ? 0 : bits; > > + > > + intel_de_rmw(display, CLKGATE_DIS_DSSDSC, clear, set); > > + > > + drm_dbg_kms(display->drm, > > + "DSS clock gating %sd for pipe_mask=0x%x > (CLKGATE_DIS_DSSDSC=0x%08x)\n", > > + str_enable_disable(!disable), irq_pipe_mask, > > + intel_de_read(display, CLKGATE_DIS_DSSDSC)); } > > + > > /* > > * Starting with Haswell, we have a "Power Down Well" that can be turned > > off > > * when not needed anymore. We have 4 registers that can request the > > power well @@ -203,15 +246,23 @@ int intel_power_well_refcount(struct > > i915_power_well *power_well) static void > hsw_power_well_post_enable(struct intel_display *display, > > u8 irq_pipe_mask) > > { > > - if (irq_pipe_mask) > > + if (irq_pipe_mask) { > > gen8_irq_power_well_post_enable(display, irq_pipe_mask); > > + > > + if (intel_display_wa(display, 22021048059)) > > + dss_pipe_gating_enable_disable(display, > irq_pipe_mask, false); > > + } > > } > > > > static void hsw_power_well_pre_disable(struct intel_display *display, > > u8 irq_pipe_mask) > > { > > - if (irq_pipe_mask) > > + if (irq_pipe_mask) { > > + if (intel_display_wa(display, 22021048059)) > > + dss_pipe_gating_enable_disable(display, > irq_pipe_mask, true); > > + > > gen8_irq_power_well_pre_disable(display, irq_pipe_mask); > > + } > > } > > > > #define ICL_AUX_PW_TO_PHY(pw_idx) \ > > diff --git a/drivers/gpu/drm/i915/display/intel_display_regs.h > > b/drivers/gpu/drm/i915/display/intel_display_regs.h > > index 9e0d853f4b61..9740f32ced24 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_regs.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_regs.h > > @@ -2211,6 +2211,13 @@ > > #define HSW_PWR_WELL_FORCE_ON (1 << 19) > > #define HSW_PWR_WELL_CTL6 _MMIO(0x45414) > > > > +/* clock gating DSS DSC disable register */ > > +#define CLKGATE_DIS_DSSDSC _MMIO(0x46548) > > +#define DSS_PIPE_D_GATING_DISABLED REG_BIT(31) > > +#define DSS_PIPE_C_GATING_DISABLED REG_BIT(29) > > +#define DSS_PIPE_B_GATING_DISABLED REG_BIT(27) > > +#define DSS_PIPE_A_GATING_DISABLED REG_BIT(25) > > + > > /* SKL Fuse Status */ > > enum skl_power_gate { > > SKL_PG0, > > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c > > b/drivers/gpu/drm/i915/display/intel_display_wa.c > > index 86a6cc45b6ab..f8e14aa34dae 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_wa.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c > > @@ -84,6 +84,8 @@ bool __intel_display_wa(struct intel_display *display, > enum intel_display_wa wa, > > return intel_display_needs_wa_16025573575(display); > > case INTEL_DISPLAY_WA_22014263786: > > return IS_DISPLAY_VERx100(display, 1100, 1400); > > + case INTEL_DISPLAY_WA_22021048059: > > + return DISPLAY_VER(display) >= 14; > > We generally don't want to use open-ended ranges like this; it might apply to > everything above version 14 that exists today, but it should only continue on > to > future platforms if the hardware teams explicitly add additional tickets > indicating that it's still needed on platforms > n+1, n+2, etc. Once a hardware issue is identified, the hardware teams > will work on a true fix that will intercept on some future platform, > eliminating > the need for a software workaround; if we use an unbounded range like this in > the code, we'll never notice and keep applying the workaround even when it > isn't needed anymore. >
I think with the above comment addressed I don't see any other issue with it LGTM, Reviewed-by: Suraj Kandpal <[email protected]> > > Matt > > > default: > > drm_WARN(display->drm, 1, "Missing Wa number: %s\n", > name); > > break; > > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h > > b/drivers/gpu/drm/i915/display/intel_display_wa.h > > index 40f989f19df1..767420d5f406 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h > > @@ -34,6 +34,7 @@ enum intel_display_wa { > > INTEL_DISPLAY_WA_16023588340, > > INTEL_DISPLAY_WA_16025573575, > > INTEL_DISPLAY_WA_22014263786, > > + INTEL_DISPLAY_WA_22021048059, > > }; > > > > bool __intel_display_wa(struct intel_display *display, enum > > intel_display_wa wa, const char *name); > > -- > > 2.43.0 > > > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
