[PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Nabendu Maiti]

Following patch series add pipe scaler functionality in atomic path.The pipe
scaler can be changed dynamically without modeset.Apart from default panel
fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
mode is added as there is no such restriction on GEn9 pipe scaler.



Nabendu Maiti (7):
  drm/i915: Add pipe scaler pipe source drm property
  drm/i915: Add pipe_src size property calculations in atomic path.
  drm/i915: Panel fitting calculation for GEN9
  drm/i915: Adding atomic fitting mode property for GEN9
  drm/i915: Add pipe scaler co-ordinate and size property for Gen9
  drm/i915: Update pipe-scaler according to destination size
  drm/i915: Pipescaler destination size limit check on Gen9

 drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
 drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |   3 +
 drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
 include/drm/drm_crtc.h               |  14 ++++
 include/uapi/drm/drm_mode.h          |   1 +
 7 files changed, 263 insertions(+), 8 deletions(-)

-- 
1.9.1

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

[PATCH 1/7] drm/i915: Add pipe scaler pipe source drm property

[Nabendu Maiti]

Initialization of pipe source size property as intel drm property to drm level
to dynamically change pipe source size.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 10 ++++++++++
 drivers/gpu/drm/drm_crtc.c   | 16 ++++++++++++++++
 include/drm/drm_crtc.h       |  7 +++++++
 3 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 0b8f33d..066f010 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -493,6 +493,12 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 					&replaced);
 		state->color_mgmt_changed |= replaced;
 		return ret;
+	} else if (property == config->prop_pipe_src_w) {
+		state->src_w = val;
+		state->pipescaler_changed = true;
+	} else if (property == config->prop_pipe_src_h) {
+		state->src_h = val;
+		state->pipescaler_changed = true;
 	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
@@ -535,6 +541,10 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = (state->ctm) ? state->ctm->base.id : 0;
 	else if (property == config->gamma_lut_property)
 		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
+	else if (property == config->prop_pipe_src_w)
+		*val =  state->src_w;
+	else if (property == config->prop_pipe_src_h)
+		*val =  state->src_h;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9316a2b..6fd6dd8 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -377,6 +377,10 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
 		drm_object_attach_property(&crtc->base, config->prop_active, 0);
 		drm_object_attach_property(&crtc->base, config->prop_mode_id, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->prop_pipe_src_w, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->prop_pipe_src_h, 0);
 	}
 
 	return 0;
@@ -943,6 +947,18 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.gamma_lut_size_property = prop;
 
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+			"PIPE_SRC_W", 0, INT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_pipe_src_w = prop;
+
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+			"PIPE_SRC_H", 0, INT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_pipe_src_h = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index bb214a1..4d6b580 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -197,6 +197,9 @@ struct drm_crtc_state {
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
+
+	bool pipescaler_changed;
+	u32 src_w, src_h;
 };
 
 /**
@@ -1942,6 +1945,10 @@ struct drm_mode_config {
 	 */
 	struct drm_property *prop_mode_id;
 
+	/* pipe scaler properties */
+	struct drm_property *prop_pipe_src_w;
+	struct drm_property *prop_pipe_src_h;
+
 	/**
 	 * @dvi_i_subconnector_property: Optional DVI-I property to
 	 * differentiate between analog or digital mode.
-- 
1.9.1

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

[PATCH 2/7] drm/i915: Add pipe_src size property calculations in atomic path.

[Nabendu Maiti]

Adding pipe source size property calculations on atomic path to
dynamically change pipe source size.

Write desired values to change the size. Write 0 to disable pipe scaler
and restore the original mode adjusted values.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 2 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e4e6141..ff5dea8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12548,6 +12548,55 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
 	return true;
 }
 
+static int skylake_pfiter_calculate(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state)
+{
+	struct drm_device *dev = crtc->dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *pipe_config =
+		to_intel_crtc_state(crtc_state);
+	bool mode_changed = needs_modeset(crtc_state);
+	struct intel_connector *intel_connector;
+	int ret = 0;
+	struct drm_display_mode *adjusted_mode =
+		&intel_crtc->config->base.adjusted_mode;
+
+	for_each_intel_connector(dev, intel_connector) {
+		if (!(intel_connector) || !(intel_connector->encoder) ||
+		    (intel_connector->encoder->base.crtc  != crtc))
+			continue;
+
+		if (((pipe_config->pipe_src_w !=
+		      intel_crtc->config->pipe_src_w) ||
+		     (pipe_config->pipe_src_h !=
+		      intel_crtc->config->pipe_src_h)) && (!mode_changed))
+			pipe_config->update_pipe = true;
+
+		if ((pipe_config->pipe_scaling_mode !=
+		    intel_connector->panel.fitting_mode) &&
+		    (!mode_changed)) {
+			if  ((adjusted_mode->hdisplay !=
+			      pipe_config->pipe_src_w) ||
+			     (adjusted_mode->vdisplay !=
+			      pipe_config->pipe_src_h)) {
+				pipe_config->pipe_scaling_mode =
+					intel_connector->panel.fitting_mode;
+				pipe_config->update_pipe = true;
+			}
+		}
+
+		if ((mode_changed) || (pipe_config->update_pipe)) {
+			ret = skl_update_scaler_crtc(pipe_config);
+			if (ret)
+				break;
+			intel_pch_panel_fitting(intel_crtc, pipe_config,
+					intel_connector->panel.fitting_mode);
+			break;
+		}
+	}
+	return ret;
+}
+
 static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 				   struct drm_crtc_state *crtc_state)
 {
@@ -12616,9 +12665,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 	}
 
 	if (INTEL_INFO(dev)->gen >= 9) {
-		if (mode_changed)
-			ret = skl_update_scaler_crtc(pipe_config);
-
+		ret = skylake_pfiter_calculate(crtc, crtc_state);
 		if (!ret)
 			ret = intel_atomic_setup_scalers(dev, intel_crtc,
 							 pipe_config);
@@ -13995,6 +14042,21 @@ static int intel_atomic_check(struct drm_device *dev,
 		struct intel_crtc_state *pipe_config =
 			to_intel_crtc_state(crtc_state);
 
+		if (crtc_state->pipescaler_changed) {
+			if (crtc_state->src_w == 0 && crtc_state->src_h == 0) {
+				struct intel_crtc *intel_crtc =
+					to_intel_crtc(crtc);
+				struct drm_display_mode *adjusted_mode =
+					&intel_crtc->config->base.adjusted_mode;
+
+				crtc_state->src_w = adjusted_mode->hdisplay;
+				crtc_state->src_h = adjusted_mode->vdisplay;
+			}
+			pipe_config->pipe_src_w = crtc_state->src_w;
+			pipe_config->pipe_src_h = crtc_state->src_h;
+			crtc_state->pipescaler_changed = false;
+		}
+
 		/* Catch I915_MODE_FLAG_INHERITED */
 		if (crtc_state->mode.private_flags != crtc->state->mode.private_flags)
 			crtc_state->mode_changed = true;
@@ -14988,6 +15050,7 @@ fail:
 	return NULL;
 }
 
