[Intel-gfx] [RFC][PATCH 0/5] Introduce drm scaling filter property

[Intel-gfx] [RFC][PATCH 0/5] Introduce drm scaling filter property

[Pankaj Bharadiya]

Integer scaling (IS) is a nearest-neighbor upscaling technique that
simply scales up the existing pixels by an integer (i.e., whole
number) multiplier. Nearest-neighbor (NN) interpolation works by
filling in the missing color values in the upscaled image with that of
the coordinate-mapped nearest source pixel value.

Both IS and NN preserve the clarity of the original image. In
contrast, traditional upscaling algorithms, such as bilinear or
bicubic interpolation, result in blurry upscaled images because they
employ interpolation techniques that smooth out the transition from
one pixel to another.  Therefore, integer scaling is particularly
useful for pixel art games that rely on sharp, blocky images to
deliver their distinctive look.

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 -
  - Introduces new scaling filter property to allow userspace to
    select  the driver's default scaling filter or Nearest-neighbor(NN)
    filter for scaling operations on crtc/plane.
  - Implements and enable integer scaling for i915

Userspace patch series link: TBD.

Thanks to Shashank for initiating this work. His initial RFC can be
found here [1]

[1] https://patchwork.freedesktop.org/patch/337082/

Modifications done in this series -
   - refactored code and incorporated initial review comments and
     added 2 scaling filter types (default and NN) to begin with.
   - added scaling filter property support for planes and new API
     helpers for drivers to setup this property.
   - rewrote code to enable integer scaling and NN filter for i915


Pankaj Bharadiya (5):
  drm: Introduce scaling filter property
  drm/drm-kms.rst: Add Scaling filter property documentation
  drm/i915: Enable scaling filter for plane and pipe
  drm/i915: Introduce scaling filter related registers and bit fields.
  drm/i915/display: Add Nearest-neighbor based integer scaling support

 Documentation/gpu/drm-kms.rst                |   6 ++
 drivers/gpu/drm/drm_atomic_uapi.c            |   8 ++
 drivers/gpu/drm/drm_crtc.c                   |  16 +++
 drivers/gpu/drm/drm_mode_config.c            |  13 +++
 drivers/gpu/drm/drm_plane.c                  |  35 +++++++
 drivers/gpu/drm/i915/display/intel_display.c | 100 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_display.h |   2 +
 drivers/gpu/drm/i915/display/intel_sprite.c  |  32 ++++--
 drivers/gpu/drm/i915/i915_reg.h              |  21 ++++
 include/drm/drm_crtc.h                       |  10 ++
 include/drm/drm_mode_config.h                |   6 ++
 include/drm/drm_plane.h                      |  14 +++
 12 files changed, 252 insertions(+), 11 deletions(-)

-- 
2.23.0

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

[Intel-gfx] [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Pankaj Bharadiya]

Introduce new scaling filter property to allow userspace to select
the driver's default scaling filter or Nearest-neighbor(NN) filter
for upscaling operations on crtc/plane.

Drivers can set up this property for a plane by calling
drm_plane_enable_scaling_filter() and for a CRTC by calling
drm_crtc_enable_scaling_filter().

NN filter works by filling in the missing color values in the upscaled
image with that of the coordinate-mapped nearest source pixel value.

NN filter for integer multiple scaling can be particularly useful for
for pixel art games that rely on sharp, blocky images to deliver their
distinctive look.

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  8 +++++++
 drivers/gpu/drm/drm_crtc.c        | 16 ++++++++++++++
 drivers/gpu/drm/drm_mode_config.c | 13 ++++++++++++
 drivers/gpu/drm/drm_plane.c       | 35 +++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h            | 10 +++++++++
 include/drm/drm_mode_config.h     |  6 ++++++
 include/drm/drm_plane.h           | 14 +++++++++++++
 7 files changed, 102 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index a1e5e262bae2..4e3c1f3176e4 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
@@ -583,6 +587,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
 					sizeof(struct drm_rect),
 					&replaced);
 		return ret;
+	} else if (property == config->scaling_filter_property) {
+		state->scaling_filter = val;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
 				property, val);
@@ -641,6 +647,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 	} else if (property == config->prop_fb_damage_clips) {
 		*val = (state->fb_damage_clips) ?
 			state->fb_damage_clips->base.id : 0;
+	} else if (property == config->scaling_filter_property) {
+		*val = state->scaling_filter;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
 	} else {
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4936e1080e41..1ce7b2ac9eb5 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -748,3 +748,19 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
 
 	return ret;
 }
+
+/**
+ * drm_crtc_enable_scaling_filter - Enables crtc scaling filter property.
+ * @crtc: CRTC on which to enable scaling filter property.
+ *
+ * This function lets driver to enable the scaling filter property on a crtc.
+ */
+void drm_crtc_enable_scaling_filter(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+
+	drm_object_attach_property(&crtc->base,
+				   dev->mode_config.scaling_filter_property,
+				   0);
+}
+EXPORT_SYMBOL(drm_crtc_enable_scaling_filter);
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 08e6eff6a179..1024a8d1cd5d 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -214,6 +214,11 @@ static const struct drm_prop_enum_list drm_plane_type_enum_list[] = {
 	{ DRM_PLANE_TYPE_CURSOR, "Cursor" },
 };
 
+static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
+	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
+	{ DRM_SCALING_FILTER_NEAREST_NEIGHBOR, "Nearest Neighbor" },
+};
+
 static int drm_mode_create_standard_properties(struct drm_device *dev)
 {
 	struct drm_property *prop;
@@ -370,6 +375,14 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.modifiers_property = prop;
 
+	prop = drm_property_create_enum(dev, 0,
+				"SCALING_FILTER",
+				drm_scaling_filter_enum_list,
+				ARRAY_SIZE(drm_scaling_filter_enum_list));
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.scaling_filter_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index d6ad60ab0d38..ace7ee2775c8 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1221,3 +1221,38 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 
 	return ret;
 }
+
+/**
+ * DOC: Scaling filter property
+ *
+ *
+ * SCALING_FILTER:
+ *
+ *	Indicates scaling filter to be used for CRTC/plane scaler
+ *
+ *	The value of this property can be one of the following:
+ *	Default:
+ *		Driver's default scaling filter
+ *	Nearest Neighbor:
+ *		Nearest Neighbor scaling filter
+ *
+ * Drivers can set up this property for a plane by calling
+ * drm_plane_enable_scaling_filter() and for a CRTC by calling
+ * drm_crtc_enable_scaling_filter()
+ */
+
+/**
+ * drm_plane_enable_scaling_filter - Enables plane scaling filter property.
+ * @plane: Plane on which to enable scaling filter property.
+ *
+ * This function lets driver to enable the scaling filter property on a plane.
+ */
+void drm_plane_enable_scaling_filter(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+
+	drm_object_attach_property(&plane->base,
+				   dev->mode_config.scaling_filter_property,
+				   0);
+}
+EXPORT_SYMBOL(drm_plane_enable_scaling_filter);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 59b51a09cae6..770f9328a5ba 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -58,6 +58,7 @@ struct device_node;
 struct dma_fence;
 struct edid;
 
+
 static inline int64_t U642I64(uint64_t val)
 {
 	return (int64_t)*((int64_t *)&val);
@@ -296,6 +297,13 @@ struct drm_crtc_state {
 	 */
 	u32 target_vblank;
 
+	/**
+	 * @scaling_filter:
+	 *
+	 * Scaling filter mode to be applied
+	 */
+	enum drm_scaling_filter scaling_filter;
+
 	/**
 	 * @async_flip:
 	 *
@@ -1266,4 +1274,6 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
 #define drm_for_each_crtc(crtc, dev) \
 	list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
 
+void drm_crtc_enable_scaling_filter(struct drm_crtc *crtc);
+
 #endif /* __DRM_CRTC_H__ */
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 3bcbe30339f0..8c308ae1056d 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/plane property to apply a particular
+	 * filter while scaling.
+	 */
+	struct drm_property *scaling_filter_property;
+
 	/* cursor size */
 	uint32_t cursor_width, cursor_height;
 
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 3f396d94afe4..2bc665cc6071 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -35,6 +35,11 @@ struct drm_crtc;
 struct drm_printer;
 struct drm_modeset_acquire_ctx;
 
+
+enum drm_scaling_filter {
+	DRM_SCALING_FILTER_DEFAULT,
+	DRM_SCALING_FILTER_NEAREST_NEIGHBOR,
+};
 /**
  * struct drm_plane_state - mutable plane state
  *
@@ -214,6 +219,13 @@ struct drm_plane_state {
 	 */
 	bool visible;
 
+	/**
+	 * @scaling_filter:
+	 *
+	 * Scaling filter mode to be applied
+	 */
+	enum drm_scaling_filter scaling_filter;
+
 	/**
 	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
 	 * and for async plane updates.
@@ -862,4 +874,6 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state)
 					state->fb_damage_clips->data : NULL);
 }
 
+void drm_plane_enable_scaling_filter(struct drm_plane *plane);
+
 #endif
-- 
2.23.0

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

[Intel-gfx] [RFC][PATCH 2/5] drm/drm-kms.rst: Add Scaling filter property documentation

[Pankaj Bharadiya]

Add documentation for newly introduced KMS scaling filter property.

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
---
 Documentation/gpu/drm-kms.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 906771e03103..7b71a1e3edda 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -509,6 +509,12 @@ Variable Refresh Properties
 .. kernel-doc:: drivers/gpu/drm/drm_connector.c
    :doc: Variable refresh properties
 
+Scaling Filter Property
+-----------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_plane.c
+   :doc: Scaling filter property
+
 Existing KMS Properties
 -----------------------
 
-- 
2.23.0

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

[Intel-gfx] [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and pipe

[Pankaj Bharadiya]

Attach scaling filter property for crtc and plane and program the
scaler control register for the selected filter type.

This is preparatory patch to enable Nearest-neighbor integer scaling.

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 17 +++++++++++++++--
 drivers/gpu/drm/i915/display/intel_sprite.c  | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_reg.h              |  1 +
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3031e64ee518..b5903ef3c5a0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6242,6 +6242,8 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
+	const struct drm_crtc_state *state = &crtc_state->uapi;
+	u32 scaling_filter = PS_FILTER_MEDIUM;
 	const struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
 
@@ -6258,6 +6260,11 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
 		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
 
+		if (state->scaling_filter ==
+		    DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
+			scaling_filter = PS_FILTER_PROGRAMMED;
+		}
+
 		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
 		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
 
@@ -6268,8 +6275,10 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 
 		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
-				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
+		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
+				  PS_SCALER_EN |
+				  scaling_filter |
+				  scaler_state->scalers[id].mode);
 		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
 				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
 		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
@@ -16695,6 +16704,10 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
 	}
 
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		drm_crtc_enable_scaling_filter(&crtc->base);
+
 	intel_color_init(crtc);
 
 	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 7abeefe8dce5..fd7b31a21723 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -414,6 +414,12 @@ skl_program_scaler(struct intel_plane *plane,
 	u16 y_hphase, uv_rgb_hphase;
 	u16 y_vphase, uv_rgb_vphase;
 	int hscale, vscale;
+	const struct drm_plane_state *state = &plane_state->uapi;
+	u32 scaling_filter = PS_FILTER_MEDIUM;
+
+	if (state->scaling_filter == DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
+		scaling_filter = PS_FILTER_PROGRAMMED;
+	}
 
 	hscale = drm_rect_calc_hscale(&plane_state->uapi.src,
 				      &plane_state->uapi.dst,
@@ -441,7 +447,8 @@ skl_program_scaler(struct intel_plane *plane,
 	}
 
 	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, scaler_id),
-			  PS_SCALER_EN | PS_PLANE_SEL(plane->id) | scaler->mode);
+			  scaling_filter | PS_SCALER_EN |
+			  PS_PLANE_SEL(plane->id) | scaler->mode);
 	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, scaler_id),
 			  PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase));
 	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, scaler_id),
@@ -3104,6 +3111,9 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
 
 	drm_plane_create_zpos_immutable_property(&plane->base, plane_id);
 
+	if (INTEL_GEN(dev_priv) >= 11)
+		drm_plane_enable_scaling_filter(&plane->base);
+
 	drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
 
 	return plane;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f45b5e86ec63..34923b1c284c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7212,6 +7212,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)
-- 
2.23.0

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

[Intel-gfx] [RFC][PATCH 4/5] drm/i915: Introduce scaling filter related registers and bit fields.

[Pankaj Bharadiya]

Introduce scaler registers and bit fields needed to configure the
scaling filter in prgrammed mode and configure scaling filter
coefficients.

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 34923b1c284c..bba4ad3be611 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7289,6 +7289,18 @@ 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_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_COEE_INDEX_AUTO_INC (1 << 10)
+
 #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),       \
@@ -7318,6 +7330,14 @@ enum {
 			_ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A),   \
 			_ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B))
 
+#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_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_2B))
+
 /* legacy palette */
 #define _LGC_PALETTE_A           0x4a000
 #define _LGC_PALETTE_B           0x4a800
-- 
2.23.0

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

[Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Pankaj Bharadiya]

Integer scaling (IS) is a nearest-neighbor upscaling technique that
simply scales up the existing pixels by an integer
(i.e., whole number) multiplier.Nearest-neighbor (NN) interpolation
works by filling in the missing color values in the upscaled image
with that of the coordinate-mapped nearest source pixel value.

Both IS and NN preserve the clarity of the original image. Integer
scaling is particularly useful for pixel art games that rely on
sharp, blocky images to deliver their distinctive look.

Program the scaler filter coefficients to enable the NN filter if
scaling filter property is set to DRM_SCALING_FILTER_NEAREST_NEIGHBOR
and enable integer scaling.

Bspec: 49247

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 83 +++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_display.h |  2 +
 drivers/gpu/drm/i915/display/intel_sprite.c  | 20 +++--
 3 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b5903ef3c5a0..6d5f59203258 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6237,6 +6237,73 @@ void skl_scaler_disable(const struct intel_crtc_state *old_crtc_state)
 		skl_detach_scaler(crtc, i);
 }
 
+/**
+ *  Theory behind setting nearest-neighbor integer scaling:
+ *
+ *  17 phase of 7 taps requires 119 coefficients in 60 dwords per set.
+ *  The letter represents the filter tap (D is the center tap) and the number
+ *  represents the coefficient set for a phase (0-16).
+ *
+ *         +------------+------------------------+------------------------+
+ *         |Index value | Data value coeffient 1 | Data value coeffient 2 |
+ *         +------------+------------------------+------------------------+
+ *         |   00h      |          B0            |          A0            |
+ *         +------------+------------------------+------------------------+
+ *         |   01h      |          D0            |          C0            |
+ *         +------------+------------------------+------------------------+
+ *         |   02h      |          F0            |          E0            |
+ *         +------------+------------------------+------------------------+
+ *         |   03h      |          A1            |          G0            |
+ *         +------------+------------------------+------------------------+
+ *         |   04h      |          C1            |          B1            |
+ *         +------------+------------------------+------------------------+
+ *         |   ...      |          ...           |          ...           |
+ *         +------------+------------------------+------------------------+
+ *         |   38h      |          B16           |          A16           |
+ *         +------------+------------------------+------------------------+
+ *         |   39h      |          D16           |          C16           |
+ *         +------------+------------------------+------------------------+
+ *         |   3Ah      |          F16           |          C16           |
+ *         +------------+------------------------+------------------------+
+ *         |   3Bh      |        Reserved        |          G16           |
+ *         +------------+------------------------+------------------------+
+ *
+ *  To enable nearest-neighbor scaling:  program scaler coefficents with
+ *  the center tap (Dxx) values set to 1 and all other values set to 0 as per
+ *  SCALER_COEFFICIENT_FORMAT
+ *
+ */
+void skl_setup_nearest_neighbor_filter(struct drm_i915_private *dev_priv,
+				  enum pipe pipe, int scaler_id)
+{
+
+	int coeff = 0;
+	int phase = 0;
+	int tap;
+	int val = 0;
+
+	/*enable the index auto increment.*/
+	intel_de_write_fw(dev_priv, SKL_PS_COEF_INDEX_SET0(pipe, scaler_id),
+			  _PS_COEE_INDEX_AUTO_INC);
+
+	for (phase = 0; phase < 17; phase++) {
+		for (tap = 0; tap < 7; tap++) {
+			coeff++;
+			if (tap == 3)
+				val = (phase % 2) ? (0x800) : (0x800 << 16);
+
+			if (coeff % 2 == 0) {
+				intel_de_write_fw(dev_priv, SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);
+				val = 0;
+			}
+
+		}
+
+	}
+
+	intel_de_write_fw(dev_priv, SKL_PS_COEF_DATA_SET0(pipe, scaler_id), 0);
+}
+
 static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -6260,9 +6327,23 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
 		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
 		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
 
+		id = scaler_state->scaler_id;
+
 		if (state->scaling_filter ==
 		    DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
 			scaling_filter = PS_FILTER_PROGRAMMED;
+			skl_setup_nearest_neighbor_filter(dev_priv, pipe, id);
+
+			/* Make the scaling window size to integer multiple of
+			 * source.
+			 *
+			 * TODO: Should userspace take desision to round
+			 * scaling window to integer multiple?
+			 */
+			pfit_w = rounddown(pfit_w,
+					   (crtc_state->pipe_src_w << 16));
+			pfit_h = rounddown(pfit_h,
+					   (crtc_state->pipe_src_h << 16));
 		}
 
 		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
@@ -6271,8 +6352,6 @@ static void skl_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);
 
-		id = scaler_state->scaler_id;
-
 		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
 		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index f92efbbec838..49f58d3c98fe 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -586,6 +586,8 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
 u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 void skl_scaler_disable(const struct intel_crtc_state *old_crtc_state);
+void skl_setup_nearest_neighbor_filter(struct drm_i915_private *dev_priv,
+				  enum pipe pipe, int scaler_id);
 void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state);
 u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 			const struct intel_plane_state *plane_state);
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index fd7b31a21723..5bef5c031374 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -415,18 +415,26 @@ skl_program_scaler(struct intel_plane *plane,
 	u16 y_vphase, uv_rgb_vphase;
 	int hscale, vscale;
 	const struct drm_plane_state *state = &plane_state->uapi;
+	u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
+	u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
 	u32 scaling_filter = PS_FILTER_MEDIUM;
+	struct drm_rect dst;
 
 	if (state->scaling_filter == DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
 		scaling_filter = PS_FILTER_PROGRAMMED;
+		skl_setup_nearest_neighbor_filter(dev_priv, pipe, scaler_id);
+
+		/* Make the scaling window size to integer multiple of source
+		 * TODO: Should userspace take desision to round scaling window
+		 * to integer multiple?
+		 */
+		crtc_w = rounddown(crtc_w, src_w);
+		crtc_h = rounddown(crtc_h, src_h);
 	}
 
-	hscale = drm_rect_calc_hscale(&plane_state->uapi.src,
-				      &plane_state->uapi.dst,
-				      0, INT_MAX);
-	vscale = drm_rect_calc_vscale(&plane_state->uapi.src,
-				      &plane_state->uapi.dst,
-				      0, INT_MAX);
+	drm_rect_init(&dst, crtc_x, crtc_y, crtc_w, crtc_h);
+	hscale = drm_rect_calc_hscale(&plane_state->uapi.src, &dst, 0, INT_MAX);
+	vscale = drm_rect_calc_vscale(&plane_state->uapi.src, &dst, 0, INT_MAX);
 
 	/* TODO: handle sub-pixel coordinates */
 	if (intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier) &&
-- 
2.23.0

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Introduce drm scaling filter property

[Patchwork]

== Series Details ==

Series: Introduce drm scaling filter property
URL   : https://patchwork.freedesktop.org/series/73883/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
21c6a10f68e0 drm: Introduce scaling filter property
-:110: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#110: FILE: drivers/gpu/drm/drm_mode_config.c:379:
+	prop = drm_property_create_enum(dev, 0,
+				"SCALING_FILTER",

-:171: CHECK:LINE_SPACING: Please don't use multiple blank lines
#171: FILE: include/drm/drm_crtc.h:61:
 
+

-:221: CHECK:LINE_SPACING: Please don't use multiple blank lines
#221: FILE: include/drm/drm_plane.h:38:
 
+

total: 0 errors, 0 warnings, 3 checks, 182 lines checked
bce91bc41fe1 drm/drm-kms.rst: Add Scaling filter property documentation
582f010154af drm/i915: Enable scaling filter for plane and pipe
-:56: CHECK:LINE_SPACING: Please don't use multiple blank lines
#56: FILE: drivers/gpu/drm/i915/display/intel_display.c:16707:
 
+

total: 0 errors, 0 warnings, 1 checks, 78 lines checked
1e1be21b4151 drm/i915: Introduce scaling filter related registers and bit fields.
-:41: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects?
#41: FILE: drivers/gpu/drm/i915/i915_reg.h:7333:
+#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_2B))

-:45: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects?
#45: FILE: drivers/gpu/drm/i915/i915_reg.h:7337:
+#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_2B))

total: 0 errors, 0 warnings, 2 checks, 32 lines checked
02e442488928 drm/i915/display: Add Nearest-neighbor based integer scaling support
-:71: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#71: FILE: drivers/gpu/drm/i915/display/intel_display.c:6277:
+void skl_setup_nearest_neighbor_filter(struct drm_i915_private *dev_priv,
+				  enum pipe pipe, int scaler_id)

-:73: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#73: FILE: drivers/gpu/drm/i915/display/intel_display.c:6279:
+{
+

-:90: WARNING:LONG_LINE: line over 100 characters
#90: FILE: drivers/gpu/drm/i915/display/intel_display.c:6296:
+				intel_de_write_fw(dev_priv, SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);

-:94: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#94: FILE: drivers/gpu/drm/i915/display/intel_display.c:6300:
+
+		}

-:96: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#96: FILE: drivers/gpu/drm/i915/display/intel_display.c:6302:
+
+	}

-:146: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#146: FILE: drivers/gpu/drm/i915/display/intel_display.h:590:
+void skl_setup_nearest_neighbor_filter(struct drm_i915_private *dev_priv,
+				  enum pipe pipe, int scaler_id);

total: 0 errors, 1 warnings, 5 checks, 144 lines checked

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

Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Daniel Stone]

Hi,

