On Tue, 11 Feb 2025, Suraj Kandpal <suraj.kand...@intel.com> wrote:
> Change the argument of for_each_shared_dpll to take intel_display which
> helps move as an ongoing effort to get rid off the dependency on
> drm_i915_private. Some opportunistic changes in intel_pch_refclk done
> too.
>
> Signed-off-by: Suraj Kandpal <suraj.kand...@intel.com>
> ---
> .../drm/i915/display/intel_display_debugfs.c | 2 +-
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 24 ++++++++-----
> drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 6 ++--
> .../gpu/drm/i915/display/intel_pch_refclk.c | 36 +++++++++----------
> 4 files changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 991c1726f522..87e6f4000101 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -642,7 +642,7 @@ static int i915_shared_dplls_info(struct seq_file *m,
> void *unused)
> display->dpll.ref_clks.nssc,
> display->dpll.ref_clks.ssc);
>
> - for_each_shared_dpll(dev_priv, pll, i) {
> + for_each_shared_dpll(display, pll, i) {
> drm_printf(&p, "DPLL%i: %s, id: %i\n", pll->index,
> pll->info->name, pll->info->id);
> drm_printf(&p, " pipe_mask: 0x%x, active: 0x%x, on: %s\n",
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index cb2ef317d219..171d16e91c61 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -121,10 +121,11 @@ intel_atomic_duplicate_dpll_state(struct
> drm_i915_private *i915,
> struct intel_shared_dpll_state *shared_dpll)
> {
> struct intel_shared_dpll *pll;
> + struct intel_display *display = to_intel_display(&i915->drm);
Nitpick, these could just be:
struct intel_display *display = &i915->display;
And they'll go away once the function parameter gets changed to display.
Anyway,
Reviewed-by: Jani Nikula <jani.nik...@intel.com>
either way
> int i;
>
> /* Copy shared dpll state */
> - for_each_shared_dpll(i915, pll, i)
> + for_each_shared_dpll(display, pll, i)
> shared_dpll[pll->index] = pll->state;
> }
>
> @@ -157,10 +158,11 @@ struct intel_shared_dpll *
> intel_get_shared_dpll_by_id(struct drm_i915_private *i915,
> enum intel_dpll_id id)
> {
> + struct intel_display *display = to_intel_display(&i915->drm);
> struct intel_shared_dpll *pll;
> int i;
>
> - for_each_shared_dpll(i915, pll, i) {
> + for_each_shared_dpll(display, pll, i) {
> if (pll->info->id == id)
> return pll;
> }
> @@ -344,12 +346,13 @@ void intel_disable_shared_dpll(const struct
> intel_crtc_state *crtc_state)
> static unsigned long
> intel_dpll_mask_all(struct drm_i915_private *i915)
> {
> + struct intel_display *display = to_intel_display(&i915->drm);
> struct intel_shared_dpll *pll;
> unsigned long dpll_mask = 0;
> int i;
>
> - for_each_shared_dpll(i915, pll, i) {
> - drm_WARN_ON(&i915->drm, dpll_mask & BIT(pll->info->id));
> + for_each_shared_dpll(display, pll, i) {
> + drm_WARN_ON(display->drm, dpll_mask & BIT(pll->info->id));
>
> dpll_mask |= BIT(pll->info->id);
> }
> @@ -513,7 +516,7 @@ static void intel_put_dpll(struct intel_atomic_state
> *state,
> */
> void intel_shared_dpll_swap_state(struct intel_atomic_state *state)
> {
> - struct drm_i915_private *i915 = to_i915(state->base.dev);
> + struct intel_display *display = to_intel_display(state);
> struct intel_shared_dpll_state *shared_dpll = state->shared_dpll;
> struct intel_shared_dpll *pll;
> int i;
> @@ -521,7 +524,7 @@ void intel_shared_dpll_swap_state(struct
> intel_atomic_state *state)
> if (!state->dpll_set)
> return;
>
> - for_each_shared_dpll(i915, pll, i)
> + for_each_shared_dpll(display, pll, i)
> swap(pll->state, shared_dpll[pll->index]);
> }
>
> @@ -4551,10 +4554,11 @@ void intel_dpll_update_ref_clks(struct
> drm_i915_private *i915)
>
> void intel_dpll_readout_hw_state(struct drm_i915_private *i915)
> {
> + struct intel_display *display = to_intel_display(&i915->drm);
> struct intel_shared_dpll *pll;
> int i;
>
> - for_each_shared_dpll(i915, pll, i)
> + for_each_shared_dpll(display, pll, i)
> readout_dpll_hw_state(i915, pll);
> }
>
> @@ -4578,10 +4582,11 @@ static void sanitize_dpll_state(struct
> drm_i915_private *i915,
>
> void intel_dpll_sanitize_state(struct drm_i915_private *i915)
> {
> + struct intel_display *display = to_intel_display(&i915->drm);
> struct intel_shared_dpll *pll;
> int i;
>
> - for_each_shared_dpll(i915, pll, i)
> + for_each_shared_dpll(display, pll, i)
> sanitize_dpll_state(i915, pll);
> }
>
> @@ -4728,10 +4733,11 @@ void intel_shared_dpll_state_verify(struct
> intel_atomic_state *state,
>
> void intel_shared_dpll_verify_disabled(struct intel_atomic_state *state)
> {
> + struct intel_display *display = to_intel_display(state);
> struct drm_i915_private *i915 = to_i915(state->base.dev);
> struct intel_shared_dpll *pll;
> int i;
>
> - for_each_shared_dpll(i915, pll, i)
> + for_each_shared_dpll(display, pll, i)
> verify_single_dpll_state(i915, pll, NULL, NULL);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> index 3eee76874304..382bdf8f0b65 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> @@ -30,9 +30,9 @@
> #include "intel_display_power.h"
> #include "intel_wakeref.h"
>
> -#define for_each_shared_dpll(__i915, __pll, __i) \
> - for ((__i) = 0; (__i) < (__i915)->display.dpll.num_shared_dpll && \
> - ((__pll) = &(__i915)->display.dpll.shared_dplls[(__i)]) ;
> (__i)++)
> +#define for_each_shared_dpll(__display, __pll, __i) \
> + for ((__i) = 0; (__i) < (__display)->dpll.num_shared_dpll && \
> + ((__pll) = &(__display)->dpll.shared_dplls[(__i)]) ;
> (__i)++)
>
> enum tc_port;
> struct drm_i915_private;
> diff --git a/drivers/gpu/drm/i915/display/intel_pch_refclk.c
> b/drivers/gpu/drm/i915/display/intel_pch_refclk.c
> index 71471c1d7dc9..68e953d2b124 100644
> --- a/drivers/gpu/drm/i915/display/intel_pch_refclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_pch_refclk.c
> @@ -492,7 +492,7 @@ static void lpt_init_pch_refclk(struct drm_i915_private
> *dev_priv)
>
> static void ilk_init_pch_refclk(struct drm_i915_private *dev_priv)
> {
> - struct intel_display *display = &dev_priv->display;
> + struct intel_display *display = to_intel_display(&dev_priv->drm);
> struct intel_encoder *encoder;
> struct intel_shared_dpll *pll;
> int i;
> @@ -505,7 +505,7 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> bool using_ssc_source = false;
>
> /* We need to take the global config into account */
> - for_each_intel_encoder(&dev_priv->drm, encoder) {
> + for_each_intel_encoder(display->drm, encoder) {
> switch (encoder->type) {
> case INTEL_OUTPUT_LVDS:
> has_panel = true;
> @@ -522,7 +522,7 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> }
>
> if (HAS_PCH_IBX(dev_priv)) {
> - has_ck505 = dev_priv->display.vbt.display_clock_mode;
> + has_ck505 = display->vbt.display_clock_mode;
> can_ssc = has_ck505;
> } else {
> has_ck505 = false;
> @@ -530,10 +530,10 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> }
>
> /* Check if any DPLLs are using the SSC source */
> - for_each_shared_dpll(dev_priv, pll, i) {
> + for_each_shared_dpll(display, pll, i) {
> u32 temp;
>
> - temp = intel_de_read(dev_priv, PCH_DPLL(pll->info->id));
> + temp = intel_de_read(display, PCH_DPLL(pll->info->id));
>
> if (!(temp & DPLL_VCO_ENABLE))
> continue;
> @@ -545,7 +545,7 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> }
> }
>
> - drm_dbg_kms(&dev_priv->drm,
> + drm_dbg_kms(display->drm,
> "has_panel %d has_lvds %d has_ck505 %d using_ssc_source
> %d\n",
> has_panel, has_lvds, has_ck505, using_ssc_source);
>
> @@ -554,7 +554,7 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> * PCH B stepping, previous chipset stepping should be
> * ignoring this setting.
> */
> - val = intel_de_read(dev_priv, PCH_DREF_CONTROL);
> + val = intel_de_read(display, PCH_DREF_CONTROL);
>
> /* As we must carefully and slowly disable/enable each source in turn,
> * compute the final state we want first and check if we need to
> @@ -614,8 +614,8 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> }
>
> /* Get SSC going before enabling the outputs */
> - intel_de_write(dev_priv, PCH_DREF_CONTROL, val);
> - intel_de_posting_read(dev_priv, PCH_DREF_CONTROL);
> + intel_de_write(display, PCH_DREF_CONTROL, val);
> + intel_de_posting_read(display, PCH_DREF_CONTROL);
> udelay(200);
>
> val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> @@ -633,23 +633,23 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> }
>
> - intel_de_write(dev_priv, PCH_DREF_CONTROL, val);
> - intel_de_posting_read(dev_priv, PCH_DREF_CONTROL);
> + intel_de_write(display, PCH_DREF_CONTROL, val);
> + intel_de_posting_read(display, PCH_DREF_CONTROL);
> udelay(200);
> } else {
> - drm_dbg_kms(&dev_priv->drm, "Disabling CPU source output\n");
> + drm_dbg_kms(display->drm, "Disabling CPU source output\n");
>
> val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
>
> /* Turn off CPU output */
> val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
>
> - intel_de_write(dev_priv, PCH_DREF_CONTROL, val);
> - intel_de_posting_read(dev_priv, PCH_DREF_CONTROL);
> + intel_de_write(display, PCH_DREF_CONTROL, val);
> + intel_de_posting_read(display, PCH_DREF_CONTROL);
> udelay(200);
>
> if (!using_ssc_source) {
> - drm_dbg_kms(&dev_priv->drm, "Disabling SSC source\n");
> + drm_dbg_kms(display->drm, "Disabling SSC source\n");
>
> /* Turn off the SSC source */
> val &= ~DREF_SSC_SOURCE_MASK;
> @@ -658,13 +658,13 @@ static void ilk_init_pch_refclk(struct drm_i915_private
> *dev_priv)
> /* Turn off SSC1 */
> val &= ~DREF_SSC1_ENABLE;
>
> - intel_de_write(dev_priv, PCH_DREF_CONTROL, val);
> - intel_de_posting_read(dev_priv, PCH_DREF_CONTROL);
> + intel_de_write(display, PCH_DREF_CONTROL, val);
> + intel_de_posting_read(display, PCH_DREF_CONTROL);
> udelay(200);
> }
> }
>
> - drm_WARN_ON(&dev_priv->drm, val != final);
> + drm_WARN_ON(display->drm, val != final);
> }
>
> /*
--
Jani Nikula, Intel