+
 void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
 {
 	if (!dev->mode_config.rotation_property) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 570a7ca..189856c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -532,6 +532,7 @@ struct intel_crtc_state {
 	 * All planes will be positioned inside this space,
 	 * and get clipped at the edges. */
 	int pipe_src_w, pipe_src_h;
+	u32 pipe_scaling_mode;
 
 	/* Whether to set up the PCH/FDI. Note that we never allow sharing
 	 * between pch encoders and cpu encoders. */
-- 
1.9.1

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

[PATCH 3/7] drm/i915: Panel fitting calculation for GEN9

[Nabendu Maiti]

No boundary condition was checked while calculating pipe scaler size
in pch_panel_fitting(), which might result in blank out or
corruptions for invalid values. This patch fixes this by adding
appropriate checks and calculation for each fitting mode.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/i915/intel_panel.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 96c65d7..b5ea1b6 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -107,18 +107,36 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 {
 	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	int x = 0, y = 0, width = 0, height = 0;
+	bool downscale = false;
 
 	/* Native modes don't need fitting */
 	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
 	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h)
 		goto done;
 
+	/* Downscale pfiter */
+	if (IS_GEN9(intel_crtc->base.dev) &&
+	     (adjusted_mode->hdisplay < pipe_config->pipe_src_w ||
+	    adjusted_mode->vdisplay < pipe_config->pipe_src_h))
+		downscale = true;
+
 	switch (fitting_mode) {
 	case DRM_MODE_SCALE_CENTER:
 		width = pipe_config->pipe_src_w;
 		height = pipe_config->pipe_src_h;
 		x = (adjusted_mode->crtc_hdisplay - width + 1)/2;
 		y = (adjusted_mode->crtc_vdisplay - height + 1)/2;
+
+		if (downscale) {
+			if (x < 0) {
+				x = 0;
+				width = adjusted_mode->hdisplay;
+			}
+			if (y < 0) {
+				y = 0;
+				height = adjusted_mode->vdisplay;
+			}
+		}
 		break;
 
 	case DRM_MODE_SCALE_ASPECT:
@@ -132,14 +150,26 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
 				width = scaled_height / pipe_config->pipe_src_h;
 				if (width & 1)
 					width++;
-				x = (adjusted_mode->crtc_hdisplay - width + 1) / 2;
+				if (adjusted_mode->hdisplay > width) {
+					x = (adjusted_mode->hdisplay -
+					     width + 1) / 2;
+				} else if (downscale) {
+					width = adjusted_mode->hdisplay;
+					x = 0;
+				}
 				y = 0;
 				height = adjusted_mode->crtc_vdisplay;
 			} else if (scaled_width < scaled_height) { /* letter */
 				height = scaled_width / pipe_config->pipe_src_w;
 				if (height & 1)
 				    height++;
-				y = (adjusted_mode->crtc_vdisplay - height + 1) / 2;
+				if (adjusted_mode->vdisplay > height) {
+					y = (adjusted_mode->vdisplay -
+					     height + 1) / 2;
+				} else if (downscale) {
+					height = adjusted_mode->vdisplay;
+					y = 0;
+				}
 				x = 0;
 				width = adjusted_mode->crtc_hdisplay;
 			} else {
-- 
1.9.1

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

[PATCH 4/7] drm/i915: Adding atomic fitting mode property for GEN9

[Nabendu Maiti]

Pipe scaler Fitting mode property is added as crtc atomic property.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/drm_atomic.c         |  5 ++++
 drivers/gpu/drm/drm_crtc.c           |  8 ++++++
 drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++--------------------------
 include/drm/drm_crtc.h               |  2 ++
 4 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 066f010..3fddfa4 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -499,6 +499,9 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 	} else if (property == config->prop_pipe_src_h) {
 		state->src_h = val;
 		state->pipescaler_changed = true;
+	} else if (property == config->prop_fitting_mode) {
+		state->fitting_mode = val;
+		state->pipescaler_changed = true;
 	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
@@ -545,6 +548,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val =  state->src_w;
 	else if (property == config->prop_pipe_src_h)
 		*val =  state->src_h;
+	else if (property == config->prop_fitting_mode)
+		*val = state->fitting_mode;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6fd6dd8..b24e839 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -381,6 +381,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 					   config->prop_pipe_src_w, 0);
 		drm_object_attach_property(&crtc->base,
 					   config->prop_pipe_src_h, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->prop_fitting_mode, 0);
 	}
 
 	return 0;
@@ -959,6 +961,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_pipe_src_h = prop;
 
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+			"SCALER_MODE", 0, INT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_fitting_mode = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ff5dea8..e3ecb8b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12551,48 +12551,25 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
 static int skylake_pfiter_calculate(struct drm_crtc *crtc,
 				   struct drm_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_crtc_state *pipe_config =
 		to_intel_crtc_state(crtc_state);
 	bool mode_changed = needs_modeset(crtc_state);
-	struct intel_connector *intel_connector;
 	int ret = 0;
-	struct drm_display_mode *adjusted_mode =
-		&intel_crtc->config->base.adjusted_mode;
-
-	for_each_intel_connector(dev, intel_connector) {
-		if (!(intel_connector) || !(intel_connector->encoder) ||
-		    (intel_connector->encoder->base.crtc  != crtc))
-			continue;
 
-		if (((pipe_config->pipe_src_w !=
-		      intel_crtc->config->pipe_src_w) ||
-		     (pipe_config->pipe_src_h !=
-		      intel_crtc->config->pipe_src_h)) && (!mode_changed))
-			pipe_config->update_pipe = true;
-
-		if ((pipe_config->pipe_scaling_mode !=
-		    intel_connector->panel.fitting_mode) &&
-		    (!mode_changed)) {
-			if  ((adjusted_mode->hdisplay !=
-			      pipe_config->pipe_src_w) ||
-			     (adjusted_mode->vdisplay !=
-			      pipe_config->pipe_src_h)) {
-				pipe_config->pipe_scaling_mode =
-					intel_connector->panel.fitting_mode;
-				pipe_config->update_pipe = true;
-			}
-		}
+	if (((pipe_config->pipe_scaling_mode !=
+	      intel_crtc->config->pipe_scaling_mode) ||
+	     (pipe_config->pipe_src_w != intel_crtc->config->pipe_src_w) ||
+	     (pipe_config->pipe_src_h != intel_crtc->config->pipe_src_h)) &&
+	    (!mode_changed)) {
+		pipe_config->update_pipe = true;
+	}
 
-		if ((mode_changed) || (pipe_config->update_pipe)) {
-			ret = skl_update_scaler_crtc(pipe_config);
-			if (ret)
-				break;
+	if ((mode_changed) || (pipe_config->update_pipe)) {
+		ret = skl_update_scaler_crtc(pipe_config);
+		if (!ret)
 			intel_pch_panel_fitting(intel_crtc, pipe_config,
-					intel_connector->panel.fitting_mode);
-			break;
-		}
+						pipe_config->pipe_scaling_mode);
 	}
 	return ret;
 }
@@ -14054,6 +14031,8 @@ static int intel_atomic_check(struct drm_device *dev,
 			}
 			pipe_config->pipe_src_w = crtc_state->src_w;
 			pipe_config->pipe_src_h = crtc_state->src_h;
+			pipe_config->pipe_scaling_mode =
+				crtc_state->fitting_mode;
 			crtc_state->pipescaler_changed = false;
 		}
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 4d6b580..e245746 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -200,6 +200,7 @@ struct drm_crtc_state {
 
 	bool pipescaler_changed;
 	u32 src_w, src_h;
+	u32 fitting_mode;
 };
 
 /**
@@ -1948,6 +1949,7 @@ struct drm_mode_config {
 	/* pipe scaler properties */
 	struct drm_property *prop_pipe_src_w;
 	struct drm_property *prop_pipe_src_h;
+	struct drm_property *prop_fitting_mode;
 
 	/**
 	 * @dvi_i_subconnector_property: Optional DVI-I property to
-- 
1.9.1

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

[PATCH 5/7] drm/i915: Add pipe scaler co-ordinate and size property for Gen9

[Nabendu Maiti]

Adding pipe destination co-ordinate and size property as crtc atomic drm
property to dynamically change pipe destination attributes on GEN9.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/drm_atomic.c         | 20 +++++++++++
 drivers/gpu/drm/drm_crtc.c           | 32 +++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 70 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 include/drm/drm_crtc.h               |  5 +++
 include/uapi/drm/drm_mode.h          |  1 +
 6 files changed, 126 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3fddfa4..3df78c0 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -502,6 +502,18 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 	} else if (property == config->prop_fitting_mode) {
 		state->fitting_mode = val;
 		state->pipescaler_changed = true;
+	} else if (property == config->prop_pipe_dst_x) {
+		state->dst_x = val;
+		state->pipescaler_changed = true;
+	} else if (property == config->prop_pipe_dst_y) {
+		state->dst_y = val;
+		state->pipescaler_changed = true;
+	} else if (property == config->prop_pipe_dst_w) {
+		state->dst_w = val;
+		state->pipescaler_changed = true;
+	} else if (property == config->prop_pipe_dst_h) {
+		state->dst_h = val;
+		state->pipescaler_changed = true;
 	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
@@ -550,6 +562,14 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val =  state->src_h;
 	else if (property == config->prop_fitting_mode)
 		*val = state->fitting_mode;
+	else if (property == config->prop_pipe_dst_x)
+		*val = state->dst_x;
+	else if (property == config->prop_pipe_dst_y)
+		*val = state->dst_y;
+	else if (property == config->prop_pipe_dst_w)
+		*val = state->dst_w;
+	else if (property == config->prop_pipe_dst_h)
+		*val = state->dst_h;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b24e839..c941f58 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -383,6 +383,14 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
 					   config->prop_pipe_src_h, 0);
 		drm_object_attach_property(&crtc->base,
 					   config->prop_fitting_mode, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->prop_pipe_dst_x, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->prop_pipe_dst_y, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->prop_pipe_dst_w, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->prop_pipe_dst_h, 0);
 	}
 
 	return 0;
@@ -967,6 +975,30 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_fitting_mode = prop;
 
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+			"PIPE_DST_X", 0, INT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_pipe_dst_x = prop;
+
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+			"PIPE_DST_Y", 0, INT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_pipe_dst_y = prop;
+
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+			"PIPE_DST_W", 0, INT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_pipe_dst_w = prop;
+
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+			"PIPE_DST_H", 0, INT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.prop_pipe_dst_h = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e3ecb8b..9c7434f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12548,6 +12548,35 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
 	return true;
 }
 
+void
+intel_gen9_pipe_scale(struct intel_crtc *intel_crtc,
+			struct intel_crtc_state *pipe_config,
+			int fitting_mode)
+{
+	const struct drm_display_mode *adjusted_mode =
+		&pipe_config->base.adjusted_mode;
+	int x = 0, y = 0, width = 0, height = 0;
+
+	/* Native modes don't need fitting */
+	if ((adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
+	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) &&
+	   ((pipe_config->pipe_dst_x + pipe_config->pipe_dst_w) ==
+	    pipe_config->pipe_src_w) &&
+	   ((pipe_config->pipe_dst_y + pipe_config->pipe_dst_h) ==
+	    pipe_config->pipe_src_w))
+		goto done;
+
+	x = pipe_config->pipe_dst_x;
+	y = pipe_config->pipe_dst_y;
+	width = pipe_config->pipe_dst_w;
+	height = pipe_config->pipe_dst_h;
+
+done:
+	pipe_config->pch_pfit.pos = (x << 16) | y;
+	pipe_config->pch_pfit.size = (width << 16) | height;
+	pipe_config->pch_pfit.enabled = pipe_config->pch_pfit.size != 0;
+}
+
 static int skylake_pfiter_calculate(struct drm_crtc *crtc,
 				   struct drm_crtc_state *crtc_state)
 {
@@ -12560,16 +12589,36 @@ static int skylake_pfiter_calculate(struct drm_crtc *crtc,
 	if (((pipe_config->pipe_scaling_mode !=
 	      intel_crtc->config->pipe_scaling_mode) ||
 	     (pipe_config->pipe_src_w != intel_crtc->config->pipe_src_w) ||
-	     (pipe_config->pipe_src_h != intel_crtc->config->pipe_src_h)) &&
+	     (pipe_config->pipe_src_h != intel_crtc->config->pipe_src_h) ||
+	     (pipe_config->pipe_dst_x != intel_crtc->config->pipe_dst_x) ||
+	     (pipe_config->pipe_dst_y != intel_crtc->config->pipe_dst_y) ||
+	     (pipe_config->pipe_dst_w != intel_crtc->config->pipe_dst_w) ||
+	     (pipe_config->pipe_dst_h != intel_crtc->config->pipe_dst_h)) &&
 	    (!mode_changed)) {
 		pipe_config->update_pipe = true;
 	}
 
 	if ((mode_changed) || (pipe_config->update_pipe)) {
 		ret = skl_update_scaler_crtc(pipe_config);
-		if (!ret)
-			intel_pch_panel_fitting(intel_crtc, pipe_config,
-						pipe_config->pipe_scaling_mode);
+		if (!ret) {
+			if (pipe_config->pipe_scaling_mode ==
+			    DRM_MODE_SCALE_CUSTOM) {
+				intel_gen9_pipe_scale(intel_crtc, pipe_config,
+					pipe_config->pipe_scaling_mode);
+			} else {
+				intel_pch_panel_fitting(intel_crtc,
+							pipe_config,
+					pipe_config->pipe_scaling_mode);
+				pipe_config->pipe_dst_x =
+					(pipe_config->pch_pfit.pos >> 16);
+				pipe_config->pipe_dst_y =
+					(pipe_config->pch_pfit.pos & 0xffff);
+				pipe_config->pipe_dst_w =
+					(pipe_config->pch_pfit.size >> 16);
+				pipe_config->pipe_dst_h =
+					(pipe_config->pch_pfit.size & 0xffff);
+			}
+		}
 	}
 	return ret;
 }
@@ -13045,6 +13094,10 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 		pipe_config->output_types |= 1 << encoder->type;
 	}
 