On Tue, 25 Feb 2020 at 07:17, Pankaj Bharadiya
<pankaj.laxminarayan.bharadiya@intel.com> wrote:
> @@ -415,18 +415,26 @@ skl_program_scaler(struct intel_plane *plane,
>         u16 y_vphase, uv_rgb_vphase;
>         int hscale, vscale;
>         const struct drm_plane_state *state = &plane_state->uapi;
> +       u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
> +       u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
>         u32 scaling_filter = PS_FILTER_MEDIUM;
> +       struct drm_rect dst;
>
>         if (state->scaling_filter == DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
>                 scaling_filter = PS_FILTER_PROGRAMMED;
> +               skl_setup_nearest_neighbor_filter(dev_priv, pipe, scaler_id);
> +
> +               /* Make the scaling window size to integer multiple of source
> +                * TODO: Should userspace take desision to round scaling window
> +                * to integer multiple?
> +                */
> +               crtc_w = rounddown(crtc_w, src_w);
> +               crtc_h = rounddown(crtc_h, src_h);

The kernel should absolutely not be changing the co-ordinates that
userspace requested.

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for Introduce drm scaling filter property

[Patchwork]

== Series Details ==

Series: Introduce drm scaling filter property
URL   : https://patchwork.freedesktop.org/series/73883/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8001 -> Patchwork_16697
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_requests:
    - fi-bwr-2160:        [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8001/fi-bwr-2160/igt@i915_selftest@live_requests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16697/fi-bwr-2160/igt@i915_selftest@live_requests.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][3] -> [FAIL][4] ([fdo#111096] / [i915#323])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8001/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16697/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][5] -> [DMESG-WARN][6] ([i915#44])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8001/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16697/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_self_import@basic-with_one_bo_two_files:
    - fi-tgl-y:           [PASS][7] -> [DMESG-WARN][8] ([CI#94] / [i915#402])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8001/fi-tgl-y/igt@prime_self_import@basic-with_one_bo_two_files.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16697/fi-tgl-y/igt@prime_self_import@basic-with_one_bo_two_files.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8700k:       [INCOMPLETE][9] ([i915#424]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8001/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16697/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_addfb_basic@addfb25-bad-modifier:
    - fi-tgl-y:           [DMESG-WARN][11] ([CI#94] / [i915#402]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8001/fi-tgl-y/igt@kms_addfb_basic@addfb25-bad-modifier.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16697/fi-tgl-y/igt@kms_addfb_basic@addfb25-bad-modifier.html

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

  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [i915#1233]: https://gitlab.freedesktop.org/drm/intel/issues/1233
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424
  [i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44


Participating hosts (52 -> 45)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8001 -> Patchwork_16697

  CI-20190529: 20190529
  CI_DRM_8001: 58d4ab7db0e7eb32167833dd46ed9175d1067c1c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5464: 8cf2f8684992052ab89de1cf328c418224c0c2a7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16697: 02e44248892890033ab6e803e2fd1370604f6b5b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

02e442488928 drm/i915/display: Add Nearest-neighbor based integer scaling support
1e1be21b4151 drm/i915: Introduce scaling filter related registers and bit fields.
582f010154af drm/i915: Enable scaling filter for plane and pipe
bce91bc41fe1 drm/drm-kms.rst: Add Scaling filter property documentation
21c6a10f68e0 drm: Introduce scaling filter property

== Logs ==

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

Re: [Intel-gfx] [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Jani Nikula]

On Tue, 25 Feb 2020, Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

What did Shashank and Ankit do, should one or the other perhaps retain
authorship?

In any case, when taking over code and submitting, you should add your
sign-off *last*. Please see [1] for what Signed-off-by means.

BR,
Jani.


[1] https://developercertificate.org/

-- 
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] [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Laxminarayan Bharadiya, Pankaj]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: 25 February 2020 15:26
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>; daniel@ffwll.ch; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> ville.syrjala@linux.intel.com; airlied@linux.ie;
> maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> mripard@kernel.org; mihail.atanassov@arm.com
> Cc: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>; linux-kernel@vger.kernel.org;
> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: Re: [RFC][PATCH 1/5] drm: Introduce scaling filter property
> 
> On Tue, 25 Feb 2020, Pankaj Bharadiya
> <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> > Signed-off-by: Pankaj Bharadiya
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> What did Shashank and Ankit do, should one or the other perhaps retain
> authorship?

I kind of refactored the code, added plane scaling support added new APIs & documentation
 and rewrote the scaling filter setting logic. Since I made significant changes (IMHO), I thought
of changing the authorship. I spoke with Ankit regarding this during my initial discussion with him.

Will you please review the present RFC and the older one and suggest.  I have
no issues with changing the authorship 😊.
 
> 
> In any case, when taking over code and submitting, you should add your sign-off
> *last*. Please see [1] for what Signed-off-by means.

Was not aware, will do the needful.

Thanks,
Pankaj

> 
> BR,
> Jani.
> 
> 
> [1] https://developercertificate.org/
> 
> --
> 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] [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Jani Nikula]

On Tue, 25 Feb 2020, "Laxminarayan Bharadiya, Pankaj"	<pankaj.laxminarayan.bharadiya@intel.com> wrote:
>> On Tue, 25 Feb 2020, Pankaj Bharadiya
>> <pankaj.laxminarayan.bharadiya@intel.com> wrote:
>> > Signed-off-by: Pankaj Bharadiya
>> > <pankaj.laxminarayan.bharadiya@intel.com>
>> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> 
>> What did Shashank and Ankit do, should one or the other perhaps retain
>> authorship?
>
> I kind of refactored the code, added plane scaling support added new
> APIs & documentation and rewrote the scaling filter setting
> logic. Since I made significant changes (IMHO), I thought of changing
> the authorship. I spoke with Ankit regarding this during my initial
> discussion with him.
>
> Will you please review the present RFC and the older one and suggest.  I have
> no issues with changing the authorship 😊.

It's fine with me if it's fine with Ankit. Thanks for checking.

BR,
Jani.


-- 
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] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Laxminarayan Bharadiya, Pankaj]

> -----Original Message-----
> From: Daniel Stone <daniel@fooishbar.org>
> Sent: 25 February 2020 13:00
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>; Daniel Vetter
> <daniel@ffwll.ch>; intel-gfx <intel-gfx@lists.freedesktop.org>; dri-devel
> <dri-devel@lists.freedesktop.org>; Ville Syrjälä
> <ville.syrjala@linux.intel.com>; David Airlie <airlied@linux.ie>; Maarten
> Lankhorst <maarten.lankhorst@linux.intel.com>; tzimmermann@suse.de;
> Maxime Ripard <mripard@kernel.org>; mihail.atanassov@arm.com; Joonas
> Lahtinen <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Souza,
> Jose <jose.souza@intel.com>; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>; Deak, Imre <imre.deak@intel.com>;
> Shankar, Uma <uma.shankar@intel.com>; Nautiyal, Ankit K
> <ankit.k.nautiyal@intel.com>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>
> Subject: Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-
> neighbor based integer scaling support
> 
> Hi,
> 
> On Tue, 25 Feb 2020 at 07:17, Pankaj Bharadiya
> <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> > @@ -415,18 +415,26 @@ skl_program_scaler(struct intel_plane *plane,
> >         u16 y_vphase, uv_rgb_vphase;
> >         int hscale, vscale;
> >         const struct drm_plane_state *state = &plane_state->uapi;
> > +       u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
> > +       u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
> >         u32 scaling_filter = PS_FILTER_MEDIUM;
> > +       struct drm_rect dst;
> >
> >         if (state->scaling_filter ==
> DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> >                 scaling_filter = PS_FILTER_PROGRAMMED;
> > +               skl_setup_nearest_neighbor_filter(dev_priv, pipe,
> > + scaler_id);
> > +
> > +               /* Make the scaling window size to integer multiple of source
> > +                * TODO: Should userspace take desision to round scaling window
> > +                * to integer multiple?
> > +                */
> > +               crtc_w = rounddown(crtc_w, src_w);
> > +               crtc_h = rounddown(crtc_h, src_h);
> 
> The kernel should absolutely not be changing the co-ordinates that
> userspace requested.

Thanks, Will get rid of this in V2.

Thanks,
Pankaj
> 
> Cheers,
> Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Ville Syrjälä]

On Tue, Feb 25, 2020 at 12:35:41PM +0530, Pankaj Bharadiya wrote:
> Introduce new scaling filter property to allow userspace to select
> the driver's default scaling filter or Nearest-neighbor(NN) filter
> for upscaling operations on crtc/plane.
> 
> Drivers can set up this property for a plane by calling
> drm_plane_enable_scaling_filter() and for a CRTC by calling
> drm_crtc_enable_scaling_filter().
> 
> NN filter works by filling in the missing color values in the upscaled
> image with that of the coordinate-mapped nearest source pixel value.
> 
> NN filter for integer multiple scaling can be particularly useful for
> for pixel art games that rely on sharp, blocky images to deliver their
> distinctive look.
> 
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  8 +++++++
>  drivers/gpu/drm/drm_crtc.c        | 16 ++++++++++++++
>  drivers/gpu/drm/drm_mode_config.c | 13 ++++++++++++
>  drivers/gpu/drm/drm_plane.c       | 35 +++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h            | 10 +++++++++
>  include/drm/drm_mode_config.h     |  6 ++++++
>  include/drm/drm_plane.h           | 14 +++++++++++++
>  7 files changed, 102 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index a1e5e262bae2..4e3c1f3176e4 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;

I think we want a per-plane/per-crtc prop for this. If we start adding
more filters we are surely going to need different sets for different hw
blocks.

>  	} 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
> @@ -583,6 +587,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>  					sizeof(struct drm_rect),
>  					&replaced);
>  		return ret;
> +	} else if (property == config->scaling_filter_property) {
> +		state->scaling_filter = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
>  				property, val);
> @@ -641,6 +647,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  	} else if (property == config->prop_fb_damage_clips) {
>  		*val = (state->fb_damage_clips) ?
>  			state->fb_damage_clips->base.id : 0;
> +	} else if (property == config->scaling_filter_property) {
> +		*val = state->scaling_filter;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
>  	} else {
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 4936e1080e41..1ce7b2ac9eb5 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -748,3 +748,19 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
>  
>  	return ret;
>  }
> +
> +/**
> + * drm_crtc_enable_scaling_filter - Enables crtc scaling filter property.
> + * @crtc: CRTC on which to enable scaling filter property.
> + *
> + * This function lets driver to enable the scaling filter property on a crtc.
> + */
> +void drm_crtc_enable_scaling_filter(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   dev->mode_config.scaling_filter_property,
> +				   0);
> +}
> +EXPORT_SYMBOL(drm_crtc_enable_scaling_filter);
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 08e6eff6a179..1024a8d1cd5d 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -214,6 +214,11 @@ static const struct drm_prop_enum_list drm_plane_type_enum_list[] = {
>  	{ DRM_PLANE_TYPE_CURSOR, "Cursor" },
>  };
>  
> +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> +	{ DRM_SCALING_FILTER_NEAREST_NEIGHBOR, "Nearest Neighbor" },
> +};
> +
>  static int drm_mode_create_standard_properties(struct drm_device *dev)
>  {
>  	struct drm_property *prop;
> @@ -370,6 +375,14 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.modifiers_property = prop;
>  
> +	prop = drm_property_create_enum(dev, 0,
> +				"SCALING_FILTER",
> +				drm_scaling_filter_enum_list,
> +				ARRAY_SIZE(drm_scaling_filter_enum_list));
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.scaling_filter_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index d6ad60ab0d38..ace7ee2775c8 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1221,3 +1221,38 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  
>  	return ret;
>  }
> +
> +/**
> + * DOC: Scaling filter property
> + *
> + *
> + * SCALING_FILTER:
> + *
> + *	Indicates scaling filter to be used for CRTC/plane scaler
> + *
> + *	The value of this property can be one of the following:
> + *	Default:
> + *		Driver's default scaling filter
> + *	Nearest Neighbor:
> + *		Nearest Neighbor scaling filter
> + *
> + * Drivers can set up this property for a plane by calling
> + * drm_plane_enable_scaling_filter() and for a CRTC by calling
> + * drm_crtc_enable_scaling_filter()
> + */
> +
> +/**
> + * drm_plane_enable_scaling_filter - Enables plane scaling filter property.
> + * @plane: Plane on which to enable scaling filter property.
> + *
> + * This function lets driver to enable the scaling filter property on a plane.
> + */
> +void drm_plane_enable_scaling_filter(struct drm_plane *plane)
> +{
> +	struct drm_device *dev = plane->dev;
> +
> +	drm_object_attach_property(&plane->base,
> +				   dev->mode_config.scaling_filter_property,
> +				   0);
> +}
> +EXPORT_SYMBOL(drm_plane_enable_scaling_filter);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 59b51a09cae6..770f9328a5ba 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -58,6 +58,7 @@ struct device_node;
>  struct dma_fence;
>  struct edid;
>  
> +
>  static inline int64_t U642I64(uint64_t val)
>  {
>  	return (int64_t)*((int64_t *)&val);
> @@ -296,6 +297,13 @@ struct drm_crtc_state {
>  	 */
>  	u32 target_vblank;
>  
> +	/**
> +	 * @scaling_filter:
> +	 *
> +	 * Scaling filter mode to be applied
> +	 */
> +	enum drm_scaling_filter scaling_filter;
> +
>  	/**
>  	 * @async_flip:
>  	 *
> @@ -1266,4 +1274,6 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
>  #define drm_for_each_crtc(crtc, dev) \
>  	list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
>  
> +void drm_crtc_enable_scaling_filter(struct drm_crtc *crtc);
> +
>  #endif /* __DRM_CRTC_H__ */
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 3bcbe30339f0..8c308ae1056d 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/plane property to apply a particular
> +	 * filter while scaling.
> +	 */
> +	struct drm_property *scaling_filter_property;
> +
>  	/* cursor size */
>  	uint32_t cursor_width, cursor_height;
>  
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 3f396d94afe4..2bc665cc6071 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -35,6 +35,11 @@ struct drm_crtc;
>  struct drm_printer;
>  struct drm_modeset_acquire_ctx;
>  
> +
> +enum drm_scaling_filter {
> +	DRM_SCALING_FILTER_DEFAULT,
> +	DRM_SCALING_FILTER_NEAREST_NEIGHBOR,
> +};
>  /**
>   * struct drm_plane_state - mutable plane state
>   *
> @@ -214,6 +219,13 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
>  
> +	/**
> +	 * @scaling_filter:
> +	 *
> +	 * Scaling filter mode to be applied
> +	 */
> +	enum drm_scaling_filter scaling_filter;
> +
>  	/**
>  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
>  	 * and for async plane updates.
> @@ -862,4 +874,6 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state)
>  					state->fb_damage_clips->data : NULL);
>  }
>  
> +void drm_plane_enable_scaling_filter(struct drm_plane *plane);
> +
>  #endif
> -- 
> 2.23.0

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

Re: [Intel-gfx] [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and pipe

[Ville Syrjälä]

On Tue, Feb 25, 2020 at 12:35:43PM +0530, Pankaj Bharadiya wrote:
> Attach scaling filter property for crtc and plane and program the
> scaler control register for the selected filter type.
> 
> This is preparatory patch to enable Nearest-neighbor integer scaling.
> 
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 17 +++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_sprite.c  | 12 +++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h              |  1 +
>  3 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3031e64ee518..b5903ef3c5a0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6242,6 +6242,8 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
> +	const struct drm_crtc_state *state = &crtc_state->uapi;
> +	u32 scaling_filter = PS_FILTER_MEDIUM;
>  	const struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
>  
> @@ -6258,6 +6260,11 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
>  		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
>  
> +		if (state->scaling_filter ==
> +		    DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> +			scaling_filter = PS_FILTER_PROGRAMMED;
> +		}

Just make that a function that can be used all over.
skl_scaler_filter(scaling_filter) or something.

> +
>  		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
>  		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
>  
> @@ -6268,8 +6275,10 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  
>  		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> -				  PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> +		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> +				  PS_SCALER_EN |
> +				  scaling_filter |
> +				  scaler_state->scalers[id].mode);
>  		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
>  				  PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
>  		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> @@ -16695,6 +16704,10 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  		dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
>  	}
>  
> +
> +	if (INTEL_GEN(dev_priv) >= 11)

gen >= 10 actually. Even glk seems to have it but bspec says not to
use it on glk. Supposedly not validated.

ilk/snb/ivb pfits also has programmable coefficients actually. So
IMO we should enable this on those as well.

The bigger problem will be how is userspace supposed to use this if it's
a crtc property? Those will not get automagically exposed via xrandr.

> +		drm_crtc_enable_scaling_filter(&crtc->base);
> +
>  	intel_color_init(crtc);
>  
>  	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 7abeefe8dce5..fd7b31a21723 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -414,6 +414,12 @@ skl_program_scaler(struct intel_plane *plane,
>  	u16 y_hphase, uv_rgb_hphase;
>  	u16 y_vphase, uv_rgb_vphase;
>  	int hscale, vscale;
> +	const struct drm_plane_state *state = &plane_state->uapi;
> +	u32 scaling_filter = PS_FILTER_MEDIUM;
> +
> +	if (state->scaling_filter == DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> +		scaling_filter = PS_FILTER_PROGRAMMED;
> +	}
>  
>  	hscale = drm_rect_calc_hscale(&plane_state->uapi.src,
>  				      &plane_state->uapi.dst,
> @@ -441,7 +447,8 @@ skl_program_scaler(struct intel_plane *plane,
>  	}
>  
>  	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, scaler_id),
> -			  PS_SCALER_EN | PS_PLANE_SEL(plane->id) | scaler->mode);
> +			  scaling_filter | PS_SCALER_EN |
> +			  PS_PLANE_SEL(plane->id) | scaler->mode);
>  	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, scaler_id),
>  			  PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase));
>  	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, scaler_id),
> @@ -3104,6 +3111,9 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  
>  	drm_plane_create_zpos_immutable_property(&plane->base, plane_id);
>  
> +	if (INTEL_GEN(dev_priv) >= 11)

also gen>=10

Also this patch breaks things as we don't yet have the code to program
the coefficients. So the series needs to be reordered.

> +		drm_plane_enable_scaling_filter(&plane->base);
> +
>  	drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
>  
>  	return plane;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f45b5e86ec63..34923b1c284c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7212,6 +7212,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)
> -- 
> 2.23.0

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

Re: [Intel-gfx] [RFC][PATCH 4/5] drm/i915: Introduce scaling filter related registers and bit fields.

[Ville Syrjälä]

On Tue, Feb 25, 2020 at 12:35:44PM +0530, Pankaj Bharadiya wrote:
> Introduce scaler registers and bit fields needed to configure the
> scaling filter in prgrammed mode and configure scaling filter
> coefficients.
> 
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 34923b1c284c..bba4ad3be611 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7289,6 +7289,18 @@ 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_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
> +

Sourious whitespace.

> +#define _PS_COEE_INDEX_AUTO_INC (1 << 10)

Wrong indentation (though looks like most scaler register
definitions get that wrong already), and the leading '_' shouldn't
be here at all.

> +
>  #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),       \
> @@ -7318,6 +7330,14 @@ enum {
>  			_ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A),   \
>  			_ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B))
>  
> +#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_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_2B))

Please parametrize by 'set' as well.

> +
>  /* legacy palette */
>  #define _LGC_PALETTE_A           0x4a000
>  #define _LGC_PALETTE_B           0x4a800
> -- 
> 2.23.0

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

Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Ville Syrjälä]

On Tue, Feb 25, 2020 at 12:35:45PM +0530, Pankaj Bharadiya wrote:
> Integer scaling (IS) is a nearest-neighbor upscaling technique that
> simply scales up the existing pixels by an integer
> (i.e., whole number) multiplier.Nearest-neighbor (NN) interpolation
> works by filling in the missing color values in the upscaled image
> with that of the coordinate-mapped nearest source pixel value.
> 
> Both IS and NN preserve the clarity of the original image. Integer
> scaling is particularly useful for pixel art games that rely on
> sharp, blocky images to deliver their distinctive look.
> 
> Program the scaler filter coefficients to enable the NN filter if
> scaling filter property is set to DRM_SCALING_FILTER_NEAREST_NEIGHBOR
> and enable integer scaling.
> 
> Bspec: 49247
> 
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 83 +++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_display.h |  2 +
>  drivers/gpu/drm/i915/display/intel_sprite.c  | 20 +++--
>  3 files changed, 97 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b5903ef3c5a0..6d5f59203258 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6237,6 +6237,73 @@ void skl_scaler_disable(const struct intel_crtc_state *old_crtc_state)
>  		skl_detach_scaler(crtc, i);
>  }
>  
> +/**
> + *  Theory behind setting nearest-neighbor integer scaling:
> + *
> + *  17 phase of 7 taps requires 119 coefficients in 60 dwords per set.
> + *  The letter represents the filter tap (D is the center tap) and the number
> + *  represents the coefficient set for a phase (0-16).
> + *
> + *         +------------+------------------------+------------------------+
> + *         |Index value | Data value coeffient 1 | Data value coeffient 2 |
> + *         +------------+------------------------+------------------------+
> + *         |   00h      |          B0            |          A0            |
> + *         +------------+------------------------+------------------------+
> + *         |   01h      |          D0            |          C0            |
> + *         +------------+------------------------+------------------------+
> + *         |   02h      |          F0            |          E0            |
> + *         +------------+------------------------+------------------------+
> + *         |   03h      |          A1            |          G0            |
> + *         +------------+------------------------+------------------------+
> + *         |   04h      |          C1            |          B1            |
> + *         +------------+------------------------+------------------------+
> + *         |   ...      |          ...           |          ...           |
> + *         +------------+------------------------+------------------------+
> + *         |   38h      |          B16           |          A16           |
> + *         +------------+------------------------+------------------------+
> + *         |   39h      |          D16           |          C16           |
> + *         +------------+------------------------+------------------------+
> + *         |   3Ah      |          F16           |          C16           |
> + *         +------------+------------------------+------------------------+
> + *         |   3Bh      |        Reserved        |          G16           |
> + *         +------------+------------------------+------------------------+
> + *
> + *  To enable nearest-neighbor scaling:  program scaler coefficents with
> + *  the center tap (Dxx) values set to 1 and all other values set to 0 as per
> + *  SCALER_COEFFICIENT_FORMAT
> + *
> + */
> +void skl_setup_nearest_neighbor_filter(struct drm_i915_private *dev_priv,
> +				  enum pipe pipe, int scaler_id)

skl_scaler_... 

> +{
> +
> +	int coeff = 0;
> +	int phase = 0;
> +	int tap;
> +	int val = 0;

Needlessly wide scope for most of these.

> +
> +	/*enable the index auto increment.*/
> +	intel_de_write_fw(dev_priv, SKL_PS_COEF_INDEX_SET0(pipe, scaler_id),
> +			  _PS_COEE_INDEX_AUTO_INC);
> +
> +	for (phase = 0; phase < 17; phase++) {
> +		for (tap = 0; tap < 7; tap++) {
> +			coeff++;

Can be part of the % check.

> +			if (tap == 3)
> +				val = (phase % 2) ? (0x800) : (0x800 << 16);

Parens overload.

> +
> +			if (coeff % 2 == 0) {
> +				intel_de_write_fw(dev_priv, SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);
> +				val = 0;

Can drop this val=0 if you move the variable into tight scope and
initialize there.

I was trying to think of a bit more generic way to do this, but couldn't
really think of anything apart from pre-filling the entire coefficient
set and the programming blindly. And that seems a bit wasteful if we only
care about nearest neighbour.

> +			}
> +
> +		}
> +
> +	}
> +
> +	intel_de_write_fw(dev_priv, SKL_PS_COEF_DATA_SET0(pipe, scaler_id), 0);
> +}
> +
>  static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -6260,9 +6327,23 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
>  		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
>  		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
>  
> +		id = scaler_state->scaler_id;
> +
>  		if (state->scaling_filter ==
>  		    DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
>  			scaling_filter = PS_FILTER_PROGRAMMED;
> +			skl_setup_nearest_neighbor_filter(dev_priv, pipe, id);

This should be sitting alongside the other register writes.

> +
> +			/* Make the scaling window size to integer multiple of
> +			 * source.
> +			 *
> +			 * TODO: Should userspace take desision to round
> +			 * scaling window to integer multiple?

To give userspace actual control of the pfit window size we need the border
props (or something along those lines). Step 1 is
https://patchwork.freedesktop.org/series/68409/. There are further steps
in my branch after that, but it's still missing the border props for
eDP/LVDS/DSI since I was too lazy to think how they should interact with
the existing scaling mode prop.

> +			 */
> +			pfit_w = rounddown(pfit_w,
> +					   (crtc_state->pipe_src_w << 16));
> +			pfit_h = rounddown(pfit_h,
> +					   (crtc_state->pipe_src_h << 16));
>  		}

This part should be dropped as Daniel mentioned.

>  
>  		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> @@ -6271,8 +6352,6 @@ static void skl_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);
>  
> -		id = scaler_state->scaler_id;
> -
>  		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
>  		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),

