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

[Murthy, Arun R]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Monday, December 12, 2022 7:59 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula at intel.com>
> Subject: [Intel-gfx] [PATCH 2/7] drm/i915/display: move more scanline
> functions to intel_vblank.[ch]
> 
> Reduce clutter in intel_display.c by moving the scanline moving/stopped wait
> functions to intel_vblank.[ch].
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 36 +-------------------
> drivers/gpu/drm/i915/display/intel_vblank.c  | 35 +++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_vblank.h  |  2 ++
>  3 files changed, 38 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 6cdfdae2c712..0cdb514d7ee0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -115,6 +115,7 @@
>  #include "intel_quirks.h"
>  #include "intel_sprite.h"
>  #include "intel_tc.h"
> +#include "intel_vblank.h"
>  #include "intel_vga.h"
>  #include "i9xx_plane.h"
>  #include "skl_scaler.h"
> @@ -386,41 +387,6 @@ struct intel_crtc *intel_master_crtc(const struct
> intel_crtc_state *crtc_state)
>  		return to_intel_crtc(crtc_state->uapi.crtc);
>  }
> 
> -static bool pipe_scanline_is_moving(struct drm_i915_private *dev_priv,
> -				    enum pipe pipe)
> -{
> -	i915_reg_t reg = PIPEDSL(pipe);
> -	u32 line1, line2;
> -
> -	line1 = intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK;
> -	msleep(5);
> -	line2 = intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK;
> -
> -	return line1 != line2;
> -}
> -
> -static void wait_for_pipe_scanline_moving(struct intel_crtc *crtc, bool state)
> -{
> -	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));
> -}
> -
> -static void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *crtc) -{
> -	wait_for_pipe_scanline_moving(crtc, false);
> -}
> -
> -static void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc) -{
> -	wait_for_pipe_scanline_moving(crtc, true);
> -}
> -
>  static void
>  intel_wait_for_pipe_off(const struct intel_crtc_state *old_crtc_state)  { diff --
> git a/drivers/gpu/drm/i915/display/intel_vblank.c
> b/drivers/gpu/drm/i915/display/intel_vblank.c
> index 78a579496ad1..f25ec643a0a3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -417,3 +417,38 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
> 
>  	return position;
>  }
> +
> +static bool pipe_scanline_is_moving(struct drm_i915_private *dev_priv,
> +				    enum pipe pipe)
> +{
> +	i915_reg_t reg = PIPEDSL(pipe);
> +	u32 line1, line2;
> +
> +	line1 = intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK;
> +	msleep(5);
> +	line2 = intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK;
> +
> +	return line1 != line2;
> +}
> +
> +static void wait_for_pipe_scanline_moving(struct intel_crtc *crtc, bool
> +state) {
> +	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)); }
> +
> +void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *crtc) {
> +	wait_for_pipe_scanline_moving(crtc, false); }
> +
Is this wrapper function required, since nothing else is being other than calling the function wait_for_pipe_scanline_moving, can this be replaced?

Thanks and Regards,
Arun R Murthy
--------------------

> +void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc) {
> +	wait_for_pipe_scanline_moving(crtc, true); }
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.h
> b/drivers/gpu/drm/i915/display/intel_vblank.h
> index 9c0034d7454d..54870cabd734 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.h
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.h
> @@ -17,5 +17,7 @@ u32 g4x_get_vblank_counter(struct drm_crtc *crtc);
> bool intel_crtc_get_vblank_timestamp(struct drm_crtc *crtc, int *max_error,
>  				     ktime_t *vblank_time, bool in_vblank_irq);
> int intel_get_crtc_scanline(struct intel_crtc *crtc);
> +void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *crtc);
> +void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc);
> 
>  #endif /* __INTEL_VBLANK_H__ */
> --
> 2.34.1