On Tue, 2025-11-25 at 15:24 +0200, Jani Nikula wrote: > Add another level of macro abstraction, and declare the wakeref within > the for loop using __UNIQUE_ID. This allows us to drop a bunch of > boilerplate declarations and parameter passing. > > Signed-off-by: Jani Nikula <[email protected]> > --- > drivers/gpu/drm/i915/display/g4x_dp.c | 3 +- > drivers/gpu/drm/i915/display/intel_pps.c | 56 +++++++----------------- > drivers/gpu/drm/i915/display/intel_pps.h | 7 ++- > 3 files changed, 22 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c > b/drivers/gpu/drm/i915/display/g4x_dp.c > index a3ff21b2f69f..27f4c55d7484 100644 > --- a/drivers/gpu/drm/i915/display/g4x_dp.c > +++ b/drivers/gpu/drm/i915/display/g4x_dp.c > @@ -684,12 +684,11 @@ static void intel_enable_dp(struct intel_atomic_state > *state, > struct intel_display *display = to_intel_display(state); > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > u32 dp_reg = intel_de_read(display, intel_dp->output_reg); > - intel_wakeref_t wakeref; > > if (drm_WARN_ON(display->drm, dp_reg & DP_PORT_EN)) > return; > > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > if (display->platform.valleyview || > display->platform.cherryview) > vlv_pps_port_enable_unlocked(encoder, pipe_config); > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > b/drivers/gpu/drm/i915/display/intel_pps.c > index 25692a547764..34376255b85c 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.c > +++ b/drivers/gpu/drm/i915/display/intel_pps.c > @@ -697,12 +697,10 @@ static void wait_panel_power_cycle(struct intel_dp > *intel_dp) > > void intel_pps_wait_power_cycle(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > - > if (!intel_dp_is_edp(intel_dp)) > return; > > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > wait_panel_power_cycle(intel_dp); > } > > @@ -811,14 +809,13 @@ bool intel_pps_vdd_on_unlocked(struct intel_dp > *intel_dp) > void intel_pps_vdd_on(struct intel_dp *intel_dp) > { > struct intel_display *display = to_intel_display(intel_dp); > - intel_wakeref_t wakeref; > bool vdd; > > if (!intel_dp_is_edp(intel_dp)) > return; > > vdd = false; > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > vdd = intel_pps_vdd_on_unlocked(intel_dp); > INTEL_DISPLAY_STATE_WARN(display, !vdd, "[ENCODER:%d:%s] %s VDD already > requested on\n", > dp_to_dig_port(intel_dp)->base.base.base.id, > @@ -873,8 +870,6 @@ static void intel_pps_vdd_off_sync_unlocked(struct > intel_dp *intel_dp) > > void intel_pps_vdd_off_sync(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > - > if (!intel_dp_is_edp(intel_dp)) > return; > > @@ -883,7 +878,7 @@ void intel_pps_vdd_off_sync(struct intel_dp *intel_dp) > * vdd might still be enabled due to the delayed vdd off. > * Make sure vdd is actually turned off here. > */ > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > intel_pps_vdd_off_sync_unlocked(intel_dp); > } > > @@ -892,9 +887,8 @@ static void edp_panel_vdd_work(struct work_struct *__work) > struct intel_pps *pps = container_of(to_delayed_work(__work), > struct intel_pps, panel_vdd_work); > struct intel_dp *intel_dp = container_of(pps, struct intel_dp, pps); > - intel_wakeref_t wakeref; > > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > if (!intel_dp->pps.want_panel_vdd) > intel_pps_vdd_off_sync_unlocked(intel_dp); > } > @@ -952,12 +946,10 @@ void intel_pps_vdd_off_unlocked(struct intel_dp > *intel_dp, bool sync) > > void intel_pps_vdd_off(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > - > if (!intel_dp_is_edp(intel_dp)) > return; > > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > intel_pps_vdd_off_unlocked(intel_dp, false); > } > > @@ -1026,12 +1018,10 @@ void intel_pps_on_unlocked(struct intel_dp *intel_dp) > > void intel_pps_on(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > - > if (!intel_dp_is_edp(intel_dp)) > return; > > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > intel_pps_on_unlocked(intel_dp); > } > > @@ -1082,12 +1072,10 @@ void intel_pps_off_unlocked(struct intel_dp *intel_dp) > > void intel_pps_off(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > - > if (!intel_dp_is_edp(intel_dp)) > return; > > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > intel_pps_off_unlocked(intel_dp); > } > > @@ -1095,7 +1083,6 @@ void intel_pps_off(struct intel_dp *intel_dp) > void intel_pps_backlight_on(struct intel_dp *intel_dp) > { > struct intel_display *display = to_intel_display(intel_dp); > - intel_wakeref_t wakeref; > > /* > * If we enable the backlight right away following a panel power > @@ -1105,7 +1092,7 @@ void intel_pps_backlight_on(struct intel_dp *intel_dp) > */ > wait_backlight_on(intel_dp); > > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > i915_reg_t pp_ctrl_reg = _pp_ctrl_reg(intel_dp); > u32 pp; > > @@ -1121,12 +1108,11 @@ void intel_pps_backlight_on(struct intel_dp *intel_dp) > void intel_pps_backlight_off(struct intel_dp *intel_dp) > { > struct intel_display *display = to_intel_display(intel_dp); > - intel_wakeref_t wakeref; > > if (!intel_dp_is_edp(intel_dp)) > return; > > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > i915_reg_t pp_ctrl_reg = _pp_ctrl_reg(intel_dp); > u32 pp; > > @@ -1149,11 +1135,10 @@ void intel_pps_backlight_power(struct intel_connector > *connector, bool enable) > { > struct intel_display *display = to_intel_display(connector); > struct intel_dp *intel_dp = intel_attached_dp(connector); > - intel_wakeref_t wakeref; > bool is_enabled; > > is_enabled = false; > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > is_enabled = ilk_get_pp_control(intel_dp) & EDP_BLC_ENABLE; > if (is_enabled == enable) > return; > @@ -1251,9 +1236,7 @@ void vlv_pps_pipe_init(struct intel_dp *intel_dp) > /* Call on all DP, not just eDP */ > void vlv_pps_pipe_reset(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > - > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > intel_dp->pps.vlv_active_pipe = vlv_active_pipe(intel_dp); > } > > @@ -1329,9 +1312,7 @@ void vlv_pps_port_disable(struct intel_encoder *encoder, > { > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > - intel_wakeref_t wakeref; > - > - with_intel_pps_lock(intel_dp, wakeref) > + with_intel_pps_lock(intel_dp) > intel_dp->pps.vlv_active_pipe = INVALID_PIPE; > } > > @@ -1362,10 +1343,9 @@ static void pps_vdd_init(struct intel_dp *intel_dp) > > bool intel_pps_have_panel_power_or_vdd(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > bool have_power = false; > > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > have_power = edp_have_panel_power(intel_dp) || > edp_have_panel_vdd(intel_dp); > } > @@ -1692,12 +1672,11 @@ static void pps_init_registers(struct intel_dp > *intel_dp, bool force_disable_vdd > void intel_pps_encoder_reset(struct intel_dp *intel_dp) > { > struct intel_display *display = to_intel_display(intel_dp); > - intel_wakeref_t wakeref; > > if (!intel_dp_is_edp(intel_dp)) > return; > > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > /* > * Reinit the power sequencer also on the resume path, in case > * BIOS did something nasty with it. > @@ -1716,7 +1695,6 @@ void intel_pps_encoder_reset(struct intel_dp *intel_dp) > > bool intel_pps_init(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > bool ret; > > intel_dp->pps.initializing = true; > @@ -1724,7 +1702,7 @@ bool intel_pps_init(struct intel_dp *intel_dp) > > pps_init_timestamps(intel_dp); > > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > ret = pps_initial_setup(intel_dp); > > pps_init_delays(intel_dp); > @@ -1760,9 +1738,7 @@ static void pps_init_late(struct intel_dp *intel_dp) > > void intel_pps_init_late(struct intel_dp *intel_dp) > { > - intel_wakeref_t wakeref; > - > - with_intel_pps_lock(intel_dp, wakeref) { > + with_intel_pps_lock(intel_dp) { > /* Reinit delays after per-panel info has been parsed from VBT > */ > pps_init_late(intel_dp); > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.h > b/drivers/gpu/drm/i915/display/intel_pps.h > index c83007152f07..ad5c458ccdaf 100644 > --- a/drivers/gpu/drm/i915/display/intel_pps.h > +++ b/drivers/gpu/drm/i915/display/intel_pps.h > @@ -20,8 +20,11 @@ struct intel_encoder; > intel_wakeref_t intel_pps_lock(struct intel_dp *intel_dp); > intel_wakeref_t intel_pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t > wakeref); > > -#define with_intel_pps_lock(dp, wf) > \ > - for ((wf) = intel_pps_lock(dp); (wf); (wf) = intel_pps_unlock((dp), > (wf))) > +#define __with_intel_pps_lock(dp, wf) \ > + for (intel_wakeref_t (wf) = intel_pps_lock(dp); (wf); (wf) = > intel_pps_unlock((dp), (wf))) > + > +#define with_intel_pps_lock(dp) \ > + __with_intel_pps_lock((dp), __UNIQUE_ID(wakeref)) > > void intel_pps_backlight_on(struct intel_dp *intel_dp); > void intel_pps_backlight_off(struct intel_dp *intel_dp);
Nice! Reviewed-by: Luca Coelho <[email protected]> -- Cheers, Luca.
