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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,


On 8/22/2019 12:13 AM, Chris Wilson wrote:
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@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx>
---
  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);

Ok.

+
+       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?

Yes, execution is latched with TAIL - it the trigger to start dsb execution.

+       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?

Can be optimized by filling zero in the unused space before DSB_TAIL_PTR. Will do.
Currently have single commit between get and put call.
For multiple commit we can use same buffer, so want to clear the buffer so that can be used for next commit call.

Regards,
Animesh
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux