[PATCH 09/14] drm/i915/dsb: Introduce intel_dsb_wait_scanline_{in, out}()

[Manna, Animesh]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 25, 2024 12:40 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [PATCH 09/14] drm/i915/dsb: Introduce
> intel_dsb_wait_scanline_{in, out}()
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Add functions to emit a DSB scanline window wait instructions.
> We can either wait for the scanline to be IN the window or OUT of the
> window.
> 
> The hardware doesn't handle wraparound so we must manually deal with it
> by swapping the IN range to the inverse OUT range, or vice versa.
> 
> Also add a bit of paranoia to catch the edge case of waiting for the entire
> frame. That doesn't make sense since an IN wait would be a nop, and an
> OUT wait would imply waiting forever. Most of the time this also results in
> both scanline ranges (original and inverted) to have lower=upper+1 which is
> nonsense from the hw POV.
> 
> For now we are only handling the case where the scanline wait happens prior
> to latching the double buffered registers during the commit (which might
> change the timings due to LRR/VRR/etc.)
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 73 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dsb.h |  6 ++
>  2 files changed, 79 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 81937908c798..092cf082ac39 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -362,6 +362,79 @@ void intel_dsb_nonpost_end(struct intel_dsb *dsb)
>  	intel_dsb_noop(dsb, 4);
>  }
> 
> +static void intel_dsb_emit_wait_dsl(struct intel_dsb *dsb,
> +				    u32 opcode, int lower, int upper) {
> +	u64 window = ((u64)upper << DSB_SCANLINE_UPPER_SHIFT) |
> +		((u64)lower << DSB_SCANLINE_LOWER_SHIFT);
> +
> +	intel_dsb_emit(dsb, lower_32_bits(window),
> +		       (opcode << DSB_OPCODE_SHIFT) |
> +		       upper_32_bits(window));
> +}
> +
> +static void intel_dsb_wait_dsl(struct intel_atomic_state *state,
> +			       struct intel_dsb *dsb,
> +			       int lower_in, int upper_in,

Lower/upper keyword maybe confusing for during intel_dsb_wait_scanline_out(), maybe good to have name like in_start and in_end, similarly out_start and out_end.

> +			       int lower_out, int upper_out) {
> +	struct intel_crtc *crtc = dsb->crtc;
> +
> +	lower_in = dsb_scanline_to_hw(state, crtc, lower_in);
> +	upper_in = dsb_scanline_to_hw(state, crtc, upper_in);
> +
> +	lower_out = dsb_scanline_to_hw(state, crtc, lower_out);
> +	upper_out = dsb_scanline_to_hw(state, crtc, upper_out);

If lower_in > upper_in, then calculation for lower_out and upper_out is not needed. Even before calling dsb_scanline_to_hw() we can compare and check if it is for scanline_in or scanline_out... rt?

Regards,
Animesh
> +
> +	if (upper_in >= lower_in)
> +		intel_dsb_emit_wait_dsl(dsb, DSB_OPCODE_WAIT_DSL_IN,
> +					lower_in, upper_in);
> +	else if (upper_out >= lower_out)
> +		intel_dsb_emit_wait_dsl(dsb, DSB_OPCODE_WAIT_DSL_OUT,
> +					lower_out, upper_out);
> +	else
> +		drm_WARN_ON(crtc->base.dev, 1); /* assert_dsl_ok() should
> have caught
> +it already */ }
> +
> +static void assert_dsl_ok(struct intel_atomic_state *state,
> +			  struct intel_dsb *dsb,
> +			  int start, int end)
> +{
> +	struct intel_crtc *crtc = dsb->crtc;
> +	int vtotal = dsb_vtotal(state, crtc);
> +
> +	/*
> +	 * Waiting for the entire frame doesn't make sense,
> +	 * (IN==don't wait, OUT=wait forever).
> +	 */
> +	drm_WARN(crtc->base.dev, (end - start + vtotal) % vtotal == vtotal -
> 1,
> +		 "[CRTC:%d:%s] DSB %d bad scanline window wait: %d-%d
> (vt=%d)\n",
> +		 crtc->base.base.id, crtc->base.name, dsb->id,
> +		 start, end, vtotal);
> +}
> +
> +void intel_dsb_wait_scanline_in(struct intel_atomic_state *state,
> +				struct intel_dsb *dsb,
> +				int start, int end)
> +{
> +	assert_dsl_ok(state, dsb, start, end);
> +
> +	intel_dsb_wait_dsl(state, dsb,
> +			   start, end,
> +			   end + 1, start - 1);
> +}
> +
> +void intel_dsb_wait_scanline_out(struct intel_atomic_state *state,
> +				 struct intel_dsb *dsb,
> +				 int start, int end)
> +{
> +	assert_dsl_ok(state, dsb, start, end);
> +
> +	intel_dsb_wait_dsl(state, dsb,
> +			   end + 1, start - 1,
> +			   start, end);
> +}
> +
>  static void intel_dsb_align_tail(struct intel_dsb *dsb)  {
>  	u32 aligned_tail, tail;
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h
> b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 84fc2f8434d1..d0737cefb393 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -39,6 +39,12 @@ void intel_dsb_reg_write_masked(struct intel_dsb
> *dsb,  void intel_dsb_noop(struct intel_dsb *dsb, int count);  void
> intel_dsb_nonpost_start(struct intel_dsb *dsb);  void
> intel_dsb_nonpost_end(struct intel_dsb *dsb);
> +void intel_dsb_wait_scanline_in(struct intel_atomic_state *state,
> +				struct intel_dsb *dsb,
> +				int lower, int upper);
> +void intel_dsb_wait_scanline_out(struct intel_atomic_state *state,
> +				 struct intel_dsb *dsb,
> +				 int lower, int upper);
> 
>  void intel_dsb_commit(struct intel_dsb *dsb,
>  		      bool wait_for_vblank);
> --
> 2.44.2