+	pipe_config->pipe_dst_w = pipe_config->pipe_src_w;
+	pipe_config->pipe_dst_h = pipe_config->pipe_src_h;
+
+
 encoder_retry:
 	/* Ensure the port clock defaults are reset when retrying. */
 	pipe_config->port_clock = 0;
@@ -14028,11 +14081,20 @@ static int intel_atomic_check(struct drm_device *dev,
 
 				crtc_state->src_w = adjusted_mode->hdisplay;
 				crtc_state->src_h = adjusted_mode->vdisplay;
+				crtc_state->dst_w = adjusted_mode->hdisplay;
+				crtc_state->dst_h = adjusted_mode->vdisplay;
+				crtc_state->dst_x =  0;
+				crtc_state->dst_y =  0;
+				crtc_state->fitting_mode = 0;
 			}
 			pipe_config->pipe_src_w = crtc_state->src_w;
 			pipe_config->pipe_src_h = crtc_state->src_h;
 			pipe_config->pipe_scaling_mode =
 				crtc_state->fitting_mode;
+			pipe_config->pipe_dst_x = crtc_state->dst_x;
+			pipe_config->pipe_dst_y = crtc_state->dst_y;
+			pipe_config->pipe_dst_w = crtc_state->dst_w;
+			pipe_config->pipe_dst_h = crtc_state->dst_h;
 			crtc_state->pipescaler_changed = false;
 		}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 189856c..eab05d5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -533,6 +533,8 @@ struct intel_crtc_state {
 	 * and get clipped at the edges. */
 	int pipe_src_w, pipe_src_h;
 	u32 pipe_scaling_mode;
+	int pipe_dst_x, pipe_dst_y;
+	int pipe_dst_w, pipe_dst_h;
 
 	/* Whether to set up the PCH/FDI. Note that we never allow sharing
 	 * between pch encoders and cpu encoders. */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index e245746..99ff789 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -201,6 +201,7 @@ struct drm_crtc_state {
 	bool pipescaler_changed;
 	u32 src_w, src_h;
 	u32 fitting_mode;
+	u32 dst_x, dst_y, dst_w, dst_h;
 };
 
 /**
@@ -1950,6 +1951,10 @@ struct drm_mode_config {
 	struct drm_property *prop_pipe_src_w;
 	struct drm_property *prop_pipe_src_h;
 	struct drm_property *prop_fitting_mode;
+	struct drm_property *prop_pipe_dst_x;
+	struct drm_property *prop_pipe_dst_y;
+	struct drm_property *prop_pipe_dst_w;
+	struct drm_property *prop_pipe_dst_h;
 
 	/**
 	 * @dvi_i_subconnector_property: Optional DVI-I property to
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 49a7265..91bd0f7 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -91,6 +91,7 @@ extern "C" {
 #define DRM_MODE_SCALE_FULLSCREEN	1 /* Full screen, ignore aspect */
 #define DRM_MODE_SCALE_CENTER		2 /* Centered, no scaling */
 #define DRM_MODE_SCALE_ASPECT		3 /* Full screen, preserve aspect */
+#define DRM_MODE_SCALE_CUSTOM		4 /* Custom pipe scaling */
 
 /* Picture aspect ratio options */
 #define DRM_MODE_PICTURE_ASPECT_NONE	0
-- 
1.9.1

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

[PATCH 6/7] drm/i915: Update pipe-scaler according to destination size

[Nabendu Maiti]

Pipe scaler is scaler registers are updated according to provided
destination size from user.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c7434f..15d185e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4710,7 +4710,6 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 int skl_update_scaler_crtc(struct intel_crtc_state *state)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
-	const struct drm_display_mode *adjusted_mode = &state->base.adjusted_mode;
 
 	DRM_DEBUG_KMS("Updating scaler for [CRTC:%d:%s] scaler_user index %u.%u\n",
 		      intel_crtc->base.base.id, intel_crtc->base.name,
@@ -4719,7 +4718,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
 	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
 		&state->scaler_state.scaler_id, DRM_ROTATE_0,
 		state->pipe_src_w, state->pipe_src_h,
-		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
+		state->pipe_dst_w, state->pipe_dst_h);
 }
 
 /**
@@ -12609,6 +12608,7 @@ static int skylake_pfiter_calculate(struct drm_crtc *crtc,
 				intel_pch_panel_fitting(intel_crtc,
 							pipe_config,
 					pipe_config->pipe_scaling_mode);
+			}
 				pipe_config->pipe_dst_x =
 					(pipe_config->pch_pfit.pos >> 16);
 				pipe_config->pipe_dst_y =
@@ -12617,7 +12617,6 @@ static int skylake_pfiter_calculate(struct drm_crtc *crtc,
 					(pipe_config->pch_pfit.size >> 16);
 				pipe_config->pipe_dst_h =
 					(pipe_config->pch_pfit.size & 0xffff);
-			}
 		}
 	}
 	return ret;
@@ -13096,7 +13095,10 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 
 	pipe_config->pipe_dst_w = pipe_config->pipe_src_w;
 	pipe_config->pipe_dst_h = pipe_config->pipe_src_h;
-
+	pipe_config->base.dst_w = pipe_config->pipe_src_w;
+	pipe_config->base.dst_h = pipe_config->pipe_src_h;
+	pipe_config->base.src_w = pipe_config->pipe_src_w;
+	pipe_config->base.src_h = pipe_config->pipe_src_h;
 
 encoder_retry:
 	/* Ensure the port clock defaults are reset when retrying. */
@@ -13106,7 +13108,6 @@ encoder_retry:
 	/* Fill in default crtc timings, allow encoders to overwrite them. */
 	drm_mode_set_crtcinfo(&pipe_config->base.adjusted_mode,
 			      CRTC_STEREO_DOUBLE);
-
 	/* Pass our mode to the connectors and the CRTC to give them a chance to
 	 * adjust it according to limitations or connector properties, and also
 	 * a chance to reject the mode entirely.
@@ -14081,8 +14082,8 @@ static int intel_atomic_check(struct drm_device *dev,
 
 				crtc_state->src_w = adjusted_mode->hdisplay;
 				crtc_state->src_h = adjusted_mode->vdisplay;
-				crtc_state->dst_w = adjusted_mode->hdisplay;
-				crtc_state->dst_h = adjusted_mode->vdisplay;
+				crtc_state->dst_w = crtc_state->src_w;
+				crtc_state->dst_h = crtc_state->src_h;
 				crtc_state->dst_x =  0;
 				crtc_state->dst_y =  0;
 				crtc_state->fitting_mode = 0;
-- 
1.9.1

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

[PATCH 7/7] drm/i915: Pipescaler destination size limit check on Gen9

[Nabendu Maiti]

Pipe scaler on  gen9 destination size may go out of adjusted modeset
size.This patch add limit check on user custom crtc destination size and
clamp it within modeset size.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 15d185e..45e5204 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12565,6 +12565,17 @@ intel_gen9_pipe_scale(struct intel_crtc *intel_crtc,
 	    pipe_config->pipe_src_w))
 		goto done;
 
+	/* Out of boundary, clamping it */
+	if ((pipe_config->pipe_dst_x + pipe_config->pipe_dst_w) >
+	     adjusted_mode->hdisplay)
+		pipe_config->pipe_dst_w =
+		(adjusted_mode->hdisplay - pipe_config->pipe_dst_x);
+
+	if ((pipe_config->pipe_dst_y + pipe_config->pipe_dst_h) >
+	    adjusted_mode->vdisplay)
+		pipe_config->pipe_dst_h =
+		(adjusted_mode->vdisplay - pipe_config->pipe_dst_y);
+
 	x = pipe_config->pipe_dst_x;
 	y = pipe_config->pipe_dst_y;
 	width = pipe_config->pipe_dst_w;
-- 
1.9.1

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

✓ Fi.CI.BAT: success for drm/i915: Add pipe scaler for Gen9 in atomic path

[Patchwork]

== Series Details ==

Series: drm/i915: Add pipe scaler for Gen9 in atomic path
URL   : https://patchwork.freedesktop.org/series/11764/
State : success

== Summary ==

Series 11764v1 drm/i915: Add pipe scaler for Gen9 in atomic path
http://patchwork.freedesktop.org/api/1.0/series/11764/revisions/1/mbox/


fi-bdw-5557u     total:252  pass:235  dwarn:0   dfail:0   fail:2   skip:15 
fi-bsw-n3050     total:252  pass:204  dwarn:0   dfail:0   fail:2   skip:46 
fi-hsw-4770k     total:252  pass:228  dwarn:0   dfail:0   fail:2   skip:22 
fi-hsw-4770r     total:252  pass:224  dwarn:0   dfail:0   fail:2   skip:26 
fi-ivb-3520m     total:252  pass:220  dwarn:0   dfail:0   fail:1   skip:31 
fi-skl-6260u     total:252  pass:203  dwarn:33  dfail:1   fail:1   skip:14 
fi-skl-6700k     total:252  pass:189  dwarn:33  dfail:1   fail:1   skip:28 
fi-snb-2520m     total:252  pass:207  dwarn:0   dfail:0   fail:2   skip:43 
fi-snb-2600      total:252  pass:206  dwarn:0   dfail:0   fail:2   skip:44 

Results at /archive/results/CI_IGT_test/Patchwork_2450/

57de27e40b9741c17c6749a366e891faf8b22fcb drm-intel-nightly: 2016y-08m-29d-15h-38m-26s UTC integration manifest
12be5fb drm/i915: Pipescaler destination size limit check on Gen9
fc9e212 drm/i915: Update pipe-scaler according to destination size
f84c7a4 drm/i915: Add pipe scaler co-ordinate and size property for Gen9
a2958a2 drm/i915: Adding atomic fitting mode property for GEN9
14decbe drm/i915: Panel fitting calculation for GEN9
03dc183 drm/i915: Add pipe_src size property calculations in atomic path.
417cfb0 drm/i915: Add pipe scaler pipe source drm property

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

Re: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Ville Syrjälä]

On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
> Following patch series add pipe scaler functionality in atomic path.The pipe
> scaler can be changed dynamically without modeset.Apart from default panel
> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
> mode is added as there is no such restriction on GEn9 pipe scaler.

Some quick observations:
- missing any interaction with drm core, so all generic fb size checks
  and whatnot will not work out, I think
- the way it's done goes against what I've suggested in the past. My
  idea has been to add a "fixed mode" property to connectors instead,
  which I think would minimize the impact on the core, and it would
  follow closely the way eDP/LVDS/DSI already works today.
