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

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

 




On 9/12/2019 7:09 PM, Jani Nikula wrote:
On Thu, 12 Sep 2019, "Sharma, Shashank" <shashank.sharma@xxxxxxxxx> wrote:
On 9/12/2019 12:41 AM, Animesh Manna wrote:
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.

v1: Initial version.
v2: Optimized code few places. (Chris)
v3: USed DRM_ERROR for dsb head/tail programming failure. (Shashank)

Cc: Imre Deak <imre.deak@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx>
---
   drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
   drivers/gpu/drm/i915/display/intel_dsb.h |  1 +
   drivers/gpu/drm/i915/i915_reg.h          |  2 ++
   3 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 2b0ffc0afb74..eea86afb0583 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -212,3 +212,45 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
   			       (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
   			       i915_mmio_reg_offset(reg);
   }
+
+void intel_dsb_commit(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum pipe pipe = crtc->pipe;
+	u32 tail;
+
+	if (!dsb->free_pos)
+		return;
+
+	if (!intel_dsb_enable_engine(dsb))
+		goto reset;
+
+	if (is_dsb_busy(dsb)) {
+		DRM_ERROR("HEAD_PTR write failed - dsb engine is busy.\n");
+		goto reset;
+	}
+	I915_WRITE(DSB_HEAD(pipe, dsb->id), i915_ggtt_offset(dsb->vma));
+
+	tail = ALIGN(dsb->free_pos * 4, CACHELINE_BYTES);
+	if (tail > dsb->free_pos * 4)
+		memset(&dsb->cmd_buf[dsb->free_pos], 0,
+		       (tail - dsb->free_pos * 4));
+
+	if (is_dsb_busy(dsb)) {
+		DRM_ERROR("TAIL_PTR write failed - dsb engine is busy.\n");
+		goto reset;
+	}
+	DRM_DEBUG_KMS("DSB execution started - head 0x%x, tail 0x%x\n",
+		      i915_ggtt_offset(dsb->vma), tail);
+	I915_WRITE(DSB_TAIL(pipe, dsb->id), i915_ggtt_offset(dsb->vma) + tail);
+	if (wait_for(!is_dsb_busy(dsb), 1)) {
+		DRM_ERROR("Timed out waiting for DSB workload completion.\n");
+		goto reset;
+	}
+
+reset:
+	dsb->free_pos = 0;
+	intel_dsb_disable_engine(dsb);
I am still not very convince if a commit function should be void, I
would still want it to return success or failure, so that we would know
if the last operation was successful or not.

I would wait Jani N to comment here, on what he feels about this.
The question becomes, what do you *do* with the return value? If none of
the callers are going to use it, don't return it.

I was thinking we should check the return value of the DSB commit (if not writes), so that we would be aware that the register programming failed, and later even can think about a fallback method. Too ambitious ?

- Shashank

BR,
Jani.

- Shashank

+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 9b2522f20bfb..7389c8c5b665 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -43,5 +43,6 @@ void intel_dsb_put(struct intel_dsb *dsb);
   void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
   void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
   				 u32 val);
+void intel_dsb_commit(struct intel_dsb *dsb);
#endif
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2dbaa49f5c74..c77b5066d8dd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11687,6 +11687,8 @@ enum skl_power_gate {
   #define _DSBSL_INSTANCE_BASE		0x70B00
   #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
   					 (pipe) * 0x1000 + (id) * 100)
+#define DSB_HEAD(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x0)
+#define DSB_TAIL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x4)
   #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
   #define   DSB_ENABLE			(1 << 31)
   #define   DSB_STATUS			(1 << 0)
_______________________________________________
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