[PATCH v2 00/15] DSB enablement.

[PATCH v2 00/15] DSB enablement.

[Animesh Manna]

Display State Buffer (DSB) is hardware capability which allows driver to batch submit HW programming.

As part of initial enablement common api created which currently used to program gamma lut proramming.

Going forwad DSB support can be added for HDR and flip related operation.

HSDES: 1209978241
BSpec: 32020

v1: Initial version.

v2: Move intel_dsb files under display folder and fixed an issue.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>

Animesh Manna (15):
  drm/i915/dsb: feature flag added for display state buffer.
  drm/i915/dsb: DSB context creation.
  drm/i915/dsb: single register write function for DSB.
  drm/i915/dsb: Added enum for reg write capability.
  drm/i915/dsb: Indexed register write function for DSB.
  drm/i915/dsb: Update i915_write to call dsb-write.
  drm/i915/dsb: Register definition of DSB registers.
  drm/i915/dsb: Check DSB engine status.
  drm/i915/dsb: functions to enable/disable DSB engine.
  drm/i915/dsb: function to trigger workload execution of DSB.
  drm/i915/dsb: function to destroy DSB context.
  drm/i915/dsb: Early prepare of dsb context.
  drm/i915/dsb: Cleanup of DSB context.
  drm/i915/dsb: Documentation for DSB.
  drm/i915/dsb: Enable gamma lut programming using DSB.

 Documentation/gpu/i915.rst                    |   9 +
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/display/intel_color.c    |   4 +
 drivers/gpu/drm/i915/display/intel_display.c  |  27 ++
 .../drm/i915/display/intel_display_types.h    |   6 +
 drivers/gpu/drm/i915/display/intel_dsb.c      | 313 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h      |  48 +++
 drivers/gpu/drm/i915/i915_drv.h               |   9 +-
 drivers/gpu/drm/i915/i915_reg.h               |  53 ++-
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 10 files changed, 461 insertions(+), 10 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.h

-- 
2.22.0

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

[PATCH v2 01/15] drm/i915/dsb: feature flag added for display state buffer.

[Animesh Manna]

From gen12 onwards Display State Buffer(DSB) is hardware capability
added which allows driver to batch submit display HW programming.
Feature flag has_dsb added to identify the driver/platform support
at runtime.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          | 2 ++
 drivers/gpu/drm/i915/intel_device_info.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb31c1656cea..9e03c162a5f8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1850,6 +1850,8 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 	(BUILD_BUG_ON_ZERO(!__builtin_constant_p(n)) + \
 	 INTEL_INFO(dev_priv)->gen == (n))
 
+#define HAS_DSB(dev_priv)	(INTEL_INFO(dev_priv)->display.has_dsb)
+
 /*
  * Return true if revision is in range [since,until] inclusive.
  *
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 92e0c2e0954c..e206f298f48e 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -135,6 +135,7 @@ enum intel_ppgtt_type {
 	func(has_csr); \
 	func(has_ddi); \
 	func(has_dp_mst); \
+	func(has_dsb); \
 	func(has_fbc); \
 	func(has_gmch); \
 	func(has_hotplug); \
-- 
2.22.0

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

[PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Animesh Manna]

The function will internally get the gem buffer from global GTT
which is mapped in cpu domain to feed the data + opcode for DSB engine.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |  1 +
 .../drm/i915/display/intel_display_types.h    |  4 ++
 drivers/gpu/drm/i915/display/intel_dsb.c      | 66 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++++
 drivers/gpu/drm/i915/i915_drv.h               |  1 +
 5 files changed, 103 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 45add812048b..5232b2607822 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -171,6 +171,7 @@ i915-y += \
 	display/intel_display_power.o \
 	display/intel_dpio_phy.o \
 	display/intel_dpll_mgr.o \
+	display/intel_dsb.o \
 	display/intel_fbc.o \
 	display/intel_fifo_underrun.o \
 	display/intel_frontbuffer.o \
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 449abaea619f..e62450a72dc2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1026,6 +1026,10 @@ struct intel_crtc {
 
 	/* scalers available on this crtc */
 	int num_scalers;
+
+	/* per pipe DSB related info */
+	struct intel_dsb dsb[MAX_DSB_PER_PIPE];
+	int dsb_in_use;
 };
 
 struct intel_plane {
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
new file mode 100644
index 000000000000..6cde3af30643
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ */
+
+#include "../i915_drv.h"
+#include "intel_display_types.h"
+
+#define DSB_BUF_SIZE    (2 * PAGE_SIZE)
+
+struct intel_dsb *
+intel_dsb_get(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *i915 = to_i915(dev);
+	struct drm_i915_gem_object *obj;
+	struct i915_vma *vma;
+	struct intel_dsb *dsb;
+	intel_wakeref_t wakeref;
+	int i;
+
+	WARN_ON(crtc->dsb_in_use >= MAX_DSB_PER_PIPE);
+
+	for (i = 0; i < MAX_DSB_PER_PIPE; i++) {
+		if (!crtc->dsb[i].cmd_buf) {
+			dsb = &crtc->dsb[i];
+			dsb->id = i;
+		}
+	}
+
+	dsb = &crtc->dsb[crtc->dsb_in_use];
+	dsb->crtc = crtc;
+	if (!HAS_DSB(i915))
+		return dsb;
+
+	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
+	mutex_lock(&i915->drm.struct_mutex);
+
+	obj = i915_gem_object_create_shmem(i915, DSB_BUF_SIZE);
+	if (IS_ERR(obj))
+		goto err;
+
+	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE);
+	if (IS_ERR(vma)) {
+		DRM_DEBUG_KMS("Vma creation failed.\n");
+		i915_gem_object_put(obj);
+		goto err;
+	}
+
+	dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
+	if (IS_ERR(dsb->cmd_buf)) {
+		DRM_DEBUG_KMS("Command buffer creation failed.\n");
+		dsb->cmd_buf = NULL;
+		goto err;
+	}
+	crtc->dsb_in_use++;
+	dsb->cmd_buf_head = (uintptr_t)i915_ggtt_offset(vma);
+	dsb->vma = vma;
+
+	memset(dsb->cmd_buf, 0, DSB_BUF_SIZE);
+err:
+	mutex_unlock(&i915->drm.struct_mutex);
+	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
+	return dsb;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
new file mode 100644
index 000000000000..50a2a6590a71
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef _INTEL_DSB_H
+#define _INTEL_DSB_H
+
+struct intel_crtc;
+struct i915_vma;
+
+enum dsb_id {
+	INVALID_DSB = -1,
+	DSB1,
+	DSB2,
+	DSB3,
+	MAX_DSB_PER_PIPE
+};
+
+struct intel_dsb {
+	struct intel_crtc *crtc;
+	enum dsb_id id;
+	u32 *cmd_buf;
+	u32 cmd_buf_head;
+	struct i915_vma *vma;
+};
+
+struct intel_dsb *
+intel_dsb_get(struct intel_crtc *crtc);
+
+#endif
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9e03c162a5f8..643fd6d6fd73 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -67,6 +67,7 @@
 #include "display/intel_display.h"
 #include "display/intel_display_power.h"
 #include "display/intel_dpll_mgr.h"
+#include "display/intel_dsb.h"
 #include "display/intel_frontbuffer.h"
 #include "display/intel_gmbus.h"
 #include "display/intel_opregion.h"
-- 
2.22.0

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

[PATCH v2 03/15] drm/i915/dsb: single register write function for DSB.

[Animesh Manna]

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@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 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 6cde3af30643..8a9d082b1601 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
+#define DSB_OPCODE_WAIT_FOR_SL_OUT	0x6
+#define DSB_OPCODE_GENERATE_INT		0x7
+#define DSB_OPCODE_INDEXED_WRITE	0x9
+#define DSB_OPCODE_POLL			0xA
+#define DSB_BYTE_EN			(0xf << 20)
+
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
 {
@@ -64,3 +78,25 @@ intel_dsb_get(struct intel_crtc *crtc)
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 	return dsb;
 }
+
+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;
+
+	if (!buf) {
+		I915_WRITE(reg, val);
+		return;
+	}
+
+	if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
+		DRM_DEBUG_KMS("DSB buffer overflow.\n");
+		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);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 50a2a6590a71..2015c372b0d5 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;
 
@@ -23,9 +25,16 @@ struct intel_dsb {
 	u32 *cmd_buf;
 	u32 cmd_buf_head;
 	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_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
 
 #endif
-- 
2.22.0

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

[PATCH v2 04/15] drm/i915/dsb: Added enum for reg write capability.

[Animesh Manna]

DSB can access specific register, To identify those register
which can be written through DSB, enum reg_write_cap is added
to hold the capability.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2abd199093c5..c4a17034d4dc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -178,11 +178,22 @@
  */
 #define REG_FIELD_GET(__mask, __val)	((u32)FIELD_GET(__mask, __val))
 
+/*
+ * Added enum to hold the capability for those registers which can be written
+ * through DSB.
+ */
+enum reg_write_cap {
+	MMIO_WRITE,
+	DSB_WRITE,
+	DSB_INDEX_WRITE
+};
+
 typedef struct {
 	u32 reg;
+	enum reg_write_cap cap;
 } i915_reg_t;
 
-#define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
+#define _MMIO(r, ...) ((const i915_reg_t){ .reg = (r), ##__VA_ARGS__})
 
 #define INVALID_MMIO_REG _MMIO(0)
 
-- 
2.22.0

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

[PATCH v2 05/15] drm/i915/dsb: Indexed register write function for DSB.

[Animesh Manna]

DSB can program large set of data through indexed register write
(opcode 0x9) in one shot. Will be using for bulk register programming
e.g. gamma lut programming, HDR meta data programming.

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

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 8a9d082b1601..4fe8cac6246a 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -22,6 +22,7 @@
 #define DSB_OPCODE_INDEXED_WRITE	0x9
 #define DSB_OPCODE_POLL			0xA
 #define DSB_BYTE_EN			(0xf << 20)
+#define DSB_REG_VALUE_MASK		0xfffff
 
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
@@ -79,6 +80,42 @@ intel_dsb_get(struct intel_crtc *crtc)
 	return dsb;
 }
 