- There's no need to restrict the feature to gen9+ only. It should work
  out just fine for at least all pch platforms. gmch platforms would be
  more challenging
- the pfiter_recalculate thing looks pretty wrong atomic wise

> 
> 
> 
> Nabendu Maiti (7):
>   drm/i915: Add pipe scaler pipe source drm property
>   drm/i915: Add pipe_src size property calculations in atomic path.
>   drm/i915: Panel fitting calculation for GEN9
>   drm/i915: Adding atomic fitting mode property for GEN9
>   drm/i915: Add pipe scaler co-ordinate and size property for Gen9
>   drm/i915: Update pipe-scaler according to destination size
>   drm/i915: Pipescaler destination size limit check on Gen9
> 
>  drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
>  drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>  drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
>  include/drm/drm_crtc.h               |  14 ++++
>  include/uapi/drm/drm_mode.h          |   1 +
>  7 files changed, 263 insertions(+), 8 deletions(-)
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Maiti, Nabendu Bikash]

Hi,


On 9/20/2016 1:55 PM, Ville Syrjälä wrote:
> On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
>> Following patch series add pipe scaler functionality in atomic path.The pipe
>> scaler can be changed dynamically without modeset.Apart from default panel
>> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
>> mode is added as there is no such restriction on GEn9 pipe scaler.
>
> Some quick observations:
> - missing any interaction with drm core, so all generic fb size checks
>   and whatnot will not work out, I think
Pipe scaler is not dependent on fp I think. We have fb size checks are 
done in plane check.

> - the way it's done goes against what I've suggested in the past. My
>   idea has been to add a "fixed mode" property to connectors instead,
>   which I think would minimize the impact on the core, and it would
>   follow closely the way eDP/LVDS/DSI already works today.
yes using fixed mode we can do also but I wanted to be part of crtc 
property instead of connector property. As fixed mode is basically 
intended for fixed mode panels.But we may use pipe scaler for fixed mode 
and dynamic mode panels.

> - There's no need to restrict the feature to gen9+ only. It should work
>   out just fine for at least all pch platforms. gmch platforms would be
>   more challenging
This code I designed to use gen9+, and properties like crtc destination 
size and offsets also exposed.There is no restrictions on modes (eg. 
pillerbox/letterbox) and down scaling ratios as previous platforms. 
Currently scaling mode is part of connector property and implemented as 
legacy property. I created new scaling mode as atomic property. I think 
gen9+ onward platforms may have proper atomic pipe scaling properties 
and user space may use it fully dynamically without modeset.

> - the pfiter_recalculate thing looks pretty wrong atomic wise
Sorry, I couldn't get it. Are you referring pipe scaler registers are 
not written together with other registers?  pfiter_calculate only 
calculate and stores the data for later commit. Please provide more 
details on it.
>
>>
>>
>>
>> Nabendu Maiti (7):
>>   drm/i915: Add pipe scaler pipe source drm property
>>   drm/i915: Add pipe_src size property calculations in atomic path.
>>   drm/i915: Panel fitting calculation for GEN9
>>   drm/i915: Adding atomic fitting mode property for GEN9
>>   drm/i915: Add pipe scaler co-ordinate and size property for Gen9
>>   drm/i915: Update pipe-scaler according to destination size
>>   drm/i915: Pipescaler destination size limit check on Gen9
>>
>>  drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
>>  drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
>>  drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
>>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>>  drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
>>  include/drm/drm_crtc.h               |  14 ++++
>>  include/uapi/drm/drm_mode.h          |   1 +
>>  7 files changed, 263 insertions(+), 8 deletions(-)
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

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

Re: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Ville Syrjälä]

On Wed, Sep 21, 2016 at 07:47:37PM +0530, Maiti, Nabendu Bikash wrote:
> Hi,
> 
> 
> On 9/20/2016 1:55 PM, Ville Syrjälä wrote:
> > On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
> >> Following patch series add pipe scaler functionality in atomic path.The pipe
> >> scaler can be changed dynamically without modeset.Apart from default panel
> >> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
> >> mode is added as there is no such restriction on GEn9 pipe scaler.
> >
> > Some quick observations:
> > - missing any interaction with drm core, so all generic fb size checks
> >   and whatnot will not work out, I think
> Pipe scaler is not dependent on fp I think. We have fb size checks are 
> done in plane check.

You need to explain how this all interacts with the legacy pipe/plane
size == mode hdisplay/vdisplay stuff.

> 
> > - the way it's done goes against what I've suggested in the past. My
> >   idea has been to add a "fixed mode" property to connectors instead,
> >   which I think would minimize the impact on the core, and it would
> >   follow closely the way eDP/LVDS/DSI already works today.
> yes using fixed mode we can do also but I wanted to be part of crtc 
> property instead of connector property. As fixed mode is basically 
> intended for fixed mode panels.But we may use pipe scaler for fixed mode 
> and dynamic mode panels.

That doesn't say much. The fixed mode apporach, I think, might be easier
to incorporate in a way that keeps the legacy apporach working. Adding a
totally different way to configure the pipe src size will mean more weird
interactions between the properties.

Also it culd be supported with non-atomic userspace reasonably easily.
We'll need some sort of userspace for this anyway, otherwise it's just
untested/unused code.

> 
> > - There's no need to restrict the feature to gen9+ only. It should work
> >   out just fine for at least all pch platforms. gmch platforms would be
> >   more challenging
> This code I designed to use gen9+, and properties like crtc destination 
> size and offsets also exposed.There is no restrictions on modes (eg. 
> pillerbox/letterbox) and down scaling ratios as previous platforms. 
> Currently scaling mode is part of connector property and implemented as 
> legacy property. I created new scaling mode as atomic property. I think 
> gen9+ onward platforms may have proper atomic pipe scaling properties 
> and user space may use it fully dynamically without modeset.

None of that tells me why it's gen9+ only. IIRC the panel fitter
configuration been very flexible ever since ILK, so the only real
difference should be which registers to write.

> 
> > - the pfiter_recalculate thing looks pretty wrong atomic wise
> Sorry, I couldn't get it. Are you referring pipe scaler registers are 
> not written together with other registers?  pfiter_calculate only 
> calculate and stores the data for later commit. Please provide more 
> details on it.

It's going through encoder->crtc links and whanot. That's not going
to fly.

> >
> >>
> >>
> >>
> >> Nabendu Maiti (7):
> >>   drm/i915: Add pipe scaler pipe source drm property
> >>   drm/i915: Add pipe_src size property calculations in atomic path.
> >>   drm/i915: Panel fitting calculation for GEN9
> >>   drm/i915: Adding atomic fitting mode property for GEN9
> >>   drm/i915: Add pipe scaler co-ordinate and size property for Gen9
> >>   drm/i915: Update pipe-scaler according to destination size
> >>   drm/i915: Pipescaler destination size limit check on Gen9
> >>
> >>  drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
> >>  drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
> >>  drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
> >>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
> >>  drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
> >>  include/drm/drm_crtc.h               |  14 ++++
> >>  include/uapi/drm/drm_mode.h          |   1 +
> >>  7 files changed, 263 insertions(+), 8 deletions(-)
> >>
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> 
> -- 
> Regards,
> Nabendu

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

Re: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Nabendu Maiti]

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

Attaching old discussion thread for easy reference.

On Tue, Jan 05, 2016 at 05:18:40PM +0200, Ville Syrjälä wrote:

 > On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote:

 > > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote:

 > > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote:

 > > > > Hey,

 > > > >

 > > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti:

 > > > > > This patch is adding pipesource size as property as intel

 > > > > > property.User application is allowed to change the pipe 
source size in runtime on BXT/SKL.

 > > > > > Added the property as inteli crtc property.

 > > > > >

 > > > > > Comments and suggestions are requested for whether we can keep

 > > > > > the property as intel crtc property or move to drm level.

 > > > > >

 > > > > This property would get lost on a modeset. But why do you need 
a pipe_src property?

 > > >

 > > > It's needed if we want to use the panel fitter with

 > > > non-eDP/LVDS/DSI displays.

 > > >

 > > > Last time this came up I decided that we want to expose this via a

 > > > new "fixed_mode" property. Ie. userspace can choose the actual

 > > > display timings by setting the "fixed_mode" property with a

 > > > specific mode, and then the normal mode property will basically

 > > > just control just the pipe src size just like it already does for

 > > > eDP/LVDS/DSI where we already have the fixed_mode internally (just

 > > > not exposed to usersapce). If the fixed_mode is not specified,

 > > > things will work just as they do right now. Obviously for

 > > > eDP/LVDS/DSI we will reject any request to change/unset the fixed 
mode.

 > > >

 > > > The benefit of that approach is that things will work exactly the

 > > > same way as before unless the user explicitly sets the fixed_mode,

 > > > and once it's set thigns will work exactly the same way as they

 > > > have worked so far for eDP/LVDS/DSI. Also it keeps the policy of

 > > > choosing the fixed mode strictly is userspace for external displays.

 > > >

 > > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to

 > > > userspace giving userspace more information on what the actual

 > > > panel timings are. Currently userspace has to more or less guess

 > > > that one of the modes the connector claims to support has the

 > > > actual panel timings.

 > > >

 > > > So far I've not heard any opposing opinions to this plan.

 > >

 > > Hm yeah, pipe_src would run into all kinds of fun in conjunction

 > > with the existing fixed mode stuff we have. Just exposing the fixed

 > > as a prop might be simpler. But then we need to figure out what to

 > > do wrt the clock in the mode passed in through userspace - currently

 > > the fixed mode always wins, but for manual DRRS it would be nice if

 > > userspace could control it, and doesn't have to know that there's a 
fixed_mode.

 >

 > We could have the user mode vrefresh indicate the desired refresh rate

 > of the fixed mode as well. In fact I've been wanting to add a check to

 > make sure the user mode refresh rate isn't too far off from the

 > fixed/downlclock mode vrefresh, but didn't get around to it yet.

Yeah agreed, userspace vrefresh should control (or at least be checked

against) the fixed mode vrefresh.

 > > So semantically I think only exposing the pipe src to expose panel

 > > fitters would be cleaner. Then we'd need to deal with some internal

 > > troubles to make sure we combine everything correctly, but that

 > > shouldn't be too hard really.

 >

 > I think it's quite a bit more work since we have to redo all the fb

 > size checks and whatnot to use the property when specified. I once

 > outlined a more detailed plan for his approach too (too lazy to dig up

 > the mail), but I think the fixed mode prop is a simpler approach and

 > won't actually require much in the way of userspace changes either.

 > It'll be enough to set the property once and then even the legacy

 > modeset ioctls will work exactly like they do now for eDP/LVDS/DSI.

