[PATCH v4 00/10] DSB enablement.

[PATCH v4 00/10] 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.

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 (10):
  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: 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    |  64 ++--
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/display/intel_dsb.c      | 337 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h      |  49 +++
 drivers/gpu/drm/i915/i915_drv.h               |   5 +
 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, 458 insertions(+), 24 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 v4 01/10] 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>
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 v4 02/10] 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)

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    |  3 +
 drivers/gpu/drm/i915/display/intel_dsb.c      | 83 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++
 drivers/gpu/drm/i915/i915_drv.h               |  1 +
 5 files changed, 119 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 61277a87dbe7..da36867189cb 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[MAX_DSB_PER_PIPE];
 };
 
 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..007ef13481d5
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -0,0 +1,83 @@
+// 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;
+
+	for (i = 0; i < MAX_DSB_PER_PIPE; i++)
+		if (!crtc->dsb[i].cmd_buf)
+			break;
+
+	if (WARN_ON(i == MAX_DSB_PER_PIPE))
+		return NULL;
+
+	dsb = &crtc->dsb[i];
+	dsb->id = i;
+	dsb->crtc = crtc;
+	if (!HAS_DSB(i915))
+		return dsb;
+
+	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;
+	struct drm_i915_private *i915;
+
+	if (!dsb)
+		return;
+
+	crtc = dsb->crtc;
+	i915 = to_i915(crtc->base.dev);
+
+	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..4a4091cadc1e
--- /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;
+	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 v4 03/10] 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>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 29 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsb.h |  9 ++++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 007ef13481d5..5cde6412a89b 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)
 {
@@ -81,3 +88,25 @@ void intel_dsb_put(struct intel_dsb *dsb)
 		mutex_unlock(&i915->drm.struct_mutex);
 	}
 }
+
+void intel_dsb_reg_write(struct intel_dsb *dsb, i915_reg_t reg, u32 val)
+{
+	struct intel_crtc *crtc = dsb->crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 *buf = dsb->cmd_buf;
+
+	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 4a4091cadc1e..88b2ca031917 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -6,6 +6,8 @@
 #ifndef _INTEL_DSB_H
 #define _INTEL_DSB_H
 
+#include "i915_reg.h"
+
 struct intel_crtc;
 struct i915_vma;
 
@@ -22,10 +24,17 @@ struct intel_dsb {
 	enum dsb_id id;
 	u32 *cmd_buf;
 	struct i915_vma *vma;
+
+	/*
+	 * free_pos will point the first free entry position
+	 * and help in calculating 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 v4 04/10] 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 5cde6412a89b..934d7aca3bc5 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)
@@ -89,6 +90,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 = dsb->crtc;
+	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 = dsb->crtc;
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h
index 88b2ca031917..ff3ea147ccf9 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -30,11 +30,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 v4 05/10] 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 +++++++++
 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 934d7aca3bc5..1022b5720d17 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 = 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)
 {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3cfdab18c0cf..3e35d7a4d2c4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11670,4 +11670,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 v4 06/10] 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>
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 1022b5720d17..485748e63a94 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 = 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);
+
+	POSTING_READ(DSB_CTRL(pipe, dsb->id));
+	return true;
+}
+
+static inline 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);
+
+	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 3e35d7a4d2c4..067f86d3a9de 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11675,6 +11675,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 v4 07/10] 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>
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 485748e63a94..daa1cae5a992 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -225,3 +225,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 = dsb->crtc;
+	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 ff3ea147ccf9..0bf0c96c1df5 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.h
+++ b/drivers/gpu/drm/i915/display/intel_dsb.h
@@ -44,5 +44,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 067f86d3a9de..ea1107e05c4d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -11674,6 +11674,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 v4 08/10] 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.

v1: Initial version.
v2: Directly call dsb-api at callsites. (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 | 64 ++++++++++++++--------
 drivers/gpu/drm/i915/i915_drv.h            |  2 +
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 71a0201437a9..e4090d8e4547 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
 	const struct drm_color_lut *lut = blob->data;
 	int i, lut_size = drm_color_lut_size(blob);
 	enum pipe pipe = crtc->pipe;
+	struct intel_dsb *dsb = dev_priv->dsb;
 
-	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
-		   PAL_PREC_AUTO_INCREMENT);
+	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
+			    prec_index | PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < hw_lut_size; i++) {
 		/* We discard half the user entries in split gamma mode */
 		const struct drm_color_lut *entry =
 			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
 
-		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
+		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
+					    ilk_lut_10(entry));
 	}
 
 	/*
 	 * Reset the index, otherwise it prevents the legacy palette to be
 	 * written properly.
 	 */
-	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
 }
 
 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 = dev_priv->dsb;
 	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,9 +628,12 @@ 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);
 	}
 }
 
@@ -788,12 +795,13 @@ icl_load_gcmax(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);
+	struct intel_dsb *dsb = dev_priv->dsb;
 	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);
 }
 
 static void
@@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
 	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 = dev_priv->dsb;
 	enum pipe pipe = crtc->pipe;
 	u32 i;
 
@@ -813,15 +822,16 @@ 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));
 	}
 }
 
@@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 	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 = dev_priv->dsb;
 	enum pipe pipe = crtc->pipe;
 	u32 i;
 
@@ -847,11 +858,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,8 +881,10 @@ 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 */
@@ -980,7 +995,10 @@ 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 = intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));
 	dev_priv->display.load_luts(crtc_state);
+	intel_dsb_commit(dev_priv->dsb);
+	intel_dsb_put(dev_priv->dsb);
 }
 
 void intel_color_commit(const struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7aa11e3dd413..98f546c4ad49 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1760,6 +1760,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.
-- 
2.22.0

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

[PATCH v4 09/10] 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>
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 1974e4c78a43..613210385fb5 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 v4 10/10] 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>
Acked-by: 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 | 68 ++++++++++++++++++++++++
 2 files changed, 77 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 daa1cae5a992..ee4ca0e629c6 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)
 {
@@ -121,6 +149,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;
@@ -141,6 +177,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)
 {
@@ -204,6 +253,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 = dsb->crtc;
@@ -226,6 +287,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;
-- 
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. (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

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

total: 0 errors, 1 warnings, 0 checks, 137 lines checked
4c2a8e0b6ca6 drm/i915/dsb: single register write function for DSB.
0c39c376c3cd drm/i915/dsb: Indexed register write function for DSB.
e83352fdf612 drm/i915/dsb: Check DSB engine status.
1a17ca9e22a5 drm/i915/dsb: functions to enable/disable DSB engine.
f45c02f8d61e drm/i915/dsb: function to trigger workload execution of DSB.
f3de20d9f1d2 drm/i915/dsb: Enable gamma lut programming using DSB.
706ebab10a9b drm/i915/dsb: Enable DSB for gen12.
d1ac6633815b 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. (rev4)

[Patchwork]

== Series Details ==

Series: DSB enablement. (rev4)
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

Re: [PATCH v4 08/10] drm/i915/dsb: Enable gamma lut programming using DSB.

[Jani Nikula]

On Fri, 30 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.
>
> v1: Initial version.
> v2: Directly call dsb-api at callsites. (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 | 64 ++++++++++++++--------
>  drivers/gpu/drm/i915/i915_drv.h            |  2 +
>  2 files changed, 43 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 71a0201437a9..e4090d8e4547 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
>  	const struct drm_color_lut *lut = blob->data;
>  	int i, lut_size = drm_color_lut_size(blob);
>  	enum pipe pipe = crtc->pipe;
> +	struct intel_dsb *dsb = dev_priv->dsb;

No. Don't put dsb in dev_priv. You have 12 DSB engines, 3 per pipe. You
have intel_crtc for that.

Was this not clear from my previous review?

You have tons of functions here that will never have a DSB engine, it
makes no sense to convert all of them to use the DSB.

>  
> -	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
> -		   PAL_PREC_AUTO_INCREMENT);
> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
> +			    prec_index | PAL_PREC_AUTO_INCREMENT);
>  
>  	for (i = 0; i < hw_lut_size; i++) {
>  		/* We discard half the user entries in split gamma mode */
>  		const struct drm_color_lut *entry =
>  			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>  
> -		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
> +		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
> +					    ilk_lut_10(entry));
>  	}
>  
>  	/*
>  	 * Reset the index, otherwise it prevents the legacy palette to be
>  	 * written properly.
>  	 */
> -	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
>  }
>  
>  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 = dev_priv->dsb;
>  	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,9 +628,12 @@ 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);
>  	}
>  }
>  
> @@ -788,12 +795,13 @@ icl_load_gcmax(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);
> +	struct intel_dsb *dsb = dev_priv->dsb;
>  	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);
>  }
>  
>  static void
> @@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>  	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 = dev_priv->dsb;
>  	enum pipe pipe = crtc->pipe;
>  	u32 i;
>  
> @@ -813,15 +822,16 @@ 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));
>  	}
>  }
>  
> @@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>  	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 = dev_priv->dsb;
>  	enum pipe pipe = crtc->pipe;
>  	u32 i;
>  
> @@ -847,11 +858,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,8 +881,10 @@ 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 */
> @@ -980,7 +995,10 @@ 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 = intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));

No, don't do this. Don't store crtc specific info in a device global
structure.

>  	dev_priv->display.load_luts(crtc_state);
> +	intel_dsb_commit(dev_priv->dsb);
> +	intel_dsb_put(dev_priv->dsb);

Please localize the use of DSB where needed. Move the gets and puts
within the relevant platform specific ->load_luts hooks.

>  }
>  
>  void intel_color_commit(const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7aa11e3dd413..98f546c4ad49 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1760,6 +1760,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.

-- 
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 v4 02/10] drm/i915/dsb: DSB context creation.

[Jani Nikula]

On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@intel.com> 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)
>
> 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    |  3 +
>  drivers/gpu/drm/i915/display/intel_dsb.c      | 83 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++
>  drivers/gpu/drm/i915/i915_drv.h               |  1 +
>  5 files changed, 119 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 61277a87dbe7..da36867189cb 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[MAX_DSB_PER_PIPE];
>  };
>  
>  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..007ef13481d5
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -0,0 +1,83 @@
> +// 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;
> +
> +	for (i = 0; i < MAX_DSB_PER_PIPE; i++)
> +		if (!crtc->dsb[i].cmd_buf)
> +			break;
> +
> +	if (WARN_ON(i == MAX_DSB_PER_PIPE))
> +		return NULL;
> +
> +	dsb = &crtc->dsb[i];
> +	dsb->id = i;
> +	dsb->crtc = crtc;
> +	if (!HAS_DSB(i915))
> +		return dsb;

Why do you go through any of the trouble if there is no DSB? Just early
return NULL, and make the write functions handle NULL dsb pointer.

