[PATCH v5 00/11] DSB enablement.

[PATCH v5 00/11] 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.

v3: As per review comments from Chris and Jani,
- removed some unwanted code. (Chris)
- Used i915_gem_object_create_internal instead of _shmem. (Chris)
- cmd_buf_tail removed and can be derived through vma object. (Chris)
- Simplified and optimized code few places. (Chris)
- Called dsb-api directly in callsites instead going via I915_WRITE. (Jani)

v4: Addressed review commnets from Shashank.

v5: Addressed review commnets from Shashank and Jani.

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 (11):
  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: Indexed register write function for DSB.
  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: added dsb refcount to synchronize between get/put.
  drm/i915/dsb: Enable gamma lut programming using DSB.
  drm/i915/dsb: Enable DSB for gen12.
  drm/i915/dsb: Documentation for DSB.

 Documentation/gpu/i915.rst                    |   9 +
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/display/intel_color.c    |  63 ++--
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/display/intel_dsb.c      | 335 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h      |  49 +++
 drivers/gpu/drm/i915/i915_drv.h               |   3 +
 drivers/gpu/drm/i915/i915_pci.c               |   3 +-
 drivers/gpu/drm/i915/i915_reg.h               |  10 +
 drivers/gpu/drm/i915/intel_device_info.h      |   1 +
 10 files changed, 454 insertions(+), 23 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 v5 01/11] drm/i915/dsb: feature flag added for display state buffer.

[Animesh Manna]

Display State Buffer(DSB) is a new hardware capability, introduced
in GEN12 display. DSB allows a driver to batch-program display HW
registers.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Shashank Sharma <shashank.sharma@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 db7480831e52..804bfe7aec2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1857,6 +1857,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 v5 02/11] drm/i915/dsb: DSB context creation.

[Animesh Manna]

This patch adds a function, which will internally get the gem buffer
for DSB engine. The GEM buffer is from global GTT, and is mapped into
CPU domain, contains the data + opcode to be feed to DSB engine.

v1: Initial version.

v2:
- removed some unwanted code. (Chris)
- Used i915_gem_object_create_internal instead of _shmem. (Chris)
- cmd_buf_tail removed and can be derived through vma object. (Chris)

v3: vma realeased if i915_gem_object_pin_map() failed. (Shashank)

v4: for simplification and based on current usage added single dsb
object in intel_crtc. (Shashank)

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>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |  1 +
 .../drm/i915/display/intel_display_types.h    |  3 +
 drivers/gpu/drm/i915/display/intel_dsb.c      | 70 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h      | 30 ++++++++
 drivers/gpu/drm/i915/i915_drv.h               |  1 +
 5 files changed, 105 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 658b930d34a8..6313e7b4bd78 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -172,6 +172,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 d5cc4b810d9e..49c902b00484 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1033,6 +1033,9 @@ struct intel_crtc {
 
 	/* scalers available on this crtc */
 	int num_scalers;
+
+	/* per pipe DSB related info */
+	struct intel_dsb dsb;
 };
 
 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..cba5c8d37659
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -0,0 +1,70 @@
+// 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 = &crtc->dsb;
+	intel_wakeref_t wakeref;
+
+	if ((!HAS_DSB(i915)) || dsb->cmd_buf)
+		return dsb;
+
+	dsb->id = DSB1;
+	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
+
+	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
+	if (IS_ERR(obj))
+		goto err;
+
+	mutex_lock(&i915->drm.struct_mutex);
+	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE);
+	mutex_unlock(&i915->drm.struct_mutex);
+	if (IS_ERR(vma)) {
+		DRM_ERROR("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_ERROR("Command buffer creation failed.\n");
+		i915_vma_unpin_and_release(&vma, 0);
+		dsb->cmd_buf = NULL;
+		goto err;
+	}
+	dsb->vma = vma;
+
+err:
+	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
+	return dsb;
+}
+
+void intel_dsb_put(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+
+	if (!dsb)
+		return;
+
+	if (dsb->cmd_buf) {
+		mutex_lock(&i915->drm.struct_mutex);
+		i915_gem_object_unpin_map(dsb->vma->obj);
+		i915_vma_unpin_and_release(&dsb->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
new file mode 100644
index 000000000000..27eb68eb5392
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -0,0 +1,30 @@
+/* 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 {
+	enum dsb_id id;
+	u32 *cmd_buf;
+	struct i915_vma *vma;
+};
+
+struct intel_dsb *
+intel_dsb_get(struct intel_crtc *crtc);
+void intel_dsb_put(struct intel_dsb *dsb);
+
+#endif
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 804bfe7aec2b..7aa11e3dd413 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 v5 03/11] 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.

v1: Initial version.
v2: Unused macro removed and cosmetic changes done. (Shashank)

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

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index cba5c8d37659..150be81fdfb3 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -9,6 +9,13 @@
 
 #define DSB_BUF_SIZE    (2 * PAGE_SIZE)
 
+/* DSB opcodes. */
+#define DSB_OPCODE_SHIFT		24
+#define DSB_OPCODE_MMIO_WRITE		0x1
+#define DSB_OPCODE_INDEXED_WRITE	0x9
+#define DSB_BYTE_EN			0xF
+#define DSB_BYTE_EN_SHIFT		20
+
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
 {
@@ -46,6 +53,7 @@ intel_dsb_get(struct intel_crtc *crtc)
 		goto err;
 	}
 	dsb->vma = vma;
+	dsb->free_pos = 0;
 
 err:
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
@@ -68,3 +76,25 @@ void intel_dsb_put(struct intel_dsb *dsb)
 		mutex_unlock(&i915->drm.struct_mutex);
 	}
 }
+
+void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 *buf = dsb->cmd_buf;
+
+	if (!buf) {
+		I915_WRITE(reg, val);
+		return;
+	}
+
+	if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
+		DRM_DEBUG_KMS("DSB buffer overflow.\n");
+		return;
+	}
+
+	buf[dsb->free_pos++] = val;
+	buf[dsb->free_pos++] = (DSB_OPCODE_MMIO_WRITE  << DSB_OPCODE_SHIFT) |
+			       (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
+			       i915_mmio_reg_offset(reg);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 27eb68eb5392..31b87dcfe160 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -6,6 +6,8 @@
 #ifndef _INTEL_DSB_H
 #define _INTEL_DSB_H
 
+#include "i915_reg.h"
+
 struct intel_crtc;
 struct i915_vma;
 
@@ -21,10 +23,17 @@ struct intel_dsb {
 	enum dsb_id id;
 	u32 *cmd_buf;
 	struct i915_vma *vma;
+
+	/*
+	 * free_pos will point the first free entry position
+	 * and help in calculating tail of command buffer.
+	 */
+	int free_pos;
 };
 
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc);
 void intel_dsb_put(struct intel_dsb *dsb);