+static void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
+					u32 val)
+{
+	u32 *buf = dsb->cmd_buf;
+	u32 reg_val;
+
+	reg_val = buf[dsb->ins_start_offset + 1] & DSB_REG_VALUE_MASK;
+	if (reg_val != i915_mmio_reg_offset(reg)) {
+		/* Every instruction should be 8 byte aligned. */
+		if (dsb->free_pos & 0x1)
+			dsb->free_pos++;
+
+		/* Update the size. */
+		dsb->ins_start_offset = dsb->free_pos;
+		buf[dsb->free_pos++] = 1;
+
+		/* Update the opcode and reg. */
+		buf[dsb->free_pos++] = (DSB_OPCODE_INDEXED_WRITE  <<
+					DSB_OPCODE_SHIFT) |
+					i915_mmio_reg_offset(reg);
+
+		/* Update the value. */
+		buf[dsb->free_pos++] = val;
+	} else {
+		/* Update the new value. */
+		buf[dsb->free_pos++] = val;
+
+		/* Update the size. */
+		buf[dsb->ins_start_offset]++;
+	}
+
+	/* if number of data words is odd, then the last dword should be 0.*/
+	if (dsb->free_pos & 0x1)
+		buf[dsb->free_pos] = 0;
+}
+
 void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
 {
 	struct intel_crtc *crtc = dsb->crtc;
@@ -95,6 +132,11 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
 		return;
 	}
 
+	if (reg.cap == DSB_INDEX_WRITE) {
+		intel_dsb_indexed_reg_write(dsb, reg, val);
+		return;
+	}
+
 	buf[dsb->free_pos++] = val;
 	buf[dsb->free_pos++] = (DSB_OPCODE_MMIO_WRITE  <<
 				DSB_OPCODE_SHIFT) | DSB_BYTE_EN |
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 2015c372b0d5..1fa893cc8c2e 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -31,6 +31,12 @@ struct intel_dsb {
 	 * and help in calculating cmd_buf_tail.
 	 */
 	int free_pos;
+
+	/*
+	 * ins_start_offset will help to store start address
+	 * of the dsb instuction of auto-increment register.
+	 */
+	u32 ins_start_offset;
 };
 
 struct intel_dsb *
-- 
2.22.0

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

[PATCH v2 06/15] drm/i915/dsb: Update i915_write to call dsb-write.

[Animesh Manna]

Existing mmio-reg-write need intel_uncore handle which is part
of dev_priv structure and the same design is followed by
adding dsb handle in dev_priv for programming registers through DSB.

I915_WRITE is modified to check for register capability and call
dsb-reg-write based on its capability.

No changes in I915_READ definition as DSB do not have support to
read any register.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 2 +-
 drivers/gpu/drm/i915/i915_drv.h          | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 4fe8cac6246a..6f1999140085 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -123,7 +123,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
 	u32 *buf = dsb->cmd_buf;
 
 	if (!buf) {
-		I915_WRITE(reg, val);
+		intel_uncore_write(&(dev_priv)->uncore, reg, val);
 		return;
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 643fd6d6fd73..7aed957362c9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1753,6 +1753,8 @@ struct drm_i915_private {
 	/* Mutex to protect the above hdcp component related values. */
 	struct mutex hdcp_comp_mutex;
 
+	struct intel_dsb *dsb;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
@@ -2414,7 +2416,9 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 	intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
 
 #define I915_READ(reg__)	 __I915_REG_OP(read, dev_priv, (reg__))
-#define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
+#define I915_WRITE(reg__, val__) \
+	(reg__.cap) ? intel_dsb_reg_write(dev_priv->dsb, (reg__), (val__)) : \
+	__I915_REG_OP(write, dev_priv, (reg__), (val__))
 
 #define POSTING_READ(reg__)	__I915_REG_OP(posting_read, dev_priv, (reg__))
 
-- 
2.22.0

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

[PATCH v2 07/15] drm/i915/dsb: Register definition of DSB registers.

[Animesh Manna]

Added key register definitions of DSB.

dsb-ctrl register is required to enable dsb-engine.

head-ptr register hold the head of buffer address from where the
execution will start.

Programming tail-ptr register is a trigger point to start execution.

Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c4a17034d4dc..a1a9d09b6420 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11550,4 +11550,19 @@ enum skl_power_gate {
 #define PORT_TX_DFLEXDPCSSS(fia)		_MMIO_FIA((fia), 0x00894)
 #define   DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)		(1 << (tc_port))
 
+/* This register controls the Display State Buffer (DSB) engines. */
+#define _DSBSL_INSTANCE_BASE		0x70B00
+#define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
+					 (pipe) * 0x1000 + (id) * 100)
+#define DSB_HEAD_PTR(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x0)
+#define DSB_TAIL_PTR(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_BUFFER_REITERATE		(1 << 29)
+#define   DSB_WAIT_FOR_VBLANK		(1 << 28)
+#define   DSB_WAIT_FOR_LINE_IN_RANGE	(1 << 27)
+#define   DSB_HALT			(1 << 16)
+#define   DSB_NON_POSTED_ENABLE		(1 << 8)
+#define   DSB_STATUS			(1 << 0)
+
 #endif /* _I915_REG_H_ */
-- 
2.22.0

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

[PATCH v2 08/15] drm/i915/dsb: Check DSB engine status.

[Animesh Manna]

As per bspec check for DSB status before programming any
of its register. Inline function added to check the dsb status.

Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 6f1999140085..4a38277dc4b1 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -24,6 +24,15 @@
 #define DSB_BYTE_EN			(0xf << 20)
 #define DSB_REG_VALUE_MASK		0xfffff
 
+static inline bool is_dsb_busy(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = dsb->crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+
+	return DSB_STATUS & I915_READ(DSB_CTRL(pipe, dsb->id));
+}
+
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
 {
-- 
2.22.0

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

[PATCH v2 09/15] drm/i915/dsb: functions to enable/disable DSB engine.

[Animesh Manna]

DSB will be used for performance improvement for some special scenario.
DSB engine will be enabled based on need and after completion of its work
will be disabled. Api added for enable/disable operation by using DSB_CTRL
register.

Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 40 ++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 4a38277dc4b1..f97d0c06a049 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -33,6 +33,46 @@ static inline bool is_dsb_busy(struct intel_dsb *dsb)
 	return DSB_STATUS & I915_READ(DSB_CTRL(pipe, dsb->id));
 }
 
+static bool intel_dsb_enable_engine(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = dsb->crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	u32 dsb_ctrl;
+
+	dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
+
+	if (DSB_STATUS & dsb_ctrl) {
+		DRM_DEBUG_KMS("DSB engine is busy.\n");
+		return false;
+	}
+
+	dsb_ctrl |= DSB_ENABLE;
+	I915_WRITE(DSB_CTRL(pipe, dsb->id), dsb_ctrl);
+
+	return true;
+}
+
+static bool intel_dsb_disable_engine(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = dsb->crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	u32 dsb_ctrl;
+
+	dsb_ctrl = I915_READ(DSB_CTRL(pipe, dsb->id));
+
+	if (DSB_STATUS & dsb_ctrl) {
+		DRM_DEBUG_KMS("DSB engine is busy.\n");
+		return false;
+	}
+
+	dsb_ctrl &= ~DSB_ENABLE;
+	I915_WRITE(DSB_CTRL(pipe, dsb->id), dsb_ctrl);
+
+	return true;
+}
+
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
 {
-- 
2.22.0

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

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

[Animesh Manna]

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@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@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);
+
+	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);
+	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);
+	dsb->free_pos = 0;
+	intel_dsb_disable_engine(dsb);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 1fa893cc8c2e..7330add3c96f 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -42,5 +42,6 @@ struct intel_dsb {
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc);
 void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
+void intel_dsb_commit(struct intel_dsb *dsb);
 
 #endif
-- 
2.22.0

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

[PATCH v2 11/15] drm/i915/dsb: function to destroy DSB context.

[Animesh Manna]

Freed the gem object after completion of dsb workload.

Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 7e0a9b37f702..bfb138952f61 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -234,3 +234,26 @@ void intel_dsb_commit(struct intel_dsb *dsb)
 	dsb->free_pos = 0;
 	intel_dsb_disable_engine(dsb);
 }
+
+void intel_dsb_put(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc;
+	struct drm_i915_private *i915;
+	struct i915_vma *vma;
+
+	if (!dsb)
+		return;
+
+	crtc = dsb->crtc;
+	i915 = to_i915(crtc->base.dev);
+
+	if (dsb->cmd_buf) {
+		vma = dsb->vma;
+		mutex_lock(&i915->drm.struct_mutex);
+		crtc->dsb_in_use--;
+		i915_gem_object_unpin_map(vma->obj);
+		i915_vma_unpin_and_release(&vma, 0);
+		dsb->cmd_buf = NULL;
+		mutex_unlock(&i915->drm.struct_mutex);
+	}
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 7330add3c96f..7b94fd9bc067 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -43,5 +43,6 @@ struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc);
 void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
 void intel_dsb_commit(struct intel_dsb *dsb);
+void intel_dsb_put(struct intel_dsb *dsb);
 
 #endif
-- 
2.22.0

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

[PATCH v2 12/15] drm/i915/dsb: Early prepare of dsb context.

[Animesh Manna]

The dsb get call added part of the prepare so that we don't
have things that can fail in the commit proper.

The allocated dsb-context will be tracked under intel_crtc_state
instead of intel_crtc per atomic-commit.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c       | 11 +++++++++++
 drivers/gpu/drm/i915/display/intel_display_types.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b51d1ceb8739..c6a5f38bdc87 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13833,6 +13833,16 @@ static void skl_update_crtcs(struct intel_atomic_state *state)
 		icl_dbuf_slices_update(dev_priv, required_slices);
 }
 
+static void intel_prepare_dsb(struct intel_atomic_state *state)
+{
+	struct intel_crtc_state *config;
+	struct intel_crtc *crtc;
+	int i;
+
+	for_each_new_intel_crtc_in_state(state, crtc, config, i)
+		config->dsb = intel_dsb_get(crtc);
+}
+
 static void intel_atomic_helper_free_state(struct drm_i915_private *dev_priv)
 {
 	struct intel_atomic_state *state, *next;
@@ -14185,6 +14195,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 	dev_priv->wm.distrust_bios_wm = false;
 	intel_shared_dpll_swap_state(state);
 	intel_atomic_track_fbs(state);
+	intel_prepare_dsb(state);
 
 	if (state->modeset) {
 		memcpy(dev_priv->min_cdclk, state->min_cdclk,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index e62450a72dc2..0eee65b2eb1c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -984,6 +984,8 @@ struct intel_crtc_state {
 
 	/* Forward Error correction State */
 	bool fec_enable;
+
+	struct intel_dsb *dsb;
 };
 
 struct intel_crtc {
-- 
2.22.0

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

[PATCH v2 13/15] drm/i915/dsb: Cleanup of DSB context.

[Animesh Manna]

DSB context destroyed using intel_dsb_put() in cleanup function.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index c6a5f38bdc87..7415c4cb8eee 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13843,6 +13843,20 @@ static void intel_prepare_dsb(struct intel_atomic_state *state)
 		config->dsb = intel_dsb_get(crtc);
 }
 
+static void intel_cleanup_dsb(struct intel_atomic_state *state)
+{
+	struct intel_crtc_state *config;
+	struct intel_crtc *crtc;
+	int i;
+
+	for_each_new_intel_crtc_in_state(state, crtc, config, i) {
+		if (config->dsb) {
+			intel_dsb_put(config->dsb);
+			config->dsb = NULL;
+		}
+	}
+}
+
 static void intel_atomic_helper_free_state(struct drm_i915_private *dev_priv)
 {
 	struct intel_atomic_state *state, *next;
@@ -14061,6 +14075,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 
 	drm_atomic_helper_commit_hw_done(&state->base);
 
+	intel_cleanup_dsb(state);
+
 	if (state->modeset) {
 		/* As one of the primary mmio accessors, KMS has a high
 		 * likelihood of triggering bugs in unclaimed access. After we
-- 
2.22.0

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

[PATCH v2 14/15] drm/i915/dsb: Documentation for DSB.

[Animesh Manna]

Added docbook info regarding Display State Buffer(DSB) which
is added from gen12 onwards to batch submit display HW programming.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 Documentation/gpu/i915.rst               |  9 ++++
 drivers/gpu/drm/i915/display/intel_dsb.c | 54 ++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 3415255ad3dc..38e31223a24c 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -246,6 +246,15 @@ Display PLLs
 .. kernel-doc:: drivers/gpu/drm/i915/display/intel_dpll_mgr.h
    :internal:
 
+Display State Buffer
+--------------------
+
+.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dsb.c
+   :doc: DSB
+
+.. kernel-doc:: drivers/gpu/drm/i915/display/intel_dsb.c
+   :internal:
+
 Memory Management and Command Submission
 ========================================
 
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index bfb138952f61..259b89273071 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -9,6 +9,23 @@
 
 #define DSB_BUF_SIZE    (2 * PAGE_SIZE)
 
+/**
+ * DOC: DSB
+ *
+ * A DSB (Display State Buffer) is a queue of MMIO instructions in the memory
+ * which can be offloaded to DSB HW in Display Controller. DSB HW is a DMA
+ * engine that can be programmed to download the DSB from memory.
+ * It allows driver to batch submit display HW programming. This helps to
+ * reduce loading time and CPU activity, thereby making the context switch
+ * faster. DSB Support added from Gen12 Intel graphics based platform.
+ *
+ * DSB's can access only the pipe, plane, and transcoder Data Island Packet
+ * registers.
+ *
+ * DSB HW can support only register writes (both indexed and direct MMIO
+ * writes). There are no registers reads possible with DSB HW engine.
+ */
+
 /* DSB opcodes. */
 #define DSB_OPCODE_SHIFT		24
 #define DSB_OPCODE_NOOP			0x0
@@ -73,6 +90,17 @@ static bool intel_dsb_disable_engine(struct intel_dsb *dsb)
 	return true;
 }
 
+/**
+ * intel_dsb_get() - Allocate dsb context and return a dsb instance.
+ * @crtc: intel_crtc structure to get pipe info.
+ *
+ * This function will give handle of the DSB instance which
+ * user want to operate on.
+ *
+ * Return : address of Intel_dsb instance requested for.
+ * In failure case, the dsb instance will not have any command buffer.
+ */
+
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
 {
@@ -165,6 +193,18 @@ static void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
 		buf[dsb->free_pos] = 0;
 }
 
+/**
+ * intel_dsb_reg_write() -Write to the dsb context for normal
+ * register.
+ * @dsb: intel_dsb structure.
+ * @reg: register address.
+ * @val: value.
+ *
+ * This function is used for writing register-value pair in command
+ * buffer of DSB. During command buffer overflow, a warning
+ * is thrown and rest all erroneous condition register programming is done
+ * through mmio write.
+ */
 void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
 {
 	struct intel_crtc *crtc = dsb->crtc;
@@ -192,6 +232,13 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
 				i915_mmio_reg_offset(reg);
 }
 
+/**
+ * intel_dsb_commit() - Trigger workload execution of DSB.
+ * @dsb: intel_dsb structure.
+ *
+ * This function is used to do actual write to hardware using DSB.
+ * On errors, fall back to MMIO. Also this function help to reset the context.
+ */
 void intel_dsb_commit(struct intel_dsb *dsb)
 {
 	struct intel_crtc *crtc = dsb->crtc;
@@ -235,6 +282,13 @@ void intel_dsb_commit(struct intel_dsb *dsb)
 	intel_dsb_disable_engine(dsb);
 }
 
+/**
+ * intel_dsb_put() - To destroy DSB context.
+ * @dsb: intel_dsb structure.
+ *
+ * This function is used to destroy the dsb-context by doing unpin
+ * and release the vma object.
+ */
 void intel_dsb_put(struct intel_dsb *dsb)
 {
 	struct intel_crtc *crtc;
-- 
2.22.0

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

[PATCH v2 15/15] drm/i915/dsb: Enable gamma lut programming using DSB.

[Animesh Manna]

Gamma lut programming can be programmed using DSB
where bulk register programming can be done using indexed
register write which takes number of data and the mmio offset
to be written.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c |  4 ++++
 drivers/gpu/drm/i915/i915_reg.h            | 25 +++++++++++++++-------
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 71a0201437a9..866e9306f4c9 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -882,6 +882,7 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
 	if (crtc_state->base.degamma_lut)
 		glk_load_degamma_lut(crtc_state);
@@ -900,6 +901,8 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc);
 	}
+
+	intel_dsb_commit(dev_priv->dsb);
 }
 
 static u32 chv_cgm_degamma_ldw(const struct drm_color_lut *color)
@@ -980,6 +983,7 @@ void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
 
+	dev_priv->dsb = crtc_state->dsb;
 	dev_priv->display.load_luts(crtc_state);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a1a9d09b6420..4d70ae6a6a2d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -242,7 +242,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define _PORT(port, a, b)		_PICK_EVEN(port, a, b)
 #define _PLL(pll, a, b)			_PICK_EVEN(pll, a, b)
 
-#define _MMIO_PIPE(pipe, a, b)		_MMIO(_PIPE(pipe, a, b))
+#define _MMIO_PIPE(pipe, a, b, ...)	_MMIO(_PIPE(pipe, a, b), ##__VA_ARGS__)
 #define _MMIO_PLANE(plane, a, b)	_MMIO(_PLANE(plane, a, b))
 #define _MMIO_TRANS(tran, a, b)		_MMIO(_TRANS(tran, a, b))
 #define _MMIO_PORT(port, a, b)		_MMIO(_PORT(port, a, b))
@@ -10246,11 +10246,18 @@ enum skl_power_gate {
 #define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
 #define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
 
-#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
-#define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
-#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
-#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
-#define PREC_PAL_EXT2_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT2_GC_MAX_A, _PAL_PREC_EXT2_GC_MAX_B) + (i) * 4)
+#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, \
+						   _PAL_PREC_INDEX_B, \
+						   DSB_WRITE)
+#define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, \
+						   _PAL_PREC_DATA_B, \
+						   DSB_INDEX_WRITE)
+#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4, \
+					      DSB_WRITE)
+#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4, \
+					      DSB_WRITE)
+#define PREC_PAL_EXT2_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT2_GC_MAX_A, _PAL_PREC_EXT2_GC_MAX_B) + (i) * 4, \
+					      DSB_WRITE)
 
 #define _PRE_CSC_GAMC_INDEX_A	0x4A484
 #define _PRE_CSC_GAMC_INDEX_B	0x4AC84
@@ -10274,10 +10281,12 @@ enum skl_power_gate {
 
 #define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
 					_PAL_PREC_MULTI_SEG_INDEX_A, \
-					_PAL_PREC_MULTI_SEG_INDEX_B)
+					_PAL_PREC_MULTI_SEG_INDEX_B, \
+					DSB_WRITE)
 #define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
 					_PAL_PREC_MULTI_SEG_DATA_A, \
-					_PAL_PREC_MULTI_SEG_DATA_B)
+					_PAL_PREC_MULTI_SEG_DATA_B, \
+					DSB_INDEX_WRITE)
 
 /* pipe CSC & degamma/gamma LUTs on CHV */
 #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
-- 
2.22.0

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

✗ Fi.CI.CHECKPATCH: warning for DSB enablement. (rev2)

[Patchwork]

== Series Details ==

Series: DSB enablement. (rev2)
URL   : https://patchwork.freedesktop.org/series/63013/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e65049952809 drm/i915/dsb: feature flag added for display state buffer.
ff9965b61f73 drm/i915/dsb: DSB context creation.
-:43: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#43: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 121 lines checked
5e026ff7ced1 drm/i915/dsb: single register write function for DSB.
c2a803f1f6c9 drm/i915/dsb: Added enum for reg write capability.
c432f5814713 drm/i915/dsb: Indexed register write function for DSB.
363815055070 drm/i915/dsb: Update i915_write to call dsb-write.
-:51: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#51: FILE: drivers/gpu/drm/i915/i915_drv.h:2419:
+#define I915_WRITE(reg__, val__) \
+	(reg__.cap) ? intel_dsb_reg_write(dev_priv->dsb, (reg__), (val__)) : \
+	__I915_REG_OP(write, dev_priv, (reg__), (val__))

-:51: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'reg__' - possible side-effects?
#51: FILE: drivers/gpu/drm/i915/i915_drv.h:2419:
+#define I915_WRITE(reg__, val__) \
+	(reg__.cap) ? intel_dsb_reg_write(dev_priv->dsb, (reg__), (val__)) : \
+	__I915_REG_OP(write, dev_priv, (reg__), (val__))

-:51: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'val__' - possible side-effects?
#51: FILE: drivers/gpu/drm/i915/i915_drv.h:2419:
+#define I915_WRITE(reg__, val__) \
+	(reg__.cap) ? intel_dsb_reg_write(dev_priv->dsb, (reg__), (val__)) : \
+	__I915_REG_OP(write, dev_priv, (reg__), (val__))

total: 1 errors, 0 warnings, 2 checks, 26 lines checked
6a23be979f1b drm/i915/dsb: Register definition of DSB registers.
20dcb2d6beeb drm/i915/dsb: Check DSB engine status.
58c392a91a63 drm/i915/dsb: functions to enable/disable DSB engine.
4a715966094e drm/i915/dsb: function to trigger workload execution of DSB.
0607b6bd8f57 drm/i915/dsb: function to destroy DSB context.
16d2ad5f1cf2 drm/i915/dsb: Early prepare of dsb context.
cb19df1870f3 drm/i915/dsb: Cleanup of DSB context.
d88ece297e3e drm/i915/dsb: Documentation for DSB.
5ff614b2aa70 drm/i915/dsb: Enable gamma lut programming using DSB.
-:72: WARNING:LONG_LINE: line over 100 characters
#72: FILE: drivers/gpu/drm/i915/i915_reg.h:10255:
+#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4, \