> +
> +	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;
> +	struct drm_i915_private *i915;
> +
> +	if (!dsb)
> +		return;
> +
> +	crtc = dsb->crtc;
> +	i915 = to_i915(crtc->base.dev);
> +
> +	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..4a4091cadc1e
> --- /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;
> +	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"

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

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

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6808 -> Patchwork_14237
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_linear_blits@basic:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/fi-icl-u3/igt@gem_linear_blits@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/fi-icl-u3/igt@gem_linear_blits@basic.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       [INCOMPLETE][3] ([fdo#107718]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/fi-blb-e6850/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/fi-blb-e6850/igt@i915_module_load@reload.html
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724] / [fdo#111214]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/fi-icl-u3/igt@i915_module_load@reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/fi-icl-u3/igt@i915_module_load@reload.html

  * igt@i915_selftest@live_requests:
    - {fi-icl-guc}:       [INCOMPLETE][7] ([fdo#107713] / [fdo#109644] / [fdo#110464]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/fi-icl-guc/igt@i915_selftest@live_requests.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/fi-icl-guc/igt@i915_selftest@live_requests.html

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

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

  [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#109644]: https://bugs.freedesktop.org/show_bug.cgi?id=109644
  [fdo#110464]: https://bugs.freedesktop.org/show_bug.cgi?id=110464
  [fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214


Participating hosts (53 -> 45)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6808 -> Patchwork_14237

  CI-20190529: 20190529
  CI_DRM_6808: 5eb3ce9411651f22f64a7bd4e83e5e19796247ac @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5160: 522bab4503db480dddd1985c9976bc0a6e6da986 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14237: d1ac6633815bffed9689fcbbce6f6210247e3cad @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d1ac6633815b drm/i915/dsb: Documentation for DSB.
706ebab10a9b drm/i915/dsb: Enable DSB for gen12.
f3de20d9f1d2 drm/i915/dsb: Enable gamma lut programming using DSB.
f45c02f8d61e drm/i915/dsb: function to trigger workload execution of DSB.
1a17ca9e22a5 drm/i915/dsb: functions to enable/disable DSB engine.
e83352fdf612 drm/i915/dsb: Check DSB engine status.
0c39c376c3cd drm/i915/dsb: Indexed register write function for DSB.
4c2a8e0b6ca6 drm/i915/dsb: single register write function for DSB.
726f4aedf3ef drm/i915/dsb: DSB context creation.
d4d1dfe5ffbf 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_14237/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for DSB enablement. (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6808_full -> Patchwork_14237_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110841])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_eio@reset-stress:
    - shard-skl:          [PASS][3] -> [FAIL][4] ([fdo#109661])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-skl6/igt@gem_eio@reset-stress.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-skl8/igt@gem_eio@reset-stress.html
    - shard-glk:          [PASS][5] -> [FAIL][6] ([fdo#109661])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-glk6/igt@gem_eio@reset-stress.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-glk2/igt@gem_eio@reset-stress.html

  * igt@gem_exec_flush@basic-wb-rw-default:
    - shard-apl:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103927]) +4 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-apl5/igt@gem_exec_flush@basic-wb-rw-default.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-apl3/igt@gem_exec_flush@basic-wb-rw-default.html

  * igt@gem_exec_parallel@bcs0-contexts:
    - shard-hsw:          [PASS][9] -> [FAIL][10] ([fdo#111469])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-hsw2/igt@gem_exec_parallel@bcs0-contexts.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-hsw6/igt@gem_exec_parallel@bcs0-contexts.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#111325]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb6/igt@gem_exec_schedule@wide-bsd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-skl:          [PASS][13] -> [INCOMPLETE][14] ([fdo#104108])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-skl2/igt@gem_workarounds@suspend-resume-fd.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-skl4/igt@gem_workarounds@suspend-resume-fd.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [PASS][15] -> [FAIL][16] ([fdo#105767])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-hsw4/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-iclb:         [PASS][17] -> [INCOMPLETE][18] ([fdo#107713])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb1/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb1/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [PASS][19] -> [DMESG-WARN][20] ([fdo#108566]) +4 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-apl4/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#103167]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#108145] / [fdo#110403])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][25] -> [FAIL][26] ([fdo#103166])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.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_6808/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb5/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][29] -> [FAIL][30] ([fdo#108341])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb5/igt@kms_psr@no_drrs.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb5/igt@kms_psr@psr2_cursor_render.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][33] -> [FAIL][34] ([fdo#99912])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-apl6/igt@kms_setmode@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-apl5/igt@kms_setmode@basic.html

  * igt@perf@blocking:
    - shard-skl:          [PASS][35] -> [FAIL][36] ([fdo#110728])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-skl3/igt@perf@blocking.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-skl2/igt@perf@blocking.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][37] -> [SKIP][38] ([fdo#109276]) +18 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb4/igt@prime_busy@hang-bsd2.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb3/igt@prime_busy@hang-bsd2.html

  
#### Possible fixes ####

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [SKIP][39] ([fdo#109276]) -> [PASS][40] +23 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb3/igt@gem_exec_schedule@independent-bsd2.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb4/igt@gem_exec_schedule@independent-bsd2.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [SKIP][41] ([fdo#111325]) -> [PASS][42] +7 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb5/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [DMESG-WARN][43] ([fdo#108566]) -> [PASS][44] +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-apl1/igt@gem_softpin@noreloc-s3.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-apl2/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_rpm@pm-caching:
    - shard-iclb:         [INCOMPLETE][45] ([fdo#107713] / [fdo#108840]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb2/igt@i915_pm_rpm@pm-caching.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb5/igt@i915_pm_rpm@pm-caching.html

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
    - shard-glk:          [FAIL][47] ([fdo#104873]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-glk3/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-glk7/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-ytiled:
    - shard-skl:          [FAIL][49] ([fdo#103184] / [fdo#103232]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-skl3/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-ytiled.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-skl2/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-ytiled.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [INCOMPLETE][51] ([fdo#109507]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-skl6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-skl8/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render:
    - shard-iclb:         [FAIL][53] ([fdo#103167]) -> [PASS][54] +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - shard-skl:          [FAIL][55] ([fdo#103191]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-skl9/igt@kms_pipe_crc_basic@hang-read-crc-pipe-c.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-skl4/igt@kms_pipe_crc_basic@hang-read-crc-pipe-c.html

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

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][59] ([fdo#109441]) -> [PASS][60] +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb7/igt@kms_psr@psr2_primary_mmap_cpu.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][61] ([fdo#111329]) -> [SKIP][62] ([fdo#109276])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb4/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][63] ([fdo#111330]) -> [SKIP][64] ([fdo#109276])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb5/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [SKIP][65] ([fdo#109276]) -> [FAIL][66] ([fdo#111330]) +2 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-iclb6/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][67] ([fdo#108566]) -> [INCOMPLETE][68] ([fdo#103927])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6808/shard-apl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14237/shard-apl5/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [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#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [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#111469]: https://bugs.freedesktop.org/show_bug.cgi?id=111469
  [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_6808 -> Patchwork_14237

  CI-20190529: 20190529
  CI_DRM_6808: 5eb3ce9411651f22f64a7bd4e83e5e19796247ac @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5160: 522bab4503db480dddd1985c9976bc0a6e6da986 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14237: d1ac6633815bffed9689fcbbce6f6210247e3cad @ 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_14237/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 02/10] drm/i915/dsb: DSB context creation.

[Sharma, Shashank]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani
> Nikula
> Sent: Friday, August 30, 2019 7:06 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Thierry, Michel <michel.thierry@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v4 02/10] drm/i915/dsb: DSB context creation.
> 
> On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@intel.com> 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)
> >
> > 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    |  3 +
> >  drivers/gpu/drm/i915/display/intel_dsb.c      | 83 +++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++
> >  drivers/gpu/drm/i915/i915_drv.h               |  1 +
> >  5 files changed, 119 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 61277a87dbe7..da36867189cb 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[MAX_DSB_PER_PIPE];
> >  };
> >
> >  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..007ef13481d5
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> > @@ -0,0 +1,83 @@
> > +// 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;
> > +
> > +	for (i = 0; i < MAX_DSB_PER_PIPE; i++)
> > +		if (!crtc->dsb[i].cmd_buf)
> > +			break;
> > +
> > +	if (WARN_ON(i == MAX_DSB_PER_PIPE))
> > +		return NULL;
> > +
> > +	dsb = &crtc->dsb[i];
> > +	dsb->id = i;
> > +	dsb->crtc = crtc;
> > +	if (!HAS_DSB(i915))
> > +		return dsb;
> 
> Why do you go through any of the trouble if there is no DSB? Just early return NULL,
> and make the write functions handle NULL dsb pointer.
> 
I agree on this. Even I thought it would be better if we populate the DSB ptr only when we find a valid DSB support on the platform. And the caller of get() can understand this and act accordingly. 
> > +
> > +	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;
> > +	struct drm_i915_private *i915;
> > +
> > +	if (!dsb)
> > +		return;
> > +
> > +	crtc = dsb->crtc;
> > +	i915 = to_i915(crtc->base.dev);
> > +
> > +	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..4a4091cadc1e
> > --- /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;
> > +	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"
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 08/10] drm/i915/dsb: Enable gamma lut programming using DSB.

[Sharma, Shashank]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani
> Nikula
> Sent: Friday, August 30, 2019 7:02 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v4 08/10] drm/i915/dsb: Enable gamma lut
> programming using DSB.
> 
> On Fri, 30 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.
> >
> > v1: Initial version.
> > v2: Directly call dsb-api at callsites. (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 | 64 ++++++++++++++--------
> >  drivers/gpu/drm/i915/i915_drv.h            |  2 +
> >  2 files changed, 43 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > b/drivers/gpu/drm/i915/display/intel_color.c
> > index 71a0201437a9..e4090d8e4547 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
> >  	const struct drm_color_lut *lut = blob->data;
> >  	int i, lut_size = drm_color_lut_size(blob);
> >  	enum pipe pipe = crtc->pipe;
> > +	struct intel_dsb *dsb = dev_priv->dsb;
> 
> No. Don't put dsb in dev_priv. You have 12 DSB engines, 3 per pipe. You have
> intel_crtc for that.
> 
> Was this not clear from my previous review?
> 
> You have tons of functions here that will never have a DSB engine, it makes no sense
> to convert all of them to use the DSB.
> 
Jani, 
I was thinking even among the 3 engines available per pipe, would it make more sense to keep them reserve on the basis of user ? like DSB0 for all pipe operations, DSB1 for all plane, and DSB2 for all encoder related stuff. We can create a DSB user (like we have for scaler) and index these engines based on that. Do you think so ?

> >
> > -	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
> > -		   PAL_PREC_AUTO_INCREMENT);
> > +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
> > +			    prec_index | PAL_PREC_AUTO_INCREMENT);
> >
> >  	for (i = 0; i < hw_lut_size; i++) {
> >  		/* We discard half the user entries in split gamma mode */
> >  		const struct drm_color_lut *entry =
> >  			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
> >
> > -		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
> > +		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
> > +					    ilk_lut_10(entry));
> >  	}
> >
> >  	/*
> >  	 * Reset the index, otherwise it prevents the legacy palette to be
> >  	 * written properly.
> >  	 */
> > -	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> > +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
> >  }
> >
> >  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 = dev_priv->dsb;
> >  	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,9 +628,12 @@ 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);
> >  	}
> >  }
> >
> > @@ -788,12 +795,13 @@ icl_load_gcmax(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);
> > +	struct intel_dsb *dsb = dev_priv->dsb;
> >  	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);
> >  }
> >
> >  static void
> > @@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct
> intel_crtc_state *crtc_state)
> >  	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 = dev_priv->dsb;
> >  	enum pipe pipe = crtc->pipe;
> >  	u32 i;
> >
> > @@ -813,15 +822,16 @@ 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));
> >  	}
> >  }
> >
> > @@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct
> intel_crtc_state *crtc_state)
> >  	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 = dev_priv->dsb;
> >  	enum pipe pipe = crtc->pipe;
> >  	u32 i;
> >
> > @@ -847,11 +858,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,8 +881,10 @@ 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 */ @@
> > -980,7 +995,10 @@ 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 = intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));
> 
> No, don't do this. Don't store crtc specific info in a device global structure.
> 
> >  	dev_priv->display.load_luts(crtc_state);
> > +	intel_dsb_commit(dev_priv->dsb);
> > +	intel_dsb_put(dev_priv->dsb);
> 
> Please localize the use of DSB where needed. Move the gets and puts within the
> relevant platform specific ->load_luts hooks.
> 
> >  }
> >
> >  void intel_color_commit(const struct intel_crtc_state *crtc_state)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index 7aa11e3dd413..98f546c4ad49
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1760,6 +1760,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.
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 08/10] drm/i915/dsb: Enable gamma lut programming using DSB.

[Sharma, Shashank]

On 9/3/2019 1:29 PM, Jani Nikula wrote:
> On Tue, 03 Sep 2019, "Sharma, Shashank" <shashank.sharma@intel.com> wrote:
>>> -----Original Message-----
>>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani
>>> Nikula
>>> Sent: Friday, August 30, 2019 7:02 PM
>>> To: Manna, Animesh <animesh.manna@intel.com>; intel-gfx@lists.freedesktop.org
>>> Subject: Re: [Intel-gfx] [PATCH v4 08/10] drm/i915/dsb: Enable gamma lut
>>> programming using DSB.
>>> You have tons of functions here that will never have a DSB engine, it
>>> makes no sense to convert all of them to use the DSB.
>>>
>> Jani,
>> I was thinking even among the 3 engines available per pipe, would it
>> make more sense to keep them reserve on the basis of user ? like DSB0
>> for all pipe operations, DSB1 for all plane, and DSB2 for all encoder
>> related stuff. We can create a DSB user (like we have for scaler) and
>> index these engines based on that. Do you think so ?
> And perhaps use some DSB engines to write immediately, some to write at
> vblank. However, all of this should be postponed to follow-up work.
>
> For now, if we use intel_dsb_write and friends with the dsb parameter as
> local variable passed in, converting to use a different engine is a
> metter of changing the preceding intel_dsb_get call to fetch the dsb
> pointer.
>
> Considering the progress of a patch series, the focus should be on
> getting patches merged. Getting the minimum sebsible enabling of DSB
> merged should be the focus here. The further iteration should happen
> in-tree, not out-of-tree.

Sure, it makes sense to simplify this in steps.

- Shashank

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

Re: [PATCH v4 08/10] drm/i915/dsb: Enable gamma lut programming using DSB.

[Jani Nikula]

On Fri, 30 Aug 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> On Fri, 30 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.
>>
>> v1: Initial version.
>> v2: Directly call dsb-api at callsites. (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 | 64 ++++++++++++++--------
>>  drivers/gpu/drm/i915/i915_drv.h            |  2 +
>>  2 files changed, 43 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>> index 71a0201437a9..e4090d8e4547 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
>>  	const struct drm_color_lut *lut = blob->data;
>>  	int i, lut_size = drm_color_lut_size(blob);
>>  	enum pipe pipe = crtc->pipe;
>> +	struct intel_dsb *dsb = dev_priv->dsb;
>
> No. Don't put dsb in dev_priv. You have 12 DSB engines, 3 per pipe. You
> have intel_crtc for that.
>
> Was this not clear from my previous review?
>
> You have tons of functions here that will never have a DSB engine, it
> makes no sense to convert all of them to use the DSB.

To clarify:

All functions that could and should use the DSB, need to be converted to
use intel_dsb_write and friends. That means replacing I915_WRITE with
the relevant intel_dsb_write calls. *NOT* having if (dsb)
intel_dsb_write else I915_WRITE. As agreed a long time ago,
intel_dsb_write should handle the non-DSB cases by falling back to
regular intel_uncore_write.

My point is, if there are functions that will never be called on a
platform that has DSB, there's no point in converting them over to use
DSB. Obviously there are e.g. legacy gamma functions that also get
called on newer platforms.

Maybe I was hasty in my assesment, need to double check if all of these
paths are reachable from DSB platforms.


BR,
Jani.


>
>>  
>> -	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
>> -		   PAL_PREC_AUTO_INCREMENT);
>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
>> +			    prec_index | PAL_PREC_AUTO_INCREMENT);
>>  
>>  	for (i = 0; i < hw_lut_size; i++) {
>>  		/* We discard half the user entries in split gamma mode */
>>  		const struct drm_color_lut *entry =
>>  			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>>  
>> -		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
>> +		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>> +					    ilk_lut_10(entry));
>>  	}
>>  
>>  	/*
>>  	 * Reset the index, otherwise it prevents the legacy palette to be
>>  	 * written properly.
>>  	 */
>> -	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
>>  }
>>  
>>  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 = dev_priv->dsb;
>>  	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,9 +628,12 @@ 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);
>>  	}
>>  }
>>  
>> @@ -788,12 +795,13 @@ icl_load_gcmax(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);
>> +	struct intel_dsb *dsb = dev_priv->dsb;
>>  	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);
>>  }
>>  
>>  static void
>> @@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>>  	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 = dev_priv->dsb;
>>  	enum pipe pipe = crtc->pipe;
>>  	u32 i;
>>  
>> @@ -813,15 +822,16 @@ 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));
>>  	}
>>  }
>>  
>> @@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>>  	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 = dev_priv->dsb;
>>  	enum pipe pipe = crtc->pipe;
>>  	u32 i;
>>  
>> @@ -847,11 +858,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,8 +881,10 @@ 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 */
>> @@ -980,7 +995,10 @@ 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 = intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));
>
> No, don't do this. Don't store crtc specific info in a device global
> structure.
>
>>  	dev_priv->display.load_luts(crtc_state);
>> +	intel_dsb_commit(dev_priv->dsb);
>> +	intel_dsb_put(dev_priv->dsb);
>
> Please localize the use of DSB where needed. Move the gets and puts
> within the relevant platform specific ->load_luts hooks.
>
>>  }
>>  
>>  void intel_color_commit(const struct intel_crtc_state *crtc_state)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 7aa11e3dd413..98f546c4ad49 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1760,6 +1760,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.