One downside of the fixed mode against an explicit pipe src rectangle is 
that the explict rectangle allows us to letter/pillar/stretch/move too.

With just a fixed_mode that's much harder to pull off.

I think for i915 it should be fairly ok-ish to implement this. We just 
need to move pipe src rectangle to drm_crtc_state, and adjust all the 
pfit config logic to work on the pipe level. Panels would then only 
replace the output mode with the fixed panel mode, and leave 
scaling/pfit selection to the core crtc code.

-Daniel

--

Daniel Vetter

Software Engineer, Intel Corporation

http://blog.ffwll.ch

_______________________________________________

Intel-gfx mailing list

Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org>

http://lists.freedesktop.org/mailman/listinfo/intel-gfx

..>
On 10/03/2016 04:57 PM, Ville Syrjälä wrote:
> On Wed, Sep 21, 2016 at 07:47:37PM +0530, Maiti, Nabendu Bikash wrote:
>> Hi,
>>
>>
>> On 9/20/2016 1:55 PM, Ville Syrjälä wrote:
>>> On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
>>>> Following patch series add pipe scaler functionality in atomic path.The pipe
>>>> scaler can be changed dynamically without modeset.Apart from default panel
>>>> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
>>>> mode is added as there is no such restriction on GEn9 pipe scaler.
>>> Some quick observations:
>>> - missing any interaction with drm core, so all generic fb size checks
>>>    and whatnot will not work out, I think
>> Pipe scaler is not dependent on fp I think. We have fb size checks are
>> done in plane check.
> You need to explain how this all interacts with the legacy pipe/plane
> size == mode hdisplay/vdisplay stuff.
There is no direct interaction with the plane. This properties use 
current mode-set timings - horizontal and vertical timing as stored in 
pipe_src_w and pipe_src_h variables.
They are filled just after pll and clock are set and adjusted modeset 
structures are filled up. This design will not interfere with current 
existing code of pipe/plane clock setting.
PIpe scaler hardware will only use scaling of data not the clock.

>>> - the way it's done goes against what I've suggested in the past. My
>>>    idea has been to add a "fixed mode" property to connectors instead,
>>>    which I think would minimize the impact on the core, and it would
>>>    follow closely the way eDP/LVDS/DSI already works today.
>> yes using fixed mode we can do also but I wanted to be part of crtc
>> property instead of connector property. As fixed mode is basically
>> intended for fixed mode panels.But we may use pipe scaler for fixed mode
>> and dynamic mode panels.
> That doesn't say much. The fixed mode apporach, I think, might be easier
> to incorporate in a way that keeps the legacy apporach working. Adding a
> totally different way to configure the pipe src size will mean more weird
> interactions between the properties.
>
> Also it culd be supported with non-atomic userspace reasonably easily.
> We'll need some sort of userspace for this anyway, otherwise it's just
> untested/unused code.
There will be no change in legacy code path. They will work as it is.  
Only downside will be mixup of legacy and non-atomic use.
But individually they should work as fine as before. The configuration 
of pipe_src is done as before. They will be
changed if user explicitly want to change them. Internal checks are 
there if some odd unaccepted values are provided. Morever
the current modeset values (hsize a vsize) are exposed to userspace 
through this design. If required vrefresh data can
be feed to userspace to by adding another property.
This way complexity in user space can be reduced by only exposing 
horizontal/vertical size, instead of full mode structure.
>>> - There's no need to restrict the feature to gen9+ only. It should work
>>>    out just fine for at least all pch platforms. gmch platforms would be
>>>    more challenging
>> This code I designed to use gen9+, and properties like crtc destination
>> size and offsets also exposed.There is no restrictions on modes (eg.
>> pillerbox/letterbox) and down scaling ratios as previous platforms.
>> Currently scaling mode is part of connector property and implemented as
>> legacy property. I created new scaling mode as atomic property. I think
>> gen9+ onward platforms may have proper atomic pipe scaling properties
>> and user space may use it fully dynamically without modeset.
> None of that tells me why it's gen9+ only. IIRC the panel fitter
> configuration been very flexible ever since ILK, so the only real
> difference should be which registers to write.
There is no hardware pipe scaler before gen7 (VLV/BXT). VLV +CHT 
platform also can be supported on this design
with little additional modification. Using this Letterbox/pillerbox 
modes in Gen7 can be programmatically selected and according to proper
register write. But as far as I know old platforms has limited 
functionality of panel fitting either by changing clock/ doing
nearby supported modeset. I understand fixed mode is the timing required 
by panel and adjusted mode is the nearest permissive
timing available from gen platform. But using hardware scaler we don't 
change timing. We keep the timing intact, but modify the
blended out pixel data from pipe.

simple use case I can think of: Display is showing some document blended 
with other planes. Double clicking should enlarge
some portion of the screen and fir to the pannel without screen blanking 
due to modeset.

..
>>> - the pfiter_recalculate thing looks pretty wrong atomic wise
>> Sorry, I couldn't get it. Are you referring pipe scaler registers are
>> not written together with other registers?  pfiter_calculate only
>> calculate and stores the data for later commit. Please provide more
>> details on it.
> It's going through encoder->crtc links and whanot. That's not going
> to fly.
There is no use of encoder->crtc in the design, everything maintained in 
crtc. (Note Intial few patches using the legacy encoder->crtc
iteration to find current scaling mode). Pfit_calculate just find out 
the pipe out size in panel if not provided by user. If target pixel area
in crtc going out of panel clipping/clamming is used. Currently clamping 
is implemented.

Regards,
Nabendu

Nabendu Maiti (7):
>>>>    drm/i915: Add pipe scaler pipe source drm property
>>>>    drm/i915: Add pipe_src size property calculations in atomic path.
>>>>    drm/i915: Panel fitting calculation for GEN9
>>>>    drm/i915: Adding atomic fitting mode property for GEN9
>>>>    drm/i915: Add pipe scaler co-ordinate and size property for Gen9
>>>>    drm/i915: Update pipe-scaler according to destination size
>>>>    drm/i915: Pipescaler destination size limit check on Gen9
>>>>
>>>>   drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
>>>>   drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
>>>>   drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
>>>>   drivers/gpu/drm/i915/intel_drv.h     |   3 +
>>>>   drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
>>>>   include/drm/drm_crtc.h               |  14 ++++
>>>>   include/uapi/drm/drm_mode.h          |   1 +
>>>>   7 files changed, 263 insertions(+), 8 deletions(-)
>>>>
>>>> --
>>>> 1.9.1
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> -- 
>> Regards,
>> Nabendu