+void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
 
 #endif
-- 
2.22.0

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

[PATCH v5 04/11] 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. DSB feature can be used for bulk register
programming e.g. gamma lut programming, HDR meta data programming.

v1: initial version.
v2: simplified code by using ALIGN(). (Chris)
v3: ascii table added as code comment. (Shashank)

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 | 64 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h |  8 +++
 2 files changed, 72 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 150be81fdfb3..0f55ed683d41 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -15,6 +15,7 @@
 #define DSB_OPCODE_INDEXED_WRITE	0x9
 #define DSB_BYTE_EN			0xF
 #define DSB_BYTE_EN_SHIFT		20
+#define DSB_REG_VALUE_MASK		0xfffff
 
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
@@ -77,6 +78,69 @@ void intel_dsb_put(struct intel_dsb *dsb)
 	}
 }
 
+void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
+				 u32 val)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 *buf = dsb->cmd_buf;
+	u32 reg_val;
+
+	if (!buf) {
+		I915_WRITE(reg, val);
+		return;
+	}
+
+	if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
+		DRM_DEBUG_KMS("DSB buffer overflow.\n");
+		return;
+	}
+
+	/*
+	 * For example the buffer will look like below for 3 dwords for auto
+	 * increment register:
+	 * +--------------------------------------------------------+
+	 * | size = 3 | offset &| value1 | value2 | value3 | zero   |
+	 * |          | opcode  |        |        |        |        |
+	 * +--------------------------------------------------------+
+	 * +          +         +        +        +        +        +
+	 * 0          4         8        12       16       20       24
+	 * Byte
+	 *
+	 * As every instruction is 8 byte aligned the index of dsb instruction
+	 * will start always from even number while dealing with u32 array and
+	 * zero to be added for odd number of dwords at the last.
+	 */
+	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. */
+		dsb->free_pos = ALIGN(dsb->free_pos, 2);
+
+		dsb->ins_start_offset = dsb->free_pos;
+
+		/* Update the size. */
+		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 = container_of(dsb, typeof(*crtc), dsb);
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 31b87dcfe160..9b2522f20bfb 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -29,11 +29,19 @@ struct intel_dsb {
 	 * and help in calculating tail of command buffer.
 	 */
 	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 *
 intel_dsb_get(struct intel_crtc *crtc);
 void intel_dsb_put(struct intel_dsb *dsb);
 void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
+void intel_dsb_indexed_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 v5 05/11] 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>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 9 +++++++++
 drivers/gpu/drm/i915/i915_reg.h          | 7 +++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 0f55ed683d41..2c8415518c65 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -17,6 +17,15 @@
 #define DSB_BYTE_EN_SHIFT		20
 #define DSB_REG_VALUE_MASK		0xfffff
 
+static inline bool is_dsb_busy(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	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)
 {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 006cffd56be2..a3099f712ae6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11676,4 +11676,11 @@ 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_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
+#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 v5 06/11] 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.

v1: Initial version.
v2: POSTING_READ added after writing control register. (Shashank)

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

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 2c8415518c65..56bf41b00f62 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -26,6 +26,48 @@ static inline bool is_dsb_busy(struct intel_dsb *dsb)
 	return DSB_STATUS & I915_READ(DSB_CTRL(pipe, dsb->id));
 }
 
+static inline bool intel_dsb_enable_engine(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	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);
+
+	POSTING_READ(DSB_CTRL(pipe, dsb->id));
+	return true;
+}
+
+static inline bool intel_dsb_disable_engine(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	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);
+
+	POSTING_READ(DSB_CTRL(pipe, dsb->id));
+	return true;
+}
+
 struct intel_dsb *
 intel_dsb_get(struct intel_crtc *crtc)
 {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a3099f712ae6..2df01386e3de 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11681,6 +11681,7 @@ enum skl_power_gate {
 #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
 					 (pipe) * 0x1000 + (id) * 100)
 #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
+#define   DSB_ENABLE			(1 << 31)
 #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 v5 07/11] 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.

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

Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Shashank Sharma <shashank.sharma@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 |  1 +
 drivers/gpu/drm/i915/i915_reg.h          |  2 ++
 3 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 56bf41b00f62..853685751540 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -213,3 +213,45 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
 			       (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
 			       i915_mmio_reg_offset(reg);
 }
+
+void intel_dsb_commit(struct intel_dsb *dsb)
+{
+	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum pipe pipe = crtc->pipe;
+	u32 tail;
+
+	if (!dsb->free_pos)
+		return;
+
+	if (!intel_dsb_enable_engine(dsb))
+		goto reset;
+
+	if (is_dsb_busy(dsb)) {
+		DRM_ERROR("HEAD_PTR write failed - dsb engine is busy.\n");
+		goto reset;
+	}
+	I915_WRITE(DSB_HEAD(pipe, dsb->id), i915_ggtt_offset(dsb->vma));
+
+	tail = ALIGN(dsb->free_pos * 4, CACHELINE_BYTES);
+	if (tail > dsb->free_pos * 4)
+		memset(&dsb->cmd_buf[dsb->free_pos], 0,
+		       (tail - dsb->free_pos * 4));
+
+	if (is_dsb_busy(dsb)) {
+		DRM_ERROR("TAIL_PTR write failed - dsb engine is busy.\n");
+		goto reset;
+	}
+	DRM_DEBUG_KMS("DSB execution started - head 0x%x, tail 0x%x\n",
+		      i915_ggtt_offset(dsb->vma), tail);
+	I915_WRITE(DSB_TAIL(pipe, dsb->id), i915_ggtt_offset(dsb->vma) + tail);
+	if (wait_for(!is_dsb_busy(dsb), 1)) {
+		DRM_ERROR("Timed out waiting for DSB workload completion.\n");
+		goto reset;
+	}
+
+reset:
+	dsb->free_pos = 0;
+	intel_dsb_disable_engine(dsb);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 9b2522f20bfb..7389c8c5b665 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -43,5 +43,6 @@ void intel_dsb_put(struct intel_dsb *dsb);
 void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
 void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
 				 u32 val);
+void intel_dsb_commit(struct intel_dsb *dsb);
 
 #endif
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2df01386e3de..cfb78a2f94fe 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11680,6 +11680,8 @@ enum skl_power_gate {
 #define _DSBSL_INSTANCE_BASE		0x70B00
 #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
 					 (pipe) * 0x1000 + (id) * 100)
+#define DSB_HEAD(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x0)
+#define DSB_TAIL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x4)
 #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
 #define   DSB_ENABLE			(1 << 31)
 #define   DSB_STATUS			(1 << 0)
-- 
2.22.0

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

[PATCH v5 08/11] drm/i915/dsb: added dsb refcount to synchronize between get/put.

[Animesh Manna]

The lifetime of command buffer can be controlled by the dsb user
throuh refcount. Added refcount mechanism is dsb get/put call
which create/destroy dsb context.

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

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 853685751540..b951a6b5264a 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -78,7 +78,12 @@ intel_dsb_get(struct intel_crtc *crtc)
 	struct intel_dsb *dsb = &crtc->dsb;
 	intel_wakeref_t wakeref;
 
-	if ((!HAS_DSB(i915)) || dsb->cmd_buf)
+	if (!HAS_DSB(i915))
+		return dsb;
+
+	atomic_inc(&dsb->refcount);
+
+	if (dsb->cmd_buf)
 		return dsb;
 
 	dsb->id = DSB1;
@@ -94,6 +99,7 @@ intel_dsb_get(struct intel_crtc *crtc)
 	if (IS_ERR(vma)) {
 		DRM_ERROR("Vma creation failed.\n");
 		i915_gem_object_put(obj);
+		atomic_dec(&dsb->refcount);
 		goto err;
 	}
 
@@ -102,6 +108,7 @@ intel_dsb_get(struct intel_crtc *crtc)
 		DRM_ERROR("Command buffer creation failed.\n");
 		i915_vma_unpin_and_release(&vma, 0);
 		dsb->cmd_buf = NULL;
+		atomic_dec(&dsb->refcount);
 		goto err;
 	}
 	dsb->vma = vma;
@@ -121,11 +128,14 @@ void intel_dsb_put(struct intel_dsb *dsb)
 		return;
 
 	if (dsb->cmd_buf) {
-		mutex_lock(&i915->drm.struct_mutex);
-		i915_gem_object_unpin_map(dsb->vma->obj);
-		i915_vma_unpin_and_release(&dsb->vma, 0);
-		dsb->cmd_buf = NULL;
-		mutex_unlock(&i915->drm.struct_mutex);
+		atomic_dec(&dsb->refcount);
+		if (!atomic_read(&dsb->refcount)) {
+			mutex_lock(&i915->drm.struct_mutex);
+			i915_gem_object_unpin_map(dsb->vma->obj);
+			i915_vma_unpin_and_release(&dsb->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 7389c8c5b665..dca4e632dd3c 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -20,6 +20,7 @@ enum dsb_id {
 };
 
 struct intel_dsb {
+	atomic_t refcount;
 	enum dsb_id id;
 	u32 *cmd_buf;
 	struct i915_vma *vma;
-- 
2.22.0

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

[PATCH v5 09/11] 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.

Currently enabled for 12-bit gamma LUT which is enabled by
default and later 8-bit/10-bit will be enabled in future
based on need.

v1: Initial version.
v2: Directly call dsb-api at callsites. (Jani)
v3:
- modified the code as per single dsb instance per crtc. (Shashank)
- Added dsb get/put call in platform specific load_lut hook. (Jani)
- removed dsb pointer from dev_priv. (Jani)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 63 ++++++++++++++--------
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 6d641e159899..259142a8db8b 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -611,12 +611,13 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
 static void ivb_load_lut_ext_max(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
 
 	/* Program the max register to clamp values > 1.0. */
-	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
-	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
-	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
+	intel_dsb_reg_write(dsb, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
+	intel_dsb_reg_write(dsb, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
+	intel_dsb_reg_write(dsb, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
 
 	/*
 	 * Program the gc max 2 register to clamp values > 1.0.
@@ -624,10 +625,14 @@ static void ivb_load_lut_ext_max(struct intel_crtc *crtc)
 	 * from 3.0 to 7.0
 	 */
 	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
-		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
-		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
-		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
+		intel_dsb_reg_write(dsb, PREC_PAL_EXT2_GC_MAX(pipe, 0),
+				    1 << 16);
+		intel_dsb_reg_write(dsb, PREC_PAL_EXT2_GC_MAX(pipe, 1),
+				    1 << 16);
+		intel_dsb_reg_write(dsb, PREC_PAL_EXT2_GC_MAX(pipe, 2),
+				    1 << 16);
 	}
+	intel_dsb_put(dsb);
 }
 
 static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
@@ -787,22 +792,24 @@ icl_load_gcmax(const struct intel_crtc_state *crtc_state,
 	       const struct drm_color_lut *color)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
 
 	/* Fixme: LUT entries are 16 bit only, so we can prog 0xFFFF max */
-	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), color->red);
-	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), color->green);
-	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), color->blue);
+	intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 0), color->red);
+	intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 1), color->green);
+	intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 2), color->blue);
+
+	intel_dsb_put(dsb);
 }
 
 static void
 icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
 	const struct drm_color_lut *lut = blob->data;
+	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
 	u32 i;
 
@@ -813,26 +820,29 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
 	 * Superfine segment has 9 entries, corresponding to values
 	 * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
 	 */
-	I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
+	intel_dsb_reg_write(dsb, PREC_PAL_MULTI_SEG_INDEX(pipe),
+			    PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < 9; i++) {
 		const struct drm_color_lut *entry = &lut[i];
 
-		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
-			   ilk_lut_12p4_ldw(entry));
-		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
-			   ilk_lut_12p4_udw(entry));
+		intel_dsb_indexed_reg_write(dsb, PREC_PAL_MULTI_SEG_DATA(pipe),
+					    ilk_lut_12p4_ldw(entry));
+		intel_dsb_indexed_reg_write(dsb, PREC_PAL_MULTI_SEG_DATA(pipe),
+					    ilk_lut_12p4_udw(entry));
 	}
+
+	intel_dsb_put(dsb);
 }
 
 static void
 icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
 	const struct drm_color_lut *lut = blob->data;
 	const struct drm_color_lut *entry;
+	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
 	u32 i;
 
@@ -847,11 +857,13 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 	 * PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
 	 * with seg2[0] being unused by the hardware.
 	 */
-	I915_WRITE(PREC_PAL_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
+	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
 	for (i = 1; i < 257; i++) {
 		entry = &lut[i * 8];
-		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
-		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
+		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
+					    ilk_lut_12p4_ldw(entry));
+		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
+					    ilk_lut_12p4_udw(entry));
 	}
 
 	/*
@@ -868,20 +880,24 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 	 */
 	for (i = 0; i < 256; i++) {
 		entry = &lut[i * 8 * 128];
-		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
-		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
+		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
+					    ilk_lut_12p4_ldw(entry));
+		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
+					    ilk_lut_12p4_udw(entry));
 	}
 
 	/* The last entry in the LUT is to be programmed in GCMAX */
 	entry = &lut[256 * 8 * 128];
 	icl_load_gcmax(crtc_state, entry);
 	ivb_load_lut_ext_max(crtc);
+	intel_dsb_put(dsb);
 }
 
 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 intel_dsb *dsb = intel_dsb_get(crtc);
 
 	if (crtc_state->base.degamma_lut)
 		glk_load_degamma_lut(crtc_state);
@@ -900,6 +916,9 @@ 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(dsb);
+	intel_dsb_put(dsb);
 }
 
 static u32 chv_cgm_degamma_ldw(const struct drm_color_lut *color)
-- 
2.22.0

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

[PATCH v5 10/11] drm/i915/dsb: Enable DSB for gen12.

[Animesh Manna]

Enabling DSB by setting 1 to has_dsb flag for gen12.

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

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index fbe98a2db88e..512b26e161f8 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -787,7 +787,8 @@ static const struct intel_device_info intel_elkhartlake_info = {
 		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET, \
 		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
 	}, \
-	.has_global_mocs = 1
+	.has_global_mocs = 1, \
+	.display.has_dsb = 1
 
 static const struct intel_device_info intel_tigerlake_12_info = {
 	GEN12_FEATURES,
-- 
2.22.0

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

[PATCH v5 11/11] 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.

v1: Initial version as RFC.

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

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index e249ea7b0ec7..465779670fd4 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 b951a6b5264a..4db360fcc31e 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_MMIO_WRITE		0x1
@@ -68,6 +85,17 @@ static inline 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)
 {
@@ -119,6 +147,14 @@ intel_dsb_get(struct intel_crtc *crtc)
 	return 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 = container_of(dsb, typeof(*crtc), dsb);
@@ -139,6 +175,19 @@ void intel_dsb_put(struct intel_dsb *dsb)
 	}
 }
 
