[PATCH 0/3] Add scaling filters in DRM layer

[PATCH 0/3] Add scaling filters in DRM layer

[Shashank Sharma]

Blurry outputs during upscaling the video buffer, is a generic problem
of graphics industry. One of the major reason behind this blurriness is
the interpolation of pixel values used by most of the upscaling hardwares.

Nearest-neighbor is a scaling mode, which works by filling in the missing
color values in the upscaled image with that of the coordinate-mapped
nearest source pixel value.

Nearest-neighbor can produce (almost) non-blurry scaling outputs when
the scaling ratio is complete integer. For example:
- input buffer resolution: 1280x720(HD)
- output buffer resolution: 3840x2160(UHD/4K)
- scaling ratio (h) = 3840/1280 = 3  
- scaling ratio (v) = 2160/720 = 3

In such scenarios, we should try to pick Nearest-neighbor as scaling
method when possible.

Many gaming communities have been asking for integer-mode scaling
support, some links and background:
https://software.intel.com/en-us/articles/integer-scaling-support-on-intel-graphics
http://tanalin.com/en/articles/lossless-scaling/
https://community.amd.com/thread/209107
https://www.nvidia.com/en-us/geforce/forums/game-ready-drivers/13/1002/feature-request-nonblurry-upscaling-at-integer-rat/

This patch series adds support for programmable scaling filters, which
can help a user to pick and enables NN scaling on Intel display hardware
(ICL onwards). There is also one option to pick NN only when the scaling
ratios are integer, to achieve Integer scaling, without(alsmost) any side effets.

there was an RFC series published for the same, which can be seen here: 
https://patchwork.freedesktop.org/series/66175/ 

Shashank Sharma (3):
  drm: Introduce scaling filter mode property
  drm/i915: Add support for scaling filters
  drm/i915: Handle nearest-neighbor scaling filter

 drivers/gpu/drm/drm_atomic_uapi.c             |   4 +
 drivers/gpu/drm/i915/display/intel_display.c  | 161 +++++++++++++++++-
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/i915_reg.h               |  31 ++++
 include/drm/drm_crtc.h                        |  26 +++
 include/drm/drm_mode_config.h                 |   6 +
 6 files changed, 230 insertions(+), 1 deletion(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm: Introduce scaling filter mode property

[Shashank Sharma]

This patch adds a scaling filter mode porperty
to allow:
- A driver/HW to showcase it's scaling filter capabilities.
- A userspace to pick a desired effect while scaling.

This option will be particularly useful in the scenarios where
Integer mode scaling is possible, and a UI client wants to pick
filters like Nearest-neighbor applied for non-blurry outputs.

There was a RFC patch series published, to discus the request to enable
Integer mode scaling by some of the gaming communities, which can be
found here:
https://patchwork.freedesktop.org/series/66175/

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
 include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
 include/drm/drm_mode_config.h     |  6 ++++++
 3 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 0d466d3b0809..883329453a86 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		return ret;
 	} else if (property == config->prop_vrr_enabled) {
 		state->vrr_enabled = val;
+	} else if (property == config->scaling_filter_property) {
+		state->scaling_filter = val;
 	} else if (property == config->degamma_lut_property) {
 		ret = drm_atomic_replace_property_blob_from_id(dev,
 					&state->degamma_lut,
@@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (property == config->prop_out_fence_ptr)
 		*val = 0;
+	else if (property == config->scaling_filter_property)
+		*val = state->scaling_filter;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5e9b15a0e8c5..94c5509474a8 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -58,6 +58,25 @@ struct device_node;
 struct dma_fence;
 struct edid;
 
+enum drm_scaling_filters {
+	DRM_SCALING_FILTER_DEFAULT,
+	DRM_SCALING_FILTER_MEDIUM,
+	DRM_SCALING_FILTER_BILINEAR,
+	DRM_SCALING_FILTER_NN,
+	DRM_SCALING_FILTER_NN_IS_ONLY,
+	DRM_SCALING_FILTER_EDGE_ENHANCE,
+	DRM_SCALING_FILTER_INVALID,
+};
+
+static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
+	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
+	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
+	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
+	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
+	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
+	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
+};
+
 static inline int64_t U642I64(uint64_t val)
 {
 	return (int64_t)*((int64_t *)&val);
@@ -283,6 +302,13 @@ struct drm_crtc_state {
 	 */
 	u32 target_vblank;
 
+	/**
+	 * @scaling_filter:
+	 *
+	 * Scaling filter mode to be applied
+	 */
+	u32 scaling_filter;
+
 	/**
 	 * @async_flip:
 	 *
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 3bcbe30339f0..efd6fd55770f 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -914,6 +914,12 @@ struct drm_mode_config {
 	 */
 	struct drm_property *modifiers_property;
 
+	/**
+	 * @scaling_filter_property: CRTC property to apply a particular filter
+	 * while scaling in panel fitter mode.
+	 */
+	struct drm_property *scaling_filter_property;
+
 	/* cursor size */
 	uint32_t cursor_width, cursor_height;
 
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/i915: Add support for scaling filters

[Shashank Sharma]

This patch does the following:
- Creates the CRTC property for scaling filter mode (for GEN11 and +).
- Applies the chosen filter value while enabling the panel fitter.
- Adds CRTC state readouts and comparisons.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 59 +++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1a533ccdb54f..21993f9fd2ae 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5615,11 +5615,35 @@ static void skylake_scaler_disable(struct intel_crtc *crtc)
 		skl_detach_scaler(crtc, i);
 }
 
+static u32
+icelake_get_scaler_filter(const struct intel_crtc_state *crtc_state)
+{
+	const struct drm_crtc_state *state = &crtc_state->base;
+
+	switch (state->scaling_filter) {
+	case DRM_SCALING_FILTER_BILINEAR:
+		return PS_FILTER_BILINEAR;
+	case DRM_SCALING_FILTER_EDGE_ENHANCE:
+		return PS_FILTER_EDGE_ENHANCE;
+	case DRM_SCALING_FILTER_NN:
+	case DRM_SCALING_FILTER_NN_IS_ONLY:
+		return PS_FILTER_PROGRAMMED;
+	case DRM_SCALING_FILTER_INVALID:
+		DRM_ERROR("Ignoring invalid scaler filter mode\n");
+		return PS_FILTER_MEDIUM;
+	case DRM_SCALING_FILTER_DEFAULT:
+	case DRM_SCALING_FILTER_MEDIUM:
+	default:
+		return PS_FILTER_MEDIUM;
+	}
+}
+
 static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
+	u32 scaler_filter;
 	const struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
 
@@ -5640,9 +5664,11 @@ static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
 		uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
 		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
 
+		scaler_filter = icelake_get_scaler_filter(crtc_state);
+
 		id = scaler_state->scaler_id;
 		I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
-			PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
+			scaler_filter | scaler_state->scalers[id].mode);
 		I915_WRITE_FW(SKL_PS_VPHASE(pipe, id),
 			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
 		I915_WRITE_FW(SKL_PS_HPHASE(pipe, id),
@@ -12192,6 +12218,10 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
 			      pipe_config->scaler_state.scaler_users,
 		              pipe_config->scaler_state.scaler_id);
 
+	if (INTEL_GEN(dev_priv) >= 11)
+		DRM_DEBUG_KMS("scaling_filter: %d\n",
+			      pipe_config->base.scaling_filter);
+
 	if (HAS_GMCH(dev_priv))
 		DRM_DEBUG_KMS("gmch pfit: control: 0x%08x, ratios: 0x%08x, lvds border: 0x%08x\n",
 			      pipe_config->gmch_pfit.control,
@@ -12858,6 +12888,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		}
 
 		PIPE_CONF_CHECK_I(scaler_state.scaler_id);
+		PIPE_CONF_CHECK_I(base.scaling_filter);
 		PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
 
 		PIPE_CONF_CHECK_X(gamma_mode);
@@ -14996,6 +15027,29 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	return ERR_PTR(ret);
 }
 
+static void icl_create_scaler_filter_property(struct intel_crtc *crtc,
+				    struct intel_crtc_state *crtc_state)
+{
+	struct drm_property *prop;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_mode_config *mode_config = &dev->mode_config;
+	u8 size;
+
+	if (mode_config->scaling_filter_property)
+		return;
+
+	size = ARRAY_SIZE(drm_scaling_filter_enum_list);
+	prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
+					"SCALING_FILTERS",
+					drm_scaling_filter_enum_list, size);
+	if (!prop) {
+		DRM_ERROR("Failed to create scaling filter property\n");
+		return;
+	}
+
+	dev->mode_config.scaling_filter_property = prop;
+}
+
 static void intel_crtc_init_scalers(struct intel_crtc *crtc,
 				    struct intel_crtc_state *crtc_state)
 {
@@ -15016,6 +15070,9 @@ static void intel_crtc_init_scalers(struct intel_crtc *crtc,
 	}
 
 	scaler_state->scaler_id = -1;
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		icl_create_scaler_filter_property(crtc, crtc_state);
 }
 
 #define INTEL_CRTC_FUNCS \
-- 
2.17.1

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

[PATCH 3/3] drm/i915: Handle nearest-neighbor scaling filter

[Shashank Sharma]

This patch adds support to handle nearest-neighbor(NN) filter
option for the panel fitter / pipe scaler.

Nearest-neighbor filter, when applied for integer upscaling ratios,
can produce sharp and blurless outputs. This is pretty useful while
upscaling the old games to higher resolution displays.

To enable NN:
- we need to set the scaler filter select value to "programmed"
- and then we need to program specific values to the scaler data registers.

Naturally, this patch also handles the Integer scaling scenarios. If the
user selects scaler filter value as "DRM_SCALING_FILTER_NN_IS_ONLY", this
means user wants to enable NN filter only when the upscaling ratios are
complete integer.

PS: There are two 80 char warnings in this patch, which is left
deliberately for the better readibility of register definision and to
maintain the existing formatting of the file. Maintainers can suggest.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 102 ++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/i915_reg.h               |  31 ++++++
 3 files changed, 136 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 21993f9fd2ae..b882e9886fde 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5411,6 +5411,17 @@ u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_cosited)
 #define SKL_MIN_YUV_420_SRC_W 16
 #define SKL_MIN_YUV_420_SRC_H 16
 
+static inline bool
+scaling_ratio_is_integer(int src_w, int dst_w, int src_h, int dst_h)
+{
+	/* Integer mode scaling is applicable only for upscaling scenarios */
+	if (dst_w < src_w || dst_h < src_h)
+		return false;
+	if (dst_w % src_w == 0 && dst_h % src_h == 0)
+		return true;
+	return false;
+}
+
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		  unsigned int scaler_user, int *scaler_id,
@@ -5445,6 +5456,15 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		return -EINVAL;
 	}
 
+	/*
+	 * If we are upscaling, and the scaling ratios are integer, we can
+	 * pick nearest-neighbour method in HW for scaling, which produces
+	 * blurless outputs in such scenarios.
+	 */
+	if (INTEL_GEN(dev_priv) >= 11 &&
+	    scaling_ratio_is_integer(src_w, dst_w, src_h, dst_h))
+		scaler_state->integer_scaling = true;
+
 	/*
 	 * if plane is being disabled or scaler is no more required or force detach
 	 *  - free scaler binded to this plane/crtc
@@ -5615,6 +5635,86 @@ static void skylake_scaler_disable(struct intel_crtc *crtc)
 		skl_detach_scaler(crtc, i);
 }
 
+static void
+icl_setup_nearest_neighbor_filter(const struct intel_crtc_state *crtc_state)
+{
+	int count;
+	int phase;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int scaler_id = crtc_state->scaler_state.scaler_id;
+	enum pipe pipe = crtc->pipe;
+
+	/*
+	 * To setup nearest-neighbor integer scaling mode:
+	 * Write 60 dwords: represnting 119 coefficients.
+	 *
+	 * Seven basic Coefficients are named from An......Gn.
+	 * Value of every D'th coefficent must be 1, all others to be 0.
+	 *
+	 * 17 such phases of 7 such coefficients = 119 coefficients.
+	 * Arrange these 119 coefficients in 60 dwords, 2 coefficient
+	 * per dword, in the sequence shown below:
+	 *
+	 *+------------+--------------+
+	 *|     B0     |      A0      |
+	 *+---------------------------+
+	 *|    D0 = 1  |      C0      |
+	 *+---------------------------+
+	 *|     F0     |      E0      |
+	 *+---------------------------+
+	 *|     A1     |      G0      |
+	 *+---------------------------+
+	 *|     C1     |      B1      |
+	 *+---------------------------+
+	 *|     E1     |      D1 = 1  |
+	 *+---------------------------+
+	 *|    .....   |     .....    |
+	 *+---------------------------+
+	 *|    ......  |     ......   |
+	 *+---------------------------+
+	 *|    Res     |      G16     |
+	 *+------------+--------------+
+	 *
+	 */
+
+	for (phase = 0; phase < 17; phase++) {
+		for (count = 0; count < 7; count++) {
+			u32 val = 0;
+
+			/* Every D'th entry needs to be 1 */
+			if (count == 3)
+				(phase % 2) ? (val = 1) : (val = (1 << 16));
+
+			I915_WRITE_FW(SKL_PS_COEF_INDEX_SET0(pipe, scaler_id),
+				      phase * 17 + count);
+			I915_WRITE_FW(SKL_PS_COEF_DATA_SET0(pipe, scaler_id),
+				      val);
+
+			I915_WRITE_FW(SKL_PS_COEF_INDEX_SET1(pipe, scaler_id),
+				      phase * 17 + count);
+			I915_WRITE_FW(SKL_PS_COEF_DATA_SET1(pipe, scaler_id),
+				      val);
+		}
+	}
+}
+
+static u32 icl_program_pfit_filter(const struct intel_crtc_state *crtc_state)
+{
+	const struct drm_crtc_state *state = &crtc_state->base;
+	const struct intel_crtc_scaler_state *scaler_state =
+		&crtc_state->scaler_state;
+
+	if (state->scaling_filter == DRM_SCALING_FILTER_NN_IS_ONLY &&
+		!scaler_state->integer_scaling) {
+		DRM_DEBUG_DRIVER("Scaling ratio not integer, not picking NN\n");
+		return PS_FILTER_MEDIUM;
+	}
+
+	icl_setup_nearest_neighbor_filter(crtc_state);
+	return PS_FILTER_PROGRAMMED;
+}
+
 static u32
 icelake_get_scaler_filter(const struct intel_crtc_state *crtc_state)
 {
@@ -5665,6 +5765,8 @@ static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
 		uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
 
 		scaler_filter = icelake_get_scaler_filter(crtc_state);
+		if (scaler_filter == PS_FILTER_PROGRAMMED)
+			scaler_filter = icl_program_pfit_filter(crtc_state);
 
 		id = scaler_state->scaler_id;
 		I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..8da499031bc2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -620,6 +620,9 @@ struct intel_crtc_scaler_state {
 
 	/* scaler used by crtc for panel fitting purpose */
 	int scaler_id;
+
+	/* indicate if scaling ratio is integer, to apply scaling filters */
+	bool integer_scaling;
 };
 
 /* drm_mode->private_flags */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1dc067fc57ab..f1ed5aa96ffd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7114,6 +7114,7 @@ enum {
 #define PS_PLANE_SEL(plane) (((plane) + 1) << 25)
 #define PS_FILTER_MASK         (3 << 23)
 #define PS_FILTER_MEDIUM       (0 << 23)
+#define PS_FILTER_PROGRAMMED   (1 << 23)
 #define PS_FILTER_EDGE_ENHANCE (2 << 23)
 #define PS_FILTER_BILINEAR     (3 << 23)
 #define PS_VERT3TAP            (1 << 21)
@@ -7190,6 +7191,24 @@ enum {
 #define _PS_ECC_STAT_2B     0x68AD0
 #define _PS_ECC_STAT_1C     0x691D0
 
+#define _PS_COEF_SET0_INDEX_1A     0x68198
+#define _PS_COEF_SET0_INDEX_2A     0x68298
+#define _PS_COEF_SET0_INDEX_1B     0x68998
+#define _PS_COEF_SET0_INDEX_2B     0x68A98
+#define _PS_COEF_SET1_INDEX_1A     0x681A0
+#define _PS_COEF_SET1_INDEX_2A     0x682A0
+#define _PS_COEF_SET1_INDEX_1B     0x689A0
+#define _PS_COEF_SET1_INDEX_2B     0x68AA0
+
+#define _PS_COEF_SET0_DATA_1A     0x6819C
+#define _PS_COEF_SET0_DATA_2A     0x6829C
+#define _PS_COEF_SET0_DATA_1B     0x6899C
+#define _PS_COEF_SET0_DATA_2B     0x68A9C
+#define _PS_COEF_SET1_DATA_1A     0x681A4
+#define _PS_COEF_SET1_DATA_2A     0x682A4
+#define _PS_COEF_SET1_DATA_1B     0x689A4
+#define _PS_COEF_SET1_DATA_2B     0x68AA4
+
 #define _ID(id, a, b) _PICK_EVEN(id, a, b)
 #define SKL_PS_CTRL(pipe, id) _MMIO_PIPE(pipe,        \
 			_ID(id, _PS_1A_CTRL, _PS_2A_CTRL),       \
@@ -7218,6 +7237,18 @@ enum {
 #define SKL_PS_ECC_STAT(pipe, id)  _MMIO_PIPE(pipe,     \
 			_ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A),   \
 			_ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B))
+#define SKL_PS_COEF_DATA_SET0(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET0_DATA_1A, _PS_COEF_SET0_DATA_2A), \
+			_ID(id, _PS_COEF_SET0_DATA_1B, _PS_COEF_SET0_DATA_1B))
+#define SKL_PS_COEF_DATA_SET1(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET1_DATA_1A, _PS_COEF_SET1_DATA_2A), \
+			_ID(id, _PS_COEF_SET1_DATA_1B, _PS_COEF_SET1_DATA_1B))
+#define SKL_PS_COEF_INDEX_SET0(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET0_INDEX_1A, _PS_COEF_SET0_INDEX_2A), \
+			_ID(id, _PS_COEF_SET0_INDEX_1B, _PS_COEF_SET0_INDEX_1B))
+#define SKL_PS_COEF_INDEX_SET1(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET1_INDEX_1A, _PS_COEF_SET1_INDEX_2A), \
+			_ID(id, _PS_COEF_SET1_INDEX_1B, _PS_COEF_SET1_INDEX_1B))
 
 /* legacy palette */
 #define _LGC_PALETTE_A           0x4a000
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Daniel Vetter]

On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> This patch adds a scaling filter mode porperty
> to allow:
> - A driver/HW to showcase it's scaling filter capabilities.
> - A userspace to pick a desired effect while scaling.
> 
> This option will be particularly useful in the scenarios where
> Integer mode scaling is possible, and a UI client wants to pick
> filters like Nearest-neighbor applied for non-blurry outputs.
> 
> There was a RFC patch series published, to discus the request to enable
> Integer mode scaling by some of the gaming communities, which can be
> found here:
> https://patchwork.freedesktop.org/series/66175/
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>

Two things:

- needs real property docs for this in the kms property chapter
- would be good if we could somehow subsume the panel fitter prop into
  this? Or at least document possible interactions with that.

Plus userspace ofc, recommended best practices is to add a Link: tag
pointing at the userspace patch series/merge request directly to the patch
that adds the new uapi.

Cheers, Daniel
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 0d466d3b0809..883329453a86 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		return ret;
>  	} else if (property == config->prop_vrr_enabled) {
>  		state->vrr_enabled = val;
> +	} else if (property == config->scaling_filter_property) {
> +		state->scaling_filter = val;
>  	} else if (property == config->degamma_lut_property) {
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->degamma_lut,
> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (property == config->prop_out_fence_ptr)
>  		*val = 0;
> +	else if (property == config->scaling_filter_property)
> +		*val = state->scaling_filter;
>  	else if (crtc->funcs->atomic_get_property)
>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>  	else
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5e9b15a0e8c5..94c5509474a8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -58,6 +58,25 @@ struct device_node;
>  struct dma_fence;
>  struct edid;
>  
> +enum drm_scaling_filters {
> +	DRM_SCALING_FILTER_DEFAULT,
> +	DRM_SCALING_FILTER_MEDIUM,
> +	DRM_SCALING_FILTER_BILINEAR,
> +	DRM_SCALING_FILTER_NN,
> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> +	DRM_SCALING_FILTER_INVALID,
> +};
> +
> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> +};
> +
>  static inline int64_t U642I64(uint64_t val)
>  {
>  	return (int64_t)*((int64_t *)&val);
> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>  	 */
>  	u32 target_vblank;
>  
> +	/**
> +	 * @scaling_filter:
> +	 *
> +	 * Scaling filter mode to be applied
> +	 */
> +	u32 scaling_filter;
> +
>  	/**
>  	 * @async_flip:
>  	 *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 3bcbe30339f0..efd6fd55770f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -914,6 +914,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *modifiers_property;
>  
> +	/**
> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> +	 * while scaling in panel fitter mode.
> +	 */
> +	struct drm_property *scaling_filter_property;
> +
>  	/* cursor size */
>  	uint32_t cursor_width, cursor_height;
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Sharma, Shashank]

Hey Daniel, 

> -----Original Message-----
> From: Daniel Vetter <daniel.vetter@ffwll.ch> On Behalf Of Daniel Vetter
> Sent: Tuesday, October 22, 2019 3:39 PM
> To: Sharma, Shashank <shashank.sharma@intel.com>
> Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property
> 
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> > This patch adds a scaling filter mode porperty to allow:
> > - A driver/HW to showcase it's scaling filter capabilities.
> > - A userspace to pick a desired effect while scaling.
> >
> > This option will be particularly useful in the scenarios where Integer
> > mode scaling is possible, and a UI client wants to pick filters like
> > Nearest-neighbor applied for non-blurry outputs.
> >
> > There was a RFC patch series published, to discus the request to
> > enable Integer mode scaling by some of the gaming communities, which
> > can be found here:
> > https://patchwork.freedesktop.org/series/66175/
> >
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> 
> Two things:
> 
> - needs real property docs for this in the kms property chapter
Got it, 
> - would be good if we could somehow subsume the panel fitter prop into
>   this? Or at least document possible interactions with that.
> 
Sure, let me see what can I do here. 
> Plus userspace ofc, recommended best practices is to add a Link: tag pointing at the
> userspace patch series/merge request directly to the patch that adds the new uapi.
> 

Yep, WIP, will soon drop a link here. 

- Shashank
> Cheers, Daniel
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >  include/drm/drm_mode_config.h     |  6 ++++++
> >  3 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 0d466d3b0809..883329453a86 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
> *crtc,
> >  		return ret;
> >  	} else if (property == config->prop_vrr_enabled) {
> >  		state->vrr_enabled = val;
> > +	} else if (property == config->scaling_filter_property) {
> > +		state->scaling_filter = val;
> >  	} else if (property == config->degamma_lut_property) {
> >  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >  					&state->degamma_lut,
> > @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >  	else if (property == config->prop_out_fence_ptr)
> >  		*val = 0;
> > +	else if (property == config->scaling_filter_property)
> > +		*val = state->scaling_filter;
> >  	else if (crtc->funcs->atomic_get_property)
> >  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >  	else
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> > 5e9b15a0e8c5..94c5509474a8 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -58,6 +58,25 @@ struct device_node;  struct dma_fence;  struct
> > edid;
> >
> > +enum drm_scaling_filters {
> > +	DRM_SCALING_FILTER_DEFAULT,
> > +	DRM_SCALING_FILTER_MEDIUM,
> > +	DRM_SCALING_FILTER_BILINEAR,
> > +	DRM_SCALING_FILTER_NN,
> > +	DRM_SCALING_FILTER_NN_IS_ONLY,
> > +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> > +	DRM_SCALING_FILTER_INVALID,
> > +};
> > +
> > +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> > +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> > +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> > +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> > +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> > +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> > +	{ DRM_SCALING_FILTER_INVALID, "Invalid" }, };
> > +
> >  static inline int64_t U642I64(uint64_t val)  {
> >  	return (int64_t)*((int64_t *)&val);
> > @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >  	 */
> >  	u32 target_vblank;
> >
> > +	/**
> > +	 * @scaling_filter:
> > +	 *
> > +	 * Scaling filter mode to be applied
> > +	 */
> > +	u32 scaling_filter;
> > +
> >  	/**
> >  	 * @async_flip:
> >  	 *
> > diff --git a/include/drm/drm_mode_config.h
> > b/include/drm/drm_mode_config.h index 3bcbe30339f0..efd6fd55770f
> > 100644
> > --- a/include/drm/drm_mode_config.h
> > +++ b/include/drm/drm_mode_config.h
> > @@ -914,6 +914,12 @@ struct drm_mode_config {
> >  	 */
> >  	struct drm_property *modifiers_property;
> >
> > +	/**
> > +	 * @scaling_filter_property: CRTC property to apply a particular filter
> > +	 * while scaling in panel fitter mode.
> > +	 */
> > +	struct drm_property *scaling_filter_property;
> > +
> >  	/* cursor size */
> >  	uint32_t cursor_width, cursor_height;
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> This patch adds a scaling filter mode porperty
> to allow:
> - A driver/HW to showcase it's scaling filter capabilities.
> - A userspace to pick a desired effect while scaling.
> 
> This option will be particularly useful in the scenarios where
> Integer mode scaling is possible, and a UI client wants to pick
> filters like Nearest-neighbor applied for non-blurry outputs.
> 
> There was a RFC patch series published, to discus the request to enable
> Integer mode scaling by some of the gaming communities, which can be
> found here:
> https://patchwork.freedesktop.org/series/66175/
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 0d466d3b0809..883329453a86 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		return ret;
>  	} else if (property == config->prop_vrr_enabled) {
>  		state->vrr_enabled = val;
> +	} else if (property == config->scaling_filter_property) {
> +		state->scaling_filter = val;
>  	} else if (property == config->degamma_lut_property) {
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->degamma_lut,
> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (property == config->prop_out_fence_ptr)
>  		*val = 0;
> +	else if (property == config->scaling_filter_property)
> +		*val = state->scaling_filter;
>  	else if (crtc->funcs->atomic_get_property)
>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>  	else
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5e9b15a0e8c5..94c5509474a8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -58,6 +58,25 @@ struct device_node;
>  struct dma_fence;
>  struct edid;
>  
> +enum drm_scaling_filters {
> +	DRM_SCALING_FILTER_DEFAULT,
> +	DRM_SCALING_FILTER_MEDIUM,
> +	DRM_SCALING_FILTER_BILINEAR,
> +	DRM_SCALING_FILTER_NN,

Please use real words.

> +	DRM_SCALING_FILTER_NN_IS_ONLY,

Not a big fan of this. I'd just add the explicit nearest filter
and leave the decision whether to use it to userspace.

> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> +	DRM_SCALING_FILTER_INVALID,

That invalid enum value seems entirely pointless.

This set of filters is pretty arbitrary. It doesn't even cover all
Intel hw. I would probably just leave it at "default+linear+nearest"
initially. Exposing more vague hw specific choices needs more though,
and I'd prefer not to spend those brain cells until a real use case
emerges.

> +};
> +
> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> +};
> +
>  static inline int64_t U642I64(uint64_t val)
>  {
>  	return (int64_t)*((int64_t *)&val);
> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>  	 */
>  	u32 target_vblank;
>  
> +	/**
> +	 * @scaling_filter:
> +	 *
> +	 * Scaling filter mode to be applied
> +	 */
> +	u32 scaling_filter;

We have an enum so should use it. The documentation should probably
point out that this applies to full crtc scaling only, not plane
scaling.

> +
>  	/**
>  	 * @async_flip:
>  	 *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 3bcbe30339f0..efd6fd55770f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -914,6 +914,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *modifiers_property;
>  
> +	/**
> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> +	 * while scaling in panel fitter mode.
> +	 */
> +	struct drm_property *scaling_filter_property;
> +
>  	/* cursor size */
>  	uint32_t cursor_width, cursor_height;
>  
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Tue, Oct 22, 2019 at 10:12:29AM +0000, Sharma, Shashank wrote:
> Hey Daniel, 
> 
> > -----Original Message-----
> > From: Daniel Vetter <daniel.vetter@ffwll.ch> On Behalf Of Daniel Vetter
> > Sent: Tuesday, October 22, 2019 3:39 PM
> > To: Sharma, Shashank <shashank.sharma@intel.com>
> > Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property
> > 
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> > > This patch adds a scaling filter mode porperty to allow:
> > > - A driver/HW to showcase it's scaling filter capabilities.
> > > - A userspace to pick a desired effect while scaling.
> > >
> > > This option will be particularly useful in the scenarios where Integer
> > > mode scaling is possible, and a UI client wants to pick filters like
> > > Nearest-neighbor applied for non-blurry outputs.
> > >
> > > There was a RFC patch series published, to discus the request to
> > > enable Integer mode scaling by some of the gaming communities, which
> > > can be found here:
> > > https://patchwork.freedesktop.org/series/66175/
> > >
> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > 
> > Two things:
> > 
> > - needs real property docs for this in the kms property chapter
> Got it, 
> > - would be good if we could somehow subsume the panel fitter prop into
> >   this? Or at least document possible interactions with that.
> > 
> Sure, let me see what can I do here. 

The scaling mode prop has nothing really to do with the filter being
used. The scaling mode prop just provides a bad mechanism for
configuring the destination coordinates of the scaler.

Trying to shoehorn these things into one prop would be a mistake IMO.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> This patch adds a scaling filter mode porperty
> to allow:
> - A driver/HW to showcase it's scaling filter capabilities.
> - A userspace to pick a desired effect while scaling.
> 
> This option will be particularly useful in the scenarios where
> Integer mode scaling is possible, and a UI client wants to pick
> filters like Nearest-neighbor applied for non-blurry outputs.
> 
> There was a RFC patch series published, to discus the request to enable
> Integer mode scaling by some of the gaming communities, which can be
> found here:
> https://patchwork.freedesktop.org/series/66175/
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 0d466d3b0809..883329453a86 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		return ret;
>  	} else if (property == config->prop_vrr_enabled) {
>  		state->vrr_enabled = val;
> +	} else if (property == config->scaling_filter_property) {
> +		state->scaling_filter = val;
>  	} else if (property == config->degamma_lut_property) {
>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>  					&state->degamma_lut,
> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (property == config->prop_out_fence_ptr)
>  		*val = 0;
> +	else if (property == config->scaling_filter_property)
> +		*val = state->scaling_filter;
>  	else if (crtc->funcs->atomic_get_property)
>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>  	else
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5e9b15a0e8c5..94c5509474a8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -58,6 +58,25 @@ struct device_node;
>  struct dma_fence;
>  struct edid;
>  
> +enum drm_scaling_filters {
> +	DRM_SCALING_FILTER_DEFAULT,
> +	DRM_SCALING_FILTER_MEDIUM,
> +	DRM_SCALING_FILTER_BILINEAR,
> +	DRM_SCALING_FILTER_NN,
> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> +	DRM_SCALING_FILTER_INVALID,
> +};
> +
> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> +};
> +
>  static inline int64_t U642I64(uint64_t val)
>  {
>  	return (int64_t)*((int64_t *)&val);
> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>  	 */
>  	u32 target_vblank;
>  
> +	/**
> +	 * @scaling_filter:
> +	 *
> +	 * Scaling filter mode to be applied
> +	 */
> +	u32 scaling_filter;
> +
>  	/**
>  	 * @async_flip:
>  	 *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 3bcbe30339f0..efd6fd55770f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -914,6 +914,12 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *modifiers_property;
>  
> +	/**
> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> +	 * while scaling in panel fitter mode.
> +	 */
> +	struct drm_property *scaling_filter_property;

This needs to per-crtc/plane.

> +
>  	/* cursor size */
>  	uint32_t cursor_width, cursor_height;
>  
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.CHECKPATCH: warning for Add scaling filters in DRM layer

[Patchwork]

== Series Details ==

Series: Add scaling filters in DRM layer
URL   : https://patchwork.freedesktop.org/series/68378/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
bd6ebd85fce2 drm: Introduce scaling filter mode property
13901f40e66e drm/i915: Add support for scaling filters
-:90: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#90: FILE: drivers/gpu/drm/i915/display/intel_display.c:15496:
+static void icl_create_scaler_filter_property(struct intel_crtc *crtc,
+				    struct intel_crtc_state *crtc_state)

total: 0 errors, 0 warnings, 1 checks, 102 lines checked
5fced141d9e4 drm/i915: Handle nearest-neighbor scaling filter
-:141: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#141: FILE: drivers/gpu/drm/i915/display/intel_display.c:5723:
+	if (state->scaling_filter == DRM_SCALING_FILTER_NN_IS_ONLY &&
+		!scaler_state->integer_scaling) {

-:217: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects?
#217: FILE: drivers/gpu/drm/i915/i915_reg.h:7249:
+#define SKL_PS_COEF_DATA_SET0(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET0_DATA_1A, _PS_COEF_SET0_DATA_2A), \
+			_ID(id, _PS_COEF_SET0_DATA_1B, _PS_COEF_SET0_DATA_1B))

-:220: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects?
#220: FILE: drivers/gpu/drm/i915/i915_reg.h:7252:
+#define SKL_PS_COEF_DATA_SET1(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET1_DATA_1A, _PS_COEF_SET1_DATA_2A), \
+			_ID(id, _PS_COEF_SET1_DATA_1B, _PS_COEF_SET1_DATA_1B))

-:223: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects?
#223: FILE: drivers/gpu/drm/i915/i915_reg.h:7255:
+#define SKL_PS_COEF_INDEX_SET0(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET0_INDEX_1A, _PS_COEF_SET0_INDEX_2A), \
+			_ID(id, _PS_COEF_SET0_INDEX_1B, _PS_COEF_SET0_INDEX_1B))

-:226: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects?
#226: FILE: drivers/gpu/drm/i915/i915_reg.h:7258:
+#define SKL_PS_COEF_INDEX_SET1(pipe, id)  _MMIO_PIPE(pipe,     \
+			_ID(id, _PS_COEF_SET1_INDEX_1A, _PS_COEF_SET1_INDEX_2A), \
+			_ID(id, _PS_COEF_SET1_INDEX_1B, _PS_COEF_SET1_INDEX_1B))

