On Thu, 15 Dec 2022, "Murthy, Arun R" <arun.r.murthy@xxxxxxxxx> wrote: >> -----Original Message----- >> From: Nikula, Jani <jani.nikula@xxxxxxxxx> >> Sent: Thursday, December 15, 2022 3:50 PM >> To: Murthy, Arun R <arun.r.murthy@xxxxxxxxx>; intel- >> gfx@xxxxxxxxxxxxxxxxxxxxx >> Subject: RE: [PATCH 2/7] drm/i915/display: move more scanline >> functions to intel_vblank.[ch] >> >> On Thu, 15 Dec 2022, "Murthy, Arun R" <arun.r.murthy@xxxxxxxxx> wrote: >> >> >> And how would you propose to drop the wrapper? The wrappers are >> >> >> all about readability in the caller side: >> >> >> >> >> > I didn’t mean to drop the wrapper, understand that wrapper is more >> >> readable, what I meant is to replace >> >> wait_for_pipe_scanline_moving/stopped with its function contents. >> >> >> >> Why should we duplicate the code? >> > >> > static void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc) { > Bool state can be added as a parameter to this function, on the other hand, can have state = false as a magic value as well. Then it boils down to what we already have? Too much talk now, please send actual working code instead. BR, Jani. > > Thanks and Regards, > Arun R Murthy > ------------------- >> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> > enum pipe pipe = crtc->pipe >> > >> > /* Wait for the display line to settle/start moving */ >> > if (wait_for(pipe_scanline_is_moving(dev_priv, pipe) == state, >> > 100)) >> > >> > drm_err(&dev_priv->drm, >> > "pipe %c scanline %s wait timed out\n", >> > pipe_name(pipe), str_on_off(state)); } >> >> And the state variable comes from where? >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center