> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 25, 2024 12:40 AM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: [PATCH 09/14] drm/i915/dsb: Introduce
> intel_dsb_wait_scanline_{in, out}()
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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