total: 0 errors, 0 warnings, 5 checks, 184 lines checked

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

✗ Fi.CI.SPARSE: warning for Add scaling filters in DRM layer

[Patchwork]

== Series Details ==

Series: Add scaling filters in DRM layer
URL   : https://patchwork.freedesktop.org/series/68378/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm: Introduce scaling filter mode property
Okay!

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

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Mihail Atanassov]

Hi Shashank,

On Tuesday, 22 October 2019 10:59:44 BST Shashank Sharma wrote:
> This patch adds a scaling filter mode porperty
> to allow:
> - A driver/HW to showcase it's scaling filter capabilities.
> - A userspace to pick a desired effect while scaling.
>
> This option will be particularly useful in the scenarios where
> Integer mode scaling is possible, and a UI client wants to pick
> filters like Nearest-neighbor applied for non-blurry outputs.
>
> There was a RFC patch series published, to discus the request to enable
> Integer mode scaling by some of the gaming communities, which can be
> found here:
> https://patchwork.freedesktop.org/series/66175/
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 0d466d3b0809..883329453a86 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>               return ret;
>       } else if (property == config->prop_vrr_enabled) {
>               state->vrr_enabled = val;
> +     } else if (property == config->scaling_filter_property) {
> +             state->scaling_filter = val;
>       } else if (property == config->degamma_lut_property) {
>               ret = drm_atomic_replace_property_blob_from_id(dev,
>                                       &state->degamma_lut,
> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>               *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>       else if (property == config->prop_out_fence_ptr)
>               *val = 0;
> +     else if (property == config->scaling_filter_property)
> +             *val = state->scaling_filter;
>       else if (crtc->funcs->atomic_get_property)
>               return crtc->funcs->atomic_get_property(crtc, state, property, val);
>       else
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5e9b15a0e8c5..94c5509474a8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -58,6 +58,25 @@ struct device_node;
>  struct dma_fence;
>  struct edid;
>
> +enum drm_scaling_filters {
> +     DRM_SCALING_FILTER_DEFAULT,
> +     DRM_SCALING_FILTER_MEDIUM,
> +     DRM_SCALING_FILTER_BILINEAR,
> +     DRM_SCALING_FILTER_NN,
> +     DRM_SCALING_FILTER_NN_IS_ONLY,
> +     DRM_SCALING_FILTER_EDGE_ENHANCE,

This one looks to be missing a stringified version below. Just wanted
to jump in early and suggest dropping it from the scaling filter enum.
Edge enhancement is orthogonal to the mode used for scaling, at least
on komeda HW, so we wouldn't be able to make the best use of the
property in its current form.

> +     DRM_SCALING_FILTER_INVALID,
> +};
> +
> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> +     { DRM_SCALING_FILTER_DEFAULT, "Default" },
> +     { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> +     { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> +     { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> +     { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> +     { DRM_SCALING_FILTER_INVALID, "Invalid" },
> +};
> +
>  static inline int64_t U642I64(uint64_t val)
>  {
>       return (int64_t)*((int64_t *)&val);
> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>        */
>       u32 target_vblank;
>
> +     /**
> +      * @scaling_filter:
> +      *
> +      * Scaling filter mode to be applied
> +      */
> +     u32 scaling_filter;
> +
>       /**
>        * @async_flip:
>        *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 3bcbe30339f0..efd6fd55770f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -914,6 +914,12 @@ struct drm_mode_config {
>        */
>       struct drm_property *modifiers_property;
>
> +     /**
> +      * @scaling_filter_property: CRTC property to apply a particular filter

A scaling filter option can also be useful for scaling individual
planes, do you have any plans to extend the property's applications
in that direction?

> +      * while scaling in panel fitter mode.
> +      */
> +     struct drm_property *scaling_filter_property;
> +
>       /* cursor size */
>       uint32_t cursor_width, cursor_height;
>
>


--
Mihail



IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Mihail Atanassov]

On Tuesday, 22 October 2019 14:26:38 BST Mihail Atanassov wrote:
> Hi Shashank,
> 
> On Tuesday, 22 October 2019 10:59:44 BST Shashank Sharma wrote:
> > This patch adds a scaling filter mode porperty
> > to allow:
> > - A driver/HW to showcase it's scaling filter capabilities.
> > - A userspace to pick a desired effect while scaling.
> >
> > This option will be particularly useful in the scenarios where
> > Integer mode scaling is possible, and a UI client wants to pick
> > filters like Nearest-neighbor applied for non-blurry outputs.
> >
> > There was a RFC patch series published, to discus the request to enable
> > Integer mode scaling by some of the gaming communities, which can be
> > found here:
> > https://patchwork.freedesktop.org/series/66175/
> >
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >  include/drm/drm_mode_config.h     |  6 ++++++
> >  3 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index 0d466d3b0809..883329453a86 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >               return ret;
> >       } else if (property == config->prop_vrr_enabled) {
> >               state->vrr_enabled = val;
> > +     } else if (property == config->scaling_filter_property) {
> > +             state->scaling_filter = val;
> >       } else if (property == config->degamma_lut_property) {
> >               ret = drm_atomic_replace_property_blob_from_id(dev,
> >                                       &state->degamma_lut,
> > @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >               *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >       else if (property == config->prop_out_fence_ptr)
> >               *val = 0;
> > +     else if (property == config->scaling_filter_property)
> > +             *val = state->scaling_filter;
> >       else if (crtc->funcs->atomic_get_property)
> >               return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >       else
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 5e9b15a0e8c5..94c5509474a8 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -58,6 +58,25 @@ struct device_node;
> >  struct dma_fence;
> >  struct edid;
> >
> > +enum drm_scaling_filters {
> > +     DRM_SCALING_FILTER_DEFAULT,
> > +     DRM_SCALING_FILTER_MEDIUM,
> > +     DRM_SCALING_FILTER_BILINEAR,
> > +     DRM_SCALING_FILTER_NN,
> > +     DRM_SCALING_FILTER_NN_IS_ONLY,
> > +     DRM_SCALING_FILTER_EDGE_ENHANCE,
> 
> This one looks to be missing a stringified version below. Just wanted
> to jump in early and suggest dropping it from the scaling filter enum.
> Edge enhancement is orthogonal to the mode used for scaling, at least
> on komeda HW, so we wouldn't be able to make the best use of the
> property in its current form.
> 
> > +     DRM_SCALING_FILTER_INVALID,
> > +};
> > +
> > +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> > +     { DRM_SCALING_FILTER_DEFAULT, "Default" },
> > +     { DRM_SCALING_FILTER_MEDIUM, "Medium" },
> > +     { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> > +     { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> > +     { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> > +     { DRM_SCALING_FILTER_INVALID, "Invalid" },
> > +};
> > +
> >  static inline int64_t U642I64(uint64_t val)
> >  {
> >       return (int64_t)*((int64_t *)&val);
> > @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >        */
> >       u32 target_vblank;
> >
> > +     /**
> > +      * @scaling_filter:
> > +      *
> > +      * Scaling filter mode to be applied
> > +      */
> > +     u32 scaling_filter;
> > +
> >       /**
> >        * @async_flip:
> >        *
> > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > index 3bcbe30339f0..efd6fd55770f 100644
> > --- a/include/drm/drm_mode_config.h
> > +++ b/include/drm/drm_mode_config.h
> > @@ -914,6 +914,12 @@ struct drm_mode_config {
> >        */
> >       struct drm_property *modifiers_property;
> >
> > +     /**
> > +      * @scaling_filter_property: CRTC property to apply a particular filter
> 
> A scaling filter option can also be useful for scaling individual
> planes, do you have any plans to extend the property's applications
> in that direction?
> 
> > +      * while scaling in panel fitter mode.
> > +      */
> > +     struct drm_property *scaling_filter_property;
> > +
> >       /* cursor size */
> >       uint32_t cursor_width, cursor_height;
> >
> >
> 
> 
> --
> Mihail
> 
> 
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Sorry about that notice, not intentional :-(

> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


-- 
Mihail



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.BAT: success for Add scaling filters in DRM layer

[Patchwork]

== Series Details ==

Series: Add scaling filters in DRM layer
URL   : https://patchwork.freedesktop.org/series/68378/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7149 -> Patchwork_14916
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-icl-u3:          [INCOMPLETE][1] ([fdo#107713] / [fdo#109100]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/fi-icl-u3/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/fi-icl-u3/igt@gem_ctx_create@basic-files.html

  * igt@gem_ctx_switch@legacy-render:
    - {fi-cml-s}:         [INCOMPLETE][3] ([fdo#110566] / [fdo#111381]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/fi-cml-s/igt@gem_ctx_switch@legacy-render.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/fi-cml-s/igt@gem_ctx_switch@legacy-render.html

  * {igt@i915_selftest@live_gt_heartbeat}:
    - fi-cml-u:           [DMESG-FAIL][5] ([fdo#112096]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/fi-cml-u/igt@i915_selftest@live_gt_heartbeat.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/fi-cml-u/igt@i915_selftest@live_gt_heartbeat.html
    - fi-cfl-guc:         [DMESG-FAIL][7] ([fdo#112096]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/fi-cfl-guc/igt@i915_selftest@live_gt_heartbeat.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/fi-cfl-guc/igt@i915_selftest@live_gt_heartbeat.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][9] ([fdo#111045] / [fdo#111096]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

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

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108743]: https://bugs.freedesktop.org/show_bug.cgi?id=108743
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111736]: https://bugs.freedesktop.org/show_bug.cgi?id=111736
  [fdo#111850]: https://bugs.freedesktop.org/show_bug.cgi?id=111850
  [fdo#112096]: https://bugs.freedesktop.org/show_bug.cgi?id=112096


Participating hosts (49 -> 45)
------------------------------

  Additional (4): fi-kbl-soraka fi-cml-u2 fi-icl-u2 fi-tgl-u2 
  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7149 -> Patchwork_14916

  CI-20190529: 20190529
  CI_DRM_7149: 44d2673963fd51800de361180554f1dad14f25a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5235: da9abbab69be80dd00812a4607a4ea2dffcc4544 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14916: 5fced141d9e40100a93d2851a9860ef2172b620e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5fced141d9e4 drm/i915: Handle nearest-neighbor scaling filter
13901f40e66e drm/i915: Add support for scaling filters
bd6ebd85fce2 drm: Introduce scaling filter mode property

== Logs ==

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

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Harry Wentland]

On 2019-10-22 8:20 a.m., Ville Syrjälä wrote:
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>>
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>>
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>  include/drm/drm_mode_config.h     |  6 ++++++
>>  3 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>  		return ret;
>>  	} else if (property == config->prop_vrr_enabled) {
>>  		state->vrr_enabled = val;
>> +	} else if (property == config->scaling_filter_property) {
>> +		state->scaling_filter = val;
>>  	} else if (property == config->degamma_lut_property) {
>>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>>  					&state->degamma_lut,
>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>  	else if (property == config->prop_out_fence_ptr)
>>  		*val = 0;
>> +	else if (property == config->scaling_filter_property)
>> +		*val = state->scaling_filter;
>>  	else if (crtc->funcs->atomic_get_property)
>>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>  	else
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>  struct dma_fence;
>>  struct edid;
>>  
>> +enum drm_scaling_filters {
>> +	DRM_SCALING_FILTER_DEFAULT,
>> +	DRM_SCALING_FILTER_MEDIUM,
>> +	DRM_SCALING_FILTER_BILINEAR,
>> +	DRM_SCALING_FILTER_NN,
> 
> Please use real words.
> 
>> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> 
> Not a big fan of this. I'd just add the explicit nearest filter
> and leave the decision whether to use it to userspace.
> 
>> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +	DRM_SCALING_FILTER_INVALID,
> 
> That invalid enum value seems entirely pointless.
> 
> This set of filters is pretty arbitrary. It doesn't even cover all
> Intel hw. I would probably just leave it at "default+linear+nearest"
> initially. Exposing more vague hw specific choices needs more though,
> and I'd prefer not to spend those brain cells until a real use case
> emerges.
> 

FWIW, AMD HW allows us to specify a number of horizontal and vertical
taps for scaling. Number of taps are limited by our linebuffer size. The
default is 4 in each dimension but you could have 2 v_taps and 4 h_taps
if your're running a large horizontal resolution on some ASICs.

I'm not sure it makes sense to define filters here that aren't used. It
sounds like default and nearest neighbour would suffice for now in order
to support integer scaling.

Harry

>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>  static inline int64_t U642I64(uint64_t val)
>>  {
>>  	return (int64_t)*((int64_t *)&val);
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>  	 */
>>  	u32 target_vblank;
>>  
>> +	/**
>> +	 * @scaling_filter:
>> +	 *
>> +	 * Scaling filter mode to be applied
>> +	 */
>> +	u32 scaling_filter;
> 
> We have an enum so should use it. The documentation should probably
> point out that this applies to full crtc scaling only, not plane
> scaling.
> 
>> +
>>  	/**
>>  	 * @async_flip:
>>  	 *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>  	 */
>>  	struct drm_property *modifiers_property;
>>  
>> +	/**
>> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> +	 * while scaling in panel fitter mode.
>> +	 */
>> +	struct drm_property *scaling_filter_property;
>> +
>>  	/* cursor size */
>>  	uint32_t cursor_width, cursor_height;
>>  
>> -- 
>> 2.17.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Sharma, Shashank]

Hello Ville,

Thanks for the comments, mine inline.


On 10/22/2019 5:50 PM, Ville Syrjälä wrote:
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>>
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>>
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>   include/drm/drm_mode_config.h     |  6 ++++++
>>   3 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>   		return ret;
>>   	} else if (property == config->prop_vrr_enabled) {
>>   		state->vrr_enabled = val;
>> +	} else if (property == config->scaling_filter_property) {
>> +		state->scaling_filter = val;
>>   	} else if (property == config->degamma_lut_property) {
>>   		ret = drm_atomic_replace_property_blob_from_id(dev,
>>   					&state->degamma_lut,
>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>   		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>   	else if (property == config->prop_out_fence_ptr)
>>   		*val = 0;
>> +	else if (property == config->scaling_filter_property)
>> +		*val = state->scaling_filter;
>>   	else if (crtc->funcs->atomic_get_property)
>>   		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>   	else
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>   struct dma_fence;
>>   struct edid;
>>   
>> +enum drm_scaling_filters {
>> +	DRM_SCALING_FILTER_DEFAULT,
>> +	DRM_SCALING_FILTER_MEDIUM,
>> +	DRM_SCALING_FILTER_BILINEAR,
>> +	DRM_SCALING_FILTER_NN,
> Please use real words.
Yes, I saw that coming. It was getting difficult with the 80 char stuff, 
it was even more difficult while using it :). But let me see how better 
can I do here.
>> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> Not a big fan of this. I'd just add the explicit nearest filter
> and leave the decision whether to use it to userspace.
Agree, that's also one option. I was thinking to make it convenient for 
userspace,  For example if a compositor just want to checkout NN only 
when its beneficial (like old gaming scenarios) but doesn't have enough 
information (or intent), it can leave it to kernel too. But I agree, 
this can cause corner cases. Let's discuss and see if we need it at all, 
as you mentioned.
>> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +	DRM_SCALING_FILTER_INVALID,
> That invalid enum value seems entirely pointless.
I was thinking about loops and all, but yeah, we can remove it.
>
> This set of filters is pretty arbitrary. It doesn't even cover all
> Intel hw. I would probably just leave it at "default+linear+nearest"
> initially. Exposing more vague hw specific choices needs more though,
> and I'd prefer not to spend those brain cells until a real use case
> emerges.
Sure, lets start with smaller set.
>
>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>   static inline int64_t U642I64(uint64_t val)
>>   {
>>   	return (int64_t)*((int64_t *)&val);
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>   	 */
>>   	u32 target_vblank;
>>   
>> +	/**
>> +	 * @scaling_filter:
>> +	 *
>> +	 * Scaling filter mode to be applied
>> +	 */
>> +	u32 scaling_filter;
> We have an enum so should use it.
Got it.
> The documentation should probably
> point out that this applies to full crtc scaling only, not plane
> scaling.
The comment is actually with the property, Here I think its clear that 
it's for CRTC scaling, as its a part of CRTC state.
>
>> +
>>   	/**
>>   	 * @async_flip:
>>   	 *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>   	 */
>>   	struct drm_property *modifiers_property;
>>   
>> +	/**
>> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> +	 * while scaling in panel fitter mode.
>> +	 */
>> +	struct drm_property *scaling_filter_property;
>> +
>>   	/* cursor size */
>>   	uint32_t cursor_width, cursor_height;
>>   
>> -- 
>> 2.17.1
> - Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Sharma, Shashank]

On 10/22/2019 5:56 PM, Ville Syrjälä wrote:
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>>
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>>
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>   include/drm/drm_mode_config.h     |  6 ++++++
>>   3 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>   		return ret;
>>   	} else if (property == config->prop_vrr_enabled) {
>>   		state->vrr_enabled = val;
>> +	} else if (property == config->scaling_filter_property) {
>> +		state->scaling_filter = val;
>>   	} else if (property == config->degamma_lut_property) {
>>   		ret = drm_atomic_replace_property_blob_from_id(dev,
>>   					&state->degamma_lut,
>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>   		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>   	else if (property == config->prop_out_fence_ptr)
>>   		*val = 0;
>> +	else if (property == config->scaling_filter_property)
>> +		*val = state->scaling_filter;
>>   	else if (crtc->funcs->atomic_get_property)
>>   		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>   	else
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>   struct dma_fence;
>>   struct edid;
>>   
>> +enum drm_scaling_filters {
>> +	DRM_SCALING_FILTER_DEFAULT,
>> +	DRM_SCALING_FILTER_MEDIUM,
>> +	DRM_SCALING_FILTER_BILINEAR,
>> +	DRM_SCALING_FILTER_NN,
>> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +	DRM_SCALING_FILTER_INVALID,
>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>   static inline int64_t U642I64(uint64_t val)
>>   {
>>   	return (int64_t)*((int64_t *)&val);
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>   	 */
>>   	u32 target_vblank;
>>   
>> +	/**
>> +	 * @scaling_filter:
>> +	 *
>> +	 * Scaling filter mode to be applied
>> +	 */
>> +	u32 scaling_filter;
>> +
>>   	/**
>>   	 * @async_flip:
>>   	 *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>   	 */
>>   	struct drm_property *modifiers_property;
>>   
>> +	/**
>> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> +	 * while scaling in panel fitter mode.
>> +	 */
>> +	struct drm_property *scaling_filter_property;
> This needs to per-crtc/plane.

I am not getting this, why ? It's not different than any other CRTC 
property like gamma/CSC etc, where we just attach them to corresponding 
CRTC, isn't it ?

- Shashank

>> +
>>   	/* cursor size */
>>   	uint32_t cursor_width, cursor_height;
>>   
>> -- 
>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Sharma, Shashank]

Hello Mihail,

Thanks for your review, my comments inline.

On 10/22/2019 6:56 PM, Mihail Atanassov wrote:
> Hi Shashank,
>
> On Tuesday, 22 October 2019 10:59:44 BST Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>>
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>>
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>   include/drm/drm_mode_config.h     |  6 ++++++
>>   3 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>                return ret;
>>        } else if (property == config->prop_vrr_enabled) {
>>                state->vrr_enabled = val;
>> +     } else if (property == config->scaling_filter_property) {
>> +             state->scaling_filter = val;
>>        } else if (property == config->degamma_lut_property) {
>>                ret = drm_atomic_replace_property_blob_from_id(dev,
>>                                        &state->degamma_lut,
>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>                *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>        else if (property == config->prop_out_fence_ptr)
>>                *val = 0;
>> +     else if (property == config->scaling_filter_property)
>> +             *val = state->scaling_filter;
>>        else if (crtc->funcs->atomic_get_property)
>>                return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>        else
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>   struct dma_fence;
>>   struct edid;
>>
>> +enum drm_scaling_filters {
>> +     DRM_SCALING_FILTER_DEFAULT,
>> +     DRM_SCALING_FILTER_MEDIUM,
>> +     DRM_SCALING_FILTER_BILINEAR,
>> +     DRM_SCALING_FILTER_NN,
>> +     DRM_SCALING_FILTER_NN_IS_ONLY,
>> +     DRM_SCALING_FILTER_EDGE_ENHANCE,
> This one looks to be missing a stringified version below.
Oh yes, it did. Thanks for pointing this out.
>   Just wanted
> to jump in early and suggest dropping it from the scaling filter enum.
> Edge enhancement is orthogonal to the mode used for scaling, at least
> on komeda HW, so we wouldn't be able to make the best use of the
> property in its current form.
Yes, Ville also suggested similar, I guess we can start with the smaller 
set.
>
>> +     DRM_SCALING_FILTER_INVALID,
>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +     { DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +     { DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +     { DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +     { DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +     { DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +     { DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>   static inline int64_t U642I64(uint64_t val)
>>   {
>>        return (int64_t)*((int64_t *)&val);
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>         */
>>        u32 target_vblank;
>>
>> +     /**
>> +      * @scaling_filter:
>> +      *
>> +      * Scaling filter mode to be applied
>> +      */
>> +     u32 scaling_filter;
>> +
>>        /**
>>         * @async_flip:
>>         *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>         */
>>        struct drm_property *modifiers_property;
>>
>> +     /**
>> +      * @scaling_filter_property: CRTC property to apply a particular filter
> A scaling filter option can also be useful for scaling individual
> planes, do you have any plans to extend the property's applications
> in that direction?

Yes, the second stage of development should contain the plane level 
filtering too, but as you know, that would be a complex thing to handle, 
as planes apply filter pre-blending. We need to discus that (in a 
parallel thread :)) how should that look like.

- Shashank

>
>> +      * while scaling in panel fitter mode.
>> +      */
>> +     struct drm_property *scaling_filter_property;
>> +
>>        /* cursor size */
>>        uint32_t cursor_width, cursor_height;
>>
>>
>
> --
> Mihail
>
>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Sharma, Shashank]

Hello Harry,

Thanks for your comments, please find mine inline.

On 10/22/2019 7:36 PM, Harry Wentland wrote:
>
> On 2019-10-22 8:20 a.m., Ville Syrjälä wrote:
>> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>>> This patch adds a scaling filter mode porperty
>>> to allow:
>>> - A driver/HW to showcase it's scaling filter capabilities.
>>> - A userspace to pick a desired effect while scaling.
>>>
>>> This option will be particularly useful in the scenarios where
>>> Integer mode scaling is possible, and a UI client wants to pick
>>> filters like Nearest-neighbor applied for non-blurry outputs.
>>>
>>> There was a RFC patch series published, to discus the request to enable
>>> Integer mode scaling by some of the gaming communities, which can be
>>> found here:
>>> https://patchwork.freedesktop.org/series/66175/
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> ---
>>>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>>   include/drm/drm_mode_config.h     |  6 ++++++
>>>   3 files changed, 36 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>>> index 0d466d3b0809..883329453a86 100644
>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>>   		return ret;
>>>   	} else if (property == config->prop_vrr_enabled) {
>>>   		state->vrr_enabled = val;
>>> +	} else if (property == config->scaling_filter_property) {
>>> +		state->scaling_filter = val;
>>>   	} else if (property == config->degamma_lut_property) {
>>>   		ret = drm_atomic_replace_property_blob_from_id(dev,
>>>   					&state->degamma_lut,
>>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>>   		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>>   	else if (property == config->prop_out_fence_ptr)
>>>   		*val = 0;
>>> +	else if (property == config->scaling_filter_property)
>>> +		*val = state->scaling_filter;
>>>   	else if (crtc->funcs->atomic_get_property)
>>>   		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>>   	else
>>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>>> index 5e9b15a0e8c5..94c5509474a8 100644
>>> --- a/include/drm/drm_crtc.h
>>> +++ b/include/drm/drm_crtc.h
>>> @@ -58,6 +58,25 @@ struct device_node;
>>>   struct dma_fence;
>>>   struct edid;
>>>   
>>> +enum drm_scaling_filters {
>>> +	DRM_SCALING_FILTER_DEFAULT,
>>> +	DRM_SCALING_FILTER_MEDIUM,
>>> +	DRM_SCALING_FILTER_BILINEAR,
>>> +	DRM_SCALING_FILTER_NN,
>> Please use real words.
>>
>>> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> Not a big fan of this. I'd just add the explicit nearest filter
>> and leave the decision whether to use it to userspace.
>>
>>> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>>> +	DRM_SCALING_FILTER_INVALID,
>> That invalid enum value seems entirely pointless.
>>
>> This set of filters is pretty arbitrary. It doesn't even cover all
>> Intel hw. I would probably just leave it at "default+linear+nearest"
>> initially. Exposing more vague hw specific choices needs more though,
>> and I'd prefer not to spend those brain cells until a real use case
>> emerges.
>>
> FWIW, AMD HW allows us to specify a number of horizontal and vertical
> taps for scaling. Number of taps are limited by our linebuffer size. The
> default is 4 in each dimension but you could have 2 v_taps and 4 h_taps
> if your're running a large horizontal resolution on some ASICs.
>
> I'm not sure it makes sense to define filters here that aren't used. It
> sounds like default and nearest neighbour would suffice for now in order
> to support integer scaling.

Agree, this seems to be a consistent feedback from the community, I will 
probably start with a smaller set of most popular/used ones.

- Shashank

>
> Harry
>
>>> +};
>>> +
>>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>>> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>>> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>>> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>>> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>>> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>>> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>>> +};
>>> +
>>>   static inline int64_t U642I64(uint64_t val)
>>>   {
>>>   	return (int64_t)*((int64_t *)&val);
>>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>>   	 */
>>>   	u32 target_vblank;
>>>   
>>> +	/**
>>> +	 * @scaling_filter:
>>> +	 *
>>> +	 * Scaling filter mode to be applied
>>> +	 */
>>> +	u32 scaling_filter;
>> We have an enum so should use it. The documentation should probably
>> point out that this applies to full crtc scaling only, not plane
>> scaling.
>>
>>> +
>>>   	/**
>>>   	 * @async_flip:
>>>   	 *
>>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>>> index 3bcbe30339f0..efd6fd55770f 100644
>>> --- a/include/drm/drm_mode_config.h
>>> +++ b/include/drm/drm_mode_config.h
>>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>>   	 */
>>>   	struct drm_property *modifiers_property;
>>>   
>>> +	/**
>>> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>>> +	 * while scaling in panel fitter mode.
>>> +	 */
>>> +	struct drm_property *scaling_filter_property;
>>> +
>>>   	/* cursor size */
>>>   	uint32_t cursor_width, cursor_height;
>>>   
>>> -- 
>>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Tue, Oct 22, 2019 at 08:51:20PM +0530, Sharma, Shashank wrote:
> 
> On 10/22/2019 5:56 PM, Ville Syrjälä wrote:
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> This patch adds a scaling filter mode porperty
> >> to allow:
> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> - A userspace to pick a desired effect while scaling.
> >>
> >> This option will be particularly useful in the scenarios where
> >> Integer mode scaling is possible, and a UI client wants to pick
> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >>
> >> There was a RFC patch series published, to discus the request to enable
> >> Integer mode scaling by some of the gaming communities, which can be
> >> found here:
> >> https://patchwork.freedesktop.org/series/66175/
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >>   include/drm/drm_mode_config.h     |  6 ++++++
> >>   3 files changed, 36 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 0d466d3b0809..883329453a86 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>   		return ret;
> >>   	} else if (property == config->prop_vrr_enabled) {
> >>   		state->vrr_enabled = val;
> >> +	} else if (property == config->scaling_filter_property) {
> >> +		state->scaling_filter = val;
> >>   	} else if (property == config->degamma_lut_property) {
> >>   		ret = drm_atomic_replace_property_blob_from_id(dev,
> >>   					&state->degamma_lut,
> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >>   		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >>   	else if (property == config->prop_out_fence_ptr)
> >>   		*val = 0;
> >> +	else if (property == config->scaling_filter_property)
> >> +		*val = state->scaling_filter;
> >>   	else if (crtc->funcs->atomic_get_property)
> >>   		return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >>   	else
> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -58,6 +58,25 @@ struct device_node;
> >>   struct dma_fence;
> >>   struct edid;
> >>   
> >> +enum drm_scaling_filters {
> >> +	DRM_SCALING_FILTER_DEFAULT,
> >> +	DRM_SCALING_FILTER_MEDIUM,
> >> +	DRM_SCALING_FILTER_BILINEAR,
> >> +	DRM_SCALING_FILTER_NN,
> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> +	DRM_SCALING_FILTER_INVALID,
> >> +};
> >> +
> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> >> +};
> >> +
> >>   static inline int64_t U642I64(uint64_t val)
> >>   {
> >>   	return (int64_t)*((int64_t *)&val);
> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >>   	 */
> >>   	u32 target_vblank;
> >>   
> >> +	/**
> >> +	 * @scaling_filter:
> >> +	 *
> >> +	 * Scaling filter mode to be applied
> >> +	 */
> >> +	u32 scaling_filter;
> >> +
> >>   	/**
> >>   	 * @async_flip:
> >>   	 *
> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> >> index 3bcbe30339f0..efd6fd55770f 100644
> >> --- a/include/drm/drm_mode_config.h
> >> +++ b/include/drm/drm_mode_config.h
> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
> >>   	 */
> >>   	struct drm_property *modifiers_property;
> >>   
> >> +	/**
> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> >> +	 * while scaling in panel fitter mode.
> >> +	 */
> >> +	struct drm_property *scaling_filter_property;
> > This needs to per-crtc/plane.
> 
> I am not getting this, why ? It's not different than any other CRTC 
> property like gamma/CSC etc, where we just attach them to corresponding 
> CRTC, isn't it ?

Different crtcs/planes can have different capabilities, and so not all
of them may want to expose all the possible filters.

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

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Tue, Oct 22, 2019 at 02:06:22PM +0000, Harry Wentland wrote:
> 
> 
> On 2019-10-22 8:20 a.m., Ville Syrjälä wrote:
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> This patch adds a scaling filter mode porperty
> >> to allow:
> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> - A userspace to pick a desired effect while scaling.
> >>
> >> This option will be particularly useful in the scenarios where
> >> Integer mode scaling is possible, and a UI client wants to pick
> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >>
> >> There was a RFC patch series published, to discus the request to enable
> >> Integer mode scaling by some of the gaming communities, which can be
> >> found here:
> >> https://patchwork.freedesktop.org/series/66175/
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >>  include/drm/drm_mode_config.h     |  6 ++++++
> >>  3 files changed, 36 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 0d466d3b0809..883329453a86 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>  		return ret;
> >>  	} else if (property == config->prop_vrr_enabled) {
> >>  		state->vrr_enabled = val;
> >> +	} else if (property == config->scaling_filter_property) {
> >> +		state->scaling_filter = val;
> >>  	} else if (property == config->degamma_lut_property) {
> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >>  					&state->degamma_lut,
> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >>  	else if (property == config->prop_out_fence_ptr)
> >>  		*val = 0;
> >> +	else if (property == config->scaling_filter_property)
> >> +		*val = state->scaling_filter;
> >>  	else if (crtc->funcs->atomic_get_property)
> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >>  	else
> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -58,6 +58,25 @@ struct device_node;
> >>  struct dma_fence;
> >>  struct edid;
> >>  
> >> +enum drm_scaling_filters {
> >> +	DRM_SCALING_FILTER_DEFAULT,
> >> +	DRM_SCALING_FILTER_MEDIUM,
> >> +	DRM_SCALING_FILTER_BILINEAR,
> >> +	DRM_SCALING_FILTER_NN,
> > 
> > Please use real words.
> > 
> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> > 
> > Not a big fan of this. I'd just add the explicit nearest filter
> > and leave the decision whether to use it to userspace.
> > 
> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> +	DRM_SCALING_FILTER_INVALID,
> > 
> > That invalid enum value seems entirely pointless.
> > 
> > This set of filters is pretty arbitrary. It doesn't even cover all
> > Intel hw. I would probably just leave it at "default+linear+nearest"
> > initially. Exposing more vague hw specific choices needs more though,
> > and I'd prefer not to spend those brain cells until a real use case
> > emerges.
> > 
> 
> FWIW, AMD HW allows us to specify a number of horizontal and vertical
> taps for scaling. Number of taps are limited by our linebuffer size. The
> default is 4 in each dimension but you could have 2 v_taps and 4 h_taps
> if your're running a large horizontal resolution on some ASICs.
> 
> I'm not sure it makes sense to define filters here that aren't used. It
> sounds like default and nearest neighbour would suffice for now in order
> to support integer scaling.

Yeah, even linear is somewhat questionable since we don't have clear
need for it. Although given that it is well defined it's much less
of a problem than a bunch of the other proposed filters.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.IGT: failure for Add scaling filters in DRM layer

[Patchwork]

== Series Details ==

Series: Add scaling filters in DRM layer
URL   : https://patchwork.freedesktop.org/series/68378/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7149_full -> Patchwork_14916_full
====================================================

Summary
-------

  **FAILURE**

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

  

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-iclb:         [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb1/igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb1/igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping.html

  
#### Suppressed ####

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

  * igt@kms_big_fb@linear-8bpp-rotate-270:
    - {shard-tglb}:       [SKIP][3] ([fdo#111614]) -> [INCOMPLETE][4] +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-tglb6/igt@kms_big_fb@linear-8bpp-rotate-270.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-tglb6/igt@kms_big_fb@linear-8bpp-rotate-270.html

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-a:
    - {shard-tglb}:       [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-tglb6/igt@kms_busy@extended-pageflip-hang-oldfb-render-a.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-tglb1/igt@kms_busy@extended-pageflip-hang-oldfb-render-a.html

  * igt@kms_ccs@pipe-c-bad-rotation-90:
    - {shard-tglb}:       [SKIP][7] ([fdo#111595]) -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-tglb1/igt@kms_ccs@pipe-c-bad-rotation-90.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-tglb6/igt@kms_ccs@pipe-c-bad-rotation-90.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - {shard-tglb}:       [PASS][9] -> [INCOMPLETE][10] +15 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-tglb7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-tglb4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_cursor@pipe-b-primary-size-128:
    - {shard-tglb}:       NOTRUN -> [INCOMPLETE][11] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-tglb2/igt@kms_plane_cursor@pipe-b-primary-size-128.html

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - {shard-tglb}:       [PASS][12] -> [DMESG-WARN][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-tglb8/igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-tglb3/igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping.html

  
New tests
---------

  New tests have been introduced between CI_DRM_7149_full and Patchwork_14916_full:

### New Piglit tests (1) ###

  * spec@arb_gpu_shader5@texturegather@vs-rg-1-uint-cubearray:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vcs1-none:
    - shard-iclb:         [PASS][14] -> [SKIP][15] ([fdo#109276] / [fdo#112080])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb4/igt@gem_ctx_isolation@vcs1-none.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb3/igt@gem_ctx_isolation@vcs1-none.html

  * igt@gem_ctx_isolation@vecs0-s3:
    - shard-kbl:          [PASS][16] -> [INCOMPLETE][17] ([fdo#103665])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-kbl1/igt@gem_ctx_isolation@vecs0-s3.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-kbl4/igt@gem_ctx_isolation@vecs0-s3.html
    - shard-iclb:         [PASS][18] -> [INCOMPLETE][19] ([fdo#107713] / [fdo#109100])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb8/igt@gem_ctx_isolation@vecs0-s3.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb8/igt@gem_ctx_isolation@vecs0-s3.html

  * igt@gem_ctx_shared@exec-shared-gtt-bsd2:
    - shard-iclb:         [PASS][20] -> [SKIP][21] ([fdo#109276])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb4/igt@gem_ctx_shared@exec-shared-gtt-bsd2.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb3/igt@gem_ctx_shared@exec-shared-gtt-bsd2.html

  * igt@gem_ctx_switch@legacy-default-queue:
    - shard-iclb:         [PASS][22] -> [INCOMPLETE][23] ([fdo#107713]) +23 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb3/igt@gem_ctx_switch@legacy-default-queue.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb1/igt@gem_ctx_switch@legacy-default-queue.html

  * igt@gem_eio@reset-stress:
    - shard-snb:          [PASS][24] -> [FAIL][25] ([fdo#109661])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-snb1/igt@gem_eio@reset-stress.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-snb6/igt@gem_eio@reset-stress.html

  * igt@gem_linear_blits@interruptible:
    - shard-apl:          [PASS][26] -> [INCOMPLETE][27] ([fdo#103927] / [fdo#112067])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-apl5/igt@gem_linear_blits@interruptible.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-apl6/igt@gem_linear_blits@interruptible.html

  * igt@gem_userptr_blits@dmabuf-unsync:
    - shard-hsw:          [PASS][28] -> [DMESG-WARN][29] ([fdo#111870])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-hsw8/igt@gem_userptr_blits@dmabuf-unsync.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-hsw5/igt@gem_userptr_blits@dmabuf-unsync.html

  * igt@gem_wait@wait-vcs0:
    - shard-skl:          [PASS][30] -> [DMESG-WARN][31] ([fdo#106107])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-skl6/igt@gem_wait@wait-vcs0.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-skl4/igt@gem_wait@wait-vcs0.html

  * igt@i915_pm_rpm@legacy-planes-dpms:
    - shard-iclb:         [PASS][32] -> [INCOMPLETE][33] ([fdo#107713] / [fdo#108840] / [fdo#109960])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb1/igt@i915_pm_rpm@legacy-planes-dpms.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb5/igt@i915_pm_rpm@legacy-planes-dpms.html

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-apl:          [PASS][34] -> [INCOMPLETE][35] ([fdo#103927]) +1 similar issue
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-apl2/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-apl7/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][36] -> [FAIL][37] ([fdo#105363])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-glk2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-hsw:          [PASS][38] -> [INCOMPLETE][39] ([fdo#103540])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-hsw1/igt@kms_flip@flip-vs-suspend-interruptible.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-hsw1/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [PASS][40] -> [DMESG-WARN][41] ([fdo#108566]) +3 similar issues
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-apl2/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-onoff:
    - shard-iclb:         [PASS][42] -> [INCOMPLETE][43] ([fdo#106978] / [fdo#107713])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-onoff.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb4/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-onoff.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][44] -> [FAIL][45] ([fdo#108145]) +1 similar issue
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

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

  * igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping:
    - shard-iclb:         [PASS][48] -> [INCOMPLETE][49] ([fdo#107713] / [fdo#110041])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb2/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb2/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html

  * igt@kms_psr@psr2_sprite_mmap_cpu:
    - shard-iclb:         [PASS][50] -> [SKIP][51] ([fdo#109441]) +1 similar issue
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_cpu.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb8/igt@kms_psr@psr2_sprite_mmap_cpu.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][52] ([fdo#108566]) -> [PASS][53] +3 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-apl3/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd:
    - shard-iclb:         [SKIP][54] ([fdo#111325]) -> [PASS][55] +1 similar issue
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb4/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb5/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html

  * igt@gem_exec_suspend@basic-s3:
    - shard-skl:          [INCOMPLETE][56] ([fdo#104108]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-skl5/igt@gem_exec_suspend@basic-s3.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-skl10/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_fence_thrash@bo-write-verify-threaded-y:
    - shard-iclb:         [INCOMPLETE][58] ([fdo#107713] / [fdo#109100]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb7/igt@gem_fence_thrash@bo-write-verify-threaded-y.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb6/igt@gem_fence_thrash@bo-write-verify-threaded-y.html

  * igt@i915_suspend@sysfs-reader:
    - shard-kbl:          [INCOMPLETE][60] ([fdo#103665] / [fdo#108767]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-kbl4/igt@i915_suspend@sysfs-reader.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-kbl2/igt@i915_suspend@sysfs-reader.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][62] ([fdo#105363]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-skl4/igt@kms_flip@flip-vs-expired-vblank.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-skl6/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][64] ([fdo#108145]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  
#### Warnings ####

  * igt@kms_big_fb@linear-8bpp-rotate-270:
    - shard-iclb:         [SKIP][66] ([fdo#110725]) -> [INCOMPLETE][67] ([fdo#107713]) +2 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7149/shard-iclb7/igt@kms_big_fb@linear-8bpp-rotate-270.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14916/shard-iclb7/igt@kms_big_fb@linear-8bpp-rotate-270.html

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

  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#109960]: https://bugs.freedesktop.org/show_bug.cgi?id=109960
  [fdo#110041]: https://bugs.freedesktop.org/show_bug.cgi?id=110041
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110725]: https://bugs.freedesktop.org/show_bug.cgi?id=110725
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111595]: https://bugs.freedesktop.org/show_bug.cgi?id=111595
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111747]: https://bugs.freedesktop.org/show_bug.cgi?id=111747
  [fdo#111832]: https://bugs.freedesktop.org/show_bug.cgi?id=111832
  [fdo#111850]: https://bugs.freedesktop.org/show_bug.cgi?id=111850
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [fdo#112035 ]: https://bugs.freedesktop.org/show_bug.cgi?id=112035 
  [fdo#112067]: https://bugs.freedesktop.org/show_bug.cgi?id=112067
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7149 -> Patchwork_14916

  CI-20190529: 20190529
  CI_DRM_7149: 44d2673963fd51800de361180554f1dad14f25a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5235: da9abbab69be80dd00812a4607a4ea2dffcc4544 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14916: 5fced141d9e40100a93d2851a9860ef2172b620e @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 2697 bytes --]

On Tue, 22 Oct 2019 20:48:02 +0530
"Sharma, Shashank" <shashank.sharma@intel.com> wrote:

> Hello Ville,
> 
> Thanks for the comments, mine inline.
> 
> 
> On 10/22/2019 5:50 PM, Ville Syrjälä wrote:
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:  
> >> This patch adds a scaling filter mode porperty
> >> to allow:
> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> - A userspace to pick a desired effect while scaling.
> >>
> >> This option will be particularly useful in the scenarios where
> >> Integer mode scaling is possible, and a UI client wants to pick
> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >>
> >> There was a RFC patch series published, to discus the request to enable
> >> Integer mode scaling by some of the gaming communities, which can be
> >> found here:
> >> https://patchwork.freedesktop.org/series/66175/
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >>   include/drm/drm_mode_config.h     |  6 ++++++
> >>   3 files changed, 36 insertions(+)

...

> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -58,6 +58,25 @@ struct device_node;
> >>   struct dma_fence;
> >>   struct edid;
> >>   
> >> +enum drm_scaling_filters {
> >> +	DRM_SCALING_FILTER_DEFAULT,
> >> +	DRM_SCALING_FILTER_MEDIUM,
> >> +	DRM_SCALING_FILTER_BILINEAR,
> >> +	DRM_SCALING_FILTER_NN,  
> > Please use real words.  
> Yes, I saw that coming. It was getting difficult with the 80 char stuff, 
> it was even more difficult while using it :). But let me see how better 
> can I do here.
> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,  
> > Not a big fan of this. I'd just add the explicit nearest filter
> > and leave the decision whether to use it to userspace.  
> Agree, that's also one option. I was thinking to make it convenient for 
> userspace,  For example if a compositor just want to checkout NN only 
> when its beneficial (like old gaming scenarios) but doesn't have enough 
> information (or intent), it can leave it to kernel too. But I agree, 
> this can cause corner cases. Let's discuss and see if we need it at all, 
> as you mentioned.

Hi,

how could the kernel have more information or knowledge of intent than
a display server? I cannot see how, because everything the kernel gets
comes through the display server.

I agree with Ville here.


Thanks,
pq

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Wed, Oct 23, 2019 at 10:34:05AM +0300, Pekka Paalanen wrote:
> On Tue, 22 Oct 2019 20:48:02 +0530
> "Sharma, Shashank" <shashank.sharma@intel.com> wrote:
> 
> > Hello Ville,
> > 
> > Thanks for the comments, mine inline.
> > 
> > 
> > On 10/22/2019 5:50 PM, Ville Syrjälä wrote:
> > > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:  
> > >> This patch adds a scaling filter mode porperty
> > >> to allow:
> > >> - A driver/HW to showcase it's scaling filter capabilities.
> > >> - A userspace to pick a desired effect while scaling.
> > >>
> > >> This option will be particularly useful in the scenarios where
> > >> Integer mode scaling is possible, and a UI client wants to pick
> > >> filters like Nearest-neighbor applied for non-blurry outputs.
> > >>
> > >> There was a RFC patch series published, to discus the request to enable
> > >> Integer mode scaling by some of the gaming communities, which can be
> > >> found here:
> > >> https://patchwork.freedesktop.org/series/66175/
> > >>
> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> > >>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> > >>   include/drm/drm_mode_config.h     |  6 ++++++
> > >>   3 files changed, 36 insertions(+)
> 
> ...
> 
> > >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > >> index 5e9b15a0e8c5..94c5509474a8 100644
> > >> --- a/include/drm/drm_crtc.h
> > >> +++ b/include/drm/drm_crtc.h
> > >> @@ -58,6 +58,25 @@ struct device_node;
> > >>   struct dma_fence;
> > >>   struct edid;
> > >>   
> > >> +enum drm_scaling_filters {
> > >> +	DRM_SCALING_FILTER_DEFAULT,
> > >> +	DRM_SCALING_FILTER_MEDIUM,
> > >> +	DRM_SCALING_FILTER_BILINEAR,
> > >> +	DRM_SCALING_FILTER_NN,  
> > > Please use real words.  
> > Yes, I saw that coming. It was getting difficult with the 80 char stuff, 
> > it was even more difficult while using it :). But let me see how better 
> > can I do here.
> > >> +	DRM_SCALING_FILTER_NN_IS_ONLY,  
> > > Not a big fan of this. I'd just add the explicit nearest filter
> > > and leave the decision whether to use it to userspace.  
> > Agree, that's also one option. I was thinking to make it convenient for 
> > userspace,  For example if a compositor just want to checkout NN only 
> > when its beneficial (like old gaming scenarios) but doesn't have enough 
> > information (or intent), it can leave it to kernel too. But I agree, 
> > this can cause corner cases. Let's discuss and see if we need it at all, 
> > as you mentioned.
> 
> Hi,
> 
> how could the kernel have more information or knowledge of intent than
> a display server? I cannot see how, because everything the kernel gets
> comes through the display server.

The only exception is due to that annoying scaling mode property.
Currently userspace just tells the kernel "center vs. fullscreen vs.
aspect" so in theory only the kernel knows the actual output size
(though userspace should be able to calculate it as well). 

But maybe we can just avoid that little issue by also exposing the
"TV" margin properties on local panels, at which point userspace
can be more explicit about what it wants.

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

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Wed, Oct 23, 2019 at 10:34:05AM +0300, Pekka Paalanen wrote:
> On Tue, 22 Oct 2019 20:48:02 +0530
> "Sharma, Shashank" <shashank.sharma@intel.com> wrote:
> 
> > Hello Ville,
> > 
> > Thanks for the comments, mine inline.
> > 
> > 
> > On 10/22/2019 5:50 PM, Ville Syrjälä wrote:
> > > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:  
> > >> This patch adds a scaling filter mode porperty
> > >> to allow:
> > >> - A driver/HW to showcase it's scaling filter capabilities.
> > >> - A userspace to pick a desired effect while scaling.
> > >>
> > >> This option will be particularly useful in the scenarios where
> > >> Integer mode scaling is possible, and a UI client wants to pick
> > >> filters like Nearest-neighbor applied for non-blurry outputs.
> > >>
> > >> There was a RFC patch series published, to discus the request to enable
> > >> Integer mode scaling by some of the gaming communities, which can be
> > >> found here:
> > >> https://patchwork.freedesktop.org/series/66175/
> > >>
> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> > >>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> > >>   include/drm/drm_mode_config.h     |  6 ++++++
> > >>   3 files changed, 36 insertions(+)
> 
> ...
> 
> > >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > >> index 5e9b15a0e8c5..94c5509474a8 100644
> > >> --- a/include/drm/drm_crtc.h
> > >> +++ b/include/drm/drm_crtc.h
> > >> @@ -58,6 +58,25 @@ struct device_node;
> > >>   struct dma_fence;
> > >>   struct edid;
> > >>   
> > >> +enum drm_scaling_filters {
> > >> +	DRM_SCALING_FILTER_DEFAULT,
> > >> +	DRM_SCALING_FILTER_MEDIUM,
> > >> +	DRM_SCALING_FILTER_BILINEAR,
> > >> +	DRM_SCALING_FILTER_NN,  
> > > Please use real words.  
> > Yes, I saw that coming. It was getting difficult with the 80 char stuff, 
> > it was even more difficult while using it :). But let me see how better 
> > can I do here.
> > >> +	DRM_SCALING_FILTER_NN_IS_ONLY,  
> > > Not a big fan of this. I'd just add the explicit nearest filter
> > > and leave the decision whether to use it to userspace.  
> > Agree, that's also one option. I was thinking to make it convenient for 
> > userspace,  For example if a compositor just want to checkout NN only 
> > when its beneficial (like old gaming scenarios) but doesn't have enough 
> > information (or intent), it can leave it to kernel too. But I agree, 
> > this can cause corner cases. Let's discuss and see if we need it at all, 
> > as you mentioned.
> 
> Hi,
> 
> how could the kernel have more information or knowledge of intent than
> a display server? I cannot see how, because everything the kernel gets
> comes through the display server.

The only exception is due to that annoying scaling mode property.
Currently userspace just tells the kernel "center vs. fullscreen vs.
aspect" so in theory only the kernel knows the actual output size
(though userspace should be able to calculate it as well). 

But maybe we can just avoid that little issue by also exposing the
"TV" margin properties on local panels, at which point userspace
can be more explicit about what it wants.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Ville Syrjälä]

On Wed, Oct 23, 2019 at 10:34:05AM +0300, Pekka Paalanen wrote:
> On Tue, 22 Oct 2019 20:48:02 +0530
> "Sharma, Shashank" <shashank.sharma@intel.com> wrote:
> 
> > Hello Ville,
> > 
> > Thanks for the comments, mine inline.
> > 
> > 
> > On 10/22/2019 5:50 PM, Ville Syrjälä wrote:
> > > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:  
> > >> This patch adds a scaling filter mode porperty
> > >> to allow:
> > >> - A driver/HW to showcase it's scaling filter capabilities.
> > >> - A userspace to pick a desired effect while scaling.
> > >>
> > >> This option will be particularly useful in the scenarios where
> > >> Integer mode scaling is possible, and a UI client wants to pick
> > >> filters like Nearest-neighbor applied for non-blurry outputs.
> > >>
> > >> There was a RFC patch series published, to discus the request to enable
> > >> Integer mode scaling by some of the gaming communities, which can be
> > >> found here:
> > >> https://patchwork.freedesktop.org/series/66175/
> > >>
> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> > >>   include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> > >>   include/drm/drm_mode_config.h     |  6 ++++++
> > >>   3 files changed, 36 insertions(+)
> 
> ...
> 
> > >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > >> index 5e9b15a0e8c5..94c5509474a8 100644
> > >> --- a/include/drm/drm_crtc.h
> > >> +++ b/include/drm/drm_crtc.h
> > >> @@ -58,6 +58,25 @@ struct device_node;
> > >>   struct dma_fence;
> > >>   struct edid;
> > >>   
> > >> +enum drm_scaling_filters {
> > >> +	DRM_SCALING_FILTER_DEFAULT,
> > >> +	DRM_SCALING_FILTER_MEDIUM,
> > >> +	DRM_SCALING_FILTER_BILINEAR,
> > >> +	DRM_SCALING_FILTER_NN,  
> > > Please use real words.  
> > Yes, I saw that coming. It was getting difficult with the 80 char stuff, 
> > it was even more difficult while using it :). But let me see how better 
> > can I do here.
> > >> +	DRM_SCALING_FILTER_NN_IS_ONLY,  
> > > Not a big fan of this. I'd just add the explicit nearest filter
> > > and leave the decision whether to use it to userspace.  
> > Agree, that's also one option. I was thinking to make it convenient for 
> > userspace,  For example if a compositor just want to checkout NN only 
> > when its beneficial (like old gaming scenarios) but doesn't have enough 
> > information (or intent), it can leave it to kernel too. But I agree, 
> > this can cause corner cases. Let's discuss and see if we need it at all, 
> > as you mentioned.
> 
> Hi,
> 
> how could the kernel have more information or knowledge of intent than
> a display server? I cannot see how, because everything the kernel gets
> comes through the display server.

The only exception is due to that annoying scaling mode property.
Currently userspace just tells the kernel "center vs. fullscreen vs.
aspect" so in theory only the kernel knows the actual output size
(though userspace should be able to calculate it as well). 

But maybe we can just avoid that little issue by also exposing the
"TV" margin properties on local panels, at which point userspace
can be more explicit about what it wants.

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

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>> 
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>> 
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>> 
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>
> Two things:
>
> - needs real property docs for this in the kms property chapter
> - would be good if we could somehow subsume the panel fitter prop into
>   this? Or at least document possible interactions with that.
>
> Plus userspace ofc, recommended best practices is to add a Link: tag
> pointing at the userspace patch series/merge request directly to the patch
> that adds the new uapi.

I think Link: should only be used for referencing the patch that became
the commit?

References: perhaps?

>
> Cheers, Daniel
>> ---
>>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>  include/drm/drm_mode_config.h     |  6 ++++++
>>  3 files changed, 36 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>  		return ret;
>>  	} else if (property == config->prop_vrr_enabled) {
>>  		state->vrr_enabled = val;
>> +	} else if (property == config->scaling_filter_property) {
>> +		state->scaling_filter = val;
>>  	} else if (property == config->degamma_lut_property) {
>>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>>  					&state->degamma_lut,
>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>  	else if (property == config->prop_out_fence_ptr)
>>  		*val = 0;
>> +	else if (property == config->scaling_filter_property)
>> +		*val = state->scaling_filter;
>>  	else if (crtc->funcs->atomic_get_property)
>>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>  	else
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>  struct dma_fence;
>>  struct edid;
>>  
>> +enum drm_scaling_filters {
>> +	DRM_SCALING_FILTER_DEFAULT,
>> +	DRM_SCALING_FILTER_MEDIUM,
>> +	DRM_SCALING_FILTER_BILINEAR,
>> +	DRM_SCALING_FILTER_NN,
>> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +	DRM_SCALING_FILTER_INVALID,
>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>  static inline int64_t U642I64(uint64_t val)
>>  {
>>  	return (int64_t)*((int64_t *)&val);
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>  	 */
>>  	u32 target_vblank;
>>  
>> +	/**
>> +	 * @scaling_filter:
>> +	 *
>> +	 * Scaling filter mode to be applied
>> +	 */
>> +	u32 scaling_filter;
>> +
>>  	/**
>>  	 * @async_flip:
>>  	 *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>  	 */
>>  	struct drm_property *modifiers_property;
>>  
>> +	/**
>> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> +	 * while scaling in panel fitter mode.
>> +	 */
>> +	struct drm_property *scaling_filter_property;
>> +
>>  	/* cursor size */
>>  	uint32_t cursor_width, cursor_height;
>>  
>> -- 
>> 2.17.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>> 
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>> 
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>> 
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>
> Two things:
>
> - needs real property docs for this in the kms property chapter
> - would be good if we could somehow subsume the panel fitter prop into
>   this? Or at least document possible interactions with that.
>
> Plus userspace ofc, recommended best practices is to add a Link: tag
> pointing at the userspace patch series/merge request directly to the patch
> that adds the new uapi.

I think Link: should only be used for referencing the patch that became
the commit?

References: perhaps?

>
> Cheers, Daniel
>> ---
>>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>  include/drm/drm_mode_config.h     |  6 ++++++
>>  3 files changed, 36 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>  		return ret;
>>  	} else if (property == config->prop_vrr_enabled) {
>>  		state->vrr_enabled = val;
>> +	} else if (property == config->scaling_filter_property) {
>> +		state->scaling_filter = val;
>>  	} else if (property == config->degamma_lut_property) {
>>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>>  					&state->degamma_lut,
>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>  	else if (property == config->prop_out_fence_ptr)
>>  		*val = 0;
>> +	else if (property == config->scaling_filter_property)
>> +		*val = state->scaling_filter;
>>  	else if (crtc->funcs->atomic_get_property)
>>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>  	else
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>  struct dma_fence;
>>  struct edid;
>>  
>> +enum drm_scaling_filters {
>> +	DRM_SCALING_FILTER_DEFAULT,
>> +	DRM_SCALING_FILTER_MEDIUM,
>> +	DRM_SCALING_FILTER_BILINEAR,
>> +	DRM_SCALING_FILTER_NN,
>> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +	DRM_SCALING_FILTER_INVALID,
>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>  static inline int64_t U642I64(uint64_t val)
>>  {
>>  	return (int64_t)*((int64_t *)&val);
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>  	 */
>>  	u32 target_vblank;
>>  
>> +	/**
>> +	 * @scaling_filter:
>> +	 *
>> +	 * Scaling filter mode to be applied
>> +	 */
>> +	u32 scaling_filter;
>> +
>>  	/**
>>  	 * @async_flip:
>>  	 *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>  	 */
>>  	struct drm_property *modifiers_property;
>>  
>> +	/**
>> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> +	 * while scaling in panel fitter mode.
>> +	 */
>> +	struct drm_property *scaling_filter_property;
>> +
>>  	/* cursor size */
>>  	uint32_t cursor_width, cursor_height;
>>  
>> -- 
>> 2.17.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> This patch adds a scaling filter mode porperty
>> to allow:
>> - A driver/HW to showcase it's scaling filter capabilities.
>> - A userspace to pick a desired effect while scaling.
>> 
>> This option will be particularly useful in the scenarios where
>> Integer mode scaling is possible, and a UI client wants to pick
>> filters like Nearest-neighbor applied for non-blurry outputs.
>> 
>> There was a RFC patch series published, to discus the request to enable
>> Integer mode scaling by some of the gaming communities, which can be
>> found here:
>> https://patchwork.freedesktop.org/series/66175/
>> 
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>
> Two things:
>
> - needs real property docs for this in the kms property chapter
> - would be good if we could somehow subsume the panel fitter prop into
>   this? Or at least document possible interactions with that.
>
> Plus userspace ofc, recommended best practices is to add a Link: tag
> pointing at the userspace patch series/merge request directly to the patch
> that adds the new uapi.

I think Link: should only be used for referencing the patch that became
the commit?

References: perhaps?

>
> Cheers, Daniel
>> ---
>>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>>  include/drm/drm_mode_config.h     |  6 ++++++
>>  3 files changed, 36 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index 0d466d3b0809..883329453a86 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>>  		return ret;
>>  	} else if (property == config->prop_vrr_enabled) {
>>  		state->vrr_enabled = val;
>> +	} else if (property == config->scaling_filter_property) {
>> +		state->scaling_filter = val;
>>  	} else if (property == config->degamma_lut_property) {
>>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>>  					&state->degamma_lut,
>> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>>  	else if (property == config->prop_out_fence_ptr)
>>  		*val = 0;
>> +	else if (property == config->scaling_filter_property)
>> +		*val = state->scaling_filter;
>>  	else if (crtc->funcs->atomic_get_property)
>>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>>  	else
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 5e9b15a0e8c5..94c5509474a8 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -58,6 +58,25 @@ struct device_node;
>>  struct dma_fence;
>>  struct edid;
>>  
>> +enum drm_scaling_filters {
>> +	DRM_SCALING_FILTER_DEFAULT,
>> +	DRM_SCALING_FILTER_MEDIUM,
>> +	DRM_SCALING_FILTER_BILINEAR,
>> +	DRM_SCALING_FILTER_NN,
>> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> +	DRM_SCALING_FILTER_INVALID,
>> +};
>> +
>> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> +};
>> +
>>  static inline int64_t U642I64(uint64_t val)
>>  {
>>  	return (int64_t)*((int64_t *)&val);
>> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>>  	 */
>>  	u32 target_vblank;
>>  
>> +	/**
>> +	 * @scaling_filter:
>> +	 *
>> +	 * Scaling filter mode to be applied
>> +	 */
>> +	u32 scaling_filter;
>> +
>>  	/**
>>  	 * @async_flip:
>>  	 *
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index 3bcbe30339f0..efd6fd55770f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -914,6 +914,12 @@ struct drm_mode_config {
>>  	 */
>>  	struct drm_property *modifiers_property;
>>  
>> +	/**
>> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> +	 * while scaling in panel fitter mode.
>> +	 */
>> +	struct drm_property *scaling_filter_property;
>> +
>>  	/* cursor size */
>>  	uint32_t cursor_width, cursor_height;
>>  
>> -- 
>> 2.17.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Daniel Vetter]

On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> This patch adds a scaling filter mode porperty
> >> to allow:
> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> - A userspace to pick a desired effect while scaling.
> >> 
> >> This option will be particularly useful in the scenarios where
> >> Integer mode scaling is possible, and a UI client wants to pick
> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >> 
> >> There was a RFC patch series published, to discus the request to enable
> >> Integer mode scaling by some of the gaming communities, which can be
> >> found here:
> >> https://patchwork.freedesktop.org/series/66175/
> >> 
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >
> > Two things:
> >
> > - needs real property docs for this in the kms property chapter
> > - would be good if we could somehow subsume the panel fitter prop into
> >   this? Or at least document possible interactions with that.
> >
> > Plus userspace ofc, recommended best practices is to add a Link: tag
> > pointing at the userspace patch series/merge request directly to the patch
> > that adds the new uapi.
> 
> I think Link: should only be used for referencing the patch that became
> the commit?

Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
herd here. But yeah maybe we want something else.
> 
> References: perhaps?

Or Userspace: to make it real obvious?
-Daniel

> 
> >
> > Cheers, Daniel
> >> ---
> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >>  include/drm/drm_mode_config.h     |  6 ++++++
> >>  3 files changed, 36 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 0d466d3b0809..883329453a86 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>  		return ret;
> >>  	} else if (property == config->prop_vrr_enabled) {
> >>  		state->vrr_enabled = val;
> >> +	} else if (property == config->scaling_filter_property) {
> >> +		state->scaling_filter = val;
> >>  	} else if (property == config->degamma_lut_property) {
> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >>  					&state->degamma_lut,
> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >>  	else if (property == config->prop_out_fence_ptr)
> >>  		*val = 0;
> >> +	else if (property == config->scaling_filter_property)
> >> +		*val = state->scaling_filter;
> >>  	else if (crtc->funcs->atomic_get_property)
> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >>  	else
> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -58,6 +58,25 @@ struct device_node;
> >>  struct dma_fence;
> >>  struct edid;
> >>  
> >> +enum drm_scaling_filters {
> >> +	DRM_SCALING_FILTER_DEFAULT,
> >> +	DRM_SCALING_FILTER_MEDIUM,
> >> +	DRM_SCALING_FILTER_BILINEAR,
> >> +	DRM_SCALING_FILTER_NN,
> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> +	DRM_SCALING_FILTER_INVALID,
> >> +};
> >> +
> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> >> +};
> >> +
> >>  static inline int64_t U642I64(uint64_t val)
> >>  {
> >>  	return (int64_t)*((int64_t *)&val);
> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >>  	 */
> >>  	u32 target_vblank;
> >>  
> >> +	/**
> >> +	 * @scaling_filter:
> >> +	 *
> >> +	 * Scaling filter mode to be applied
> >> +	 */
> >> +	u32 scaling_filter;
> >> +
> >>  	/**
> >>  	 * @async_flip:
> >>  	 *
> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> >> index 3bcbe30339f0..efd6fd55770f 100644
> >> --- a/include/drm/drm_mode_config.h
> >> +++ b/include/drm/drm_mode_config.h
> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
> >>  	 */
> >>  	struct drm_property *modifiers_property;
> >>  
> >> +	/**
> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> >> +	 * while scaling in panel fitter mode.
> >> +	 */
> >> +	struct drm_property *scaling_filter_property;
> >> +
> >>  	/* cursor size */
> >>  	uint32_t cursor_width, cursor_height;
> >>  
> >> -- 
> >> 2.17.1
> >> 
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Daniel Vetter]

On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> This patch adds a scaling filter mode porperty
> >> to allow:
> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> - A userspace to pick a desired effect while scaling.
> >> 
> >> This option will be particularly useful in the scenarios where
> >> Integer mode scaling is possible, and a UI client wants to pick
> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >> 
> >> There was a RFC patch series published, to discus the request to enable
> >> Integer mode scaling by some of the gaming communities, which can be
> >> found here:
> >> https://patchwork.freedesktop.org/series/66175/
> >> 
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >
> > Two things:
> >
> > - needs real property docs for this in the kms property chapter
> > - would be good if we could somehow subsume the panel fitter prop into
> >   this? Or at least document possible interactions with that.
> >
> > Plus userspace ofc, recommended best practices is to add a Link: tag
> > pointing at the userspace patch series/merge request directly to the patch
> > that adds the new uapi.
> 
> I think Link: should only be used for referencing the patch that became
> the commit?

Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
herd here. But yeah maybe we want something else.
> 
> References: perhaps?

Or Userspace: to make it real obvious?
-Daniel

> 
> >
> > Cheers, Daniel
> >> ---
> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >>  include/drm/drm_mode_config.h     |  6 ++++++
> >>  3 files changed, 36 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index 0d466d3b0809..883329453a86 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>  		return ret;
> >>  	} else if (property == config->prop_vrr_enabled) {
> >>  		state->vrr_enabled = val;
> >> +	} else if (property == config->scaling_filter_property) {
> >> +		state->scaling_filter = val;
> >>  	} else if (property == config->degamma_lut_property) {
> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >>  					&state->degamma_lut,
> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >>  	else if (property == config->prop_out_fence_ptr)
> >>  		*val = 0;
> >> +	else if (property == config->scaling_filter_property)
> >> +		*val = state->scaling_filter;
> >>  	else if (crtc->funcs->atomic_get_property)
> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >>  	else
> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -58,6 +58,25 @@ struct device_node;
> >>  struct dma_fence;
> >>  struct edid;
> >>  
> >> +enum drm_scaling_filters {
> >> +	DRM_SCALING_FILTER_DEFAULT,
> >> +	DRM_SCALING_FILTER_MEDIUM,
> >> +	DRM_SCALING_FILTER_BILINEAR,
> >> +	DRM_SCALING_FILTER_NN,
> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> +	DRM_SCALING_FILTER_INVALID,
> >> +};
> >> +
> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> >> +};
> >> +
> >>  static inline int64_t U642I64(uint64_t val)
> >>  {
> >>  	return (int64_t)*((int64_t *)&val);
> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >>  	 */
> >>  	u32 target_vblank;
> >>  
> >> +	/**
> >> +	 * @scaling_filter:
> >> +	 *
> >> +	 * Scaling filter mode to be applied
> >> +	 */
> >> +	u32 scaling_filter;
> >> +
> >>  	/**
> >>  	 * @async_flip:
> >>  	 *
> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> >> index 3bcbe30339f0..efd6fd55770f 100644
> >> --- a/include/drm/drm_mode_config.h
> >> +++ b/include/drm/drm_mode_config.h
> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
> >>  	 */
> >>  	struct drm_property *modifiers_property;
> >>  
> >> +	/**
> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> >> +	 * while scaling in panel fitter mode.
> >> +	 */
> >> +	struct drm_property *scaling_filter_property;
> >> +
> >>  	/* cursor size */
> >>  	uint32_t cursor_width, cursor_height;
> >>  
> >> -- 
> >> 2.17.1
> >> 
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> This patch adds a scaling filter mode porperty
>> >> to allow:
>> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> - A userspace to pick a desired effect while scaling.
>> >> 
>> >> This option will be particularly useful in the scenarios where
>> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> 
>> >> There was a RFC patch series published, to discus the request to enable
>> >> Integer mode scaling by some of the gaming communities, which can be
>> >> found here:
>> >> https://patchwork.freedesktop.org/series/66175/
>> >> 
>> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> >
>> > Two things:
>> >
>> > - needs real property docs for this in the kms property chapter
>> > - would be good if we could somehow subsume the panel fitter prop into
>> >   this? Or at least document possible interactions with that.
>> >
>> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> > pointing at the userspace patch series/merge request directly to the patch
>> > that adds the new uapi.
>> 
>> I think Link: should only be used for referencing the patch that became
>> the commit?
>
> Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> herd here. But yeah maybe we want something else.
>> 
>> References: perhaps?
>
> Or Userspace: to make it real obvious?

Works for me, as long as we don't conflate Link. (Maybe Link: should've
been Patch: or Submission: or whatever.)

BR,
Jani.


> -Daniel
>
>> 
>> >
>> > Cheers, Daniel
>> >> ---
>> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>> >>  include/drm/drm_mode_config.h     |  6 ++++++
>> >>  3 files changed, 36 insertions(+)
>> >> 
>> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> index 0d466d3b0809..883329453a86 100644
>> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>> >>  		return ret;
>> >>  	} else if (property == config->prop_vrr_enabled) {
>> >>  		state->vrr_enabled = val;
>> >> +	} else if (property == config->scaling_filter_property) {
>> >> +		state->scaling_filter = val;
>> >>  	} else if (property == config->degamma_lut_property) {
>> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>> >>  					&state->degamma_lut,
>> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>> >>  	else if (property == config->prop_out_fence_ptr)
>> >>  		*val = 0;
>> >> +	else if (property == config->scaling_filter_property)
>> >> +		*val = state->scaling_filter;
>> >>  	else if (crtc->funcs->atomic_get_property)
>> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>> >>  	else
>> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> --- a/include/drm/drm_crtc.h
>> >> +++ b/include/drm/drm_crtc.h
>> >> @@ -58,6 +58,25 @@ struct device_node;
>> >>  struct dma_fence;
>> >>  struct edid;
>> >>  
>> >> +enum drm_scaling_filters {
>> >> +	DRM_SCALING_FILTER_DEFAULT,
>> >> +	DRM_SCALING_FILTER_MEDIUM,
>> >> +	DRM_SCALING_FILTER_BILINEAR,
>> >> +	DRM_SCALING_FILTER_NN,
>> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> +	DRM_SCALING_FILTER_INVALID,
>> >> +};
>> >> +
>> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> >> +};
>> >> +
>> >>  static inline int64_t U642I64(uint64_t val)
>> >>  {
>> >>  	return (int64_t)*((int64_t *)&val);
>> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>> >>  	 */
>> >>  	u32 target_vblank;
>> >>  
>> >> +	/**
>> >> +	 * @scaling_filter:
>> >> +	 *
>> >> +	 * Scaling filter mode to be applied
>> >> +	 */
>> >> +	u32 scaling_filter;
>> >> +
>> >>  	/**
>> >>  	 * @async_flip:
>> >>  	 *
>> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> >> index 3bcbe30339f0..efd6fd55770f 100644
>> >> --- a/include/drm/drm_mode_config.h
>> >> +++ b/include/drm/drm_mode_config.h
>> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
>> >>  	 */
>> >>  	struct drm_property *modifiers_property;
>> >>  
>> >> +	/**
>> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> >> +	 * while scaling in panel fitter mode.
>> >> +	 */
>> >> +	struct drm_property *scaling_filter_property;
>> >> +
>> >>  	/* cursor size */
>> >>  	uint32_t cursor_width, cursor_height;
>> >>  
>> >> -- 
>> >> 2.17.1
>> >> 
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> This patch adds a scaling filter mode porperty
>> >> to allow:
>> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> - A userspace to pick a desired effect while scaling.
>> >> 
>> >> This option will be particularly useful in the scenarios where
>> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> 
>> >> There was a RFC patch series published, to discus the request to enable
>> >> Integer mode scaling by some of the gaming communities, which can be
>> >> found here:
>> >> https://patchwork.freedesktop.org/series/66175/
>> >> 
>> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> >
>> > Two things:
>> >
>> > - needs real property docs for this in the kms property chapter
>> > - would be good if we could somehow subsume the panel fitter prop into
>> >   this? Or at least document possible interactions with that.
>> >
>> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> > pointing at the userspace patch series/merge request directly to the patch
>> > that adds the new uapi.
>> 
>> I think Link: should only be used for referencing the patch that became
>> the commit?
>
> Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> herd here. But yeah maybe we want something else.
>> 
>> References: perhaps?
>
> Or Userspace: to make it real obvious?

Works for me, as long as we don't conflate Link. (Maybe Link: should've
been Patch: or Submission: or whatever.)

BR,
Jani.


> -Daniel
>
>> 
>> >
>> > Cheers, Daniel
>> >> ---
>> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>> >>  include/drm/drm_mode_config.h     |  6 ++++++
>> >>  3 files changed, 36 insertions(+)
>> >> 
>> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> index 0d466d3b0809..883329453a86 100644
>> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>> >>  		return ret;
>> >>  	} else if (property == config->prop_vrr_enabled) {
>> >>  		state->vrr_enabled = val;
>> >> +	} else if (property == config->scaling_filter_property) {
>> >> +		state->scaling_filter = val;
>> >>  	} else if (property == config->degamma_lut_property) {
>> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>> >>  					&state->degamma_lut,
>> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>> >>  	else if (property == config->prop_out_fence_ptr)
>> >>  		*val = 0;
>> >> +	else if (property == config->scaling_filter_property)
>> >> +		*val = state->scaling_filter;
>> >>  	else if (crtc->funcs->atomic_get_property)
>> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>> >>  	else
>> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> --- a/include/drm/drm_crtc.h
>> >> +++ b/include/drm/drm_crtc.h
>> >> @@ -58,6 +58,25 @@ struct device_node;
>> >>  struct dma_fence;
>> >>  struct edid;
>> >>  
>> >> +enum drm_scaling_filters {
>> >> +	DRM_SCALING_FILTER_DEFAULT,
>> >> +	DRM_SCALING_FILTER_MEDIUM,
>> >> +	DRM_SCALING_FILTER_BILINEAR,
>> >> +	DRM_SCALING_FILTER_NN,
>> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> +	DRM_SCALING_FILTER_INVALID,
>> >> +};
>> >> +
>> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> >> +};
>> >> +
>> >>  static inline int64_t U642I64(uint64_t val)
>> >>  {
>> >>  	return (int64_t)*((int64_t *)&val);
>> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>> >>  	 */
>> >>  	u32 target_vblank;
>> >>  
>> >> +	/**
>> >> +	 * @scaling_filter:
>> >> +	 *
>> >> +	 * Scaling filter mode to be applied
>> >> +	 */
>> >> +	u32 scaling_filter;
>> >> +
>> >>  	/**
>> >>  	 * @async_flip:
>> >>  	 *
>> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> >> index 3bcbe30339f0..efd6fd55770f 100644
>> >> --- a/include/drm/drm_mode_config.h
>> >> +++ b/include/drm/drm_mode_config.h
>> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
>> >>  	 */
>> >>  	struct drm_property *modifiers_property;
>> >>  
>> >> +	/**
>> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> >> +	 * while scaling in panel fitter mode.
>> >> +	 */
>> >> +	struct drm_property *scaling_filter_property;
>> >> +
>> >>  	/* cursor size */
>> >>  	uint32_t cursor_width, cursor_height;
>> >>  
>> >> -- 
>> >> 2.17.1
>> >> 
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Daniel Vetter]

On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
> On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> >> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> >> This patch adds a scaling filter mode porperty
> >> >> to allow:
> >> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> >> - A userspace to pick a desired effect while scaling.
> >> >> 
> >> >> This option will be particularly useful in the scenarios where
> >> >> Integer mode scaling is possible, and a UI client wants to pick
> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >> >> 
> >> >> There was a RFC patch series published, to discus the request to enable
> >> >> Integer mode scaling by some of the gaming communities, which can be
> >> >> found here:
> >> >> https://patchwork.freedesktop.org/series/66175/
> >> >> 
> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> >
> >> > Two things:
> >> >
> >> > - needs real property docs for this in the kms property chapter
> >> > - would be good if we could somehow subsume the panel fitter prop into
> >> >   this? Or at least document possible interactions with that.
> >> >
> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
> >> > pointing at the userspace patch series/merge request directly to the patch
> >> > that adds the new uapi.
> >> 
> >> I think Link: should only be used for referencing the patch that became
> >> the commit?
> >
> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> > herd here. But yeah maybe we want something else.
> >> 
> >> References: perhaps?
> >
> > Or Userspace: to make it real obvious?
> 
> Works for me, as long as we don't conflate Link. (Maybe Link: should've
> been Patch: or Submission: or whatever.)

The Powers That Be (kernel summit) decided that it's Link for reference to
a http link of an archive of some sort that contains the msg-id.

Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
lore.k.o. They'll backfill the entire archive too. Imo still better we
point at patchwork, since that's where hopefully the CI result hangs out.
-Daniel

> 
> BR,
> Jani.
> 
> 
> > -Daniel
> >
> >> 
> >> >
> >> > Cheers, Daniel
> >> >> ---
> >> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >> >>  include/drm/drm_mode_config.h     |  6 ++++++
> >> >>  3 files changed, 36 insertions(+)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> index 0d466d3b0809..883329453a86 100644
> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >> >>  		return ret;
> >> >>  	} else if (property == config->prop_vrr_enabled) {
> >> >>  		state->vrr_enabled = val;
> >> >> +	} else if (property == config->scaling_filter_property) {
> >> >> +		state->scaling_filter = val;
> >> >>  	} else if (property == config->degamma_lut_property) {
> >> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >> >>  					&state->degamma_lut,
> >> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >> >>  	else if (property == config->prop_out_fence_ptr)
> >> >>  		*val = 0;
> >> >> +	else if (property == config->scaling_filter_property)
> >> >> +		*val = state->scaling_filter;
> >> >>  	else if (crtc->funcs->atomic_get_property)
> >> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >> >>  	else
> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> >> --- a/include/drm/drm_crtc.h
> >> >> +++ b/include/drm/drm_crtc.h
> >> >> @@ -58,6 +58,25 @@ struct device_node;
> >> >>  struct dma_fence;
> >> >>  struct edid;
> >> >>  
> >> >> +enum drm_scaling_filters {
> >> >> +	DRM_SCALING_FILTER_DEFAULT,
> >> >> +	DRM_SCALING_FILTER_MEDIUM,
> >> >> +	DRM_SCALING_FILTER_BILINEAR,
> >> >> +	DRM_SCALING_FILTER_NN,
> >> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> >> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> >> +	DRM_SCALING_FILTER_INVALID,
> >> >> +};
> >> >> +
> >> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> >> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> >> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> >> >> +};
> >> >> +
> >> >>  static inline int64_t U642I64(uint64_t val)
> >> >>  {
> >> >>  	return (int64_t)*((int64_t *)&val);
> >> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >> >>  	 */
> >> >>  	u32 target_vblank;
> >> >>  
> >> >> +	/**
> >> >> +	 * @scaling_filter:
> >> >> +	 *
> >> >> +	 * Scaling filter mode to be applied
> >> >> +	 */
> >> >> +	u32 scaling_filter;
> >> >> +
> >> >>  	/**
> >> >>  	 * @async_flip:
> >> >>  	 *
> >> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> >> >> index 3bcbe30339f0..efd6fd55770f 100644
> >> >> --- a/include/drm/drm_mode_config.h
> >> >> +++ b/include/drm/drm_mode_config.h
> >> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
> >> >>  	 */
> >> >>  	struct drm_property *modifiers_property;
> >> >>  
> >> >> +	/**
> >> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> >> >> +	 * while scaling in panel fitter mode.
> >> >> +	 */
> >> >> +	struct drm_property *scaling_filter_property;
> >> >> +
> >> >>  	/* cursor size */
> >> >>  	uint32_t cursor_width, cursor_height;
> >> >>  
> >> >> -- 
> >> >> 2.17.1
> >> >> 
> >> >> _______________________________________________
> >> >> Intel-gfx mailing list
> >> >> Intel-gfx@lists.freedesktop.org
> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Daniel Vetter]

On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
> On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
> >> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
> >> >> This patch adds a scaling filter mode porperty
> >> >> to allow:
> >> >> - A driver/HW to showcase it's scaling filter capabilities.
> >> >> - A userspace to pick a desired effect while scaling.
> >> >> 
> >> >> This option will be particularly useful in the scenarios where
> >> >> Integer mode scaling is possible, and a UI client wants to pick
> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
> >> >> 
> >> >> There was a RFC patch series published, to discus the request to enable
> >> >> Integer mode scaling by some of the gaming communities, which can be
> >> >> found here:
> >> >> https://patchwork.freedesktop.org/series/66175/
> >> >> 
> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> >
> >> > Two things:
> >> >
> >> > - needs real property docs for this in the kms property chapter
> >> > - would be good if we could somehow subsume the panel fitter prop into
> >> >   this? Or at least document possible interactions with that.
> >> >
> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
> >> > pointing at the userspace patch series/merge request directly to the patch
> >> > that adds the new uapi.
> >> 
> >> I think Link: should only be used for referencing the patch that became
> >> the commit?
> >
> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
> > herd here. But yeah maybe we want something else.
> >> 
> >> References: perhaps?
> >
> > Or Userspace: to make it real obvious?
> 
> Works for me, as long as we don't conflate Link. (Maybe Link: should've
> been Patch: or Submission: or whatever.)

The Powers That Be (kernel summit) decided that it's Link for reference to
a http link of an archive of some sort that contains the msg-id.

Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
lore.k.o. They'll backfill the entire archive too. Imo still better we
point at patchwork, since that's where hopefully the CI result hangs out.
-Daniel

> 
> BR,
> Jani.
> 
> 
> > -Daniel
> >
> >> 
> >> >
> >> > Cheers, Daniel
> >> >> ---
> >> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
> >> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
> >> >>  include/drm/drm_mode_config.h     |  6 ++++++
> >> >>  3 files changed, 36 insertions(+)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> index 0d466d3b0809..883329453a86 100644
> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >> >>  		return ret;
> >> >>  	} else if (property == config->prop_vrr_enabled) {
> >> >>  		state->vrr_enabled = val;
> >> >> +	} else if (property == config->scaling_filter_property) {
> >> >> +		state->scaling_filter = val;
> >> >>  	} else if (property == config->degamma_lut_property) {
> >> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
> >> >>  					&state->degamma_lut,
> >> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> >> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> >> >>  	else if (property == config->prop_out_fence_ptr)
> >> >>  		*val = 0;
> >> >> +	else if (property == config->scaling_filter_property)
> >> >> +		*val = state->scaling_filter;
> >> >>  	else if (crtc->funcs->atomic_get_property)
> >> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
> >> >>  	else
> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
> >> >> --- a/include/drm/drm_crtc.h
> >> >> +++ b/include/drm/drm_crtc.h
> >> >> @@ -58,6 +58,25 @@ struct device_node;
> >> >>  struct dma_fence;
> >> >>  struct edid;
> >> >>  
> >> >> +enum drm_scaling_filters {
> >> >> +	DRM_SCALING_FILTER_DEFAULT,
> >> >> +	DRM_SCALING_FILTER_MEDIUM,
> >> >> +	DRM_SCALING_FILTER_BILINEAR,
> >> >> +	DRM_SCALING_FILTER_NN,
> >> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
> >> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
> >> >> +	DRM_SCALING_FILTER_INVALID,
> >> >> +};
> >> >> +
> >> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> >> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> >> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
> >> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
> >> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
> >> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
> >> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
> >> >> +};
> >> >> +
> >> >>  static inline int64_t U642I64(uint64_t val)
> >> >>  {
> >> >>  	return (int64_t)*((int64_t *)&val);
> >> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
> >> >>  	 */
> >> >>  	u32 target_vblank;
> >> >>  
> >> >> +	/**
> >> >> +	 * @scaling_filter:
> >> >> +	 *
> >> >> +	 * Scaling filter mode to be applied
> >> >> +	 */
> >> >> +	u32 scaling_filter;
> >> >> +
> >> >>  	/**
> >> >>  	 * @async_flip:
> >> >>  	 *
> >> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> >> >> index 3bcbe30339f0..efd6fd55770f 100644
> >> >> --- a/include/drm/drm_mode_config.h
> >> >> +++ b/include/drm/drm_mode_config.h
> >> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
> >> >>  	 */
> >> >>  	struct drm_property *modifiers_property;
> >> >>  
> >> >> +	/**
> >> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
> >> >> +	 * while scaling in panel fitter mode.
> >> >> +	 */
> >> >> +	struct drm_property *scaling_filter_property;
> >> >> +
> >> >>  	/* cursor size */
> >> >>  	uint32_t cursor_width, cursor_height;
> >> >>  
> >> >> -- 
> >> >> 2.17.1
> >> >> 
> >> >> _______________________________________________
> >> >> Intel-gfx mailing list
> >> >> Intel-gfx@lists.freedesktop.org
> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
>> On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> >> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> >> This patch adds a scaling filter mode porperty
>> >> >> to allow:
>> >> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> >> - A userspace to pick a desired effect while scaling.
>> >> >> 
>> >> >> This option will be particularly useful in the scenarios where
>> >> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> >> 
>> >> >> There was a RFC patch series published, to discus the request to enable
>> >> >> Integer mode scaling by some of the gaming communities, which can be
>> >> >> found here:
>> >> >> https://patchwork.freedesktop.org/series/66175/
>> >> >> 
>> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> >> >
>> >> > Two things:
>> >> >
>> >> > - needs real property docs for this in the kms property chapter
>> >> > - would be good if we could somehow subsume the panel fitter prop into
>> >> >   this? Or at least document possible interactions with that.
>> >> >
>> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> >> > pointing at the userspace patch series/merge request directly to the patch
>> >> > that adds the new uapi.
>> >> 
>> >> I think Link: should only be used for referencing the patch that became
>> >> the commit?
>> >
>> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
>> > herd here. But yeah maybe we want something else.
>> >> 
>> >> References: perhaps?
>> >
>> > Or Userspace: to make it real obvious?
>> 
>> Works for me, as long as we don't conflate Link. (Maybe Link: should've
>> been Patch: or Submission: or whatever.)
>
> The Powers That Be (kernel summit) decided that it's Link for reference to
> a http link of an archive of some sort that contains the msg-id.

Oh totally aware of that. But hopefully they also assert that Link is
not also for something else.

> Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
> lore.k.o. They'll backfill the entire archive too. Imo still better we
> point at patchwork, since that's where hopefully the CI result hangs out.

Thanks, appreciated. Agreed on patchwork.

BR,
Jani.


> -Daniel
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> > -Daniel
>> >
>> >> 
>> >> >
>> >> > Cheers, Daniel
>> >> >> ---
>> >> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>> >> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>> >> >>  include/drm/drm_mode_config.h     |  6 ++++++
>> >> >>  3 files changed, 36 insertions(+)
>> >> >> 
>> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> index 0d466d3b0809..883329453a86 100644
>> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>> >> >>  		return ret;
>> >> >>  	} else if (property == config->prop_vrr_enabled) {
>> >> >>  		state->vrr_enabled = val;
>> >> >> +	} else if (property == config->scaling_filter_property) {
>> >> >> +		state->scaling_filter = val;
>> >> >>  	} else if (property == config->degamma_lut_property) {
>> >> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>> >> >>  					&state->degamma_lut,
>> >> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>> >> >>  	else if (property == config->prop_out_fence_ptr)
>> >> >>  		*val = 0;
>> >> >> +	else if (property == config->scaling_filter_property)
>> >> >> +		*val = state->scaling_filter;
>> >> >>  	else if (crtc->funcs->atomic_get_property)
>> >> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>> >> >>  	else
>> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> >> --- a/include/drm/drm_crtc.h
>> >> >> +++ b/include/drm/drm_crtc.h
>> >> >> @@ -58,6 +58,25 @@ struct device_node;
>> >> >>  struct dma_fence;
>> >> >>  struct edid;
>> >> >>  
>> >> >> +enum drm_scaling_filters {
>> >> >> +	DRM_SCALING_FILTER_DEFAULT,
>> >> >> +	DRM_SCALING_FILTER_MEDIUM,
>> >> >> +	DRM_SCALING_FILTER_BILINEAR,
>> >> >> +	DRM_SCALING_FILTER_NN,
>> >> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> >> +	DRM_SCALING_FILTER_INVALID,
>> >> >> +};
>> >> >> +
>> >> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> >> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> >> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> >> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> >> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> >> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> >> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> >> >> +};
>> >> >> +
>> >> >>  static inline int64_t U642I64(uint64_t val)
>> >> >>  {
>> >> >>  	return (int64_t)*((int64_t *)&val);
>> >> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>> >> >>  	 */
>> >> >>  	u32 target_vblank;
>> >> >>  
>> >> >> +	/**
>> >> >> +	 * @scaling_filter:
>> >> >> +	 *
>> >> >> +	 * Scaling filter mode to be applied
>> >> >> +	 */
>> >> >> +	u32 scaling_filter;
>> >> >> +
>> >> >>  	/**
>> >> >>  	 * @async_flip:
>> >> >>  	 *
>> >> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> >> >> index 3bcbe30339f0..efd6fd55770f 100644
>> >> >> --- a/include/drm/drm_mode_config.h
>> >> >> +++ b/include/drm/drm_mode_config.h
>> >> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
>> >> >>  	 */
>> >> >>  	struct drm_property *modifiers_property;
>> >> >>  
>> >> >> +	/**
>> >> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> >> >> +	 * while scaling in panel fitter mode.
>> >> >> +	 */
>> >> >> +	struct drm_property *scaling_filter_property;
>> >> >> +
>> >> >>  	/* cursor size */
>> >> >>  	uint32_t cursor_width, cursor_height;
>> >> >>  
>> >> >> -- 
>> >> >> 2.17.1
>> >> >> 
>> >> >> _______________________________________________
>> >> >> Intel-gfx mailing list
>> >> >> Intel-gfx@lists.freedesktop.org
>> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> 
>> >> -- 
>> >> Jani Nikula, Intel Open Source Graphics Center
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
>> On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> >> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> >> This patch adds a scaling filter mode porperty
>> >> >> to allow:
>> >> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> >> - A userspace to pick a desired effect while scaling.
>> >> >> 
>> >> >> This option will be particularly useful in the scenarios where
>> >> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> >> 
>> >> >> There was a RFC patch series published, to discus the request to enable
>> >> >> Integer mode scaling by some of the gaming communities, which can be
>> >> >> found here:
>> >> >> https://patchwork.freedesktop.org/series/66175/
>> >> >> 
>> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> >> >
>> >> > Two things:
>> >> >
>> >> > - needs real property docs for this in the kms property chapter
>> >> > - would be good if we could somehow subsume the panel fitter prop into
>> >> >   this? Or at least document possible interactions with that.
>> >> >
>> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> >> > pointing at the userspace patch series/merge request directly to the patch
>> >> > that adds the new uapi.
>> >> 
>> >> I think Link: should only be used for referencing the patch that became
>> >> the commit?
>> >
>> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
>> > herd here. But yeah maybe we want something else.
>> >> 
>> >> References: perhaps?
>> >
>> > Or Userspace: to make it real obvious?
>> 
>> Works for me, as long as we don't conflate Link. (Maybe Link: should've
>> been Patch: or Submission: or whatever.)
>
> The Powers That Be (kernel summit) decided that it's Link for reference to
> a http link of an archive of some sort that contains the msg-id.

Oh totally aware of that. But hopefully they also assert that Link is
not also for something else.

> Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
> lore.k.o. They'll backfill the entire archive too. Imo still better we
> point at patchwork, since that's where hopefully the CI result hangs out.

Thanks, appreciated. Agreed on patchwork.

BR,
Jani.


> -Daniel
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> > -Daniel
>> >
>> >> 
>> >> >
>> >> > Cheers, Daniel
>> >> >> ---
>> >> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>> >> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>> >> >>  include/drm/drm_mode_config.h     |  6 ++++++
>> >> >>  3 files changed, 36 insertions(+)
>> >> >> 
>> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> index 0d466d3b0809..883329453a86 100644
>> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>> >> >>  		return ret;
>> >> >>  	} else if (property == config->prop_vrr_enabled) {
>> >> >>  		state->vrr_enabled = val;
>> >> >> +	} else if (property == config->scaling_filter_property) {
>> >> >> +		state->scaling_filter = val;
>> >> >>  	} else if (property == config->degamma_lut_property) {
>> >> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>> >> >>  					&state->degamma_lut,
>> >> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>> >> >>  	else if (property == config->prop_out_fence_ptr)
>> >> >>  		*val = 0;
>> >> >> +	else if (property == config->scaling_filter_property)
>> >> >> +		*val = state->scaling_filter;
>> >> >>  	else if (crtc->funcs->atomic_get_property)
>> >> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>> >> >>  	else
>> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> >> --- a/include/drm/drm_crtc.h
>> >> >> +++ b/include/drm/drm_crtc.h
>> >> >> @@ -58,6 +58,25 @@ struct device_node;
>> >> >>  struct dma_fence;
>> >> >>  struct edid;
>> >> >>  
>> >> >> +enum drm_scaling_filters {
>> >> >> +	DRM_SCALING_FILTER_DEFAULT,
>> >> >> +	DRM_SCALING_FILTER_MEDIUM,
>> >> >> +	DRM_SCALING_FILTER_BILINEAR,
>> >> >> +	DRM_SCALING_FILTER_NN,
>> >> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> >> +	DRM_SCALING_FILTER_INVALID,
>> >> >> +};
>> >> >> +
>> >> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> >> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> >> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> >> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> >> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> >> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> >> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> >> >> +};
>> >> >> +
>> >> >>  static inline int64_t U642I64(uint64_t val)
>> >> >>  {
>> >> >>  	return (int64_t)*((int64_t *)&val);
>> >> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>> >> >>  	 */
>> >> >>  	u32 target_vblank;
>> >> >>  
>> >> >> +	/**
>> >> >> +	 * @scaling_filter:
>> >> >> +	 *
>> >> >> +	 * Scaling filter mode to be applied
>> >> >> +	 */
>> >> >> +	u32 scaling_filter;
>> >> >> +
>> >> >>  	/**
>> >> >>  	 * @async_flip:
>> >> >>  	 *
>> >> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> >> >> index 3bcbe30339f0..efd6fd55770f 100644
>> >> >> --- a/include/drm/drm_mode_config.h
>> >> >> +++ b/include/drm/drm_mode_config.h
>> >> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
>> >> >>  	 */
>> >> >>  	struct drm_property *modifiers_property;
>> >> >>  
>> >> >> +	/**
>> >> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> >> >> +	 * while scaling in panel fitter mode.
>> >> >> +	 */
>> >> >> +	struct drm_property *scaling_filter_property;
>> >> >> +
>> >> >>  	/* cursor size */
>> >> >>  	uint32_t cursor_width, cursor_height;
>> >> >>  
>> >> >> -- 
>> >> >> 2.17.1
>> >> >> 
>> >> >> _______________________________________________
>> >> >> Intel-gfx mailing list
>> >> >> Intel-gfx@lists.freedesktop.org
>> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> 
>> >> -- 
>> >> Jani Nikula, Intel Open Source Graphics Center
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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

Re: [Intel-gfx] [PATCH 1/3] drm: Introduce scaling filter mode property

[Jani Nikula]

On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 24, 2019 at 03:12:07PM +0300, Jani Nikula wrote:
>> On Thu, 24 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Wed, Oct 23, 2019 at 03:44:17PM +0300, Jani Nikula wrote:
>> >> On Tue, 22 Oct 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >> > On Tue, Oct 22, 2019 at 03:29:44PM +0530, Shashank Sharma wrote:
>> >> >> This patch adds a scaling filter mode porperty
>> >> >> to allow:
>> >> >> - A driver/HW to showcase it's scaling filter capabilities.
>> >> >> - A userspace to pick a desired effect while scaling.
>> >> >> 
>> >> >> This option will be particularly useful in the scenarios where
>> >> >> Integer mode scaling is possible, and a UI client wants to pick
>> >> >> filters like Nearest-neighbor applied for non-blurry outputs.
>> >> >> 
>> >> >> There was a RFC patch series published, to discus the request to enable
>> >> >> Integer mode scaling by some of the gaming communities, which can be
>> >> >> found here:
>> >> >> https://patchwork.freedesktop.org/series/66175/
>> >> >> 
>> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> >> >
>> >> > Two things:
>> >> >
>> >> > - needs real property docs for this in the kms property chapter
>> >> > - would be good if we could somehow subsume the panel fitter prop into
>> >> >   this? Or at least document possible interactions with that.
>> >> >
>> >> > Plus userspace ofc, recommended best practices is to add a Link: tag
>> >> > pointing at the userspace patch series/merge request directly to the patch
>> >> > that adds the new uapi.
>> >> 
>> >> I think Link: should only be used for referencing the patch that became
>> >> the commit?
>> >
>> > Dave brought up the Link: bikeshed in his uapi rfc, I'm just following the
>> > herd here. But yeah maybe we want something else.
>> >> 
>> >> References: perhaps?
>> >
>> > Or Userspace: to make it real obvious?
>> 
>> Works for me, as long as we don't conflate Link. (Maybe Link: should've
>> been Patch: or Submission: or whatever.)
>
> The Powers That Be (kernel summit) decided that it's Link for reference to
> a http link of an archive of some sort that contains the msg-id.

Oh totally aware of that. But hopefully they also assert that Link is
not also for something else.

> Aside: I asked k.o admins to add dri-devel/amdgpu-dev/intel-gfx to
> lore.k.o. They'll backfill the entire archive too. Imo still better we
> point at patchwork, since that's where hopefully the CI result hangs out.

Thanks, appreciated. Agreed on patchwork.

BR,
Jani.


> -Daniel
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> > -Daniel
>> >
>> >> 
>> >> >
>> >> > Cheers, Daniel
>> >> >> ---
>> >> >>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
>> >> >>  include/drm/drm_crtc.h            | 26 ++++++++++++++++++++++++++
>> >> >>  include/drm/drm_mode_config.h     |  6 ++++++
>> >> >>  3 files changed, 36 insertions(+)
>> >> >> 
>> >> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> index 0d466d3b0809..883329453a86 100644
>> >> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> >> >> @@ -435,6 +435,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>> >> >>  		return ret;
>> >> >>  	} else if (property == config->prop_vrr_enabled) {
>> >> >>  		state->vrr_enabled = val;
>> >> >> +	} else if (property == config->scaling_filter_property) {
>> >> >> +		state->scaling_filter = val;
>> >> >>  	} else if (property == config->degamma_lut_property) {
>> >> >>  		ret = drm_atomic_replace_property_blob_from_id(dev,
>> >> >>  					&state->degamma_lut,
>> >> >> @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>> >> >>  		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>> >> >>  	else if (property == config->prop_out_fence_ptr)
>> >> >>  		*val = 0;
>> >> >> +	else if (property == config->scaling_filter_property)
>> >> >> +		*val = state->scaling_filter;
>> >> >>  	else if (crtc->funcs->atomic_get_property)
>> >> >>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>> >> >>  	else
>> >> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> >> >> index 5e9b15a0e8c5..94c5509474a8 100644
>> >> >> --- a/include/drm/drm_crtc.h
>> >> >> +++ b/include/drm/drm_crtc.h
>> >> >> @@ -58,6 +58,25 @@ struct device_node;
>> >> >>  struct dma_fence;
>> >> >>  struct edid;
>> >> >>  
>> >> >> +enum drm_scaling_filters {
>> >> >> +	DRM_SCALING_FILTER_DEFAULT,
>> >> >> +	DRM_SCALING_FILTER_MEDIUM,
>> >> >> +	DRM_SCALING_FILTER_BILINEAR,
>> >> >> +	DRM_SCALING_FILTER_NN,
>> >> >> +	DRM_SCALING_FILTER_NN_IS_ONLY,
>> >> >> +	DRM_SCALING_FILTER_EDGE_ENHANCE,
>> >> >> +	DRM_SCALING_FILTER_INVALID,
>> >> >> +};
>> >> >> +
>> >> >> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
>> >> >> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
>> >> >> +	{ DRM_SCALING_FILTER_MEDIUM, "Medium" },
>> >> >> +	{ DRM_SCALING_FILTER_BILINEAR, "Bilinear" },
>> >> >> +	{ DRM_SCALING_FILTER_NN, "Nearest Neighbor" },
>> >> >> +	{ DRM_SCALING_FILTER_NN_IS_ONLY, "Integer Mode Scaling" },
>> >> >> +	{ DRM_SCALING_FILTER_INVALID, "Invalid" },
>> >> >> +};
>> >> >> +
>> >> >>  static inline int64_t U642I64(uint64_t val)
>> >> >>  {
>> >> >>  	return (int64_t)*((int64_t *)&val);
>> >> >> @@ -283,6 +302,13 @@ struct drm_crtc_state {
>> >> >>  	 */
>> >> >>  	u32 target_vblank;
>> >> >>  
>> >> >> +	/**
>> >> >> +	 * @scaling_filter:
>> >> >> +	 *
>> >> >> +	 * Scaling filter mode to be applied
>> >> >> +	 */
>> >> >> +	u32 scaling_filter;
>> >> >> +
>> >> >>  	/**
>> >> >>  	 * @async_flip:
>> >> >>  	 *
>> >> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> >> >> index 3bcbe30339f0..efd6fd55770f 100644
>> >> >> --- a/include/drm/drm_mode_config.h
>> >> >> +++ b/include/drm/drm_mode_config.h
>> >> >> @@ -914,6 +914,12 @@ struct drm_mode_config {
>> >> >>  	 */
>> >> >>  	struct drm_property *modifiers_property;
>> >> >>  
>> >> >> +	/**
>> >> >> +	 * @scaling_filter_property: CRTC property to apply a particular filter
>> >> >> +	 * while scaling in panel fitter mode.
>> >> >> +	 */
>> >> >> +	struct drm_property *scaling_filter_property;
>> >> >> +
>> >> >>  	/* cursor size */
>> >> >>  	uint32_t cursor_width, cursor_height;
>> >> >>  
>> >> >> -- 
>> >> >> 2.17.1
>> >> >> 
>> >> >> _______________________________________________
>> >> >> Intel-gfx mailing list
>> >> >> Intel-gfx@lists.freedesktop.org
>> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> 
>> >> -- 
>> >> Jani Nikula, Intel Open Source Graphics Center
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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