+/**
+ * intel_dsb_indexed_reg_write() -Write to the dsb context for auto
+ * increment register.
+ * @dsb: intel_dsb structure.
+ * @reg: register address.
+ * @val: value.
+ *
+ * This function is used for auto-increment register and intel_dsb_reg_write()
+ * is used for normal register. During command buffer overflow, a warning
+ * is thrown and rest all erroneous condition register programming is done
+ * through mmio write.
+ */
+
 void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
 				 u32 val)
 {
@@ -202,6 +251,18 @@ 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 = container_of(dsb, typeof(*crtc), dsb);
@@ -224,6 +285,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 = container_of(dsb, typeof(*crtc), dsb);
-- 
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. (rev5)

[Patchwork]

== Series Details ==

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

== Summary ==

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

total: 0 errors, 1 warnings, 0 checks, 123 lines checked
930b25e9b1c6 drm/i915/dsb: single register write function for DSB.
d9fb8fb3f7ef drm/i915/dsb: Indexed register write function for DSB.
f60d7da4412f drm/i915/dsb: Check DSB engine status.
c75e72c321cd drm/i915/dsb: functions to enable/disable DSB engine.
ea1905d6f32a drm/i915/dsb: function to trigger workload execution of DSB.
a08b6dcfe6b0 drm/i915/dsb: added dsb refcount to synchronize between get/put.
2d68c8a3e8bd drm/i915/dsb: Enable gamma lut programming using DSB.
ca096b1ecbc5 drm/i915/dsb: Enable DSB for gen12.
1b669e5a5035 drm/i915/dsb: Documentation for DSB.

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

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

[Patchwork]

== Series Details ==

Series: DSB enablement. (rev5)
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: failure for DSB enablement. (rev5)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6849 -> Patchwork_14314
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-icl-u2:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14314/fi-icl-u2/igt@kms_chamelium@hdmi-crc-fast.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_sync@basic-each:
    - {fi-tgl-u}:         NOTRUN -> [INCOMPLETE][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14314/fi-tgl-u/igt@gem_sync@basic-each.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_cpu_reloc@basic:
    - fi-icl-u3:          [PASS][4] -> [DMESG-WARN][5] ([fdo#107724])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-icl-u3/igt@gem_cpu_reloc@basic.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14314/fi-icl-u3/igt@gem_cpu_reloc@basic.html

  * igt@i915_module_load@reload:
    - fi-icl-u3:          [PASS][6] -> [DMESG-WARN][7] ([fdo#107724] / [fdo#111214])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-icl-u3/igt@i915_module_load@reload.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14314/fi-icl-u3/igt@i915_module_load@reload.html

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

  
#### Possible fixes ####

  * igt@gem_exec_gttfill@basic:
    - {fi-tgl-u}:         [INCOMPLETE][10] -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-tgl-u/igt@gem_exec_gttfill@basic.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14314/fi-tgl-u/igt@gem_exec_gttfill@basic.html

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

  * igt@i915_module_load@reload-no-display:
    - fi-icl-u3:          [DMESG-WARN][14] ([fdo#107724]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6849/fi-icl-u3/igt@i915_module_load@reload-no-display.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14314/fi-icl-u3/igt@i915_module_load@reload-no-display.html

  
#### Warnings ####

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

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [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#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111155]: https://bugs.freedesktop.org/show_bug.cgi?id=111155
  [fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407


Participating hosts (52 -> 44)
------------------------------

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6849 -> Patchwork_14314

  CI-20190529: 20190529
  CI_DRM_6849: d4111d3dd34455068de02ce18b796a631c7fe3a9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5173: 3fb0f227d8856008f89a797879e27094745ce97e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14314: 1b669e5a50358e88dee03389ea4c091b3e9a4e24 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1b669e5a5035 drm/i915/dsb: Documentation for DSB.
ca096b1ecbc5 drm/i915/dsb: Enable DSB for gen12.
2d68c8a3e8bd drm/i915/dsb: Enable gamma lut programming using DSB.
a08b6dcfe6b0 drm/i915/dsb: added dsb refcount to synchronize between get/put.
ea1905d6f32a drm/i915/dsb: function to trigger workload execution of DSB.
c75e72c321cd drm/i915/dsb: functions to enable/disable DSB engine.
f60d7da4412f drm/i915/dsb: Check DSB engine status.
d9fb8fb3f7ef drm/i915/dsb: Indexed register write function for DSB.
930b25e9b1c6 drm/i915/dsb: single register write function for DSB.
41a4427c2474 drm/i915/dsb: DSB context creation.
0f9bce67daf4 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_14314/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 02/11] drm/i915/dsb: DSB context creation.

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> This patch adds a function, which will internally get the gem buffer
> for DSB engine. The GEM buffer is from global GTT, and is mapped into
> CPU domain, contains the data + opcode to be feed to DSB engine.
>
> v1: Initial version.
>
> v2:
> - removed some unwanted code. (Chris)
> - Used i915_gem_object_create_internal instead of _shmem. (Chris)
> - cmd_buf_tail removed and can be derived through vma object. (Chris)
>
> v3: vma realeased if i915_gem_object_pin_map() failed. (Shashank)
>
> v4: for simplification and based on current usage added single dsb
> object in intel_crtc. (Shashank)
>
> 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>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile                 |  1 +
>   .../drm/i915/display/intel_display_types.h    |  3 +
>   drivers/gpu/drm/i915/display/intel_dsb.c      | 70 +++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_dsb.h      | 30 ++++++++
>   drivers/gpu/drm/i915/i915_drv.h               |  1 +
>   5 files changed, 105 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 658b930d34a8..6313e7b4bd78 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -172,6 +172,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 d5cc4b810d9e..49c902b00484 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1033,6 +1033,9 @@ struct intel_crtc {
>   
>   	/* scalers available on this crtc */
>   	int num_scalers;
> +
> +	/* per pipe DSB related info */
> +	struct intel_dsb dsb;
>   };
>   
>   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..cba5c8d37659
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -0,0 +1,70 @@
> +// 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 = &crtc->dsb;
> +	intel_wakeref_t wakeref;
> +
> +	if ((!HAS_DSB(i915)) || dsb->cmd_buf)
> +		return dsb;
> +
> +	dsb->id = DSB1;
> +	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> +	obj = i915_gem_object_create_internal(i915, DSB_BUF_SIZE);
> +	if (IS_ERR(obj))
> +		goto err;
> +
> +	mutex_lock(&i915->drm.struct_mutex);
> +	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, PIN_MAPPABLE);
> +	mutex_unlock(&i915->drm.struct_mutex);
> +	if (IS_ERR(vma)) {
> +		DRM_ERROR("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_ERROR("Command buffer creation failed.\n");
> +		i915_vma_unpin_and_release(&vma, 0);
> +		dsb->cmd_buf = NULL;
> +		goto err;
> +	}
> +	dsb->vma = vma;
> +
> +err:
> +	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> +	return dsb;
> +}
> +
> +void intel_dsb_put(struct intel_dsb *dsb)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +
> +	if (!dsb)
> +		return;
> +
> +	if (dsb->cmd_buf) {
> +		mutex_lock(&i915->drm.struct_mutex);
> +		i915_gem_object_unpin_map(dsb->vma->obj);
> +		i915_vma_unpin_and_release(&dsb->vma, 0);
> +		dsb->cmd_buf = NULL;

This can be done outside mutex_unlock();

- Shashank

> +		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
> new file mode 100644
> index 000000000000..27eb68eb5392
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -0,0 +1,30 @@
> +/* 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 {
> +	enum dsb_id id;
> +	u32 *cmd_buf;
> +	struct i915_vma *vma;
> +};
> +
> +struct intel_dsb *
> +intel_dsb_get(struct intel_crtc *crtc);
> +void intel_dsb_put(struct intel_dsb *dsb);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 804bfe7aec2b..7aa11e3dd413 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"
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 03/11] drm/i915/dsb: single register write function for DSB.

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> DSB support single register write through opcode 0x1. Generic
> api created which accumulate all single register write in a batch
> buffer and once DSB is triggered, it will program all the registers
> at the same time.
>
> v1: Initial version.
> v2: Unused macro removed and cosmetic changes done. (Shashank)
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 30 ++++++++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_dsb.h |  9 +++++++
>   2 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index cba5c8d37659..150be81fdfb3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -9,6 +9,13 @@
>   
>   #define DSB_BUF_SIZE    (2 * PAGE_SIZE)
>   
> +/* DSB opcodes. */
> +#define DSB_OPCODE_SHIFT		24
> +#define DSB_OPCODE_MMIO_WRITE		0x1
> +#define DSB_OPCODE_INDEXED_WRITE	0x9
> +#define DSB_BYTE_EN			0xF
> +#define DSB_BYTE_EN_SHIFT		20
> +
>   struct intel_dsb *
>   intel_dsb_get(struct intel_crtc *crtc)
>   {
> @@ -46,6 +53,7 @@ intel_dsb_get(struct intel_crtc *crtc)
>   		goto err;
>   	}
>   	dsb->vma = vma;
> +	dsb->free_pos = 0;
This should be done in dsb_put();
>   
>   err:
>   	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> @@ -68,3 +76,25 @@ void intel_dsb_put(struct intel_dsb *dsb)
>   		mutex_unlock(&i915->drm.struct_mutex);
>   	}
>   }
> +
> +void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	u32 *buf = dsb->cmd_buf;
> +
> +	if (!buf) {
> +		I915_WRITE(reg, val);
> +		return;
> +	}
> +
> +	if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
> +		DRM_DEBUG_KMS("DSB buffer overflow.\n");

Lets remove this '.' in the end, to maintain consistency in the log.

- Shashank

> +		return;
> +	}
> +
> +	buf[dsb->free_pos++] = val;
> +	buf[dsb->free_pos++] = (DSB_OPCODE_MMIO_WRITE  << DSB_OPCODE_SHIFT) |
> +			       (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
> +			       i915_mmio_reg_offset(reg);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 27eb68eb5392..31b87dcfe160 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -6,6 +6,8 @@
>   #ifndef _INTEL_DSB_H
>   #define _INTEL_DSB_H
>   
> +#include "i915_reg.h"
> +
>   struct intel_crtc;
>   struct i915_vma;
>   
> @@ -21,10 +23,17 @@ struct intel_dsb {
>   	enum dsb_id id;
>   	u32 *cmd_buf;
>   	struct i915_vma *vma;
> +
> +	/*
> +	 * free_pos will point the first free entry position
> +	 * and help in calculating tail of command buffer.
> +	 */
> +	int free_pos;
>   };
>   
>   struct intel_dsb *
>   intel_dsb_get(struct intel_crtc *crtc);
>   void intel_dsb_put(struct intel_dsb *dsb);
> +void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
>   
>   #endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 04/11] drm/i915/dsb: Indexed register write function for DSB.

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> DSB can program large set of data through indexed register write
> (opcode 0x9) in one shot. DSB feature can be used for bulk register
> programming e.g. gamma lut programming, HDR meta data programming.
>
> v1: initial version.
> v2: simplified code by using ALIGN(). (Chris)
> v3: ascii table added as code comment. (Shashank)
>
> 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 | 64 ++++++++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_dsb.h |  8 +++
>   2 files changed, 72 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 150be81fdfb3..0f55ed683d41 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -15,6 +15,7 @@
>   #define DSB_OPCODE_INDEXED_WRITE	0x9
>   #define DSB_BYTE_EN			0xF
>   #define DSB_BYTE_EN_SHIFT		20
> +#define DSB_REG_VALUE_MASK		0xfffff
>   
>   struct intel_dsb *
>   intel_dsb_get(struct intel_crtc *crtc)
> @@ -77,6 +78,69 @@ void intel_dsb_put(struct intel_dsb *dsb)
>   	}
>   }
>   
> +void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
> +				 u32 val)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	u32 *buf = dsb->cmd_buf;
> +	u32 reg_val;
> +
> +	if (!buf) {
> +		I915_WRITE(reg, val);
> +		return;
> +	}
> +
> +	if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
> +		DRM_DEBUG_KMS("DSB buffer overflow.\n");
Again, '.' in the end can be removed
> +		return;
> +	}
> +
> +	/*
> +	 * For example the buffer will look like below for 3 dwords for auto
> +	 * increment register:
> +	 * +--------------------------------------------------------+
> +	 * | size = 3 | offset &| value1 | value2 | value3 | zero   |
> +	 * |          | opcode  |        |        |        |        |
> +	 * +--------------------------------------------------------+
> +	 * +          +         +        +        +        +        +
> +	 * 0          4         8        12       16       20       24
> +	 * Byte
> +	 *
> +	 * As every instruction is 8 byte aligned the index of dsb instruction
> +	 * will start always from even number while dealing with u32 array and
> +	 * zero to be added for odd number of dwords at the last.

Let's split this comment in two parts, to make even more useful, like:

"As every instruction ......................... array". "If we are 
writing odd no of dwords, Zeros will be added in the end for padding."

- Shashank

> +	 */
> +	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. */
> +		dsb->free_pos = ALIGN(dsb->free_pos, 2);
> +
> +		dsb->ins_start_offset = dsb->free_pos;
> +
> +		/* Update the size. */
> +		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 = container_of(dsb, typeof(*crtc), dsb);
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 31b87dcfe160..9b2522f20bfb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -29,11 +29,19 @@ struct intel_dsb {
>   	 * and help in calculating tail of command buffer.
>   	 */
>   	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 *
>   intel_dsb_get(struct intel_crtc *crtc);
>   void intel_dsb_put(struct intel_dsb *dsb);
>   void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
> +void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
> +				 u32 val);
>   
>   #endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 05/11] drm/i915/dsb: Check DSB engine status.

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> 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>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 9 +++++++++
>   drivers/gpu/drm/i915/i915_reg.h          | 7 +++++++
>   2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 0f55ed683d41..2c8415518c65 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -17,6 +17,15 @@
>   #define DSB_BYTE_EN_SHIFT		20
>   #define DSB_REG_VALUE_MASK		0xfffff
>   
> +static inline bool is_dsb_busy(struct intel_dsb *dsb)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	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)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 006cffd56be2..a3099f712ae6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11676,4 +11676,11 @@ 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)

Why is pipe in () ?

- Shashank

> +#define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
> +#define   DSB_STATUS			(1 << 0)
> +
>   #endif /* _I915_REG_H_ */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 06/11] drm/i915/dsb: functions to enable/disable DSB engine.

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> 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.
>
> v1: Initial version.
> v2: POSTING_READ added after writing control register. (Shashank)
>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 42 ++++++++++++++++++++++++
>   drivers/gpu/drm/i915/i915_reg.h          |  1 +
>   2 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 2c8415518c65..56bf41b00f62 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -26,6 +26,48 @@ static inline bool is_dsb_busy(struct intel_dsb *dsb)
>   	return DSB_STATUS & I915_READ(DSB_CTRL(pipe, dsb->id));
>   }
>   
> +static inline bool intel_dsb_enable_engine(struct intel_dsb *dsb)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	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));
> +
This space not required.
> +	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);
> +
> +	POSTING_READ(DSB_CTRL(pipe, dsb->id));
> +	return true;
> +}
> +
> +static inline bool intel_dsb_disable_engine(struct intel_dsb *dsb)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	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));
> +
Same here.
> +	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);
> +
> +	POSTING_READ(DSB_CTRL(pipe, dsb->id));
> +	return true;
> +}
> +
>   struct intel_dsb *
>   intel_dsb_get(struct intel_crtc *crtc)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a3099f712ae6..2df01386e3de 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11681,6 +11681,7 @@ enum skl_power_gate {
>   #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
>   					 (pipe) * 0x1000 + (id) * 100)
>   #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
> +#define   DSB_ENABLE			(1 << 31)
>   #define   DSB_STATUS			(1 << 0)
>   
>   #endif /* _I915_REG_H_ */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> Batch buffer will be created through dsb-reg-write function which can have
> single/multiple request based on usecase and once the buffer is ready
> commit function will trigger the execution of the batch buffer. All
> the registers will be updated simultaneously.
>
> v1: Initial version.
> v2: Optimized code few places. (Chris)
> v3: USed DRM_ERROR for dsb head/tail programming failure. (Shashank)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Shashank Sharma <shashank.sharma@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 |  1 +
>   drivers/gpu/drm/i915/i915_reg.h          |  2 ++
>   3 files changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 56bf41b00f62..853685751540 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -213,3 +213,45 @@ void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
>   			       (DSB_BYTE_EN << DSB_BYTE_EN_SHIFT) |
>   			       i915_mmio_reg_offset(reg);
>   }
> +
> +void intel_dsb_commit(struct intel_dsb *dsb)
> +{
> +	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	enum pipe pipe = crtc->pipe;
> +	u32 tail;
> +
> +	if (!dsb->free_pos)

I am seeing that in both success and failure cases, we are not returning 
anything. We have some error messages, but I would still like the caller 
to know if the commit was successful or not, with a return value. Do you 
think so Jani?

- Shashank

> +		return;
> +
> +	if (!intel_dsb_enable_engine(dsb))
> +		goto reset;
> +
> +	if (is_dsb_busy(dsb)) {
> +		DRM_ERROR("HEAD_PTR write failed - dsb engine is busy.\n");
> +		goto reset;
> +	}
> +	I915_WRITE(DSB_HEAD(pipe, dsb->id), i915_ggtt_offset(dsb->vma));
> +
> +	tail = ALIGN(dsb->free_pos * 4, CACHELINE_BYTES);
> +	if (tail > dsb->free_pos * 4)
> +		memset(&dsb->cmd_buf[dsb->free_pos], 0,
> +		       (tail - dsb->free_pos * 4));
> +
> +	if (is_dsb_busy(dsb)) {
> +		DRM_ERROR("TAIL_PTR write failed - dsb engine is busy.\n");
> +		goto reset;
> +	}
> +	DRM_DEBUG_KMS("DSB execution started - head 0x%x, tail 0x%x\n",
> +		      i915_ggtt_offset(dsb->vma), tail);
> +	I915_WRITE(DSB_TAIL(pipe, dsb->id), i915_ggtt_offset(dsb->vma) + tail);
> +	if (wait_for(!is_dsb_busy(dsb), 1)) {
> +		DRM_ERROR("Timed out waiting for DSB workload completion.\n");
> +		goto reset;
> +	}
> +
> +reset:
> +	dsb->free_pos = 0;
> +	intel_dsb_disable_engine(dsb);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
> index 9b2522f20bfb..7389c8c5b665 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -43,5 +43,6 @@ void intel_dsb_put(struct intel_dsb *dsb);
>   void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val);
>   void intel_dsb_indexed_reg_write(struct intel_dsb *dsb, i915_reg_t reg,
>   				 u32 val);
> +void intel_dsb_commit(struct intel_dsb *dsb);
>   
>   #endif
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2df01386e3de..cfb78a2f94fe 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -11680,6 +11680,8 @@ enum skl_power_gate {
>   #define _DSBSL_INSTANCE_BASE		0x70B00
>   #define DSBSL_INSTANCE(pipe, id)	(_DSBSL_INSTANCE_BASE + \
>   					 (pipe) * 0x1000 + (id) * 100)
> +#define DSB_HEAD(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x0)
> +#define DSB_TAIL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x4)
>   #define DSB_CTRL(pipe, id)		_MMIO(DSBSL_INSTANCE(pipe, id) + 0x8)
>   #define   DSB_ENABLE			(1 << 31)
>   #define   DSB_STATUS			(1 << 0)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 08/11] drm/i915/dsb: added dsb refcount to synchronize between get/put.

[Sharma, Shashank]

On 9/7/2019 4:37 PM, Animesh Manna wrote:
> The lifetime of command buffer can be controlled by the dsb user
> throuh refcount. Added refcount mechanism is dsb get/put call
> which create/destroy dsb context.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 22 ++++++++++++++++------
>   drivers/gpu/drm/i915/display/intel_dsb.h |  1 +
>   2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 853685751540..b951a6b5264a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -78,7 +78,12 @@ intel_dsb_get(struct intel_crtc *crtc)
>   	struct intel_dsb *dsb = &crtc->dsb;
>   	intel_wakeref_t wakeref;
>   
> -	if ((!HAS_DSB(i915)) || dsb->cmd_buf)
> +	if (!HAS_DSB(i915))
> +		return dsb;
> +
> +	atomic_inc(&dsb->refcount);
> +

As discussed we are not solving any problem with reference counting, 
rather, we are adding a complexity here. It may be useful, when we are 
extending single instance of DSB to DSB pool but not right now.

I would say we drop this patch all together, and just have the simple 
implementation now.

- Shashank

> +	if (dsb->cmd_buf)
>   		return dsb;
>   
>   	dsb->id = DSB1;
> @@ -94,6 +99,7 @@ intel_dsb_get(struct intel_crtc *crtc)
>   	if (IS_ERR(vma)) {
>   		DRM_ERROR("Vma creation failed.\n");
>   		i915_gem_object_put(obj);
> +		atomic_dec(&dsb->refcount);
>   		goto err;
>   	}
>   
> @@ -102,6 +108,7 @@ intel_dsb_get(struct intel_crtc *crtc)
>   		DRM_ERROR("Command buffer creation failed.\n");
>   		i915_vma_unpin_and_release(&vma, 0);
>   		dsb->cmd_buf = NULL;
> +		atomic_dec(&dsb->refcount);
>   		goto err;
>   	}
>   	dsb->vma = vma;
> @@ -121,11 +128,14 @@ void intel_dsb_put(struct intel_dsb *dsb)
>   		return;
>   
>   	if (dsb->cmd_buf) {
> -		mutex_lock(&i915->drm.struct_mutex);
> -		i915_gem_object_unpin_map(dsb->vma->obj);
> -		i915_vma_unpin_and_release(&dsb->vma, 0);
> -		dsb->cmd_buf = NULL;
> -		mutex_unlock(&i915->drm.struct_mutex);
> +		atomic_dec(&dsb->refcount);
> +		if (!atomic_read(&dsb->refcount)) {
> +			mutex_lock(&i915->drm.struct_mutex);
> +			i915_gem_object_unpin_map(dsb->vma->obj);
> +			i915_vma_unpin_and_release(&dsb->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 7389c8c5b665..dca4e632dd3c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.h
> @@ -20,6 +20,7 @@ enum dsb_id {
>   };
>   
>   struct intel_dsb {
> +	atomic_t refcount;
>   	enum dsb_id id;
>   	u32 *cmd_buf;
>   	struct i915_vma *vma;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 02/11] drm/i915/dsb: DSB context creation.

[Animesh Manna]

On 9/9/2019 6:26 PM, Sharma, Shashank wrote:
>
> On 9/7/2019 4:37 PM, Animesh Manna wrote:
>> +void intel_dsb_put(struct intel_dsb *dsb)
>> +{
>> +    struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
>> +    struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>> +
>> +    if (!dsb)
>> +        return;
>> +
>> +    if (dsb->cmd_buf) {
>> +        mutex_lock(&i915->drm.struct_mutex);
>> +        i915_gem_object_unpin_map(dsb->vma->obj);
>> +        i915_vma_unpin_and_release(&dsb->vma, 0);
>> +        dsb->cmd_buf = NULL;
>
> This can be done outside mutex_unlock();

Ok.

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

Re: [PATCH v5 03/11] drm/i915/dsb: single register write function for DSB.

[Animesh Manna]

On 9/9/2019 6:28 PM, Sharma, Shashank wrote:
>
> On 9/7/2019 4:37 PM, Animesh Manna wrote:
>> DSB support single register write through opcode 0x1. Generic
>> api created which accumulate all single register write in a batch
>> buffer and once DSB is triggered, it will program all the registers
>> at the same time.
>>
>> v1: Initial version.
>> v2: Unused macro removed and cosmetic changes done. (Shashank)
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dsb.c | 30 ++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dsb.h |  9 +++++++
>>   2 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c 
>> b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index cba5c8d37659..150be81fdfb3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -9,6 +9,13 @@
>>     #define DSB_BUF_SIZE    (2 * PAGE_SIZE)
>>   +/* DSB opcodes. */
>> +#define DSB_OPCODE_SHIFT        24
>> +#define DSB_OPCODE_MMIO_WRITE        0x1
>> +#define DSB_OPCODE_INDEXED_WRITE    0x9
>> +#define DSB_BYTE_EN            0xF
>> +#define DSB_BYTE_EN_SHIFT        20
>> +
>>   struct intel_dsb *
>>   intel_dsb_get(struct intel_crtc *crtc)
>>   {
>> @@ -46,6 +53,7 @@ intel_dsb_get(struct intel_crtc *crtc)
>>           goto err;
>>       }
>>       dsb->vma = vma;
>> +    dsb->free_pos = 0;
> This should be done in dsb_put();
>>     err:
>>       intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>> @@ -68,3 +76,25 @@ void intel_dsb_put(struct intel_dsb *dsb)
>>           mutex_unlock(&i915->drm.struct_mutex);
>>       }
>>   }
>> +
>> +void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 
>> val)
>> +{
>> +    struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
>> +    struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +    u32 *buf = dsb->cmd_buf;
>> +
>> +    if (!buf) {
>> +        I915_WRITE(reg, val);
>> +        return;
>> +    }
>> +
>> +    if (WARN_ON(dsb->free_pos >= DSB_BUF_SIZE)) {
>> +        DRM_DEBUG_KMS("DSB buffer overflow.\n");
>
> Lets remove this '.' in the end, to maintain consistency in the log.

Sure.

Regards,
Animesh

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

Re: [PATCH v5 05/11] drm/i915/dsb: Check DSB engine status.

[Animesh Manna]

On 9/9/2019 6:43 PM, Sharma, Shashank wrote:
>
> On 9/7/2019 4:37 PM, Animesh Manna wrote:
>> 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>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dsb.c | 9 +++++++++
>>   drivers/gpu/drm/i915/i915_reg.h          | 7 +++++++
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c 
>> b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index 0f55ed683d41..2c8415518c65 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -17,6 +17,15 @@
>>   #define DSB_BYTE_EN_SHIFT        20
>>   #define DSB_REG_VALUE_MASK        0xfffff
>>   +static inline bool is_dsb_busy(struct intel_dsb *dsb)
>> +{
>> +    struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
>> +    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)
>>   {
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 006cffd56be2..a3099f712ae6 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -11676,4 +11676,11 @@ 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)
>
> Why is pipe in () ?

mmio offset per DSB depend on pipe and dsb-id (3 DSB per pipe.)
offset of PIPE_A DSB1-> 70B00
offset of PIPE_B DSB1-> 71B00 and so on...

Regards,
Animesh

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

Re: [PATCH v5 08/11] drm/i915/dsb: added dsb refcount to synchronize between get/put.

[Animesh Manna]

On 9/9/2019 6:51 PM, Sharma, Shashank wrote:
>
> On 9/7/2019 4:37 PM, Animesh Manna wrote:
>> The lifetime of command buffer can be controlled by the dsb user
>> throuh refcount. Added refcount mechanism is dsb get/put call
>> which create/destroy dsb context.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dsb.c | 22 ++++++++++++++++------
>>   drivers/gpu/drm/i915/display/intel_dsb.h |  1 +
>>   2 files changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c 
>> b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index 853685751540..b951a6b5264a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -78,7 +78,12 @@ intel_dsb_get(struct intel_crtc *crtc)
>>       struct intel_dsb *dsb = &crtc->dsb;
>>       intel_wakeref_t wakeref;
>>   -    if ((!HAS_DSB(i915)) || dsb->cmd_buf)
>> +    if (!HAS_DSB(i915))
>> +        return dsb;
>> +
>> +    atomic_inc(&dsb->refcount);
>> +
>
> As discussed we are not solving any problem with reference counting, 
> rather, we are adding a complexity here. It may be useful, when we are 
> extending single instance of DSB to DSB pool but not right now.
>
> I would say we drop this patch all together, and just have the simple 
> implementation now.

Tried to have synchronization in single thread multiple user scenario 
through ref-count mechanism, but can be taken in phased approach in future.

Regards,
Animesh

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

Re: [PATCH v5 05/11] drm/i915/dsb: Check DSB engine status.

[Sharma, Shashank]

> -----Original Message-----
> From: Manna, Animesh
> Sent: Monday, September 9, 2019 10:27 PM
> To: Sharma, Shashank <shashank.sharma@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Thierry, Michel <michel.thierry@intel.com>; Nikula, Jani <jani.nikula@intel.com>;
> Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: Re: [PATCH v5 05/11] drm/i915/dsb: Check DSB engine status.
> 
> 
> 
> On 9/9/2019 6:43 PM, Sharma, Shashank wrote:
> >
> > On 9/7/2019 4:37 PM, Animesh Manna wrote:
> >> 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>
> >> Cc: Shashank Sharma <shashank.sharma@intel.com>
> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_dsb.c | 9 +++++++++
> >>   drivers/gpu/drm/i915/i915_reg.h          | 7 +++++++
> >>   2 files changed, 16 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> >> b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> index 0f55ed683d41..2c8415518c65 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> @@ -17,6 +17,15 @@
> >>   #define DSB_BYTE_EN_SHIFT        20
> >>   #define DSB_REG_VALUE_MASK        0xfffff
> >>   +static inline bool is_dsb_busy(struct intel_dsb *dsb)
> >> +{
> >> +    struct intel_crtc *crtc = container_of(dsb, typeof(*crtc), dsb);
> >> +    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)
> >>   {
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> b/drivers/gpu/drm/i915/i915_reg.h index 006cffd56be2..a3099f712ae6
> >> 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -11676,4 +11676,11 @@ 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)
> >
> > Why is pipe in () ?
> 
> mmio offset per DSB depend on pipe and dsb-id (3 DSB per pipe.) offset of PIPE_A
> DSB1-> 70B00 offset of PIPE_B DSB1-> 71B00 and so on...
> 
The question here is why is the 'pipe' in braces ? why is it '(pipe)', instead of 'pipe'. I don’t see a reason. 
- Shashank
> Regards,
> Animesh

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

Re: [PATCH v5 05/11] drm/i915/dsb: Check DSB engine status.

[Animesh Manna]

On 9/10/2019 8:44 AM, Sharma, Shashank wrote:
>
>> -----Original Message-----
>> From: Manna, Animesh
>> Sent: Monday, September 9, 2019 10:27 PM
>> To: Sharma, Shashank <shashank.sharma@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Thierry, Michel <michel.thierry@intel.com>; Nikula, Jani <jani.nikula@intel.com>;
>> Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Subject: Re: [PATCH v5 05/11] drm/i915/dsb: Check DSB engine status.
>>
>>
>>
>> On 9/9/2019 6:43 PM, Sharma, Shashank wrote:
>>> On 9/7/2019 4:37 PM, Animesh Manna wrote:
>>>> +#define _DSBSL_INSTANCE_BASE        0x70B00
>>>> +#define DSBSL_INSTANCE(pipe, id)    (_DSBSL_INSTANCE_BASE + \
>>>> +                     (pipe) * 0x1000 + (id) * 100)
>>> Why is pipe in () ?
>> mmio offset per DSB depend on pipe and dsb-id (3 DSB per pipe.) offset of PIPE_A
>> DSB1-> 70B00 offset of PIPE_B DSB1-> 71B00 and so on...
>>
> The question here is why is the 'pipe' in braces ? why is it '(pipe)', instead of 'pipe'. I don’t see a reason.
Just followed general macro definition rule, am I missing anything.
for example: #define double(x) x*2
double(2+3) will give wrong result if we do not define as
#define double(x) (x)*2

Regards,
Animesh
>   
> - Shashank
>> Regards,
>> Animesh

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