[Intel-gfx] [PATCH 1/6] drm/i915/display: Add support for global histogram

[Intel-gfx] [PATCH 1/6] drm/i915/display: Add support for global histogram

[Arun R Murthy]

API are added to enable/disable histogram. Upon generation of histogram
interrupt its notified to the usespace. User can then use this histogram
and generate a LUT which is then fed back to the enahancement block.
Histogram is an image statistics based on the input pixel stream.
LUT is a look up table consisiting of pixel data.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |   1 +
 .../drm/i915/display/intel_display_types.h    |   3 +
 .../gpu/drm/i915/display/intel_global_hist.c  | 295 ++++++++++++++++++
 .../gpu/drm/i915/display/intel_global_hist.h  | 117 +++++++
 4 files changed, 416 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/display/intel_global_hist.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_global_hist.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 5ab909ec24e5..eac1e0d7bd30 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -295,6 +295,7 @@ i915-y += \
 	display/intel_dpll.o \
 	display/intel_dpll_mgr.o \
 	display/intel_dpt.o \
+	display/intel_global_hist.o \
 	display/intel_drrs.o \
 	display/intel_dsb.o \
 	display/intel_fb.o \
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index ac6951b3e5bd..9848fcf73b87 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1462,6 +1462,9 @@ struct intel_crtc {
 	/* for loading single buffered registers during vblank */
 	struct pm_qos_request vblank_pm_qos;
 
+	/* GLOBAL_HIST data */
+	struct intel_global_hist *global_hist;
+
 #ifdef CONFIG_DEBUG_FS
 	struct intel_pipe_crc pipe_crc;
 	u32 cpu_fifo_underrun_count;
diff --git a/drivers/gpu/drm/i915/display/intel_global_hist.c b/drivers/gpu/drm/i915/display/intel_global_hist.c
new file mode 100644
index 000000000000..ea5bcd195017
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_global_hist.c
@@ -0,0 +1,295 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
+#include "i915_reg.h"
+#include "i915_drv.h"
+#include "intel_display_types.h"
+#include "intel_de.h"
+#include "intel_global_hist.h"
+
+static int intel_global_hist_get_data(struct drm_i915_private *i915,
+		enum pipe pipe)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(
+			drm_crtc_from_index(&i915->drm, pipe));
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+	u32 dpstbin;
+	int ret = 0, i = 0;
+
+	/*
+	 * TODO: PSR to be exited while reading the Histogram data
+	 * Set DPST_CTL Bin Reg function select to TC
+	 * Set DPST_CTL Bin Register Index to 0
+	 */
+	intel_de_rmw(i915, DPST_CTL(pipe),
+		     DPST_CTL_BIN_REG_FUNC_SEL | DPST_CTL_BIN_REG_MASK, 0);
+
+	for (i = 0; i < GLOBAL_HIST_BIN_COUNT; i++) {
+		dpstbin = intel_de_read(i915, DPST_BIN(pipe));
+		if (dpstbin & DPST_BIN_BUSY) {
+			/*
+			 * If DPST_BIN busy bit is set, then set the
+			 * DPST_CTL bin reg index to 0 and proceed
+			 * from begining
+			 */
+			intel_de_rmw(i915, DPST_CTL(pipe),
+				     DPST_CTL_BIN_REG_MASK, 0);
+			i = 0;
+		}
+		global_hist->bindata[i] = dpstbin & DPST_BIN_DATA_MASK;
+		drm_dbg_atomic(&i915->drm, "Hist[%d]=%x\n",
+				i, global_hist->bindata[i]);
+	}
+
+	/* Clear histogram interrupt by setting histogram interrupt status bit*/
+	intel_de_rmw(i915, DPST_GUARD(pipe),
+			DPST_GUARD_HIST_EVENT_STATUS, 1);
+
+	return ret;
+}
+
+int intel_global_hist_compute_config(struct intel_crtc *intel_crtc)
+{
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+	struct drm_i915_private *i915 = global_hist->i915;
+
+	if (!global_hist->has_edp) {
+		drm_err(&i915->drm, "Not a eDP panel\n");
+		return -EINVAL;
+	}
+	if (!global_hist->has_pwm) {
+		drm_err(&i915->drm, "eDP doesn't have PWM based backlight, cannot enable GLOBAL_HIST\n");
+		return -EINVAL;
+	}
+	/* Restrictions for enabling GLOBAL_HIST */
+	global_hist->can_enable = true;
+	return 0;
+}
+
+static void intel_global_hist_handle_int_work(struct work_struct *work)
+{
+	struct intel_global_hist *global_hist = container_of(work,
+		struct intel_global_hist, handle_global_hist_int_work.work);
+	struct drm_i915_private *i915 = global_hist->i915;
+	char *global_hist_event[] = {"GLOBAL_HIST=1", NULL};
+
+	/* Notify user for Histogram rediness */
+	if (kobject_uevent_env(&i915->drm.primary->kdev->kobj, KOBJ_CHANGE,
+				global_hist_event))
+		drm_err(&i915->drm, "sending GLOBAL_HIST event failed\n");
+	intel_global_hist_get_data(i915, global_hist->pipe);
+}
+
+void intel_global_hist_irq_handler(struct drm_i915_private *i915, enum pipe pipe)
+{
+	struct intel_crtc *intel_crtc =
+		to_intel_crtc(drm_crtc_from_index(&i915->drm, pipe));
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+
+	if (!global_hist->has_pwm) {
+		drm_err(&i915->drm,
+			"eDP doesn't have PWM based backlight, failure in GLOBAL_HIST\n");
+		return;
+	}
+	queue_delayed_work(global_hist->wq,
+			   &global_hist->handle_global_hist_int_work, 0);
+}
+static void intel_global_hist_enable_dithering(struct drm_i915_private *dev_priv,
+		enum pipe pipe)
+{
+	intel_de_rmw(dev_priv, PIPEMISC(pipe), PIPEMISC_DITHER_ENABLE,
+		     PIPEMISC_DITHER_ENABLE);
+}
+
+static int intel_global_hist_enable(struct intel_crtc *intel_crtc)
+{
+	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+	int pipe = intel_crtc->pipe;
+	uint32_t gbandthreshold;
+
+	if (!global_hist->has_pwm) {
+		drm_err(&i915->drm,
+			"eDP doesn't have PWM based backlight, cannot enable GLOBAL_HIST\n");
+		return -EINVAL;
+	}
+
+	/* Pipe Dithering should be enabled with GLOBAL_HIST */
+	intel_global_hist_enable_dithering(i915, pipe);
+
+	/*
+	 * enable DPST_CTL Histogram mode
+	 * Clear DPST_CTL Bin Reg function select to TC
+	 */
+	intel_de_rmw(i915, DPST_CTL(pipe),
+		     DPST_CTL_BIN_REG_FUNC_SEL | DPST_CTL_IE_HIST_EN |
+		     DPST_CTL_HIST_MODE | DPST_CTL_IE_TABLE_VALUE_FORMAT,
+		     DPST_CTL_BIN_REG_FUNC_TC | DPST_CTL_IE_HIST_EN |
+		     DPST_CTL_HIST_MODE_HSV |
+		     DPST_CTL_IE_TABLE_VALUE_FORMAT_1INT_9FRAC);
+	/* check if wait for one vblank is required */
+	drm_crtc_wait_one_vblank(&intel_crtc->base);
+
+	/* TODO: one time programming: Program GuardBand Threshold */
+	gbandthreshold = ((intel_crtc->config->hw.adjusted_mode.vtotal *
+				intel_crtc->config->hw.adjusted_mode.htotal) *
+				GLOBAL_HIST_GUARDBAND_THRESHOLD_DEFAULT) /
+				GLOBAL_HIST_GUARDBAND_PRECISION_FACTOR;
+
+	/* Enable histogram interrupt mode */
+	intel_de_rmw(i915, DPST_GUARD(pipe),
+		     DPST_GUARD_THRESHOLD_GB_MASK |
+		     DPST_GUARD_INTERRUPT_DELAY_MASK | DPST_GUARD_HIST_INT_EN,
+		     DPST_GUARD_THRESHOLD_GB(gbandthreshold) |
+		     DPST_GUARD_INTERRUPT_DELAY(GLOBAL_HIST_DEFAULT_GUARDBAND_DELAY) |
+		     DPST_GUARD_HIST_INT_EN);
+
+	/* Clear pending interrupts has to be done on seperate write */
+	intel_de_rmw(i915, DPST_GUARD(pipe),
+			DPST_GUARD_HIST_EVENT_STATUS, 1);
+
+	global_hist->enable = true;
+
+	return 0;
+}
+
+static int intel_global_hist_disable(struct intel_crtc *intel_crtc)
+{
+	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+	int pipe = intel_crtc->pipe;
+
+	/* Pipe Dithering should be enabled with GLOBAL_HIST */
+	intel_global_hist_enable_dithering(i915, pipe);
+
+	/* Clear pending interrupts and disable interrupts */
+	intel_de_rmw(i915, DPST_GUARD(pipe),
+		     DPST_GUARD_HIST_INT_EN | DPST_GUARD_HIST_EVENT_STATUS, 0);
+
+	/* disable DPST_CTL Histogram mode */
+	intel_de_rmw(i915, DPST_CTL(pipe),
+		     DPST_CTL_IE_HIST_EN, 0);
+
+	cancel_delayed_work(&global_hist->handle_global_hist_int_work);
+	global_hist->enable = false;
+
+	return 0;
+}
+
+int intel_global_hist_update(struct intel_crtc *intel_crtc, bool enable)
+{
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
+
+	if (!global_hist->can_enable) {
+		drm_err(&i915->drm,
+			"GLOBAL_HIST not supported, compute config failed\n");
+		return 0;
+	}
+
+	if (enable)
+		return intel_global_hist_enable(intel_crtc);
+	else
+		return intel_global_hist_disable(intel_crtc);
+}
+
+int intel_global_hist_set_iet_lut(struct intel_crtc *intel_crtc, u32 *data)
+{
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
+	int pipe = intel_crtc->pipe;
+	int i = 0;
+
+	if (!global_hist->enable) {
+		drm_err(&i915->drm, "GLOBAL_HIST not enabled");
+		return -EINVAL;
+	}
+
+	/*
+	 * Set DPST_CTL Bin Reg function select to IE
+	 * Set DPST_CTL Bin Register Index to 0
+	 */
+	intel_de_rmw(i915, DPST_CTL(pipe),
+		     DPST_CTL_BIN_REG_FUNC_SEL | DPST_CTL_BIN_REG_MASK,
+		     DPST_CTL_BIN_REG_FUNC_IE | DPST_CTL_BIN_REG_CLEAR);
+	for (i = 0; i < GLOBAL_HIST_IET_LENGTH; i++) {
+		intel_de_rmw(i915, DPST_BIN(pipe),
+			     DPST_BIN_DATA_MASK, data[i]);
+		drm_dbg_atomic(&i915->drm, "iet_lut[%d]=%x\n", i, data[i]);
+	}
+	intel_de_rmw(i915, DPST_CTL(pipe),
+		     DPST_CTL_ENHANCEMENT_MODE_MASK | DPST_CTL_IE_MODI_TABLE_EN,
+		     DPST_CTL_EN_MULTIPLICATIVE | DPST_CTL_IE_MODI_TABLE_EN);
+	/* Once IE is applied, change DPST CTL to TC */
+	intel_de_rmw(i915, DPST_CTL(pipe),
+		     DPST_CTL_BIN_REG_FUNC_SEL, DPST_CTL_BIN_REG_FUNC_TC);
+
+	return 0;
+}
+
+void intel_global_hist_deinit(struct intel_crtc *intel_crtc)
+{
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+
+	cancel_delayed_work(&global_hist->handle_global_hist_int_work);
+	destroy_workqueue(global_hist->wq);
+	kfree(global_hist);
+}
+
+int intel_global_hist_init(struct intel_crtc *intel_crtc)
+{
+	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
+	struct intel_global_hist *global_hist;
+
+	/* Allocate global_hist internal struct */
+	global_hist = kzalloc(sizeof(*global_hist), GFP_KERNEL);
+	if (unlikely(!global_hist)) {
+		drm_err(&i915->drm,
+			"Failed to allocate GLOBAL_HIST event\n");
+		kfree(global_hist);
+		return -ENOMEM;
+	}
+
+	intel_crtc->global_hist = global_hist;
+	global_hist->pipe = intel_crtc->pipe;
+	global_hist->can_enable = false;
+	global_hist->wq = alloc_ordered_workqueue("global_hist_wq",
+						 WQ_MEM_RECLAIM);
+	if (global_hist->wq == NULL) {
+		drm_err(&i915->drm,
+			"failed to create work queue\n");
+		kfree(global_hist);
+		return -ENOMEM;
+	}
+
+	INIT_DEFERRABLE_WORK(&global_hist->handle_global_hist_int_work,
+		intel_global_hist_handle_int_work);
+
+	global_hist->i915 = i915;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_global_hist.h b/drivers/gpu/drm/i915/display/intel_global_hist.h
new file mode 100644
index 000000000000..c6621bf4ea61
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_global_hist.h
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __INTEL_GLOBAL_HIST_H__
+#define __INTEL_GLOBAL_HIST_H__
+
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
+#include "intel_display.h"
+#include "../i915_reg.h"
+
+/* GLOBAL_HIST related registers */
+#define _DPST_CTL_A					0x490C0
+#define _DPST_CTL_B					0x491C0
+#define DPST_CTL(pipe)					_MMIO_PIPE(pipe, _DPST_CTL_A, _DPST_CTL_B)
+#define DPST_CTL_IE_HIST_EN				REG_BIT(31)
+#define DPST_CTL_IE_MODI_TABLE_EN			REG_BIT(27)
+#define DPST_CTL_HIST_MODE				REG_BIT(24)
+#define DPST_CTL_ENHANCEMENT_MODE_MASK			REG_GENMASK(14, 13)
+#define DPST_CTL_EN_MULTIPLICATIVE			REG_FIELD_PREP(DPST_CTL_ENHANCEMENT_MODE_MASK, 2)
+#define DPST_CTL_IE_TABLE_VALUE_FORMAT			REG_BIT(15)
+#define DPST_CTL_BIN_REG_FUNC_SEL			REG_BIT(11)
+#define DPST_CTL_BIN_REG_FUNC_TC			(0 << 0)
+#define DPST_CTL_BIN_REG_FUNC_IE			(1 << 11)
+#define DPST_CTL_BIN_REG_MASK				REG_GENMASK(6, 0)
+#define DPST_CTL_BIN_REG_CLEAR				REG_FIELD_PREP(DPST_CTL_BIN_REG_MASK, 0)
+
+#define _DPST_GUARD_A					0x490C8
+#define _DPST_GUARD_B					0x491C8
+#define DPST_GUARD(pipe)				_MMIO_PIPE(pipe, _DPST_GUARD_A, _DPST_GUARD_B)
+#define DPST_GUARD_HIST_INT_EN				REG_BIT(31)
+#define DPST_GUARD_HIST_EVENT_STATUS			REG_BIT(30)
+#define DPST_GUARD_INTERRUPT_DELAY_MASK			REG_GENMASK(29, 22)
+#define DPST_GUARD_INTERRUPT_DELAY(val)			REG_FIELD_PREP(DPST_GUARD_INTERRUPT_DELAY_MASK, val)
+#define DPST_GUARD_THRESHOLD_GB_MASK			REG_GENMASK(21, 0)
+#define DPST_GUARD_THRESHOLD_GB(val)			REG_FIELD_PREP(DPST_GUARD_THRESHOLD_GB_MASK, val)
+
+#define _DPST_BIN_A					0x490C4
+#define _DPST_BIN_B					0x491C4
+#define DPST_BIN(pipe)					_MMIO_PIPE(pipe, _DPST_BIN_A, _DPST_BIN_B)
+#define DPST_BIN_DATA_MASK				REG_GENMASK(23, 0)
+#define DPST_BIN_BUSY					REG_BIT(31)
+
+#define DPST_CTL_IE_TABLE_VALUE_FORMAT_2INT_8FRAC	(1 << 15)
+#define DPST_CTL_IE_TABLE_VALUE_FORMAT_1INT_9FRAC	(0 << 15)
+#define DPST_CTL_HIST_MODE_YUV				(0 << 24)
+#define DPST_CTL_HIST_MODE_HSV				(1 << 24)
+
+
+#define INTEL_GLOBAL_HISTOGRAM_PIPEA	0x90000000
+#define INTEL_GLOBAL_HISTOGRAM_PIPEB	0x90000002
+#define INTEL_GLOBAL_HISTOGRAM_EVENT(pipe)	_PIPE(pipe, \
+						INTEL_GLOBAL_HISTOGRAM_PIPEA, \
+						INTEL_GLOBAL_HISTOGRAM_PIPEB)
+
+#define GLOBAL_HIST_BIN_COUNT			32
+#define GLOBAL_HIST_IET_LENGTH			33
+
+#define GLOBAL_HIST_GUARDBAND_THRESHOLD_DEFAULT 300    // 3.0% of the pipe's current pixel count.
+#define GLOBAL_HIST_GUARDBAND_PRECISION_FACTOR 10000   // Precision factor for threshold guardband.
+#define GLOBAL_HIST_DEFAULT_GUARDBAND_DELAY 0x04
+
+enum intel_global_hist_status {
+	INTEL_GLOBAL_HIST_ENABLE,
+	INTEL_GLOBAL_HIST_DISABLE,
+};
+
+enum intel_global_histogram {
+	INTEL_GLOBAL_HISTOGRAM,
+};
+
+enum intel_global_hist_lut {
+	INTEL_GLOBAL_HIST_PIXEL_FACTOR,
+};
+
+struct intel_global_hist {
+	struct drm_i915_private *i915;
+	struct workqueue_struct *wq;
+	struct delayed_work handle_global_hist_int_work;
+	bool enable;
+	bool has_pwm;
+	bool has_edp;
+	bool can_enable;
+	enum pipe pipe;
+	u32 bindata[GLOBAL_HIST_BIN_COUNT];
+};
+
+int intel_global_hist_compute_config(struct intel_crtc *intel_crtc);
+void intel_global_hist_irq_handler(struct drm_i915_private *i915, enum pipe pipe);
+int intel_global_hist_update(struct intel_crtc *intel_crtc, bool enable);
+int intel_global_hist_set_iet_lut(struct intel_crtc *intel_crtc, u32 *data);
+int intel_global_hist_init(struct intel_crtc *intel_crtc);
+void intel_global_hist_deinit(struct intel_crtc *intel_crtc);
+
+#endif /* __INTEL_GLOBAL_HIST_H__ */
-- 
2.25.1

[Intel-gfx] [PATCH 2/6] drm/i915/display: global histogram irq handler

[Arun R Murthy]

With the enablement of global histogram, upon generation of histogram,
an interrupt is triggered. This patch handles the irq.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 6 +++++-
 drivers/gpu/drm/i915/i915_reg.h | 5 +++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e28bfb5f7347..d72fb6d9282d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -43,6 +43,7 @@
 #include "display/intel_hotplug.h"
 #include "display/intel_lpe_audio.h"
 #include "display/intel_psr.h"
+#include "display/intel_global_hist.h"
 
 #include "gt/intel_breadcrumbs.h"
 #include "gt/intel_gt.h"
@@ -2765,6 +2766,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 		ret = IRQ_HANDLED;
 		intel_uncore_write(&dev_priv->uncore, GEN8_DE_PIPE_IIR(pipe), iir);
 
+		if (iir & GEN9_PIPE_GLOBAL_HIST_EVENT)
+			intel_global_hist_irq_handler(dev_priv, pipe);
+
 		if (iir & GEN8_PIPE_VBLANK)
 			intel_handle_vblank(dev_priv, pipe);
 
@@ -5043,7 +5047,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	u32 de_pipe_masked = gen8_de_pipe_fault_mask(dev_priv) |
-		GEN8_PIPE_CDCLK_CRC_DONE;
+		GEN8_PIPE_CDCLK_CRC_DONE | GEN9_PIPE_GLOBAL_HIST_EVENT;
 	u32 de_pipe_enables;
 	u32 de_port_masked = gen8_de_port_aux_mask(dev_priv);
 	u32 de_port_enables;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 94d0c8d14d43..546207ac4859 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3887,7 +3887,7 @@
 #define   PIPE_HOTPLUG_INTERRUPT_ENABLE		(1UL << 26)
 #define   PIPE_VSYNC_INTERRUPT_ENABLE		(1UL << 25)
 #define   PIPE_DISPLAY_LINE_COMPARE_ENABLE	(1UL << 24)
-#define   PIPE_DPST_EVENT_ENABLE		(1UL << 23)
+#define   PIPE_GLOBAL_HIST_EVENT_ENABLE		(1UL << 23)
 #define   SPRITE0_FLIP_DONE_INT_EN_VLV		(1UL << 22)
 #define   PIPE_LEGACY_BLC_EVENT_ENABLE		(1UL << 22)
 #define   PIPE_ODD_FIELD_INTERRUPT_ENABLE	(1UL << 21)
@@ -3910,7 +3910,7 @@
 #define   PIPE_HOTPLUG_INTERRUPT_STATUS		(1UL << 10)
 #define   PIPE_VSYNC_INTERRUPT_STATUS		(1UL << 9)
 #define   PIPE_DISPLAY_LINE_COMPARE_STATUS	(1UL << 8)
-#define   PIPE_DPST_EVENT_STATUS		(1UL << 7)
+#define   PIPE_GLOBAL_HIST_EVENT_STATUS		(1UL << 7)
 #define   PIPE_A_PSR_STATUS_VLV			(1UL << 6)
 #define   PIPE_LEGACY_BLC_EVENT_STATUS		(1UL << 6)
 #define   PIPE_ODD_FIELD_INTERRUPT_STATUS	(1UL << 5)
@@ -5815,6 +5815,7 @@
 #define  GEN8_PIPE_VSYNC		(1 << 1)
 #define  GEN8_PIPE_VBLANK		(1 << 0)
 #define  GEN9_PIPE_CURSOR_FAULT		(1 << 11)
+#define  GEN9_PIPE_GLOBAL_HIST_EVENT	(1 << 12)
 #define  GEN11_PIPE_PLANE7_FAULT	(1 << 22)
 #define  GEN11_PIPE_PLANE6_FAULT	(1 << 21)
 #define  GEN11_PIPE_PLANE5_FAULT	(1 << 20)
-- 
2.25.1

[Intel-gfx] [PATCH 3/6] drm/i915/display: global histogram restrictions

[Arun R Murthy]

For global histogram the panel should be edp and should have pwm based
backlight controller. Flags are updated accordingly.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_modeset_setup.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index cd21b0ddbabb..975d6bdb59f3 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -17,12 +17,14 @@
 #include "intel_crtc_state_dump.h"
 #include "intel_ddi.h"
 #include "intel_de.h"
+#include "intel_dp.h"
 #include "intel_display.h"
 #include "intel_display_power.h"
 #include "intel_display_types.h"
 #include "intel_modeset_setup.h"
 #include "intel_pch_display.h"
 #include "intel_pm.h"
+#include "intel_global_hist.h"
 
 static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
 					struct drm_modeset_acquire_ctx *ctx)
@@ -309,6 +311,7 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	struct intel_crtc_state *crtc_state = crtc ?
 		to_intel_crtc_state(crtc->base.state) : NULL;
+	struct intel_panel *panel;
 
 	/*
 	 * We need to check both for a crtc link (meaning that the encoder is
@@ -376,6 +379,15 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
 
 	if (HAS_DDI(i915))
 		intel_ddi_sanitize_encoder_pll_mapping(encoder);
+
+	/* validate the global hist struct elements */
+	if (intel_dp_is_port_edp(i915, encoder->port)) {
+		crtc->global_hist->has_edp = true;
+		panel = &connector->panel;
+		if (panel->backlight.present == true)
+			crtc->global_hist->has_pwm = true;
+	}
+
 }
 
 /* FIXME read out full plane state for all planes */
-- 
2.25.1

[Intel-gfx] [PATCH 4/6] drm/i915/display: Add crtc properties for global histogram

[Arun R Murthy]

CRTC properties have been added for enable/disable histogram, reading
the histogram data and writing the IET data.
"GLOBAL_HIST_EN" is the crtc property to enable/disable the global
histogram and takes a value 0/1 accordingly.
"Global Histogram" is a crtc property to read the binary histogram data.
"Global IET" is a crtc property to write the IET binary LUT data.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |   5 +
 drivers/gpu/drm/i915/display/intel_crtc.c     | 200 +++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_crtc.h     |   5 +
 drivers/gpu/drm/i915/display/intel_display.c  |  13 ++
 .../drm/i915/display/intel_display_types.h    |  19 +-
 .../gpu/drm/i915/display/intel_global_hist.c  |   7 +
 6 files changed, 246 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index 0e5d57c978fe..bed682854071 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -245,6 +245,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 
 	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->uapi);
 
+	if (crtc_state->global_iet)
+		drm_property_blob_get(crtc_state->global_iet);
 	/* copy color blobs */
 	if (crtc_state->hw.degamma_lut)
 		drm_property_blob_get(crtc_state->hw.degamma_lut);
@@ -266,6 +268,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	crtc_state->fb_bits = 0;
 	crtc_state->update_planes = 0;
 	crtc_state->dsb = NULL;
+	crtc_state->global_hist_en_changed = false;
 
 	return &crtc_state->uapi;
 }
@@ -298,6 +301,8 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
 
 	drm_WARN_ON(crtc->dev, crtc_state->dsb);
 
+	if (crtc_state->global_iet)
+		drm_property_blob_put(crtc_state->global_iet);
 	__drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
 	intel_crtc_free_hw_state(crtc_state);
 	kfree(crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 521dc676e2d0..501bcf732aba 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -11,6 +11,7 @@
 #include <drm/drm_plane.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_vblank_work.h>
+#include <drm/drm_atomic_uapi.h>
 
 #include "i915_irq.h"
 #include "i915_vgpu.h"
@@ -26,6 +27,7 @@
 #include "intel_display_types.h"
 #include "intel_drrs.h"
 #include "intel_dsi.h"
+#include "intel_global_hist.h"
 #include "intel_pipe_crc.h"
 #include "intel_psr.h"
 #include "intel_sprite.h"
@@ -196,6 +198,7 @@ static struct intel_crtc *intel_crtc_alloc(void)
 static void intel_crtc_free(struct intel_crtc *crtc)
 {
 	intel_crtc_destroy_state(&crtc->base, crtc->base.state);
+	intel_global_hist_deinit(crtc);
 	kfree(crtc);
 }
 
@@ -215,6 +218,99 @@ static int intel_crtc_late_register(struct drm_crtc *crtc)
 	return 0;
 }
 
