Re: [PATCH v3 5/8] drm/i915/display: move GLK clock gating init to display

Luca Coelho
2026-04-20T20:08:34Z

On Mon, 2026-04-20 at 15:18 +0300, Jani Nikula wrote:
> On Mon, 20 Apr 2026, Luca Coelho <luciano.coelho@intel.com> wrote:
> > Move the GLK-specific display clock gating programming into display
> > intel_display_clock_gating.c, to remove more dependencies from i915 to
> > display registers.
> >
> > Now that all remaining Gen9-family callers moved into display, we can
> > move the shared Gen9 display clock gating helper into display and
> > remove the old local helper from intel_clock_gating.c.
> >
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > ---
> > .../i915/display/intel_display_clock_gating.c | 57 +++++++++++++++++++
> > .../i915/display/intel_display_clock_gating.h | 1 +
> > drivers/gpu/drm/i915/intel_clock_gating.c | 44 +-------------
> > 3 files changed, 59 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_clock_gating.c b/drivers/gpu/drm/i915/display/intel_display_clock_gating.c
> > index 59041c807d6d..b2cb18478577 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_clock_gating.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_clock_gating.c
> > @@ -6,11 +6,39 @@
> > #include <drm/intel/intel_gmd_misc_regs.h>
> >
> > #include "intel_de.h"
> > +#include "intel_display.h"
> > #include "intel_display_clock_gating.h"
> > +#include "intel_display_core.h"
> > #include "intel_display_regs.h"
> >
> > +static void intel_display_gen9_init_clock_gating(struct intel_display *display)
> > +{
> > + /* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */
> > + intel_de_rmw(display, CHICKEN_PAR1_1, 0, SKL_EDP_PSR_FIX_RDWRAP);
> > +
> > + /* WaEnableChickenDCPR:skl,bxt,kbl,glk,cfl */
> > + intel_de_rmw(display, GEN8_CHICKEN_DCPR_1, 0, MASK_WAKEMEM);
> > +
> > + /*
> > + * WaFbcWakeMemOn:skl,bxt,kbl,glk,cfl
> > + * Display WA #0859: skl,bxt,kbl,glk,cfl
> > + */
> > + intel_de_rmw(display, DISP_ARB_CTL, 0, DISP_FBC_MEMORY_WAKE);
> > +}
> > +
> > void intel_display_skl_init_clock_gating(struct intel_display *display)
> > {
> > + /*
> > + * WaCompressedResourceDisplayNewHashMode:skl,kbl
> > + * Display WA #0390: skl,kbl
> > + *
> > + * Must match Sampler, Pixel Back End, and Media. See
> > + * WaCompressedResourceSamplerPbeMediaNewHashMode.
> > + */
> > + intel_de_rmw(display, CHICKEN_PAR1_1, 0, SKL_DE_COMPRESSED_HASH_MODE);
> > +
> > + intel_display_gen9_init_clock_gating(display);
> > +
> > /*
> > * WaFbcTurnOffFbcWatermark:skl
> > * Display WA #0562: skl
> > @@ -20,6 +48,17 @@ void intel_display_skl_init_clock_gating(struct intel_display *display)
> >
> > void intel_display_kbl_init_clock_gating(struct intel_display *display)
> > {
> > + /*
> > + * WaCompressedResourceDisplayNewHashMode:skl,kbl
> > + * Display WA #0390: skl,kbl
> > + *
> > + * Must match Sampler, Pixel Back End, and Media. See
> > + * WaCompressedResourceSamplerPbeMediaNewHashMode.
> > + */
> > + intel_de_rmw(display, CHICKEN_PAR1_1, 0, SKL_DE_COMPRESSED_HASH_MODE);
> > +
> > + intel_display_gen9_init_clock_gating(display);
> > +
> > /*
> > * WaFbcTurnOffFbcWatermark:kbl
> > * Display WA #0562: kbl
> > @@ -29,6 +68,8 @@ void intel_display_kbl_init_clock_gating(struct intel_display *display)
> >
> > void intel_display_cfl_init_clock_gating(struct intel_display *display)
> > {
> > + intel_display_gen9_init_clock_gating(display);
> > +
> > /*
> > * WaFbcTurnOffFbcWatermark:cfl
> > * Display WA #0562: cfl
> > @@ -38,6 +79,8 @@ void intel_display_cfl_init_clock_gating(struct intel_display *display)
> >
> > void intel_display_bxt_init_clock_gating(struct intel_display *display)
> > {
> > + intel_display_gen9_init_clock_gating(display);
> > +
> > /*
> > * Wa: Backlight PWM may stop in the asserted state, causing backlight
> > * to stay fully on.
> > @@ -60,3 +103,17 @@ void intel_display_bxt_init_clock_gating(struct intel_display *display)
> > */
> > intel_de_rmw(display, DISP_ARB_CTL, 0, DISP_FBC_WM_DIS);
> > }
> > +
> > +void intel_display_glk_init_clock_gating(struct intel_display *display)
> > +{
> > + intel_display_gen9_init_clock_gating(display);
> > +
> > + /*
> > + * WaDisablePWMClockGating:glk
> > + * Backlight PWM may stop in the asserted state, causing backlight
> > + * to stay fully on.
> > + */
> > + intel_de_write(display, GEN9_CLKGATE_DIS_0,
> > + intel_de_read(display, GEN9_CLKGATE_DIS_0) |
> > + PWM1_GATING_DIS | PWM2_GATING_DIS);
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_clock_gating.h b/drivers/gpu/drm/i915/display/intel_display_clock_gating.h
> > index 6bc84a9a4342..a7784db9d97a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_clock_gating.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_clock_gating.h
> > @@ -12,5 +12,6 @@ void intel_display_skl_init_clock_gating(struct intel_display *display);
> > void intel_display_kbl_init_clock_gating(struct intel_display *display);
> > void intel_display_cfl_init_clock_gating(struct intel_display *display);
> > void intel_display_bxt_init_clock_gating(struct intel_display *display);
> > +void intel_display_glk_init_clock_gating(struct intel_display *display);
> >
> > #endif /* __INTEL_DISPLAY_CLOCK_GATING_H__ */
> > diff --git a/drivers/gpu/drm/i915/intel_clock_gating.c b/drivers/gpu/drm/i915/intel_clock_gating.c
> > index 4c1937d922b2..777314e0c75d 100644
> > --- a/drivers/gpu/drm/i915/intel_clock_gating.c
> > +++ b/drivers/gpu/drm/i915/intel_clock_gating.c
> > @@ -49,36 +49,8 @@ struct drm_i915_clock_gating_funcs {
> > void (*init_clock_gating)(struct drm_i915_private *i915);
> > };
> >
> > -static void gen9_init_clock_gating(struct drm_i915_private *i915)
> > -{
> > - if (HAS_LLC(i915)) {
>
> The commit message should explain why removing this is okay.

Good point. This check was incidental because it just happened that
the only Gen9 platforms that need this workaround (namely SKL and KBL)
have LLC. Now that the code is moved, the check is not needed and we
apply the workaround in the SKL/KBL-specific functions instead of doing
it in the Gen9 function protected by HAS_LLC().

--
Cheers,
Luca.