[-- Attachment #1.2: Type: text/html, Size: 56474 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: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Nabendu Maiti]

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

Attaching old discussion thread for easy reference.

On Tue, Jan 05, 2016 at 05:18:40PM +0200, Ville Syrjälä wrote:

 > On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote:

 > > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote:

 > > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote:

 > > > > Hey,

 > > > >

 > > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti:

 > > > > > This patch is adding pipesource size as property as intel

 > > > > > property.User application is allowed to change the pipe 
source size in runtime on BXT/SKL.

 > > > > > Added the property as inteli crtc property.

 > > > > >

 > > > > > Comments and suggestions are requested for whether we can keep

 > > > > > the property as intel crtc property or move to drm level.

 > > > > >

 > > > > This property would get lost on a modeset. But why do you need 
a pipe_src property?

 > > >

 > > > It's needed if we want to use the panel fitter with

 > > > non-eDP/LVDS/DSI displays.

 > > >

 > > > Last time this came up I decided that we want to expose this via a

 > > > new "fixed_mode" property. Ie. userspace can choose the actual

 > > > display timings by setting the "fixed_mode" property with a

 > > > specific mode, and then the normal mode property will basically

 > > > just control just the pipe src size just like it already does for

 > > > eDP/LVDS/DSI where we already have the fixed_mode internally (just

 > > > not exposed to usersapce). If the fixed_mode is not specified,

 > > > things will work just as they do right now. Obviously for

 > > > eDP/LVDS/DSI we will reject any request to change/unset the fixed 
mode.

 > > >

 > > > The benefit of that approach is that things will work exactly the

 > > > same way as before unless the user explicitly sets the fixed_mode,

 > > > and once it's set thigns will work exactly the same way as they

 > > > have worked so far for eDP/LVDS/DSI. Also it keeps the policy of

 > > > choosing the fixed mode strictly is userspace for external displays.

 > > >

 > > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to

 > > > userspace giving userspace more information on what the actual

 > > > panel timings are. Currently userspace has to more or less guess

 > > > that one of the modes the connector claims to support has the

 > > > actual panel timings.

 > > >

 > > > So far I've not heard any opposing opinions to this plan.

 > >

 > > Hm yeah, pipe_src would run into all kinds of fun in conjunction

 > > with the existing fixed mode stuff we have. Just exposing the fixed

 > > as a prop might be simpler. But then we need to figure out what to

 > > do wrt the clock in the mode passed in through userspace - currently

 > > the fixed mode always wins, but for manual DRRS it would be nice if

 > > userspace could control it, and doesn't have to know that there's a 
fixed_mode.

 >

 > We could have the user mode vrefresh indicate the desired refresh rate

 > of the fixed mode as well. In fact I've been wanting to add a check to

 > make sure the user mode refresh rate isn't too far off from the

 > fixed/downlclock mode vrefresh, but didn't get around to it yet.

Yeah agreed, userspace vrefresh should control (or at least be checked

against) the fixed mode vrefresh.

 > > So semantically I think only exposing the pipe src to expose panel

 > > fitters would be cleaner. Then we'd need to deal with some internal

 > > troubles to make sure we combine everything correctly, but that

 > > shouldn't be too hard really.

 >

 > I think it's quite a bit more work since we have to redo all the fb

 > size checks and whatnot to use the property when specified. I once

 > outlined a more detailed plan for his approach too (too lazy to dig up

 > the mail), but I think the fixed mode prop is a simpler approach and

 > won't actually require much in the way of userspace changes either.

 > It'll be enough to set the property once and then even the legacy

 > modeset ioctls will work exactly like they do now for eDP/LVDS/DSI.

One downside of the fixed mode against an explicit pipe src rectangle is 
that the explict rectangle allows us to letter/pillar/stretch/move too.

With just a fixed_mode that's much harder to pull off.

I think for i915 it should be fairly ok-ish to implement this. We just 
need to move pipe src rectangle to drm_crtc_state, and adjust all the 
pfit config logic to work on the pipe level. Panels would then only 
replace the output mode with the fixed panel mode, and leave 
scaling/pfit selection to the core crtc code.

-Daniel

--

Daniel Vetter

Software Engineer, Intel Corporation

http://blog.ffwll.ch

_______________________________________________

Intel-gfx mailing list

Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org>

http://lists.freedesktop.org/mailman/listinfo/intel-gfx

..>
On 10/03/2016 04:57 PM, Ville Syrjälä wrote:
> On Wed, Sep 21, 2016 at 07:47:37PM +0530, Maiti, Nabendu Bikash wrote:
>> Hi,
>>
>>
>> On 9/20/2016 1:55 PM, Ville Syrjälä wrote:
>>> On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
>>>> Following patch series add pipe scaler functionality in atomic path.The pipe
>>>> scaler can be changed dynamically without modeset.Apart from default panel
>>>> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
>>>> mode is added as there is no such restriction on GEn9 pipe scaler.
>>> Some quick observations:
>>> - missing any interaction with drm core, so all generic fb size checks
>>>    and whatnot will not work out, I think
>> Pipe scaler is not dependent on fp I think. We have fb size checks are
>> done in plane check.
> You need to explain how this all interacts with the legacy pipe/plane
> size == mode hdisplay/vdisplay stuff.
There is no direct interaction with the plane. This properties use 
current modeset timings - horizontal and
vertical timing as stored in pipe_src_w and pipe_src_h variables. They 
are filled just after pll and clock are set
and adjusted modeset structures are filled up. This design will not 
interfere with current existing code of pipe/plane.

Clipping and clamping on plane level is already done on Blended data 
output of  pipe using existing code. pipe
level clamping and clipping will happen when pipe scaler using out of 
boundary value. Current code use clamping only.

>>> - the way it's done goes against what I've suggested in the past. My
>>>    idea has been to add a "fixed mode" property to connectors instead,
>>>    which I think would minimize the impact on the core, and it would
>>>    follow closely the way eDP/LVDS/DSI already works today.
>> yes using fixed mode we can do also but I wanted to be part of crtc
>> property instead of connector property. As fixed mode is basically
>> intended for fixed mode panels.But we may use pipe scaler for fixed mode
>> and dynamic mode panels.
> That doesn't say much. The fixed mode apporach, I think, might be easier
> to incorporate in a way that keeps the legacy apporach working. Adding a
> totally different way to configure the pipe src size will mean more weird
> interactions between the properties.
>
> Also it culd be supported with non-atomic userspace reasonably easily.
> We'll need some sort of userspace for this anyway, otherwise it's just
> untested/unused code.
There will be no change in legacy code path. They will work as it is.  
Only downside will be mixup of legacy and non-atomic use.
But individually they should work as fine as before. The configuration 
of pipe_src is done as before. They will be
changed if user explicitly want to change them. Internal checks are 
there if some odd unaccepted values are provided.

Yes I can write the userspace igt if this design is fine. This design is 
tested with inhouse val tool (igt based) on android
plaatform.
>>> - There's no need to restrict the feature to gen9+ only. It should work
>>>    out just fine for at least all pch platforms. gmch platforms would be
>>>    more challenging
>> This code I designed to use gen9+, and properties like crtc destination
>> size and offsets also exposed.There is no restrictions on modes (eg.
>> pillerbox/letterbox) and down scaling ratios as previous platforms.
>> Currently scaling mode is part of connector property and implemented as
>> legacy property. I created new scaling mode as atomic property. I think
>> gen9+ onward platforms may have proper atomic pipe scaling properties
>> and user space may use it fully dynamically without modeset.
> None of that tells me why it's gen9+ only. IIRC the panel fitter
> configuration been very flexible ever since ILK, so the only real
> difference should be which registers to write.
There is no hardware pipe scaler before gen7 (VLV/BXT). VLV +CHT 
platform also can be supported on this design
with little modification. Also Letterbox/pillerbox modes can be 
programmatically selected and according to proper
register write. But as far as I know old platforms has limited 
functionality of panel fitting either by changing clock/ doing
nearby supported modeset. I understand fixed mode is the timing required 
by panel and adjusted mode is the nearest permissive
timing available from gen platform. But using hardware scaler we don't 
change timing. We keep the timing intact, but modify the
pixel data came from blender pipe. We modify the data size by 
enlarging/encorching in between pipe and encoder.
..
>>> - the pfiter_recalculate thing looks pretty wrong atomic wise
>> Sorry, I couldn't get it. Are you referring pipe scaler registers are
>> not written together with other registers?  pfiter_calculate only
>> calculate and stores the data for later commit. Please provide more
>> details on it.
> It's going through encoder->crtc links and whanot. That's not going
> to fly.
There is no use of encoder->crtc in the design, everything maintained in 
drm crtc level. (Note Initial few patches using the legacy encoder->crtc
iteration to find current scaling mode). Final code will work 
independently without using legacy fitting mode.
Pfit_calculate find out the pipe out size and position in panel if not 
provided by user depending on the source size.
One use case for this kind of design may be following. One document 
visibile on panel using several planes. Now clicking on it
will enlarge some part of the document and fit the display without 
blanking (no modeset happens).And go back to original
on another double click. This is the the dynamic way we change pipe 
scaler config pillerbox/letterbox etc.

Regards,
Nabendu

>>>> Nabendu Maiti (7):
>>>>    drm/i915: Add pipe scaler pipe source drm property
>>>>    drm/i915: Add pipe_src size property calculations in atomic path.
>>>>    drm/i915: Panel fitting calculation for GEN9
>>>>    drm/i915: Adding atomic fitting mode property for GEN9
>>>>    drm/i915: Add pipe scaler co-ordinate and size property for Gen9
>>>>    drm/i915: Update pipe-scaler according to destination size
>>>>    drm/i915: Pipescaler destination size limit check on Gen9
>>>>
>>>>   drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
>>>>   drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
>>>>   drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
>>>>   drivers/gpu/drm/i915/intel_drv.h     |   3 +
>>>>   drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
>>>>   include/drm/drm_crtc.h               |  14 ++++
>>>>   include/uapi/drm/drm_mode.h          |   1 +
>>>>   7 files changed, 263 insertions(+), 8 deletions(-)
>>>>
>>>> --
>>>> 1.9.1
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> -- 
>> Regards,
>> Nabendu


[-- Attachment #1.2: Type: text/html, Size: 56642 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: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Maiti, Nabendu Bikash]

Hi,

Please review the patches and comments.


On 10/28/2016 6:05 PM, Nabendu Maiti wrote:
> Attaching old discussion thread for easy reference.
>
> On Tue, Jan 05, 2016 at 05:18:40PM +0200, Ville Syrjälä wrote:
>
>> On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote:
>
>> > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote:
>
>> > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote:
>
>> > > > Hey,
>
>> > > >
>
>> > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti:
>
>> > > > > This patch is adding pipesource size as property as intel
>
>> > > > > property.User application is allowed to change the pipe source
> size in runtime on BXT/SKL.
>
>> > > > > Added the property as inteli crtc property.
>
>> > > > >
>
>> > > > > Comments and suggestions are requested for whether we can keep
>
>> > > > > the property as intel crtc property or move to drm level.
>
>> > > > >
>
>> > > > This property would get lost on a modeset. But why do you need a
> pipe_src property?
>
>> > >
>
>> > > It's needed if we want to use the panel fitter with
>
>> > > non-eDP/LVDS/DSI displays.
>
>> > >
>
>> > > Last time this came up I decided that we want to expose this via a
>
>> > > new "fixed_mode" property. Ie. userspace can choose the actual
>
>> > > display timings by setting the "fixed_mode" property with a
>
>> > > specific mode, and then the normal mode property will basically
>
>> > > just control just the pipe src size just like it already does for
>
>> > > eDP/LVDS/DSI where we already have the fixed_mode internally (just
>
>> > > not exposed to usersapce). If the fixed_mode is not specified,
>
>> > > things will work just as they do right now. Obviously for
>
>> > > eDP/LVDS/DSI we will reject any request to change/unset the fixed
> mode.
>
>> > >
>
>> > > The benefit of that approach is that things will work exactly the
>
>> > > same way as before unless the user explicitly sets the fixed_mode,
>
>> > > and once it's set thigns will work exactly the same way as they
>
>> > > have worked so far for eDP/LVDS/DSI. Also it keeps the policy of
>
>> > > choosing the fixed mode strictly is userspace for external displays.
>
>> > >
>
>> > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to
>
>> > > userspace giving userspace more information on what the actual
>
>> > > panel timings are. Currently userspace has to more or less guess
>
>> > > that one of the modes the connector claims to support has the
>
>> > > actual panel timings.
>
>> > >
>
>> > > So far I've not heard any opposing opinions to this plan.
>
>> >
>
>> > Hm yeah, pipe_src would run into all kinds of fun in conjunction
>
>> > with the existing fixed mode stuff we have. Just exposing the fixed
>
>> > as a prop might be simpler. But then we need to figure out what to
>
>> > do wrt the clock in the mode passed in through userspace - currently
>
>> > the fixed mode always wins, but for manual DRRS it would be nice if
>
>> > userspace could control it, and doesn't have to know that there's a
> fixed_mode.
>
>>
>
>> We could have the user mode vrefresh indicate the desired refresh rate
>
>> of the fixed mode as well. In fact I've been wanting to add a check to
>
>> make sure the user mode refresh rate isn't too far off from the
>
>> fixed/downlclock mode vrefresh, but didn't get around to it yet.
>
>
>
> Yeah agreed, userspace vrefresh should control (or at least be checked
>
> against) the fixed mode vrefresh.
>
>
>
>> > So semantically I think only exposing the pipe src to expose panel
>
>> > fitters would be cleaner. Then we'd need to deal with some internal
>
>> > troubles to make sure we combine everything correctly, but that
>
>> > shouldn't be too hard really.
>
>>
>
>> I think it's quite a bit more work since we have to redo all the fb
>
>> size checks and whatnot to use the property when specified. I once
>
>> outlined a more detailed plan for his approach too (too lazy to dig up
>
>> the mail), but I think the fixed mode prop is a simpler approach and
>
>> won't actually require much in the way of userspace changes either.
>
>> It'll be enough to set the property once and then even the legacy
>
>> modeset ioctls will work exactly like they do now for eDP/LVDS/DSI.
>
>
>
> One downside of the fixed mode against an explicit pipe src rectangle is
> that the explict rectangle allows us to letter/pillar/stretch/move too.
>
> With just a fixed_mode that's much harder to pull off.
>
>
>
> I think for i915 it should be fairly ok-ish to implement this. We just
> need to move pipe src rectangle to drm_crtc_state, and adjust all the
> pfit config logic to work on the pipe level. Panels would then only
> replace the output mode with the fixed panel mode, and leave
> scaling/pfit selection to the core crtc code.
>
> -Daniel
>
> --
>
> Daniel Vetter
>
> Software Engineer, Intel Corporation
>
> http://blog.ffwll.ch
>
> _______________________________________________
>
> Intel-gfx mailing list
>
> Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org>
>
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> ..>
> On 10/03/2016 04:57 PM, Ville Syrjälä wrote:
>> On Wed, Sep 21, 2016 at 07:47:37PM +0530, Maiti, Nabendu Bikash wrote:
>>> Hi,
>>>
>>>
>>> On 9/20/2016 1:55 PM, Ville Syrjälä wrote:
>>>> On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
>>>>> Following patch series add pipe scaler functionality in atomic path.The pipe
>>>>> scaler can be changed dynamically without modeset.Apart from default panel
>>>>> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
>>>>> mode is added as there is no such restriction on GEn9 pipe scaler.
>>>> Some quick observations:
>>>> - missing any interaction with drm core, so all generic fb size checks
>>>>   and whatnot will not work out, I think
>>> Pipe scaler is not dependent on fp I think. We have fb size checks are
>>> done in plane check.
>> You need to explain how this all interacts with the legacy pipe/plane
>> size == mode hdisplay/vdisplay stuff.
> There is no direct interaction with the plane. This properties use
> current modeset timings - horizontal and
> vertical timing as stored in pipe_src_w and pipe_src_h variables. They
> are filled just after pll and clock are set
> and adjusted modeset structures are filled up. This design will not
> interfere with current existing code of pipe/plane.
>
> Clipping and clamping on plane level is already done on Blended data
> output of  pipe using existing code. pipe
> level clamping and clipping will happen when pipe scaler using out of
> boundary value. Current code use clamping only.
>
>>>> - the way it's done goes against what I've suggested in the past. My
>>>>   idea has been to add a "fixed mode" property to connectors instead,
>>>>   which I think would minimize the impact on the core, and it would
>>>>   follow closely the way eDP/LVDS/DSI already works today.
>>> yes using fixed mode we can do also but I wanted to be part of crtc
>>> property instead of connector property. As fixed mode is basically
>>> intended for fixed mode panels.But we may use pipe scaler for fixed mode
>>> and dynamic mode panels.
>> That doesn't say much. The fixed mode apporach, I think, might be easier
>> to incorporate in a way that keeps the legacy apporach working. Adding a
>> totally different way to configure the pipe src size will mean more weird
>> interactions between the properties.
>>
>> Also it culd be supported with non-atomic userspace reasonably easily.
>> We'll need some sort of userspace for this anyway, otherwise it's just
>> untested/unused code.
> There will be no change in legacy code path. They will work as it is.
> Only downside will be mixup of legacy and non-atomic use.
> But individually they should work as fine as before. The configuration
> of pipe_src is done as before. They will be
> changed if user explicitly want to change them. Internal checks are
> there if some odd unaccepted values are provided.
>
> Yes I can write the userspace igt if this design is fine. This design is
> tested with inhouse val tool (igt based) on android
> plaatform.
>>>> - There's no need to restrict the feature to gen9+ only. It should work
>>>>   out just fine for at least all pch platforms. gmch platforms would be
>>>>   more challenging
>>> This code I designed to use gen9+, and properties like crtc destination
>>> size and offsets also exposed.There is no restrictions on modes (eg.
>>> pillerbox/letterbox) and down scaling ratios as previous platforms.
>>> Currently scaling mode is part of connector property and implemented as
>>> legacy property. I created new scaling mode as atomic property. I think
>>> gen9+ onward platforms may have proper atomic pipe scaling properties
>>> and user space may use it fully dynamically without modeset.
>> None of that tells me why it's gen9+ only. IIRC the panel fitter
>> configuration been very flexible ever since ILK, so the only real
>> difference should be which registers to write.
> There is no hardware pipe scaler before gen7 (VLV/BXT). VLV +CHT
> platform also can be supported on this design
> with little modification. Also Letterbox/pillerbox modes can be
> programmatically selected and according to proper
> register write. But as far as I know old platforms has limited
> functionality of panel fitting either by changing clock/ doing
> nearby supported modeset. I understand fixed mode is the timing required
> by panel and adjusted mode is the nearest permissive
> timing available from gen platform. But using hardware scaler we don't
> change timing. We keep the timing intact, but modify the
> pixel data came from blender pipe. We modify the data size by
> enlarging/encorching in between pipe and encoder.
> ..
>>>> - the pfiter_recalculate thing looks pretty wrong atomic wise
>>> Sorry, I couldn't get it. Are you referring pipe scaler registers are
>>> not written together with other registers?  pfiter_calculate only
>>> calculate and stores the data for later commit. Please provide more
>>> details on it.
>> It's going through encoder->crtc links and whanot. That's not going
>> to fly.
> There is no use of encoder->crtc in the design, everything maintained in
> drm crtc level. (Note Initial few patches using the legacy encoder->crtc
> iteration to find current scaling mode). Final code will work
> independently without using legacy fitting mode.
> Pfit_calculate find out the pipe out size and position in panel if not
> provided by user depending on the source size.
> One use case for this kind of design may be following. One document
> visibile on panel using several planes. Now clicking on it
> will enlarge some part of the document and fit the display without
> blanking (no modeset happens).And go back to original
> on another double click. This is the the dynamic way we change pipe
> scaler config pillerbox/letterbox etc.
>
> Regards,
> Nabendu
>
>>>>> Nabendu Maiti (7):
>>>>>   drm/i915: Add pipe scaler pipe source drm property
>>>>>   drm/i915: Add pipe_src size property calculations in atomic path.
>>>>>   drm/i915: Panel fitting calculation for GEN9
>>>>>   drm/i915: Adding atomic fitting mode property for GEN9
>>>>>   drm/i915: Add pipe scaler co-ordinate and size property for Gen9
>>>>>   drm/i915: Update pipe-scaler according to destination size
>>>>>   drm/i915: Pipescaler destination size limit check on Gen9
>>>>>
>>>>>  drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
>>>>>  drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
>>>>>  drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
>>>>>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>>>>>  drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
>>>>>  include/drm/drm_crtc.h               |  14 ++++
>>>>>  include/uapi/drm/drm_mode.h          |   1 +
>>>>>  7 files changed, 263 insertions(+), 8 deletions(-)
>>>>>
>>>>> --
>>>>> 1.9.1
>>>>>
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>> --
>>> Regards,
>>> Nabendu
>
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

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

Re: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Maiti, Nabendu Bikash]

Hi Ville,

Please find more clarification on new pipe scaler design as asked in 
discussion.
Please add your thoughts and suggest any change if have to do on the design.


Notes on design:-
A) Only on Gen9 case skylake_pfit_enable() is called
B) Only on Gen9+, intel_crtc_atomic_check has call to
    skl_update_scaler_crtc and is done from encoder->compute_config and
    intel_crtc_atomic_check().
    New implementation use skl_update_scaler_crtc in
    intel_crtc_atomic_check, and  use pch_fitting_mode for computing
    destination position and size.
    Old implementation had pch_panel_fitting() call only once in
    encoder->compute_config()