-:74: WARNING:LONG_LINE: line over 100 characters
#74: FILE: drivers/gpu/drm/i915/i915_reg.h:10257:
+#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4, \

-:76: WARNING:LONG_LINE: line over 100 characters
#76: FILE: drivers/gpu/drm/i915/i915_reg.h:10259:
+#define PREC_PAL_EXT2_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT2_GC_MAX_A, _PAL_PREC_EXT2_GC_MAX_B) + (i) * 4, \

total: 0 errors, 3 warnings, 0 checks, 67 lines checked

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

✗ Fi.CI.SPARSE: warning for DSB enablement. (rev2)

[Patchwork]

== Series Details ==

Series: DSB enablement. (rev2)
URL   : https://patchwork.freedesktop.org/series/63013/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915/dsb: feature flag added for display state buffer.
Okay!

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

✓ Fi.CI.BAT: success for DSB enablement. (rev2)

[Patchwork]

== Series Details ==

Series: DSB enablement. (rev2)
URL   : https://patchwork.freedesktop.org/series/63013/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6750 -> Patchwork_14117
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/

Known issues
------------

  Here are the changes found in Patchwork_14117 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][1] -> [INCOMPLETE][2] ([fdo#107718])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_busy@basic-flip-c:
    - fi-kbl-7500u:       [PASS][3] -> [SKIP][4] ([fdo#109271] / [fdo#109278]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-cml-u2:          [PASS][5] -> [DMESG-WARN][6] ([fdo#102505])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][7] -> [DMESG-WARN][8] ([fdo#102614])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_reset:
    - fi-icl-u2:          [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-icl-u2/igt@i915_selftest@live_reset.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/fi-icl-u2/igt@i915_selftest@live_reset.html

  
#### Warnings ####

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-icl-u2:          [FAIL][11] ([fdo#111407]) -> [DMESG-FAIL][12] ([fdo#110595])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][13] ([fdo#111096]) -> [FAIL][14] ([fdo#111407])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110595]: https://bugs.freedesktop.org/show_bug.cgi?id=110595
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407


Participating hosts (50 -> 42)
------------------------------

  Additional (3): fi-skl-guc fi-apl-guc fi-pnv-d510 
  Missing    (11): fi-kbl-soraka fi-icl-u4 fi-bxt-dsi fi-ilk-m540 fi-hsw-4200u fi-glk-dsi fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6750 -> Patchwork_14117

  CI-20190529: 20190529
  CI_DRM_6750: ba37f74dbdc1e78e70a5a2e5f4ce8d762d06eaa7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14117: 5ff614b2aa70d503bf4c9e280927dc5bd3fb9e07 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5ff614b2aa70 drm/i915/dsb: Enable gamma lut programming using DSB.
d88ece297e3e drm/i915/dsb: Documentation for DSB.
cb19df1870f3 drm/i915/dsb: Cleanup of DSB context.
16d2ad5f1cf2 drm/i915/dsb: Early prepare of dsb context.
0607b6bd8f57 drm/i915/dsb: function to destroy DSB context.
4a715966094e drm/i915/dsb: function to trigger workload execution of DSB.
58c392a91a63 drm/i915/dsb: functions to enable/disable DSB engine.
20dcb2d6beeb drm/i915/dsb: Check DSB engine status.
6a23be979f1b drm/i915/dsb: Register definition of DSB registers.
363815055070 drm/i915/dsb: Update i915_write to call dsb-write.
c432f5814713 drm/i915/dsb: Indexed register write function for DSB.
c2a803f1f6c9 drm/i915/dsb: Added enum for reg write capability.
5e026ff7ced1 drm/i915/dsb: single register write function for DSB.
ff9965b61f73 drm/i915/dsb: DSB context creation.
e65049952809 drm/i915/dsb: feature flag added for display state buffer.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Chris Wilson]

Quoting Animesh Manna (2019-08-21 07:32:22)
> The function will internally get the gem buffer from global GTT
> which is mapped in cpu domain to feed the data + opcode for DSB engine.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |  1 +
>  .../drm/i915/display/intel_display_types.h    |  4 ++
>  drivers/gpu/drm/i915/display/intel_dsb.c      | 66 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++++
>  drivers/gpu/drm/i915/i915_drv.h               |  1 +
>  5 files changed, 103 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 45add812048b..5232b2607822 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -171,6 +171,7 @@ i915-y += \
>         display/intel_display_power.o \
>         display/intel_dpio_phy.o \
>         display/intel_dpll_mgr.o \
> +       display/intel_dsb.o \
>         display/intel_fbc.o \
>         display/intel_fifo_underrun.o \
>         display/intel_frontbuffer.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 449abaea619f..e62450a72dc2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1026,6 +1026,10 @@ struct intel_crtc {
>  
>         /* scalers available on this crtc */
>         int num_scalers;
> +
> +       /* per pipe DSB related info */
> +       struct intel_dsb dsb[MAX_DSB_PER_PIPE];
> +       int dsb_in_use;
>  };
>  
>  struct intel_plane {
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> new file mode 100644
> index 000000000000..6cde3af30643
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + */
> +
> +#include "../i915_drv.h"
> +#include "intel_display_types.h"
> +
> +#define DSB_BUF_SIZE    (2 * PAGE_SIZE)
> +
> +struct intel_dsb *
> +intel_dsb_get(struct intel_crtc *crtc)
> +{
> +       struct drm_device *dev = crtc->base.dev;
> +       struct drm_i915_private *i915 = to_i915(dev);
> +       struct drm_i915_gem_object *obj;
> +       struct i915_vma *vma;
> +       struct intel_dsb *dsb;
> +       intel_wakeref_t wakeref;
> +       int i;
> +
> +       WARN_ON(crtc->dsb_in_use >= MAX_DSB_PER_PIPE);
> +
> +       for (i = 0; i < MAX_DSB_PER_PIPE; i++) {
> +               if (!crtc->dsb[i].cmd_buf) {
> +                       dsb = &crtc->dsb[i];
> +                       dsb->id = i;
> +               }
> +       }

That seems out of place.

> +
> +       dsb = &crtc->dsb[crtc->dsb_in_use];

What lock guards dsb_in_use?

> +       dsb->crtc = crtc;
> +       if (!HAS_DSB(i915))
> +               return dsb;
> +
> +       wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +       mutex_lock(&i915->drm.struct_mutex);
> +
> +       obj = i915_gem_object_create_shmem(i915, DSB_BUF_SIZE);

_internal not shmem, you never need to swap.

> +       if (IS_ERR(obj))
> +               goto err;
> +
> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE);

Only this (currently) still requires struct_mutex

Does it have to mappable? Is that the HW constraint?

> +       if (IS_ERR(vma)) {
> +               DRM_DEBUG_KMS("Vma creation failed.\n");
> +               i915_gem_object_put(obj);
> +               goto err;
> +       }
> +
> +       dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);

Are you sure you WC? You spend more time reading it back than writing.

> +       if (IS_ERR(dsb->cmd_buf)) {
> +               DRM_DEBUG_KMS("Command buffer creation failed.\n");
> +               dsb->cmd_buf = NULL;
> +               goto err;
> +       }
> +       crtc->dsb_in_use++;
> +       dsb->cmd_buf_head = (uintptr_t)i915_ggtt_offset(vma);

Pardon? Do not even pretend an offset into the global virtual address of
the GPU is a CPU pointer. Just don't bother, you store the vma, so you
already have the offset stored.

> +       dsb->vma = vma;
> +
> +       memset(dsb->cmd_buf, 0, DSB_BUF_SIZE);

That's overkill.

> +err:
> +       mutex_unlock(&i915->drm.struct_mutex);
> +       intel_runtime_pm_put(&i915->runtime_pm, wakeref);

So what did you do here that required a wakeref?

> +       return dsb;
> +}

Include the complimentary teardown routine. That should be part of the
API you are presenting.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/15] drm/i915/dsb: Indexed register write function for DSB.

[Chris Wilson]

Quoting Animesh Manna (2019-08-21 07:32:25)
> DSB can program large set of data through indexed register write
> (opcode 0x9) in one shot. Will be using for bulk register programming
> e.g. gamma lut programming, HDR meta data programming.
> 
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dsb.h |  6 ++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 8a9d082b1601..4fe8cac6246a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -22,6 +22,7 @@
>  #define DSB_OPCODE_INDEXED_WRITE       0x9
>  #define DSB_OPCODE_POLL                        0xA
>  #define DSB_BYTE_EN                    (0xf << 20)
> +#define DSB_REG_VALUE_MASK             0xfffff
>  
>  struct intel_dsb *
>  intel_dsb_get(struct intel_crtc *crtc)
> @@ -79,6 +80,42 @@ intel_dsb_get(struct intel_crtc *crtc)
>         return dsb;
>  }
>  
> +static void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
> +                                       u32 val)
> +{
> +       u32 *buf = dsb->cmd_buf;
> +       u32 reg_val;
> +
> +       reg_val = buf[dsb->ins_start_offset + 1] & DSB_REG_VALUE_MASK;

Uncached read.

> +       if (reg_val != i915_mmio_reg_offset(reg)) {
> +               /* Every instruction should be 8 byte aligned. */
> +               if (dsb->free_pos & 0x1)
> +                       dsb->free_pos++;

dsb->free_pos = ALIGN(dsb->free_pos, 2);

> +
> +               /* Update the size. */
> +               dsb->ins_start_offset = dsb->free_pos;
> +               buf[dsb->free_pos++] = 1;
> +
> +               /* Update the opcode and reg. */
> +               buf[dsb->free_pos++] = (DSB_OPCODE_INDEXED_WRITE  <<
> +                                       DSB_OPCODE_SHIFT) |
> +                                       i915_mmio_reg_offset(reg);
> +
> +               /* Update the value. */
> +               buf[dsb->free_pos++] = val;
> +       } else {
> +               /* Update the new value. */
> +               buf[dsb->free_pos++] = val;
> +
> +               /* Update the size. */
> +               buf[dsb->ins_start_offset]++;

Uncached read and write. So far this is working out to be _more_
expensive than mmio.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/15] drm/i915/dsb: Update i915_write to call dsb-write.

[Chris Wilson]

Quoting Animesh Manna (2019-08-21 07:32:26)
> Existing mmio-reg-write need intel_uncore handle which is part
> of dev_priv structure and the same design is followed by
> adding dsb handle in dev_priv for programming registers through DSB.
> 
> I915_WRITE is modified to check for register capability and call
> dsb-reg-write based on its capability.
> 
> No changes in I915_READ definition as DSB do not have support to
> read any register.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 2 +-
>  drivers/gpu/drm/i915/i915_drv.h          | 6 +++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 4fe8cac6246a..6f1999140085 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -123,7 +123,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
>         u32 *buf = dsb->cmd_buf;
>  
>         if (!buf) {
> -               I915_WRITE(reg, val);
> +               intel_uncore_write(&(dev_priv)->uncore, reg, val);
>                 return;
>         }
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 643fd6d6fd73..7aed957362c9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1753,6 +1753,8 @@ struct drm_i915_private {
>         /* Mutex to protect the above hdcp component related values. */
>         struct mutex hdcp_comp_mutex;
>  
> +       struct intel_dsb *dsb;
> +
>         /*
>          * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>          * will be rejected. Instead look for a better place.
> @@ -2414,7 +2416,9 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>         intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
>  
>  #define I915_READ(reg__)        __I915_REG_OP(read, dev_priv, (reg__))
> -#define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
> +#define I915_WRITE(reg__, val__) \
> +       (reg__.cap) ? intel_dsb_reg_write(dev_priv->dsb, (reg__), (val__)) : \
> +       __I915_REG_OP(write, dev_priv, (reg__), (val__))

Jani, over to you.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 11/15] drm/i915/dsb: function to destroy DSB context.

[Chris Wilson]

Quoting Animesh Manna (2019-08-21 07:32:31)
> Freed the gem object after completion of dsb workload.
> 
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 23 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dsb.h |  1 +
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 7e0a9b37f702..bfb138952f61 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -234,3 +234,26 @@ void intel_dsb_commit(struct intel_dsb *dsb)
>         dsb->free_pos = 0;
>         intel_dsb_disable_engine(dsb);
>  }
> +
> +void intel_dsb_put(struct intel_dsb *dsb)
> +{
> +       struct intel_crtc *crtc;
> +       struct drm_i915_private *i915;
> +       struct i915_vma *vma;
> +
> +       if (!dsb)
> +               return;
> +
> +       crtc = dsb->crtc;
> +       i915 = to_i915(crtc->base.dev);
> +
> +       if (dsb->cmd_buf) {
> +               vma = dsb->vma;
> +               mutex_lock(&i915->drm.struct_mutex);
> +               crtc->dsb_in_use--;

That is complete garbage. This dsb just happens to be crtc->dsb_in_use?

> +               i915_gem_object_unpin_map(vma->obj);
> +               i915_vma_unpin_and_release(&vma, 0);

i915_vma_unpin_and_release(vma, I915_VMA_RELEASE_MAP);

Does not require struct_mutex.

Put this with the allocator.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for DSB enablement. (rev2)

[Patchwork]

== Series Details ==

Series: DSB enablement. (rev2)
URL   : https://patchwork.freedesktop.org/series/63013/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6750_full -> Patchwork_14117_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14117_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14117_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14117_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-glk3/igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-glk8/igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@cursor-vs-flip-toggle:
    - shard-skl:          [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl4/igt@kms_cursor_legacy@cursor-vs-flip-toggle.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl4/igt@kms_cursor_legacy@cursor-vs-flip-toggle.html

  * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl4/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl7/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html

  
Known issues
------------

  Here are the changes found in Patchwork_14117_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#110841])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#111325]) +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@gem_exec_async@concurrent-writes-bsd.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb2/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([fdo#102670])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
    - shard-apl:          [PASS][13] -> [INCOMPLETE][14] ([fdo#103927])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl6/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-apl2/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
    - shard-kbl:          [PASS][15] -> [INCOMPLETE][16] ([fdo#103665])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-kbl2/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-kbl2/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [PASS][17] -> [INCOMPLETE][18] ([fdo#103540])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-hsw8/igt@kms_flip@flip-vs-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-hsw4/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         [PASS][19] -> [FAIL][20] ([fdo#103167]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-skl:          [PASS][21] -> [INCOMPLETE][22] ([fdo#104108])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][23] -> [DMESG-WARN][24] ([fdo#108566]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-apl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145] / [fdo#110403])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109642] / [fdo#111068])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb3/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109441]) +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb5/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109276]) +22 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@prime_busy@hang-bsd2.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb7/igt@prime_busy@hang-bsd2.html

  * igt@tools_test@tools_test:
    - shard-kbl:          [PASS][33] -> [SKIP][34] ([fdo#109271])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-kbl2/igt@tools_test@tools_test.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-kbl1/igt@tools_test@tools_test.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         [SKIP][35] ([fdo#109276]) -> [PASS][36] +14 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@gem_ctx_isolation@vcs1-dirty-create.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb2/igt@gem_ctx_isolation@vcs1-dirty-create.html

  * igt@gem_eio@reset-stress:
    - shard-skl:          [FAIL][37] ([fdo#109661]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@gem_eio@reset-stress.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl1/igt@gem_eio@reset-stress.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [SKIP][39] ([fdo#111325]) -> [PASS][40] +9 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb3/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@i915_pm_rps@min-max-config-loaded:
    - shard-iclb:         [FAIL][41] ([fdo#111409]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@i915_pm_rps@min-max-config-loaded.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb4/igt@i915_pm_rps@min-max-config-loaded.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [DMESG-WARN][43] ([fdo#108566]) -> [PASS][44] +6 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl7/igt@i915_suspend@sysfs-reader.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-apl8/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding:
    - shard-iclb:         [INCOMPLETE][45] ([fdo#107713]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb1/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html

  * igt@kms_cursor_legacy@pipe-c-single-move:
    - shard-apl:          [INCOMPLETE][47] ([fdo#103927]) -> [PASS][48] +2 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl1/igt@kms_cursor_legacy@pipe-c-single-move.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-apl2/igt@kms_cursor_legacy@pipe-c-single-move.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [INCOMPLETE][49] ([fdo#104108]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@kms_fbcon_fbt@psr-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl6/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [FAIL][51] ([fdo#103167]) -> [PASS][52] +3 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-blt:
    - shard-skl:          [FAIL][53] ([fdo#103167]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl1/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-blt.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl6/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][55] ([fdo#108145]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][57] ([fdo#103166]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [SKIP][59] ([fdo#109441]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_psr@psr2_sprite_render.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb2/igt@kms_psr@psr2_sprite_render.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][61] ([fdo#99912]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl5/igt@kms_setmode@basic.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-apl5/igt@kms_setmode@basic.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][63] ([fdo#111329]) -> [SKIP][64] ([fdo#109276])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb8/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][65] ([fdo#111330]) -> [SKIP][66] ([fdo#109276]) +2 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/shard-iclb5/igt@gem_mocs_settings@mocs-reset-bsd2.html

  
  [fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111409]: https://bugs.freedesktop.org/show_bug.cgi?id=111409
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6750 -> Patchwork_14117

  CI-20190529: 20190529
  CI_DRM_6750: ba37f74dbdc1e78e70a5a2e5f4ce8d762d06eaa7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14117: 5ff614b2aa70d503bf4c9e280927dc5bd3fb9e07 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14117/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Animesh Manna]

Hi,


On 8/21/2019 11:41 PM, Chris Wilson wrote:
> Quoting Animesh Manna (2019-08-21 07:32:22)
>> The function will internally get the gem buffer from global GTT
>> which is mapped in cpu domain to feed the data + opcode for DSB engine.
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/Makefile                 |  1 +
>>   .../drm/i915/display/intel_display_types.h    |  4 ++
>>   drivers/gpu/drm/i915/display/intel_dsb.c      | 66 +++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++++
>>   drivers/gpu/drm/i915/i915_drv.h               |  1 +
>>   5 files changed, 103 insertions(+)
>>   create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.c
>>   create mode 100644 drivers/gpu/drm/i915/display/intel_dsb.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 45add812048b..5232b2607822 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -171,6 +171,7 @@ i915-y += \
>>          display/intel_display_power.o \
>>          display/intel_dpio_phy.o \
>>          display/intel_dpll_mgr.o \
>> +       display/intel_dsb.o \
>>          display/intel_fbc.o \
>>          display/intel_fifo_underrun.o \
>>          display/intel_frontbuffer.o \
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 449abaea619f..e62450a72dc2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1026,6 +1026,10 @@ struct intel_crtc {
>>   
>>          /* scalers available on this crtc */
>>          int num_scalers;
>> +
>> +       /* per pipe DSB related info */
>> +       struct intel_dsb dsb[MAX_DSB_PER_PIPE];
>> +       int dsb_in_use;
>>   };
>>   
>>   struct intel_plane {
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
>> new file mode 100644
>> index 000000000000..6cde3af30643
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -0,0 +1,66 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2019 Intel Corporation
>> + *
>> + */
>> +
>> +#include "../i915_drv.h"
>> +#include "intel_display_types.h"
>> +
>> +#define DSB_BUF_SIZE    (2 * PAGE_SIZE)
>> +
>> +struct intel_dsb *
>> +intel_dsb_get(struct intel_crtc *crtc)
>> +{
>> +       struct drm_device *dev = crtc->base.dev;
>> +       struct drm_i915_private *i915 = to_i915(dev);
>> +       struct drm_i915_gem_object *obj;
>> +       struct i915_vma *vma;
>> +       struct intel_dsb *dsb;
>> +       intel_wakeref_t wakeref;
>> +       int i;
>> +
>> +       WARN_ON(crtc->dsb_in_use >= MAX_DSB_PER_PIPE);
>> +
>> +       for (i = 0; i < MAX_DSB_PER_PIPE; i++) {
>> +               if (!crtc->dsb[i].cmd_buf) {
>> +                       dsb = &crtc->dsb[i];
>> +                       dsb->id = i;
>> +               }
>> +       }
> That seems out of place.

Yes, will fix it.
>
>> +
>> +       dsb = &crtc->dsb[crtc->dsb_in_use];
> What lock guards dsb_in_use?

Thanks for catching, need to add mutex_lock.
>
>> +       dsb->crtc = crtc;
>> +       if (!HAS_DSB(i915))
>> +               return dsb;
>> +
>> +       wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>> +       mutex_lock(&i915->drm.struct_mutex);
>> +
>> +       obj = i915_gem_object_create_shmem(i915, DSB_BUF_SIZE);
> _internal not shmem, you never need to swap.

Will do.
>
>> +       if (IS_ERR(obj))
>> +               goto err;
>> +
>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE);
> Only this (currently) still requires struct_mutex

Sure will add.
>
> Does it have to mappable? Is that the HW constraint?

Yes, as per HW design need a cpu mapped buffer to write opcode+data from 
driver.
>
>> +       if (IS_ERR(vma)) {
>> +               DRM_DEBUG_KMS("Vma creation failed.\n");
>> +               i915_gem_object_put(obj);
>> +               goto err;
>> +       }
>> +
>> +       dsb->cmd_buf = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
> Are you sure you WC? You spend more time reading it back than writing.

DSB functionality is focused about buffer creation with opcode + data 
and execute it.
Currently not reading the buffer back.  Please suggest if need any 
improvement here.
>
>> +       if (IS_ERR(dsb->cmd_buf)) {
>> +               DRM_DEBUG_KMS("Command buffer creation failed.\n");
>> +               dsb->cmd_buf = NULL;
>> +               goto err;
>> +       }
>> +       crtc->dsb_in_use++;
>> +       dsb->cmd_buf_head = (uintptr_t)i915_ggtt_offset(vma);
> Pardon? Do not even pretend an offset into the global virtual address of
> the GPU is a CPU pointer. Just don't bother, you store the vma, so you
> already have the offset stored.

Got it, may not need cmd_buf_head separately. Will fix it.
>
>> +       dsb->vma = vma;
>> +
>> +       memset(dsb->cmd_buf, 0, DSB_BUF_SIZE);
> That's overkill.

ok, will remove, as DSB_TAIL_PTR is CACHELINE aligned so will fill the 
unused space with zero in intel_dsb_commit().
>
>> +err:
>> +       mutex_unlock(&i915->drm.struct_mutex);
>> +       intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> So what did you do here that required a wakeref?

while referring some existing code of getting page from global gtt, I 
assumed that wakeref maybe needed.
If I am wrong, please let me know, will correct.
>
>> +       return dsb;
>> +}
> Include the complimentary teardown routine. That should be part of the
> API you are presenting.

Sure I can add intel_dsb_put function in this patch.
Thanks Chris for code review.

Regards,
Animesh
> -Chris

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

Re: [PATCH v2 05/15] drm/i915/dsb: Indexed register write function for DSB.

[Animesh Manna]

Hi,


On 8/21/2019 11:57 PM, Chris Wilson wrote:
> Quoting Animesh Manna (2019-08-21 07:32:25)
>> DSB can program large set of data through indexed register write
>> (opcode 0x9) in one shot. Will be using for bulk register programming
>> e.g. gamma lut programming, HDR meta data programming.
>>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dsb.h |  6 ++++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index 8a9d082b1601..4fe8cac6246a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -22,6 +22,7 @@
>>   #define DSB_OPCODE_INDEXED_WRITE       0x9
>>   #define DSB_OPCODE_POLL                        0xA
>>   #define DSB_BYTE_EN                    (0xf << 20)
>> +#define DSB_REG_VALUE_MASK             0xfffff
>>   
>>   struct intel_dsb *
>>   intel_dsb_get(struct intel_crtc *crtc)
>> @@ -79,6 +80,42 @@ intel_dsb_get(struct intel_crtc *crtc)
>>          return dsb;
>>   }
>>   
>> +static void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
>> +                                       u32 val)
>> +{
>> +       u32 *buf = dsb->cmd_buf;
>> +       u32 reg_val;
>> +
>> +       reg_val = buf[dsb->ins_start_offset + 1] & DSB_REG_VALUE_MASK;
> Uncached read.
>
>> +       if (reg_val != i915_mmio_reg_offset(reg)) {
>> +               /* Every instruction should be 8 byte aligned. */
>> +               if (dsb->free_pos & 0x1)
>> +                       dsb->free_pos++;
> dsb->free_pos = ALIGN(dsb->free_pos, 2);

Ok.
>
>> +
>> +               /* Update the size. */
>> +               dsb->ins_start_offset = dsb->free_pos;
>> +               buf[dsb->free_pos++] = 1;
>> +
>> +               /* Update the opcode and reg. */
>> +               buf[dsb->free_pos++] = (DSB_OPCODE_INDEXED_WRITE  <<
>> +                                       DSB_OPCODE_SHIFT) |
>> +                                       i915_mmio_reg_offset(reg);
>> +
>> +               /* Update the value. */
>> +               buf[dsb->free_pos++] = val;
>> +       } else {
>> +               /* Update the new value. */
>> +               buf[dsb->free_pos++] = val;
>> +
>> +               /* Update the size. */
>> +               buf[dsb->ins_start_offset]++;
> Uncached read and write. So far this is working out to be _more_
> expensive than mmio.

Understood your concern, currently tried to align with I915_WRITE() call 
and coded accordingly.
If we can introduce a separate call for auto-increment register like below.

I915_WRITE_BULK(<reg-offset>, <buf-start-address>, <count>);

The implementation will be simpler and more optimized.
Good to know your feedback or any better way can be done.

Regards,
Animesh
> -Chris

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

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

[Animesh Manna]

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@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@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);

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Chris Wilson]

Quoting Animesh Manna (2019-08-22 13:05:06)
> Hi,
> 
> 
> On 8/21/2019 11:41 PM, Chris Wilson wrote:
> > Quoting Animesh Manna (2019-08-21 07:32:22)
> >> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE);
> > Only this (currently) still requires struct_mutex
> 
> Sure will add.
> >
> > Does it have to mappable? Is that the HW constraint?
> 
> Yes, as per HW design need a cpu mapped buffer to write opcode+data from 
> driver.

PIN_MAPPABLE refers to the iomem aperture portion of the Global GTT (i.e.
the low 64-512MiB). You never use a GGTT mmap for your CPU access, so the
placement should be entirely dictated by the DSB requirements. If you
don't need to be in the low region, don't force it to be, so we have
less congestion for the objects that have to be placed in that region.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 04/15] drm/i915/dsb: Added enum for reg write capability.

[Jani Nikula]

On Wed, 21 Aug 2019, Animesh Manna <animesh.manna@intel.com> wrote:
> DSB can access specific register, To identify those register
> which can be written through DSB, enum reg_write_cap is added
> to hold the capability.

This patch alone increases i915.ko size by about 200k.

Obviously, no.

We've considered making reg a bitfield with 31 bits for offset and 1 bit
to denote display uncore. Might be worthwhile. But I don't think this is
the way to go here.

BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2abd199093c5..c4a17034d4dc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -178,11 +178,22 @@
>   */
>  #define REG_FIELD_GET(__mask, __val)	((u32)FIELD_GET(__mask, __val))
>  
> +/*
> + * Added enum to hold the capability for those registers which can be written
> + * through DSB.
> + */
> +enum reg_write_cap {
> +	MMIO_WRITE,
> +	DSB_WRITE,
> +	DSB_INDEX_WRITE
> +};
> +
>  typedef struct {
>  	u32 reg;
> +	enum reg_write_cap cap;
>  } i915_reg_t;
>  
> -#define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
> +#define _MMIO(r, ...) ((const i915_reg_t){ .reg = (r), ##__VA_ARGS__})
>  
>  #define INVALID_MMIO_REG _MMIO(0)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/15] drm/i915/dsb: Update i915_write to call dsb-write.

