Re: [PATCH] drm/i915/power_well: Enable workaround for DSS clock gating issue (22021048059)

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 28 Jan 2026, Mika Kahola <mika.kahola@intel.com> 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 WA 22021048059 by disabling DSS clock gating for the affected pipes
> before turning off their power wells. This avoids DSC corruption on
> external displays.
>
> WA: 22021048059
> BSpec: 690991, 666241
>

Superfluous blank line. The git commit trailers belong together.

> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> ---
>  .../i915/display/intel_display_power_well.c   | 78 ++++++++++++++++++-
>  .../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, 86 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..1ef450f26879 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -14,10 +14,13 @@
>  #include "intel_crt.h"
>  #include "intel_de.h"
>  #include "intel_display_irq.h"
> +#include "intel_display_limits.h"
>  #include "intel_display_power_well.h"
>  #include "intel_display_regs.h"
>  #include "intel_display_rpm.h"
>  #include "intel_display_types.h"
> +#include "intel_display_utils.h"
> +#include "intel_display_wa.h"
>  #include "intel_dkl_phy.h"
>  #include "intel_dkl_phy_regs.h"
>  #include "intel_dmc.h"
> @@ -194,6 +197,69 @@ int intel_power_well_refcount(struct i915_power_well *power_well)
>  	return power_well->count;
>  }
>  
> +static void clock_gating_dss_enable_disable(struct intel_display *display,
> +					    u8 irq_pipe_mask,
> +					    bool disable)
> +{
> +	struct drm_printer p;
> +	enum pipe pipe;
> +
> +	switch (irq_pipe_mask) {
> +	case BIT(PIPE_A):
> +		pipe = PIPE_A;
> +
> +		if (disable)
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     0, DSS_PIPE_A_GATING_DISABLED);
> +		else
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     DSS_PIPE_A_GATING_DISABLED, 0);
> +		break;
> +	case BIT(PIPE_B):
> +		pipe = PIPE_B;
> +
> +		if (disable)
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     0, DSS_PIPE_B_GATING_DISABLED);
> +		else
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     DSS_PIPE_B_GATING_DISABLED, 0);
> +		break;
> +	case BIT(PIPE_C):
> +		pipe = PIPE_C;
> +
> +		if (disable)
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     0, DSS_PIPE_C_GATING_DISABLED);
> +		else
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     DSS_PIPE_C_GATING_DISABLED, 0);
> +		break;
> +	case BIT(PIPE_D):
> +		pipe = PIPE_D;
> +
> +		if (disable)
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     0, DSS_PIPE_D_GATING_DISABLED);
> +		else
> +			intel_de_rmw(display, CLKGATE_DIS_DSSDSC,
> +				     DSS_PIPE_D_GATING_DISABLED, 0);
> +		break;
> +	default:
> +		MISSING_CASE(irq_pipe_mask);
> +		break;
> +	}

irq_pipe_mask implies it can have multiple pipes set. That will lead to
a warning here. Does this need to use for_each_pipe_masked() instead?

The whole thing is awfully verbose as well. Perhaps figure out the bits
to set/unset based on the pipes, and have just one intel_de_rmw()
statement?

> +
> +	if (!drm_debug_enabled(DRM_UT_KMS))
> +		return;

This is redundant.

> +
> +	p = drm_dbg_printer(display->drm, DRM_UT_KMS, NULL);
> +
> +	drm_printf(&p, "dss clock gating %sd on pipe %c (0x%.8x)\n",
> +		   str_enable_disable(!disable), pipe_name(pipe),
> +		   intel_de_read(display, CLKGATE_DIS_DSSDSC));

Using a printer is overkill for one line. This should just be a
drm_dbg_kms().

And this also assumes just one pipe.

BR,
Jani.


> +}
> +
>  /*
>   * 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 +269,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))
> +			clock_gating_dss_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))
> +			clock_gating_dss_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;
>  	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);

-- 
Jani Nikula, Intel