All of 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: 14+ 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
2026-08-17 14:35 ` ✓ i915.CI.BAT: success for drm/i915/display: reduce clock_gating interface between core and display (rev3) Patchwork
2026-08-18  2:32 ` ✗ i915.CI.Full: failure " Patchwork
2026-08-25  9:17   ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.