[Jani Nikula]

On Wed, 21 Aug 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Animesh Manna (2019-08-21 07:32:26)
>> Existing mmio-reg-write need intel_uncore handle which is part
>> of dev_priv structure and the same design is followed by
>> adding dsb handle in dev_priv for programming registers through DSB.
>> 
>> I915_WRITE is modified to check for register capability and call
>> dsb-reg-write based on its capability.
>> 
>> No changes in I915_READ definition as DSB do not have support to
>> read any register.
>> 
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dsb.c | 2 +-
>>  drivers/gpu/drm/i915/i915_drv.h          | 6 +++++-
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index 4fe8cac6246a..6f1999140085 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -123,7 +123,7 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
>>         u32 *buf = dsb->cmd_buf;
>>  
>>         if (!buf) {
>> -               I915_WRITE(reg, val);
>> +               intel_uncore_write(&(dev_priv)->uncore, reg, val);
>>                 return;
>>         }
>>  
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 643fd6d6fd73..7aed957362c9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1753,6 +1753,8 @@ struct drm_i915_private {
>>         /* Mutex to protect the above hdcp component related values. */
>>         struct mutex hdcp_comp_mutex;
>>  
>> +       struct intel_dsb *dsb;
>> +
>>         /*
>>          * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>>          * will be rejected. Instead look for a better place.
>> @@ -2414,7 +2416,9 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>>         intel_uncore_##op__(&(dev_priv__)->uncore, __VA_ARGS__)
>>  
>>  #define I915_READ(reg__)        __I915_REG_OP(read, dev_priv, (reg__))
>> -#define I915_WRITE(reg__, val__) __I915_REG_OP(write, dev_priv, (reg__), (val__))
>> +#define I915_WRITE(reg__, val__) \
>> +       (reg__.cap) ? intel_dsb_reg_write(dev_priv->dsb, (reg__), (val__)) : \
>> +       __I915_REG_OP(write, dev_priv, (reg__), (val__))
>
> Jani, over to you.

This is not right, in so many levels.

First of all, I don't think we should hide DSB usage in the driver like
this. It needs to be explicit in the code.

I'm not sure if it's even possible to do it like this. When should the
writes take place? Who's going to "commit" the writes to DSB? The DSB
only makes sense for batches. In code with just I915_WRITE, you can't
tell whether the commit is needed or not.

You have a number of DSB engines per pipe, and this forces using just
one in dev_priv.

Please stick to the plan of adding the simplest API that I think we've
agreed on, the one that allows DSB use call sites to switch over to
intel_dsb_reg_write() etc. and allows us to tweak the guts of the DSB
implementation later. Then we don't need to change the DSB use sites so
much, merely the internals.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 15/15] drm/i915/dsb: Enable gamma lut programming using DSB.

[Jani Nikula]

On Wed, 21 Aug 2019, Animesh Manna <animesh.manna@intel.com> wrote:
> Gamma lut programming can be programmed using DSB
> where bulk register programming can be done using indexed
> register write which takes number of data and the mmio offset
> to be written.

No. Please stick to adding and using the relevant intel dsb write
routines for switching areas to using DSB.

BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c |  4 ++++
>  drivers/gpu/drm/i915/i915_reg.h            | 25 +++++++++++++++-------
>  2 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 71a0201437a9..866e9306f4c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -882,6 +882,7 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>  {
>  	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  
>  	if (crtc_state->base.degamma_lut)
>  		glk_load_degamma_lut(crtc_state);
> @@ -900,6 +901,8 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>  		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc);
>  	}
> +
> +	intel_dsb_commit(dev_priv->dsb);
>  }
>  
>  static u32 chv_cgm_degamma_ldw(const struct drm_color_lut *color)
> @@ -980,6 +983,7 @@ void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>  
> +	dev_priv->dsb = crtc_state->dsb;

Cringe.

>  	dev_priv->display.load_luts(crtc_state);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a1a9d09b6420..4d70ae6a6a2d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -242,7 +242,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define _PORT(port, a, b)		_PICK_EVEN(port, a, b)
>  #define _PLL(pll, a, b)			_PICK_EVEN(pll, a, b)
>  
> -#define _MMIO_PIPE(pipe, a, b)		_MMIO(_PIPE(pipe, a, b))
> +#define _MMIO_PIPE(pipe, a, b, ...)	_MMIO(_PIPE(pipe, a, b), ##__VA_ARGS__)
>  #define _MMIO_PLANE(plane, a, b)	_MMIO(_PLANE(plane, a, b))
>  #define _MMIO_TRANS(tran, a, b)		_MMIO(_TRANS(tran, a, b))
>  #define _MMIO_PORT(port, a, b)		_MMIO(_PORT(port, a, b))
> @@ -10246,11 +10246,18 @@ enum skl_power_gate {
>  #define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
>  #define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
>  
> -#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
> -#define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
> -#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
> -#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
> -#define PREC_PAL_EXT2_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT2_GC_MAX_A, _PAL_PREC_EXT2_GC_MAX_B) + (i) * 4)
> +#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, \
> +						   _PAL_PREC_INDEX_B, \
> +						   DSB_WRITE)
> +#define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, \
> +						   _PAL_PREC_DATA_B, \
> +						   DSB_INDEX_WRITE)
> +#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4, \
> +					      DSB_WRITE)
> +#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4, \
> +					      DSB_WRITE)
> +#define PREC_PAL_EXT2_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT2_GC_MAX_A, _PAL_PREC_EXT2_GC_MAX_B) + (i) * 4, \
> +					      DSB_WRITE)
>  
>  #define _PRE_CSC_GAMC_INDEX_A	0x4A484
>  #define _PRE_CSC_GAMC_INDEX_B	0x4AC84
> @@ -10274,10 +10281,12 @@ enum skl_power_gate {
>  
>  #define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
>  					_PAL_PREC_MULTI_SEG_INDEX_A, \
> -					_PAL_PREC_MULTI_SEG_INDEX_B)
> +					_PAL_PREC_MULTI_SEG_INDEX_B, \
> +					DSB_WRITE)
>  #define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
>  					_PAL_PREC_MULTI_SEG_DATA_A, \
> -					_PAL_PREC_MULTI_SEG_DATA_B)
> +					_PAL_PREC_MULTI_SEG_DATA_B, \
> +					DSB_INDEX_WRITE)
>  
>  /* pipe CSC & degamma/gamma LUTs on CHV */
>  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 15/15] drm/i915/dsb: Enable gamma lut programming using DSB.