I think we should also explicitly indicate here which cofficient set(s)
we're going to use, even if using set0 does mean those bits will be 0.

> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index f92efbbec838..49f58d3c98fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -586,6 +586,8 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
>  u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
>  int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
>  void skl_scaler_disable(const struct intel_crtc_state *old_crtc_state);
> +void skl_setup_nearest_neighbor_filter(struct drm_i915_private *dev_priv,
> +				  enum pipe pipe, int scaler_id);
>  void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state);
>  u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  			const struct intel_plane_state *plane_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index fd7b31a21723..5bef5c031374 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -415,18 +415,26 @@ skl_program_scaler(struct intel_plane *plane,
>  	u16 y_vphase, uv_rgb_vphase;
>  	int hscale, vscale;
>  	const struct drm_plane_state *state = &plane_state->uapi;
> +	u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
> +	u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
>  	u32 scaling_filter = PS_FILTER_MEDIUM;
> +	struct drm_rect dst;
>  
>  	if (state->scaling_filter == DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
>  		scaling_filter = PS_FILTER_PROGRAMMED;
> +		skl_setup_nearest_neighbor_filter(dev_priv, pipe, scaler_id);
> +
> +		/* Make the scaling window size to integer multiple of source
> +		 * TODO: Should userspace take desision to round scaling window
> +		 * to integer multiple?
> +		 */
> +		crtc_w = rounddown(crtc_w, src_w);
> +		crtc_h = rounddown(crtc_h, src_h);
>  	}
>  
> -	hscale = drm_rect_calc_hscale(&plane_state->uapi.src,
> -				      &plane_state->uapi.dst,
> -				      0, INT_MAX);
> -	vscale = drm_rect_calc_vscale(&plane_state->uapi.src,
> -				      &plane_state->uapi.dst,
> -				      0, INT_MAX);
> +	drm_rect_init(&dst, crtc_x, crtc_y, crtc_w, crtc_h);

Drop as well.

> +	hscale = drm_rect_calc_hscale(&plane_state->uapi.src, &dst, 0, INT_MAX);
> +	vscale = drm_rect_calc_vscale(&plane_state->uapi.src, &dst, 0, INT_MAX);
>  
>  	/* TODO: handle sub-pixel coordinates */
>  	if (intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier) &&
> -- 
> 2.23.0

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

Re: [Intel-gfx] [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and pipe

[Laxminarayan Bharadiya, Pankaj]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: 10 March 2020 21:36
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>
> Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> Chris Wilson <chris@chris-wilson.co.uk>; Souza, Jose
> <jose.souza@intel.com>; Juha-Pekka Heikkila
> <juhapekka.heikkila@gmail.com>; linux-kernel@vger.kernel.org; Nautiyal,
> Ankit K <ankit.k.nautiyal@intel.com>
> Subject: Re: [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and
> pipe
> 
> On Tue, Feb 25, 2020 at 12:35:43PM +0530, Pankaj Bharadiya wrote:
> > Attach scaling filter property for crtc and plane and program the
> > scaler control register for the selected filter type.
> >
> > This is preparatory patch to enable Nearest-neighbor integer scaling.
> >
> > Signed-off-by: Pankaj Bharadiya
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 17 +++++++++++++++--
> > drivers/gpu/drm/i915/display/intel_sprite.c  | 12 +++++++++++-
> >  drivers/gpu/drm/i915/i915_reg.h              |  1 +
> >  3 files changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 3031e64ee518..b5903ef3c5a0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6242,6 +6242,8 @@ static void skl_pfit_enable(const struct
> intel_crtc_state *crtc_state)
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	enum pipe pipe = crtc->pipe;
> > +	const struct drm_crtc_state *state = &crtc_state->uapi;
> > +	u32 scaling_filter = PS_FILTER_MEDIUM;
> >  	const struct intel_crtc_scaler_state *scaler_state =
> >  		&crtc_state->scaler_state;
> >
> > @@ -6258,6 +6260,11 @@ static void skl_pfit_enable(const struct
> intel_crtc_state *crtc_state)
> >  		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> >  		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> >
> > +		if (state->scaling_filter ==
> > +		    DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> > +			scaling_filter = PS_FILTER_PROGRAMMED;
> > +		}
> 
> Just make that a function that can be used all over.
> skl_scaler_filter(scaling_filter) or something.
> 
> > +
> >  		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> >  		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> >
> > @@ -6268,8 +6275,10 @@ static void skl_pfit_enable(const struct
> > intel_crtc_state *crtc_state)
> >
> >  		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >
> > -		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> PS_SCALER_EN |
> > -				  PS_FILTER_MEDIUM | scaler_state-
> >scalers[id].mode);
> > +		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> > +				  PS_SCALER_EN |
> > +				  scaling_filter |
> > +				  scaler_state->scalers[id].mode);
> >  		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> >  				  PS_Y_PHASE(0) |
> PS_UV_RGB_PHASE(uv_rgb_vphase));
> >  		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id), @@
> -16695,6
> > +16704,10 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv,
> enum pipe pipe)
> >  		dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
> >  	}
> >
> > +
> > +	if (INTEL_GEN(dev_priv) >= 11)
> 
> gen >= 10 actually. Even glk seems to have it but bspec says not to use it on
> glk. Supposedly not validated.
> 
> ilk/snb/ivb pfits also has programmable coefficients actually. So IMO we
> should enable this on those as well.

OK. I need to explore bspec more for these platforms.
To begin with I would like to stick to gen >=10.

