On Thu, Jun 26, 2025 at 09:29:18PM +0300, Jani Nikula wrote: > Prefer the register read specific wait function over i915 wait_for_us(). > > The existing condition is quite complicated. Simplify by checking for > requesters first, and determine timeout based on that. Refresh > requesters in case of timeouts, should one have popped up during the > wait. The downside is that this does not cut the wait short if > requesters show up *during* the wait, but we're talking about 1 ms so > shouldn't be an issue. > > Cc: Imre Deak <[email protected]> > Signed-off-by: Jani Nikula <[email protected]> > --- > .../drm/i915/display/intel_display_power_well.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c > b/drivers/gpu/drm/i915/display/intel_display_power_well.c > index 5c9ca8141fcc..9d310a33e701 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c > @@ -308,8 +308,8 @@ static void hsw_wait_for_power_well_disable(struct > intel_display *display, > { > const struct i915_power_well_regs *regs = power_well->desc->ops->regs; > int pw_idx = i915_power_well_instance(power_well)->hsw.idx; > - bool disabled; > u32 reqs; > + int ret; > > /* > * Bspec doesn't require waiting for PWs to get disabled, but still do > @@ -320,12 +320,17 @@ static void hsw_wait_for_power_well_disable(struct > intel_display *display, > * Skip the wait in case any of the request bits are set and print a > * diagnostic message. > */ > - wait_for((disabled = !(intel_de_read(display, regs->driver) & > - HSW_PWR_WELL_CTL_STATE(pw_idx))) || > - (reqs = hsw_power_well_requesters(display, regs, pw_idx)), 1); > - if (disabled) > + reqs = hsw_power_well_requesters(display, regs, pw_idx); > + > + ret = intel_de_wait_for_clear(display, regs->driver, > + HSW_PWR_WELL_CTL_STATE(pw_idx), > + reqs ? 0 : 1); > + if (!ret) > return; > > + /* Refresh requesters in case they popped up during the wait. */ > + reqs = hsw_power_well_requesters(display, regs, pw_idx);
Ok, but I'd only update here reqs if it's 0 to get a better debug info if the wait was skipped because a requester before the wait, but the request was removed for some reason before the update here. > + > drm_dbg_kms(display->drm, > "%s forced on (bios:%d driver:%d kvmr:%d debug:%d)\n", > intel_power_well_name(power_well), > -- > 2.39.5 >
