On Tue, 2024-10-22 at 07:55 -0300, Gustavo Sousa wrote:
> Quoting Jani Nikula (2024-10-22 06:34:44-03:00)
> > On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@xxxxxxxxx> wrote:
> > > Some display MMIO transactions for offsets in the range that requires
> > > the DMC wakelock happen in atomic context (this has been confirmed
> > > during tests on PTL). That means that we need to use a non-sleeping
> > > variant of MMIO waiting function.
> > >
> > > Implement __intel_de_wait_for_register_atomic_nowl() and use it when
> > > waiting for acknowledgment of acquire/release.
> > >
> > > Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_de.h | 11 +++++++++++
> > > drivers/gpu/drm/i915/display/intel_dmc_wl.c | 20 ++++++++++++--------
> > > 2 files changed, 23 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
> > > index e017cd4a8168..4116783a62dd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_de.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_de.h
> > > @@ -121,6 +121,17 @@ ____intel_de_wait_for_register_nowl(struct intel_display *display,
> > > }
> > > #define __intel_de_wait_for_register_nowl(p,...) ____intel_de_wait_for_register_nowl(__to_intel_display(p), __VA_ARGS__)
> > >
> > > +static inline int
> > > +____intel_de_wait_for_register_atomic_nowl(struct intel_display *display,
> > > + i915_reg_t reg,
> > > + u32 mask, u32 value,
> > > + unsigned int fast_timeout_us)
> > > +{
> > > + return __intel_wait_for_register(__to_uncore(display), reg, mask,
> > > + value, fast_timeout_us, 0, NULL);
> > > +}
> > > +#define __intel_de_wait_for_register_atomic_nowl(p,...) ____intel_de_wait_for_register_atomic_nowl(__to_intel_display(p), __VA_ARGS__)
> > > +
> >
> > There's no need to add the wrapper when all users pass struct
> > intel_display. And we don't want new users that pass i915.
>
> Ah, okay. Thanks.
>
> >
> > And why are we adding new stuff and users with double underscores?
>
> Well, this is a no-wakelock variant and it shouldn't be used broadly. I
> believe that was the motivation of all "__intel_de_*nowl" variants being
> prefixed with the underscores.
Yes, that's exactly the idea in the code I added earlier. The double
underscore is used for non-locking functions that are called by their
locking versions. And should only be used elsewhere in very specific
cases.
--
Cheers,
Luca.