> 
> The bigger problem will be how is userspace supposed to use this if it's a crtc
> property? Those will not get automagically exposed via xrandr.
> 
> > +		drm_crtc_enable_scaling_filter(&crtc->base);
> > +
> >  	intel_color_init(crtc);
> >
> >  	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) !=
> > crtc->pipe); diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index 7abeefe8dce5..fd7b31a21723 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -414,6 +414,12 @@ skl_program_scaler(struct intel_plane *plane,
> >  	u16 y_hphase, uv_rgb_hphase;
> >  	u16 y_vphase, uv_rgb_vphase;
> >  	int hscale, vscale;
> > +	const struct drm_plane_state *state = &plane_state->uapi;
> > +	u32 scaling_filter = PS_FILTER_MEDIUM;
> > +
> > +	if (state->scaling_filter ==
> DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> > +		scaling_filter = PS_FILTER_PROGRAMMED;
> > +	}
> >
> >  	hscale = drm_rect_calc_hscale(&plane_state->uapi.src,
> >  				      &plane_state->uapi.dst,
> > @@ -441,7 +447,8 @@ skl_program_scaler(struct intel_plane *plane,
> >  	}
> >
> >  	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, scaler_id),
> > -			  PS_SCALER_EN | PS_PLANE_SEL(plane->id) | scaler-
> >mode);
> > +			  scaling_filter | PS_SCALER_EN |
> > +			  PS_PLANE_SEL(plane->id) | scaler->mode);
> >  	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, scaler_id),
> >  			  PS_Y_PHASE(y_vphase) |
> PS_UV_RGB_PHASE(uv_rgb_vphase));
> >  	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, scaler_id), @@
> > -3104,6 +3111,9 @@ skl_universal_plane_create(struct drm_i915_private
> > *dev_priv,
> >
> >  	drm_plane_create_zpos_immutable_property(&plane->base,
> plane_id);
> >
> > +	if (INTEL_GEN(dev_priv) >= 11)
> 
> also gen>=10
> 
> Also this patch breaks things as we don't yet have the code to program the
> coefficients. So the series needs to be reordered.

Will reorder the series.

Thanks,
Pankaj
> 
> > +		drm_plane_enable_scaling_filter(&plane->base);
> > +
> >  	drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
> >
> >  	return plane;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index f45b5e86ec63..34923b1c284c
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7212,6 +7212,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)
> > --
> > 2.23.0
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Laxminarayan Bharadiya, Pankaj]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: 10 March 2020 21:47
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>
> Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> Chris Wilson <chris@chris-wilson.co.uk>; Souza, Jose
> <jose.souza@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>;
> Roper, Matthew D <matthew.d.roper@intel.com>; Deak, Imre
> <imre.deak@intel.com>; Shankar, Uma <uma.shankar@intel.com>; linux-
> kernel@vger.kernel.org; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor
> based integer scaling support
> 
> On Tue, Feb 25, 2020 at 12:35:45PM +0530, Pankaj Bharadiya wrote:
> > Integer scaling (IS) is a nearest-neighbor upscaling technique that
> > simply scales up the existing pixels by an integer (i.e., whole
> > number) multiplier.Nearest-neighbor (NN) interpolation works by
> > filling in the missing color values in the upscaled image with that of
> > the coordinate-mapped nearest source pixel value.
> >
> > Both IS and NN preserve the clarity of the original image. Integer
> > scaling is particularly useful for pixel art games that rely on sharp,
> > blocky images to deliver their distinctive look.
> >
> > Program the scaler filter coefficients to enable the NN filter if
> > scaling filter property is set to DRM_SCALING_FILTER_NEAREST_NEIGHBOR
> > and enable integer scaling.
> >
> > Bspec: 49247
> >
> > Signed-off-by: Pankaj Bharadiya
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 83
> > +++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_display.h |
> > 2 +  drivers/gpu/drm/i915/display/intel_sprite.c  | 20 +++--
> >  3 files changed, 97 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index b5903ef3c5a0..6d5f59203258 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6237,6 +6237,73 @@ void skl_scaler_disable(const struct
> intel_crtc_state *old_crtc_state)
> >  		skl_detach_scaler(crtc, i);
> >  }
> >
> > +/**
> > + *  Theory behind setting nearest-neighbor integer scaling:
> > + *
> > + *  17 phase of 7 taps requires 119 coefficients in 60 dwords per set.
> > + *  The letter represents the filter tap (D is the center tap) and
> > +the number
> > + *  represents the coefficient set for a phase (0-16).
> > + *
> > + *         +------------+------------------------+------------------------+
> > + *         |Index value | Data value coeffient 1 | Data value coeffient 2 |
> > + *         +------------+------------------------+------------------------+
> > + *         |   00h      |          B0            |          A0            |
> > + *         +------------+------------------------+------------------------+
> > + *         |   01h      |          D0            |          C0            |
> > + *         +------------+------------------------+------------------------+
> > + *         |   02h      |          F0            |          E0            |
> > + *         +------------+------------------------+------------------------+
> > + *         |   03h      |          A1            |          G0            |
> > + *         +------------+------------------------+------------------------+
> > + *         |   04h      |          C1            |          B1            |
> > + *         +------------+------------------------+------------------------+
> > + *         |   ...      |          ...           |          ...           |
> > + *         +------------+------------------------+------------------------+
> > + *         |   38h      |          B16           |          A16           |
> > + *         +------------+------------------------+------------------------+
> > + *         |   39h      |          D16           |          C16           |
> > + *         +------------+------------------------+------------------------+
> > + *         |   3Ah      |          F16           |          C16           |
> > + *         +------------+------------------------+------------------------+
> > + *         |   3Bh      |        Reserved        |          G16           |
> > + *         +------------+------------------------+------------------------+
> > + *
> > + *  To enable nearest-neighbor scaling:  program scaler coefficents
> > +with
> > + *  the center tap (Dxx) values set to 1 and all other values set to
> > +0 as per
> > + *  SCALER_COEFFICIENT_FORMAT
> > + *
> > + */
> > +void skl_setup_nearest_neighbor_filter(struct drm_i915_private
> *dev_priv,
> > +				  enum pipe pipe, int scaler_id)
> 
> skl_scaler_...
> 
> > +{
> > +
> > +	int coeff = 0;
> > +	int phase = 0;
> > +	int tap;
> > +	int val = 0;
> 
> Needlessly wide scope for most of these.
> 
> > +
> > +	/*enable the index auto increment.*/
> > +	intel_de_write_fw(dev_priv, SKL_PS_COEF_INDEX_SET0(pipe,
> scaler_id),
> > +			  _PS_COEE_INDEX_AUTO_INC);
> > +
> > +	for (phase = 0; phase < 17; phase++) {
> > +		for (tap = 0; tap < 7; tap++) {
> > +			coeff++;
> 
> Can be part of the % check.

OK.

> 
> > +			if (tap == 3)
> > +				val = (phase % 2) ? (0x800) : (0x800 << 16);
> 
> Parens overload.

OK. Will remove.
> 
> > +
> > +			if (coeff % 2 == 0) {
> > +				intel_de_write_fw(dev_priv,
> SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);
> > +				val = 0;
> 
> Can drop this val=0 if you move the variable into tight scope and initialize
> there.

Moving val=0 initialization to the tight scope will not work here as we need
to retain "val" and write only when 2 coefficients are ready (since 2 
coefficients are packed in 1 dword).

e.g. for (12th , 11th)  coefficients, coefficient reg value should be ( (0 << 16) | 0x800).
If we initialize val = 0 in tight loop, 0 will be written to  coefficient register.

> 
> I was trying to think of a bit more generic way to do this, but couldn't really
> think of anything apart from pre-filling the entire coefficient set and the
> programming blindly. And that seems a bit wasteful if we only care about
> nearest neighbour.
> 
> > +			}
> > +
> > +		}
> > +
> > +	}
> > +
> > +	intel_de_write_fw(dev_priv, SKL_PS_COEF_DATA_SET0(pipe,
> scaler_id),
> > +0); }
> > +
> >  static void skl_pfit_enable(const struct intel_crtc_state
> > *crtc_state)  {
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > @@ -6260,9 +6327,23 @@ static void skl_pfit_enable(const struct
> intel_crtc_state *crtc_state)
> >  		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> >  		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> >
> > +		id = scaler_state->scaler_id;
> > +
> >  		if (state->scaling_filter ==
> >  		    DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> >  			scaling_filter = PS_FILTER_PROGRAMMED;
> > +			skl_setup_nearest_neighbor_filter(dev_priv, pipe,
> id);
> 
> This should be sitting alongside the other register writes.

I missed this, thanks for pointing out.

> 
> > +
> > +			/* Make the scaling window size to integer multiple
> of
> > +			 * source.
> > +			 *
> > +			 * TODO: Should userspace take desision to round
> > +			 * scaling window to integer multiple?
> 
> To give userspace actual control of the pfit window size we need the border
> props (or something along those lines). Step 1 is
> https://patchwork.freedesktop.org/series/68409/. There are further steps in
> my branch after that, but it's still missing the border props for eDP/LVDS/DSI
> since I was too lazy to think how they should interact with the existing scaling
> mode prop.
> 
> > +			 */
> > +			pfit_w = rounddown(pfit_w,
> > +					   (crtc_state->pipe_src_w << 16));
> > +			pfit_h = rounddown(pfit_h,
> > +					   (crtc_state->pipe_src_h << 16));
> >  		}
> 
> This part should be dropped as Daniel mentioned.

Will remove.

Thanks,
Pankaj

> 
> >
> >  		hscale = (crtc_state->pipe_src_w << 16) / pfit_w; @@ -
> 6271,8
> > +6352,6 @@ static void skl_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);
> >
> > -		id = scaler_state->scaler_id;
> > -
> >  		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >
> >  		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> 
> I think we should also explicitly indicate here which cofficient set(s) we're
> going to use, even if using set0 does mean those bits will be 0.
> 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> > b/drivers/gpu/drm/i915/display/intel_display.h
> > index f92efbbec838..49f58d3c98fe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -586,6 +586,8 @@ void intel_crtc_arm_fifo_underrun(struct
> > intel_crtc *crtc,
> >  u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
> > int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);  void
> > skl_scaler_disable(const struct intel_crtc_state *old_crtc_state);
> > +void skl_setup_nearest_neighbor_filter(struct drm_i915_private
> *dev_priv,
> > +				  enum pipe pipe, int scaler_id);
> >  void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state);
> >  u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> >  			const struct intel_plane_state *plane_state); diff --
> git
> > a/drivers/gpu/drm/i915/display/intel_sprite.c
> > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index fd7b31a21723..5bef5c031374 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -415,18 +415,26 @@ skl_program_scaler(struct intel_plane *plane,
> >  	u16 y_vphase, uv_rgb_vphase;
> >  	int hscale, vscale;
> >  	const struct drm_plane_state *state = &plane_state->uapi;
> > +	u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
> > +	u32 src_h = drm_rect_height(&plane_state->uapi.src) >> 16;
> >  	u32 scaling_filter = PS_FILTER_MEDIUM;
> > +	struct drm_rect dst;
> >
> >  	if (state->scaling_filter ==
> DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> >  		scaling_filter = PS_FILTER_PROGRAMMED;
> > +		skl_setup_nearest_neighbor_filter(dev_priv, pipe,
> scaler_id);
> > +
> > +		/* Make the scaling window size to integer multiple of source
> > +		 * TODO: Should userspace take desision to round scaling
> window
> > +		 * to integer multiple?
> > +		 */
> > +		crtc_w = rounddown(crtc_w, src_w);
> > +		crtc_h = rounddown(crtc_h, src_h);
> >  	}
> >
> > -	hscale = drm_rect_calc_hscale(&plane_state->uapi.src,
> > -				      &plane_state->uapi.dst,
> > -				      0, INT_MAX);
> > -	vscale = drm_rect_calc_vscale(&plane_state->uapi.src,
> > -				      &plane_state->uapi.dst,
> > -				      0, INT_MAX);
> > +	drm_rect_init(&dst, crtc_x, crtc_y, crtc_w, crtc_h);
> 
> Drop as well.
> 
> > +	hscale = drm_rect_calc_hscale(&plane_state->uapi.src, &dst, 0,
> INT_MAX);
> > +	vscale = drm_rect_calc_vscale(&plane_state->uapi.src, &dst, 0,
> > +INT_MAX);
> >
> >  	/* TODO: handle sub-pixel coordinates */
> >  	if (intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)
> &&
> > --
> > 2.23.0
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and pipe

[Ville Syrjälä]

On Thu, Mar 12, 2020 at 08:58:42AM +0000, Laxminarayan Bharadiya, Pankaj wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 10 March 2020 21:36
> > To: Laxminarayan Bharadiya, Pankaj
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> > maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> > mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen
> > <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> > Chris Wilson <chris@chris-wilson.co.uk>; Souza, Jose
> > <jose.souza@intel.com>; Juha-Pekka Heikkila
> > <juhapekka.heikkila@gmail.com>; linux-kernel@vger.kernel.org; Nautiyal,
> > Ankit K <ankit.k.nautiyal@intel.com>
> > Subject: Re: [RFC][PATCH 3/5] drm/i915: Enable scaling filter for plane and
> > pipe
> > 
> > On Tue, Feb 25, 2020 at 12:35:43PM +0530, Pankaj Bharadiya wrote:
> > > Attach scaling filter property for crtc and plane and program the
> > > scaler control register for the selected filter type.
> > >
> > > This is preparatory patch to enable Nearest-neighbor integer scaling.
> > >
> > > Signed-off-by: Pankaj Bharadiya
> > > <pankaj.laxminarayan.bharadiya@intel.com>
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 17 +++++++++++++++--
> > > drivers/gpu/drm/i915/display/intel_sprite.c  | 12 +++++++++++-
> > >  drivers/gpu/drm/i915/i915_reg.h              |  1 +
> > >  3 files changed, 27 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 3031e64ee518..b5903ef3c5a0 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -6242,6 +6242,8 @@ static void skl_pfit_enable(const struct
> > intel_crtc_state *crtc_state)
> > >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > >  	enum pipe pipe = crtc->pipe;
> > > +	const struct drm_crtc_state *state = &crtc_state->uapi;
> > > +	u32 scaling_filter = PS_FILTER_MEDIUM;
> > >  	const struct intel_crtc_scaler_state *scaler_state =
> > >  		&crtc_state->scaler_state;
> > >
> > > @@ -6258,6 +6260,11 @@ static void skl_pfit_enable(const struct
> > intel_crtc_state *crtc_state)
> > >  		pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> > >  		pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> > >
> > > +		if (state->scaling_filter ==
> > > +		    DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> > > +			scaling_filter = PS_FILTER_PROGRAMMED;
> > > +		}
> > 
> > Just make that a function that can be used all over.
> > skl_scaler_filter(scaling_filter) or something.
> > 
> > > +
> > >  		hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> > >  		vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> > >
> > > @@ -6268,8 +6275,10 @@ static void skl_pfit_enable(const struct
> > > intel_crtc_state *crtc_state)
> > >
> > >  		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> > >
> > > -		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> > PS_SCALER_EN |
> > > -				  PS_FILTER_MEDIUM | scaler_state-
> > >scalers[id].mode);
> > > +		intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id),
> > > +				  PS_SCALER_EN |
> > > +				  scaling_filter |
> > > +				  scaler_state->scalers[id].mode);
> > >  		intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> > >  				  PS_Y_PHASE(0) |
> > PS_UV_RGB_PHASE(uv_rgb_vphase));
> > >  		intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id), @@
> > -16695,6
> > > +16704,10 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv,
> > enum pipe pipe)
> > >  		dev_priv->plane_to_crtc_mapping[i9xx_plane] = crtc;
> > >  	}
> > >
> > > +
> > > +	if (INTEL_GEN(dev_priv) >= 11)
> > 
> > gen >= 10 actually. Even glk seems to have it but bspec says not to use it on
> > glk. Supposedly not validated.
> > 
> > ilk/snb/ivb pfits also has programmable coefficients actually. So IMO we
> > should enable this on those as well.
> 
> OK. I need to explore bspec more for these platforms.
> To begin with I would like to stick to gen >=10.

Sure. You can also have a look at the intel_scaling_coef hacks I posted
to igt-dev for details on how to drive them all. I already reverse
engineered them sufficiently so that I was able to program them ;)

> 
> > 
> > The bigger problem will be how is userspace supposed to use this if it's a crtc
> > property? Those will not get automagically exposed via xrandr.
> > 
> > > +		drm_crtc_enable_scaling_filter(&crtc->base);
> > > +
> > >  	intel_color_init(crtc);
> > >
> > >  	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) !=
> > > crtc->pipe); diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > index 7abeefe8dce5..fd7b31a21723 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > @@ -414,6 +414,12 @@ skl_program_scaler(struct intel_plane *plane,
> > >  	u16 y_hphase, uv_rgb_hphase;
> > >  	u16 y_vphase, uv_rgb_vphase;
> > >  	int hscale, vscale;
> > > +	const struct drm_plane_state *state = &plane_state->uapi;
> > > +	u32 scaling_filter = PS_FILTER_MEDIUM;
> > > +
> > > +	if (state->scaling_filter ==
> > DRM_SCALING_FILTER_NEAREST_NEIGHBOR) {
> > > +		scaling_filter = PS_FILTER_PROGRAMMED;
> > > +	}
> > >
> > >  	hscale = drm_rect_calc_hscale(&plane_state->uapi.src,
> > >  				      &plane_state->uapi.dst,
> > > @@ -441,7 +447,8 @@ skl_program_scaler(struct intel_plane *plane,
> > >  	}
> > >
> > >  	intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, scaler_id),
> > > -			  PS_SCALER_EN | PS_PLANE_SEL(plane->id) | scaler-
> > >mode);
> > > +			  scaling_filter | PS_SCALER_EN |
> > > +			  PS_PLANE_SEL(plane->id) | scaler->mode);
> > >  	intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, scaler_id),
> > >  			  PS_Y_PHASE(y_vphase) |
> > PS_UV_RGB_PHASE(uv_rgb_vphase));
> > >  	intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, scaler_id), @@
> > > -3104,6 +3111,9 @@ skl_universal_plane_create(struct drm_i915_private
> > > *dev_priv,
> > >
> > >  	drm_plane_create_zpos_immutable_property(&plane->base,
> > plane_id);
> > >
> > > +	if (INTEL_GEN(dev_priv) >= 11)
> > 
> > also gen>=10
> > 
> > Also this patch breaks things as we don't yet have the code to program the
> > coefficients. So the series needs to be reordered.
> 
> Will reorder the series.
> 
> Thanks,
> Pankaj
> > 
> > > +		drm_plane_enable_scaling_filter(&plane->base);
> > > +
> > >  	drm_plane_helper_add(&plane->base, &intel_plane_helper_funcs);
> > >
> > >  	return plane;
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index f45b5e86ec63..34923b1c284c
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7212,6 +7212,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)
> > > --
> > > 2.23.0
> > 
> > --
> > Ville Syrjälä
> > Intel

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

Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Ville Syrjälä]

