On 9/12/2019 6:37 PM, Animesh Manna wrote:
On 9/12/2019 6:30 PM, Sharma, Shashank wrote:
On 9/12/2019 6:21 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:
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.
v1: Initial version.
v2: Unused macro removed and cosmetic changes done. (Shashank)
v3: set free_pos to zero in dsb-put() instead dsb-get() and
a cosmetic change. (Shashank)
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 | 30
++++++++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dsb.h | 9 +++++++
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
b/drivers/gpu/drm/i915/display/intel_dsb.c
index 7c1b1574788c..e2c383352145 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -9,6 +9,13 @@
#define DSB_BUF_SIZE (2 * PAGE_SIZE)
+/* DSB opcodes. */
+#define DSB_OPCODE_SHIFT 24
+#define DSB_OPCODE_MMIO_WRITE 0x1
+#define DSB_OPCODE_INDEXED_WRITE 0x9
We are not using this macro here, this should go to the Batch
/INDEXED_WRITE patch.
+#define DSB_BYTE_EN 0xF
+#define DSB_BYTE_EN_SHIFT 20
+
struct intel_dsb *
intel_dsb_get(struct intel_crtc *crtc)
{
@@ -66,5 +73,28 @@ void intel_dsb_put(struct intel_dsb *dsb)
i915_vma_unpin_and_release(&dsb->vma, 0);
mutex_unlock(&i915->drm.struct_mutex);
dsb->cmd_buf = NULL;
+ dsb->free_pos = 0;
+ }
+}
+
I hope this addition of braces are due to diff's adjustment.
+void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
u32 val)
+{
+ struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ u32 *buf = dsb->cmd_buf;
+
+ if (!buf) {
+ I915_WRITE(reg, val);
+ return;
+ }
+
+ if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
+ DRM_DEBUG_KMS("DSB buffer overflow\n");
why shouldn't we do a I915_WRITE(reg, val) here too? This is single
register write, and we can handle this.
That would assume it's okay to directly mmio write this and the
subsequent values, and write the batch already stored in the buffer
afterwards.
This is single write API, so I won't expect this to be called in an
indexed context, also, we have exceeded the buffer size already, so
no subsequent DSB write would be possible anyways. But I can still
see it would be some kind of mess only, so doesn't really matter if
we do this I915_write or not :).
Adding I915_WRITE can be done, but I feel better to break the
functionality where we have buffer overflow and based on that we can
fine tune the buffer size.
If a set of register is targetted to write through DSB then some
writing through MMIO and and rest writing though DSB may not a nice
thing.
So added only debug log to capture the issue.
Regards,
Animesh
Yeah, broken this way or other, better to warn as soon as possible.
With the above macro comment fixed,
Please feel free to use Reviewed-by: Shashank Sharma
<shashank.sharma@xxxxxxxxx>
- Shashank
- Shashank
BR,
Jani.
+ return;
}
+
+ buf[dsb->free_pos++] = val;
+ buf[dsb->free_pos++] = (DSB_OPCODE_MMIO_WRITE <<
DSB_OPCODE_SHIFT) |
+ (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
+ i915_mmio_reg_offset(reg);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h
b/drivers/gpu/drm/i915/display/intel_dsb.h
index 27eb68eb5392..31b87dcfe160 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;
@@ -21,10 +23,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 tail of command buffer.
+ */
+ 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