[Animesh Manna]

On 8/22/2019 6:53 PM, Jani Nikula wrote:
> On Wed, 21 Aug 2019, Animesh Manna <animesh.manna@intel.com> wrote:
>> Gamma lut programming can be programmed using DSB
>> where bulk register programming can be done using indexed
>> register write which takes number of data and the mmio offset
>> to be written.
> No. Please stick to adding and using the relevant intel dsb write
> routines for switching areas to using DSB.

Thanks for review Jani, will reroll a new version soon.

Regards,
Animesh
>
> BR,
> Jani.
>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_color.c |  4 ++++
>>   drivers/gpu/drm/i915/i915_reg.h            | 25 +++++++++++++++-------
>>   2 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>> index 71a0201437a9..866e9306f4c9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -882,6 +882,7 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>>   {
>>   	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>>   	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>   
>>   	if (crtc_state->base.degamma_lut)
>>   		glk_load_degamma_lut(crtc_state);
>> @@ -900,6 +901,8 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>>   		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
>>   		ivb_load_lut_ext_max(crtc);
>>   	}
>> +
>> +	intel_dsb_commit(dev_priv->dsb);
>>   }
>>   
>>   static u32 chv_cgm_degamma_ldw(const struct drm_color_lut *color)
>> @@ -980,6 +983,7 @@ void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>>   
>> +	dev_priv->dsb = crtc_state->dsb;
> Cringe.
>
>>   	dev_priv->display.load_luts(crtc_state);
>>   }
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index a1a9d09b6420..4d70ae6a6a2d 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -242,7 +242,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>>   #define _PORT(port, a, b)		_PICK_EVEN(port, a, b)
>>   #define _PLL(pll, a, b)			_PICK_EVEN(pll, a, b)
>>   
>> -#define _MMIO_PIPE(pipe, a, b)		_MMIO(_PIPE(pipe, a, b))
>> +#define _MMIO_PIPE(pipe, a, b, ...)	_MMIO(_PIPE(pipe, a, b), ##__VA_ARGS__)
>>   #define _MMIO_PLANE(plane, a, b)	_MMIO(_PLANE(plane, a, b))
>>   #define _MMIO_TRANS(tran, a, b)		_MMIO(_TRANS(tran, a, b))
>>   #define _MMIO_PORT(port, a, b)		_MMIO(_PORT(port, a, b))
>> @@ -10246,11 +10246,18 @@ enum skl_power_gate {
>>   #define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
>>   #define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
>>   
>> -#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
>> -#define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
>> -#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
>> -#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
>> -#define PREC_PAL_EXT2_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT2_GC_MAX_A, _PAL_PREC_EXT2_GC_MAX_B) + (i) * 4)
>> +#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, \
>> +						   _PAL_PREC_INDEX_B, \
>> +						   DSB_WRITE)
>> +#define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, \
>> +						   _PAL_PREC_DATA_B, \
>> +						   DSB_INDEX_WRITE)
>> +#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4, \
>> +					      DSB_WRITE)
>> +#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4, \
>> +					      DSB_WRITE)
>> +#define PREC_PAL_EXT2_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT2_GC_MAX_A, _PAL_PREC_EXT2_GC_MAX_B) + (i) * 4, \
>> +					      DSB_WRITE)
>>   
>>   #define _PRE_CSC_GAMC_INDEX_A	0x4A484
>>   #define _PRE_CSC_GAMC_INDEX_B	0x4AC84
>> @@ -10274,10 +10281,12 @@ enum skl_power_gate {
>>   
>>   #define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
>>   					_PAL_PREC_MULTI_SEG_INDEX_A, \
>> -					_PAL_PREC_MULTI_SEG_INDEX_B)
>> +					_PAL_PREC_MULTI_SEG_INDEX_B, \
>> +					DSB_WRITE)
>>   #define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
>>   					_PAL_PREC_MULTI_SEG_DATA_A, \
>> -					_PAL_PREC_MULTI_SEG_DATA_B)
>> +					_PAL_PREC_MULTI_SEG_DATA_B, \
>> +					DSB_INDEX_WRITE)
>>   
>>   /* pipe CSC & degamma/gamma LUTs on CHV */
>>   #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)

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

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Tvrtko Ursulin]

On 22/08/2019 13:09, Chris Wilson wrote:
> Quoting Animesh Manna (2019-08-22 13:05:06)
>> Hi,
>>
>>
>> On 8/21/2019 11:41 PM, Chris Wilson wrote:
>>> Quoting Animesh Manna (2019-08-21 07:32:22)
>>>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE);
>>> Only this (currently) still requires struct_mutex
>>
>> Sure will add.
>>>
>>> Does it have to mappable? Is that the HW constraint?
>>
>> Yes, as per HW design need a cpu mapped buffer to write opcode+data from
>> driver.
> 
> PIN_MAPPABLE refers to the iomem aperture portion of the Global GTT (i.e.
> the low 64-512MiB). You never use a GGTT mmap for your CPU access, so the
> placement should be entirely dictated by the DSB requirements. If you
> don't need to be in the low region, don't force it to be, so we have
> less congestion for the objects that have to be placed in that region.

I was doing a mini audit of what uses the aperture these days and 
noticed this code has been merged in the meantime, but AFAICS this 
question from Chris hasn't been answered? At least not on the mailing 
list. So does it need to be in the aperture region or not?

Regards,

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

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Animesh Manna]

On 10/17/2019 2:05 PM, Tvrtko Ursulin wrote:
>
> On 22/08/2019 13:09, Chris Wilson wrote:
>> Quoting Animesh Manna (2019-08-22 13:05:06)
>>> Hi,
>>>
>>>
>>> On 8/21/2019 11:41 PM, Chris Wilson wrote:
>>>> Quoting Animesh Manna (2019-08-21 07:32:22)
>>>>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 
>>>>> PIN_MAPPABLE);
>>>> Only this (currently) still requires struct_mutex
>>>
>>> Sure will add.
>>>>
>>>> Does it have to mappable? Is that the HW constraint?
>>>
>>> Yes, as per HW design need a cpu mapped buffer to write opcode+data 
>>> from
>>> driver.
>>
>> PIN_MAPPABLE refers to the iomem aperture portion of the Global GTT 
>> (i.e.
>> the low 64-512MiB). You never use a GGTT mmap for your CPU access, so 
>> the
>> placement should be entirely dictated by the DSB requirements. If you
>> don't need to be in the low region, don't force it to be, so we have
>> less congestion for the objects that have to be placed in that region.
>
> I was doing a mini audit of what uses the aperture these days and 
> noticed this code has been merged in the meantime, but AFAICS this 
> question from Chris hasn't been answered? At least not on the mailing 
> list. So does it need to be in the aperture region or not?

Hi,

Based on recommendation from H/w team used PIN_MAPPABLE, not very sure 
about internal details.

Regards,
Animesh
>
> Regards,
>
> Tvrtko

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

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Tvrtko Ursulin]

On 17/10/2019 13:52, Animesh Manna wrote:
> On 10/17/2019 2:05 PM, Tvrtko Ursulin wrote:
>> On 22/08/2019 13:09, Chris Wilson wrote:
>>> Quoting Animesh Manna (2019-08-22 13:05:06)
>>>> Hi,
>>>>
>>>>
>>>> On 8/21/2019 11:41 PM, Chris Wilson wrote:
>>>>> Quoting Animesh Manna (2019-08-21 07:32:22)
>>>>>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 
>>>>>> PIN_MAPPABLE);
>>>>> Only this (currently) still requires struct_mutex
>>>>
>>>> Sure will add.
>>>>>
>>>>> Does it have to mappable? Is that the HW constraint?
>>>>
>>>> Yes, as per HW design need a cpu mapped buffer to write opcode+data 
>>>> from
>>>> driver.
>>>
>>> PIN_MAPPABLE refers to the iomem aperture portion of the Global GTT 
>>> (i.e.
>>> the low 64-512MiB). You never use a GGTT mmap for your CPU access, so 
>>> the
>>> placement should be entirely dictated by the DSB requirements. If you
>>> don't need to be in the low region, don't force it to be, so we have
>>> less congestion for the objects that have to be placed in that region.
>>
>> I was doing a mini audit of what uses the aperture these days and 
>> noticed this code has been merged in the meantime, but AFAICS this 
>> question from Chris hasn't been answered? At least not on the mailing 
>> list. So does it need to be in the aperture region or not?
> 
> Hi,
> 
> Based on recommendation from H/w team used PIN_MAPPABLE, not very sure 
> about internal details.