C) intel_pch_panel_fitting  or intel_pch_panel_fitting is called only
    in case of fixed mode inside encoder->compute_config (e.g
    intel_dp_compute_config).

    The scaling mode property is a legacy connector property in old
    implementation.
D) There is no change in new design for PFIT register write in
intel_begin_crtc_commit()->intel_update_pipe_config()->skylake_pfit_updtae(),
    So behavior is as before.
E) Pipe src properties is populated by modeset properties from
    get_hw_timing()call in current design. So they are having current
    mode's hdisplay and vdisplay size.
	
    In current pipe scaler implementation scaling mode property is an
    atomic crtc property. Now Connector property is not used in nuclear
    flip.

Use Cases in concern:-----
In legacy modeset
1) Set fitting mode connector property.
2) Do set-crtc

In neuclearflip
1) Set fitting mode crtc property along with the other propery.

Both the cases the important difference will be from where the fittting
  mode is taken and pipe destination/ pfiters configutation is
  calculated accordingly.

A) There Will be no change in behavior for ILK or previous platforms
    before GEN9 as whole enhancement part is under Gen9 check on Nuclear
    flip path.
    If similar behavior is required for previous platforms need to check
    restrictions of PFIT register writting (On previous supported
    platforms) from HW Bspec.
    As far I understood we can't change PFIT registers without turning
    of pipe/modeset. Else we will see flickers/corruption.
B) If Legacy modeset used there will be no major issue should be there
    for GEN9+.
    Crtc_atomic_check will call pfit_calculate, which is still using
    connector_fitting_mode legacy property. Atomic mode property not
    used so bool pipe_scaler_changed is not updated,
    So PIpescaler will be not updated later by calling pch_panel_fitting
    in intel_crtc_atomic_check().




On 11/3/2016 2:53 PM, Maiti, Nabendu Bikash wrote:
> Hi,
>
> Please review the patches and comments.
>
>
> On 10/28/2016 6:05 PM, Nabendu Maiti wrote:
>> Attaching old discussion thread for easy reference.
>>
>> On Tue, Jan 05, 2016 at 05:18:40PM +0200, Ville Syrjälä wrote:
>>
>>> On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote:
>>
>>> > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote:
>>
>>> > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote:
>>
>>> > > > Hey,
>>
>>> > > >
>>
>>> > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti:
>>
>>> > > > > This patch is adding pipesource size as property as intel
>>
>>> > > > > property.User application is allowed to change the pipe source
>> size in runtime on BXT/SKL.
>>
>>> > > > > Added the property as inteli crtc property.
>>
>>> > > > >
>>
>>> > > > > Comments and suggestions are requested for whether we can keep
>>
>>> > > > > the property as intel crtc property or move to drm level.
>>
>>> > > > >
>>
>>> > > > This property would get lost on a modeset. But why do you need a
>> pipe_src property?
>>
>>> > >
>>
>>> > > It's needed if we want to use the panel fitter with
>>
>>> > > non-eDP/LVDS/DSI displays.
>>
>>> > >
>>
>>> > > Last time this came up I decided that we want to expose this via a
>>
>>> > > new "fixed_mode" property. Ie. userspace can choose the actual
>>
>>> > > display timings by setting the "fixed_mode" property with a
>>
>>> > > specific mode, and then the normal mode property will basically
>>
>>> > > just control just the pipe src size just like it already does for
>>
>>> > > eDP/LVDS/DSI where we already have the fixed_mode internally (just
>>
>>> > > not exposed to usersapce). If the fixed_mode is not specified,
>>
>>> > > things will work just as they do right now. Obviously for
>>
>>> > > eDP/LVDS/DSI we will reject any request to change/unset the fixed
>> mode.
>>
>>> > >
>>
>>> > > The benefit of that approach is that things will work exactly the
>>
>>> > > same way as before unless the user explicitly sets the fixed_mode,
>>
>>> > > and once it's set thigns will work exactly the same way as they
>>
>>> > > have worked so far for eDP/LVDS/DSI. Also it keeps the policy of
>>
>>> > > choosing the fixed mode strictly is userspace for external displays.
>>
>>> > >
>>
>>> > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to
>>
>>> > > userspace giving userspace more information on what the actual
>>
>>> > > panel timings are. Currently userspace has to more or less guess
>>
>>> > > that one of the modes the connector claims to support has the
>>
>>> > > actual panel timings.
>>
>>> > >
>>
>>> > > So far I've not heard any opposing opinions to this plan.
>>
>>> >
>>
>>> > Hm yeah, pipe_src would run into all kinds of fun in conjunction
>>
>>> > with the existing fixed mode stuff we have. Just exposing the fixed
>>
>>> > as a prop might be simpler. But then we need to figure out what to
>>
>>> > do wrt the clock in the mode passed in through userspace - currently
>>
>>> > the fixed mode always wins, but for manual DRRS it would be nice if
>>
>>> > userspace could control it, and doesn't have to know that there's a
>> fixed_mode.
>>
>>>
>>
>>> We could have the user mode vrefresh indicate the desired refresh rate
>>
>>> of the fixed mode as well. In fact I've been wanting to add a check to
>>
>>> make sure the user mode refresh rate isn't too far off from the
>>
>>> fixed/downlclock mode vrefresh, but didn't get around to it yet.
>>
>>
>>
>> Yeah agreed, userspace vrefresh should control (or at least be checked
>>
>> against) the fixed mode vrefresh.
>>
>>
>>
>>> > So semantically I think only exposing the pipe src to expose panel
>>
>>> > fitters would be cleaner. Then we'd need to deal with some internal
>>
>>> > troubles to make sure we combine everything correctly, but that
>>
>>> > shouldn't be too hard really.
>>
>>>
>>
>>> I think it's quite a bit more work since we have to redo all the fb
>>
>>> size checks and whatnot to use the property when specified. I once
>>
>>> outlined a more detailed plan for his approach too (too lazy to dig up
>>
>>> the mail), but I think the fixed mode prop is a simpler approach and
>>
>>> won't actually require much in the way of userspace changes either.
>>
>>> It'll be enough to set the property once and then even the legacy
>>
>>> modeset ioctls will work exactly like they do now for eDP/LVDS/DSI.
>>
>>
>>
>> One downside of the fixed mode against an explicit pipe src rectangle is
>> that the explict rectangle allows us to letter/pillar/stretch/move too.
>>
>> With just a fixed_mode that's much harder to pull off.
>>
>>
>>
>> I think for i915 it should be fairly ok-ish to implement this. We just
>> need to move pipe src rectangle to drm_crtc_state, and adjust all the
>> pfit config logic to work on the pipe level. Panels would then only
>> replace the output mode with the fixed panel mode, and leave
>> scaling/pfit selection to the core crtc code.
>>
>> -Daniel
>>
>> --
>>
>> Daniel Vetter
>>
>> Software Engineer, Intel Corporation
>>
>> http://blog.ffwll.ch
>>
>> _______________________________________________
>>
>> Intel-gfx mailing list
>>
>> Intel-gfx@lists.freedesktop.org <mailto:Intel-gfx@lists.freedesktop.org>
>>
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> ..>
>> On 10/03/2016 04:57 PM, Ville Syrjälä wrote:
>>> On Wed, Sep 21, 2016 at 07:47:37PM +0530, Maiti, Nabendu Bikash wrote:
>>>> Hi,
>>>>
>>>>
>>>> On 9/20/2016 1:55 PM, Ville Syrjälä wrote:
>>>>> On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
>>>>>> Following patch series add pipe scaler functionality in atomic
>>>>>> path.The pipe
>>>>>> scaler can be changed dynamically without modeset.Apart from
>>>>>> default panel
>>>>>> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom
>>>>>> scaling mode
>>>>>> mode is added as there is no such restriction on GEn9 pipe scaler.
>>>>> Some quick observations:
>>>>> - missing any interaction with drm core, so all generic fb size checks
>>>>>   and whatnot will not work out, I think
>>>> Pipe scaler is not dependent on fp I think. We have fb size checks are
>>>> done in plane check.
>>> You need to explain how this all interacts with the legacy pipe/plane
>>> size == mode hdisplay/vdisplay stuff.
>> There is no direct interaction with the plane. This properties use
>> current modeset timings - horizontal and
>> vertical timing as stored in pipe_src_w and pipe_src_h variables. They
>> are filled just after pll and clock are set
>> and adjusted modeset structures are filled up. This design will not
>> interfere with current existing code of pipe/plane.
>>
>> Clipping and clamping on plane level is already done on Blended data
>> output of  pipe using existing code. pipe
>> level clamping and clipping will happen when pipe scaler using out of
>> boundary value. Current code use clamping only.
>>
>>>>> - the way it's done goes against what I've suggested in the past. My
>>>>>   idea has been to add a "fixed mode" property to connectors instead,
>>>>>   which I think would minimize the impact on the core, and it would
>>>>>   follow closely the way eDP/LVDS/DSI already works today.
>>>> yes using fixed mode we can do also but I wanted to be part of crtc
>>>> property instead of connector property. As fixed mode is basically
>>>> intended for fixed mode panels.But we may use pipe scaler for fixed
>>>> mode
>>>> and dynamic mode panels.
>>> That doesn't say much. The fixed mode apporach, I think, might be easier
>>> to incorporate in a way that keeps the legacy apporach working. Adding a
>>> totally different way to configure the pipe src size will mean more
>>> weird
>>> interactions between the properties.
>>>
>>> Also it culd be supported with non-atomic userspace reasonably easily.
>>> We'll need some sort of userspace for this anyway, otherwise it's just
>>> untested/unused code.
>> There will be no change in legacy code path. They will work as it is.
>> Only downside will be mixup of legacy and non-atomic use.
>> But individually they should work as fine as before. The configuration
>> of pipe_src is done as before. They will be
>> changed if user explicitly want to change them. Internal checks are
>> there if some odd unaccepted values are provided.
>>
>> Yes I can write the userspace igt if this design is fine. This design is
>> tested with inhouse val tool (igt based) on android
>> plaatform.
>>>>> - There's no need to restrict the feature to gen9+ only. It should
>>>>> work
>>>>>   out just fine for at least all pch platforms. gmch platforms
>>>>> would be
>>>>>   more challenging
>>>> This code I designed to use gen9+, and properties like crtc destination
>>>> size and offsets also exposed.There is no restrictions on modes (eg.
>>>> pillerbox/letterbox) and down scaling ratios as previous platforms.
>>>> Currently scaling mode is part of connector property and implemented as
>>>> legacy property. I created new scaling mode as atomic property. I think
>>>> gen9+ onward platforms may have proper atomic pipe scaling properties
>>>> and user space may use it fully dynamically without modeset.
>>> None of that tells me why it's gen9+ only. IIRC the panel fitter
>>> configuration been very flexible ever since ILK, so the only real
>>> difference should be which registers to write.
>> There is no hardware pipe scaler before gen7 (VLV/BXT). VLV +CHT
>> platform also can be supported on this design
>> with little modification. Also Letterbox/pillerbox modes can be
>> programmatically selected and according to proper
>> register write. But as far as I know old platforms has limited
>> functionality of panel fitting either by changing clock/ doing
>> nearby supported modeset. I understand fixed mode is the timing required
>> by panel and adjusted mode is the nearest permissive
>> timing available from gen platform. But using hardware scaler we don't
>> change timing. We keep the timing intact, but modify the
>> pixel data came from blender pipe. We modify the data size by
>> enlarging/encorching in between pipe and encoder.
>> ..
>>>>> - the pfiter_recalculate thing looks pretty wrong atomic wise
>>>> Sorry, I couldn't get it. Are you referring pipe scaler registers are
>>>> not written together with other registers?  pfiter_calculate only
>>>> calculate and stores the data for later commit. Please provide more
>>>> details on it.
>>> It's going through encoder->crtc links and whanot. That's not going
>>> to fly.
>> There is no use of encoder->crtc in the design, everything maintained in
>> drm crtc level. (Note Initial few patches using the legacy encoder->crtc
>> iteration to find current scaling mode). Final code will work
>> independently without using legacy fitting mode.
>> Pfit_calculate find out the pipe out size and position in panel if not
>> provided by user depending on the source size.
>> One use case for this kind of design may be following. One document
>> visibile on panel using several planes. Now clicking on it
>> will enlarge some part of the document and fit the display without
>> blanking (no modeset happens).And go back to original
>> on another double click. This is the the dynamic way we change pipe
>> scaler config pillerbox/letterbox etc.
>>
>> Regards,
>> Nabendu
>>
>>>>>> Nabendu Maiti (7):
>>>>>>   drm/i915: Add pipe scaler pipe source drm property
>>>>>>   drm/i915: Add pipe_src size property calculations in atomic path.
>>>>>>   drm/i915: Panel fitting calculation for GEN9
>>>>>>   drm/i915: Adding atomic fitting mode property for GEN9
>>>>>>   drm/i915: Add pipe scaler co-ordinate and size property for Gen9
>>>>>>   drm/i915: Update pipe-scaler according to destination size
>>>>>>   drm/i915: Pipescaler destination size limit check on Gen9
>>>>>>
>>>>>>  drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
>>>>>>  drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
>>>>>>  drivers/gpu/drm/i915/intel_display.c | 128
>>>>>> +++++++++++++++++++++++++++++++++--
>>>>>>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
>>>>>>  drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
>>>>>>  include/drm/drm_crtc.h               |  14 ++++
>>>>>>  include/uapi/drm/drm_mode.h          |   1 +
>>>>>>  7 files changed, 263 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 1.9.1
>>>>>>
>>>>>> _______________________________________________
>>>>>> Intel-gfx mailing list
>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>> --
>>>> Regards,
>>>> Nabendu
>>
>>
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>

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