On Thu, Mar 12, 2020 at 09:13:24AM +0000, Laxminarayan Bharadiya, Pankaj wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 10 March 2020 21:47
> > To: Laxminarayan Bharadiya, Pankaj
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> > maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> > mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen
> > <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> > Chris Wilson <chris@chris-wilson.co.uk>; Souza, Jose
> > <jose.souza@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>;
> > Roper, Matthew D <matthew.d.roper@intel.com>; Deak, Imre
> > <imre.deak@intel.com>; Shankar, Uma <uma.shankar@intel.com>; linux-
> > kernel@vger.kernel.org; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor
> > based integer scaling support
> > 
> > On Tue, Feb 25, 2020 at 12:35:45PM +0530, Pankaj Bharadiya wrote:
> > > Integer scaling (IS) is a nearest-neighbor upscaling technique that
> > > simply scales up the existing pixels by an integer (i.e., whole
> > > number) multiplier.Nearest-neighbor (NN) interpolation works by
> > > filling in the missing color values in the upscaled image with that of
> > > the coordinate-mapped nearest source pixel value.
> > >
> > > Both IS and NN preserve the clarity of the original image. Integer
> > > scaling is particularly useful for pixel art games that rely on sharp,
> > > blocky images to deliver their distinctive look.
> > >
> > > Program the scaler filter coefficients to enable the NN filter if
> > > scaling filter property is set to DRM_SCALING_FILTER_NEAREST_NEIGHBOR
> > > and enable integer scaling.
> > >
> > > Bspec: 49247
> > >
> > > Signed-off-by: Pankaj Bharadiya
> > > <pankaj.laxminarayan.bharadiya@intel.com>
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 83
> > > +++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_display.h |
> > > 2 +  drivers/gpu/drm/i915/display/intel_sprite.c  | 20 +++--
> > >  3 files changed, 97 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index b5903ef3c5a0..6d5f59203258 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -6237,6 +6237,73 @@ void skl_scaler_disable(const struct
> > intel_crtc_state *old_crtc_state)
> > >  		skl_detach_scaler(crtc, i);
> > >  }
> > >
> > > +/**
> > > + *  Theory behind setting nearest-neighbor integer scaling:
> > > + *
> > > + *  17 phase of 7 taps requires 119 coefficients in 60 dwords per set.
> > > + *  The letter represents the filter tap (D is the center tap) and
> > > +the number
> > > + *  represents the coefficient set for a phase (0-16).
> > > + *
> > > + *         +------------+------------------------+------------------------+
> > > + *         |Index value | Data value coeffient 1 | Data value coeffient 2 |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   00h      |          B0            |          A0            |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   01h      |          D0            |          C0            |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   02h      |          F0            |          E0            |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   03h      |          A1            |          G0            |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   04h      |          C1            |          B1            |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   ...      |          ...           |          ...           |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   38h      |          B16           |          A16           |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   39h      |          D16           |          C16           |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   3Ah      |          F16           |          C16           |
> > > + *         +------------+------------------------+------------------------+
> > > + *         |   3Bh      |        Reserved        |          G16           |
> > > + *         +------------+------------------------+------------------------+
> > > + *
> > > + *  To enable nearest-neighbor scaling:  program scaler coefficents
> > > +with
> > > + *  the center tap (Dxx) values set to 1 and all other values set to
> > > +0 as per
> > > + *  SCALER_COEFFICIENT_FORMAT
> > > + *
> > > + */
> > > +void skl_setup_nearest_neighbor_filter(struct drm_i915_private
> > *dev_priv,
> > > +				  enum pipe pipe, int scaler_id)
> > 
> > skl_scaler_...
> > 
> > > +{
> > > +
> > > +	int coeff = 0;
> > > +	int phase = 0;
> > > +	int tap;
> > > +	int val = 0;
> > 
> > Needlessly wide scope for most of these.
> > 
> > > +
> > > +	/*enable the index auto increment.*/
> > > +	intel_de_write_fw(dev_priv, SKL_PS_COEF_INDEX_SET0(pipe,
> > scaler_id),
> > > +			  _PS_COEE_INDEX_AUTO_INC);
> > > +
> > > +	for (phase = 0; phase < 17; phase++) {
> > > +		for (tap = 0; tap < 7; tap++) {
> > > +			coeff++;
> > 
> > Can be part of the % check.
> 
> OK.
> 
> > 
> > > +			if (tap == 3)
> > > +				val = (phase % 2) ? (0x800) : (0x800 << 16);
> > 
> > Parens overload.
> 
> OK. Will remove.
> > 
> > > +
> > > +			if (coeff % 2 == 0) {
> > > +				intel_de_write_fw(dev_priv,
> > SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);
> > > +				val = 0;
> > 
> > Can drop this val=0 if you move the variable into tight scope and initialize
> > there.
> 
> Moving val=0 initialization to the tight scope will not work here as we need
> to retain "val" and write only when 2 coefficients are ready (since 2 
> coefficients are packed in 1 dword).
> 
> e.g. for (12th , 11th)  coefficients, coefficient reg value should be ( (0 << 16) | 0x800).
> If we initialize val = 0 in tight loop, 0 will be written to  coefficient register.

Hmm, right. I guess I'd try to rearrange this to iterate the
registers directly instead of the phases and taps. Something
like this perhaps:

static int cnl_coef_tap(int i)
{
	return i % 7;
}

static u16 cnl_coef(int t)
{
	return t == 3 ? 0x0800 : 0x3000;
}

static void cnl_program_nearest_filter_coefs(void)
{
	int i;

	for (i = 0; i < 17 * 7; i += 2) {
		uint32_t tmp;
		int t;

		t = cnl_coef_tap(i);
		tmp = cnl_nearest_filter_coef(t);

		t = cnl_coef_tap(i + 1);
		tmp |= cnl_nearest_filter_coef(t) << 16;

		intel_de_write_fw(tmp);
	}
}

More readable I think. The downside being all those modulo operations
but hopefully that's all in the noise when it comes to performance.

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

Re: [Intel-gfx] [RFC][PATCH 0/5] Introduce drm scaling filter property

[Ville Syrjälä]

On Tue, Feb 25, 2020 at 12:35:40PM +0530, Pankaj Bharadiya wrote:
> Integer scaling (IS) is a nearest-neighbor upscaling technique that
> simply scales up the existing pixels by an integer (i.e., whole
> number) multiplier. Nearest-neighbor (NN) interpolation works by
> filling in the missing color values in the upscaled image with that of
> the coordinate-mapped nearest source pixel value.
> 
> Both IS and NN preserve the clarity of the original image. In
> contrast, traditional upscaling algorithms, such as bilinear or
> bicubic interpolation, result in blurry upscaled images because they
> employ interpolation techniques that smooth out the transition from
> one pixel to another.  Therefore, integer scaling is particularly
> useful for pixel art games that rely on sharp, blocky images to
> deliver their distinctive look.
> 
> 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 -
>   - Introduces new scaling filter property to allow userspace to
>     select  the driver's default scaling filter or Nearest-neighbor(NN)
>     filter for scaling operations on crtc/plane.
>   - Implements and enable integer scaling for i915
> 
> Userspace patch series link: TBD.

That needs to be done or this will go nowhere.

> 
> Thanks to Shashank for initiating this work. His initial RFC can be
> found here [1]
> 
> [1] https://patchwork.freedesktop.org/patch/337082/
> 
> Modifications done in this series -
>    - refactored code and incorporated initial review comments and
>      added 2 scaling filter types (default and NN) to begin with.
>    - added scaling filter property support for planes and new API
>      helpers for drivers to setup this property.
>    - rewrote code to enable integer scaling and NN filter for i915
> 
> 
> Pankaj Bharadiya (5):
>   drm: Introduce scaling filter property
>   drm/drm-kms.rst: Add Scaling filter property documentation
>   drm/i915: Enable scaling filter for plane and pipe
>   drm/i915: Introduce scaling filter related registers and bit fields.
>   drm/i915/display: Add Nearest-neighbor based integer scaling support
> 
>  Documentation/gpu/drm-kms.rst                |   6 ++
>  drivers/gpu/drm/drm_atomic_uapi.c            |   8 ++
>  drivers/gpu/drm/drm_crtc.c                   |  16 +++
>  drivers/gpu/drm/drm_mode_config.c            |  13 +++
>  drivers/gpu/drm/drm_plane.c                  |  35 +++++++
>  drivers/gpu/drm/i915/display/intel_display.c | 100 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_display.h |   2 +
>  drivers/gpu/drm/i915/display/intel_sprite.c  |  32 ++++--
>  drivers/gpu/drm/i915/i915_reg.h              |  21 ++++
>  include/drm/drm_crtc.h                       |  10 ++
>  include/drm/drm_mode_config.h                |   6 ++
>  include/drm/drm_plane.h                      |  14 +++
>  12 files changed, 252 insertions(+), 11 deletions(-)
> 
> -- 
> 2.23.0

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

Re: [Intel-gfx] [RFC][PATCH 0/5] Introduce drm scaling filter property

[Laxminarayan Bharadiya, Pankaj]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: 12 March 2020 19:35
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>
> Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> mripard@kernel.org; mihail.atanassov@arm.com; linux-
> kernel@vger.kernel.org; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Subject: Re: [RFC][PATCH 0/5] Introduce drm scaling filter property
> 
> On Tue, Feb 25, 2020 at 12:35:40PM +0530, Pankaj Bharadiya wrote:
> > Integer scaling (IS) is a nearest-neighbor upscaling technique that
> > simply scales up the existing pixels by an integer (i.e., whole
> > number) multiplier. Nearest-neighbor (NN) interpolation works by
> > filling in the missing color values in the upscaled image with that of
> > the coordinate-mapped nearest source pixel value.
> >
> > Both IS and NN preserve the clarity of the original image. In
> > contrast, traditional upscaling algorithms, such as bilinear or
> > bicubic interpolation, result in blurry upscaled images because they
> > employ interpolation techniques that smooth out the transition from
> > one pixel to another.  Therefore, integer scaling is particularly
> > useful for pixel art games that rely on sharp, blocky images to
> > deliver their distinctive look.
> >
> > 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-i
> > ntel-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 -
> >   - Introduces new scaling filter property to allow userspace to
> >     select  the driver's default scaling filter or Nearest-neighbor(NN)
> >     filter for scaling operations on crtc/plane.
> >   - Implements and enable integer scaling for i915
> >
> > Userspace patch series link: TBD.
> 
> That needs to be done or this will go nowhere.

Yes, Sameer is working on enabling this feature in Kodi. 
Sameer, please share link here once you post patches.
 
Thanks,
Pankaj

> 
> >
> > Thanks to Shashank for initiating this work. His initial RFC can be
> > found here [1]
> >
> > [1] https://patchwork.freedesktop.org/patch/337082/
> >
> > Modifications done in this series -
> >    - refactored code and incorporated initial review comments and
> >      added 2 scaling filter types (default and NN) to begin with.
> >    - added scaling filter property support for planes and new API
> >      helpers for drivers to setup this property.
> >    - rewrote code to enable integer scaling and NN filter for i915
> >
> >
> > Pankaj Bharadiya (5):
> >   drm: Introduce scaling filter property
> >   drm/drm-kms.rst: Add Scaling filter property documentation
> >   drm/i915: Enable scaling filter for plane and pipe
> >   drm/i915: Introduce scaling filter related registers and bit fields.
> >   drm/i915/display: Add Nearest-neighbor based integer scaling support
> >
> >  Documentation/gpu/drm-kms.rst                |   6 ++
> >  drivers/gpu/drm/drm_atomic_uapi.c            |   8 ++
> >  drivers/gpu/drm/drm_crtc.c                   |  16 +++
> >  drivers/gpu/drm/drm_mode_config.c            |  13 +++
> >  drivers/gpu/drm/drm_plane.c                  |  35 +++++++
> >  drivers/gpu/drm/i915/display/intel_display.c | 100 ++++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_display.h |   2 +
> >  drivers/gpu/drm/i915/display/intel_sprite.c  |  32 ++++--
> >  drivers/gpu/drm/i915/i915_reg.h              |  21 ++++
> >  include/drm/drm_crtc.h                       |  10 ++
> >  include/drm/drm_mode_config.h                |   6 ++
> >  include/drm/drm_plane.h                      |  14 +++
> >  12 files changed, 252 insertions(+), 11 deletions(-)
> >
> > --
> > 2.23.0
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC][PATCH 0/5] Introduce drm scaling filter property

[Ville Syrjälä]

On Thu, Mar 12, 2020 at 03:37:03PM +0000, Laxminarayan Bharadiya, Pankaj wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 12 March 2020 19:35
> > To: Laxminarayan Bharadiya, Pankaj
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> > maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> > mripard@kernel.org; mihail.atanassov@arm.com; linux-
> > kernel@vger.kernel.org; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > Subject: Re: [RFC][PATCH 0/5] Introduce drm scaling filter property
> > 
> > On Tue, Feb 25, 2020 at 12:35:40PM +0530, Pankaj Bharadiya wrote:
> > > Integer scaling (IS) is a nearest-neighbor upscaling technique that
> > > simply scales up the existing pixels by an integer (i.e., whole
> > > number) multiplier. Nearest-neighbor (NN) interpolation works by
> > > filling in the missing color values in the upscaled image with that of
> > > the coordinate-mapped nearest source pixel value.
> > >
> > > Both IS and NN preserve the clarity of the original image. In
> > > contrast, traditional upscaling algorithms, such as bilinear or
> > > bicubic interpolation, result in blurry upscaled images because they
> > > employ interpolation techniques that smooth out the transition from
> > > one pixel to another.  Therefore, integer scaling is particularly
> > > useful for pixel art games that rely on sharp, blocky images to
> > > deliver their distinctive look.
> > >
> > > 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-i
> > > ntel-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 -
> > >   - Introduces new scaling filter property to allow userspace to
> > >     select  the driver's default scaling filter or Nearest-neighbor(NN)
> > >     filter for scaling operations on crtc/plane.
> > >   - Implements and enable integer scaling for i915
> > >
> > > Userspace patch series link: TBD.
> > 
> > That needs to be done or this will go nowhere.
> 
> Yes, Sameer is working on enabling this feature in Kodi. 
> Sameer, please share link here once you post patches.

And who is doing it for other stuff? I think this would be most useful
for games/emulators and such so IMO we should find a way to get it to
the hands of users doing those things.

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

Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Laxminarayan Bharadiya, Pankaj]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: 12 March 2020 19:25
> To: Laxminarayan Bharadiya, Pankaj
> <pankaj.laxminarayan.bharadiya@intel.com>
> Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> Chris Wilson <chris@chris-wilson.co.uk>; Souza, Jose <jose.souza@intel.com>;
> De Marchi, Lucas <lucas.demarchi@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>; Deak, Imre <imre.deak@intel.com>; Shankar,
> Uma <uma.shankar@intel.com>; linux-kernel@vger.kernel.org; Nautiyal, Ankit K
> <ankit.k.nautiyal@intel.com>
> Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based
> integer scaling support
> 
> On Thu, Mar 12, 2020 at 09:13:24AM +0000, Laxminarayan Bharadiya, Pankaj
> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Sent: 10 March 2020 21:47
> > > To: Laxminarayan Bharadiya, Pankaj
> > > <pankaj.laxminarayan.bharadiya@intel.com>
> > > Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> > > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > > airlied@linux.ie; maarten.lankhorst@linux.intel.com;
> > > tzimmermann@suse.de; mripard@kernel.org; mihail.atanassov@arm.com;
> > > Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo
> > > <rodrigo.vivi@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>;
> > > Souza, Jose <jose.souza@intel.com>; De Marchi, Lucas
> > > <lucas.demarchi@intel.com>; Roper, Matthew D
> > > <matthew.d.roper@intel.com>; Deak, Imre <imre.deak@intel.com>;
> > > Shankar, Uma <uma.shankar@intel.com>; linux- kernel@vger.kernel.org;
> > > Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > > Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor
> > > based integer scaling support
> > >
> > > On Tue, Feb 25, 2020 at 12:35:45PM +0530, Pankaj Bharadiya wrote:
> > > > Integer scaling (IS) is a nearest-neighbor upscaling technique
> > > > that simply scales up the existing pixels by an integer (i.e.,
> > > > whole
> > > > number) multiplier.Nearest-neighbor (NN) interpolation works by
> > > > filling in the missing color values in the upscaled image with
> > > > that of the coordinate-mapped nearest source pixel value.
> > > >
> > > > Both IS and NN preserve the clarity of the original image. Integer
> > > > scaling is particularly useful for pixel art games that rely on
> > > > sharp, blocky images to deliver their distinctive look.
> > > >
> > > > Program the scaler filter coefficients to enable the NN filter if
> > > > scaling filter property is set to
> > > > DRM_SCALING_FILTER_NEAREST_NEIGHBOR
> > > > and enable integer scaling.
> > > >
> > > > Bspec: 49247
> > > >
> > > > Signed-off-by: Pankaj Bharadiya
> > > > <pankaj.laxminarayan.bharadiya@intel.com>
> > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_display.c | 83
> > > > +++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_display.h
> > > > +++++++++++++++++++|
> > > > 2 +  drivers/gpu/drm/i915/display/intel_sprite.c  | 20 +++--
> > > >  3 files changed, 97 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index b5903ef3c5a0..6d5f59203258 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -6237,6 +6237,73 @@ void skl_scaler_disable(const struct
> > > intel_crtc_state *old_crtc_state)
> > > >  		skl_detach_scaler(crtc, i);
> > > >  }
> > > >
> > > > +/**
> > > > + *  Theory behind setting nearest-neighbor integer scaling:
> > > > + *
> > > > + *  17 phase of 7 taps requires 119 coefficients in 60 dwords per set.
> > > > + *  The letter represents the filter tap (D is the center tap)
> > > > +and the number
> > > > + *  represents the coefficient set for a phase (0-16).
> > > > + *
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |Index value | Data value coeffient 1 | Data value coeffient 2 |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   00h      |          B0            |          A0            |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   01h      |          D0            |          C0            |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   02h      |          F0            |          E0            |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   03h      |          A1            |          G0            |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   04h      |          C1            |          B1            |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   ...      |          ...           |          ...           |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   38h      |          B16           |          A16           |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   39h      |          D16           |          C16           |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   3Ah      |          F16           |          C16           |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *         |   3Bh      |        Reserved        |          G16           |
> > > > + *         +------------+------------------------+------------------------+
> > > > + *
> > > > + *  To enable nearest-neighbor scaling:  program scaler
> > > > +coefficents with
> > > > + *  the center tap (Dxx) values set to 1 and all other values set
> > > > +to
> > > > +0 as per
> > > > + *  SCALER_COEFFICIENT_FORMAT
> > > > + *
> > > > + */
> > > > +void skl_setup_nearest_neighbor_filter(struct drm_i915_private
> > > *dev_priv,
> > > > +				  enum pipe pipe, int scaler_id)
> > >
> > > skl_scaler_...
> > >
> > > > +{
> > > > +
> > > > +	int coeff = 0;
> > > > +	int phase = 0;
> > > > +	int tap;
> > > > +	int val = 0;
> > >
> > > Needlessly wide scope for most of these.
> > >
> > > > +
> > > > +	/*enable the index auto increment.*/
> > > > +	intel_de_write_fw(dev_priv, SKL_PS_COEF_INDEX_SET0(pipe,
> > > scaler_id),
> > > > +			  _PS_COEE_INDEX_AUTO_INC);
> > > > +
> > > > +	for (phase = 0; phase < 17; phase++) {
> > > > +		for (tap = 0; tap < 7; tap++) {
> > > > +			coeff++;
> > >
> > > Can be part of the % check.
> >
> > OK.
> >
> > >
> > > > +			if (tap == 3)
> > > > +				val = (phase % 2) ? (0x800) : (0x800 << 16);
> > >
> > > Parens overload.
> >
> > OK. Will remove.
> > >
> > > > +
> > > > +			if (coeff % 2 == 0) {
> > > > +				intel_de_write_fw(dev_priv,
> > > SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);
> > > > +				val = 0;
> > >
> > > Can drop this val=0 if you move the variable into tight scope and
> > > initialize there.
> >
> > Moving val=0 initialization to the tight scope will not work here as
> > we need to retain "val" and write only when 2 coefficients are ready
> > (since 2 coefficients are packed in 1 dword).
> >
> > e.g. for (12th , 11th)  coefficients, coefficient reg value should be ( (0 << 16) |
> 0x800).
> > If we initialize val = 0 in tight loop, 0 will be written to  coefficient register.
> 
> Hmm, right. I guess I'd try to rearrange this to iterate the registers directly
> instead of the phases and taps. Something like this perhaps:
> 
> static int cnl_coef_tap(int i)
> {
> 	return i % 7;
> }
> 
> static u16 cnl_coef(int t)

cnl_coef -> cnl_nearest_filter_coef.  Right?

> {
> 	return t == 3 ? 0x0800 : 0x3000;
> }
> 
> static void cnl_program_nearest_filter_coefs(void)
> {
> 	int i;
> 
> 	for (i = 0; i < 17 * 7; i += 2) {
> 		uint32_t tmp;
> 		int t;
> 
> 		t = cnl_coef_tap(i);
> 		tmp = cnl_nearest_filter_coef(t);
> 
> 		t = cnl_coef_tap(i + 1);
> 		tmp |= cnl_nearest_filter_coef(t) << 16;
> 
> 		intel_de_write_fw(tmp);
> 	}
> }
> 
> More readable I think. The downside being all those modulo operations but
> hopefully that's all in the noise when it comes to performance.

Looks better, thanks for spending time on this.
I will try this out.

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

Re: [Intel-gfx] [RFC][PATCH 0/5] Introduce drm scaling filter property

[Pekka Paalanen]

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

On Thu, 12 Mar 2020 18:01:12 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Thu, Mar 12, 2020 at 03:37:03PM +0000, Laxminarayan Bharadiya, Pankaj wrote:
> > 
> >   
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Sent: 12 March 2020 19:35
> > > To: Laxminarayan Bharadiya, Pankaj
> > > <pankaj.laxminarayan.bharadiya@intel.com>
> > > Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> > > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> > > maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> > > mripard@kernel.org; mihail.atanassov@arm.com; linux-
> > > kernel@vger.kernel.org; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > > Subject: Re: [RFC][PATCH 0/5] Introduce drm scaling filter property
> > > 
> > > On Tue, Feb 25, 2020 at 12:35:40PM +0530, Pankaj Bharadiya wrote:  
> > > > Integer scaling (IS) is a nearest-neighbor upscaling technique that
> > > > simply scales up the existing pixels by an integer (i.e., whole
> > > > number) multiplier. Nearest-neighbor (NN) interpolation works by
> > > > filling in the missing color values in the upscaled image with that of
> > > > the coordinate-mapped nearest source pixel value.
> > > >
> > > > Both IS and NN preserve the clarity of the original image. In
> > > > contrast, traditional upscaling algorithms, such as bilinear or
> > > > bicubic interpolation, result in blurry upscaled images because they
> > > > employ interpolation techniques that smooth out the transition from
> > > > one pixel to another.  Therefore, integer scaling is particularly
> > > > useful for pixel art games that rely on sharp, blocky images to
> > > > deliver their distinctive look.
> > > >
> > > > 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-i
> > > > ntel-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 -
> > > >   - Introduces new scaling filter property to allow userspace to
> > > >     select  the driver's default scaling filter or Nearest-neighbor(NN)
> > > >     filter for scaling operations on crtc/plane.
> > > >   - Implements and enable integer scaling for i915
> > > >
> > > > Userspace patch series link: TBD.  
> > > 
> > > That needs to be done or this will go nowhere.  
> > 
> > Yes, Sameer is working on enabling this feature in Kodi. 
> > Sameer, please share link here once you post patches.  
> 
> And who is doing it for other stuff? I think this would be most useful
> for games/emulators and such so IMO we should find a way to get it to
> the hands of users doing those things.
> 

Hi,

FWIW, being able to tell KMS to use nearest-neighbor filtering could be
useful for
https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/394
as a follow-up.


Thanks,
pq

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

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

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

Re: [Intel-gfx] [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based integer scaling support

[Ville Syrjälä]

On Fri, Mar 13, 2020 at 08:45:35AM +0000, Laxminarayan Bharadiya, Pankaj wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 12 March 2020 19:25
> > To: Laxminarayan Bharadiya, Pankaj
> > <pankaj.laxminarayan.bharadiya@intel.com>
> > Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; airlied@linux.ie;
> > maarten.lankhorst@linux.intel.com; tzimmermann@suse.de;
> > mripard@kernel.org; mihail.atanassov@arm.com; Joonas Lahtinen
> > <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> > Chris Wilson <chris@chris-wilson.co.uk>; Souza, Jose <jose.souza@intel.com>;
> > De Marchi, Lucas <lucas.demarchi@intel.com>; Roper, Matthew D
> > <matthew.d.roper@intel.com>; Deak, Imre <imre.deak@intel.com>; Shankar,
> > Uma <uma.shankar@intel.com>; linux-kernel@vger.kernel.org; Nautiyal, Ankit K
> > <ankit.k.nautiyal@intel.com>
> > Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor based
> > integer scaling support
> > 
> > On Thu, Mar 12, 2020 at 09:13:24AM +0000, Laxminarayan Bharadiya, Pankaj
> > wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Sent: 10 March 2020 21:47
> > > > To: Laxminarayan Bharadiya, Pankaj
> > > > <pankaj.laxminarayan.bharadiya@intel.com>
> > > > Cc: jani.nikula@linux.intel.com; daniel@ffwll.ch; intel-
> > > > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > > > airlied@linux.ie; maarten.lankhorst@linux.intel.com;
> > > > tzimmermann@suse.de; mripard@kernel.org; mihail.atanassov@arm.com;
> > > > Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo
> > > > <rodrigo.vivi@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>;
> > > > Souza, Jose <jose.souza@intel.com>; De Marchi, Lucas
> > > > <lucas.demarchi@intel.com>; Roper, Matthew D
> > > > <matthew.d.roper@intel.com>; Deak, Imre <imre.deak@intel.com>;
> > > > Shankar, Uma <uma.shankar@intel.com>; linux- kernel@vger.kernel.org;
> > > > Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> > > > Subject: Re: [RFC][PATCH 5/5] drm/i915/display: Add Nearest-neighbor
> > > > based integer scaling support
> > > >
> > > > On Tue, Feb 25, 2020 at 12:35:45PM +0530, Pankaj Bharadiya wrote:
> > > > > Integer scaling (IS) is a nearest-neighbor upscaling technique
> > > > > that simply scales up the existing pixels by an integer (i.e.,
> > > > > whole
> > > > > number) multiplier.Nearest-neighbor (NN) interpolation works by
> > > > > filling in the missing color values in the upscaled image with
> > > > > that of the coordinate-mapped nearest source pixel value.
> > > > >
> > > > > Both IS and NN preserve the clarity of the original image. Integer
> > > > > scaling is particularly useful for pixel art games that rely on
> > > > > sharp, blocky images to deliver their distinctive look.
> > > > >
> > > > > Program the scaler filter coefficients to enable the NN filter if
> > > > > scaling filter property is set to
> > > > > DRM_SCALING_FILTER_NEAREST_NEIGHBOR
> > > > > and enable integer scaling.
> > > > >
> > > > > Bspec: 49247
> > > > >
> > > > > Signed-off-by: Pankaj Bharadiya
> > > > > <pankaj.laxminarayan.bharadiya@intel.com>
> > > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_display.c | 83
> > > > > +++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_display.h
> > > > > +++++++++++++++++++|
> > > > > 2 +  drivers/gpu/drm/i915/display/intel_sprite.c  | 20 +++--
> > > > >  3 files changed, 97 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index b5903ef3c5a0..6d5f59203258 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -6237,6 +6237,73 @@ void skl_scaler_disable(const struct
> > > > intel_crtc_state *old_crtc_state)
> > > > >  		skl_detach_scaler(crtc, i);
> > > > >  }
> > > > >
> > > > > +/**
> > > > > + *  Theory behind setting nearest-neighbor integer scaling:
> > > > > + *
> > > > > + *  17 phase of 7 taps requires 119 coefficients in 60 dwords per set.
> > > > > + *  The letter represents the filter tap (D is the center tap)
> > > > > +and the number
> > > > > + *  represents the coefficient set for a phase (0-16).
> > > > > + *
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |Index value | Data value coeffient 1 | Data value coeffient 2 |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   00h      |          B0            |          A0            |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   01h      |          D0            |          C0            |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   02h      |          F0            |          E0            |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   03h      |          A1            |          G0            |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   04h      |          C1            |          B1            |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   ...      |          ...           |          ...           |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   38h      |          B16           |          A16           |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   39h      |          D16           |          C16           |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   3Ah      |          F16           |          C16           |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *         |   3Bh      |        Reserved        |          G16           |
> > > > > + *         +------------+------------------------+------------------------+
> > > > > + *
> > > > > + *  To enable nearest-neighbor scaling:  program scaler
> > > > > +coefficents with
> > > > > + *  the center tap (Dxx) values set to 1 and all other values set
> > > > > +to
> > > > > +0 as per
> > > > > + *  SCALER_COEFFICIENT_FORMAT
> > > > > + *
> > > > > + */
> > > > > +void skl_setup_nearest_neighbor_filter(struct drm_i915_private
> > > > *dev_priv,
> > > > > +				  enum pipe pipe, int scaler_id)
> > > >
> > > > skl_scaler_...
> > > >
> > > > > +{
> > > > > +
> > > > > +	int coeff = 0;
> > > > > +	int phase = 0;
> > > > > +	int tap;
> > > > > +	int val = 0;
> > > >
> > > > Needlessly wide scope for most of these.
> > > >
> > > > > +
> > > > > +	/*enable the index auto increment.*/
> > > > > +	intel_de_write_fw(dev_priv, SKL_PS_COEF_INDEX_SET0(pipe,
> > > > scaler_id),
> > > > > +			  _PS_COEE_INDEX_AUTO_INC);
> > > > > +
> > > > > +	for (phase = 0; phase < 17; phase++) {
> > > > > +		for (tap = 0; tap < 7; tap++) {
> > > > > +			coeff++;
> > > >
> > > > Can be part of the % check.
> > >
> > > OK.
> > >
> > > >
> > > > > +			if (tap == 3)
> > > > > +				val = (phase % 2) ? (0x800) : (0x800 << 16);
> > > >
> > > > Parens overload.
> > >
> > > OK. Will remove.
> > > >
> > > > > +
> > > > > +			if (coeff % 2 == 0) {
> > > > > +				intel_de_write_fw(dev_priv,
> > > > SKL_PS_COEF_DATA_SET0(pipe, scaler_id), val);
> > > > > +				val = 0;
> > > >
> > > > Can drop this val=0 if you move the variable into tight scope and
> > > > initialize there.
> > >
> > > Moving val=0 initialization to the tight scope will not work here as
> > > we need to retain "val" and write only when 2 coefficients are ready
> > > (since 2 coefficients are packed in 1 dword).
> > >
> > > e.g. for (12th , 11th)  coefficients, coefficient reg value should be ( (0 << 16) |
> > 0x800).
> > > If we initialize val = 0 in tight loop, 0 will be written to  coefficient register.
> > 
> > Hmm, right. I guess I'd try to rearrange this to iterate the registers directly
> > instead of the phases and taps. Something like this perhaps:
> > 
> > static int cnl_coef_tap(int i)
> > {
> > 	return i % 7;
> > }
> > 
> > static u16 cnl_coef(int t)
> 
> cnl_coef -> cnl_nearest_filter_coef.  Right?

Right.

> 
> > {
> > 	return t == 3 ? 0x0800 : 0x3000;
> > }
> > 
> > static void cnl_program_nearest_filter_coefs(void)
> > {
> > 	int i;
> > 
> > 	for (i = 0; i < 17 * 7; i += 2) {
> > 		uint32_t tmp;
> > 		int t;
> > 
> > 		t = cnl_coef_tap(i);
> > 		tmp = cnl_nearest_filter_coef(t);
> > 
> > 		t = cnl_coef_tap(i + 1);
> > 		tmp |= cnl_nearest_filter_coef(t) << 16;
> > 
> > 		intel_de_write_fw(tmp);
> > 	}
> > }
> > 
> > More readable I think. The downside being all those modulo operations but
> > hopefully that's all in the noise when it comes to performance.
> 
> Looks better, thanks for spending time on this.
> I will try this out.
> 
> Thanks,
> Pankaj 
> > 
> > --
> > Ville Syrjälä
> > Intel

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

Re: [Intel-gfx] [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Daniel Vetter]

On Tue, Mar 10, 2020 at 06:01:06PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 25, 2020 at 12:35:41PM +0530, Pankaj Bharadiya wrote:
> > Introduce new scaling filter property to allow userspace to select
> > the driver's default scaling filter or Nearest-neighbor(NN) filter
> > for upscaling operations on crtc/plane.
> > 
> > Drivers can set up this property for a plane by calling
> > drm_plane_enable_scaling_filter() and for a CRTC by calling
> > drm_crtc_enable_scaling_filter().
> > 
> > NN filter works by filling in the missing color values in the upscaled
> > image with that of the coordinate-mapped nearest source pixel value.
> > 
> > NN filter for integer multiple scaling can be particularly useful for
> > for pixel art games that rely on sharp, blocky images to deliver their
> > distinctive look.
> > 
> > Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  8 +++++++
> >  drivers/gpu/drm/drm_crtc.c        | 16 ++++++++++++++
> >  drivers/gpu/drm/drm_mode_config.c | 13 ++++++++++++
> >  drivers/gpu/drm/drm_plane.c       | 35 +++++++++++++++++++++++++++++++
> >  include/drm/drm_crtc.h            | 10 +++++++++
> >  include/drm/drm_mode_config.h     |  6 ++++++
> >  include/drm/drm_plane.h           | 14 +++++++++++++
> >  7 files changed, 102 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index a1e5e262bae2..4e3c1f3176e4 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;
> 
> I think we want a per-plane/per-crtc prop for this. If we start adding
> more filters we are surely going to need different sets for different hw
> blocks.

In the past we've only done that once we have a demonstrated need. Usually
the patch to move the property to a per-object location isn't a lot of
churn.
-Daniel

> 
> >  	} 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
> > @@ -583,6 +587,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
> >  					sizeof(struct drm_rect),
> >  					&replaced);
> >  		return ret;
> > +	} else if (property == config->scaling_filter_property) {
> > +		state->scaling_filter = val;
> >  	} else if (plane->funcs->atomic_set_property) {
> >  		return plane->funcs->atomic_set_property(plane, state,
> >  				property, val);
> > @@ -641,6 +647,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> >  	} else if (property == config->prop_fb_damage_clips) {
> >  		*val = (state->fb_damage_clips) ?
> >  			state->fb_damage_clips->base.id : 0;
> > +	} else if (property == config->scaling_filter_property) {
> > +		*val = state->scaling_filter;
> >  	} else if (plane->funcs->atomic_get_property) {
> >  		return plane->funcs->atomic_get_property(plane, state, property, val);
> >  	} else {
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 4936e1080e41..1ce7b2ac9eb5 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -748,3 +748,19 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
> >  
> >  	return ret;
> >  }
> > +
> > +/**
> > + * drm_crtc_enable_scaling_filter - Enables crtc scaling filter property.
> > + * @crtc: CRTC on which to enable scaling filter property.
> > + *
> > + * This function lets driver to enable the scaling filter property on a crtc.
> > + */
> > +void drm_crtc_enable_scaling_filter(struct drm_crtc *crtc)
> > +{
> > +	struct drm_device *dev = crtc->dev;
> > +
> > +	drm_object_attach_property(&crtc->base,
> > +				   dev->mode_config.scaling_filter_property,
> > +				   0);
> > +}
> > +EXPORT_SYMBOL(drm_crtc_enable_scaling_filter);
> > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> > index 08e6eff6a179..1024a8d1cd5d 100644
> > --- a/drivers/gpu/drm/drm_mode_config.c
> > +++ b/drivers/gpu/drm/drm_mode_config.c
> > @@ -214,6 +214,11 @@ static const struct drm_prop_enum_list drm_plane_type_enum_list[] = {
> >  	{ DRM_PLANE_TYPE_CURSOR, "Cursor" },
> >  };
> >  
> > +static const struct drm_prop_enum_list drm_scaling_filter_enum_list[] = {
> > +	{ DRM_SCALING_FILTER_DEFAULT, "Default" },
> > +	{ DRM_SCALING_FILTER_NEAREST_NEIGHBOR, "Nearest Neighbor" },
> > +};
> > +
> >  static int drm_mode_create_standard_properties(struct drm_device *dev)
> >  {
> >  	struct drm_property *prop;
> > @@ -370,6 +375,14 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> >  		return -ENOMEM;
> >  	dev->mode_config.modifiers_property = prop;
> >  
> > +	prop = drm_property_create_enum(dev, 0,
> > +				"SCALING_FILTER",
> > +				drm_scaling_filter_enum_list,
> > +				ARRAY_SIZE(drm_scaling_filter_enum_list));
> > +	if (!prop)
> > +		return -ENOMEM;
> > +	dev->mode_config.scaling_filter_property = prop;
> > +
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index d6ad60ab0d38..ace7ee2775c8 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -1221,3 +1221,38 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> >  
> >  	return ret;
> >  }
> > +
> > +/**
> > + * DOC: Scaling filter property
> > + *
> > + *
> > + * SCALING_FILTER:
> > + *
> > + *	Indicates scaling filter to be used for CRTC/plane scaler
> > + *
> > + *	The value of this property can be one of the following:
> > + *	Default:
> > + *		Driver's default scaling filter
> > + *	Nearest Neighbor:
> > + *		Nearest Neighbor scaling filter
> > + *
> > + * Drivers can set up this property for a plane by calling
> > + * drm_plane_enable_scaling_filter() and for a CRTC by calling
> > + * drm_crtc_enable_scaling_filter()
> > + */
> > +
> > +/**
> > + * drm_plane_enable_scaling_filter - Enables plane scaling filter property.
> > + * @plane: Plane on which to enable scaling filter property.
> > + *
> > + * This function lets driver to enable the scaling filter property on a plane.
> > + */
> > +void drm_plane_enable_scaling_filter(struct drm_plane *plane)
> > +{
> > +	struct drm_device *dev = plane->dev;
> > +
> > +	drm_object_attach_property(&plane->base,
> > +				   dev->mode_config.scaling_filter_property,
> > +				   0);
> > +}
> > +EXPORT_SYMBOL(drm_plane_enable_scaling_filter);
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 59b51a09cae6..770f9328a5ba 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -58,6 +58,7 @@ struct device_node;
> >  struct dma_fence;
> >  struct edid;
> >  
> > +
> >  static inline int64_t U642I64(uint64_t val)
> >  {
> >  	return (int64_t)*((int64_t *)&val);
> > @@ -296,6 +297,13 @@ struct drm_crtc_state {
> >  	 */
> >  	u32 target_vblank;
> >  
> > +	/**
> > +	 * @scaling_filter:
> > +	 *
> > +	 * Scaling filter mode to be applied
> > +	 */
> > +	enum drm_scaling_filter scaling_filter;
> > +
> >  	/**
> >  	 * @async_flip:
> >  	 *
> > @@ -1266,4 +1274,6 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
> >  #define drm_for_each_crtc(crtc, dev) \
> >  	list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
> >  
> > +void drm_crtc_enable_scaling_filter(struct drm_crtc *crtc);
> > +
> >  #endif /* __DRM_CRTC_H__ */
> > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> > index 3bcbe30339f0..8c308ae1056d 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/plane property to apply a particular
> > +	 * filter while scaling.
> > +	 */
> > +	struct drm_property *scaling_filter_property;
> > +
> >  	/* cursor size */
> >  	uint32_t cursor_width, cursor_height;
> >  
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> > index 3f396d94afe4..2bc665cc6071 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -35,6 +35,11 @@ struct drm_crtc;
> >  struct drm_printer;
> >  struct drm_modeset_acquire_ctx;
> >  
> > +
> > +enum drm_scaling_filter {
> > +	DRM_SCALING_FILTER_DEFAULT,
> > +	DRM_SCALING_FILTER_NEAREST_NEIGHBOR,
> > +};
> >  /**
> >   * struct drm_plane_state - mutable plane state
> >   *
> > @@ -214,6 +219,13 @@ struct drm_plane_state {
> >  	 */
> >  	bool visible;
> >  
> > +	/**
> > +	 * @scaling_filter:
> > +	 *
> > +	 * Scaling filter mode to be applied
> > +	 */
> > +	enum drm_scaling_filter scaling_filter;
> > +
> >  	/**
> >  	 * @commit: Tracks the pending commit to prevent use-after-free conditions,
> >  	 * and for async plane updates.
> > @@ -862,4 +874,6 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state)
> >  					state->fb_damage_clips->data : NULL);
> >  }
> >  
> > +void drm_plane_enable_scaling_filter(struct drm_plane *plane);
> > +
> >  #endif
> > -- 
> > 2.23.0
> 
> -- 
> Ville Syrjälä
> Intel

-- 
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] [RFC][PATCH 1/5] drm: Introduce scaling filter property

[Ville Syrjälä]

On Mon, Mar 16, 2020 at 09:31:32AM +0100, Daniel Vetter wrote:
> On Tue, Mar 10, 2020 at 06:01:06PM +0200, Ville Syrjälä wrote:
> > On Tue, Feb 25, 2020 at 12:35:41PM +0530, Pankaj Bharadiya wrote:
> > > Introduce new scaling filter property to allow userspace to select
> > > the driver's default scaling filter or Nearest-neighbor(NN) filter
> > > for upscaling operations on crtc/plane.
> > > 
> > > Drivers can set up this property for a plane by calling
> > > drm_plane_enable_scaling_filter() and for a CRTC by calling
> > > drm_crtc_enable_scaling_filter().
> > > 
> > > NN filter works by filling in the missing color values in the upscaled
> > > image with that of the coordinate-mapped nearest source pixel value.
> > > 
> > > NN filter for integer multiple scaling can be particularly useful for
> > > for pixel art games that rely on sharp, blocky images to deliver their
> > > distinctive look.
> > > 
> > > Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_uapi.c |  8 +++++++
> > >  drivers/gpu/drm/drm_crtc.c        | 16 ++++++++++++++
> > >  drivers/gpu/drm/drm_mode_config.c | 13 ++++++++++++
> > >  drivers/gpu/drm/drm_plane.c       | 35 +++++++++++++++++++++++++++++++
> > >  include/drm/drm_crtc.h            | 10 +++++++++
> > >  include/drm/drm_mode_config.h     |  6 ++++++
> > >  include/drm/drm_plane.h           | 14 +++++++++++++
> > >  7 files changed, 102 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index a1e5e262bae2..4e3c1f3176e4 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;
> > 
> > I think we want a per-plane/per-crtc prop for this. If we start adding
> > more filters we are surely going to need different sets for different hw
> > blocks.
> 
> In the past we've only done that once we have a demonstrated need. Usually
> the patch to move the property to a per-object location isn't a lot of
> churn.

Seems silly to not do it from the start when we already know there is
hardware out there that has different capabilities per hw block.

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