What did the recommendation exactly say? That it has to be in GGTT or 
aperture?

Regards,

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

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Animesh Manna]

On 10/17/2019 6:39 PM, Tvrtko Ursulin wrote:
>
> On 17/10/2019 13:52, Animesh Manna wrote:
>> On 10/17/2019 2:05 PM, Tvrtko Ursulin wrote:
>>> On 22/08/2019 13:09, Chris Wilson wrote:
>>>> Quoting Animesh Manna (2019-08-22 13:05:06)
>>>>> Hi,
>>>>>
>>>>>
>>>>> On 8/21/2019 11:41 PM, Chris Wilson wrote:
>>>>>> Quoting Animesh Manna (2019-08-21 07:32:22)
>>>>>>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 
>>>>>>> PIN_MAPPABLE);
>>>>>> Only this (currently) still requires struct_mutex
>>>>>
>>>>> Sure will add.
>>>>>>
>>>>>> Does it have to mappable? Is that the HW constraint?
>>>>>
>>>>> Yes, as per HW design need a cpu mapped buffer to write 
>>>>> opcode+data from
>>>>> driver.
>>>>
>>>> PIN_MAPPABLE refers to the iomem aperture portion of the Global GTT 
>>>> (i.e.
>>>> the low 64-512MiB). You never use a GGTT mmap for your CPU access, 
>>>> so the
>>>> placement should be entirely dictated by the DSB requirements. If you
>>>> don't need to be in the low region, don't force it to be, so we have
>>>> less congestion for the objects that have to be placed in that region.
>>>
>>> I was doing a mini audit of what uses the aperture these days and 
>>> noticed this code has been merged in the meantime, but AFAICS this 
>>> question from Chris hasn't been answered? At least not on the 
>>> mailing list. So does it need to be in the aperture region or not?
>>
>> Hi,
>>
>> Based on recommendation from H/w team used PIN_MAPPABLE, not very 
>> sure about internal details.
>
> What did the recommendation exactly say? That it has to be in GGTT or 
> aperture?

It said:
"GMM to allocate buffer from global GTT, get CPU mapped address as well 
(not stolen memory) ... ".

Regards,
Animesh

>
> Regards,
>
> Tvrtko

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

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Tvrtko Ursulin]

On 17/10/2019 14:53, Animesh Manna wrote:
> On 10/17/2019 6:39 PM, Tvrtko Ursulin wrote:
>> On 17/10/2019 13:52, Animesh Manna wrote:
>>> On 10/17/2019 2:05 PM, Tvrtko Ursulin wrote:
>>>> On 22/08/2019 13:09, Chris Wilson wrote:
>>>>> Quoting Animesh Manna (2019-08-22 13:05:06)
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> On 8/21/2019 11:41 PM, Chris Wilson wrote:
>>>>>>> Quoting Animesh Manna (2019-08-21 07:32:22)
>>>>>>>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 
>>>>>>>> PIN_MAPPABLE);
>>>>>>> Only this (currently) still requires struct_mutex
>>>>>>
>>>>>> Sure will add.
>>>>>>>
>>>>>>> Does it have to mappable? Is that the HW constraint?
>>>>>>
>>>>>> Yes, as per HW design need a cpu mapped buffer to write 
>>>>>> opcode+data from
>>>>>> driver.
>>>>>
>>>>> PIN_MAPPABLE refers to the iomem aperture portion of the Global GTT 
>>>>> (i.e.
>>>>> the low 64-512MiB). You never use a GGTT mmap for your CPU access, 
>>>>> so the
>>>>> placement should be entirely dictated by the DSB requirements. If you
>>>>> don't need to be in the low region, don't force it to be, so we have
>>>>> less congestion for the objects that have to be placed in that region.
>>>>
>>>> I was doing a mini audit of what uses the aperture these days and 
>>>> noticed this code has been merged in the meantime, but AFAICS this 
>>>> question from Chris hasn't been answered? At least not on the 
>>>> mailing list. So does it need to be in the aperture region or not?
>>>
>>> Hi,
>>>
>>> Based on recommendation from H/w team used PIN_MAPPABLE, not very 
>>> sure about internal details.
>>
>> What did the recommendation exactly say? That it has to be in GGTT or 
>> aperture?
> 
> It said:
> "GMM to allocate buffer from global GTT, get CPU mapped address as well 
> (not stolen memory) ... ".

So it's possible you don't need PIN_MAPPABLE.

Do we have some test coverage for this? In other words if I send a patch 
which removes it, will we know if the feature is healthy?

Regards,

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

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Animesh Manna]

On 10/17/2019 8:08 PM, Tvrtko Ursulin wrote:
>
> On 17/10/2019 14:53, Animesh Manna wrote:
>> On 10/17/2019 6:39 PM, Tvrtko Ursulin wrote:
>>> On 17/10/2019 13:52, Animesh Manna wrote:
>>>> On 10/17/2019 2:05 PM, Tvrtko Ursulin wrote:
>>>>> On 22/08/2019 13:09, Chris Wilson wrote:
>>>>>> Quoting Animesh Manna (2019-08-22 13:05:06)
>>>>>>> Hi,
>>>>>>>
>>>>>>>
>>>>>>> On 8/21/2019 11:41 PM, Chris Wilson wrote:
>>>>>>>> Quoting Animesh Manna (2019-08-21 07:32:22)
>>>>>>>>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 
>>>>>>>>> PIN_MAPPABLE);
>>>>>>>> Only this (currently) still requires struct_mutex
>>>>>>>
>>>>>>> Sure will add.
>>>>>>>>
>>>>>>>> Does it have to mappable? Is that the HW constraint?
>>>>>>>
>>>>>>> Yes, as per HW design need a cpu mapped buffer to write 
>>>>>>> opcode+data from
>>>>>>> driver.
>>>>>>
>>>>>> PIN_MAPPABLE refers to the iomem aperture portion of the Global 
>>>>>> GTT (i.e.
>>>>>> the low 64-512MiB). You never use a GGTT mmap for your CPU 
>>>>>> access, so the
>>>>>> placement should be entirely dictated by the DSB requirements. If 
>>>>>> you
>>>>>> don't need to be in the low region, don't force it to be, so we have
>>>>>> less congestion for the objects that have to be placed in that 
>>>>>> region.
>>>>>
>>>>> I was doing a mini audit of what uses the aperture these days and 
>>>>> noticed this code has been merged in the meantime, but AFAICS this 
>>>>> question from Chris hasn't been answered? At least not on the 
>>>>> mailing list. So does it need to be in the aperture region or not?
>>>>
>>>> Hi,
>>>>
>>>> Based on recommendation from H/w team used PIN_MAPPABLE, not very 
>>>> sure about internal details.
>>>
>>> What did the recommendation exactly say? That it has to be in GGTT 
>>> or aperture?
>>
>> It said:
>> "GMM to allocate buffer from global GTT, get CPU mapped address as 
>> well (not stolen memory) ... ".
>
> So it's possible you don't need PIN_MAPPABLE.

As per I remember from initial discussion from h/w team DSB is not cache 
coherent. Due to that we have used I915_MAP_WC during mapping the buffer 
and want to keep the buffer in aperture region.
Could not find the discussion thread as it was quiet old like around 
initial enablement of DSB feature.

>
>
> Do we have some test coverage for this? In other words if I send a 
> patch which removes it, will we know if the feature is healthy?

Gamma lut programming is enabled through DSB. kms_color igt is having 
the coverage for this.

Not sure cache coherence issue is easily reproducible or not, will go 
with expert's opinion.

Regards,
Animesh

>
> Regards,
>
> Tvrtko

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

Re: [PATCH v2 02/15] drm/i915/dsb: DSB context creation.

[Chris Wilson]

Quoting Animesh Manna (2019-10-21 11:11:04)
> 
> 
> On 10/17/2019 8:08 PM, Tvrtko Ursulin wrote:
> >
> > On 17/10/2019 14:53, Animesh Manna wrote:
> >> On 10/17/2019 6:39 PM, Tvrtko Ursulin wrote:
> >>> On 17/10/2019 13:52, Animesh Manna wrote:
> >>>> On 10/17/2019 2:05 PM, Tvrtko Ursulin wrote:
> >>>>> On 22/08/2019 13:09, Chris Wilson wrote:
> >>>>>> Quoting Animesh Manna (2019-08-22 13:05:06)
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>>
> >>>>>>> On 8/21/2019 11:41 PM, Chris Wilson wrote:
> >>>>>>>> Quoting Animesh Manna (2019-08-21 07:32:22)
> >>>>>>>>> +       vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 
> >>>>>>>>> PIN_MAPPABLE);
> >>>>>>>> Only this (currently) still requires struct_mutex
> >>>>>>>
> >>>>>>> Sure will add.
> >>>>>>>>
> >>>>>>>> Does it have to mappable? Is that the HW constraint?
> >>>>>>>
> >>>>>>> Yes, as per HW design need a cpu mapped buffer to write 
> >>>>>>> opcode+data from
> >>>>>>> driver.
> >>>>>>
> >>>>>> PIN_MAPPABLE refers to the iomem aperture portion of the Global 
> >>>>>> GTT (i.e.
> >>>>>> the low 64-512MiB). You never use a GGTT mmap for your CPU 
> >>>>>> access, so the
> >>>>>> placement should be entirely dictated by the DSB requirements. If 
> >>>>>> you
> >>>>>> don't need to be in the low region, don't force it to be, so we have
> >>>>>> less congestion for the objects that have to be placed in that 
> >>>>>> region.
> >>>>>
> >>>>> I was doing a mini audit of what uses the aperture these days and 
> >>>>> noticed this code has been merged in the meantime, but AFAICS this 
> >>>>> question from Chris hasn't been answered? At least not on the 
> >>>>> mailing list. So does it need to be in the aperture region or not?
> >>>>
> >>>> Hi,
> >>>>
> >>>> Based on recommendation from H/w team used PIN_MAPPABLE, not very 
> >>>> sure about internal details.
> >>>
> >>> What did the recommendation exactly say? That it has to be in GGTT 
> >>> or aperture?
> >>
> >> It said:
> >> "GMM to allocate buffer from global GTT, get CPU mapped address as 
> >> well (not stolen memory) ... ".
> >
> > So it's possible you don't need PIN_MAPPABLE.
> 
> As per I remember from initial discussion from h/w team DSB is not cache 
> coherent. Due to that we have used I915_MAP_WC during mapping the buffer 
> and want to keep the buffer in aperture region.

The mappable aperture has nothing to do with I915_MAP_WC which is a
processor attribute on its page tables. If only I mentioned we had a way
to flush caches!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx