[RFC 0/5] Introduce drm sharpening property

[RFC 0/5] Introduce drm sharpening property

[Nemesa Garg]

	Many a times images are blurred or upscaled content is also not as
crisp as original rendered image. Traditional sharpening techniques often
apply a uniform level of enhancement across entire image, which sometimes
result in over-sharpening of some areas and potential loss of natural details. 

Intel has come up with Display Engine based adaptive sharpening filter 
with minimal power and performance impact. From LNL onwards, the Display
hardware can use one of the pipe scaler for adaptive sharpness filter.
This can be used for both gaming and non-gaming use cases like photos,
image viewing. It works on a region of pixels depending on the tap size.

This RFC is an attempt to introduce an adaptive sharpness solution which
helps in improving the image quality. For this new CRTC property is added.
The user can set this property with desired sharpness strength value with
0-255. A value of 1 representing minimum sharpening strength and 255
representing maximum sharpness strength. A strength value of 0 means no
sharpening or sharpening feature disabled.
It works on a region of pixels depending on the tap size. The coefficients
are used to generate an alpha value which is used to blend the sharpened
image to original image.
 
Userspace implementation for sharpening feature and IGT implementation
is in progress.

Nemesa Garg (5):
  drm: Introduce sharpeness mode property
  drm/i915/display/: Compute the scaler filter coefficients
  drm/i915/dispaly/: Enable the second scaler
  drm/i915/display/: Add registers and compute the strength
  drm/i915/display: Load the lut values and enable sharpness

 drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
 drivers/gpu/drm/drm_crtc.c                    |  17 ++
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/display/intel_crtc.c     |   3 +
 drivers/gpu/drm/i915/display/intel_display.c  |  20 +-
 .../drm/i915/display/intel_display_types.h    |  11 +
 .../drm/i915/display/intel_modeset_verify.c   |   1 +
 .../drm/i915/display/intel_sharpen_filter.c   | 214 ++++++++++++++++++
 .../drm/i915/display/intel_sharpen_filter.h   |  31 +++
 drivers/gpu/drm/i915/display/skl_scaler.c     |  86 ++++++-
 drivers/gpu/drm/i915/display/skl_scaler.h     |   1 +
 drivers/gpu/drm/i915/i915_reg.h               |  19 ++
 drivers/gpu/drm/xe/Makefile                   |   1 +
 include/drm/drm_crtc.h                        |  17 ++
 14 files changed, 416 insertions(+), 10 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.h

-- 
2.25.1

[RFC 1/5] drm: Introduce sharpeness mode property

[Nemesa Garg]

This allows the user to set the intensity
so as to get the sharpness effect.

It is useful in scenario when the output is blurry
and user want to sharpen the pixels.

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
 drivers/gpu/drm/drm_crtc.c        | 17 +++++++++++++++++
 include/drm/drm_crtc.h            | 17 +++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 29d4940188d4..773873726b87 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -417,6 +417,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		set_out_fence_for_crtc(state->state, crtc, fence_ptr);
 	} else if (property == crtc->scaling_filter_property) {
 		state->scaling_filter = val;
+	} else if (property == crtc->sharpening_strength_prop) {
+		state->sharpeness_strength = val;
 	} else if (crtc->funcs->atomic_set_property) {
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	} else {
@@ -454,6 +456,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = 0;
 	else if (property == crtc->scaling_filter_property)
 		*val = state->scaling_filter;
+	else if (property == crtc->sharpening_strength_prop)
+		*val = state->sharpeness_strength;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else {
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index cb90e70d85e8..d01ab76a719f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -955,3 +955,20 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
 	return 0;
 }
 EXPORT_SYMBOL(drm_crtc_create_scaling_filter_property);
+
+int drm_crtc_create_sharpening_strength_property(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+
+	struct drm_property *prop =
+		drm_property_create_range(dev, 0, "SHARPENESS_STRENGTH", 0, 255);
+
+	if (!prop)
+		return -ENOMEM;
+
+	crtc->sharpening_strength_prop = prop;
+	drm_object_attach_property(&crtc->base, prop, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_crtc_create_sharpening_strength_property);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8b48a1974da3..241514fc3eea 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -317,6 +317,16 @@ struct drm_crtc_state {
 	 */
 	enum drm_scaling_filter scaling_filter;
 
+	/**
+	 * @sharpness_strength
+	 *
+	 * Used by the user to set the sharpness intensity.
+	 * The value ranges from 0-255.
+	 * Any value greater than 0 means enabling the featuring
+	 * along with setting the value for sharpness.
+	 */
+	u8 sharpeness_strength;
+
 	/**
 	 * @event:
 	 *
@@ -1088,6 +1098,12 @@ struct drm_crtc {
 	 */
 	struct drm_property *scaling_filter_property;
 
+	/**
+	 * @sharpening_strength_prop: property to apply
+	 * the intensity of the sharpness requested.
+	 */
+	struct drm_property *sharpening_strength_prop;
+
 	/**
 	 * @state:
 	 *
@@ -1324,4 +1340,5 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
 int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc,
 					    unsigned int supported_filters);
 
+int drm_crtc_create_sharpening_strength_property(struct drm_crtc *crtc);
 #endif /* __DRM_CRTC_H__ */
-- 
2.25.1

[RFC 2/5] drm/i915/display/: Compute the scaler filter coefficients

[Nemesa Garg]

Scaler coefficient values are based on experiments
and vary for different tap value/win size. These values
are normalized by taking the sum of all values and then
dividing each value with a sum.

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  |   3 +
 .../drm/i915/display/intel_display_types.h    |   9 ++
 .../drm/i915/display/intel_sharpen_filter.c   | 121 ++++++++++++++++++
 .../drm/i915/display/intel_sharpen_filter.h   |  27 ++++
 drivers/gpu/drm/i915/i915_reg.h               |   2 +
 drivers/gpu/drm/xe/Makefile                   |   1 +
 7 files changed, 164 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c13f14edb508..97d1cf705b40 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -305,6 +305,7 @@ i915-y += \
 	display/intel_pmdemand.o \
 	display/intel_psr.o \
 	display/intel_quirks.o \
+	display/intel_sharpen_filter.o \
 	display/intel_sprite.o \
 	display/intel_sprite_uapi.o \
 	display/intel_tc.o \
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7db0655d8c9e..5a93bbd1fe25 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2444,6 +2444,9 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
 	if (crtc_state->has_pch_encoder)
 		return ilk_fdi_compute_config(crtc, crtc_state);
 
+	if (crtc_state->hw.casf_params.strength_changed)
+		intel_sharpness_scaler_compute_config(crtc_state);
+
 	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 01eb6e4e6049..a7a24d177586 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -53,6 +53,7 @@
 #include "intel_display_limits.h"
 #include "intel_display_power.h"
 #include "intel_dpll_mgr.h"
+#include "intel_sharpen_filter.h"
 #include "intel_wm_types.h"
 
 struct drm_printer;
@@ -794,6 +795,13 @@ struct intel_scaler {
 	u32 mode;
 };
 
+struct intel_sharpen_filter {
+	struct scaler_filter_coeff coeff[7];
+	u32 scaler_coefficient[119];
+	bool strength_changed;
+	u8 win_size;
+};
+
 struct intel_crtc_scaler_state {
 #define SKL_NUM_SCALERS 2
 	struct intel_scaler scalers[SKL_NUM_SCALERS];
@@ -1075,6 +1083,7 @@ struct intel_crtc_state {
 		struct drm_property_blob *degamma_lut, *gamma_lut, *ctm;
 		struct drm_display_mode mode, pipe_mode, adjusted_mode;
 		enum drm_scaling_filter scaling_filter;
+		struct intel_sharpen_filter casf_params;
 	} hw;
 
 	/* actual state of LUTs */
diff --git a/drivers/gpu/drm/i915/display/intel_sharpen_filter.c b/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
new file mode 100644
index 000000000000..366739d9dead
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 Intel Corporation
+ *
+ */
+
+#include "i915_reg.h"
+#include "intel_de.h"
+#include "intel_display_types.h"
+#include "skl_scaler.h"
+
+#define MAX_NUM_UNIQUE_COEF_FOR_SHARPNESS_FILTER 7
+#define SCALER_FILTER_NUM_TAPS 7
+#define SCALER_FILTER_NUM_PHASES 17
+#define filter_coeff_0_125 125
+#define filter_coeff_0_25 250
+#define filter_coeff_0_5 500
+#define filter_coeff_1_0 1000
+#define filter_coeff_0_0 0
+
+void intel_sharpen_filter_enable(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int id = crtc_state->scaler_state.scaler_id;
+
+	intel_de_write_fw(dev_priv, GLK_PS_COEF_INDEX_SET(crtc->pipe, id, 0),
+			  PS_COEF_INDEX_AUTO_INC);
+
+	intel_de_write_fw(dev_priv, GLK_PS_COEF_INDEX_SET(crtc->pipe, id, 1),
+			  PS_COEF_INDEX_AUTO_INC);
+
+	for (int index = 0; index < 60; index++) {
+		intel_de_write_fw(dev_priv, GLK_PS_COEF_DATA_SET(crtc->pipe, id, 0),
+				  crtc_state->hw.casf_params.scaler_coefficient[index]);
+		intel_de_write_fw(dev_priv, GLK_PS_COEF_DATA_SET(crtc->pipe, id, 1),
+				  crtc_state->hw.casf_params. scaler_coefficient[index]);
+	}
+}
+
+static void convert_sharpness_coef_binary(struct scaler_filter_coeff *coeff,
+					  u16 coefficient)
+{
+	if (coefficient < 25) {
+		coeff->mantissa = (coefficient * 2048) / 100;
+		coeff->exp = 3;
+	}
+
+	else if (coefficient < 50) {
+		coeff->mantissa = (coefficient * 1024) / 100;
+		coeff->exp = 2;
+	}
+
+	else if (coefficient < 100) {
+		coeff->mantissa = (coefficient * 512) / 100;
+		coeff->exp = 1;
+	} else {
+		coeff->mantissa = (coefficient * 256) / 100;
+		coeff->exp = 0;
+	}
+}
+
+static void intel_sharpness_filter_coeff(struct intel_crtc_state *crtc_state)
+{
+	u16 filtercoeff[MAX_NUM_UNIQUE_COEF_FOR_SHARPNESS_FILTER];
+	u16 sumcoeff = 0;
+	u8 i;
+
+	if (crtc_state->hw.casf_params.win_size == 0) {
+		filtercoeff[0] = filter_coeff_0_0;
+		filtercoeff[1] = filter_coeff_0_0;
+		filtercoeff[2] = filter_coeff_0_5;
+		filtercoeff[3] = filter_coeff_1_0;
+		filtercoeff[4] = filter_coeff_0_5;
+		filtercoeff[5] = filter_coeff_0_0;
+		filtercoeff[6] = filter_coeff_0_0;
+	}
+
+	else if (crtc_state->hw.casf_params.win_size == 1) {
+		filtercoeff[0] = filter_coeff_0_0;
+		filtercoeff[1] = filter_coeff_0_25;
+		filtercoeff[2] = filter_coeff_0_5;
+		filtercoeff[3] = filter_coeff_1_0;
+		filtercoeff[4] = filter_coeff_0_5;
+		filtercoeff[5] = filter_coeff_0_25;
+		filtercoeff[6] = filter_coeff_0_0;
+	} else {
+		filtercoeff[0] = filter_coeff_0_125;
+		filtercoeff[1] = filter_coeff_0_25;
+		filtercoeff[2] = filter_coeff_0_5;
+		filtercoeff[3] = filter_coeff_1_0;
+		filtercoeff[4] = filter_coeff_0_5;
+		filtercoeff[5] = filter_coeff_0_25;
+		filtercoeff[6] = filter_coeff_0_125;
+	}
+
+	for (i = 0; i < MAX_NUM_UNIQUE_COEF_FOR_SHARPNESS_FILTER; i++)
+		sumcoeff += filtercoeff[i];
+
+	for (i = 0; i < MAX_NUM_UNIQUE_COEF_FOR_SHARPNESS_FILTER; i++) {
+		filtercoeff[i] = (filtercoeff[i] * 100 / sumcoeff);
+		convert_sharpness_coef_binary(&crtc_state->hw.casf_params.coeff[i],
+					      filtercoeff[i]);
+	}
+}
+
+void intel_sharpness_scaler_compute_config(struct intel_crtc_state *crtc_state)
+{
+	u16 phase, tapindex, phaseoffset;
+	u16 *coeff = (u16 *)crtc_state->hw.casf_params.scaler_coefficient;
+
+	intel_sharpness_filter_coeff(crtc_state);
+
+	for (phase = 0; phase < SCALER_FILTER_NUM_PHASES; phase++) {
+		phaseoffset = SCALER_FILTER_NUM_TAPS * phase;
+		for (tapindex = 0; tapindex < SCALER_FILTER_NUM_TAPS; tapindex++) {
+			coeff[phaseoffset + tapindex] =
+				SHARP_COEFF_TO_REG_FORMAT(crtc_state->hw.casf_params.coeff[tapindex]);
+		}
+	}
+}
diff --git a/drivers/gpu/drm/i915/display/intel_sharpen_filter.h b/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
new file mode 100644
index 000000000000..6b668aaedf65
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+#ifndef __INTEL_SHARPEN_FLITER_H__
+#define __INTEL_SHARPEN_FILTER_H__
+
+#include <linux/types.h>
+
+#define SHARP_COEFF_TO_REG_FORMAT(coefficient) ((u16)(coefficient.sign << 15 | \
+			coefficient.exp << 12 | coefficient.mantissa << 3))
+
+struct intel_crtc;
+struct intel_crtc_state;
+struct intel_atomic_state;
+
+struct scaler_filter_coeff {
+	u16 sign;
+	u16 exp;
+	u16 mantissa;
+};
+
+void intel_sharpen_filter_enable(struct intel_crtc_state *crtc_state);
+void intel_sharpness_scaler_compute_config(struct intel_crtc_state *crtc_state);
+
+#endif /* __INTEL_SHARPEN_FLITER_H__ */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e00557e1a57f..9d759026add4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4072,6 +4072,8 @@
 #define   PS_VERT_INT_INVERT_FIELD		REG_BIT(20)
 #define   PS_PROG_SCALE_FACTOR			REG_BIT(19) /* tgl+ */
 #define   PS_PWRUP_PROGRESS			REG_BIT(17)
+#define   PS_BYPASS_ARMING			REG_BIT(10)
+#define   PS_DB_STALL				REG_BIT(9)
 #define   PS_V_FILTER_BYPASS			REG_BIT(8)
 #define   PS_VADAPT_EN				REG_BIT(7) /* skl/bxt */
 #define   PS_VADAPT_MODE_MASK			REG_GENMASK(6, 5) /* skl/bxt */
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index c531210695db..4148eb015c11 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -269,6 +269,7 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
 	i915-display/intel_psr.o \
 	i915-display/intel_qp_tables.o \
 	i915-display/intel_quirks.o \
+	i915-display/intel_sharpen_filter.o \
 	i915-display/intel_snps_phy.o \
 	i915-display/intel_tc.o \
 	i915-display/intel_vblank.o \
-- 
2.25.1

[RFC 3/5] drm/i915/dispaly/: Enable the second scaler

[Nemesa Garg]

The strength value should be greater than zero to
set to the scaler flag true and if the second scaler
is free then it can be used for sharpening purpose.

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  7 +-
 .../drm/i915/display/intel_display_types.h    |  1 +
 .../drm/i915/display/intel_modeset_verify.c   |  1 +
 .../drm/i915/display/intel_sharpen_filter.c   |  9 +++
 .../drm/i915/display/intel_sharpen_filter.h   |  2 +-
 drivers/gpu/drm/i915/display/skl_scaler.c     | 75 +++++++++++++++++--
 drivers/gpu/drm/i915/display/skl_scaler.h     |  1 +
 7 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5a93bbd1fe25..3d05bd203ca8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1940,7 +1940,7 @@ static void get_crtc_power_domains(struct intel_crtc_state *crtc_state,
 	set_bit(POWER_DOMAIN_PIPE(pipe), mask->bits);
 	set_bit(POWER_DOMAIN_TRANSCODER(cpu_transcoder), mask->bits);
 	if (crtc_state->pch_pfit.enabled ||
-	    crtc_state->pch_pfit.force_thru)
+	    crtc_state->pch_pfit.force_thru || crtc_state->hw.casf_params.need_scaler)
 		set_bit(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe), mask->bits);
 
 	drm_for_each_encoder_mask(encoder, &dev_priv->drm,
@@ -2195,7 +2195,7 @@ static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
 	 * PF-ID we'll need to adjust the pixel_rate here.
 	 */
 
-	if (!crtc_state->pch_pfit.enabled)
+	if (!crtc_state->pch_pfit.enabled || crtc_state->hw.casf_params.need_scaler)
 		return pixel_rate;
 
 	drm_rect_init(&src, 0, 0,
@@ -5334,6 +5334,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		PIPE_CONF_CHECK_I(vrr.guardband);
 	}
 
+	if (pipe_config->uapi.sharpeness_strength > 0)
+		PIPE_CONF_CHECK_BOOL(hw.casf_params.need_scaler);
+
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_BOOL
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index a7a24d177586..d43931127ec2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -800,6 +800,7 @@ struct intel_sharpen_filter {
 	u32 scaler_coefficient[119];
 	bool strength_changed;
 	u8 win_size;
+	bool need_scaler;
 };
 
 struct intel_crtc_scaler_state {
diff --git a/drivers/gpu/drm/i915/display/intel_modeset_verify.c b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
index 076298a8d405..e7e2d26a4c9c 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_verify.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
@@ -177,6 +177,7 @@ verify_crtc_state(struct intel_atomic_state *state,
 		    crtc->base.name);
 
 	hw_crtc_state->hw.enable = sw_crtc_state->hw.enable;
+	hw_crtc_state->hw.casf_params.need_scaler = sw_crtc_state->hw.casf_params.need_scaler;
 
 	intel_crtc_get_pipe_config(hw_crtc_state);
 
diff --git a/drivers/gpu/drm/i915/display/intel_sharpen_filter.c b/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
index 366739d9dead..221dca3bcba5 100644
--- a/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
+++ b/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
@@ -36,6 +36,15 @@ void intel_sharpen_filter_enable(struct intel_crtc_state *crtc_state)
 		intel_de_write_fw(dev_priv, GLK_PS_COEF_DATA_SET(crtc->pipe, id, 1),
 				  crtc_state->hw.casf_params. scaler_coefficient[index]);
 	}
+
+	casf_scaler_enable(crtc_state);
+}
+
+int intel_filter_compute_config(struct intel_crtc_state *crtc_state)
+{
+	crtc_state->hw.casf_params.need_scaler = true;
+
+	return 0;
 }
 
 static void convert_sharpness_coef_binary(struct scaler_filter_coeff *coeff,
diff --git a/drivers/gpu/drm/i915/display/intel_sharpen_filter.h b/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
index 6b668aaedf65..89c0d689469c 100644
--- a/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
+++ b/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
@@ -23,5 +23,5 @@ struct scaler_filter_coeff {
 
 void intel_sharpen_filter_enable(struct intel_crtc_state *crtc_state);
 void intel_sharpness_scaler_compute_config(struct intel_crtc_state *crtc_state);
-
+int intel_filter_compute_config(struct intel_crtc_state *crtc_state);
 #endif /* __INTEL_SHARPEN_FLITER_H__ */
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index 8a934bada624..be61a6ebd7e3 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -121,7 +121,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	 * the 90/270 degree plane rotation cases (to match the
 	 * GTT mapping), hence no need to account for rotation here.
 	 */
-	if (src_w != dst_w || src_h != dst_h)
+	if (src_w != dst_w || src_h != dst_h || crtc_state->hw.casf_params.need_scaler)
 		need_scaler = true;
 
 	/*
@@ -355,6 +355,7 @@ static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_stat
 				     int *scaler_id)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	struct intel_crtc_state *crtc_state = to_intel_crtc_state(intel_crtc->base.state);
 	int j;
 	u32 mode;
 
@@ -363,6 +364,8 @@ static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_stat
 		for (j = 0; j < intel_crtc->num_scalers; j++) {
 			if (scaler_state->scalers[j].in_use)
 				continue;
+			if (crtc_state->hw.casf_params.need_scaler && j != 1)
+				continue;
 
 			*scaler_id = j;
 			scaler_state->scalers[*scaler_id].in_use = 1;
@@ -374,6 +377,10 @@ static int intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_stat
 		     "Cannot find scaler for %s:%d\n", name, idx))
 		return -EINVAL;
 
+	if (crtc_state->hw.casf_params.need_scaler) {
+		mode = SKL_PS_SCALER_MODE_HQ;
+	}
+
 	/* set scaler mode */
 	if (plane_state && plane_state->hw.fb &&
 	    plane_state->hw.fb->format->is_yuv &&
@@ -677,6 +684,15 @@ static void glk_program_nearest_filter_coefs(struct drm_i915_private *dev_priv,
 	intel_de_write_fw(dev_priv, GLK_PS_COEF_INDEX_SET(pipe, id, set), 0);
 }
 
+static u32 scaler_filter_select(void)
+{
+	return (PS_FILTER_PROGRAMMED |
+			PS_Y_VERT_FILTER_SELECT(1) |
+			PS_Y_HORZ_FILTER_SELECT(0) |
+			PS_UV_VERT_FILTER_SELECT(1) |
+			PS_UV_HORZ_FILTER_SELECT(0));
+}
+
 static u32 skl_scaler_get_filter_select(enum drm_scaling_filter filter, int set)
 {
 	if (filter == DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
@@ -704,6 +720,48 @@ static void skl_scaler_setup_filter(struct drm_i915_private *dev_priv, enum pipe
 	}
 }
 
+void casf_scaler_enable(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct drm_display_mode *adjusted_mode =
+		&crtc_state->hw.adjusted_mode;
+	struct intel_crtc_scaler_state *scaler_state =
+		&crtc_state->scaler_state;
+	struct drm_rect src, dest;
+	int id, width, height;
+	int x, y;
+	enum pipe pipe = crtc->pipe;
+	u32 ps_ctrl;
+
+	width = adjusted_mode->crtc_hdisplay;
+	height = adjusted_mode->crtc_vdisplay;
+
+	x = y = 0;
+	drm_rect_init(&dest, x, y, width, height);
+
+	struct drm_rect *dst = &dest;
+
+	x = dst->x1;
+	y = dst->y1;
+	width = drm_rect_width(dst);
+	height = drm_rect_height(dst);
+	id = scaler_state->scaler_id;
+
+	drm_rect_init(&src, 0, 0,
+		      drm_rect_width(&crtc_state->pipe_src) << 16,
+		      drm_rect_height(&crtc_state->pipe_src) << 16);
+
+	ps_ctrl = PS_SCALER_EN | PS_BINDING_PIPE | scaler_state->scalers[id].mode |
+		PS_BYPASS_ARMING | PS_DB_STALL | scaler_filter_select();
+
+	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), ps_ctrl);
+	intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
+			  PS_WIN_XPOS(x) | PS_WIN_YPOS(y));
+	intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
+			  PS_WIN_XSIZE(width) | PS_WIN_YSIZE(height));
+}
+
 void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -874,16 +932,19 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
 			continue;
 
 		id = i;
-		crtc_state->pch_pfit.enabled = true;
+
+		if (!crtc_state->hw.casf_params.need_scaler)
+			crtc_state->pch_pfit.enabled = true;
 
 		pos = intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
 		size = intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
 
-		drm_rect_init(&crtc_state->pch_pfit.dst,
-			      REG_FIELD_GET(PS_WIN_XPOS_MASK, pos),
-			      REG_FIELD_GET(PS_WIN_YPOS_MASK, pos),
-			      REG_FIELD_GET(PS_WIN_XSIZE_MASK, size),
-			      REG_FIELD_GET(PS_WIN_YSIZE_MASK, size));
+		if (!crtc_state->hw.casf_params.need_scaler)
+			drm_rect_init(&crtc_state->pch_pfit.dst,
+				      REG_FIELD_GET(PS_WIN_XPOS_MASK, pos),
+				      REG_FIELD_GET(PS_WIN_YPOS_MASK, pos),
+				      REG_FIELD_GET(PS_WIN_XSIZE_MASK, size),
+				      REG_FIELD_GET(PS_WIN_YSIZE_MASK, size));
 
 		scaler_state->scalers[i].in_use = true;
 		break;
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.h b/drivers/gpu/drm/i915/display/skl_scaler.h
index 63f93ca03c89..444527e6a15b 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.h
+++ b/drivers/gpu/drm/i915/display/skl_scaler.h
@@ -33,5 +33,6 @@ void skl_detach_scalers(const struct intel_crtc_state *crtc_state);
 void skl_scaler_disable(const struct intel_crtc_state *old_crtc_state);
 
 void skl_scaler_get_config(struct intel_crtc_state *crtc_state);
+void casf_scaler_enable(struct intel_crtc_state *crtc_state);
 
 #endif
-- 
2.25.1

[RFC 4/5] drm/i915/display/: Add registers and compute the strength

[Nemesa Garg]

Add new registers and related bits. Compute the strength
value and tap value based on display mode.

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |  1 +
 .../drm/i915/display/intel_sharpen_filter.c   | 84 +++++++++++++++++++
 .../drm/i915/display/intel_sharpen_filter.h   |  4 +
 drivers/gpu/drm/i915/i915_reg.h               | 17 ++++
 4 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index d43931127ec2..ff1facc1c2c6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -801,6 +801,7 @@ struct intel_sharpen_filter {
 	bool strength_changed;
 	u8 win_size;
 	bool need_scaler;
+	u8 strength;
 };
 
 struct intel_crtc_scaler_state {
diff --git a/drivers/gpu/drm/i915/display/intel_sharpen_filter.c b/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
index 221dca3bcba5..285e341acc94 100644
--- a/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
+++ b/drivers/gpu/drm/i915/display/intel_sharpen_filter.c
@@ -18,11 +18,83 @@
 #define filter_coeff_1_0 1000
 #define filter_coeff_0_0 0
 
+/*
+ * Default LUT values to be loaded one time.
+ */
+static const u16 lut_data[] = {
+	4095, 2047, 1364, 1022, 816, 678, 579,
+	504, 444, 397, 357, 323, 293, 268, 244, 224,
+	204, 187, 170, 154, 139, 125, 111, 98, 85,
+	73, 60, 48, 36, 24, 12, 0
+};
+
+void intel_filter_lut_load(struct intel_crtc *crtc,
+			   const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int i;
+
+	intel_de_write(dev_priv, SHRPLUT_INDEX(crtc->pipe), INDEX_AUTO_INCR | INDEX_VALUE(0));
+
+	for (i = 0; i < ARRAY_SIZE(lut_data); i++)
+		intel_de_write(dev_priv, SHRPLUT_DATA(crtc->pipe), lut_data[i]);
+}
+
+/*
+ * HW takes a value in form (1.0 + strength) in 4.4 fixed format.
+ * Strength is from 0.0-14.9375 ie from 0-239.
+ * User can give value from 0-255 but is clamped to 239.
+ * Ex. User gives 85 which is 5.3125 and adding 1.0 gives 6.3125.
+ * 6.3125 in 4.4 format is 01100101 which is equal to 101.
+ * Also 85 + 16 = 101.
+ */
+static void intel_filter_strength_compute(struct intel_crtc_state *crtc_state, u8 *val)
+{
+	*val = min(crtc_state->hw.casf_params.strength, 0xEF) + 0x10;
+}
+
+static void intel_filter_size_compute(struct intel_crtc_state *crtc_state)
+{
+	const struct drm_display_mode *mode = &crtc_state->hw.adjusted_mode;
+
+	if (mode->hdisplay <= 1920 && mode->vdisplay <= 1080)
+		crtc_state->hw.casf_params.win_size = 0;
+	else if (mode->hdisplay <= 3840 && mode->vdisplay <= 2160)
+		crtc_state->hw.casf_params.win_size = 1;
+	else
+		crtc_state->hw.casf_params.win_size = 2;
+}
+
+void intel_sharpen_strength_changed(struct intel_atomic_state *state)
+{
+	int i;
+	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+	struct intel_crtc *crtc;
+
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		if (new_crtc_state->uapi.sharpeness_strength !=
+				old_crtc_state->uapi.sharpeness_strength)
+			new_crtc_state->hw.casf_params.strength_changed = true;
+	}
+}
+
 void intel_sharpen_filter_enable(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int id = crtc_state->scaler_state.scaler_id;
+	u32 sharpness_ctl;
+	u8 val;
+
+	intel_filter_strength_compute(crtc_state, &val);
+	drm_dbg(&dev_priv->drm, "Filter strength value: %d\n", val);
+
+	sharpness_ctl =	FILTER_EN | FILTER_STRENGTH(val) |
+		FILTER_SIZE(crtc_state->hw.casf_params.win_size);
+
+	intel_de_write(dev_priv, SHARPNESS_CTL(crtc->pipe),
+		       sharpness_ctl);
 
 	intel_de_write_fw(dev_priv, GLK_PS_COEF_INDEX_SET(crtc->pipe, id, 0),
 			  PS_COEF_INDEX_AUTO_INC);
@@ -42,7 +114,19 @@ void intel_sharpen_filter_enable(struct intel_crtc_state *crtc_state)
 
 int intel_filter_compute_config(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+
+	if (crtc_state->uapi.sharpeness_strength == 0)
+		return -EINVAL;
+
+	crtc_state->hw.casf_params.strength =
+		crtc_state->uapi.sharpeness_strength;
 	crtc_state->hw.casf_params.need_scaler = true;
+	
+	intel_filter_size_compute(crtc_state);
+	drm_dbg(&dev_priv->drm, "Tap Size: %d\n",
+		crtc_state->hw.casf_params.win_size);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_sharpen_filter.h b/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
index 89c0d689469c..3100ef696448 100644
--- a/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
+++ b/drivers/gpu/drm/i915/display/intel_sharpen_filter.h
@@ -24,4 +24,8 @@ struct scaler_filter_coeff {
 void intel_sharpen_filter_enable(struct intel_crtc_state *crtc_state);
 void intel_sharpness_scaler_compute_config(struct intel_crtc_state *crtc_state);
 int intel_filter_compute_config(struct intel_crtc_state *crtc_state);
+void intel_filter_lut_load(struct intel_crtc *crtc,
+			   const struct intel_crtc_state *crtc_state);
+void intel_sharpen_strength_changed(struct intel_atomic_state *state);
+
 #endif /* __INTEL_SHARPEN_FLITER_H__ */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9d759026add4..a49d946c4479 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4211,6 +4211,23 @@
 			_ID(id, _PS_COEF_SET0_DATA_1A, _PS_COEF_SET0_DATA_2A) + (set) * 8, \
 			_ID(id, _PS_COEF_SET0_DATA_1B, _PS_COEF_SET0_DATA_2B) + (set) * 8)
 
+#define _SHARPNESS_CTL_A               0x682B0
+#define SHARPNESS_CTL(trans)           _MMIO_PIPE2(trans, _SHARPNESS_CTL_A)
+#define  FILTER_EN                      REG_BIT(31)
+#define  FILTER_STRENGTH_MASK           REG_GENMASK(15, 8)
+#define  FILTER_STRENGTH(x)             REG_FIELD_PREP(FILTER_STRENGTH_MASK, (x))
+#define  FILTER_SIZE_MASK               REG_GENMASK(1, 0)
+#define  FILTER_SIZE(x)                 REG_FIELD_PREP(FILTER_SIZE_MASK, (x))
+
+#define _SHRPLUT_DATA_A			0x682B8
+#define SHRPLUT_DATA(trans)		_MMIO_PIPE2(trans, _SHRPLUT_DATA_A)
+
+#define _SHRPLUT_INDEX_A		0x682B4
+#define SHRPLUT_INDEX(trans)		_MMIO_PIPE2(trans, _SHRPLUT_INDEX_A)
+#define  INDEX_AUTO_INCR		REG_BIT(10)
+#define  INDEX_VALUE_MASK		REG_GENMASK(4, 0)
+#define  INDEX_VALUE(x)			REG_FIELD_PREP(INDEX_VALUE_MASK, (x))
+
 /* Display Internal Timeout Register */
 #define RM_TIMEOUT		_MMIO(0x42060)
 #define  MMIO_TIMEOUT_US(us)	((us) << 0)
-- 
2.25.1

[RFC 5/5] drm/i915/display: Load the lut values and enable sharpness

[Nemesa Garg]

Load the lut values during pipe enable.

Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c    |  3 +++
 drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++++++-
 drivers/gpu/drm/i915/display/skl_scaler.c    | 11 ++++++++++-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 25593f6aae7d..74c498733283 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -383,6 +383,9 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
 
+	if (DISPLAY_VER(dev_priv) >= 20)
+		drm_crtc_create_sharpening_strength_property(&crtc->base);
+
 	return 0;
 
 fail:
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3d05bd203ca8..e8bd615e6977 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1722,6 +1722,9 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 		intel_crtc_wait_for_next_vblank(wa_crtc);
 		intel_crtc_wait_for_next_vblank(wa_crtc);
 	}
+
+	if (new_crtc_state->hw.casf_params.strength_changed)
+		intel_filter_lut_load(crtc, new_crtc_state);
 }
 
 void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state)
@@ -2444,8 +2447,12 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
 	if (crtc_state->has_pch_encoder)
 		return ilk_fdi_compute_config(crtc, crtc_state);
 
-	if (crtc_state->hw.casf_params.strength_changed)
+	intel_sharpen_strength_changed(state);
+
+	if (crtc_state->hw.casf_params.strength_changed) {
 		intel_sharpness_scaler_compute_config(crtc_state);
+		intel_filter_compute_config(crtc_state);
+	}
 
 	return 0;
 }
@@ -6744,6 +6751,9 @@ static void intel_update_crtc(struct intel_atomic_state *state,
 	if (intel_crtc_needs_fastset(new_crtc_state) &&
 	    old_crtc_state->inherited)
 		intel_crtc_arm_fifo_underrun(crtc, new_crtc_state);
+
+	if (new_crtc_state->hw.casf_params.strength_changed)
+		intel_sharpen_filter_enable(new_crtc_state);
 }
 
 static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c
index be61a6ebd7e3..cb828b3880b2 100644
--- a/drivers/gpu/drm/i915/display/skl_scaler.c
+++ b/drivers/gpu/drm/i915/display/skl_scaler.c
@@ -925,7 +925,7 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
 
 	/* find scaler attached to this pipe */
 	for (i = 0; i < crtc->num_scalers; i++) {
-		u32 ctl, pos, size;
+		u32 ctl, pos, size, sharp;
 
 		ctl = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
 		if ((ctl & (PS_SCALER_EN | PS_BINDING_MASK)) != (PS_SCALER_EN | PS_BINDING_PIPE))
@@ -933,6 +933,15 @@ void skl_scaler_get_config(struct intel_crtc_state *crtc_state)
 
 		id = i;
 
+		sharp = intel_de_read(dev_priv, SHARPNESS_CTL(crtc->pipe));
+		if (sharp & FILTER_EN) {
+			crtc_state->hw.casf_params.strength =
+				REG_FIELD_GET(FILTER_STRENGTH_MASK, sharp) - 16;
+			crtc_state->hw.casf_params.need_scaler = true;
+			crtc_state->hw.casf_params.win_size =
+				REG_FIELD_GET(FILTER_SIZE_MASK, sharp);
+		}
+
 		if (!crtc_state->hw.casf_params.need_scaler)
 			crtc_state->pch_pfit.enabled = true;
 
-- 
2.25.1

✗ Fi.CI.CHECKPATCH: warning for Introduce drm sharpening property

[Patchwork]

== Series Details ==

Series: Introduce drm sharpening property
URL   : https://patchwork.freedesktop.org/series/129888/
State : warning

== Summary ==

Error: dim checkpatch failed
f800fb698aab drm: Introduce sharpeness mode property
3f3bad5af877 drm/i915/display/: Compute the scaler filter coefficients
Traceback (most recent call last):
  File "scripts/spdxcheck.py", line 6, in <module>
    from ply import lex, yacc
ModuleNotFoundError: No module named 'ply'
Traceback (most recent call last):
  File "scripts/spdxcheck.py", line 6, in <module>
    from ply import lex, yacc
ModuleNotFoundError: No module named 'ply'
-:74: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#74: 
new file mode 100644

-:196: WARNING:LONG_LINE: line length of 102 exceeds 100 columns
#196: FILE: drivers/gpu/drm/i915/display/intel_sharpen_filter.c:118:
+				SHARP_COEFF_TO_REG_FORMAT(crtc_state->hw.casf_params.coeff[tapindex]);

-:216: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'coefficient' - possible side-effects?
#216: FILE: drivers/gpu/drm/i915/display/intel_sharpen_filter.h:11:
+#define SHARP_COEFF_TO_REG_FORMAT(coefficient) ((u16)(coefficient.sign << 15 | \
+			coefficient.exp << 12 | coefficient.mantissa << 3))

