[Intel-gfx] [PATCH 2/8] drm/i915/display: remove intel_wait_for_vblank()

[Jani Nikula]

On Thu, 25 Nov 2021, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Thu, Nov 25, 2021 at 12:23:22PM +0200, Ville Syrjälä wrote:
>> On Mon, Nov 22, 2021 at 03:51:03PM +0200, Jani Nikula wrote:
>> > There are only three call sites remaining for
>> > intel_wait_for_vblank(). Remove the function, and open code it to avoid
>> > new users from showing up.
>> > 
>> > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_cdclk.c         | 2 +-
>> >  drivers/gpu/drm/i915/display/intel_crt.c           | 2 +-
>> >  drivers/gpu/drm/i915/display/intel_display.c       | 8 ++++++--
>> >  drivers/gpu/drm/i915/display/intel_display_types.h | 8 --------
>> >  4 files changed, 8 insertions(+), 12 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > index 91c19e0a98d7..e3b863ee0bbb 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > @@ -1690,7 +1690,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>> >  	intel_de_write(dev_priv, CDCLK_CTL, val);
>> >  
>> >  	if (pipe != INVALID_PIPE)
>> > -		intel_wait_for_vblank(dev_priv, pipe);
>> > +		drm_crtc_wait_one_vblank(&intel_get_crtc_for_pipe(dev_priv, pipe)->base);
>> 
>> That looks rather hideuous. I think I'd prefer to keep the wrapper.
>
> I guess if we had an intel_crtc based version of the vblank wait
> function it might not look so terrible.

That's what I had initially, actually, and was divided between the
approaches. Ended up on this on the general principle of not adding
local no-op wrappers.

In any case, I think I do want to ditch the pipe-based wrapper because I
don't want to encourage people to use that instead of the direct crtc
version. If adding an intel_crtc based wrapper is good enough to make
that happen, let's do that?

> We could also s/intel_get_crtc_for_pipe/intel_crtc_for_pipe/ to make it
> a bit more succinct and look less like some refcounted thing.

Yeah, thought about that too but left it out of this series.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center