On 8/28/2019 12:40 AM, Animesh Manna wrote:
DSB support single register write through opcode 0x1. Generic
api created which accumulate all single register write in a batch
buffer and once DSB is triggered, it will program all the registers
at the same time.
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 | 36 ++++++++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dsb.h | 9 ++++++
2 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 a2845df90573..df288446caeb 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -9,6 +9,20 @@
#define DSB_BUF_SIZE (2 * PAGE_SIZE)
+/* DSB opcodes. */
+#define DSB_OPCODE_SHIFT 24
+#define DSB_OPCODE_NOOP 0x0
+#define DSB_OPCODE_MMIO_WRITE 0x1
+#define DSB_OPCODE_WAIT_FOR_US 0x2
+#define DSB_OPCODE_WAIT_FOR_LINES 0x3
+#define DSB_OPCODE_WAIT_FOR_VBLANK 0x4
+#define DSB_OPCODE_WAIT_FOR_SL_IN 0x5
May be SL_IR (to indicate in range)
+#define DSB_OPCODE_WAIT_FOR_SL_OUT 0x6
May be SL_OOR (out of range) or _OR
+#define DSB_OPCODE_GENERATE_INT 0x7
+#define DSB_OPCODE_INDEXED_WRITE 0x9
+#define DSB_OPCODE_POLL 0xA
+#define DSB_BYTE_EN (0xf << 20)
But its a nitpick but as we are going by shifts for OPCODE, may be would look consistent to create 2 different macros for enable also.
#define DSB_BYTE_EN 0xF
#define DSB_BYTE_EN_SHIFT 20
+
This patch is using only 3 of the defined commands:
DSB_OPCODE_MMIO_WRITE, DSB_OPCODE_SHIFT, DSB_BYTE_EN
All other definitions are unused. Please add them only in the patch
where they are being used.
struct intel_dsb *
intel_dsb_get(struct intel_crtc *crtc)
{
@@ -81,3 +95,25 @@ void intel_dsb_put(struct intel_dsb *dsb)
mutex_unlock(&i915->drm.struct_mutex);
}
}
+
+void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
+{
+ struct intel_crtc *crtc = dsb->crtc;
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ u32 *buf = dsb->cmd_buf;
+
Do we need a HAS_DSB() check here ? Or would it be taken care in the
caller ?
+ if (!buf) {
+ I915_WRITE(reg, val);
We can debate on if this is a good idea to fallback to I915_write() if
DSB is not available. May be we should think about why are we seeing
this condition (!buf), and that can be checked only when I see the caller.
For now, lets assume we will fallback to I915_WRITE();
+ return;
+ }
+
+ if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
+ DRM_DEBUG_KMS("DSB buffer overflow.\n");
Wouldn't it make sense to do a I915_WRITE(reg, val) here also, like above ?
+ return;
+ }
+
+ buf[dsb->free_pos++] = val;
+ buf[dsb->free_pos++] = (DSB_OPCODE_MMIO_WRITE <<
+ DSB_OPCODE_SHIFT) | DSB_BYTE_EN |
+ i915_mmio_reg_offset(reg);
Once the write is done, are we planning to reset this free_pos ? May be
in some upcoming patch in the series ?
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 4a4091cadc1e..1b33ab118640 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -6,6 +6,8 @@
#ifndef _INTEL_DSB_H
#define _INTEL_DSB_H
+#include "i915_reg.h"
+
struct intel_crtc;
struct i915_vma;
@@ -22,10 +24,17 @@ struct intel_dsb {
enum dsb_id id;
u32 *cmd_buf;
struct i915_vma *vma;
+
+ /*
+ * free_pos will point the first free entry position
+ * and help in calculating cmd_buf_tail.
+ */
+ int free_pos;
};
struct intel_dsb *
intel_dsb_get(struct intel_crtc *crtc);
void intel_dsb_put(struct intel_dsb *dsb);
+void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
#endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
