[Intel-gfx] [PATCH v2 10/15] drm/i915/dsb: function to trigger workload execution of DSB.

[Chris Wilson]

Quoting Animesh Manna (2019-08-21 07:32:30)
> Batch buffer will be created through dsb-reg-write function which can have
> single/multiple request based on usecase and once the buffer is ready
> commit function will trigger the execution of the batch buffer. All
> the registers will be updated simultaneously.
> 
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 43 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dsb.h |  1 +
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index f97d0c06a049..7e0a9b37f702 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -191,3 +191,46 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
>                                 DSB_OPCODE_SHIFT) | DSB_BYTE_EN |
>                                 i915_mmio_reg_offset(reg);
>  }
> +
> +void intel_dsb_commit(struct intel_dsb *dsb)
> +{
> +       struct intel_crtc *crtc = dsb->crtc;
> +       struct drm_device *dev = crtc->base.dev;
> +       struct drm_i915_private *dev_priv = to_i915(dev);
> +       enum pipe pipe = crtc->pipe;
> +       u32 cmd_buf_tail, cmd_buf_size;
> +
> +       if (!dsb->free_pos)
> +               return;
> +
> +       if (!intel_dsb_enable_engine(dsb))
> +               goto reset;
> +
> +       if (is_dsb_busy(dsb)) {
> +               DRM_DEBUG_KMS("HEAD_PTR write failed - dsb engine is busy.\n");
> +               goto reset;
> +       }
> +       I915_WRITE(DSB_HEAD_PTR(pipe, dsb->id), dsb->cmd_buf_head);
> +
> +       cmd_buf_size = dsb->free_pos * 4;
> +       cmd_buf_tail = round_up((dsb->cmd_buf_head + cmd_buf_size),
> +                               CACHELINE_BYTES);

head is already page-aligned.

tail = ALIGN(dst->free_pos * 4, CACHELINE_BYTES);
I915_WRITE(DSB_TAIL(pipe, dsb->id), i915_ggtt_offset(dsb->vma) + tail);

> +
> +       if (is_dsb_busy(dsb)) {
> +               DRM_DEBUG_KMS("TAIL_PTR write failed - dsb engine is busy.\n");
> +               goto reset;
> +       }
> +       DRM_DEBUG_KMS("DSB execution started - buf-size %u, head 0x%x,"
> +                     "tail 0x%x\n", cmd_buf_size, dsb->cmd_buf_head,
> +                     cmd_buf_tail);
> +       I915_WRITE(DSB_TAIL_PTR(pipe, dsb->id), cmd_buf_tail);

This looks very suspect. You enable the HW before setting up the cmdbuf,
so what is executing? Is the execution latched on TAIL or the CTL? Is it
latched at all?

> +       if (wait_for(!is_dsb_busy(dsb), 1)) {
> +               DRM_ERROR("Timed out waiting for DSB workload completion.\n");
> +               goto reset;
> +       }
> +
> +reset:
> +       memset(dsb->cmd_buf, 0, DSB_BUF_SIZE);

Again, why?
-Chris