total: 0 errors, 2 warnings, 1 checks, 206 lines checked
cc434912b492 drm/i915/dispaly/: Enable the second scaler
-:177: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#177: FILE: drivers/gpu/drm/i915/display/skl_scaler.c:740:
+	x = y = 0;

total: 0 errors, 0 warnings, 1 checks, 187 lines checked
7cc66558b5a9 drm/i915/display/: Add registers and compute the strength
-:124: ERROR:TRAILING_WHITESPACE: trailing whitespace
#124: FILE: drivers/gpu/drm/i915/display/intel_sharpen_filter.c:126:
+^I$

total: 1 errors, 0 warnings, 0 checks, 140 lines checked
87344f4fc30c drm/i915/display: Load the lut values and enable sharpness

✗ Fi.CI.SPARSE: warning for Introduce drm sharpening property

[Patchwork]

== Series Details ==

Series: Introduce drm sharpening property
URL   : https://patchwork.freedesktop.org/series/129888/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for Introduce drm sharpening property

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 9854 bytes --]

== Series Details ==

Series: Introduce drm sharpening property
URL   : https://patchwork.freedesktop.org/series/129888/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14269 -> Patchwork_129888v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_129888v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_129888v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/index.html

Participating hosts (37 -> 35)
------------------------------

  Missing    (2): fi-glk-j4005 fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-dg1-7:          [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-dg1-7/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-dg1-7/igt@i915_module_load@load.html
    - bat-adlp-9:         [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-adlp-9/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-adlp-9/igt@i915_module_load@load.html
    - fi-skl-guc:         [PASS][5] -> [ABORT][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/fi-skl-guc/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/fi-skl-guc/igt@i915_module_load@load.html
    - bat-dg2-11:         [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-dg2-11/igt@i915_module_load@load.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-dg2-11/igt@i915_module_load@load.html
    - fi-kbl-7567u:       [PASS][9] -> [INCOMPLETE][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/fi-kbl-7567u/igt@i915_module_load@load.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/fi-kbl-7567u/igt@i915_module_load@load.html
    - fi-cfl-8700k:       [PASS][11] -> [INCOMPLETE][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/fi-cfl-8700k/igt@i915_module_load@load.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/fi-cfl-8700k/igt@i915_module_load@load.html
    - fi-cfl-guc:         [PASS][13] -> [INCOMPLETE][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/fi-cfl-guc/igt@i915_module_load@load.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/fi-cfl-guc/igt@i915_module_load@load.html
    - bat-dg2-9:          [PASS][15] -> [INCOMPLETE][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-dg2-9/igt@i915_module_load@load.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-dg2-9/igt@i915_module_load@load.html
    - fi-cfl-8109u:       [PASS][17] -> [INCOMPLETE][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/fi-cfl-8109u/igt@i915_module_load@load.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/fi-cfl-8109u/igt@i915_module_load@load.html
    - bat-dg2-8:          [PASS][19] -> [INCOMPLETE][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-dg2-8/igt@i915_module_load@load.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-dg2-8/igt@i915_module_load@load.html

  
#### Suppressed ####

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

  * igt@i915_module_load@load:
    - {bat-arls-1}:       [PASS][21] -> [ABORT][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-arls-1/igt@i915_module_load@load.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-arls-1/igt@i915_module_load@load.html
    - {bat-dg2-13}:       [PASS][23] -> [INCOMPLETE][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-dg2-13/igt@i915_module_load@load.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-dg2-13/igt@i915_module_load@load.html
    - {bat-dg2-14}:       [PASS][25] -> [INCOMPLETE][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/bat-dg2-14/igt@i915_module_load@load.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-dg2-14/igt@i915_module_load@load.html

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

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

### CI changes ###

#### Issues hit ####

  * boot:
    - fi-apl-guc:         [PASS][27] -> [FAIL][28] ([i915#8293])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14269/fi-apl-guc/boot.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/fi-apl-guc/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@verify-random:
    - bat-mtlp-6:         NOTRUN -> [SKIP][29] ([i915#4613]) +3 other tests skip
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@gem_lmem_swapping@verify-random.html

  * igt@i915_pm_rps@basic-api:
    - bat-mtlp-6:         NOTRUN -> [SKIP][30] ([i915#6621])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@i915_pm_rps@basic-api.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-mtlp-6:         NOTRUN -> [SKIP][31] ([fdo#109285] / [i915#9792])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-mtlp-6:         NOTRUN -> [SKIP][32] ([i915#5274] / [i915#9792])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-mtlp-6:         NOTRUN -> [SKIP][33] ([i915#4342] / [i915#5354] / [i915#9792])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@hang-read-crc:
    - bat-mtlp-6:         NOTRUN -> [SKIP][34] ([i915#9792]) +6 other tests skip
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@kms_pipe_crc_basic@hang-read-crc.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-mtlp-6:         NOTRUN -> [SKIP][35] ([i915#5354] / [i915#9792])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-mtlp-6:         NOTRUN -> [SKIP][36] ([i915#3555] / [i915#8809] / [i915#9792])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-mtlp-6:         NOTRUN -> [SKIP][37] ([i915#3708] / [i915#9792])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-mtlp-6:         NOTRUN -> [SKIP][38] ([i915#3708] / [i915#4077]) +1 other test skip
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-write:
    - bat-mtlp-6:         NOTRUN -> [SKIP][39] ([i915#3708]) +2 other tests skip
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/bat-mtlp-6/igt@prime_vgem@basic-write.html

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

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#10194]: https://gitlab.freedesktop.org/drm/intel/issues/10194
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4342]: https://gitlab.freedesktop.org/drm/intel/issues/4342
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
  [i915#8809]: https://gitlab.freedesktop.org/drm/intel/issues/8809
  [i915#9673]: https://gitlab.freedesktop.org/drm/intel/issues/9673
  [i915#9732]: https://gitlab.freedesktop.org/drm/intel/issues/9732
  [i915#9792]: https://gitlab.freedesktop.org/drm/intel/issues/9792


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

  * Linux: CI_DRM_14269 -> Patchwork_129888v1

  CI-20190529: 20190529
  CI_DRM_14269: 403e424a8684b55d90b90f293a992e54f37892b3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7711: 7711
  Patchwork_129888v1: 403e424a8684b55d90b90f293a992e54f37892b3 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

cf8aa8d0cb91 drm/i915/display: Load the lut values and enable sharpness
6ac1624999aa drm/i915/display/: Add registers and compute the strength
4c880d61f033 drm/i915/dispaly/: Enable the second scaler
43138116787c drm/i915/display/: Compute the scaler filter coefficients
4f11b0a17895 drm: Introduce sharpeness mode property

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_129888v1/index.html

[-- Attachment #2: Type: text/html, Size: 11282 bytes --]

Re: [RFC 0/5] Introduce drm sharpening property

[Simon Ser]

How much of this is Intel-specific? Are there any hardware vendors with
a similar feature? How much is the "sharpness" knob tied to Intel hardware?

Re: [RFC 0/5] Introduce drm sharpening property

[Harry Wentland]

Adding a couple of compositor devs as they might be interested in this.

On 2024-02-14 06:24, Nemesa Garg wrote:
> 	Many a times images are blurred or upscaled content is also not as
> crisp as original rendered image. Traditional sharpening techniques often
> apply a uniform level of enhancement across entire image, which sometimes
> result in over-sharpening of some areas and potential loss of natural details. 
> 
> Intel has come up with Display Engine based adaptive sharpening filter 
> with minimal power and performance impact. From LNL onwards, the Display
> hardware can use one of the pipe scaler for adaptive sharpness filter.
> This can be used for both gaming and non-gaming use cases like photos,
> image viewing. It works on a region of pixels depending on the tap size.
> 
> This RFC is an attempt to introduce an adaptive sharpness solution which
> helps in improving the image quality. For this new CRTC property is added.

I don't think CRTC is the right place for this. Scaling tends to be more
of a plane thing. Planes can be scaled independently, or is that not the
case for Intel? Or does Intel HW do this sharpening operation independent
of any scaling, on the entire output?

> The user can set this property with desired sharpness strength value with
> 0-255. A value of 1 representing minimum sharpening strength and 255
> representing maximum sharpness strength. A strength value of 0 means no
> sharpening or sharpening feature disabled.
> It works on a region of pixels depending on the tap size. The coefficients
> are used to generate an alpha value which is used to blend the sharpened
> image to original image.
>  
> Userspace implementation for sharpening feature and IGT implementation
> is in progress.

It would be very helpful to have an idea how this looks in userspace, and
which compositors will implement this.

Harry

> 
> Nemesa Garg (5):
>   drm: Introduce sharpeness mode property
>   drm/i915/display/: Compute the scaler filter coefficients
>   drm/i915/dispaly/: Enable the second scaler
>   drm/i915/display/: Add registers and compute the strength
>   drm/i915/display: Load the lut values and enable sharpness
> 
>  drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
>  drivers/gpu/drm/drm_crtc.c                    |  17 ++
>  drivers/gpu/drm/i915/Makefile                 |   1 +
>  drivers/gpu/drm/i915/display/intel_crtc.c     |   3 +
>  drivers/gpu/drm/i915/display/intel_display.c  |  20 +-
>  .../drm/i915/display/intel_display_types.h    |  11 +
>  .../drm/i915/display/intel_modeset_verify.c   |   1 +
>  .../drm/i915/display/intel_sharpen_filter.c   | 214 ++++++++++++++++++
>  .../drm/i915/display/intel_sharpen_filter.h   |  31 +++
>  drivers/gpu/drm/i915/display/skl_scaler.c     |  86 ++++++-
>  drivers/gpu/drm/i915/display/skl_scaler.h     |   1 +
>  drivers/gpu/drm/i915/i915_reg.h               |  19 ++
>  drivers/gpu/drm/xe/Makefile                   |   1 +
>  include/drm/drm_crtc.h                        |  17 ++
>  14 files changed, 416 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.h
> 

Re: [RFC 0/5] Introduce drm sharpening property

[Ville Syrjälä]

On Thu, Feb 15, 2024 at 11:37:54AM -0500, Harry Wentland wrote:
> Adding a couple of compositor devs as they might be interested in this.
> 
> On 2024-02-14 06:24, Nemesa Garg wrote:
> > 	Many a times images are blurred or upscaled content is also not as
> > crisp as original rendered image. Traditional sharpening techniques often
> > apply a uniform level of enhancement across entire image, which sometimes
> > result in over-sharpening of some areas and potential loss of natural details. 
> > 
> > Intel has come up with Display Engine based adaptive sharpening filter 
> > with minimal power and performance impact. From LNL onwards, the Display
> > hardware can use one of the pipe scaler for adaptive sharpness filter.
> > This can be used for both gaming and non-gaming use cases like photos,
> > image viewing. It works on a region of pixels depending on the tap size.
> > 
> > This RFC is an attempt to introduce an adaptive sharpness solution which
> > helps in improving the image quality. For this new CRTC property is added.
> 
> I don't think CRTC is the right place for this. Scaling tends to be more
> of a plane thing. Planes can be scaled independently, or is that not the
> case for Intel? Or does Intel HW do this sharpening operation independent
> of any scaling, on the entire output?

We can scale either individual planes, or the entire crtc.

> 
> > The user can set this property with desired sharpness strength value with
> > 0-255. A value of 1 representing minimum sharpening strength and 255
> > representing maximum sharpness strength. A strength value of 0 means no
> > sharpening or sharpening feature disabled.
> > It works on a region of pixels depending on the tap size. The coefficients
> > are used to generate an alpha value which is used to blend the sharpened
> > image to original image.
> >  
> > Userspace implementation for sharpening feature and IGT implementation
> > is in progress.
> 
> It would be very helpful to have an idea how this looks in userspace, and
> which compositors will implement this.

Someone will probably need to spend some real time thinking
how this interacts with the scaling filter propery (eg. if
we want to extend that property with new values) and
the laptop panel scaling mode property. We also want to
implement the margin properties for external displays
which also involves scaling. Ie. we need some kind of
consistent story how all those things will work together.

> 
> Harry
> 
> > 
> > Nemesa Garg (5):
> >   drm: Introduce sharpeness mode property
> >   drm/i915/display/: Compute the scaler filter coefficients
> >   drm/i915/dispaly/: Enable the second scaler
> >   drm/i915/display/: Add registers and compute the strength
> >   drm/i915/display: Load the lut values and enable sharpness
> > 
> >  drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
> >  drivers/gpu/drm/drm_crtc.c                    |  17 ++
> >  drivers/gpu/drm/i915/Makefile                 |   1 +
> >  drivers/gpu/drm/i915/display/intel_crtc.c     |   3 +
> >  drivers/gpu/drm/i915/display/intel_display.c  |  20 +-
> >  .../drm/i915/display/intel_display_types.h    |  11 +
> >  .../drm/i915/display/intel_modeset_verify.c   |   1 +
> >  .../drm/i915/display/intel_sharpen_filter.c   | 214 ++++++++++++++++++
> >  .../drm/i915/display/intel_sharpen_filter.h   |  31 +++
> >  drivers/gpu/drm/i915/display/skl_scaler.c     |  86 ++++++-
> >  drivers/gpu/drm/i915/display/skl_scaler.h     |   1 +
> >  drivers/gpu/drm/i915/i915_reg.h               |  19 ++
> >  drivers/gpu/drm/xe/Makefile                   |   1 +
> >  include/drm/drm_crtc.h                        |  17 ++
> >  14 files changed, 416 insertions(+), 10 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_sharpen_filter.h
> > 

-- 
Ville Syrjälä
Intel

RE: [RFC 0/5] Introduce drm sharpening property

[Garg, Nemesa]

It is not intel specific and the goal is to have a generic API for configuring Sharpness, accessible to various vendors. Intel currently offers sharpness support through the Display Engine, while other vendors seem to support Sharpness through the GPU using GL shaders (Vulcan/Open GL based).
 
In terms of sharpness intensity adjustment, the plan is to provide users with the ability to customize and regulate sharpness levels. We suggest setting a minimum and maximum strength range from 1 to 255, where a value of 0 signifies that the sharpness feature is disabled, indicated by a u8 data type. 
For now we have mapped the strength value 0.0 to 14.9375 to 0-239 but as the datatype is u8 user can give value upto 255 which is gets clamped to 239.

We are also open to have alternative scales, such as 1-100 or 1-10, as long as a general consensus is reached within the community comprising OEMs and vendors.

> -----Original Message-----
> From: Simon Ser <contact@emersion.fr>
> Sent: Thursday, February 15, 2024 2:03 PM
> To: Garg, Nemesa <nemesa.garg@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: Re: [RFC 0/5] Introduce drm sharpening property
> 
> How much of this is Intel-specific? Are there any hardware vendors with a similar
> feature? How much is the "sharpness" knob tied to Intel hardware?

Re: [RFC 0/5] Introduce drm sharpening property

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]

On Fri, 16 Feb 2024 04:28:41 +0000
"Garg, Nemesa" <nemesa.garg@intel.com> wrote:

> It is not intel specific and the goal is to have a generic API for
> configuring Sharpness, accessible to various vendors. Intel currently
> offers sharpness support through the Display Engine, while other
> vendors seem to support Sharpness through the GPU using GL shaders
> (Vulcan/Open GL based). 

Do you mean that all vendors use the exact same mathematical algorithm
(with only differences in operation precision at most)?

If yes, good.

If not, then we need to know where exactly in the virtual KMS color
pipeline the operation happens, whether this can be generic or not.

Does this also work the same regardless of pixel formats, dynamic
range, color gamut, transfer functions etc. on both plane input and
connector output configurations?

> In terms of sharpness intensity adjustment, the plan is to provide
> users with the ability to customize and regulate sharpness levels. We
> suggest setting a minimum and maximum strength range from 1 to 255,
> where a value of 0 signifies that the sharpness feature is disabled,
> indicated by a u8 data type. For now we have mapped the strength
> value 0.0 to 14.9375 to 0-239 but as the datatype is u8 user can give
> value upto 255 which is gets clamped to 239.

Naturally you will need to document all that, so that all drivers and
vendors do the exact same thing.

I did not see any actual documentation in the patch series yet, e.g. a
reference to a specific algorithm.

As Ville pointed out, there was also no specification at which point of
the virtual color pipeline this operation will apply. Before/after
DEGAMMA/CTM/GAMMA/scaling in plane/blending/CRTC?

Is the property being added to the list in
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-crtc-properties
or where-ever it belongs?


Thanks,
pq

> We are also open to have alternative scales, such as 1-100 or 1-10,
> as long as a general consensus is reached within the community
> comprising OEMs and vendors.
> 
> > -----Original Message-----
> > From: Simon Ser <contact@emersion.fr>
> > Sent: Thursday, February 15, 2024 2:03 PM
> > To: Garg, Nemesa <nemesa.garg@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> > Subject: Re: [RFC 0/5] Introduce drm sharpening property
> > 
> > How much of this is Intel-specific? Are there any hardware vendors
> > with a similar feature? How much is the "sharpness" knob tied to
> > Intel hardware?  


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [RFC 0/5] Introduce drm sharpening property

[Garg, Nemesa]

This is generic as sharpness effect is applied post blending. Depending on the color gamut, pixel format and other inputs the image gets blended and once we get blended output it can be sharpened based on strength value provided by the user.

On intel platform it is implemented through pipe scaler. Pipe scaler can be used as either scaler or sharpness filter.  As mentioned earlier the client can provide any strength value  between 0-255 or any other scale based on discussions.  Perhaps userspace can have provide options like low-med-high sharpness or in percentage form or steps which is mapped to 0-255.

I will add the documentation regarding property.

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Pekka
> Paalanen
> Sent: Friday, February 16, 2024 2:06 PM
> To: Garg, Nemesa <nemesa.garg@intel.com>
> Cc: Simon Ser <contact@emersion.fr>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org; G M, Adarsh <adarsh.g.m@intel.com>
> Subject: Re: [RFC 0/5] Introduce drm sharpening property
> 
> On Fri, 16 Feb 2024 04:28:41 +0000
> "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
> 
> > It is not intel specific and the goal is to have a generic API for
> > configuring Sharpness, accessible to various vendors. Intel currently
> > offers sharpness support through the Display Engine, while other
> > vendors seem to support Sharpness through the GPU using GL shaders
> > (Vulcan/Open GL based).
> 
> Do you mean that all vendors use the exact same mathematical algorithm (with
> only differences in operation precision at most)?
> 
> If yes, good.
> 
> If not, then we need to know where exactly in the virtual KMS color pipeline the
> operation happens, whether this can be generic or not.
> 
> Does this also work the same regardless of pixel formats, dynamic range, color
> gamut, transfer functions etc. on both plane input and connector output
> configurations?
> 
> > In terms of sharpness intensity adjustment, the plan is to provide
> > users with the ability to customize and regulate sharpness levels. We
> > suggest setting a minimum and maximum strength range from 1 to 255,
> > where a value of 0 signifies that the sharpness feature is disabled,
> > indicated by a u8 data type. For now we have mapped the strength value
> > 0.0 to 14.9375 to 0-239 but as the datatype is u8 user can give value
> > upto 255 which is gets clamped to 239.
> 
> Naturally you will need to document all that, so that all drivers and vendors do the
> exact same thing.
> 
> I did not see any actual documentation in the patch series yet, e.g. a reference to
> a specific algorithm.
> 
> As Ville pointed out, there was also no specification at which point of the virtual
> color pipeline this operation will apply. Before/after
> DEGAMMA/CTM/GAMMA/scaling in plane/blending/CRTC?
> 
> Is the property being added to the list in
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-crtc-
> properties
> or where-ever it belongs?
> 
> 
> Thanks,
> pq
> 
> > We are also open to have alternative scales, such as 1-100 or 1-10, as
> > long as a general consensus is reached within the community comprising
> > OEMs and vendors.
> >
> > > -----Original Message-----
> > > From: Simon Ser <contact@emersion.fr>
> > > Sent: Thursday, February 15, 2024 2:03 PM
> > > To: Garg, Nemesa <nemesa.garg@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> > > Subject: Re: [RFC 0/5] Introduce drm sharpening property
> > >
> > > How much of this is Intel-specific? Are there any hardware vendors
> > > with a similar feature? How much is the "sharpness" knob tied to
> > > Intel hardware?

RE: [RFC 0/5] Introduce drm sharpening property

[Simon Ser]

On Monday, March 4th, 2024 at 15:04, Garg, Nemesa <nemesa.garg@intel.com> wrote:

> This is generic as sharpness effect is applied post blending. Depending
> on the color gamut, pixel format and other inputs the image gets
> blended and once we get blended output it can be sharpened based on
> strength value provided by the user.

It would really help if you could provide the exact mathematical formula
applied by this KMS property.

RE: [RFC 0/5] Introduce drm sharpening property

[Garg, Nemesa]

This  KMS property is not implementing any formula and the values that are being used are based on empirical analysis and certain experiments done on the hardware. These values are fixed and is not expected to change and this can change from vendor to vendor. 
The client can choose any sharpness value on the scale and on the basis of it the sharpness will be set. The sharpness effect can be changed from content to content and from display to display so user needs to adjust the optimum intensity value so as to get good experience on the screen.

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Simon
> Ser
> Sent: Monday, March 4, 2024 7:46 PM
> To: Garg, Nemesa <nemesa.garg@intel.com>
> Cc: Pekka Paalanen <pekka.paalanen@haloniitty.fi>; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; G M, Adarsh
> <adarsh.g.m@intel.com>
> Subject: RE: [RFC 0/5] Introduce drm sharpening property
> 
> On Monday, March 4th, 2024 at 15:04, Garg, Nemesa <nemesa.garg@intel.com>
> wrote:
> 
> > This is generic as sharpness effect is applied post blending.
> > Depending on the color gamut, pixel format and other inputs the image
> > gets blended and once we get blended output it can be sharpened based
> > on strength value provided by the user.
> 
> It would really help if you could provide the exact mathematical formula applied
> by this KMS property.

Re: [RFC 0/5] Introduce drm sharpening property

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 3088 bytes --]

On Tue, 12 Mar 2024 08:30:34 +0000
"Garg, Nemesa" <nemesa.garg@intel.com> wrote:

> This  KMS property is not implementing any formula

Sure it is. Maybe Intel just does not want to tell what the algorithm
is, or maybe it's even patented.

> and the values
> that are being used are based on empirical analysis and certain
> experiments done on the hardware. These values are fixed and is not
> expected to change and this can change from vendor to vendor. The
> client can choose any sharpness value on the scale and on the basis
> of it the sharpness will be set. The sharpness effect can be changed
> from content to content and from display to display so user needs to
> adjust the optimum intensity value so as to get good experience on
> the screen.
> 

IOW, it's an opaque box operation, and there is no way to reproduce its
results without the specific Intel hardware. Definitely no way to
reproduce its results in free open source software alone.

Such opaque box operations can only occur after KMS blending, at the
CRTC or later stage. They cannot appear before blending, not in the new
KMS color pipeline design at least. The reason is that the modern way
to use KMS planes is opportunistic composition off-loading.
Opportunistic means that userspace decides from time to time whether it
composes the final picture using KMS or some other rendering method
(usually GPU and shaders). Since userspace will arbitrarily switch
between KMS and render composition, both must result in the exact same
image, or end users will observe unwanted flicker.

Such opaque box operations are fine after blending, because there they
can be configured once and remain on forever. No switching, no flicker.

Where does "sharpeness" operation occur in the Intel color processing
chain? Is it before or after blending?

What kind of transfer characteristics does it expect from the image,
and can those be realized with KMS CRTC properties if KMS is configured
such that the blending happens using some other characteristics (e.g.
blending in optical space)?

What about SDR vs. HDR imagery?


Thanks,
pq

> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Simon
> > Ser
> > Sent: Monday, March 4, 2024 7:46 PM
> > To: Garg, Nemesa <nemesa.garg@intel.com>
> > Cc: Pekka Paalanen <pekka.paalanen@haloniitty.fi>; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; G M, Adarsh
> > <adarsh.g.m@intel.com>
> > Subject: RE: [RFC 0/5] Introduce drm sharpening property
> > 
> > On Monday, March 4th, 2024 at 15:04, Garg, Nemesa <nemesa.garg@intel.com>
> > wrote:
> >   
> > > This is generic as sharpness effect is applied post blending.
> > > Depending on the color gamut, pixel format and other inputs the image
> > > gets blended and once we get blended output it can be sharpened based
> > > on strength value provided by the user.  
> > 
> > It would really help if you could provide the exact mathematical formula applied
> > by this KMS property.  


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC 0/5] Introduce drm sharpening property

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 4070 bytes --]

On Tue, 12 Mar 2024 16:26:00 +0200
Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:

> On Tue, 12 Mar 2024 08:30:34 +0000
> "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
> 
> > This  KMS property is not implementing any formula  
> 
> Sure it is. Maybe Intel just does not want to tell what the algorithm
> is, or maybe it's even patented.
> 
> > and the values
> > that are being used are based on empirical analysis and certain
> > experiments done on the hardware. These values are fixed and is not
> > expected to change and this can change from vendor to vendor. The
> > client can choose any sharpness value on the scale and on the basis
> > of it the sharpness will be set. The sharpness effect can be changed
> > from content to content and from display to display so user needs to
> > adjust the optimum intensity value so as to get good experience on
> > the screen.
> >   
> 
> IOW, it's an opaque box operation, and there is no way to reproduce its
> results without the specific Intel hardware. Definitely no way to
> reproduce its results in free open source software alone.
> 
> Such opaque box operations can only occur after KMS blending, at the
> CRTC or later stage. They cannot appear before blending, not in the new
> KMS color pipeline design at least. The reason is that the modern way
> to use KMS planes is opportunistic composition off-loading.
> Opportunistic means that userspace decides from time to time whether it
> composes the final picture using KMS or some other rendering method
> (usually GPU and shaders). Since userspace will arbitrarily switch
> between KMS and render composition, both must result in the exact same
> image, or end users will observe unwanted flicker.
> 
> Such opaque box operations are fine after blending, because there they
> can be configured once and remain on forever. No switching, no flicker.

If you want to see how sharpness property would apply in Wayland
design, it would be in step 5 "Adjust (settings UI)" of
https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/color-management-model.md#compositor-color-management-model

To relate that diagram to KMS color processing, you can identify step 3
"Compose" as the KMS blending step. Everything before step 3 happens in
KMS plane color processing, and steps 4-5 happen in KMS CRTC color
processing.

Sharpening would essentially be a "compositor color effect", it just
happens to be implementable only by specific Intel hardware.

If a color effect is dynamic or content-dependant, it will preclude
colorimetric monitor calibration.


Thanks,
pq


> Where does "sharpeness" operation occur in the Intel color processing
> chain? Is it before or after blending?
> 
> What kind of transfer characteristics does it expect from the image,
> and can those be realized with KMS CRTC properties if KMS is configured
> such that the blending happens using some other characteristics (e.g.
> blending in optical space)?
> 
> What about SDR vs. HDR imagery?
> 
> 
> Thanks,
> pq
> 
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Simon
> > > Ser
> > > Sent: Monday, March 4, 2024 7:46 PM
> > > To: Garg, Nemesa <nemesa.garg@intel.com>
> > > Cc: Pekka Paalanen <pekka.paalanen@haloniitty.fi>; intel-
> > > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; G M, Adarsh
> > > <adarsh.g.m@intel.com>
> > > Subject: RE: [RFC 0/5] Introduce drm sharpening property
> > > 
> > > On Monday, March 4th, 2024 at 15:04, Garg, Nemesa <nemesa.garg@intel.com>
> > > wrote:
> > >     
> > > > This is generic as sharpness effect is applied post blending.
> > > > Depending on the color gamut, pixel format and other inputs the image
> > > > gets blended and once we get blended output it can be sharpened based
> > > > on strength value provided by the user.    
> > > 
> > > It would really help if you could provide the exact mathematical formula applied
> > > by this KMS property.    
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [RFC 0/5] Introduce drm sharpening property

[Garg, Nemesa]

> -----Original Message-----
> From: Pekka Paalanen <pekka.paalanen@haloniitty.fi>
> Sent: Wednesday, March 13, 2024 3:07 PM
> To: Garg, Nemesa <nemesa.garg@intel.com>
> Cc: Simon Ser <contact@emersion.fr>; intel-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org; G M, Adarsh <adarsh.g.m@intel.com>
> Subject: Re: [RFC 0/5] Introduce drm sharpening property
> 
> On Tue, 12 Mar 2024 16:26:00 +0200
> Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:
> 
> > On Tue, 12 Mar 2024 08:30:34 +0000
> > "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
> >
> > > This  KMS property is not implementing any formula
> >
> > Sure it is. Maybe Intel just does not want to tell what the algorithm
> > is, or maybe it's even patented.
> >
> > > and the values
> > > that are being used are based on empirical analysis and certain
> > > experiments done on the hardware. These values are fixed and is not
> > > expected to change and this can change from vendor to vendor. The
> > > client can choose any sharpness value on the scale and on the basis
> > > of it the sharpness will be set. The sharpness effect can be changed
> > > from content to content and from display to display so user needs to
> > > adjust the optimum intensity value so as to get good experience on
> > > the screen.
> > >
> >
> > IOW, it's an opaque box operation, and there is no way to reproduce
> > its results without the specific Intel hardware. Definitely no way to
> > reproduce its results in free open source software alone.
> >
> > Such opaque box operations can only occur after KMS blending, at the
> > CRTC or later stage. They cannot appear before blending, not in the
> > new KMS color pipeline design at least. The reason is that the modern
> > way to use KMS planes is opportunistic composition off-loading.
> > Opportunistic means that userspace decides from time to time whether
> > it composes the final picture using KMS or some other rendering method
> > (usually GPU and shaders). Since userspace will arbitrarily switch
> > between KMS and render composition, both must result in the exact same
> > image, or end users will observe unwanted flicker.
> >
> > Such opaque box operations are fine after blending, because there they
> > can be configured once and remain on forever. No switching, no flicker.
> 
> If you want to see how sharpness property would apply in Wayland design, it
> would be in step 5 "Adjust (settings UI)" of
> https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/color-
> management-model.md#compositor-color-management-model
> 
> To relate that diagram to KMS color processing, you can identify step 3 "Compose"
> as the KMS blending step. Everything before step 3 happens in KMS plane color
> processing, and steps 4-5 happen in KMS CRTC color processing.
> 
> Sharpening would essentially be a "compositor color effect", it just happens to be
> implementable only by specific Intel hardware.
> 
> If a color effect is dynamic or content-dependant, it will preclude colorimetric
> monitor calibration.
> 
> 
> Thanks,
> pq
> 
> 
> > Where does "sharpeness" operation occur in the Intel color processing
> > chain? Is it before or after blending?
> > 
Thank you for detail explanation and link.
Sharpness operation occur post blending in CRTC ie on the final 
composed output after blending . Yes Pekka you are right as per the 
diagram it is done at step 5  "Adjust (settings UI)").  I  will also document this thing 
along with documentation change.

> > What kind of transfer characteristics does it expect from the image,
> > and can those be realized with KMS CRTC properties if KMS is
> > configured such that the blending happens using some other characteristics
> (e.g.
> > blending in optical space)?
> >
The filter values are not dependent/calculated on the inputs of 
 image but depending on the blending space and other inputs the 
blended output gets changed and the sharpness is applied post 
blending so according to the content user needs to adjust the 
strength value to get the better visual effect. So tuning of sharpness strength 
may be needed by user based on  the input contents and blending policy
to get the desired experience.

> > What about SDR vs. HDR imagery?
> >
The interface can be used for both HDR and SDR. The effect is more prominent for SDR use cases.
For HDR filter values and tap value may change.

Thanks,
Nemesa
> >
> > Thanks,
> > pq
> >
> > > > -----Original Message-----
> > > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On
> > > > Behalf Of Simon Ser
> > > > Sent: Monday, March 4, 2024 7:46 PM
> > > > To: Garg, Nemesa <nemesa.garg@intel.com>
> > > > Cc: Pekka Paalanen <pekka.paalanen@haloniitty.fi>; intel-
> > > > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; G M,
> > > > Adarsh <adarsh.g.m@intel.com>
> > > > Subject: RE: [RFC 0/5] Introduce drm sharpening property
> > > >
> > > > On Monday, March 4th, 2024 at 15:04, Garg, Nemesa
> > > > <nemesa.garg@intel.com>
> > > > wrote:
> > > >
> > > > > This is generic as sharpness effect is applied post blending.
> > > > > Depending on the color gamut, pixel format and other inputs the
> > > > > image gets blended and once we get blended output it can be sharpened
> based
> > > > > on strength value provided by the user.
> > > >
> > > > It would really help if you could provide the exact mathematical formula
> applied
> > > > by this KMS property.
> >

Re: [RFC 0/5] Introduce drm sharpening property

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 5205 bytes --]

On Wed, 27 Mar 2024 07:11:48 +0000
"Garg, Nemesa" <nemesa.garg@intel.com> wrote:

> > -----Original Message-----
> > From: Pekka Paalanen <pekka.paalanen@haloniitty.fi>
> > Sent: Wednesday, March 13, 2024 3:07 PM
> > To: Garg, Nemesa <nemesa.garg@intel.com>
> > Cc: Simon Ser <contact@emersion.fr>; intel-gfx@lists.freedesktop.org; dri-
> > devel@lists.freedesktop.org; G M, Adarsh <adarsh.g.m@intel.com>
> > Subject: Re: [RFC 0/5] Introduce drm sharpening property
> > 
> > On Tue, 12 Mar 2024 16:26:00 +0200
> > Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:
> >   
> > > On Tue, 12 Mar 2024 08:30:34 +0000
> > > "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
> > >  
> > > > This  KMS property is not implementing any formula  
> > >
> > > Sure it is. Maybe Intel just does not want to tell what the algorithm
> > > is, or maybe it's even patented.
> > >  
> > > > and the values
> > > > that are being used are based on empirical analysis and certain
> > > > experiments done on the hardware. These values are fixed and is not
> > > > expected to change and this can change from vendor to vendor. The
> > > > client can choose any sharpness value on the scale and on the basis
> > > > of it the sharpness will be set. The sharpness effect can be changed
> > > > from content to content and from display to display so user needs to
> > > > adjust the optimum intensity value so as to get good experience on
> > > > the screen.
> > > >  
> > >
> > > IOW, it's an opaque box operation, and there is no way to reproduce
> > > its results without the specific Intel hardware. Definitely no way to
> > > reproduce its results in free open source software alone.
> > >
> > > Such opaque box operations can only occur after KMS blending, at the
> > > CRTC or later stage. They cannot appear before blending, not in the
> > > new KMS color pipeline design at least. The reason is that the modern
> > > way to use KMS planes is opportunistic composition off-loading.
> > > Opportunistic means that userspace decides from time to time whether
> > > it composes the final picture using KMS or some other rendering method
> > > (usually GPU and shaders). Since userspace will arbitrarily switch
> > > between KMS and render composition, both must result in the exact same
> > > image, or end users will observe unwanted flicker.
> > >
> > > Such opaque box operations are fine after blending, because there they
> > > can be configured once and remain on forever. No switching, no flicker.  
> > 
> > If you want to see how sharpness property would apply in Wayland design, it
> > would be in step 5 "Adjust (settings UI)" of
> > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/color-
> > management-model.md#compositor-color-management-model
> > 
> > To relate that diagram to KMS color processing, you can identify step 3 "Compose"
> > as the KMS blending step. Everything before step 3 happens in KMS plane color
> > processing, and steps 4-5 happen in KMS CRTC color processing.
> > 
> > Sharpening would essentially be a "compositor color effect", it just happens to be
> > implementable only by specific Intel hardware.
> > 
> > If a color effect is dynamic or content-dependant, it will preclude colorimetric
> > monitor calibration.
> > 
> > 
> > Thanks,
> > pq
> > 
> >   
> > > Where does "sharpeness" operation occur in the Intel color processing
> > > chain? Is it before or after blending?
> > >   
> Thank you for detail explanation and link.
> Sharpness operation occur post blending in CRTC ie on the final 
> composed output after blending . Yes Pekka you are right as per the 
> diagram it is done at step 5  "Adjust (settings UI)").  I  will also document this thing 
> along with documentation change.
> 
> > > What kind of transfer characteristics does it expect from the image,
> > > and can those be realized with KMS CRTC properties if KMS is
> > > configured such that the blending happens using some other characteristics  
> > (e.g.  
> > > blending in optical space)?
> > >  
> The filter values are not dependent/calculated on the inputs of 
>  image but depending on the blending space and other inputs the 
> blended output gets changed and the sharpness is applied post 
> blending so according to the content user needs to adjust the 
> strength value to get the better visual effect. So tuning of sharpness strength 
> may be needed by user based on  the input contents and blending policy
> to get the desired experience.
> 
> > > What about SDR vs. HDR imagery?
> > >  
> The interface can be used for both HDR and SDR. The effect is more prominent for SDR use cases.
> For HDR filter values and tap value may change.

Who will be providing these values?

The kernel driver cannot know if it is dealing with SDR or HDR or which
transfer function is in effect at that point of the post-blending color
pipeline.

If the UAPI is one "strength" value, then how can it work?

Maybe the UAPI needs more controls, if not providing all "filter and
tap" values directly. Maybe all the filter and tap values should be
provided by userspace?


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC 0/5] Introduce drm sharpening property

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 6521 bytes --]

On Wed, 27 Mar 2024 13:29:16 +0200
Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:

> On Wed, 27 Mar 2024 07:11:48 +0000
> "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Pekka Paalanen <pekka.paalanen@haloniitty.fi>
> > > Sent: Wednesday, March 13, 2024 3:07 PM
> > > To: Garg, Nemesa <nemesa.garg@intel.com>
> > > Cc: Simon Ser <contact@emersion.fr>; intel-gfx@lists.freedesktop.org; dri-
> > > devel@lists.freedesktop.org; G M, Adarsh <adarsh.g.m@intel.com>
> > > Subject: Re: [RFC 0/5] Introduce drm sharpening property
> > > 
> > > On Tue, 12 Mar 2024 16:26:00 +0200
> > > Pekka Paalanen <pekka.paalanen@haloniitty.fi> wrote:
> > >     
> > > > On Tue, 12 Mar 2024 08:30:34 +0000
> > > > "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
> > > >    
> > > > > This  KMS property is not implementing any formula    
> > > >
> > > > Sure it is. Maybe Intel just does not want to tell what the algorithm
> > > > is, or maybe it's even patented.
> > > >    
> > > > > and the values
> > > > > that are being used are based on empirical analysis and certain
> > > > > experiments done on the hardware. These values are fixed and is not
> > > > > expected to change and this can change from vendor to vendor. The
> > > > > client can choose any sharpness value on the scale and on the basis
> > > > > of it the sharpness will be set. The sharpness effect can be changed
> > > > > from content to content and from display to display so user needs to
> > > > > adjust the optimum intensity value so as to get good experience on
> > > > > the screen.
> > > > >    
> > > >
> > > > IOW, it's an opaque box operation, and there is no way to reproduce
> > > > its results without the specific Intel hardware. Definitely no way to
> > > > reproduce its results in free open source software alone.
> > > >
> > > > Such opaque box operations can only occur after KMS blending, at the
> > > > CRTC or later stage. They cannot appear before blending, not in the
> > > > new KMS color pipeline design at least. The reason is that the modern
> > > > way to use KMS planes is opportunistic composition off-loading.
> > > > Opportunistic means that userspace decides from time to time whether
> > > > it composes the final picture using KMS or some other rendering method
> > > > (usually GPU and shaders). Since userspace will arbitrarily switch
> > > > between KMS and render composition, both must result in the exact same
> > > > image, or end users will observe unwanted flicker.
> > > >
> > > > Such opaque box operations are fine after blending, because there they
> > > > can be configured once and remain on forever. No switching, no flicker.    
> > > 
> > > If you want to see how sharpness property would apply in Wayland design, it
> > > would be in step 5 "Adjust (settings UI)" of
> > > https://gitlab.freedesktop.org/pq/color-and-hdr/-/blob/main/doc/color-
> > > management-model.md#compositor-color-management-model
> > > 
> > > To relate that diagram to KMS color processing, you can identify step 3 "Compose"
> > > as the KMS blending step. Everything before step 3 happens in KMS plane color
> > > processing, and steps 4-5 happen in KMS CRTC color processing.
> > > 
> > > Sharpening would essentially be a "compositor color effect", it just happens to be
> > > implementable only by specific Intel hardware.
> > > 
> > > If a color effect is dynamic or content-dependant, it will preclude colorimetric
> > > monitor calibration.
> > > 
> > > 
> > > Thanks,
> > > pq
> > > 
> > >     
> > > > Where does "sharpeness" operation occur in the Intel color processing
> > > > chain? Is it before or after blending?
> > > >     
> > Thank you for detail explanation and link.
> > Sharpness operation occur post blending in CRTC ie on the final 
> > composed output after blending . Yes Pekka you are right as per the 
> > diagram it is done at step 5  "Adjust (settings UI)").  I  will also document this thing 
> > along with documentation change.
> >   
> > > > What kind of transfer characteristics does it expect from the image,
> > > > and can those be realized with KMS CRTC properties if KMS is
> > > > configured such that the blending happens using some other characteristics    
> > > (e.g.    
> > > > blending in optical space)?
> > > >    
> > The filter values are not dependent/calculated on the inputs of 
> >  image but depending on the blending space and other inputs the 
> > blended output gets changed and the sharpness is applied post 
> > blending so according to the content user needs to adjust the 
> > strength value to get the better visual effect. So tuning of sharpness strength 
> > may be needed by user based on  the input contents and blending policy
> > to get the desired experience.
> >   
> > > > What about SDR vs. HDR imagery?
> > > >    
> > The interface can be used for both HDR and SDR. The effect is more prominent for SDR use cases.
> > For HDR filter values and tap value may change.  
> 
> Who will be providing these values?
> 
> The kernel driver cannot know if it is dealing with SDR or HDR or which
> transfer function is in effect at that point of the post-blending color
> pipeline.
> 
> If the UAPI is one "strength" value, then how can it work?
> 
> Maybe the UAPI needs more controls, if not providing all "filter and
> tap" values directly. Maybe all the filter and tap values should be
> provided by userspace?

Actually, is the hardware just doing a convolution with a filter
defined by the driver?

Convolution algorithm (it is a formula!) is pretty standard stuff I
believe. If the hardware is actually doing convolution, then the driver
really should be exposing the convolution operation. Then people can
choose to use it for sharpening with the Intel developed kernels, or
for custom effects with custom kernels. Everyone would win. Convolution
is also something that other hardware vendors could implement.

A convolution filter would fit very well in the new KMS color pipeline
design for post-compositing operations, too.

Is the sharpening element doing something similar to the unsharp
masking?

I suppose users might want different strength based on what kind of
content is the majority on the screen. That makes it something that a
Wayland compositor would adjust automatically based on Wayland content
type (similar to HDMI content type), for example.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]