Re: [Intel-gfx] [PATCH 2/7] drm/i915/display: move more scanline functions to intel_vblank.[ch]

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 15 Dec 2022, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Thursday, December 15, 2022 3:50 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Subject: RE: [Intel-gfx] [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@intel.com> 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