+int intel_crtc_get_property(struct drm_crtc *crtc,
+				   const struct drm_crtc_state *state,
+				   struct drm_property *property,
+				   uint64_t *val)
+{
+	struct drm_i915_private *i915 = to_i915(crtc->dev);
+	struct intel_crtc_state *intel_crtc_state =
+		to_intel_crtc_state(state);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (property == intel_crtc->global_hist_en_property)
+		*val = intel_crtc_state->global_hist_en;
+	else if (property == intel_crtc->global_iet_property)
+		*val = (intel_crtc_state->global_iet) ?
+			intel_crtc_state->global_iet->base.id : 0;
+	else if (property == intel_crtc->global_hist_property)
+		*val = (intel_crtc_state->global_hist) ?
+			intel_crtc_state->global_hist->base.id : 0;
+	else {
+		drm_err(&i915->drm,
+			       "Unknown property [PROP:%d:%s]\n",
+			       property->base.id, property->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+intel_atomic_replace_property_blob_from_id(struct drm_device *dev,
+					 struct drm_property_blob **blob,
+					 uint64_t blob_id,
+					 ssize_t expected_size,
+					 ssize_t expected_elem_size,
+					 bool *replaced)
+{
+	struct drm_property_blob *new_blob = NULL;
+
+	if (blob_id != 0) {
+		new_blob = drm_property_lookup_blob(dev, blob_id);
+		if (new_blob == NULL)
+			return -EINVAL;
+
+		if (expected_size > 0 &&
+		    new_blob->length != expected_size) {
+			drm_property_blob_put(new_blob);
+			return -EINVAL;
+		}
+		if (expected_elem_size > 0 &&
+		    new_blob->length % expected_elem_size != 0) {
+			drm_property_blob_put(new_blob);
+			return -EINVAL;
+		}
+	}
+
+	*replaced |= drm_property_replace_blob(blob, new_blob);
+	drm_property_blob_put(new_blob);
+
+	return 0;
+}
+
+int intel_crtc_set_property(struct drm_crtc *crtc,
+				struct drm_crtc_state *state,
+				struct drm_property *property,
+				u64 val)
+{
+	struct drm_i915_private *i915 = to_i915(crtc->dev);
+	struct intel_crtc_state *intel_crtc_state =
+		to_intel_crtc_state(state);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	bool replaced = false;
+
+	if (property == intel_crtc->global_hist_en_property) {
+		intel_crtc_state->global_hist_en = val;
+		intel_crtc_state->global_hist_en_changed = true;
+		return 0;
+	}
+
+	if (property == intel_crtc->global_iet_property) {
+		intel_atomic_replace_property_blob_from_id(crtc->dev,
+				&intel_crtc_state->global_iet, val,
+				sizeof(uint32_t) * GLOBAL_HIST_IET_LENGTH,
+				-1, &replaced);
+		if (replaced)
+			intel_crtc_state->global_iet_changed = true;
+		return 0;
+	}
+
+	drm_dbg_atomic(&i915->drm, "Unknown property [PROP:%d:%s]\n",
+		       property->base.id, property->name);
+	return -EINVAL;
+}
+
 #define INTEL_CRTC_FUNCS \
 	.set_config = drm_atomic_helper_set_config, \
 	.destroy = intel_crtc_destroy, \
@@ -225,7 +321,9 @@ static int intel_crtc_late_register(struct drm_crtc *crtc)
 	.verify_crc_source = intel_crtc_verify_crc_source, \
 	.get_crc_sources = intel_crtc_get_crc_sources, \
 	.late_register = intel_crtc_late_register, \
-	.pre_crc_read = intel_crtc_pre_crc_read
+	.pre_crc_read = intel_crtc_pre_crc_read, \
+	.atomic_set_property = intel_crtc_set_property, \
+	.atomic_get_property = intel_crtc_get_property
 
 static const struct drm_crtc_funcs bdw_crtc_funcs = {
 	INTEL_CRTC_FUNCS,
@@ -371,6 +469,10 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 	intel_crtc_drrs_init(crtc);
 	intel_crtc_crc_init(crtc);
+	intel_global_hist_init(crtc);
+
+	/* Initialize crtc properties */
+	intel_crtc_add_property(crtc);
 
 	cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE);
 
@@ -713,3 +815,99 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 
 	dbg_vblank_evade(crtc, end_vbl_time);
 }
+
+static const struct drm_prop_enum_list global_hist_en_names[] = {
+	{ INTEL_GLOBAL_HIST_DISABLE, "Disable" },
+	{ INTEL_GLOBAL_HIST_ENABLE, "Enable" },
+};
+
+/**
+ * intel_attach_global_hist_en_property() - add property to enable/disable histogram
+ * @intel_crtc: pointer to the struct intel_crtc on which the global histogram is to
+ *		be enabled/disabled
+ *
+ * "GLOBAL_HIST_EN" is the crtc proeprty to enable/disable global histogram
+ */
+void intel_attach_global_hist_en_property(struct intel_crtc *intel_crtc)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_property *prop;
+
+	prop = intel_crtc->global_hist_en_property;
+	if (prop == NULL) {
+		prop = drm_property_create_enum(dev, 0,
+					   "GLOBAL_HIST_EN",
+					   global_hist_en_names,
+					   ARRAY_SIZE(global_hist_en_names));
+		if (prop == NULL)
+			return;
+
+		intel_crtc->global_hist_en_property = prop;
+	}
+
+	drm_object_attach_property(&crtc->base, prop, 0);
+}
+
+/**
+ * intel_attach_global_iet_property() - add property to write Image Enhancement data
+ * @intel_crtc: pointer to the struct intel_crtc on which global histogram is enabled
+ *
+ * "Global IET" is the crtc property to write the Image Enhancement LUT binary data
+ */
+void intel_attach_global_iet_property(struct intel_crtc *intel_crtc)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_property *prop;
+
+	prop = intel_crtc->global_iet_property;
+	if (prop == NULL) {
+		prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | DRM_MODE_PROP_ATOMIC,
+					   "Global IET", 0);
+		if (prop == NULL)
+			return;
+
+		intel_crtc->global_iet_property = prop;
+	}
+
+	drm_object_attach_property(&crtc->base, prop, 0);
+}
+
+/**
+ * intel_atach_global_hist_property() - crtc proeprty to read the histogram.
+ * @intel_crtc: pointer to the struct intel_crtc on which the global histogram
+ *		was enabled.
+ * "Global Histogram" is the crtc property to read the binary histogram data.
+ */
+void intel_attach_global_hist_property(struct intel_crtc *intel_crtc)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_property *prop;
+	struct drm_property_blob *blob;
+
+	prop = intel_crtc->global_hist_property;
+	if (prop == NULL) {
+		prop = drm_property_create(dev, DRM_MODE_PROP_BLOB |
+					DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_IMMUTABLE,
+					"Global Histogram", 0);
+		if (prop == NULL)
+			return;
+
+		intel_crtc->global_hist_property = prop;
+	}
+	blob = drm_property_create_blob(dev, sizeof(uint32_t) * GLOBAL_HIST_BIN_COUNT, NULL);
+	intel_crtc->config->global_hist = blob;
+
+	drm_object_attach_property(&crtc->base, prop, blob->base.id);
+}
+
+int intel_crtc_add_property(struct intel_crtc *intel_crtc)
+{
+	intel_attach_global_hist_en_property(intel_crtc);
+	intel_attach_global_hist_property(intel_crtc);
+	intel_attach_global_iet_property(intel_crtc);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
index 73077137fb99..6744b6a03e19 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.h
+++ b/drivers/gpu/drm/i915/display/intel_crtc.h
@@ -7,6 +7,7 @@
 #define _INTEL_CRTC_H_
 
 #include <linux/types.h>
+#include <drm/drm_crtc.h>
 
 enum i9xx_plane_id;
 enum pipe;
@@ -36,4 +37,8 @@ void intel_wait_for_vblank_if_active(struct drm_i915_private *i915,
 				     enum pipe pipe);
 void intel_crtc_wait_for_next_vblank(struct intel_crtc *crtc);
 
+int intel_crtc_add_property(struct intel_crtc *intel_crtc);
+void intel_attach_global_hist_en_property(struct intel_crtc *intel_crtc);
+void intel_attach_global_iet_property(struct intel_crtc *intel_crtc);
+void intel_attach_global_hist_property(struct intel_crtc *intel_crtc);
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ab3c9e8d4157..6255c6443726 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -52,6 +52,7 @@
 #include "display/intel_dp.h"
 #include "display/intel_dp_mst.h"
 #include "display/intel_dpll.h"
+#include "display/intel_global_hist.h"
 #include "display/intel_dpll_mgr.h"
 #include "display/intel_drrs.h"
 #include "display/intel_dsi.h"
@@ -4921,6 +4922,10 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
 	if (ret)
 		return ret;
 
+	/* GLOBAL_HIST changed */
+	if (crtc_state->global_hist_en_changed)
+		intel_global_hist_compute_config(crtc);
+
 	return 0;
 }
 
@@ -7730,6 +7735,14 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 		 * commit_done and later do dsb cleanup in cleanup_work.
 		 */
 		old_crtc_state->dsb = fetch_and_zero(&new_crtc_state->dsb);
+
+		/* Re-Visit: GLOBAL_HIST related stuff */
+		if (new_crtc_state->global_hist_en_changed)
+			intel_global_hist_update(crtc,
+					new_crtc_state->global_hist_en);
+		if (new_crtc_state->global_iet_changed)
+			intel_global_hist_set_iet_lut(crtc,
+					(u32 *)new_crtc_state->global_iet->data);
 	}
 
 	/* Underruns don't always raise interrupts, so check manually */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9848fcf73b87..15d28e2305da 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -94,6 +94,12 @@ enum intel_broadcast_rgb {
 	INTEL_BROADCAST_RGB_LIMITED,
 };
 
+/* GLOBAL_HIST property */
+enum intel_global_hist_en_prop {
+	INTEL_GLOBAL_HIST_PROP_DISABLE,
+	INTEL_GLOBAL_HIST_PROP_ENABLE,
+};
+
 struct intel_fb_view {
 	/*
 	 * The remap information used in the remapped and rotated views to
@@ -1360,6 +1366,13 @@ struct intel_crtc_state {
 
 	/* for loading single buffered registers during vblank */
 	struct drm_vblank_work vblank_work;
+
+	/* GLOBAL_HIST data */
+	int global_hist_en;
+	struct drm_property_blob *global_iet;
+	struct drm_property_blob *global_hist;
+	bool global_iet_changed;
+	bool global_hist_en_changed;
 };
 
 enum intel_pipe_crc_source {
@@ -1462,9 +1475,11 @@ struct intel_crtc {
 	/* for loading single buffered registers during vblank */
 	struct pm_qos_request vblank_pm_qos;
 
-	/* GLOBAL_HIST data */
 	struct intel_global_hist *global_hist;
-
+	/* GLOBAL_HIST properties */
+	struct drm_property *global_hist_en_property;
+	struct drm_property *global_iet_property;
+	struct drm_property *global_hist_property;
 #ifdef CONFIG_DEBUG_FS
 	struct intel_pipe_crc pipe_crc;
 	u32 cpu_fifo_underrun_count;
diff --git a/drivers/gpu/drm/i915/display/intel_global_hist.c b/drivers/gpu/drm/i915/display/intel_global_hist.c
index ea5bcd195017..874d80d1e41b 100644
--- a/drivers/gpu/drm/i915/display/intel_global_hist.c
+++ b/drivers/gpu/drm/i915/display/intel_global_hist.c
@@ -69,6 +69,12 @@ static int intel_global_hist_get_data(struct drm_i915_private *i915,
 	intel_de_rmw(i915, DPST_GUARD(pipe),
 			DPST_GUARD_HIST_EVENT_STATUS, 1);
 
+	drm_property_replace_global_blob(&i915->drm,
+			&intel_crtc->config->global_hist,
+			sizeof(global_hist->bindata),
+			global_hist->bindata, &intel_crtc->base.base,
+			intel_crtc->global_hist_property);
+
 	return ret;
 }
 
@@ -196,6 +202,7 @@ static int intel_global_hist_disable(struct intel_crtc *intel_crtc)
 
 	cancel_delayed_work(&global_hist->handle_global_hist_int_work);
 	global_hist->enable = false;
+	intel_crtc->config->global_hist_en = false;
 
 	return 0;
 }
-- 
2.25.1

[Intel-gfx] [PATCH 5/6] drm/i915/display: crtc property for global hist selective fetch

[Arun R Murthy]

User can provide the selective fetch co-ordinates for global histogram
using crtc blob property. This patch adds the crtc blob property.
The selective fetch can be done only on the y co-ordinate and cannot be
done on the x co-ordinate.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c     | 45 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  3 ++
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 501bcf732aba..2a9dcf3b1a19 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -236,6 +236,9 @@ int intel_crtc_get_property(struct drm_crtc *crtc,
 	else if (property == intel_crtc->global_hist_property)
 		*val = (intel_crtc_state->global_hist) ?
 			intel_crtc_state->global_hist->base.id : 0;
+	else if (property == intel_crtc->global_hist_sf_clips_property)
+		*val = (intel_crtc_state->global_hist_sf_clips) ?
+			intel_crtc_state->global_hist_sf_clips->base.id : 0;
 	else {
 		drm_err(&i915->drm,
 			       "Unknown property [PROP:%d:%s]\n",
@@ -306,6 +309,18 @@ int intel_crtc_set_property(struct drm_crtc *crtc,
 		return 0;
 	}
 
+	if (property == intel_crtc->global_hist_sf_clips_property) {
+		intel_atomic_replace_property_blob_from_id(crtc->dev,
+					&intel_crtc_state->global_hist_sf_clips,
+					val,
+					-1,
+					sizeof(struct drm_rect),
+					&replaced);
+		if (replaced)
+			intel_crtc_state->global_hist_sf_clips_updates = true;
+		return 0;
+	}
+
 	drm_dbg_atomic(&i915->drm, "Unknown property [PROP:%d:%s]\n",
 		       property->base.id, property->name);
 	return -EINVAL;
@@ -903,11 +918,41 @@ void intel_attach_global_hist_property(struct intel_crtc *intel_crtc)
 	drm_object_attach_property(&crtc->base, prop, blob->base.id);
 }
 
+/**
+ * intel_attach_global_hist_sf_seg_property() - selective fetch segment property
+ * @intel_crtc: pointer to struct intel_crtc on which global histogram is enabled
+ *
+ * "Global Histogram SF CLIPS" is the crtc porperty used to provide the
+ * co-ordinates of the damage clips.
+ */
+void intel_attach_global_hist_sf_seg_property(struct intel_crtc * intel_crtc)
+{
+	struct drm_crtc *crtc = &intel_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_property *prop;
+	struct drm_property_blob *blob;
+
+	prop = intel_crtc->global_hist_sf_clips_property;
+	if (prop == NULL) {
+		prop = drm_property_create(dev,
+			DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
+			"Global Histogram SF CLIPS", 0);
+		if (prop == NULL)
+			return;
+		intel_crtc->global_hist_sf_clips_property = prop;
+	}
+	blob = drm_property_create_blob(dev, sizeof(struct drm_rect *), NULL);
+	intel_crtc->config->global_hist_sf_clips = blob;
+
+	drm_object_attach_property(&crtc->base, prop, blob->base.id);
+}
+
 int intel_crtc_add_property(struct intel_crtc *intel_crtc)
 {
 	intel_attach_global_hist_en_property(intel_crtc);
 	intel_attach_global_hist_property(intel_crtc);
 	intel_attach_global_iet_property(intel_crtc);
+	intel_attach_global_hist_sf_seg_property(intel_crtc);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 15d28e2305da..703593d4a52f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1371,8 +1371,10 @@ struct intel_crtc_state {
 	int global_hist_en;
 	struct drm_property_blob *global_iet;
 	struct drm_property_blob *global_hist;
+	struct drm_property_blob *global_hist_sf_clips;
 	bool global_iet_changed;
 	bool global_hist_en_changed;
+	bool global_hist_sf_clips_updates;
 };
 
 enum intel_pipe_crc_source {
@@ -1480,6 +1482,7 @@ struct intel_crtc {
 	struct drm_property *global_hist_en_property;
 	struct drm_property *global_iet_property;
 	struct drm_property *global_hist_property;
+	struct drm_property *global_hist_sf_clips_property;
 #ifdef CONFIG_DEBUG_FS
 	struct intel_pipe_crc pipe_crc;
 	u32 cpu_fifo_underrun_count;
-- 
2.25.1

[Intel-gfx] [PATCH 6/6] drm/i915/display: Enable global hist Selective fetch

[Arun R Murthy]

This patch enables support for selective fetch in global histogram.
User can provide the selective fetch co-ordinates and only that region
will be used in generating the histogram.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 .../gpu/drm/i915/display/intel_global_hist.c  | 65 +++++++++++++++++++
 .../gpu/drm/i915/display/intel_global_hist.h  | 14 ++++
 2 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_global_hist.c b/drivers/gpu/drm/i915/display/intel_global_hist.c
index 874d80d1e41b..13ec68463eec 100644
--- a/drivers/gpu/drm/i915/display/intel_global_hist.c
+++ b/drivers/gpu/drm/i915/display/intel_global_hist.c
@@ -31,6 +31,48 @@
 #include "intel_de.h"
 #include "intel_global_hist.h"
 
+#define MIN_SEGMENTS 32
+#define MAX_SEGMENTS 128
+
+static int intel_global_hist_calc_seg_size(struct drm_i915_private *dev_priv,
+		enum pipe pipe)
+{
+	uint32_t tmp, source_height;
+	uint16_t seg_size = MIN_SEGMENTS;
+
+	/* Get the pipe source height from the pipesr register */
+	tmp = intel_de_read(dev_priv, PIPESRC(pipe));
+	source_height = REG_FIELD_GET(PIPESRC_HEIGHT_MASK, tmp) + 1;
+
+	while (seg_size <= source_height) {
+		if ((seg_size % source_height == 0) &&
+		   ((source_height / seg_size) < MAX_SEGMENTS))
+			break;
+		seg_size++;
+	}
+
+	return seg_size;
+}
+
+int intel_global_hist_sf_update_seg(struct drm_i915_private *i915,
+		enum pipe pipe, struct drm_rect *clip)
+{
+	uint16_t seg_size;
+
+	seg_size = intel_global_hist_calc_seg_size(i915, pipe);
+	if (!seg_size)
+		return -EINVAL;
+
+	intel_de_rmw(i915, DPST_SF_SEG(pipe),
+		     DPST_SF_SEG_SIZE_MASK | DPST_SF_SEG_START_MASK |
+		     DPST_SF_SEG_END_MASK,
+		     DPST_SF_SEG_SIZE(seg_size) |
+		     DPST_SF_SEG_START((clip->y2/seg_size) * seg_size) |
+		     DPST_SF_SEG_END((clip->y1/seg_size) * seg_size));
+
+	return 0;
+}
+
 static int intel_global_hist_get_data(struct drm_i915_private *i915,
 		enum pipe pipe)
 {
@@ -258,6 +300,29 @@ int intel_global_hist_set_iet_lut(struct intel_crtc *intel_crtc, u32 *data)
 	return 0;
 }
 
+int intel_global_hist_sf_en(struct drm_i915_private *i915,
+		enum pipe pipe, struct drm_rect *clip)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(
+			drm_crtc_from_index(&i915->drm, pipe));
+	struct intel_global_hist *global_hist = intel_crtc->global_hist;
+	uint32_t dpstsfctl;
+
+	/* If DPST is not enabled, enable it first */
+	if (!global_hist->enable)
+		intel_global_hist_enable(intel_crtc);
+
+	/* Program dpst selective fetch */
+	dpstsfctl = intel_de_read(i915, DPST_SF_CTL(pipe));
+	dpstsfctl |= DPST_SF_CTL_ENABLE;
+	intel_de_write(i915, DPST_SF_CTL(pipe), dpstsfctl);
+
+	/* Program the segment size */
+	intel_global_hist_sf_update_seg(i915, pipe, clip);
+
+	return 0;
+}
+
 void intel_global_hist_deinit(struct intel_crtc *intel_crtc)
 {
 	struct intel_global_hist *global_hist = intel_crtc->global_hist;
diff --git a/drivers/gpu/drm/i915/display/intel_global_hist.h b/drivers/gpu/drm/i915/display/intel_global_hist.h
index c6621bf4ea61..827c61badf66 100644
--- a/drivers/gpu/drm/i915/display/intel_global_hist.h
+++ b/drivers/gpu/drm/i915/display/intel_global_hist.h
@@ -82,6 +82,20 @@
 #define GLOBAL_HIST_GUARDBAND_PRECISION_FACTOR 10000   // Precision factor for threshold guardband.
 #define GLOBAL_HIST_DEFAULT_GUARDBAND_DELAY 0x04
 
+#define _DPST_SF_CTL_A					0x490D0
+#define _DPST_SF_CTL_B					0x491D0
+#define DPST_SF_CTL(pipe)				_MMIO_PIPE(pipe, _DPST_SF_CTL_A, _DPST_SF_CTL_B)
+#define DPST_SF_CTL_ENABLE				(1 << 31)
+#define _DPST_SF_SEG_A					0x490D4
+#define _DPST_SF_SEG_B					0x491D4
+#define DPST_SF_SEG(pipe)				_MMIO_PIPE(pipe, _DPST_SF_CTL_A, _DPST_SF_CTL_B)
+#define DPST_SF_SEG_START_MASK				REG_GENMASK(30, 24)
+#define DPST_SF_SEG_START(val)				REG_FIELD_PREP(DPST_SF_SEG_START_MASK, val)
+#define DPST_SF_SEG_END_MASK				REG_GENMASK(22, 16)
+#define DPST_SF_SEG_END(val)				REG_FIELD_PREP(DPST_SF_SEG_END_MASK, val)
+#define DPST_SF_SEG_SIZE_MASK				REG_GENMASK(15, 0)
+#define DPST_SF_SEG_SIZE(val)				REG_FIELD_PREP(DPST_SF_SEG_SIZE_MASK, val)
+
 enum intel_global_hist_status {
 	INTEL_GLOBAL_HIST_ENABLE,
 	INTEL_GLOBAL_HIST_DISABLE,
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [1/6] drm/i915/display: Add support for global histogram

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915/display: Add support for global histogram
URL   : https://patchwork.freedesktop.org/series/117948/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/117948/revisions/1/mbox/ not applied
Applying: drm/i915/display: Add support for global histogram
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/Makefile).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915/display: Add support for global histogram
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced

Re: [Intel-gfx] [PATCH 1/6] drm/i915/display: Add support for global histogram

[Jani Nikula]

On Thu, 18 May 2023, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> API are added to enable/disable histogram. Upon generation of histogram
> interrupt its notified to the usespace. User can then use this histogram
> and generate a LUT which is then fed back to the enahancement block.
> Histogram is an image statistics based on the input pixel stream.
> LUT is a look up table consisiting of pixel data.

Where's the corresponding userspace?

See Documentation/gpu/drm-uapi.rst under "Open-Source Userspace
Requirements".

> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |   1 +
>  .../drm/i915/display/intel_display_types.h    |   3 +
>  .../gpu/drm/i915/display/intel_global_hist.c  | 295 ++++++++++++++++++
>  .../gpu/drm/i915/display/intel_global_hist.h  | 117 +++++++
>  4 files changed, 416 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_global_hist.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_global_hist.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 5ab909ec24e5..eac1e0d7bd30 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -295,6 +295,7 @@ i915-y += \
>  	display/intel_dpll.o \
>  	display/intel_dpll_mgr.o \
>  	display/intel_dpt.o \
> +	display/intel_global_hist.o \

Comment near the top of the Makefile:

# Please keep these build lists sorted!

Also, I'm not sure "global hist" is a good name.

>  	display/intel_drrs.o \
>  	display/intel_dsb.o \
>  	display/intel_fb.o \
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index ac6951b3e5bd..9848fcf73b87 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1462,6 +1462,9 @@ struct intel_crtc {
>  	/* for loading single buffered registers during vblank */
>  	struct pm_qos_request vblank_pm_qos;
>  
> +	/* GLOBAL_HIST data */

What information does this comment provide that the struct name and
member name does not already have?

> +	struct intel_global_hist *global_hist;
> +
>  #ifdef CONFIG_DEBUG_FS
>  	struct intel_pipe_crc pipe_crc;
>  	u32 cpu_fifo_underrun_count;
> diff --git a/drivers/gpu/drm/i915/display/intel_global_hist.c b/drivers/gpu/drm/i915/display/intel_global_hist.c
> new file mode 100644
> index 000000000000..ea5bcd195017
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_global_hist.c
> @@ -0,0 +1,295 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + *

When you have the SPDX header above, you can drop the license text
below.

> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_file.h>
> +#include "i915_reg.h"
> +#include "i915_drv.h"
> +#include "intel_display_types.h"
> +#include "intel_de.h"
> +#include "intel_global_hist.h"
> +
> +static int intel_global_hist_get_data(struct drm_i915_private *i915,
> +		enum pipe pipe)
> +{
> +	struct intel_crtc *intel_crtc = to_intel_crtc(
> +			drm_crtc_from_index(&i915->drm, pipe));

crtc index != pipe.

> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +	u32 dpstbin;
> +	int ret = 0, i = 0;
> +
> +	/*
> +	 * TODO: PSR to be exited while reading the Histogram data
> +	 * Set DPST_CTL Bin Reg function select to TC
> +	 * Set DPST_CTL Bin Register Index to 0
> +	 */
> +	intel_de_rmw(i915, DPST_CTL(pipe),
> +		     DPST_CTL_BIN_REG_FUNC_SEL | DPST_CTL_BIN_REG_MASK, 0);
> +
> +	for (i = 0; i < GLOBAL_HIST_BIN_COUNT; i++) {
> +		dpstbin = intel_de_read(i915, DPST_BIN(pipe));
> +		if (dpstbin & DPST_BIN_BUSY) {
> +			/*
> +			 * If DPST_BIN busy bit is set, then set the
> +			 * DPST_CTL bin reg index to 0 and proceed
> +			 * from begining
> +			 */
> +			intel_de_rmw(i915, DPST_CTL(pipe),
> +				     DPST_CTL_BIN_REG_MASK, 0);
> +			i = 0;
> +		}
> +		global_hist->bindata[i] = dpstbin & DPST_BIN_DATA_MASK;
> +		drm_dbg_atomic(&i915->drm, "Hist[%d]=%x\n",
> +				i, global_hist->bindata[i]);
> +	}
> +
> +	/* Clear histogram interrupt by setting histogram interrupt status bit*/
> +	intel_de_rmw(i915, DPST_GUARD(pipe),
> +			DPST_GUARD_HIST_EVENT_STATUS, 1);
> +
> +	return ret;
> +}
> +
> +int intel_global_hist_compute_config(struct intel_crtc *intel_crtc)
> +{
> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +	struct drm_i915_private *i915 = global_hist->i915;
> +
> +	if (!global_hist->has_edp) {
> +		drm_err(&i915->drm, "Not a eDP panel\n");
> +		return -EINVAL;
> +	}
> +	if (!global_hist->has_pwm) {
> +		drm_err(&i915->drm, "eDP doesn't have PWM based backlight, cannot enable GLOBAL_HIST\n");
> +		return -EINVAL;
> +	}

For both of the above, the histogram generation does not depend on eDP
nor PWM backlight, AFAICT. The kernel should provide mechanism, not
policy.

> +	/* Restrictions for enabling GLOBAL_HIST */
> +	global_hist->can_enable = true;

Functions that are about "compute config" should operate on the
crtc_state, not on the crtc.

> +	return 0;
> +}
> +
> +static void intel_global_hist_handle_int_work(struct work_struct *work)
> +{
> +	struct intel_global_hist *global_hist = container_of(work,
> +		struct intel_global_hist, handle_global_hist_int_work.work);
> +	struct drm_i915_private *i915 = global_hist->i915;
> +	char *global_hist_event[] = {"GLOBAL_HIST=1", NULL};
> +
> +	/* Notify user for Histogram rediness */
> +	if (kobject_uevent_env(&i915->drm.primary->kdev->kobj, KOBJ_CHANGE,
> +				global_hist_event))
> +		drm_err(&i915->drm, "sending GLOBAL_HIST event failed\n");
> +	intel_global_hist_get_data(i915, global_hist->pipe);
> +}
> +
> +void intel_global_hist_irq_handler(struct drm_i915_private *i915, enum pipe pipe)
> +{
> +	struct intel_crtc *intel_crtc =
> +		to_intel_crtc(drm_crtc_from_index(&i915->drm, pipe));

crtc index != pipe

> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +
> +	if (!global_hist->has_pwm) {
> +		drm_err(&i915->drm,
> +			"eDP doesn't have PWM based backlight, failure in GLOBAL_HIST\n");
> +		return;
> +	}

As said, this is unnecessary.

> +	queue_delayed_work(global_hist->wq,
> +			   &global_hist->handle_global_hist_int_work, 0);
> +}
> +static void intel_global_hist_enable_dithering(struct drm_i915_private *dev_priv,
> +		enum pipe pipe)
> +{
> +	intel_de_rmw(dev_priv, PIPEMISC(pipe), PIPEMISC_DITHER_ENABLE,
> +		     PIPEMISC_DITHER_ENABLE);
> +}
> +
> +static int intel_global_hist_enable(struct intel_crtc *intel_crtc)
> +{
> +	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +	int pipe = intel_crtc->pipe;
> +	uint32_t gbandthreshold;

Please only use u32 etc, not the C99 uint32_t types.

> +
> +	if (!global_hist->has_pwm) {
> +		drm_err(&i915->drm,
> +			"eDP doesn't have PWM based backlight, cannot enable GLOBAL_HIST\n");

Mechanism, not policy. Why can't the userspace request a histogram for
whatever they want?

> +		return -EINVAL;
> +	}
> +
> +	/* Pipe Dithering should be enabled with GLOBAL_HIST */
> +	intel_global_hist_enable_dithering(i915, pipe);
> +
> +	/*
> +	 * enable DPST_CTL Histogram mode
> +	 * Clear DPST_CTL Bin Reg function select to TC
> +	 */
> +	intel_de_rmw(i915, DPST_CTL(pipe),
> +		     DPST_CTL_BIN_REG_FUNC_SEL | DPST_CTL_IE_HIST_EN |
> +		     DPST_CTL_HIST_MODE | DPST_CTL_IE_TABLE_VALUE_FORMAT,
> +		     DPST_CTL_BIN_REG_FUNC_TC | DPST_CTL_IE_HIST_EN |
> +		     DPST_CTL_HIST_MODE_HSV |
> +		     DPST_CTL_IE_TABLE_VALUE_FORMAT_1INT_9FRAC);
> +	/* check if wait for one vblank is required */

The function call is quite clear on its own, I think.

> +	drm_crtc_wait_one_vblank(&intel_crtc->base);
> +
> +	/* TODO: one time programming: Program GuardBand Threshold */
> +	gbandthreshold = ((intel_crtc->config->hw.adjusted_mode.vtotal *
> +				intel_crtc->config->hw.adjusted_mode.htotal) *
> +				GLOBAL_HIST_GUARDBAND_THRESHOLD_DEFAULT) /
> +				GLOBAL_HIST_GUARDBAND_PRECISION_FACTOR;
> +
> +	/* Enable histogram interrupt mode */
> +	intel_de_rmw(i915, DPST_GUARD(pipe),
> +		     DPST_GUARD_THRESHOLD_GB_MASK |
> +		     DPST_GUARD_INTERRUPT_DELAY_MASK | DPST_GUARD_HIST_INT_EN,
> +		     DPST_GUARD_THRESHOLD_GB(gbandthreshold) |
> +		     DPST_GUARD_INTERRUPT_DELAY(GLOBAL_HIST_DEFAULT_GUARDBAND_DELAY) |
> +		     DPST_GUARD_HIST_INT_EN);
> +
> +	/* Clear pending interrupts has to be done on seperate write */
> +	intel_de_rmw(i915, DPST_GUARD(pipe),
> +			DPST_GUARD_HIST_EVENT_STATUS, 1);
> +
> +	global_hist->enable = true;
> +
> +	return 0;
> +}
> +
> +static int intel_global_hist_disable(struct intel_crtc *intel_crtc)
> +{
> +	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +	int pipe = intel_crtc->pipe;
> +
> +	/* Pipe Dithering should be enabled with GLOBAL_HIST */
> +	intel_global_hist_enable_dithering(i915, pipe);
> +
> +	/* Clear pending interrupts and disable interrupts */
> +	intel_de_rmw(i915, DPST_GUARD(pipe),
> +		     DPST_GUARD_HIST_INT_EN | DPST_GUARD_HIST_EVENT_STATUS, 0);
> +
> +	/* disable DPST_CTL Histogram mode */
> +	intel_de_rmw(i915, DPST_CTL(pipe),
> +		     DPST_CTL_IE_HIST_EN, 0);
> +
> +	cancel_delayed_work(&global_hist->handle_global_hist_int_work);
> +	global_hist->enable = false;
> +
> +	return 0;
> +}
> +
> +int intel_global_hist_update(struct intel_crtc *intel_crtc, bool enable)
> +{
> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> +
> +	if (!global_hist->can_enable) {
> +		drm_err(&i915->drm,
> +			"GLOBAL_HIST not supported, compute config failed\n");

Why is this an error?

> +		return 0;
> +	}
> +
> +	if (enable)
> +		return intel_global_hist_enable(intel_crtc);
> +	else
> +		return intel_global_hist_disable(intel_crtc);
> +}
> +
> +int intel_global_hist_set_iet_lut(struct intel_crtc *intel_crtc, u32 *data)

Stuff like this should usually pass in the length.

> +{
> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> +	int pipe = intel_crtc->pipe;
> +	int i = 0;
> +
> +	if (!global_hist->enable) {
> +		drm_err(&i915->drm, "GLOBAL_HIST not enabled");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Set DPST_CTL Bin Reg function select to IE
> +	 * Set DPST_CTL Bin Register Index to 0
> +	 */
> +	intel_de_rmw(i915, DPST_CTL(pipe),
> +		     DPST_CTL_BIN_REG_FUNC_SEL | DPST_CTL_BIN_REG_MASK,
> +		     DPST_CTL_BIN_REG_FUNC_IE | DPST_CTL_BIN_REG_CLEAR);
> +	for (i = 0; i < GLOBAL_HIST_IET_LENGTH; i++) {
> +		intel_de_rmw(i915, DPST_BIN(pipe),
> +			     DPST_BIN_DATA_MASK, data[i]);
> +		drm_dbg_atomic(&i915->drm, "iet_lut[%d]=%x\n", i, data[i]);

atomic?

> +	}
> +	intel_de_rmw(i915, DPST_CTL(pipe),
> +		     DPST_CTL_ENHANCEMENT_MODE_MASK | DPST_CTL_IE_MODI_TABLE_EN,
> +		     DPST_CTL_EN_MULTIPLICATIVE | DPST_CTL_IE_MODI_TABLE_EN);
> +	/* Once IE is applied, change DPST CTL to TC */
> +	intel_de_rmw(i915, DPST_CTL(pipe),
> +		     DPST_CTL_BIN_REG_FUNC_SEL, DPST_CTL_BIN_REG_FUNC_TC);
> +
> +	return 0;
> +}
> +
> +void intel_global_hist_deinit(struct intel_crtc *intel_crtc)
> +{
> +	struct intel_global_hist *global_hist = intel_crtc->global_hist;
> +
> +	cancel_delayed_work(&global_hist->handle_global_hist_int_work);
> +	destroy_workqueue(global_hist->wq);
> +	kfree(global_hist);
> +}
> +
> +int intel_global_hist_init(struct intel_crtc *intel_crtc)
> +{
> +	struct drm_i915_private *i915 = to_i915(intel_crtc->base.dev);
> +	struct intel_global_hist *global_hist;
> +
> +	/* Allocate global_hist internal struct */
> +	global_hist = kzalloc(sizeof(*global_hist), GFP_KERNEL);
> +	if (unlikely(!global_hist)) {

Please drop the unlikely.

> +		drm_err(&i915->drm,
> +			"Failed to allocate GLOBAL_HIST event\n");

No error messages on allocation failures, please.

> +		kfree(global_hist);

The allocation just failed, there's no need to free it.

> +		return -ENOMEM;
> +	}
> +
> +	intel_crtc->global_hist = global_hist;
> +	global_hist->pipe = intel_crtc->pipe;
> +	global_hist->can_enable = false;
> +	global_hist->wq = alloc_ordered_workqueue("global_hist_wq",
> +						 WQ_MEM_RECLAIM);
> +	if (global_hist->wq == NULL) {
> +		drm_err(&i915->drm,
> +			"failed to create work queue\n");

No error messages on allocation failures, please.

> +		kfree(global_hist);
> +		return -ENOMEM;
> +	}
> +
> +	INIT_DEFERRABLE_WORK(&global_hist->handle_global_hist_int_work,
> +		intel_global_hist_handle_int_work);
> +
> +	global_hist->i915 = i915;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_global_hist.h b/drivers/gpu/drm/i915/display/intel_global_hist.h
> new file mode 100644
> index 000000000000..c6621bf4ea61
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_global_hist.h
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation

The license text below can be dropped with SPDX.

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __INTEL_GLOBAL_HIST_H__
> +#define __INTEL_GLOBAL_HIST_H__
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_file.h>

Please use minimal includes, and use forward declarations where
possible. AFAICT neither of these are required.

> +#include "intel_display.h"
> +#include "../i915_reg.h"

What you really need is intel_display_reg_defs.h, I think, but no matter
what please don't use relative includes with "../" because the i915 top
level directory is already in include path.

> +
> +/* GLOBAL_HIST related registers */

Please put the histogram related registers to a separate file.

> +#define _DPST_CTL_A					0x490C0
> +#define _DPST_CTL_B					0x491C0
> +#define DPST_CTL(pipe)					_MMIO_PIPE(pipe, _DPST_CTL_A, _DPST_CTL_B)
> +#define DPST_CTL_IE_HIST_EN				REG_BIT(31)
> +#define DPST_CTL_IE_MODI_TABLE_EN			REG_BIT(27)
> +#define DPST_CTL_HIST_MODE				REG_BIT(24)
> +#define DPST_CTL_ENHANCEMENT_MODE_MASK			REG_GENMASK(14, 13)
> +#define DPST_CTL_EN_MULTIPLICATIVE			REG_FIELD_PREP(DPST_CTL_ENHANCEMENT_MODE_MASK, 2)
> +#define DPST_CTL_IE_TABLE_VALUE_FORMAT			REG_BIT(15)
> +#define DPST_CTL_BIN_REG_FUNC_SEL			REG_BIT(11)
> +#define DPST_CTL_BIN_REG_FUNC_TC			(0 << 0)
> +#define DPST_CTL_BIN_REG_FUNC_IE			(1 << 11)
> +#define DPST_CTL_BIN_REG_MASK				REG_GENMASK(6, 0)
> +#define DPST_CTL_BIN_REG_CLEAR				REG_FIELD_PREP(DPST_CTL_BIN_REG_MASK, 0)
> +
> +#define _DPST_GUARD_A					0x490C8
> +#define _DPST_GUARD_B					0x491C8
> +#define DPST_GUARD(pipe)				_MMIO_PIPE(pipe, _DPST_GUARD_A, _DPST_GUARD_B)
> +#define DPST_GUARD_HIST_INT_EN				REG_BIT(31)
> +#define DPST_GUARD_HIST_EVENT_STATUS			REG_BIT(30)
> +#define DPST_GUARD_INTERRUPT_DELAY_MASK			REG_GENMASK(29, 22)
> +#define DPST_GUARD_INTERRUPT_DELAY(val)			REG_FIELD_PREP(DPST_GUARD_INTERRUPT_DELAY_MASK, val)
> +#define DPST_GUARD_THRESHOLD_GB_MASK			REG_GENMASK(21, 0)
> +#define DPST_GUARD_THRESHOLD_GB(val)			REG_FIELD_PREP(DPST_GUARD_THRESHOLD_GB_MASK, val)
> +
> +#define _DPST_BIN_A					0x490C4
> +#define _DPST_BIN_B					0x491C4
> +#define DPST_BIN(pipe)					_MMIO_PIPE(pipe, _DPST_BIN_A, _DPST_BIN_B)
> +#define DPST_BIN_DATA_MASK				REG_GENMASK(23, 0)
> +#define DPST_BIN_BUSY					REG_BIT(31)
> +
> +#define DPST_CTL_IE_TABLE_VALUE_FORMAT_2INT_8FRAC	(1 << 15)
> +#define DPST_CTL_IE_TABLE_VALUE_FORMAT_1INT_9FRAC	(0 << 15)
> +#define DPST_CTL_HIST_MODE_YUV				(0 << 24)
> +#define DPST_CTL_HIST_MODE_HSV				(1 << 24)
> +
> +
> +#define INTEL_GLOBAL_HISTOGRAM_PIPEA	0x90000000
> +#define INTEL_GLOBAL_HISTOGRAM_PIPEB	0x90000002
> +#define INTEL_GLOBAL_HISTOGRAM_EVENT(pipe)	_PIPE(pipe, \
> +						INTEL_GLOBAL_HISTOGRAM_PIPEA, \
> +						INTEL_GLOBAL_HISTOGRAM_PIPEB)
> +
> +#define GLOBAL_HIST_BIN_COUNT			32
> +#define GLOBAL_HIST_IET_LENGTH			33
> +
> +#define GLOBAL_HIST_GUARDBAND_THRESHOLD_DEFAULT 300    // 3.0% of the pipe's current pixel count.
> +#define GLOBAL_HIST_GUARDBAND_PRECISION_FACTOR 10000   // Precision factor for threshold guardband.
> +#define GLOBAL_HIST_DEFAULT_GUARDBAND_DELAY 0x04
> +
> +enum intel_global_hist_status {
> +	INTEL_GLOBAL_HIST_ENABLE,
> +	INTEL_GLOBAL_HIST_DISABLE,
> +};
> +
> +enum intel_global_histogram {
> +	INTEL_GLOBAL_HISTOGRAM,
> +};
> +
> +enum intel_global_hist_lut {
> +	INTEL_GLOBAL_HIST_PIXEL_FACTOR,
> +};
> +
> +struct intel_global_hist {
> +	struct drm_i915_private *i915;
> +	struct workqueue_struct *wq;
> +	struct delayed_work handle_global_hist_int_work;
> +	bool enable;
> +	bool has_pwm;
> +	bool has_edp;
> +	bool can_enable;
> +	enum pipe pipe;
> +	u32 bindata[GLOBAL_HIST_BIN_COUNT];
> +};
> +
> +int intel_global_hist_compute_config(struct intel_crtc *intel_crtc);
> +void intel_global_hist_irq_handler(struct drm_i915_private *i915, enum pipe pipe);
> +int intel_global_hist_update(struct intel_crtc *intel_crtc, bool enable);
> +int intel_global_hist_set_iet_lut(struct intel_crtc *intel_crtc, u32 *data);
> +int intel_global_hist_init(struct intel_crtc *intel_crtc);
> +void intel_global_hist_deinit(struct intel_crtc *intel_crtc);

Some or many of these should probably operate on crtc state etc. instead
of plain crtc, but let's see.

> +
> +#endif /* __INTEL_GLOBAL_HIST_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 2/6] drm/i915/display: global histogram irq handler

[Jani Nikula]

On Thu, 18 May 2023, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> With the enablement of global histogram, upon generation of histogram,
> an interrupt is triggered. This patch handles the irq.
>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>

This needs a rebase I think.

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 6 +++++-
>  drivers/gpu/drm/i915/i915_reg.h | 5 +++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index e28bfb5f7347..d72fb6d9282d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -43,6 +43,7 @@
>  #include "display/intel_hotplug.h"
>  #include "display/intel_lpe_audio.h"
>  #include "display/intel_psr.h"
> +#include "display/intel_global_hist.h"
>  
>  #include "gt/intel_breadcrumbs.h"
>  #include "gt/intel_gt.h"
> @@ -2765,6 +2766,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  		ret = IRQ_HANDLED;
>  		intel_uncore_write(&dev_priv->uncore, GEN8_DE_PIPE_IIR(pipe), iir);
>  
> +		if (iir & GEN9_PIPE_GLOBAL_HIST_EVENT)
> +			intel_global_hist_irq_handler(dev_priv, pipe);
> +
>  		if (iir & GEN8_PIPE_VBLANK)
>  			intel_handle_vblank(dev_priv, pipe);
>  
> @@ -5043,7 +5047,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  	struct intel_uncore *uncore = &dev_priv->uncore;
>  
>  	u32 de_pipe_masked = gen8_de_pipe_fault_mask(dev_priv) |
> -		GEN8_PIPE_CDCLK_CRC_DONE;
> +		GEN8_PIPE_CDCLK_CRC_DONE | GEN9_PIPE_GLOBAL_HIST_EVENT;
>  	u32 de_pipe_enables;
>  	u32 de_port_masked = gen8_de_port_aux_mask(dev_priv);
>  	u32 de_port_enables;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 94d0c8d14d43..546207ac4859 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3887,7 +3887,7 @@
>  #define   PIPE_HOTPLUG_INTERRUPT_ENABLE		(1UL << 26)
>  #define   PIPE_VSYNC_INTERRUPT_ENABLE		(1UL << 25)
>  #define   PIPE_DISPLAY_LINE_COMPARE_ENABLE	(1UL << 24)
> -#define   PIPE_DPST_EVENT_ENABLE		(1UL << 23)
> +#define   PIPE_GLOBAL_HIST_EVENT_ENABLE		(1UL << 23)
>  #define   SPRITE0_FLIP_DONE_INT_EN_VLV		(1UL << 22)
>  #define   PIPE_LEGACY_BLC_EVENT_ENABLE		(1UL << 22)
>  #define   PIPE_ODD_FIELD_INTERRUPT_ENABLE	(1UL << 21)
> @@ -3910,7 +3910,7 @@
>  #define   PIPE_HOTPLUG_INTERRUPT_STATUS		(1UL << 10)
>  #define   PIPE_VSYNC_INTERRUPT_STATUS		(1UL << 9)
>  #define   PIPE_DISPLAY_LINE_COMPARE_STATUS	(1UL << 8)
> -#define   PIPE_DPST_EVENT_STATUS		(1UL << 7)
> +#define   PIPE_GLOBAL_HIST_EVENT_STATUS		(1UL << 7)
>  #define   PIPE_A_PSR_STATUS_VLV			(1UL << 6)
>  #define   PIPE_LEGACY_BLC_EVENT_STATUS		(1UL << 6)
>  #define   PIPE_ODD_FIELD_INTERRUPT_STATUS	(1UL << 5)
> @@ -5815,6 +5815,7 @@
>  #define  GEN8_PIPE_VSYNC		(1 << 1)
>  #define  GEN8_PIPE_VBLANK		(1 << 0)
>  #define  GEN9_PIPE_CURSOR_FAULT		(1 << 11)
> +#define  GEN9_PIPE_GLOBAL_HIST_EVENT	(1 << 12)
>  #define  GEN11_PIPE_PLANE7_FAULT	(1 << 22)
>  #define  GEN11_PIPE_PLANE6_FAULT	(1 << 21)
>  #define  GEN11_PIPE_PLANE5_FAULT	(1 << 20)

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 3/6] drm/i915/display: global histogram restrictions

[Jani Nikula]

On Thu, 18 May 2023, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> For global histogram the panel should be edp and should have pwm based
> backlight controller. Flags are updated accordingly.
>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index cd21b0ddbabb..975d6bdb59f3 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -17,12 +17,14 @@
>  #include "intel_crtc_state_dump.h"
>  #include "intel_ddi.h"
>  #include "intel_de.h"
> +#include "intel_dp.h"
>  #include "intel_display.h"
>  #include "intel_display_power.h"
>  #include "intel_display_types.h"
>  #include "intel_modeset_setup.h"
>  #include "intel_pch_display.h"
>  #include "intel_pm.h"
> +#include "intel_global_hist.h"
>  
>  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  					struct drm_modeset_acquire_ctx *ctx)
> @@ -309,6 +311,7 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>  	struct intel_crtc_state *crtc_state = crtc ?
>  		to_intel_crtc_state(crtc->base.state) : NULL;
> +	struct intel_panel *panel;
>  
>  	/*
>  	 * We need to check both for a crtc link (meaning that the encoder is
> @@ -376,6 +379,15 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
>  
>  	if (HAS_DDI(i915))
>  		intel_ddi_sanitize_encoder_pll_mapping(encoder);
> +
> +	/* validate the global hist struct elements */
> +	if (intel_dp_is_port_edp(i915, encoder->port)) {
> +		crtc->global_hist->has_edp = true;
> +		panel = &connector->panel;
> +		if (panel->backlight.present == true)
> +			crtc->global_hist->has_pwm = true;
> +	}

Again, the dependency on eDP and backlight is unnecessary policy.

Side note, backlight being present doesn't mean "has pwm". It could be
some other kind of backlight as well.

BR,
Jani.

> +
>  }
>  
>  /* FIXME read out full plane state for all planes */

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 1/6] drm/i915/display: Add support for global histogram

[Murthy, Arun R]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Tuesday, May 23, 2023 3:40 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/6] drm/i915/display: Add support for global
> histogram
> 
> On Thu, 18 May 2023, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> > API are added to enable/disable histogram. Upon generation of
> > histogram interrupt its notified to the usespace. User can then use
> > this histogram and generate a LUT which is then fed back to the
> enahancement block.
> > Histogram is an image statistics based on the input pixel stream.
> > LUT is a look up table consisiting of pixel data.
> 
> Where's the corresponding userspace?
> 
> See Documentation/gpu/drm-uapi.rst under "Open-Source Userspace
> Requirements".
> 
Will be posting info on the user space component very soon.

> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile                 |   1 +
> >  .../drm/i915/display/intel_display_types.h    |   3 +
> >  .../gpu/drm/i915/display/intel_global_hist.c  | 295
> > ++++++++++++++++++  .../gpu/drm/i915/display/intel_global_hist.h  |
> > 117 +++++++
> >  4 files changed, 416 insertions(+)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_global_hist.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_global_hist.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile index 5ab909ec24e5..eac1e0d7bd30
> > 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -295,6 +295,7 @@ i915-y += \
> >  	display/intel_dpll.o \
> >  	display/intel_dpll_mgr.o \
> >  	display/intel_dpt.o \
> > +	display/intel_global_hist.o \
> 
> Comment near the top of the Makefile:
> 
> # Please keep these build lists sorted!
> 
> Also, I'm not sure "global hist" is a good name.

The hardware block is named as histogram block and enhancement block. Hence the name global histogram. A short form of this global_hist!
Any other inputs on the naming convention?

Thanks and Regards,
Arun R Murthy
--------------------