| Message ID | 20220926191341.5495-4-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Start cleaning up the DPLL ID mess | expand |
On Mon, 26 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > No one really cares how we store the shared_dplls. Currently > it happens to be an array, but we could change that to a more > flexible scheme at some point. Hide the implementation details > behind an iterator macro. > > The slight downside is the pll variable moving out of the > loop scope, but maybe someday soon we'll start to convert > everything over to having declarations within for-statements... > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > .../drm/i915/display/intel_display_debugfs.c | 5 +-- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 38 +++++++++---------- > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 4 ++ > .../gpu/drm/i915/display/intel_pch_refclk.c | 4 +- > 4 files changed, 25 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index c5f47df0f362..c85f60f0ced8 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -932,6 +932,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused) > { > struct drm_i915_private *dev_priv = node_to_i915(m->private); > struct drm_device *dev = &dev_priv->drm; > + struct intel_shared_dpll *pll; > int i; > > drm_modeset_lock_all(dev); > @@ -940,9 +941,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused) > dev_priv->display.dpll.ref_clks.nssc, > dev_priv->display.dpll.ref_clks.ssc); > > - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { > - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; > - > + for_each_shared_dpll(dev_priv, pll, i) { > seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->info->name, Only noticed it here, but the loop variable i is no longer the DPLL number, right? No place should actually use i for anything other than the looping. This should probably be fixed in an earlier patch. BR, Jani. > pll->info->id); > seq_printf(m, " 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 25e6f7a427b0..3dd414d663f0 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c > @@ -110,14 +110,12 @@ static void > intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv, > struct intel_shared_dpll_state *shared_dpll) > { > + struct intel_shared_dpll *pll; > int i; > > /* Copy shared dpll state */ > - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { > - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; > - > + for_each_shared_dpll(dev_priv, pll, i) > shared_dpll[pll->index] = pll->state; > - } > } > > static struct intel_shared_dpll_state * > @@ -149,11 +147,10 @@ struct intel_shared_dpll * > intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, > enum intel_dpll_id id) > { > + struct intel_shared_dpll *pll; > int i; > > - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { > - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; > - > + for_each_shared_dpll(dev_priv, pll, i) { > if (pll->info->id == id) > return pll; > } > @@ -311,12 +308,11 @@ 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_shared_dpll *pll; > unsigned long dpll_mask = 0; > int i; > > - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) { > - struct intel_shared_dpll *pll = &i915->display.dpll.shared_dplls[i]; > - > + for_each_shared_dpll(i915, pll, i) { > drm_WARN_ON(&i915->drm, dpll_mask & BIT(pll->info->id)); > > dpll_mask |= BIT(pll->info->id); > @@ -449,16 +445,14 @@ void intel_shared_dpll_swap_state(struct intel_atomic_state *state) > { > struct drm_i915_private *dev_priv = to_i915(state->base.dev); > struct intel_shared_dpll_state *shared_dpll = state->shared_dpll; > + struct intel_shared_dpll *pll; > int i; > > if (!state->dpll_set) > return; > > - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { > - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; > - > + for_each_shared_dpll(dev_priv, pll, i) > swap(pll->state, shared_dpll[pll->index]); > - } > } > > static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv, > @@ -4431,10 +4425,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_shared_dpll *pll; > int i; > > - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) > - readout_dpll_hw_state(i915, &i915->display.dpll.shared_dplls[i]); > + for_each_shared_dpll(i915, pll, i) > + readout_dpll_hw_state(i915, pll); > } > > static void sanitize_dpll_state(struct drm_i915_private *i915, > @@ -4458,10 +4453,11 @@ static void sanitize_dpll_state(struct drm_i915_private *i915, > > void intel_dpll_sanitize_state(struct drm_i915_private *i915) > { > + struct intel_shared_dpll *pll; > int i; > > - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) > - sanitize_dpll_state(i915, &i915->display.dpll.shared_dplls[i]); > + for_each_shared_dpll(i915, pll, i) > + sanitize_dpll_state(i915, pll); > } > > /** > @@ -4571,9 +4567,9 @@ void intel_shared_dpll_state_verify(struct intel_crtc *crtc, > > void intel_shared_dpll_verify_disabled(struct drm_i915_private *i915) > { > + struct intel_shared_dpll *pll; > int i; > > - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) > - verify_single_dpll_state(i915, &i915->display.dpll.shared_dplls[i], > - NULL, NULL); > + for_each_shared_dpll(i915, 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 b8dce4e9ac54..d419f4f98d28 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > @@ -36,6 +36,10 @@ > (void) (&__a == &__b); \ > __a > __b ? (__a - __b) : (__b - __a); }) > > +#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)++) > + > enum tc_port; > struct drm_i915_private; > struct intel_atomic_state; > diff --git a/drivers/gpu/drm/i915/display/intel_pch_refclk.c b/drivers/gpu/drm/i915/display/intel_pch_refclk.c > index 4db4b8d57e21..bb4e70aa9f6e 100644 > --- a/drivers/gpu/drm/i915/display/intel_pch_refclk.c > +++ b/drivers/gpu/drm/i915/display/intel_pch_refclk.c > @@ -497,6 +497,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_encoder *encoder; > + struct intel_shared_dpll *pll; > int i; > u32 val, final; > bool has_lvds = false; > @@ -532,8 +533,7 @@ static void ilk_init_pch_refclk(struct drm_i915_private *dev_priv) > } > > /* Check if any DPLLs are using the SSC source */ > - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { > - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; > + for_each_shared_dpll(dev_priv, pll, i) { > u32 temp; > > temp = intel_de_read(dev_priv, PCH_DPLL(pll->info->id));
On Mon, 26 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > index b8dce4e9ac54..d419f4f98d28 100644 > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h > @@ -36,6 +36,10 @@ > (void) (&__a == &__b); \ > __a > __b ? (__a - __b) : (__b - __a); }) > > +#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)++) > + A follow-up could drop dpll.num_shared_dpll altogether, and use ARRAY_SIZE() to loop. This anyway stops at first NULL value. BR, Jani.
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index c5f47df0f362..c85f60f0ced8 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -932,6 +932,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused) { struct drm_i915_private *dev_priv = node_to_i915(m->private); struct drm_device *dev = &dev_priv->drm; + struct intel_shared_dpll *pll; int i; drm_modeset_lock_all(dev); @@ -940,9 +941,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused) dev_priv->display.dpll.ref_clks.nssc, dev_priv->display.dpll.ref_clks.ssc); - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; - + for_each_shared_dpll(dev_priv, pll, i) { seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->info->name, pll->info->id); seq_printf(m, " 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 25e6f7a427b0..3dd414d663f0 100644 --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c @@ -110,14 +110,12 @@ static void intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv, struct intel_shared_dpll_state *shared_dpll) { + struct intel_shared_dpll *pll; int i; /* Copy shared dpll state */ - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; - + for_each_shared_dpll(dev_priv, pll, i) shared_dpll[pll->index] = pll->state; - } } static struct intel_shared_dpll_state * @@ -149,11 +147,10 @@ struct intel_shared_dpll * intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, enum intel_dpll_id id) { + struct intel_shared_dpll *pll; int i; - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; - + for_each_shared_dpll(dev_priv, pll, i) { if (pll->info->id == id) return pll; } @@ -311,12 +308,11 @@ 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_shared_dpll *pll; unsigned long dpll_mask = 0; int i; - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) { - struct intel_shared_dpll *pll = &i915->display.dpll.shared_dplls[i]; - + for_each_shared_dpll(i915, pll, i) { drm_WARN_ON(&i915->drm, dpll_mask & BIT(pll->info->id)); dpll_mask |= BIT(pll->info->id); @@ -449,16 +445,14 @@ void intel_shared_dpll_swap_state(struct intel_atomic_state *state) { struct drm_i915_private *dev_priv = to_i915(state->base.dev); struct intel_shared_dpll_state *shared_dpll = state->shared_dpll; + struct intel_shared_dpll *pll; int i; if (!state->dpll_set) return; - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; - + for_each_shared_dpll(dev_priv, pll, i) swap(pll->state, shared_dpll[pll->index]); - } } static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv, @@ -4431,10 +4425,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_shared_dpll *pll; int i; - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) - readout_dpll_hw_state(i915, &i915->display.dpll.shared_dplls[i]); + for_each_shared_dpll(i915, pll, i) + readout_dpll_hw_state(i915, pll); } static void sanitize_dpll_state(struct drm_i915_private *i915, @@ -4458,10 +4453,11 @@ static void sanitize_dpll_state(struct drm_i915_private *i915, void intel_dpll_sanitize_state(struct drm_i915_private *i915) { + struct intel_shared_dpll *pll; int i; - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) - sanitize_dpll_state(i915, &i915->display.dpll.shared_dplls[i]); + for_each_shared_dpll(i915, pll, i) + sanitize_dpll_state(i915, pll); } /** @@ -4571,9 +4567,9 @@ void intel_shared_dpll_state_verify(struct intel_crtc *crtc, void intel_shared_dpll_verify_disabled(struct drm_i915_private *i915) { + struct intel_shared_dpll *pll; int i; - for (i = 0; i < i915->display.dpll.num_shared_dpll; i++) - verify_single_dpll_state(i915, &i915->display.dpll.shared_dplls[i], - NULL, NULL); + for_each_shared_dpll(i915, 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 b8dce4e9ac54..d419f4f98d28 100644 --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h @@ -36,6 +36,10 @@ (void) (&__a == &__b); \ __a > __b ? (__a - __b) : (__b - __a); }) +#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)++) + enum tc_port; struct drm_i915_private; struct intel_atomic_state; diff --git a/drivers/gpu/drm/i915/display/intel_pch_refclk.c b/drivers/gpu/drm/i915/display/intel_pch_refclk.c index 4db4b8d57e21..bb4e70aa9f6e 100644 --- a/drivers/gpu/drm/i915/display/intel_pch_refclk.c +++ b/drivers/gpu/drm/i915/display/intel_pch_refclk.c @@ -497,6 +497,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_encoder *encoder; + struct intel_shared_dpll *pll; int i; u32 val, final; bool has_lvds = false; @@ -532,8 +533,7 @@ static void ilk_init_pch_refclk(struct drm_i915_private *dev_priv) } /* Check if any DPLLs are using the SSC source */ - for (i = 0; i < dev_priv->display.dpll.num_shared_dpll; i++) { - struct intel_shared_dpll *pll = &dev_priv->display.dpll.shared_dplls[i]; + for_each_shared_dpll(dev_priv, pll, i) { u32 temp; temp = intel_de_read(dev_priv, PCH_DPLL(pll->info->id));