-- 
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 v4 02/10] drm/i915/dsb: DSB context creation.

[Animesh Manna]

Hi,


On 8/30/2019 7:05 PM, Jani Nikula wrote:
> On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@intel.com> 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)
>>
>> 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    |  3 +
>>   drivers/gpu/drm/i915/display/intel_dsb.c      | 83 +++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++
>>   drivers/gpu/drm/i915/i915_drv.h               |  1 +
>>   5 files changed, 119 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 61277a87dbe7..da36867189cb 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[MAX_DSB_PER_PIPE];
>>   };
>>   
>>   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..007ef13481d5
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -0,0 +1,83 @@
>> +// 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;
>> +
>> +	for (i = 0; i < MAX_DSB_PER_PIPE; i++)
>> +		if (!crtc->dsb[i].cmd_buf)
>> +			break;
>> +
>> +	if (WARN_ON(i == MAX_DSB_PER_PIPE))
>> +		return NULL;
>> +
>> +	dsb = &crtc->dsb[i];
>> +	dsb->id = i;
>> +	dsb->crtc = crtc;
>> +	if (!HAS_DSB(i915))
>> +		return dsb;
> Why do you go through any of the trouble if there is no DSB? Just early
> return NULL, and make the write functions handle NULL dsb pointer.

Handing NULL dsb pointer in write function is easy, but getting dev_priv 
which is needed for I915_WRITE is tricky with the following function 
prototypes which were agreed (I can understand  it was discussed long 
back, so added below). Good to get your suggestion here.

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);
void intel_dsb_commit(struct intel_dsb *dsb);

Regards,
Animesh


>
>> +
>> +	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;
>> +	struct drm_i915_private *i915;
>> +
>> +	if (!dsb)
>> +		return;
>> +
>> +	crtc = dsb->crtc;
>> +	i915 = to_i915(crtc->base.dev);
>> +
>> +	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..4a4091cadc1e
>> --- /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;
>> +	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 v4 08/10] drm/i915/dsb: Enable gamma lut programming using DSB.

[Animesh Manna]

Hi,


On 9/3/2019 1:38 PM, Jani Nikula wrote:
> On Fri, 30 Aug 2019, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Fri, 30 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.
>>>
>>> v1: Initial version.
>>> v2: Directly call dsb-api at callsites. (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 | 64 ++++++++++++++--------
>>>   drivers/gpu/drm/i915/i915_drv.h            |  2 +
>>>   2 files changed, 43 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>>> index 71a0201437a9..e4090d8e4547 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>>> @@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
>>>   	const struct drm_color_lut *lut = blob->data;
>>>   	int i, lut_size = drm_color_lut_size(blob);
>>>   	enum pipe pipe = crtc->pipe;
>>> +	struct intel_dsb *dsb = dev_priv->dsb;
>> No. Don't put dsb in dev_priv. You have 12 DSB engines, 3 per pipe. You
>> have intel_crtc for that.
>>
>> Was this not clear from my previous review?
>>
>> You have tons of functions here that will never have a DSB engine, it
>> makes no sense to convert all of them to use the DSB.
> To clarify:
>
> All functions that could and should use the DSB, need to be converted to
> use intel_dsb_write and friends. That means replacing I915_WRITE with
> the relevant intel_dsb_write calls. *NOT* having if (dsb)
> intel_dsb_write else I915_WRITE. As agreed a long time ago,
> intel_dsb_write should handle the non-DSB cases by falling back to
> regular intel_uncore_write.
>
> My point is, if there are functions that will never be called on a
> platform that has DSB, there's no point in converting them over to use
> DSB. Obviously there are e.g. legacy gamma functions that also get
> called on newer platforms.
>
> Maybe I was hasty in my assesment, need to double check if all of these
> paths are reachable from DSB platforms.

Currently multi-segmented gamma (12 bit gamma) is enabled by default for 
icl+ platform.
Will it be fine to enable only 12 bit gamma through DSB and 8-bit/10-bit 
can be enabled later based on need?

Regards,
Animesh

>
> BR,
> Jani.
>
>
>>>   
>>> -	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
>>> -		   PAL_PREC_AUTO_INCREMENT);
>>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
>>> +			    prec_index | PAL_PREC_AUTO_INCREMENT);
>>>   
>>>   	for (i = 0; i < hw_lut_size; i++) {
>>>   		/* We discard half the user entries in split gamma mode */
>>>   		const struct drm_color_lut *entry =
>>>   			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>>>   
>>> -		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
>>> +		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>>> +					    ilk_lut_10(entry));
>>>   	}
>>>   
>>>   	/*
>>>   	 * Reset the index, otherwise it prevents the legacy palette to be
>>>   	 * written properly.
>>>   	 */
>>> -	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
>>>   }
>>>   
>>>   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 = dev_priv->dsb;
>>>   	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,9 +628,12 @@ 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);
>>>   	}
>>>   }
>>>   
>>> @@ -788,12 +795,13 @@ icl_load_gcmax(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);
>>> +	struct intel_dsb *dsb = dev_priv->dsb;
>>>   	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);
>>>   }
>>>   
>>>   static void
>>> @@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>>>   	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 = dev_priv->dsb;
>>>   	enum pipe pipe = crtc->pipe;
>>>   	u32 i;
>>>   
>>> @@ -813,15 +822,16 @@ 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));
>>>   	}
>>>   }
>>>   
>>> @@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>>>   	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 = dev_priv->dsb;
>>>   	enum pipe pipe = crtc->pipe;
>>>   	u32 i;
>>>   
>>> @@ -847,11 +858,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,8 +881,10 @@ 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 */
>>> @@ -980,7 +995,10 @@ 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 = intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));
>> No, don't do this. Don't store crtc specific info in a device global
>> structure.
>>
>>>   	dev_priv->display.load_luts(crtc_state);
>>> +	intel_dsb_commit(dev_priv->dsb);
>>> +	intel_dsb_put(dev_priv->dsb);
>> Please localize the use of DSB where needed. Move the gets and puts
>> within the relevant platform specific ->load_luts hooks.
>>
>>>   }
>>>   
>>>   void intel_color_commit(const struct intel_crtc_state *crtc_state)
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 7aa11e3dd413..98f546c4ad49 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1760,6 +1760,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.

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

Re: [PATCH v4 08/10] drm/i915/dsb: Enable gamma lut programming using DSB.

[Jani Nikula]

On Tue, 03 Sep 2019, Animesh Manna <animesh.manna@intel.com> wrote:
> Hi,
>
>
> On 9/3/2019 1:38 PM, Jani Nikula wrote:
>> On Fri, 30 Aug 2019, Jani Nikula <jani.nikula@intel.com> wrote:
>>> On Fri, 30 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.
>>>>
>>>> v1: Initial version.
>>>> v2: Directly call dsb-api at callsites. (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 | 64 ++++++++++++++--------
>>>>   drivers/gpu/drm/i915/i915_drv.h            |  2 +
>>>>   2 files changed, 43 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>>>> index 71a0201437a9..e4090d8e4547 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>>>> @@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
>>>>   	const struct drm_color_lut *lut = blob->data;
>>>>   	int i, lut_size = drm_color_lut_size(blob);
>>>>   	enum pipe pipe = crtc->pipe;
>>>> +	struct intel_dsb *dsb = dev_priv->dsb;
>>> No. Don't put dsb in dev_priv. You have 12 DSB engines, 3 per pipe. You
>>> have intel_crtc for that.
>>>
>>> Was this not clear from my previous review?
>>>
>>> You have tons of functions here that will never have a DSB engine, it
>>> makes no sense to convert all of them to use the DSB.
>> To clarify:
>>
>> All functions that could and should use the DSB, need to be converted to
>> use intel_dsb_write and friends. That means replacing I915_WRITE with
>> the relevant intel_dsb_write calls. *NOT* having if (dsb)
>> intel_dsb_write else I915_WRITE. As agreed a long time ago,
>> intel_dsb_write should handle the non-DSB cases by falling back to
>> regular intel_uncore_write.
>>
>> My point is, if there are functions that will never be called on a
>> platform that has DSB, there's no point in converting them over to use
>> DSB. Obviously there are e.g. legacy gamma functions that also get
>> called on newer platforms.
>>
>> Maybe I was hasty in my assesment, need to double check if all of these
>> paths are reachable from DSB platforms.
>
> Currently multi-segmented gamma (12 bit gamma) is enabled by default for 
> icl+ platform.
> Will it be fine to enable only 12 bit gamma through DSB and 8-bit/10-bit 
> can be enabled later based on need?

Minimal enabling is fine. Let's get *something* out there first, and
iterate.

BR,
Jani.

>
> Regards,
> Animesh
>
>>
>> BR,
>> Jani.
>>
>>
>>>>   
>>>> -	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
>>>> -		   PAL_PREC_AUTO_INCREMENT);
>>>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
>>>> +			    prec_index | PAL_PREC_AUTO_INCREMENT);
>>>>   
>>>>   	for (i = 0; i < hw_lut_size; i++) {
>>>>   		/* We discard half the user entries in split gamma mode */
>>>>   		const struct drm_color_lut *entry =
>>>>   			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>>>>   
>>>> -		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
>>>> +		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>>>> +					    ilk_lut_10(entry));
>>>>   	}
>>>>   
>>>>   	/*
>>>>   	 * Reset the index, otherwise it prevents the legacy palette to be
>>>>   	 * written properly.
>>>>   	 */
>>>> -	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>>>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
>>>>   }
>>>>   
>>>>   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 = dev_priv->dsb;
>>>>   	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,9 +628,12 @@ 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);
>>>>   	}
>>>>   }
>>>>   
>>>> @@ -788,12 +795,13 @@ icl_load_gcmax(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);
>>>> +	struct intel_dsb *dsb = dev_priv->dsb;
>>>>   	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);
>>>>   }
>>>>   
>>>>   static void
>>>> @@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>>>>   	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 = dev_priv->dsb;
>>>>   	enum pipe pipe = crtc->pipe;
>>>>   	u32 i;
>>>>   
>>>> @@ -813,15 +822,16 @@ 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));
>>>>   	}
>>>>   }
>>>>   
>>>> @@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>>>>   	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 = dev_priv->dsb;
>>>>   	enum pipe pipe = crtc->pipe;
>>>>   	u32 i;
>>>>   
>>>> @@ -847,11 +858,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,8 +881,10 @@ 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 */
>>>> @@ -980,7 +995,10 @@ 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 = intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));
>>> No, don't do this. Don't store crtc specific info in a device global
>>> structure.
>>>
>>>>   	dev_priv->display.load_luts(crtc_state);
>>>> +	intel_dsb_commit(dev_priv->dsb);
>>>> +	intel_dsb_put(dev_priv->dsb);
>>> Please localize the use of DSB where needed. Move the gets and puts
>>> within the relevant platform specific ->load_luts hooks.
>>>
>>>>   }
>>>>   
>>>>   void intel_color_commit(const struct intel_crtc_state *crtc_state)
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index 7aa11e3dd413..98f546c4ad49 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -1760,6 +1760,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.
>

-- 
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 v4 02/10] drm/i915/dsb: DSB context creation.

[Jani Nikula]

On Tue, 03 Sep 2019, Animesh Manna <animesh.manna@intel.com> wrote:
> Hi,
>
>
> On 8/30/2019 7:05 PM, Jani Nikula wrote:
>> On Fri, 30 Aug 2019, Animesh Manna <animesh.manna@intel.com> 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)
>>>
>>> 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    |  3 +
>>>   drivers/gpu/drm/i915/display/intel_dsb.c      | 83 +++++++++++++++++++
>>>   drivers/gpu/drm/i915/display/intel_dsb.h      | 31 +++++++
>>>   drivers/gpu/drm/i915/i915_drv.h               |  1 +
>>>   5 files changed, 119 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 61277a87dbe7..da36867189cb 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[MAX_DSB_PER_PIPE];
>>>   };
>>>   
>>>   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..007ef13481d5
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>>> @@ -0,0 +1,83 @@
>>> +// 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;
>>> +
>>> +	for (i = 0; i < MAX_DSB_PER_PIPE; i++)
>>> +		if (!crtc->dsb[i].cmd_buf)
>>> +			break;
>>> +
>>> +	if (WARN_ON(i == MAX_DSB_PER_PIPE))
>>> +		return NULL;
>>> +
>>> +	dsb = &crtc->dsb[i];
>>> +	dsb->id = i;
>>> +	dsb->crtc = crtc;
>>> +	if (!HAS_DSB(i915))
>>> +		return dsb;
>> Why do you go through any of the trouble if there is no DSB? Just early
>> return NULL, and make the write functions handle NULL dsb pointer.
>
> Handing NULL dsb pointer in write function is easy, but getting dev_priv 
> which is needed for I915_WRITE is tricky with the following function 
> prototypes which were agreed (I can understand  it was discussed long 
> back, so added below). Good to get your suggestion here.
>
> 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);
> void intel_dsb_commit(struct intel_dsb *dsb);

Right, silly me. Need to ensure you have a fallback dsb that always has
the crtc in place to figure out i915. Both are a bit meh, but I lean
towards the latter.

BR,
Jani.

>
> Regards,
> Animesh
>
>
>>
>>> +
>>> +	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;
>>> +	struct drm_i915_private *i915;
>>> +
>>> +	if (!dsb)
>>> +		return;
>>> +
>>> +	crtc = dsb->crtc;
>>> +	i915 = to_i915(crtc->base.dev);
>>> +
>>> +	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..4a4091cadc1e
>>> --- /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;
>>> +	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

-- 
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 v4 08/10] drm/i915/dsb: Enable gamma lut programming using DSB.

[Animesh Manna]

Hi,


On 8/30/2019 7:02 PM, Jani Nikula wrote:
> On Fri, 30 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.
>>
>> v1: Initial version.
>> v2: Directly call dsb-api at callsites. (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 | 64 ++++++++++++++--------
>>   drivers/gpu/drm/i915/i915_drv.h            |  2 +
>>   2 files changed, 43 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>> index 71a0201437a9..e4090d8e4547 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
>>   	const struct drm_color_lut *lut = blob->data;
>>   	int i, lut_size = drm_color_lut_size(blob);
>>   	enum pipe pipe = crtc->pipe;
>> +	struct intel_dsb *dsb = dev_priv->dsb;
> No. Don't put dsb in dev_priv. You have 12 DSB engines, 3 per pipe. You
> have intel_crtc for that.
>
> Was this not clear from my previous review?
>
> You have tons of functions here that will never have a DSB engine, it
> makes no sense to convert all of them to use the DSB.
>
>>   
>> -	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
>> -		   PAL_PREC_AUTO_INCREMENT);
>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
>> +			    prec_index | PAL_PREC_AUTO_INCREMENT);
>>   
>>   	for (i = 0; i < hw_lut_size; i++) {
>>   		/* We discard half the user entries in split gamma mode */
>>   		const struct drm_color_lut *entry =
>>   			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>>   
>> -		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
>> +		intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>> +					    ilk_lut_10(entry));
>>   	}
>>   
>>   	/*
>>   	 * Reset the index, otherwise it prevents the legacy palette to be
>>   	 * written properly.
>>   	 */
>> -	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>> +	intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
>>   }
>>   
>>   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 = dev_priv->dsb;
>>   	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,9 +628,12 @@ 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);
>>   	}
>>   }
>>   
>> @@ -788,12 +795,13 @@ icl_load_gcmax(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);
>> +	struct intel_dsb *dsb = dev_priv->dsb;
>>   	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);
>>   }
>>   
>>   static void
>> @@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>>   	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 = dev_priv->dsb;
>>   	enum pipe pipe = crtc->pipe;
>>   	u32 i;
>>   
>> @@ -813,15 +822,16 @@ 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));
>>   	}
>>   }
>>   
>> @@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>>   	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 = dev_priv->dsb;
>>   	enum pipe pipe = crtc->pipe;
>>   	u32 i;
>>   
>> @@ -847,11 +858,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,8 +881,10 @@ 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 */
>> @@ -980,7 +995,10 @@ 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 = intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));
> No, don't do this. Don't store crtc specific info in a device global
> structure.
>
>>   	dev_priv->display.load_luts(crtc_state);
>> +	intel_dsb_commit(dev_priv->dsb);
>> +	intel_dsb_put(dev_priv->dsb);
> Please localize the use of DSB where needed. Move the gets and puts
> within the relevant platform specific ->load_luts hooks.

As per offline discussion, adding get/put call in load_luts hook will be 
tricky as crtc-state is constant, As suggested by Shashank currently 
trying to have a single element in crtc-state and will add dsb-get call 
in atomic check if there is any change in color-property. Will add 
dsb-commit & dsb-put in commit-tail. Further improvement will be done 
based on need in future.

Regards,
Animesh
>
>>   }
>>   
>>   void intel_color_commit(const struct intel_crtc_state *crtc_state)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 7aa11e3dd413..98f546c4ad49 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1760,6 +1760,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.

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