Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Luca Coelho <luciano.coelho@intel.com>, intel-gfx@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Subject: Re: [PATCH v3 2/5] drm/i915/display: consolidate remaining clock gating initialization
Date: Thu, 27 Aug 2026 10:27:51 +0300	[thread overview]
Message-ID: <1a4317911980cdd6017b906337f16df11ce35fe9@intel.com> (raw)
In-Reply-To: <20260817092911.342072-3-luciano.coelho@intel.com>

On Mon, 17 Aug 2026, Luca Coelho <luciano.coelho@intel.com> wrote:
> The remaining platform-specific display clock gating functions are
> still called directly by i915.
>
> Extend intel_display_init_clock_gating() so that it calls the correct
> functions for the remaining platforms.
>
> This groups Broadwell's three display operations into a single
> function.  This changes the call order slighlty in relation to

*slightly

> non-display workarounds, but it shouldn't be a problem.  For IVB, we
> move the trickle-feed programming to the display helper and runs with
> the other display register setup.

In general, I'd prefer not mixing the refactoring and the functional
changes, e.g. the reordering could've come first as a tiny functional
change, and the rest could be just movement.

But it's benign.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
>  .../i915/display/intel_display_clock_gating.c | 43 ++++++++++++++-----
>  .../i915/display/intel_display_clock_gating.h | 10 -----
>  drivers/gpu/drm/i915/intel_clock_gating.c     | 20 +++------
>  3 files changed, 40 insertions(+), 33 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 d5ecdaeab421..ef1ee72494df 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_clock_gating.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_clock_gating.c
> @@ -134,13 +134,13 @@ static void intel_display_glk_init_clock_gating(struct intel_display *display)
>  		       PWM1_GATING_DIS | PWM2_GATING_DIS);
>  }
>  
> -void intel_display_bdw_clock_gating_disable_fbcq(struct intel_display *display)
> +static void intel_display_bdw_clock_gating_disable_fbcq(struct intel_display *display)
>  {
>  	/* WaFbcAsynchFlipDisableFbcQueue:hsw,bdw */
>  	intel_de_rmw(display, CHICKEN_PIPESL_1(PIPE_A), 0, HSW_FBCQ_DIS);
>  }
>  
> -void intel_display_bdw_clock_gating_vblank_in_srd(struct intel_display *display)
> +static void intel_display_bdw_clock_gating_vblank_in_srd(struct intel_display *display)
>  {
>  	enum pipe pipe;
>  
> @@ -154,14 +154,21 @@ void intel_display_bdw_clock_gating_vblank_in_srd(struct intel_display *display)
>  	}
>  }
>  
> -void intel_display_bdw_clock_gating_kvm_notif(struct intel_display *display)
> +static void intel_display_bdw_clock_gating_kvm_notif(struct intel_display *display)
>  {
>  	/* WaKVMNotificationOnConfigChange:bdw */
>  	intel_de_rmw(display, CHICKEN_PAR2_1, 0,
>  		     KVM_CONFIG_CHANGE_NOTIFICATION_SELECT);
>  }
>  
> -void intel_display_hsw_init_clock_gating(struct intel_display *display)
> +static void intel_display_bdw_init_clock_gating(struct intel_display *display)
> +{
> +	intel_display_bdw_clock_gating_disable_fbcq(display);
> +	intel_display_bdw_clock_gating_vblank_in_srd(display);
> +	intel_display_bdw_clock_gating_kvm_notif(display);
> +}
> +
> +static void intel_display_hsw_init_clock_gating(struct intel_display *display)
>  {
>  	enum pipe pipe;
>  
> @@ -178,7 +185,7 @@ void intel_display_hsw_init_clock_gating(struct intel_display *display)
>  	}
>  }
>  
> -void intel_display_disable_trickle_feed(struct intel_display *display)
> +static void intel_display_disable_trickle_feed(struct intel_display *display)
>  {
>  	enum pipe pipe;
>  
> @@ -191,7 +198,7 @@ void intel_display_disable_trickle_feed(struct intel_display *display)
>  	}
>  }
>  
> -void intel_display_ilk_init_clock_gating(struct intel_display *display)
> +static void intel_display_ilk_init_clock_gating(struct intel_display *display)
>  {
>  	u32 dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
>  
> @@ -223,7 +230,7 @@ void intel_display_ilk_init_clock_gating(struct intel_display *display)
>  	intel_display_disable_trickle_feed(display);
>  }
>  
> -void intel_display_gen6_init_clock_gating(struct intel_display *display)
> +static void intel_display_gen6_init_clock_gating(struct intel_display *display)
>  {
>  	u32 dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
>  
> @@ -244,13 +251,15 @@ void intel_display_gen6_init_clock_gating(struct intel_display *display)
>  	intel_display_disable_trickle_feed(display);
>  }
>  
> -void intel_display_ivb_init_clock_gating(struct intel_display *display)
> +static void intel_display_ivb_init_clock_gating(struct intel_display *display)
>  {
>  	intel_de_write(display, ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
>  	intel_de_rmw(display, ILK_DISPLAY_CHICKEN1, 0, ILK_FBCQ_DIS);
> +
> +	intel_display_disable_trickle_feed(display);
>  }
>  
> -void intel_display_g4x_init_clock_gating(struct intel_display *display)
> +static void intel_display_g4x_init_clock_gating(struct intel_display *display)
>  {
>  	u32 dspclk_gate = VRHUNIT_CLOCK_GATE_DISABLE |
>  			  OVRUNIT_CLOCK_GATE_DISABLE |
> @@ -264,7 +273,7 @@ void intel_display_g4x_init_clock_gating(struct intel_display *display)
>  	intel_display_disable_trickle_feed(display);
>  }
>  
> -void intel_display_i965gm_init_clock_gating(struct intel_display *display)
> +static void intel_display_i965gm_init_clock_gating(struct intel_display *display)
>  {
>  	intel_de_write(display, DSPCLK_GATE_D, 0);
>  }
> @@ -281,4 +290,18 @@ void intel_display_init_clock_gating(struct intel_display *display)
>  		intel_display_bxt_init_clock_gating(display);
>  	else if (display->platform.geminilake)
>  		intel_display_glk_init_clock_gating(display);
> +	else if (display->platform.broadwell)
> +		intel_display_bdw_init_clock_gating(display);
> +	else if (display->platform.haswell)
> +		intel_display_hsw_init_clock_gating(display);
> +	else if (display->platform.ivybridge)
> +		intel_display_ivb_init_clock_gating(display);
> +	else if (display->platform.sandybridge)
> +		intel_display_gen6_init_clock_gating(display);
> +	else if (display->platform.ironlake)
> +		intel_display_ilk_init_clock_gating(display);
> +	else if (display->platform.g4x)
> +		intel_display_g4x_init_clock_gating(display);
> +	else if (display->platform.i965gm)
> +		intel_display_i965gm_init_clock_gating(display);
>  }
> 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 4e50e388656a..dbfa5892cffe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_clock_gating.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_clock_gating.h
> @@ -9,15 +9,5 @@
>  struct intel_display;
>  
>  void intel_display_init_clock_gating(struct intel_display *display);
> -void intel_display_bdw_clock_gating_disable_fbcq(struct intel_display *display);
> -void intel_display_bdw_clock_gating_vblank_in_srd(struct intel_display *display);
> -void intel_display_bdw_clock_gating_kvm_notif(struct intel_display *display);
> -void intel_display_hsw_init_clock_gating(struct intel_display *display);
> -void intel_display_disable_trickle_feed(struct intel_display *display);
> -void intel_display_ilk_init_clock_gating(struct intel_display *display);
> -void intel_display_gen6_init_clock_gating(struct intel_display *display);
> -void intel_display_ivb_init_clock_gating(struct intel_display *display);
> -void intel_display_g4x_init_clock_gating(struct intel_display *display);
> -void intel_display_i965gm_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 84f838c660fd..c5c4441f3a61 100644
> --- a/drivers/gpu/drm/i915/intel_clock_gating.c
> +++ b/drivers/gpu/drm/i915/intel_clock_gating.c
> @@ -73,7 +73,7 @@ static void ilk_init_clock_gating(struct drm_i915_private *i915)
>  	intel_uncore_write(&i915->uncore, PCH_3DCGDIS1,
>  			   VFMUNIT_CLOCK_GATE_DISABLE);
>  
> -	intel_display_ilk_init_clock_gating(i915->display);
> +	intel_display_init_clock_gating(i915->display);
>  	intel_pch_init_clock_gating(i915->display);
>  }
>  
> @@ -90,7 +90,7 @@ static void gen6_check_mch_setup(struct drm_i915_private *i915)
>  
>  static void gen6_init_clock_gating(struct drm_i915_private *i915)
>  {
> -	intel_display_gen6_init_clock_gating(i915->display);
> +	intel_display_init_clock_gating(i915->display);
>  
>  	intel_uncore_write(&i915->uncore, GEN6_UCGCTL1,
>  			   intel_uncore_read(&i915->uncore, GEN6_UCGCTL1) |
> @@ -205,13 +205,11 @@ static void skl_init_clock_gating(struct drm_i915_private *i915)
>  
>  static void bdw_init_clock_gating(struct drm_i915_private *i915)
>  {
> -	intel_display_bdw_clock_gating_disable_fbcq(i915->display);
> +	intel_display_init_clock_gating(i915->display);
>  
>  	/* WaSwitchSolVfFArbitrationPriority:bdw */
>  	intel_uncore_rmw(&i915->uncore, GAM_ECOCHK, 0, HSW_ECOCHK_ARB_PRIO_SOL);
>  
> -	intel_display_bdw_clock_gating_vblank_in_srd(i915->display);
> -
>  	/* WaVSRefCountFullforceMissDisable:bdw */
>  	/* WaDSRefCountFullforceMissDisable:bdw */
>  	intel_uncore_rmw(&i915->uncore, GEN7_FF_THREAD_MODE,
> @@ -226,8 +224,6 @@ static void bdw_init_clock_gating(struct drm_i915_private *i915)
>  	/* WaProgramL3SqcReg1Default:bdw */
>  	gen8_set_l3sqc_credits(i915, 30, 2);
>  
> -	intel_display_bdw_clock_gating_kvm_notif(i915->display);
> -
>  	intel_pch_init_clock_gating(i915->display);
>  
>  	/* WaDisableDopClockGating:bdw
> @@ -240,7 +236,7 @@ static void bdw_init_clock_gating(struct drm_i915_private *i915)
>  
>  static void hsw_init_clock_gating(struct drm_i915_private *i915)
>  {
> -	intel_display_hsw_init_clock_gating(i915->display);
> +	intel_display_init_clock_gating(i915->display);
>  
>  	/* This is required by WaCatErrorRejectionIssue:hsw */
>  	intel_uncore_rmw(&i915->uncore, GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
> @@ -256,7 +252,7 @@ static void ivb_init_clock_gating(struct drm_i915_private *i915)
>  {
>  	struct intel_display *display = i915->display;
>  
> -	intel_display_ivb_init_clock_gating(display);
> +	intel_display_init_clock_gating(display);
>  
>  	/* WaDisableBackToBackFlipFix:ivb */
>  	intel_uncore_write(&i915->uncore, IVB_CHICKEN3,
> @@ -285,8 +281,6 @@ static void ivb_init_clock_gating(struct drm_i915_private *i915)
>  	intel_uncore_rmw(&i915->uncore, GEN7_SQ_CHICKEN_MBCUNIT_CONFIG,
>  			 0, GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB);
>  
> -	intel_display_disable_trickle_feed(display);
> -
>  	intel_uncore_rmw(&i915->uncore, GEN6_MBCUNIT_SNPCR, GEN6_MBC_SNPCR_MASK,
>  			 GEN6_MBC_SNPCR_MED);
>  
> @@ -362,7 +356,7 @@ static void g4x_init_clock_gating(struct drm_i915_private *i915)
>  			   GS_UNIT_CLOCK_GATE_DISABLE |
>  			   CL_UNIT_CLOCK_GATE_DISABLE);
>  	intel_uncore_write(&i915->uncore, RAMCLK_GATE_D, 0);
> -	intel_display_g4x_init_clock_gating(i915->display);
> +	intel_display_init_clock_gating(i915->display);
>  }
>  
>  static void i965gm_init_clock_gating(struct drm_i915_private *i915)
> @@ -371,7 +365,7 @@ static void i965gm_init_clock_gating(struct drm_i915_private *i915)
>  
>  	intel_uncore_write(uncore, RENCLK_GATE_D1, I965_RCC_CLOCK_GATE_DISABLE);
>  	intel_uncore_write(uncore, RENCLK_GATE_D2, 0);
> -	intel_display_i965gm_init_clock_gating(i915->display);
> +	intel_display_init_clock_gating(i915->display);
>  	intel_uncore_write(uncore, RAMCLK_GATE_D, 0);
>  	intel_uncore_write16(uncore, DEUC, 0);
>  	intel_uncore_write(uncore,

-- 
Jani Nikula, Intel

  reply	other threads:[~2026-08-27  7:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  9:26 [PATCH v3 0/5] drm/i915/display: reduce clock_gating interface between core and display Luca Coelho
2026-08-17  9:26 ` [PATCH v3 1/5] drm/i915/display: consolidate Gen9 clock gating initialization Luca Coelho
2026-08-27  7:24   ` Jani Nikula
2026-08-17  9:26 ` [PATCH v3 2/5] drm/i915/display: consolidate remaining " Luca Coelho
2026-08-27  7:27   ` Jani Nikula [this message]
2026-08-17  9:26 ` [PATCH v3 3/5] drm/i915/display: remove unnecessary clock_gating include from intel_display.c Luca Coelho
2026-08-27  7:28   ` Jani Nikula
2026-08-31  8:13     ` Luca Coelho
2026-08-17  9:26 ` [PATCH v3 4/5] drm/i915/display: add a way to restore only display/pch registers in clock_gating Luca Coelho
2026-08-17  9:49   ` sashiko-bot
2026-08-17  9:26 ` [PATCH v3 5/5] drm/i915/display: split part of intel_display_reset_finish() to a new function Luca Coelho

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1a4317911980cdd6017b906337f16df11ce35fe9@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=luciano.coelho@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox