[PATCH v19 00/18] Add NV12 support

[PATCH v19 00/18] Add NV12 support

[Vidya Srinivas]

This patch series is adding NV12 support for Broxton display after rebasing on
latest drm-tip.
Initial series of the patches can be found here:
https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.html

Previous revision history:
The first version of patches were reviewed when floated by Chandra in 2015
but currently there was a design change with respect to
- the way fb offset is handled
- the way rotation is handled
Current NV12 patch series has been ported as per the
current changes on drm-tip

Review comments from Ville (12th June 2017) have been addressed Review
comments from Clinton A Taylor (7th July 2017) have been addressed

Review comments from Clinton A Taylor (10th July 2017)
	have been addressed. Had missed out tested-by/reviewed-by in the patches.
	Fixed that error in this series.
	Review comments from Ville (11th July 2017) addressed.
	Review comments from Paauwe, Bob (29th July 2017) addressed.

Update from rev 28 Aug 2017
	Rebased the series.
	Tested with IGT for rotation, sprite and tiling combinations.
	IGT Links:
	https://patchwork.kernel.org/patch/9995943/
	https://patchwork.kernel.org/patch/9995945/
	Review comments by Maarten are addressed in this series.
	NV12 enabled for Gen10.
	Review comments from Shashank Sharma are addressed.
	IGT debug_fs test failure fixed.
	Added reviewed-by tag from Shashank Sharma for few patches
	Addressed comments from Juha-Pekka Heikkila in few patches
	(NV12 not to be supported for SKL)
	Adding an additional patch Display WA 827 for underrun during NV12
	Adding more WA implementation to see if it helps underruns
	Addressed review comments from Ville regarding the planar formats
	Added minimum src height for yuv 420 planar formats
	Added NV12 in skl_mod_supported
	(review comments from Kristian Høgsberg <hoegsberg@gmail.com>

Update from previous series:
	Addressed review comments from Ville and Maarten
	Re-ordered the series as per Maarten's suggestion

Chandra Konduru (6):
  drm/i915: Set scaler mode for NV12
  drm/i915: Update format_is_yuv() to include NV12
  drm/i915: Upscale scaler max scale for NV12
  drm/i915: Add NV12 as supported format for primary plane
  drm/i915: Add NV12 as supported format for sprite plane
  drm/i915: Add NV12 support to intel_framebuffer_init

Mahesh Kumar (9):
  drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values
  drm/i915/skl+: refactor WM calculation for NV12
  drm/i915/skl+: add NV12 in skl_format_to_fourcc
  drm/i915/skl+: support verification of DDB HW state for NV12
  drm/i915/skl+: NV12 related changes for WM
  drm/i915/skl+: pass skl_wm_level struct to wm compute func
  drm/i915/skl+: make sure higher latency level has higher wm value
  drm/i915/skl+: nv12 workaround disable WM level 1-7
  drm/i915/skl: split skl_compute_ddb function

Vidya Srinivas (3):
  drm/i915: Display WA 827
  drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg
  drm/i915: Set src size restrictions for NV12

 drivers/gpu/drm/i915/i915_drv.h      |  10 +-
 drivers/gpu/drm/i915/i915_reg.h      |   5 +
 drivers/gpu/drm/i915/intel_atomic.c  |  14 +-
 drivers/gpu/drm/i915/intel_display.c | 162 +++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  15 +-
 drivers/gpu/drm/i915/intel_pm.c      | 438 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_sprite.c  |  50 +++-
 7 files changed, 502 insertions(+), 192 deletions(-)

-- 
2.7.4

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

[PATCH v19 01/18] drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

skl_wm_values struct contains values of pipe/plane DDB only.
so rename it for better readability of code. Similarly
skl_copy_wm_for_pipe copies DDB values.

s/skl_wm_values/skl_ddb_values
s/skl_copy_wm_for_pipe/skl_copy_ddb_for_pipe

Changes since V1:
 - also change name of skl_copy_wm_for_pipe

v2: Added reviewed by from Juha-Pekka Heikkila

v3: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 drivers/gpu/drm/i915/intel_pm.c  | 16 ++++++++--------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5373b17..d3c80d4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1186,7 +1186,7 @@ struct skl_ddb_allocation {
 	struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
 };
 
-struct skl_wm_values {
+struct skl_ddb_values {
 	unsigned dirty_pipes;
 	struct skl_ddb_allocation ddb;
 };
@@ -1885,7 +1885,7 @@ struct drm_i915_private {
 		/* current hardware state */
 		union {
 			struct ilk_wm_values hw;
-			struct skl_wm_values skl_hw;
+			struct skl_ddb_values skl_hw;
 			struct vlv_wm_values vlv;
 			struct g4x_wm_values g4x;
 		};
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d1452fd..7f77e6d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -482,7 +482,7 @@ struct intel_atomic_state {
 	bool skip_intermediate_wm;
 
 	/* Gen9+ only */
-	struct skl_wm_values wm_results;
+	struct skl_ddb_values wm_results;
 
 	struct i915_sw_fence commit_ready;
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 19e82aa..d2963bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5042,9 +5042,9 @@ skl_compute_ddb(struct drm_atomic_state *state)
 }
 
 static void
-skl_copy_wm_for_pipe(struct skl_wm_values *dst,
-		     struct skl_wm_values *src,
-		     enum pipe pipe)
+skl_copy_ddb_for_pipe(struct skl_ddb_values *dst,
+		      struct skl_ddb_values *src,
+		      enum pipe pipe)
 {
 	memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
 	       sizeof(dst->ddb.y_plane[pipe]));
@@ -5095,7 +5095,7 @@ skl_compute_wm(struct drm_atomic_state *state)
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *cstate;
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	struct skl_wm_values *results = &intel_state->wm_results;
+	struct skl_ddb_values *results = &intel_state->wm_results;
 	struct drm_device *dev = state->dev;
 	struct skl_pipe_wm *pipe_wm;
 	bool changed = false;
@@ -5197,8 +5197,8 @@ static void skl_initial_wm(struct intel_atomic_state *state,
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct skl_wm_values *results = &state->wm_results;
-	struct skl_wm_values *hw_vals = &dev_priv->wm.skl_hw;
+	struct skl_ddb_values *results = &state->wm_results;
+	struct skl_ddb_values *hw_vals = &dev_priv->wm.skl_hw;
 	enum pipe pipe = intel_crtc->pipe;
 
 	if ((results->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) == 0)
@@ -5209,7 +5209,7 @@ static void skl_initial_wm(struct intel_atomic_state *state,
 	if (cstate->base.active_changed)
 		skl_atomic_update_crtc_wm(state, cstate);
 
-	skl_copy_wm_for_pipe(hw_vals, results, pipe);
+	skl_copy_ddb_for_pipe(hw_vals, results, pipe);
 
 	mutex_unlock(&dev_priv->wm.wm_mutex);
 }
@@ -5341,7 +5341,7 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc,
 void skl_wm_get_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
+	struct skl_ddb_values *hw = &dev_priv->wm.skl_hw;
 	struct skl_ddb_allocation *ddb = &dev_priv->wm.skl_hw.ddb;
 	struct drm_crtc *crtc;
 	struct intel_crtc *intel_crtc;
-- 
2.7.4

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

[PATCH v19 02/18] drm/i915/skl+: refactor WM calculation for NV12

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

Current code calculates DDB for planar formats in such a way that we
store DDB of plane-0 in plane 1 & vice-versa.
In order to make this clean this patch refactors WM/DDB calculation for
NV12 planar formats.

v2: Addressed review comments by Maarten

v3: Rebased and addressed review comments by Maarten

v4: Fixed a compilation issue of string replacement is_nv12 to
is_planar

v5: Added reviewed by from Juha-Pekka Heikkila

v6: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |   5 +-
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 drivers/gpu/drm/i915/intel_pm.c  | 121 ++++++++++++++++++++-------------------
 3 files changed, 66 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d3c80d4..d499264 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1182,8 +1182,9 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
 }
 
 struct skl_ddb_allocation {
-	struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* packed/uv */
-	struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
+	/* packed/y */
+	struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES];
+	struct skl_ddb_entry uv_plane[I915_MAX_PIPES][I915_MAX_PLANES];
 };
 
 struct skl_ddb_values {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7f77e6d..d7310fe 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -603,6 +603,7 @@ struct intel_pipe_wm {
 struct skl_plane_wm {
 	struct skl_wm_level wm[8];
 	struct skl_wm_level trans_wm;
+	bool is_planar;
 };
 
 struct skl_pipe_wm {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d2963bc..c051cd3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4009,9 +4009,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
 static unsigned int
 skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 			     const struct drm_plane_state *pstate,
-			     int y)
+			     const int plane)
 {
-	struct intel_plane *plane = to_intel_plane(pstate->plane);
+	struct intel_plane *intel_plane = to_intel_plane(pstate->plane);
 	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
 	uint32_t data_rate;
 	uint32_t width = 0, height = 0;
@@ -4025,9 +4025,9 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 	fb = pstate->fb;
 	format = fb->format->format;
 
-	if (plane->id == PLANE_CURSOR)
+	if (intel_plane->id == PLANE_CURSOR)
 		return 0;
-	if (y && format != DRM_FORMAT_NV12)
+	if (plane == 1 && format != DRM_FORMAT_NV12)
 		return 0;
 
 	/*
@@ -4038,19 +4038,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 	width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	height = drm_rect_height(&intel_pstate->base.src) >> 16;
 
-	/* for planar format */
-	if (format == DRM_FORMAT_NV12) {
-		if (y)  /* y-plane data rate */
-			data_rate = width * height *
-				fb->format->cpp[0];
-		else    /* uv-plane data rate */
-			data_rate = (width / 2) * (height / 2) *
-				fb->format->cpp[1];
-	} else {
-		/* for packed formats */
-		data_rate = width * height * fb->format->cpp[0];
+	/* UV plane does 1/2 pixel sub-sampling */
+	if (plane == 1 && format == DRM_FORMAT_NV12) {
+		width /= 2;
+		height /= 2;
 	}
 
+	data_rate = width * height * fb->format->cpp[plane];
+
 	down_scale_amount = skl_plane_downscale_amount(cstate, intel_pstate);
 
 	return mul_round_up_u32_fixed16(data_rate, down_scale_amount);
@@ -4063,8 +4058,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
  */
 static unsigned int
 skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
-				 unsigned *plane_data_rate,
-				 unsigned *plane_y_data_rate)
+				 unsigned int *plane_data_rate,
+				 unsigned int *uv_plane_data_rate)
 {
 	struct drm_crtc_state *cstate = &intel_cstate->base;
 	struct drm_atomic_state *state = cstate->state;
@@ -4080,17 +4075,17 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
 		enum plane_id plane_id = to_intel_plane(plane)->id;
 		unsigned int rate;
 
-		/* packed/uv */
+		/* packed/y */
 		rate = skl_plane_relative_data_rate(intel_cstate,
 						    pstate, 0);
 		plane_data_rate[plane_id] = rate;
 
 		total_data_rate += rate;
 
-		/* y-plane */
+		/* uv-plane */
 		rate = skl_plane_relative_data_rate(intel_cstate,
 						    pstate, 1);
-		plane_y_data_rate[plane_id] = rate;
+		uv_plane_data_rate[plane_id] = rate;
 
 		total_data_rate += rate;
 	}
@@ -4099,8 +4094,7 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
 }
 
 static uint16_t
-skl_ddb_min_alloc(const struct drm_plane_state *pstate,
-		  const int y)
+skl_ddb_min_alloc(const struct drm_plane_state *pstate, const int plane)
 {
 	struct drm_framebuffer *fb = pstate->fb;
 	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
@@ -4111,8 +4105,8 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 	if (WARN_ON(!fb))
 		return 0;
 
-	/* For packed formats, no y-plane, return 0 */
-	if (y && fb->format->format != DRM_FORMAT_NV12)
+	/* For packed formats, and uv-plane, return 0 */
+	if (plane == 1 && fb->format->format != DRM_FORMAT_NV12)
 		return 0;
 
 	/* For Non Y-tile return 8-blocks */
@@ -4131,15 +4125,12 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 	src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
 
 	/* Halve UV plane width and height for NV12 */
-	if (fb->format->format == DRM_FORMAT_NV12 && !y) {
+	if (plane == 1) {
 		src_w /= 2;
 		src_h /= 2;
 	}
 
-	if (fb->format->format == DRM_FORMAT_NV12 && !y)
-		plane_bpp = fb->format->cpp[1];
-	else
-		plane_bpp = fb->format->cpp[0];
+	plane_bpp = fb->format->cpp[plane];
 
 	if (drm_rotation_90_or_270(pstate->rotation)) {
 		switch (plane_bpp) {
@@ -4167,7 +4158,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 
 static void
 skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
-		 uint16_t *minimum, uint16_t *y_minimum)
+		 uint16_t *minimum, uint16_t *uv_minimum)
 {
 	const struct drm_plane_state *pstate;
 	struct drm_plane *plane;
@@ -4182,7 +4173,7 @@ skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
 			continue;
 
 		minimum[plane_id] = skl_ddb_min_alloc(pstate, 0);
-		y_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
+		uv_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
 	}
 
 	minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active);
@@ -4200,17 +4191,17 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
 	uint16_t alloc_size, start;
 	uint16_t minimum[I915_MAX_PLANES] = {};
-	uint16_t y_minimum[I915_MAX_PLANES] = {};
+	uint16_t uv_minimum[I915_MAX_PLANES] = {};
 	unsigned int total_data_rate;
 	enum plane_id plane_id;
 	int num_active;
-	unsigned plane_data_rate[I915_MAX_PLANES] = {};
-	unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
+	unsigned int plane_data_rate[I915_MAX_PLANES] = {};
+	unsigned int uv_plane_data_rate[I915_MAX_PLANES] = {};
 	uint16_t total_min_blocks = 0;
 
 	/* Clear the partitioning for disabled planes. */
 	memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
-	memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
+	memset(ddb->uv_plane[pipe], 0, sizeof(ddb->uv_plane[pipe]));
 
 	if (WARN_ON(!state))
 		return 0;
@@ -4225,7 +4216,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	if (alloc_size == 0)
 		return 0;
 
-	skl_ddb_calc_min(cstate, num_active, minimum, y_minimum);
+	skl_ddb_calc_min(cstate, num_active, minimum, uv_minimum);
 
 	/*
 	 * 1. Allocate the mininum required blocks for each active plane
@@ -4235,7 +4226,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 
 	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
 		total_min_blocks += minimum[plane_id];
-		total_min_blocks += y_minimum[plane_id];
+		total_min_blocks += uv_minimum[plane_id];
 	}
 
 	if (total_min_blocks > alloc_size) {
@@ -4257,14 +4248,14 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	 */
 	total_data_rate = skl_get_total_relative_data_rate(cstate,
 							   plane_data_rate,
-							   plane_y_data_rate);
+							   uv_plane_data_rate);
 	if (total_data_rate == 0)
 		return 0;
 
 	start = alloc->start;
 	for_each_plane_id_on_crtc(intel_crtc, plane_id) {
-		unsigned int data_rate, y_data_rate;
-		uint16_t plane_blocks, y_plane_blocks = 0;
+		unsigned int data_rate, uv_data_rate;
+		uint16_t plane_blocks, uv_plane_blocks;
 
 		if (plane_id == PLANE_CURSOR)
 			continue;
@@ -4288,21 +4279,20 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 
 		start += plane_blocks;
 
-		/*
-		 * allocation for y_plane part of planar format:
-		 */
-		y_data_rate = plane_y_data_rate[plane_id];
+		/* Allocate DDB for UV plane for planar format/NV12 */
+		uv_data_rate = uv_plane_data_rate[plane_id];
 
-		y_plane_blocks = y_minimum[plane_id];
-		y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
-					total_data_rate);
+		uv_plane_blocks = uv_minimum[plane_id];
+		uv_plane_blocks += div_u64((uint64_t)alloc_size * uv_data_rate,
+					   total_data_rate);
 
-		if (y_data_rate) {
-			ddb->y_plane[pipe][plane_id].start = start;
-			ddb->y_plane[pipe][plane_id].end = start + y_plane_blocks;
+		if (uv_data_rate) {
+			ddb->uv_plane[pipe][plane_id].start = start;
+			ddb->uv_plane[pipe][plane_id].end =
+				start + uv_plane_blocks;
 		}
 
-		start += y_plane_blocks;
+		start += uv_plane_blocks;
 	}
 
 	return 0;
@@ -4430,8 +4420,7 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
 		wp->width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	}
 
-	wp->cpp = (fb->format->format == DRM_FORMAT_NV12) ? fb->format->cpp[1] :
-							    fb->format->cpp[0];
+	wp->cpp = fb->format->cpp[0];
 	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
 							     intel_pstate);
 
@@ -4660,6 +4649,9 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 			return ret;
 	}
 
+	if (intel_pstate->base.fb->format->format == DRM_FORMAT_NV12)
+		wm->is_planar = true;
+
 	return 0;
 }
 
@@ -4833,10 +4825,21 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
 
 	skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
 			    &ddb->plane[pipe][plane_id]);
-	if (INTEL_GEN(dev_priv) < 11)
+	if (INTEL_GEN(dev_priv) >= 11)
+		return skl_ddb_entry_write(dev_priv,
+					   PLANE_BUF_CFG(pipe, plane_id),
+					   &ddb->plane[pipe][plane_id]);
+	if (wm->is_planar) {
+		skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
+				    &ddb->uv_plane[pipe][plane_id]);
 		skl_ddb_entry_write(dev_priv,
 				    PLANE_NV12_BUF_CFG(pipe, plane_id),
-				    &ddb->y_plane[pipe][plane_id]);
+				    &ddb->plane[pipe][plane_id]);
+	} else {
+		skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
+				    &ddb->plane[pipe][plane_id]);
+		I915_WRITE(PLANE_NV12_BUF_CFG(pipe, plane_id), 0x0);
+	}
 }
 
 static void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
@@ -4951,8 +4954,8 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
 
 		if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][plane_id],
 					&new_ddb->plane[pipe][plane_id]) &&
-		    skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][plane_id],
-					&new_ddb->y_plane[pipe][plane_id]))
+		    skl_ddb_entry_equal(&cur_ddb->uv_plane[pipe][plane_id],
+					&new_ddb->uv_plane[pipe][plane_id]))
 			continue;
 
 		plane_state = drm_atomic_get_plane_state(state, plane);
@@ -5046,8 +5049,8 @@ skl_copy_ddb_for_pipe(struct skl_ddb_values *dst,
 		      struct skl_ddb_values *src,
 		      enum pipe pipe)
 {
-	memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
-	       sizeof(dst->ddb.y_plane[pipe]));
+	memcpy(dst->ddb.uv_plane[pipe], src->ddb.uv_plane[pipe],
+	       sizeof(dst->ddb.uv_plane[pipe]));
 	memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
 	       sizeof(dst->ddb.plane[pipe]));
 }
-- 
2.7.4

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

[PATCH v19 03/18] drm/i915/skl+: add NV12 in skl_format_to_fourcc

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

Add support of recognizing DRM_FORMAT_NV12 from plane_format
register value.

v2: Added reviewed by tag from Mika Kahola

v3: Added reviewed by from Juha-Pekka Heikkila

v4: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 415fb8c..3038218 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2662,6 +2662,8 @@ static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
 	switch (format) {
 	case PLANE_CTL_FORMAT_RGB_565:
 		return DRM_FORMAT_RGB565;
+	case PLANE_CTL_FORMAT_NV12:
+		return DRM_FORMAT_NV12;
 	default:
 	case PLANE_CTL_FORMAT_XRGB_8888:
 		if (rgb_order) {
-- 
2.7.4

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

[PATCH v19 04/18] drm/i915/skl+: support verification of DDB HW state for NV12

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

For YUV 420 Planar formats like NV12,
buffer allocation is done for Y and UV surfaces separately.
For NV12 plane formats, the UV buffer
allocation must be programmed in the Plane Buffer Config register
and the Y buffer allocation must be programmed in the
Plane NV12 Buffer Config register. Both register values
should be verified during verify_wm_state.

v2: Addressed review comments by Maarten.

v3: Addressed review comments by Shashank Sharma.

v4: Adding reviewed by tag from Shashank Sharma

v5: Added reviewed by from Juha-Pekka Heikkila

v6: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 51 +++++++++++++++++++++++++++++-------
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3038218..28de533 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2657,7 +2657,7 @@ static int i9xx_format_to_fourcc(int format)
 	}
 }
 
-static int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
+int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
 {
 	switch (format) {
 	case PLANE_CTL_FORMAT_RGB_565:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d7310fe..ed79a61 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1612,6 +1612,7 @@ u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
 int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
 			    struct intel_plane_state *plane_state);
 int i9xx_check_plane_surface(struct intel_plane_state *plane_state);
+int skl_format_to_fourcc(int format, bool rgb_order, bool alpha);
 
 /* intel_csr.c */
 void intel_csr_ucode_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c051cd3..0f99652 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3825,6 +3825,44 @@ static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, u32 reg)
 		entry->end += 1;
 }
 
+static void
+skl_ddb_get_hw_plane_state(struct drm_i915_private *dev_priv,
+			   const enum pipe pipe,
+			   const enum plane_id plane_id,
+			   struct skl_ddb_allocation *ddb /* out */)
+{
+	u32 val, val2 = 0;
+	int fourcc, pixel_format;
+
+	/* Cursor doesn't support NV12/planar, so no extra calculation needed */
+	if (plane_id == PLANE_CURSOR) {
+		val = I915_READ(CUR_BUF_CFG(pipe));
+		skl_ddb_entry_init_from_hw(&ddb->plane[pipe][plane_id], val);
+		return;
+	}
+
+	val = I915_READ(PLANE_CTL(pipe, plane_id));
+
+	/* No DDB allocated for disabled planes */
+	if (!(val & PLANE_CTL_ENABLE))
+		return;
+
+	pixel_format = val & PLANE_CTL_FORMAT_MASK;
+	fourcc = skl_format_to_fourcc(pixel_format,
+				      val & PLANE_CTL_ORDER_RGBX,
+				      val & PLANE_CTL_ALPHA_MASK);
+
+	val = I915_READ(PLANE_BUF_CFG(pipe, plane_id));
+	val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id));
+
+	if (fourcc == DRM_FORMAT_NV12) {
+		skl_ddb_entry_init_from_hw(&ddb->plane[pipe][plane_id], val2);
+		skl_ddb_entry_init_from_hw(&ddb->uv_plane[pipe][plane_id], val);
+	} else {
+		skl_ddb_entry_init_from_hw(&ddb->plane[pipe][plane_id], val);
+	}
+}
+
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 			  struct skl_ddb_allocation *ddb /* out */)
 {
@@ -3841,16 +3879,9 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 		if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 			continue;
 
-		for_each_plane_id_on_crtc(crtc, plane_id) {
-			u32 val;
-
-			if (plane_id != PLANE_CURSOR)
-				val = I915_READ(PLANE_BUF_CFG(pipe, plane_id));
-			else
-				val = I915_READ(CUR_BUF_CFG(pipe));
-
-			skl_ddb_entry_init_from_hw(&ddb->plane[pipe][plane_id], val);
-		}
+		for_each_plane_id_on_crtc(crtc, plane_id)
+			skl_ddb_get_hw_plane_state(dev_priv, pipe,
+						   plane_id, ddb);
 
 		intel_display_power_put(dev_priv, power_domain);
 	}
-- 
2.7.4

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

[PATCH v19 05/18] drm/i915/skl+: NV12 related changes for WM

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

NV12 requires WM calculation for UV plane as well.
UV plane WM should also fulfill all the WM related restrictions.

v2: Addressed review comments from Shashank Sharma.

v3: Addressed review comments from Shashank Sharma
Changed plane_num to plane_id in skl_compute_plane_wm_params
and skl_compute_plane_wm.
Adding reviewed by tag from Shashank Sharma

v4: Added reviewed by from Juha-Pekka Heikkila

v5: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 50 +++++++++++++++++++++++++++++++++-------
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d499264..3115d26 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1202,6 +1202,7 @@ struct skl_wm_level {
 struct skl_wm_params {
 	bool x_tiled, y_tiled;
 	bool rc_surface;
+	bool is_planar;
 	uint32_t width;
 	uint8_t cpp;
 	uint32_t plane_pixel_rate;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ed79a61..272c091 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -602,6 +602,7 @@ struct intel_pipe_wm {
 
 struct skl_plane_wm {
 	struct skl_wm_level wm[8];
+	struct skl_wm_level uv_wm[8];
 	struct skl_wm_level trans_wm;
 	bool is_planar;
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0f99652..854671f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4419,7 +4419,7 @@ static int
 skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
 			    struct intel_crtc_state *cstate,
 			    const struct intel_plane_state *intel_pstate,
-			    struct skl_wm_params *wp)
+			    struct skl_wm_params *wp, int plane_id)
 {
 	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
 	const struct drm_plane_state *pstate = &intel_pstate->base;
@@ -4432,6 +4432,12 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
 	if (!intel_wm_plane_visible(cstate, intel_pstate))
 		return 0;
 
+	/* only NV12 format has two planes */
+	if (plane_id == 1 && fb->format->format != DRM_FORMAT_NV12) {
+		DRM_DEBUG_KMS("Non NV12 format have single plane\n");
+		return -EINVAL;
+	}
+
 	wp->y_tiled = fb->modifier == I915_FORMAT_MOD_Y_TILED ||
 		      fb->modifier == I915_FORMAT_MOD_Yf_TILED ||
 		      fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
@@ -4439,6 +4445,7 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
 	wp->x_tiled = fb->modifier == I915_FORMAT_MOD_X_TILED;
 	wp->rc_surface = fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
 			 fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
+	wp->is_planar = fb->format->format == DRM_FORMAT_NV12;
 
 	if (plane->id == PLANE_CURSOR) {
 		wp->width = intel_pstate->base.crtc_w;
@@ -4451,7 +4458,10 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
 		wp->width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	}
 
-	wp->cpp = fb->format->cpp[0];
+	if (plane_id == 1 && wp->is_planar)
+		wp->width /= 2;
+
+	wp->cpp = fb->format->cpp[plane_id];
 	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
 							     intel_pstate);
 
@@ -4649,7 +4659,8 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 		      struct intel_crtc_state *cstate,
 		      const struct intel_plane_state *intel_pstate,
 		      const struct skl_wm_params *wm_params,
-		      struct skl_plane_wm *wm)
+		      struct skl_plane_wm *wm,
+		      int plane_id)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_plane *plane = intel_pstate->base.plane;
@@ -4657,15 +4668,19 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 	uint16_t ddb_blocks;
 	enum pipe pipe = intel_crtc->pipe;
 	int level, max_level = ilk_wm_max_level(dev_priv);
+	enum plane_id intel_plane_id = intel_plane->id;
 	int ret;
 
 	if (WARN_ON(!intel_pstate->base.fb))
 		return -EINVAL;
 
-	ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][intel_plane->id]);
+	ddb_blocks = plane_id ?
+		     skl_ddb_entry_size(&ddb->uv_plane[pipe][intel_plane_id]) :
+		     skl_ddb_entry_size(&ddb->plane[pipe][intel_plane_id]);
 
 	for (level = 0; level <= max_level; level++) {
-		struct skl_wm_level *result = &wm->wm[level];
+		struct skl_wm_level *result = plane_id ? &wm->uv_wm[level] :
+							  &wm->wm[level];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4792,20 +4807,39 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
 
 		wm = &pipe_wm->planes[plane_id];
 		ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
-		memset(&wm_params, 0, sizeof(struct skl_wm_params));
 
 		ret = skl_compute_plane_wm_params(dev_priv, cstate,
-						  intel_pstate, &wm_params);
+						  intel_pstate, &wm_params, 0);
 		if (ret)
 			return ret;
 
 		ret = skl_compute_wm_levels(dev_priv, ddb, cstate,
-					    intel_pstate, &wm_params, wm);
+					    intel_pstate, &wm_params, wm, 0);
 		if (ret)
 			return ret;
+
 		skl_compute_transition_wm(cstate, &wm_params, &wm->wm[0],
 					  ddb_blocks, &wm->trans_wm);
+
+		/* uv plane watermarks must also be validated for NV12/Planar */
+		if (wm_params.is_planar) {
+			memset(&wm_params, 0, sizeof(struct skl_wm_params));
+			wm->is_planar = true;
+
+			ret = skl_compute_plane_wm_params(dev_priv, cstate,
+							  intel_pstate,
+							  &wm_params, 1);
+			if (ret)
+				return ret;
+
+			ret = skl_compute_wm_levels(dev_priv, ddb, cstate,
+						    intel_pstate, &wm_params,
+						    wm, 1);
+			if (ret)
+				return ret;
+		}
 	}
+
 	pipe_wm->linetime = skl_compute_linetime_wm(cstate);
 
 	return 0;
-- 
2.7.4

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

[PATCH v19 06/18] drm/i915/skl+: pass skl_wm_level struct to wm compute func

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

This patch passes skl_wm_level structure itself to watermark
computation function skl_compute_plane_wm function (instead
of its internal parameters). It reduces number of arguments
required to be passed.

v2: Addressed review comments by Shashank Sharma

v3: Adding reviewed by tag from Shashank Sharma

v4: Added reviewed by from Juha-Pekka Heikkila

v5: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 854671f..eb17464 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4529,9 +4529,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				uint16_t ddb_allocation,
 				int level,
 				const struct skl_wm_params *wp,
-				uint16_t *out_blocks, /* out */
-				uint8_t *out_lines, /* out */
-				bool *enabled /* out */)
+				struct skl_wm_level *result /* out */)
 {
 	const struct drm_plane_state *pstate = &intel_pstate->base;
 	uint32_t latency = dev_priv->wm.skl_latency[level];
@@ -4545,7 +4543,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 
 	if (latency == 0 ||
 	    !intel_wm_plane_visible(cstate, intel_pstate)) {
-		*enabled = false;
+		result->plane_en = false;
 		return 0;
 	}
 
@@ -4626,7 +4624,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	if ((level > 0 && res_lines > 31) ||
 	    res_blocks >= ddb_allocation ||
 	    min_disp_buf_needed >= ddb_allocation) {
-		*enabled = false;
+		result->plane_en = false;
 
 		/*
 		 * If there are no valid level 0 watermarks, then we can't
@@ -4646,9 +4644,9 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	}
 
 	/* The number of lines are ignored for the level 0 watermark. */
-	*out_lines = level ? res_lines : 0;
-	*out_blocks = res_blocks;
-	*enabled = true;
+	result->plane_res_b = res_blocks;
+	result->plane_res_l = res_lines;
+	result->plane_en = true;
 
 	return 0;
 }
@@ -4688,9 +4686,7 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 					   ddb_blocks,
 					   level,
 					   wm_params,
-					   &result->plane_res_b,
-					   &result->plane_res_l,
-					   &result->plane_en);
+					   result);
 		if (ret)
 			return ret;
 	}
-- 
2.7.4

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

[PATCH v19 07/18] drm/i915/skl+: make sure higher latency level has higher wm value

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

DDB allocation optimization algorithm requires/assumes ddb allocation for
any memory C-state level DDB value to be as high as level below the
current level. Render decompression requires level WM to be as high as
wm level-0. This patch fulfils both the requirements.

v2: Changed plane_num to plane_id in skl_compute_wm_levels

v3: Addressed review comments from Shashank Sharma
Changed the commit message "statement can be more clear,
"DDB value to be as high as level below " what is level below ?"

v4: Added reviewed by tag from Shashank Sharma

v5: Added reviewed by from Juha-Pekka Heikkila

v6: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index eb17464..b318a27 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4529,6 +4529,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				uint16_t ddb_allocation,
 				int level,
 				const struct skl_wm_params *wp,
+				const struct skl_wm_level *result_prev,
 				struct skl_wm_level *result /* out */)
 {
 	const struct drm_plane_state *pstate = &intel_pstate->base;
@@ -4596,6 +4597,15 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 		} else {
 			res_blocks++;
 		}
+
+		/*
+		 * Make sure result blocks for higher latency levels are atleast
+		 * as high as level below the current level.
+		 * Assumption in DDB algorithm optimization for special cases.
+		 * Also covers Display WA #1125 for RC.
+		 */
+		if (result_prev->plane_res_b > res_blocks)
+			res_blocks = result_prev->plane_res_b;
 	}
 
 	if (INTEL_GEN(dev_priv) >= 11) {
@@ -4679,6 +4689,13 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 	for (level = 0; level <= max_level; level++) {
 		struct skl_wm_level *result = plane_id ? &wm->uv_wm[level] :
 							  &wm->wm[level];
+		struct skl_wm_level *result_prev;
+
+		if (level)
+			result_prev = plane_id ? &wm->uv_wm[level - 1] :
+						  &wm->wm[level - 1];
+		else
+			result_prev = plane_id ? &wm->uv_wm[0] : &wm->wm[0];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4686,6 +4703,7 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 					   ddb_blocks,
 					   level,
 					   wm_params,
+					   result_prev,
 					   result);
 		if (ret)
 			return ret;
-- 
2.7.4

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

[PATCH v19 08/18] drm/i915/skl+: nv12 workaround disable WM level 1-7

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

Display Workaround #0826 (SKL:ALL BXT:ALL) & #1059(CNL:A)
Hardware sometimes fails to wake memory from pkg C states fetching the
last few lines of planar YUV 420 (NV12) planes. This causes
intermittent underflow and corruption.
WA: Disable package C states or do not enable latency levels 1 through 7
(WM1 - WM7) on NV12 planes.

v2: Addressed review comments by Maarten.

v3: Adding reviewed by tag from Shashank Sharma

v4: Added reviewed by from Juha-Pekka Heikkila

v5: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b318a27..3210868 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4653,6 +4653,17 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 		}
 	}
 
+	/*
+	 * Display WA #826 (SKL:ALL, BXT:ALL) & #1059 (CNL:A)
+	 * disable wm level 1-7 on NV12 planes
+	 */
+	if (wp->is_planar && level >= 1 &&
+	    (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
+	     IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))) {
+		result->plane_en = false;
+		return 0;
+	}
+
 	/* The number of lines are ignored for the level 0 watermark. */
 	result->plane_res_b = res_blocks;
 	result->plane_res_l = res_lines;
-- 
2.7.4

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

[PATCH v19 09/18] drm/i915/skl: split skl_compute_ddb function

[Vidya Srinivas]

From: Mahesh Kumar <mahesh1.kumar@intel.com>

This patch splits skl_compute_wm/ddb functions into two parts.
One adds all affected pipes after the commit to atomic_state structure
and second part does compute the DDB.

v2: Added reviewed by tag from Shashank Sharma

v3: Added reviewed by from Juha-Pekka Heikkila

v4: Rebased the series

v5: Fixed checkpatch error. Changed *changed = true
to (*changed) = true;

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 157 ++++++++++++++++++++++------------------
 1 file changed, 88 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3210868..ffed139 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5059,69 +5059,16 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
 static int
 skl_compute_ddb(struct drm_atomic_state *state)
 {
-	struct drm_device *dev = state->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	const struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	struct intel_crtc *intel_crtc;
 	struct skl_ddb_allocation *ddb = &intel_state->wm_results.ddb;
-	uint32_t realloc_pipes = pipes_modified(state);
-	int ret;
-
-	/*
-	 * If this is our first atomic update following hardware readout,
-	 * we can't trust the DDB that the BIOS programmed for us.  Let's
-	 * pretend that all pipes switched active status so that we'll
-	 * ensure a full DDB recompute.
-	 */
-	if (dev_priv->wm.distrust_bios_wm) {
-		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
-				       state->acquire_ctx);
-		if (ret)
-			return ret;
-
-		intel_state->active_pipe_changes = ~0;
-
-		/*
-		 * We usually only initialize intel_state->active_crtcs if we
-		 * we're doing a modeset; make sure this field is always
-		 * initialized during the sanitization process that happens
-		 * on the first commit too.
-		 */
-		if (!intel_state->modeset)
-			intel_state->active_crtcs = dev_priv->active_crtcs;
-	}
-
-	/*
-	 * If the modeset changes which CRTC's are active, we need to
-	 * recompute the DDB allocation for *all* active pipes, even
-	 * those that weren't otherwise being modified in any way by this
-	 * atomic commit.  Due to the shrinking of the per-pipe allocations
-	 * when new active CRTC's are added, it's possible for a pipe that
-	 * we were already using and aren't changing at all here to suddenly
-	 * become invalid if its DDB needs exceeds its new allocation.
-	 *
-	 * Note that if we wind up doing a full DDB recompute, we can't let
-	 * any other display updates race with this transaction, so we need
-	 * to grab the lock on *all* CRTC's.
-	 */
-	if (intel_state->active_pipe_changes) {
-		realloc_pipes = ~0;
-		intel_state->wm_results.dirty_pipes = ~0;
-	}
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *cstate;
+	int ret, i;
 
-	/*
-	 * We're not recomputing for the pipes not included in the commit, so
-	 * make sure we start with the current state.
-	 */
 	memcpy(ddb, &dev_priv->wm.skl_hw.ddb, sizeof(*ddb));
 
-	for_each_intel_crtc_mask(dev, intel_crtc, realloc_pipes) {
-		struct intel_crtc_state *cstate;
-
-		cstate = intel_atomic_get_crtc_state(state, intel_crtc);
-		if (IS_ERR(cstate))
-			return PTR_ERR(cstate);
-
+	for_each_new_intel_crtc_in_state(intel_state, crtc, cstate, i) {
 		ret = skl_allocate_pipe_ddb(cstate, ddb);
 		if (ret)
 			return ret;
@@ -5183,23 +5130,23 @@ skl_print_wm_changes(const struct drm_atomic_state *state)
 }
 
 static int
-skl_compute_wm(struct drm_atomic_state *state)
+skl_ddb_add_affected_pipes(struct drm_atomic_state *state, bool *changed)
 {
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *cstate;
-	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	struct skl_ddb_values *results = &intel_state->wm_results;
 	struct drm_device *dev = state->dev;
-	struct skl_pipe_wm *pipe_wm;
-	bool changed = false;
+	const struct drm_i915_private *dev_priv = to_i915(dev);
+	const struct drm_crtc *crtc;
+	const struct drm_crtc_state *cstate;
+	struct intel_crtc *intel_crtc;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	uint32_t realloc_pipes = pipes_modified(state);
 	int ret, i;
 
 	/*
 	 * When we distrust bios wm we always need to recompute to set the
 	 * expected DDB allocations for each CRTC.
 	 */
-	if (to_i915(dev)->wm.distrust_bios_wm)
-		changed = true;
+	if (dev_priv->wm.distrust_bios_wm)
+		(*changed) = true;
 
 	/*
 	 * If this transaction isn't actually touching any CRTC's, don't
@@ -5210,14 +5157,86 @@ skl_compute_wm(struct drm_atomic_state *state)
 	 * hold _all_ CRTC state mutexes.
 	 */
 	for_each_new_crtc_in_state(state, crtc, cstate, i)
-		changed = true;
+		(*changed) = true;
 
-	if (!changed)
+	if (!*changed)
 		return 0;
 
+	/*
+	 * If this is our first atomic update following hardware readout,
+	 * we can't trust the DDB that the BIOS programmed for us.  Let's
+	 * pretend that all pipes switched active status so that we'll
+	 * ensure a full DDB recompute.
+	 */
+	if (dev_priv->wm.distrust_bios_wm) {
+		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
+				       state->acquire_ctx);
+		if (ret)
+			return ret;
+
+		intel_state->active_pipe_changes = ~0;
+
+		/*
+		 * We usually only initialize intel_state->active_crtcs if we
+		 * we're doing a modeset; make sure this field is always
+		 * initialized during the sanitization process that happens
+		 * on the first commit too.
+		 */
+		if (!intel_state->modeset)
+			intel_state->active_crtcs = dev_priv->active_crtcs;
+	}
+
+	/*
+	 * If the modeset changes which CRTC's are active, we need to
+	 * recompute the DDB allocation for *all* active pipes, even
+	 * those that weren't otherwise being modified in any way by this
+	 * atomic commit.  Due to the shrinking of the per-pipe allocations
+	 * when new active CRTC's are added, it's possible for a pipe that
+	 * we were already using and aren't changing at all here to suddenly
+	 * become invalid if its DDB needs exceeds its new allocation.
+	 *
+	 * Note that if we wind up doing a full DDB recompute, we can't let
+	 * any other display updates race with this transaction, so we need
+	 * to grab the lock on *all* CRTC's.
+	 */
+	if (intel_state->active_pipe_changes) {
+		realloc_pipes = ~0;
+		intel_state->wm_results.dirty_pipes = ~0;
+	}
+
+	/*
+	 * We're not recomputing for the pipes not included in the commit, so
+	 * make sure we start with the current state.
+	 */
+	for_each_intel_crtc_mask(dev, intel_crtc, realloc_pipes) {
+		struct intel_crtc_state *cstate;
+
+		cstate = intel_atomic_get_crtc_state(state, intel_crtc);
+		if (IS_ERR(cstate))
+			return PTR_ERR(cstate);
+	}
+
+	return 0;
+}
+
+static int
+skl_compute_wm(struct drm_atomic_state *state)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *cstate;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	struct skl_ddb_values *results = &intel_state->wm_results;
+	struct skl_pipe_wm *pipe_wm;
+	bool changed = false;
+	int ret, i;
+
 	/* Clear all dirty flags */
 	results->dirty_pipes = 0;
 
+	ret = skl_ddb_add_affected_pipes(state, &changed);
+	if (ret || !changed)
+		return ret;
+
 	ret = skl_compute_ddb(state);
 	if (ret)
 		return ret;
-- 
2.7.4

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

[PATCH v19 10/18] drm/i915: Display WA 827

[Vidya Srinivas]

Display WA 827 applies to GEN9 (excluede GLK) and CNL.
Switching the plane format from NV12 to RGB and leaving system idle
results in display underrun and corruption.
WA: Set the bit 15 & bit 19 to 1b in the CLKGATE_DIS_PSL
register for the pipe in which NV12 plane is enabled.

v2: Addressed review comments from Maarten and
Juha-Pekka Heikkila. Added reviewed by from
Juha-Pekka Heikkila.

v3: Rebased the series

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 176dca6..2afa96b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3812,6 +3812,9 @@ enum {
 #define _CLKGATE_DIS_PSL_A		0x46520
 #define _CLKGATE_DIS_PSL_B		0x46524
 #define _CLKGATE_DIS_PSL_C		0x46528
+#define   DUPS1_GATING_DIS		(1 << 15)
+#define   DUPS2_GATING_DIS		(1 << 19)
+#define   DUPS3_GATING_DIS		(1 << 23)
 #define   DPF_GATING_DIS		(1 << 10)
 #define   DPF_RAM_GATING_DIS		(1 << 9)
 #define   DPFR_GATING_DIS		(1 << 8)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 28de533..b27e0b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -488,6 +488,21 @@ static const struct intel_limit intel_limits_bxt = {
 	.p2 = { .p2_slow = 1, .p2_fast = 20 },
 };
 
+static void
+skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable)
+{
+	if (IS_SKYLAKE(dev_priv))
+		return;
+
+	if (enable)
+		I915_WRITE(CLKGATE_DIS_PSL(pipe),
+			   DUPS1_GATING_DIS | DUPS2_GATING_DIS);
+	else
+		I915_WRITE(CLKGATE_DIS_PSL(pipe),
+			   I915_READ(CLKGATE_DIS_PSL(pipe)) &
+			   ~(DUPS1_GATING_DIS | DUPS2_GATING_DIS));
+}
+
 static bool
 needs_modeset(const struct drm_crtc_state *state)
 {
@@ -5106,6 +5121,8 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s
 static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_atomic_state *old_state = old_crtc_state->base.state;
 	struct intel_crtc_state *pipe_config =
 		intel_atomic_get_new_crtc_state(to_intel_atomic_state(old_state),
@@ -5128,6 +5145,7 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 							 to_intel_plane(primary));
 		struct intel_plane_state *old_primary_state =
 			to_intel_plane_state(old_pri_state);
+		struct drm_framebuffer *fb = primary_state->base.fb;
 
 		intel_fbc_post_update(crtc);
 
@@ -5135,6 +5153,14 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 		    (needs_modeset(&pipe_config->base) ||
 		     !old_primary_state->base.visible))
 			intel_post_enable_primary(&crtc->base, pipe_config);
+
+		/* Display WA 827 */
+		if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
+		    IS_CANNONLAKE(dev_priv)) {
+			if (fb && fb->format->format == DRM_FORMAT_NV12)
+				skl_wa_clkgate(dev_priv, crtc->pipe, false);
+		}
+
 	}
 }
 
@@ -5161,6 +5187,14 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 							 to_intel_plane(primary));
 		struct intel_plane_state *old_primary_state =
 			to_intel_plane_state(old_pri_state);
+		struct drm_framebuffer *fb = primary_state->base.fb;
+
+		/* Display WA 827 */
+		if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
+		    IS_CANNONLAKE(dev_priv)) {
+			if (fb && fb->format->format == DRM_FORMAT_NV12)
+				skl_wa_clkgate(dev_priv, crtc->pipe, true);
+		}
 
 		intel_fbc_pre_update(crtc, pipe_config, primary_state);
 		/*
-- 
2.7.4

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

[PATCH v19 11/18] drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg

[Vidya Srinivas]

If the fb format is YUV, enable the plane CSC mode bits
for the conversion.

v2: Addressed review comments from Shashank Sharma
Alignment issue fixed in i915_reg.h

v3: Adding Reviewed By from Shashank Sharma

v4: Rebased the patch. As part of rebasing, re-using
the color series defines which are already merged.
plane_state->base.color_encoding might not be set for
NV12. For now, just using PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709
in glk_plane_color_ctl if format is NV12.

v5: Added reviewed by from Juha-Pekka Heikkila

v6: Rebased the series

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b27e0b6..951583b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3630,6 +3630,11 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
 
 	if (intel_format_is_yuv(fb->format->format)) {
+		if (fb->format->format == DRM_FORMAT_NV12) {
+			plane_color_ctl |=
+				PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
+			goto out;
+		}
 		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
 			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
 		else
@@ -3638,7 +3643,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 		if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
 			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
 	}
-
+out:
 	return plane_color_ctl;
 }
 
-- 
2.7.4

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

[PATCH v19 12/18] drm/i915: Set scaler mode for NV12

[Vidya Srinivas]

From: Chandra Konduru <chandra.konduru@intel.com>

This patch sets appropriate scaler mode for NV12 format.
In this mode, skylake scaler does either chroma-upsampling or
chroma-upsampling and resolution scaling

v2: Review comments from Ville addressed
NV12 case to be checked first for setting
the scaler

v3: Rebased (me)

v4: Rebased (me)

v5: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.

v6: Rebased (me)

v7: Rebased (me)

v8: Rebased (me)
Restricting the NV12 change for scaler to BXT and KBL
in this series.

v9: Rebased (me)

v10: As of now, NV12 has been tested on Gen9 and Gen10. However,
code is applicable to all GEN >= 9. Hence making
that change to keep it generic.
Comments under v8 is not valid anymore.

v11: Addressed review comments by Shashank Sharma.
For Gen10+, the scaler mode to be set it planar or normal
(single bit). Changed the code to be applicable to all
Gen.

v12: Addressed review comments from Shashank Sharma
For Gen9 (apart from GLK) bits 28:29 to be programmed
in PS_CTRL for NV12. For GLK and Gen10+, bit 29 to be set
for all Planar.

v13: Addressed review comments from Juha-Pekka Heikkila
"NV12 not to be supported by SKL"
Adding Reviewed by tag from Shashank Shamr

v14: Added reviewed by from Juha-Pekka Heikkila

v15: Rebased the series

Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |  2 ++
 drivers/gpu/drm/i915/intel_atomic.c | 14 ++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2afa96b..42f72a0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6652,6 +6652,8 @@ enum {
 #define PS_SCALER_MODE_MASK (3 << 28)
 #define PS_SCALER_MODE_DYN  (0 << 28)
 #define PS_SCALER_MODE_HQ  (1 << 28)
+#define SKL_PS_SCALER_MODE_NV12 (2 << 28)
+#define PS_SCALER_MODE_PLANAR (1 << 29)
 #define PS_PLANE_SEL_MASK  (7 << 25)
 #define PS_PLANE_SEL(plane) (((plane) + 1) << 25)
 #define PS_FILTER_MASK         (3 << 23)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index e9fb6920..bb8c168 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -328,8 +328,18 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
 		}
 
 		/* set scaler mode */
-		if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
-			scaler_state->scalers[*scaler_id].mode = 0;
+		if ((INTEL_GEN(dev_priv) >= 9) &&
+		    plane_state && plane_state->base.fb &&
+		    plane_state->base.fb->format->format ==
+		    DRM_FORMAT_NV12) {
+			if (INTEL_GEN(dev_priv) == 9 &&
+			    !IS_GEMINILAKE(dev_priv) &&
+			    !IS_SKYLAKE(dev_priv))
+				scaler_state->scalers[*scaler_id].mode =
+					SKL_PS_SCALER_MODE_NV12;
+			else
+				scaler_state->scalers[*scaler_id].mode =
+					PS_SCALER_MODE_PLANAR;
 		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
 			/*
 			 * when only 1 scaler is in use on either pipe A or B,
-- 
2.7.4

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

[PATCH v19 13/18] drm/i915: Update format_is_yuv() to include NV12

[Vidya Srinivas]

From: Chandra Konduru <chandra.konduru@intel.com>

This patch adds NV12 to format_is_yuv() function
for sprite planes.

v2:
-Use intel_ prefix for format_is_yuv (Ville)

v3: Rebased (me)

v4: Rebased and addressed review comments from Clinton A Taylor.
"static function in intel_sprite.c is not available
to the primary plane functions".
Changed commit message - function modified for
sprite planes.

v5: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.

v6: Rebased (me)

v7: Rebased (me)

v8: Rebased (me)

v9: Rebased (me)

v10: Changed intel_format_is_yuv function from
static to non-static. We need to use it later from
other files for check.

v11: Rebased the patch. format_is_yuv has already
been renamed to intel_format_is_yuv in the color
patch series which is already merged. This function
which was previously static has already been made
non-static. So this patch after rebase just adds
NV12 to intel_format_is_yuv function.

v12: Added reviewed by from Juha-Pekka Heikkila

v13/v14/v15: Rebased the series

Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    | 1 +
 drivers/gpu/drm/i915/intel_sprite.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 272c091..de7ddb1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2054,6 +2054,7 @@ void skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc);
 bool skl_plane_get_hw_state(struct intel_plane *plane);
 bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
 		       enum pipe pipe, enum plane_id plane_id);
+bool intel_format_is_yuv(uint32_t format);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index dbdcf85..0652e58 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -48,6 +48,7 @@ bool intel_format_is_yuv(u32 format)
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
 	case DRM_FORMAT_YVYU:
+	case DRM_FORMAT_NV12:
 		return true;
 	default:
 		return false;
-- 
2.7.4

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

[PATCH v19 14/18] drm/i915: Upscale scaler max scale for NV12

[Vidya Srinivas]

From: Chandra Konduru <chandra.konduru@intel.com>

This patch updates scaler max limit support for NV12

v2: Rebased (me)

v3: Rebased (me)

v4: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.

v5: Addressed review comments from Ville and rebased
- calculation of max_scale to be made
less convoluted by splitting it up a bit
- Indentation errors to be fixed in the series

v6: Rebased (me)
Fixed review comments from Paauwe, Bob J
Previous version, where a split of calculation
was done, was wrong. Fixed that issue here.

v7: Rebased (me)

v8: Rebased (me)

v9: Rebased (me)

v10: Rebased (me)

v11: Addressed review comments from Shashank Sharma
Alignment issues fixed.
When call to skl_update_scaler is made, 0 was being
sent instead of pixel_format.
When crtc update scaler is called, we dont have the
fb to derive the pixel format. Added the function
parameter bool plane_scaler_check to account for this.

v12: Fixed failure in IGT debugfs_test.
fb is NULL in skl_update_scaler_plane
Due to this, accessing fb->format caused failure.
Patch checks fb before using.

v13: In the previous version there was a flaw.
In skl_update_scaler during plane_scaler_check
if the format was non-NV12, it would set need_scaling
to false. This could reset the previously set need_scaling
from a previous condition check. Patch fixes this.
Patch also adds minimum src height for YUV 420 formats
to 16 (as defined in BSpec) and adds for checking this
range.

v14: Addressed review comments from Maarten
Just add a check for NV12 min src height in
skl_update_scaler and retain the remaining checks
as is. Added Reviewed By from Juha-Pekka Heikkila.

v15: Rebased the series.

v16: Changed fb height restriction to be >= 16 as per
Bspec. Earlier it was > 16.

v17: Changed NV12 scaler out of range message debug
level from DRM_DEBUG_KMS to DRM_ERROR as commit fails
if it hits the condition.

v18: Restricting the src width and height to be multiplier
of 4 for NV12 in skl_update_scaler

Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h     |  5 +++-
 drivers/gpu/drm/i915/intel_sprite.c  |  6 ++++-
 3 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 951583b..f8fea3d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3483,6 +3483,8 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
 		return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_UYVY;
 	case DRM_FORMAT_VYUY:
 		return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
+	case DRM_FORMAT_NV12:
+		return PLANE_CTL_FORMAT_NV12;
 	default:
 		MISSING_CASE(pixel_format);
 	}
@@ -4727,7 +4729,9 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		  unsigned int scaler_user, int *scaler_id,
-		  int src_w, int src_h, int dst_w, int dst_h)
+		  int src_w, int src_h, int dst_w, int dst_h,
+		  bool plane_scaler_check,
+		  uint32_t pixel_format)
 {
 	struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
@@ -4745,6 +4749,10 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	 */
 	need_scaling = src_w != dst_w || src_h != dst_h;
 
+	if (plane_scaler_check)
+		if (pixel_format == DRM_FORMAT_NV12)
+			need_scaling = true;
+
 	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
 		need_scaling = true;
 
@@ -4784,6 +4792,14 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		return 0;
 	}
 
+	if (plane_scaler_check && pixel_format == DRM_FORMAT_NV12 &&
+	    (src_h < SKL_MIN_YUV_420_SRC_H || (src_h % 4) != 0 ||
+	     (src_w % 4) != 0)) {
+		DRM_DEBUG_KMS("NV12 %dx%d: src dimensions not valid\n",
+			      src_w, src_h);
+		return -EINVAL;
+	}
+
 	/* range checks */
 	if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
 	    dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
@@ -4823,9 +4839,10 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
 	const struct drm_display_mode *adjusted_mode = &state->base.adjusted_mode;
 
 	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
-		&state->scaler_state.scaler_id,
-		state->pipe_src_w, state->pipe_src_h,
-		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
+				 &state->scaler_state.scaler_id,
+				 state->pipe_src_w, state->pipe_src_h,
+				 adjusted_mode->crtc_hdisplay,
+				 adjusted_mode->crtc_vdisplay, false, 0);
 }
 
 /**
@@ -4854,7 +4871,8 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 				drm_rect_width(&plane_state->base.src) >> 16,
 				drm_rect_height(&plane_state->base.src) >> 16,
 				drm_rect_width(&plane_state->base.dst),
-				drm_rect_height(&plane_state->base.dst));
+				drm_rect_height(&plane_state->base.dst),
+				fb ? true : false, fb ? fb->format->format : 0);
 
 	if (ret || plane_state->scaler_id < 0)
 		return ret;
@@ -4880,6 +4898,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_NV12:
 		break;
 	default:
 		DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
@@ -12882,11 +12901,13 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 }
 
 int
-skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
+skl_max_scale(struct intel_crtc *intel_crtc,
+	      struct intel_crtc_state *crtc_state,
+	      uint32_t pixel_format)
 {
 	struct drm_i915_private *dev_priv;
-	int max_scale;
-	int crtc_clock, max_dotclk;
+	int max_scale, mult;
+	int crtc_clock, max_dotclk, tmpclk1, tmpclk2;
 
 	if (!intel_crtc || !crtc_state->base.enable)
 		return DRM_PLANE_HELPER_NO_SCALING;
@@ -12908,8 +12929,10 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
 	 *            or
 	 *    cdclk/crtc_clock
 	 */
-	max_scale = min((1 << 16) * 3 - 1,
-			(1 << 8) * ((max_dotclk << 8) / crtc_clock));
+	mult = pixel_format == DRM_FORMAT_NV12 ? 2 : 3;
+	tmpclk1 = (1 << 16) * mult - 1;
+	tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock);
+	max_scale = min(tmpclk1, tmpclk2);
 
 	return max_scale;
 }
@@ -12925,12 +12948,16 @@ intel_check_primary_plane(struct intel_plane *plane,
 	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
 	bool can_position = false;
 	int ret;
+	uint32_t pixel_format = 0;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
 		/* use scaler when colorkey is not required */
 		if (!state->ckey.flags) {
 			min_scale = 1;
-			max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state);
+			if (state->base.fb)
+				pixel_format = state->base.fb->format->format;
+			max_scale = skl_max_scale(to_intel_crtc(crtc),
+						  crtc_state, pixel_format);
 		}
 		can_position = true;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index de7ddb1..66d0fe4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -552,6 +552,8 @@ struct intel_initial_plane_config {
 #define ICL_MAX_SRC_H 4096
 #define ICL_MAX_DST_W 5120
 #define ICL_MAX_DST_H 4096
+#define SKL_MIN_YUV_420_SRC_W 16
+#define SKL_MIN_YUV_420_SRC_H 16
 
 struct intel_scaler {
 	int in_use;
@@ -1596,7 +1598,8 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
-int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
+int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
+		  uint32_t pixel_format);
 
 static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
 {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0652e58..aa1dfaa 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -947,6 +947,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	int max_scale, min_scale;
 	bool can_scale;
 	int ret;
+	uint32_t pixel_format = 0;
 
 	*src = drm_plane_state_src(&state->base);
 	*dst = drm_plane_state_dest(&state->base);
@@ -970,11 +971,14 @@ intel_check_sprite_plane(struct intel_plane *plane,
 
 	/* setup can_scale, min_scale, max_scale */
 	if (INTEL_GEN(dev_priv) >= 9) {
+		if (state->base.fb)
+			pixel_format = state->base.fb->format->format;
 		/* use scaler when colorkey is not required */
 		if (!state->ckey.flags) {
 			can_scale = 1;
 			min_scale = 1;
-			max_scale = skl_max_scale(crtc, crtc_state);
+			max_scale =
+				skl_max_scale(crtc, crtc_state, pixel_format);
 		} else {
 			can_scale = 0;
 			min_scale = DRM_PLANE_HELPER_NO_SCALING;
-- 
2.7.4

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

[PATCH v19 15/18] drm/i915: Add NV12 as supported format for primary plane

[Vidya Srinivas]

From: Chandra Konduru <chandra.konduru@intel.com>

This patch adds NV12 to list of supported formats for
primary plane

v2: Rebased (Chandra Konduru)

v3: Rebased (me)

v4: Review comments by Ville addressed
Removed the skl_primary_formats_with_nv12 and
added NV12 case in existing skl_primary_formats

v5: Rebased (me)

v6: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.

v7: Review comments by Ville addressed
	Restricting the NV12 for BXT and on PIPE A and B
Rebased (me)

v8: Rebased (me)
Modified restricting the NV12 support for both BXT and KBL.

v9: Rebased (me)

v10: Addressed review comments from Maarten.
	Adding NV12 inside skl_primary_formats itself.

v11: Adding Reviewed By tag from Shashank Sharma

v12: Addressed review comments from Juha-Pekka Heikkila
"NV12 not to be supported by SKL"

v13: Addressed review comments from Ville
Added skl_pri_planar_formats to include NV12
and skl_plane_has_planar function to check for
NV12 support on plane. Added NV12 format to
skl_mod_supported. These were review comments
from Kristian Høgsberg <hoegsberg@gmail.com>

v14: Added reviewed by from Juha-Pekka Heikkila

v15: Rebased the series

Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 50 ++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f8fea3d..6941b30 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -88,6 +88,22 @@ static const uint32_t skl_primary_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
+static const uint32_t skl_pri_planar_formats[] = {
+	DRM_FORMAT_C8,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
+};
+
 static const uint64_t skl_format_modifiers_noccs[] = {
 	I915_FORMAT_MOD_Yf_TILED,
 	I915_FORMAT_MOD_Y_TILED,
@@ -13118,6 +13134,7 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_NV12:
 		if (modifier == I915_FORMAT_MOD_Yf_TILED)
 			return true;
 		/* fall through */
@@ -13325,6 +13342,30 @@ static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
 	return pipe == PIPE_A && plane_id == PLANE_PRIMARY;
 }
 
+bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
+			  enum pipe pipe, enum plane_id plane_id)
+{
+	if (plane_id == PLANE_PRIMARY) {
+		if (IS_SKYLAKE(dev_priv))
+			return false;
+		else if ((INTEL_GEN(dev_priv) == 9 && pipe == PIPE_C) &&
+			 !IS_GEMINILAKE(dev_priv))
+			return false;
+	} else if (plane_id >= PLANE_SPRITE0) {
+		if (plane_id == PLANE_CURSOR)
+			return false;
+		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) == 10) {
+			if (plane_id != PLANE_SPRITE0)
+				return false;
+		} else {
+			if (plane_id != PLANE_SPRITE0 || pipe == PIPE_C ||
+			    IS_SKYLAKE(dev_priv))
+				return false;
+		}
+	}
+	return true;
+}
+
 static struct intel_plane *
 intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
@@ -13385,8 +13426,13 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	primary->check_plane = intel_check_primary_plane;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
-		intel_primary_formats = skl_primary_formats;
-		num_formats = ARRAY_SIZE(skl_primary_formats);
+		if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY)) {
+			intel_primary_formats = skl_pri_planar_formats;
+			num_formats = ARRAY_SIZE(skl_pri_planar_formats);
+		} else {
+			intel_primary_formats = skl_primary_formats;
+			num_formats = ARRAY_SIZE(skl_primary_formats);
+		}
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
 			modifiers = skl_format_modifiers_ccs;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 66d0fe4..9c58da0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2058,6 +2058,8 @@ bool skl_plane_get_hw_state(struct intel_plane *plane);
 bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
 		       enum pipe pipe, enum plane_id plane_id);
 bool intel_format_is_yuv(uint32_t format);
+bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
+			  enum pipe pipe, enum plane_id plane_id);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_i915_private *dev_priv);
-- 
2.7.4

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

[PATCH v19 16/18] drm/i915: Add NV12 as supported format for sprite plane

[Vidya Srinivas]

From: Chandra Konduru <chandra.konduru@intel.com>

This patch adds NV12 to list of supported formats for sprite plane.

v2: Rebased (me)

v3: Review comments by Ville addressed
- Removed skl_plane_formats_with_nv12 and added
NV12 case in existing skl_plane_formats
- Added the 10bpc RGB formats

v4: Addressed review comments from Clinton A Taylor
"Why are we adding 10 bit RGB formats with the NV12 series patches?
Trying to set XR30 or AB30 results in error returned even though
the modes are advertised for the planes"
- Removed 10bit RGB formats added previously with NV12 series

v5: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.
Addressed review comments from Clinton A Taylor
"Why are we adding 10 bit RGB formats with the NV12 series patches?
Trying to set XR30 or AB30 results in error returned even though
the modes are advertised for the planes"
- Previous version has 10bit RGB format removed from VLV formats
by mistake. Fixing that in this version.
Removed 10bit RGB formats added previously with NV12 series
for SKL.

v6: Addressed review comments by Ville
Restricting the NV12 to BXT and PIPE A and B

v7: Rebased (me)

v8: Rebased (me)
Restricting NV12 changes to BXT and KBL
Restricting NV12 changes for plane 0 (overlay)

v9: Rebased (me)

v10: Addressed review comments from Maarten.
Adding NV12 to skl_plane_formats itself.

v11: Addressed review comments from Shashank Sharma

v12: Addressed review comments from Shashank Sharma
Made the condition in intel_sprite_plane_create
simple and easy to read as suggested.

v13: Adding reviewed by tag from Shashank Sharma
Addressed review comments from Juha-Pekka Heikkila
"NV12 not to be supported by SKL"

v14: Addressed review comments from Ville
Added skl_planar_formats to include NV12
and a check skl_plane_has_planar in sprite create
Added NV12 format to skl_mod_supported. These were
review comments from Kristian Høgsberg <hoegsberg@gmail.com>

v15: Added reviewed by from Juha-Pekka Heikkila

v16: Rebased the series

Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index aa1dfaa..d5dad44 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1253,6 +1253,19 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
+static uint32_t skl_planar_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
+};
+
 static const uint64_t skl_plane_format_modifiers_noccs[] = {
 	I915_FORMAT_MOD_Yf_TILED,
 	I915_FORMAT_MOD_Y_TILED,
@@ -1347,6 +1360,7 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_NV12:
 		if (modifier == I915_FORMAT_MOD_Yf_TILED)
 			return true;
 		/* fall through */
@@ -1446,8 +1460,14 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 		intel_plane->disable_plane = skl_disable_plane;
 		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
-		plane_formats = skl_plane_formats;
-		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		if (skl_plane_has_planar(dev_priv, pipe,
+					 PLANE_SPRITE0 + plane)) {
+			plane_formats = skl_planar_formats;
+			num_plane_formats = ARRAY_SIZE(skl_planar_formats);
+		} else {
+			plane_formats = skl_plane_formats;
+			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		}
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
 			modifiers = skl_plane_format_modifiers_ccs;
-- 
2.7.4

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

[PATCH v19 17/18] drm/i915: Add NV12 support to intel_framebuffer_init

[Vidya Srinivas]

From: Chandra Konduru <chandra.konduru@intel.com>

This patch adds NV12 as supported format
to intel_framebuffer_init and performs various checks.

v2:
-Fix an issue in checks added (Chandra Konduru)

v3: rebased (me)

v4: Review comments by Ville addressed
Added platform check for NV12 in intel_framebuffer_init
Removed offset checks for NV12 case

v5: Addressed review comments by Clinton A Taylor
This NV12 support only correctly works on SKL.
Plane color space conversion is different on GLK and later platforms
causing the colors to display incorrectly.
Ville's plane color space property patch series
in review will fix this issue.
- Restricted the NV12 case in intel_framebuffer_init to
SKL and BXT only.

v6: Rebased (me)

v7: Addressed review comments by Ville
Restricting the NV12 to BXT for now.

v8: Rebased (me)
Restricting the NV12 changes to BXT and KBL for now.

v9: Rebased (me)

v10: NV12 supported by all GEN >= 9.
Making this change in intel_framebuffer_init. This is
part of addressing Maarten's review comments.
Comment under v8 no longer applicable

v11: Addressed review comments from Shashank Sharma

v12: Adding Reviewed By from Shashank Sharma

v13: Addressed review comments from Juha-Pekka Heikkila
"NV12 not to be supported by SKL"

v14: Addressed review comments from Maarten.
Add checks for fb width height for NV12 and fail the fb
creation if check fails. Added reviewed by from
Juha-Pekka Heikkila

v15: Rebased the series

v16: Setting the minimum value during fb creating to 16
as per Bspec for NV12. Earlier minimum was expected
to be > 16. Now changed it to >=16.

v17: Restricting fb source width and height to multiplier of
4 in intel_framebuffer_init

Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6941b30..9f7babf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14253,6 +14253,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 			goto err;
 		}
 		break;
+	case DRM_FORMAT_NV12:
+		if (INTEL_GEN(dev_priv) < 9 || IS_SKYLAKE(dev_priv)) {
+			DRM_DEBUG_KMS("unsupported pixel format: %s\n",
+				      drm_get_format_name(mode_cmd->pixel_format,
+							  &format_name));
+			goto err;
+		}
+		break;
 	default:
 		DRM_DEBUG_KMS("unsupported pixel format: %s\n",
 			      drm_get_format_name(mode_cmd->pixel_format, &format_name));
@@ -14265,6 +14273,16 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 
 	drm_helper_mode_fill_fb_struct(&dev_priv->drm, fb, mode_cmd);
 
+	if (fb->format->format == DRM_FORMAT_NV12 &&
+	    (fb->width < SKL_MIN_YUV_420_SRC_W ||
+	     fb->height < SKL_MIN_YUV_420_SRC_H ||
+	     (fb->width % 4) != 0 ||
+	     (fb->height % 4) != 0)) {
+		DRM_DEBUG_KMS("src fb %d x %d: dimensions not "
+			      "met for planar format\n", fb->width, fb->height);
+		return -EINVAL;
+	}
+
 	for (i = 0; i < fb->format->num_planes; i++) {
 		u32 stride_alignment;
 
-- 
2.7.4

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

[PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Vidya Srinivas]

As per display WA 1106, to avoid corruption issues
NV12 plane height needs to be multiplier of 4
We expect the src dimensions to be multiplier of 4
We fail the case where src width or height is not
multiple of 4 for NV12.
We also set the scaler destination height
and width to be multiple of 4. Without this, pipe
fifo underruns were seen on APL and KBL. We also
skip src trunction/adjustments for NV12 case and handle
the sizes directly in skl_update_plane

Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  2 ++
 drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9c58da0..a1f718d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -159,6 +159,8 @@
 #define INTEL_I2C_BUS_DVO 1
 #define INTEL_I2C_BUS_SDVO 2
 
+#define MULT4(x) ((x + 3) & ~0x03)
+
 /* these are outputs from the chip - integrated only
    external chips are via DVO or SDVO output */
 enum intel_output_type {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index d5dad44..b2292dd 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
 
+	if (fb->format->format == DRM_FORMAT_NV12 &&
+		((src_h % 4) != 0 || (src_w % 4) != 0)) {
+		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
+		return;
+	}
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
 			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode);
 		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
 		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
-		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
-			      ((crtc_w + 1) << 16)|(crtc_h + 1));
-
+		if (fb->format->format == DRM_FORMAT_NV12)
+			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
+				      (MULT4(crtc_w) << 16) | MULT4(crtc_h));
+		else
+			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
+				      ((crtc_w + 1) << 16)|(crtc_h + 1));
 		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
 	} else {
 		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x);
@@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane *plane,
 		return -EINVAL;
 	}
 
+	if (fb->format->format == DRM_FORMAT_NV12)
+		goto check_plane_surface;
+
 	/* setup can_scale, min_scale, max_scale */
 	if (INTEL_GEN(dev_priv) >= 9) {
 		if (state->base.fb)
@@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	dst->y1 = crtc_y;
 	dst->y2 = crtc_y + crtc_h;
 
+check_plane_surface:
 	if (INTEL_GEN(dev_priv) >= 9) {
 		ret = skl_check_plane_surface(crtc_state, state);
 		if (ret)
-- 
2.7.4

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

✗ Fi.CI.CHECKPATCH: warning for Add NV12 support (rev7)

[Patchwork]

== Series Details ==

Series: Add NV12 support (rev7)
URL   : https://patchwork.freedesktop.org/series/39670/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b485c8e3c47f drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values
aba1b53bed73 drm/i915/skl+: refactor WM calculation for NV12
a296d167a4a6 drm/i915/skl+: add NV12 in skl_format_to_fourcc
12f80e1cff7f drm/i915/skl+: support verification of DDB HW state for NV12
bacb78f9fe3d drm/i915/skl+: NV12 related changes for WM
5c8d3c51f18f drm/i915/skl+: pass skl_wm_level struct to wm compute func
3bd8a18d51f6 drm/i915/skl+: make sure higher latency level has higher wm value
887161fdb42a drm/i915/skl+: nv12 workaround disable WM level 1-7
f906d62dcfc8 drm/i915/skl: split skl_compute_ddb function
21067380dced drm/i915: Display WA 827
0ab2b5a2cfdf drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg
69aabaf21998 drm/i915: Set scaler mode for NV12
b07ed5e02a2f drm/i915: Update format_is_yuv() to include NV12
09621b20dba4 drm/i915: Upscale scaler max scale for NV12
4585567b4b39 drm/i915: Add NV12 as supported format for primary plane
837c86649a3f drm/i915: Add NV12 as supported format for sprite plane
2a6f6cd9bf0a drm/i915: Add NV12 support to intel_framebuffer_init
b1b23aa710c9 drm/i915: Set src size restrictions for NV12
-:42: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#42: FILE: drivers/gpu/drm/i915/intel_sprite.c:259:
+	if (fb->format->format == DRM_FORMAT_NV12 &&
+		((src_h % 4) != 0 || (src_w % 4) != 0)) {

-:62: CHECK:SPACING: spaces preferred around that '|' (ctx:VxV)
#62: FILE: drivers/gpu/drm/i915/intel_sprite.c:306:
+				      ((crtc_w + 1) << 16)|(crtc_h + 1));
 				                          ^

total: 0 errors, 0 warnings, 2 checks, 51 lines checked

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

✓ Fi.CI.BAT: success for Add NV12 support (rev7)

[Patchwork]

== Series Details ==

Series: Add NV12 support (rev7)
URL   : https://patchwork.freedesktop.org/series/39670/
State : success

== Summary ==

Series 39670v7 Add NV12 support
https://patchwork.freedesktop.org/api/1.0/series/39670/revisions/7/mbox/

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:432s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:439s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:379s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:535s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:295s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:512s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:514s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:519s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:506s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:408s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:559s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:514s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:588s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:422s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:317s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:549s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:407s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:423s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:468s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:431s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:474s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:464s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:656s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:440s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:530s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:506s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:429s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:447s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:594s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:403s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:255  dwarn:4   dfail:0   fail:0   skip:26  time:528s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:486s

c46052cde6a50c5459e00791ffc4d5aa1ec58a9e drm-tip: 2018y-03m-30d-18h-56m-26s UTC integration manifest
b1b23aa710c9 drm/i915: Set src size restrictions for NV12
2a6f6cd9bf0a drm/i915: Add NV12 support to intel_framebuffer_init
837c86649a3f drm/i915: Add NV12 as supported format for sprite plane
4585567b4b39 drm/i915: Add NV12 as supported format for primary plane
09621b20dba4 drm/i915: Upscale scaler max scale for NV12
b07ed5e02a2f drm/i915: Update format_is_yuv() to include NV12
69aabaf21998 drm/i915: Set scaler mode for NV12
0ab2b5a2cfdf drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg
21067380dced drm/i915: Display WA 827
f906d62dcfc8 drm/i915/skl: split skl_compute_ddb function
887161fdb42a drm/i915/skl+: nv12 workaround disable WM level 1-7
3bd8a18d51f6 drm/i915/skl+: make sure higher latency level has higher wm value
5c8d3c51f18f drm/i915/skl+: pass skl_wm_level struct to wm compute func
bacb78f9fe3d drm/i915/skl+: NV12 related changes for WM
12f80e1cff7f drm/i915/skl+: support verification of DDB HW state for NV12
a296d167a4a6 drm/i915/skl+: add NV12 in skl_format_to_fourcc
aba1b53bed73 drm/i915/skl+: refactor WM calculation for NV12
b485c8e3c47f drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values

== Logs ==

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

✗ Fi.CI.IGT: failure for Add NV12 support (rev7)

[Patchwork]

== Series Details ==

Series: Add NV12 support (rev7)
URL   : https://patchwork.freedesktop.org/series/39670/
State : failure

== Summary ==

---- Possible new issues:

Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                dmesg-warn -> PASS       (shard-snb)
Test kms_plane_scaling:
        Subgroup pipe-a-scaler-with-clipping-clamping:
                pass       -> FAIL       (shard-apl)
        Subgroup pipe-a-scaler-with-pixel-format:
                pass       -> FAIL       (shard-apl)
        Subgroup pipe-a-scaler-with-rotation:
                pass       -> FAIL       (shard-apl)
        Subgroup pipe-b-scaler-with-clipping-clamping:
                pass       -> FAIL       (shard-apl)
        Subgroup pipe-b-scaler-with-pixel-format:
                pass       -> FAIL       (shard-apl)
        Subgroup pipe-b-scaler-with-rotation:
                pass       -> FAIL       (shard-apl)

---- Known issues:

Test gem_softpin:
        Subgroup noreloc-s3:
                incomplete -> PASS       (shard-hsw) fdo#103540 +1
Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank:
                pass       -> FAIL       (shard-hsw) fdo#102887
        Subgroup 2x-plain-flip-ts-check-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-shrfb-msflip-blt:
                dmesg-fail -> PASS       (shard-apl) fdo#104727

fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#104727 https://bugs.freedesktop.org/show_bug.cgi?id=104727

shard-apl        total:3495 pass:1825 dwarn:1   dfail:0   fail:13  skip:1655 time:12839s
shard-hsw        total:3430 pass:1749 dwarn:1   dfail:0   fail:3   skip:1675 time:11177s
shard-snb        total:3495 pass:1374 dwarn:1   dfail:0   fail:3   skip:2117 time:7042s
Blacklisted hosts:
shard-kbl        total:3495 pass:1951 dwarn:1   dfail:0   fail:15  skip:1528 time:9270s

== Logs ==

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

✗ Fi.CI.CHECKPATCH: warning for Add NV12 support (rev7)

[Patchwork]

== Series Details ==

Series: Add NV12 support (rev7)
URL   : https://patchwork.freedesktop.org/series/39670/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
028f935bfc8a drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values
c37c92d65c7c drm/i915/skl+: refactor WM calculation for NV12
79337fec4fab drm/i915/skl+: add NV12 in skl_format_to_fourcc
5231d5353cb2 drm/i915/skl+: support verification of DDB HW state for NV12
1b72ea89ace4 drm/i915/skl+: NV12 related changes for WM
dc4923fc1335 drm/i915/skl+: pass skl_wm_level struct to wm compute func
170d8caed005 drm/i915/skl+: make sure higher latency level has higher wm value
dfd59e90ed22 drm/i915/skl+: nv12 workaround disable WM level 1-7
cf28f30661e4 drm/i915/skl: split skl_compute_ddb function
a1a101fb2d41 drm/i915: Display WA 827
c6b5e1ee2b7e drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg
fe8faacef411 drm/i915: Set scaler mode for NV12
351c24e47014 drm/i915: Update format_is_yuv() to include NV12
b364afbfa912 drm/i915: Upscale scaler max scale for NV12
b14499e2f17d drm/i915: Add NV12 as supported format for primary plane
f92ae4d22dbe drm/i915: Add NV12 as supported format for sprite plane
12dfb0c771d7 drm/i915: Add NV12 support to intel_framebuffer_init
d72b38f1f959 drm/i915: Set src size restrictions for NV12
-:42: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#42: FILE: drivers/gpu/drm/i915/intel_sprite.c:259:
+	if (fb->format->format == DRM_FORMAT_NV12 &&
+		((src_h % 4) != 0 || (src_w % 4) != 0)) {

-:62: CHECK:SPACING: spaces preferred around that '|' (ctx:VxV)
#62: FILE: drivers/gpu/drm/i915/intel_sprite.c:306:
+				      ((crtc_w + 1) << 16)|(crtc_h + 1));
 				                          ^

total: 0 errors, 0 warnings, 2 checks, 51 lines checked

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

✓ Fi.CI.BAT: success for Add NV12 support (rev7)

[Patchwork]

== Series Details ==

Series: Add NV12 support (rev7)
URL   : https://patchwork.freedesktop.org/series/39670/
State : success

== Summary ==

Series 39670v7 Add NV12 support
https://patchwork.freedesktop.org/api/1.0/series/39670/revisions/7/mbox/

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:431s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:440s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:543s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:513s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:520s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:508s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:416s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:559s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:510s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:587s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:424s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:314s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:536s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:404s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:422s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:470s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:429s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:473s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:509s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:665s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:445s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:532s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:502s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:518s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:428s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:445s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:580s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:401s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:513s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:483s
fi-bxt-dsi failed to connect after reboot

9a423e97b1f6e080f29d0f7f69806b67542de83b drm-tip: 2018y-04m-03d-11h-50m-53s UTC integration manifest
d72b38f1f959 drm/i915: Set src size restrictions for NV12
12dfb0c771d7 drm/i915: Add NV12 support to intel_framebuffer_init
f92ae4d22dbe drm/i915: Add NV12 as supported format for sprite plane
b14499e2f17d drm/i915: Add NV12 as supported format for primary plane
b364afbfa912 drm/i915: Upscale scaler max scale for NV12
351c24e47014 drm/i915: Update format_is_yuv() to include NV12
fe8faacef411 drm/i915: Set scaler mode for NV12
c6b5e1ee2b7e drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg
a1a101fb2d41 drm/i915: Display WA 827
cf28f30661e4 drm/i915/skl: split skl_compute_ddb function
dfd59e90ed22 drm/i915/skl+: nv12 workaround disable WM level 1-7
170d8caed005 drm/i915/skl+: make sure higher latency level has higher wm value
dc4923fc1335 drm/i915/skl+: pass skl_wm_level struct to wm compute func
1b72ea89ace4 drm/i915/skl+: NV12 related changes for WM
5231d5353cb2 drm/i915/skl+: support verification of DDB HW state for NV12
79337fec4fab drm/i915/skl+: add NV12 in skl_format_to_fourcc
c37c92d65c7c drm/i915/skl+: refactor WM calculation for NV12
028f935bfc8a drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values

== Logs ==

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

✗ Fi.CI.IGT: failure for Add NV12 support (rev7)

[Patchwork]

== Series Details ==

Series: Add NV12 support (rev7)
URL   : https://patchwork.freedesktop.org/series/39670/
State : failure

== Summary ==

---- Possible new issues:

Test gem_linear_blits:
        Subgroup normal:
                pass       -> INCOMPLETE (shard-hsw)
Test kms_draw_crc:
        Subgroup draw-method-xrgb8888-mmap-wc-untiled:
                skip       -> PASS       (shard-snb)
Test kms_flip:
        Subgroup flip-vs-modeset-vs-hang:
                dmesg-warn -> PASS       (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-indfb-draw-mmap-cpu:
                skip       -> PASS       (shard-snb)
        Subgroup fbc-1p-primscrn-spr-indfb-move:
                skip       -> PASS       (shard-snb)
Test kms_plane_scaling:
        Subgroup pipe-a-scaler-with-clipping-clamping:
                pass       -> FAIL       (shard-apl)
        Subgroup pipe-b-scaler-with-clipping-clamping:
                pass       -> FAIL       (shard-apl)
Test prime_vgem:
        Subgroup basic-fence-flip:
                skip       -> PASS       (shard-snb)
Test syncobj_wait:
        Subgroup multi-wait-for-submit-unsubmitted-signaled:
                incomplete -> PASS       (shard-snb)

---- Known issues:

Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-toggle:
                fail       -> PASS       (shard-hsw) fdo#102670
Test kms_flip:
        Subgroup 2x-dpms-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-hsw) fdo#103060 +1
        Subgroup 2x-flip-vs-absolute-wf_vblank-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
        Subgroup flip-vs-expired-vblank-interruptible:
                fail       -> PASS       (shard-hsw) fdo#102887
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047
Test kms_vblank:
        Subgroup pipe-c-accuracy-idle:
                fail       -> PASS       (shard-hsw) fdo#102583

fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583

shard-apl        total:3496 pass:1830 dwarn:1   dfail:0   fail:9   skip:1655 time:12898s
shard-hsw        total:3480 pass:1774 dwarn:1   dfail:0   fail:4   skip:1699 time:10869s
shard-snb        total:3496 pass:1375 dwarn:1   dfail:0   fail:2   skip:2118 time:7026s
Blacklisted hosts:
shard-kbl        total:3496 pass:1931 dwarn:27  dfail:0   fail:9   skip:1529 time:9411s

== Logs ==

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

Re: ✗ Fi.CI.IGT: failure for Add NV12 support (rev7)

[Saarinen, Jani]

HI, 
Clear idea if caused by changes? 

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Patchwork
> Sent: tiistai 3. huhtikuuta 2018 18.06
> To: Srinivas, Vidya <vidya.srinivas@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] ✗ Fi.CI.IGT: failure for Add NV12 support (rev7)
> 
> == Series Details ==
> 
> Series: Add NV12 support (rev7)
> URL   : https://patchwork.freedesktop.org/series/39670/
> State : failure
> 
> == Summary ==
> 
> ---- Possible new issues:
> 
> Test gem_linear_blits:
>         Subgroup normal:
>                 pass       -> INCOMPLETE (shard-hsw)
> Test kms_draw_crc:
>         Subgroup draw-method-xrgb8888-mmap-wc-untiled:
>                 skip       -> PASS       (shard-snb)
> Test kms_flip:
>         Subgroup flip-vs-modeset-vs-hang:
>                 dmesg-warn -> PASS       (shard-hsw)
> Test kms_frontbuffer_tracking:
>         Subgroup fbc-1p-offscren-pri-indfb-draw-mmap-cpu:
>                 skip       -> PASS       (shard-snb)
>         Subgroup fbc-1p-primscrn-spr-indfb-move:
>                 skip       -> PASS       (shard-snb)
> Test kms_plane_scaling:
>         Subgroup pipe-a-scaler-with-clipping-clamping:
>                 pass       -> FAIL       (shard-apl)
>         Subgroup pipe-b-scaler-with-clipping-clamping:
>                 pass       -> FAIL       (shard-apl)
> Test prime_vgem:
>         Subgroup basic-fence-flip:
>                 skip       -> PASS       (shard-snb)
> Test syncobj_wait:
>         Subgroup multi-wait-for-submit-unsubmitted-signaled:
>                 incomplete -> PASS       (shard-snb)
> 
> ---- Known issues:
> 
> Test kms_cursor_legacy:
>         Subgroup flip-vs-cursor-toggle:
>                 fail       -> PASS       (shard-hsw) fdo#102670
> Test kms_flip:
>         Subgroup 2x-dpms-vs-vblank-race-interruptible:
>                 fail       -> PASS       (shard-hsw) fdo#103060 +1
>         Subgroup 2x-flip-vs-absolute-wf_vblank-interruptible:
>                 pass       -> FAIL       (shard-hsw) fdo#100368
>         Subgroup flip-vs-expired-vblank-interruptible:
>                 fail       -> PASS       (shard-hsw) fdo#102887
> Test kms_sysfs_edid_timing:
>                 pass       -> WARN       (shard-apl) fdo#100047
> Test kms_vblank:
>         Subgroup pipe-c-accuracy-idle:
>                 fail       -> PASS       (shard-hsw) fdo#102583
> 
> fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
> fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
> fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
> fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
> fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
> fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
> 
> shard-apl        total:3496 pass:1830 dwarn:1   dfail:0   fail:9   skip:1655
> time:12898s
> shard-hsw        total:3480 pass:1774 dwarn:1   dfail:0   fail:4   skip:1699
> time:10869s
> shard-snb        total:3496 pass:1375 dwarn:1   dfail:0   fail:2   skip:2118
> time:7026s
> Blacklisted hosts:
> shard-kbl        total:3496 pass:1931 dwarn:27  dfail:0   fail:9   skip:1529
> time:9411s
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-
> tip/Patchwork_8568/shards.html
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Maarten Lankhorst]

Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> As per display WA 1106, to avoid corruption issues
> NV12 plane height needs to be multiplier of 4
> We expect the src dimensions to be multiplier of 4
> We fail the case where src width or height is not
> multiple of 4 for NV12.
> We also set the scaler destination height
> and width to be multiple of 4. Without this, pipe
> fifo underruns were seen on APL and KBL. We also
> skip src trunction/adjustments for NV12 case and handle
> the sizes directly in skl_update_plane
>
> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c58da0..a1f718d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -159,6 +159,8 @@
>  #define INTEL_I2C_BUS_DVO 1
>  #define INTEL_I2C_BUS_SDVO 2
>  
> +#define MULT4(x) ((x + 3) & ~0x03)
> +
>  /* these are outputs from the chip - integrated only
>     external chips are via DVO or SDVO output */
>  enum intel_output_type {
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index d5dad44..b2292dd 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>  	unsigned long irqflags;
>  
> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> +		return;
> +	}
> +
You can't do this check in skl_update_plane, it's way too late. It should be rejected in the plane check with -EINVAL, or perhaps in
skl_update_scaler.
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode);
>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> -
> +		if (fb->format->format == DRM_FORMAT_NV12)
> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> +				      (MULT4(crtc_w) << 16) | MULT4(crtc_h));
See the comment above, sizes are 0 based. This will add 1 to the size, so the size is always 1 more than requested.
I don't think it would pass plane CRC tests..
> +		else
> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
>  	} else {
>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x);
> @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  		return -EINVAL;
>  	}
>  
> +	if (fb->format->format == DRM_FORMAT_NV12)
> +		goto check_plane_surface;
> +
>  	/* setup can_scale, min_scale, max_scale */
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		if (state->base.fb)
> @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  	dst->y1 = crtc_y;
>  	dst->y2 = crtc_y + crtc_h;
>  
> +check_plane_surface:
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		ret = skl_check_plane_surface(crtc_state, state);
>  		if (ret)


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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Maarten Lankhorst]

Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> As per display WA 1106, to avoid corruption issues
> NV12 plane height needs to be multiplier of 4
> We expect the src dimensions to be multiplier of 4
> We fail the case where src width or height is not
> multiple of 4 for NV12.
> We also set the scaler destination height
> and width to be multiple of 4. Without this, pipe
> fifo underruns were seen on APL and KBL. We also
> skip src trunction/adjustments for NV12 case and handle
> the sizes directly in skl_update_plane
>
> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c58da0..a1f718d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -159,6 +159,8 @@
>  #define INTEL_I2C_BUS_DVO 1
>  #define INTEL_I2C_BUS_SDVO 2
>  
> +#define MULT4(x) ((x + 3) & ~0x03)
> +
>  /* these are outputs from the chip - integrated only
>     external chips are via DVO or SDVO output */
>  enum intel_output_type {
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index d5dad44..b2292dd 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>  	unsigned long irqflags;
>  
> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> +		return;
> +	}
Also on a side note, don't add DRM_DEBUG_KMS in update_plane or disable_plane calls. It can take an arbitrary amount of time, resulting in an atomic update failure.
> +
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode);
>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> -
> +		if (fb->format->format == DRM_FORMAT_NV12)
> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> +				      (MULT4(crtc_w) << 16) | MULT4(crtc_h));
> +		else
> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
>  	} else {
>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x);
> @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  		return -EINVAL;
>  	}
>  
> +	if (fb->format->format == DRM_FORMAT_NV12)
> +		goto check_plane_surface;
> +
>  	/* setup can_scale, min_scale, max_scale */
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		if (state->base.fb)
> @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  	dst->y1 = crtc_y;
>  	dst->y2 = crtc_y + crtc_h;
>  
> +check_plane_surface:
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		ret = skl_check_plane_surface(crtc_state, state);
>  		if (ret)


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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Wednesday, April 4, 2018 1:28 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> > As per display WA 1106, to avoid corruption issues
> > NV12 plane height needs to be multiplier of 4 We expect the src
> > dimensions to be multiplier of 4 We fail the case where src width or
> > height is not multiple of 4 for NV12.
> > We also set the scaler destination height and width to be multiple of
> > 4. Without this, pipe fifo underruns were seen on APL and KBL. We also
> > skip src trunction/adjustments for NV12 case and handle the sizes
> > directly in skl_update_plane
> >
> > Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 9c58da0..a1f718d 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -159,6 +159,8 @@
> >  #define INTEL_I2C_BUS_DVO 1
> >  #define INTEL_I2C_BUS_SDVO 2
> >
> > +#define MULT4(x) ((x + 3) & ~0x03)
> > +
> >  /* these are outputs from the chip - integrated only
> >     external chips are via DVO or SDVO output */  enum
> > intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index d5dad44..b2292dd 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >  	unsigned long irqflags;
> >
> > +	if (fb->format->format == DRM_FORMAT_NV12 &&
> > +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> > +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> > +		return;
> > +	}
> > +
> You can't do this check in skl_update_plane, it's way too late. It should be
> rejected in the plane check with -EINVAL, or perhaps in skl_update_scaler.
Have done it right from intel_framebuffer_init onwards, have done it in skl_update_scaler also
In fact it will get rejected in framebuffer init and all these are duplicate checks, can remove them.
> >  	/* Sizes are 0 based */
> >  	src_w--;
> >  	src_h--;
> > @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
> >  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> scaler->mode);
> >  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> >  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> << 16) | crtc_y);
> > -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> > -
> > +		if (fb->format->format == DRM_FORMAT_NV12)
> > +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > +				      (MULT4(crtc_w) << 16) |
> MULT4(crtc_h));
> See the comment above, sizes are 0 based. This will add 1 to the size, so the
> size is always 1 more than requested.
> I don't think it would pass plane CRC tests..

When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920 back.
If we don’t do this, I see fifo underruns. The destination width and height
If not mult of 4, I am seeing underruns.

> > +		else
> > +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
> >  	} else {
> >  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16)
> | crtc_x);
> > @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >  		return -EINVAL;
> >  	}
> >
> > +	if (fb->format->format == DRM_FORMAT_NV12)
> > +		goto check_plane_surface;
> > +
> >  	/* setup can_scale, min_scale, max_scale */
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> >  		if (state->base.fb)
> > @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >  	dst->y1 = crtc_y;
> >  	dst->y2 = crtc_y + crtc_h;
> >
> > +check_plane_surface:
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> >  		ret = skl_check_plane_surface(crtc_state, state);
> >  		if (ret)
> 

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Wednesday, April 4, 2018 1:33 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> > As per display WA 1106, to avoid corruption issues
> > NV12 plane height needs to be multiplier of 4 We expect the src
> > dimensions to be multiplier of 4 We fail the case where src width or
> > height is not multiple of 4 for NV12.
> > We also set the scaler destination height and width to be multiple of
> > 4. Without this, pipe fifo underruns were seen on APL and KBL. We also
> > skip src trunction/adjustments for NV12 case and handle the sizes
> > directly in skl_update_plane
> >
> > Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 9c58da0..a1f718d 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -159,6 +159,8 @@
> >  #define INTEL_I2C_BUS_DVO 1
> >  #define INTEL_I2C_BUS_SDVO 2
> >
> > +#define MULT4(x) ((x + 3) & ~0x03)
> > +
> >  /* these are outputs from the chip - integrated only
> >     external chips are via DVO or SDVO output */  enum
> > intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index d5dad44..b2292dd 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >  	unsigned long irqflags;
> >
> > +	if (fb->format->format == DRM_FORMAT_NV12 &&
> > +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> > +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> > +		return;
> > +	}
> Also on a side note, don't add DRM_DEBUG_KMS in update_plane or
> disable_plane calls. It can take an arbitrary amount of time, resulting in an
> atomic update failure.
Sure, thank you. These are anyway duplicate checks. I will remove them.
The main one is done in intel_framebuffer_init as suggested by you.

> > +
> >  	/* Sizes are 0 based */
> >  	src_w--;
> >  	src_h--;
> > @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
> >  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> scaler->mode);
> >  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> >  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> << 16) | crtc_y);
> > -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> > -
> > +		if (fb->format->format == DRM_FORMAT_NV12)
> > +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > +				      (MULT4(crtc_w) << 16) |
> MULT4(crtc_h));
> > +		else
> > +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> > +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
> >  	} else {
> >  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16)
> | crtc_x);
> > @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >  		return -EINVAL;
> >  	}
> >
> > +	if (fb->format->format == DRM_FORMAT_NV12)
> > +		goto check_plane_surface;
> > +
> >  	/* setup can_scale, min_scale, max_scale */
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> >  		if (state->base.fb)
> > @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >  	dst->y1 = crtc_y;
> >  	dst->y2 = crtc_y + crtc_h;
> >
> > +check_plane_surface:
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> >  		ret = skl_check_plane_surface(crtc_state, state);
> >  		if (ret)
> 

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Maarten Lankhorst]

Op 04-04-18 om 10:06 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Wednesday, April 4, 2018 1:33 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
>> for NV12
>>
>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
>>> As per display WA 1106, to avoid corruption issues
>>> NV12 plane height needs to be multiplier of 4 We expect the src
>>> dimensions to be multiplier of 4 We fail the case where src width or
>>> height is not multiple of 4 for NV12.
>>> We also set the scaler destination height and width to be multiple of
>>> 4. Without this, pipe fifo underruns were seen on APL and KBL. We also
>>> skip src trunction/adjustments for NV12 case and handle the sizes
>>> directly in skl_update_plane
>>>
>>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
>>>  2 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>> b/drivers/gpu/drm/i915/intel_drv.h
>>> index 9c58da0..a1f718d 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -159,6 +159,8 @@
>>>  #define INTEL_I2C_BUS_DVO 1
>>>  #define INTEL_I2C_BUS_SDVO 2
>>>
>>> +#define MULT4(x) ((x + 3) & ~0x03)
>>> +
>>>  /* these are outputs from the chip - integrated only
>>>     external chips are via DVO or SDVO output */  enum
>>> intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>> index d5dad44..b2292dd 100644
>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>>>  	unsigned long irqflags;
>>>
>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
>>> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
>>> +		return;
>>> +	}
>> Also on a side note, don't add DRM_DEBUG_KMS in update_plane or
>> disable_plane calls. It can take an arbitrary amount of time, resulting in an
>> atomic update failure.
> Sure, thank you. These are anyway duplicate checks. I will remove them.
> The main one is done in intel_framebuffer_init as suggested by you.
They're not duplicate, they're done for related reasons:
1. It doesn't make sense to allow creation of a framebuffer with an invalid width/height if the full width/height cannot be used.
2. The checks in skl_update_scaler are the ones that will reject even showing a subset with invalid width, which can cause the fifo underruns.
>>> +
>>>  	/* Sizes are 0 based */
>>>  	src_w--;
>>>  	src_h--;
>>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
>>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
>> scaler->mode);
>>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
>> << 16) | crtc_y);
>>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
>>> -
>>> +		if (fb->format->format == DRM_FORMAT_NV12)
>>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>> +				      (MULT4(crtc_w) << 16) |
>> MULT4(crtc_h));
>>> +		else
>>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>> +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
>>>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
>>>  	} else {
>>>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16)
>> | crtc_x);
>>> @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane
>> *plane,
>>>  		return -EINVAL;
>>>  	}
>>>
>>> +	if (fb->format->format == DRM_FORMAT_NV12)
>>> +		goto check_plane_surface;
>>> +
>>>  	/* setup can_scale, min_scale, max_scale */
>>>  	if (INTEL_GEN(dev_priv) >= 9) {
>>>  		if (state->base.fb)
>>> @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane
>> *plane,
>>>  	dst->y1 = crtc_y;
>>>  	dst->y2 = crtc_y + crtc_h;
>>>
>>> +check_plane_surface:
>>>  	if (INTEL_GEN(dev_priv) >= 9) {
>>>  		ret = skl_check_plane_surface(crtc_state, state);
>>>  		if (ret)


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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Wednesday, April 4, 2018 1:40 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 04-04-18 om 10:06 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Wednesday, April 4, 2018 1:33 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >> restrictions for NV12
> >>
> >> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> >>> As per display WA 1106, to avoid corruption issues
> >>> NV12 plane height needs to be multiplier of 4 We expect the src
> >>> dimensions to be multiplier of 4 We fail the case where src width or
> >>> height is not multiple of 4 for NV12.
> >>> We also set the scaler destination height and width to be multiple
> >>> of 4. Without this, pipe fifo underruns were seen on APL and KBL. We
> >>> also skip src trunction/adjustments for NV12 case and handle the
> >>> sizes directly in skl_update_plane
> >>>
> >>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >>>  2 files changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >>> b/drivers/gpu/drm/i915/intel_drv.h
> >>> index 9c58da0..a1f718d 100644
> >>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>> @@ -159,6 +159,8 @@
> >>>  #define INTEL_I2C_BUS_DVO 1
> >>>  #define INTEL_I2C_BUS_SDVO 2
> >>>
> >>> +#define MULT4(x) ((x + 3) & ~0x03)
> >>> +
> >>>  /* these are outputs from the chip - integrated only
> >>>     external chips are via DVO or SDVO output */  enum
> >>> intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>> index d5dad44..b2292dd 100644
> >>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >>>  	unsigned long irqflags;
> >>>
> >>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> >>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> >>> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> >>> +		return;
> >>> +	}
> >> Also on a side note, don't add DRM_DEBUG_KMS in update_plane or
> >> disable_plane calls. It can take an arbitrary amount of time,
> >> resulting in an atomic update failure.
> > Sure, thank you. These are anyway duplicate checks. I will remove them.
> > The main one is done in intel_framebuffer_init as suggested by you.
> They're not duplicate, they're done for related reasons:
> 1. It doesn't make sense to allow creation of a framebuffer with an invalid
> width/height if the full width/height cannot be used.
> 2. The checks in skl_update_scaler are the ones that will reject even showing
> a subset with invalid width, which can cause the fifo underruns.

Sorry, yeah that’s correct. I will remove it only from skl_update_plane.
Regarding the crtc_w and crtc_h, I see that when its not even,
fifo underruns are seen. Kindly suggest on that. Thank you.

> >>> +
> >>>  	/* Sizes are 0 based */
> >>>  	src_w--;
> >>>  	src_h--;
> >>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> >> scaler->mode);
> >>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> >>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> >> << 16) | crtc_y);
> >>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >>> -
> >>> +		if (fb->format->format == DRM_FORMAT_NV12)
> >>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> +				      (MULT4(crtc_w) << 16) |
> >> MULT4(crtc_h));
> >>> +		else
> >>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >>>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
> >>>  	} else {
> >>>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16)
> >> | crtc_x);
> >>> @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane
> >> *plane,
> >>>  		return -EINVAL;
> >>>  	}
> >>>
> >>> +	if (fb->format->format == DRM_FORMAT_NV12)
> >>> +		goto check_plane_surface;
> >>> +
> >>>  	/* setup can_scale, min_scale, max_scale */
> >>>  	if (INTEL_GEN(dev_priv) >= 9) {
> >>>  		if (state->base.fb)
> >>> @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane
> >> *plane,
> >>>  	dst->y1 = crtc_y;
> >>>  	dst->y2 = crtc_y + crtc_h;
> >>>
> >>> +check_plane_surface:
> >>>  	if (INTEL_GEN(dev_priv) >= 9) {
> >>>  		ret = skl_check_plane_surface(crtc_state, state);
> >>>  		if (ret)
> 

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Wednesday, April 4, 2018 1:40 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 04-04-18 om 10:06 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Wednesday, April 4, 2018 1:33 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >> restrictions for NV12
> >>
> >> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> >>> As per display WA 1106, to avoid corruption issues
> >>> NV12 plane height needs to be multiplier of 4 We expect the src
> >>> dimensions to be multiplier of 4 We fail the case where src width or
> >>> height is not multiple of 4 for NV12.
> >>> We also set the scaler destination height and width to be multiple
> >>> of 4. Without this, pipe fifo underruns were seen on APL and KBL. We
> >>> also skip src trunction/adjustments for NV12 case and handle the
> >>> sizes directly in skl_update_plane
> >>>
> >>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >>>  2 files changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >>> b/drivers/gpu/drm/i915/intel_drv.h
> >>> index 9c58da0..a1f718d 100644
> >>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>> @@ -159,6 +159,8 @@
> >>>  #define INTEL_I2C_BUS_DVO 1
> >>>  #define INTEL_I2C_BUS_SDVO 2
> >>>
> >>> +#define MULT4(x) ((x + 3) & ~0x03)
> >>> +
> >>>  /* these are outputs from the chip - integrated only
> >>>     external chips are via DVO or SDVO output */  enum
> >>> intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>> index d5dad44..b2292dd 100644
> >>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >>>  	unsigned long irqflags;
> >>>
> >>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> >>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> >>> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> >>> +		return;
> >>> +	}
> >> Also on a side note, don't add DRM_DEBUG_KMS in update_plane or
> >> disable_plane calls. It can take an arbitrary amount of time,
> >> resulting in an atomic update failure.
> > Sure, thank you. These are anyway duplicate checks. I will remove them.
> > The main one is done in intel_framebuffer_init as suggested by you.
> They're not duplicate, they're done for related reasons:
> 1. It doesn't make sense to allow creation of a framebuffer with an invalid
> width/height if the full width/height cannot be used.
> 2. The checks in skl_update_scaler are the ones that will reject even showing
> a subset with invalid width, which can cause the fifo underruns.

If we don’t keep dest width and height as mult of 4, fifo underruns are seen.
Shall we reject those also? Then it will be safe. As per my observation,
We have only two options. Either change it to mult of 4 or reject it :(
Please suggest.

Thanks
Vidya

> >>> +
> >>>  	/* Sizes are 0 based */
> >>>  	src_w--;
> >>>  	src_h--;
> >>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> >> scaler->mode);
> >>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> >>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> >> << 16) | crtc_y);
> >>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >>> -
> >>> +		if (fb->format->format == DRM_FORMAT_NV12)
> >>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> +				      (MULT4(crtc_w) << 16) |
> >> MULT4(crtc_h));
> >>> +		else
> >>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> +				      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >>>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0);
> >>>  	} else {
> >>>  		I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16)
> >> | crtc_x);
> >>> @@ -969,6 +978,9 @@ intel_check_sprite_plane(struct intel_plane
> >> *plane,
> >>>  		return -EINVAL;
> >>>  	}
> >>>
> >>> +	if (fb->format->format == DRM_FORMAT_NV12)
> >>> +		goto check_plane_surface;
> >>> +
> >>>  	/* setup can_scale, min_scale, max_scale */
> >>>  	if (INTEL_GEN(dev_priv) >= 9) {
> >>>  		if (state->base.fb)
> >>> @@ -1112,6 +1124,7 @@ intel_check_sprite_plane(struct intel_plane
> >> *plane,
> >>>  	dst->y1 = crtc_y;
> >>>  	dst->y2 = crtc_y + crtc_h;
> >>>
> >>> +check_plane_surface:
> >>>  	if (INTEL_GEN(dev_priv) >= 9) {
> >>>  		ret = skl_check_plane_surface(crtc_state, state);
> >>>  		if (ret)
> 

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Maarten Lankhorst]

Op 04-04-18 om 10:29 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Wednesday, April 4, 2018 1:40 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
>> for NV12
>>
>> Op 04-04-18 om 10:06 schreef Srinivas, Vidya:
>>>> -----Original Message-----
>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>> Sent: Wednesday, April 4, 2018 1:33 PM
>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>> gfx@lists.freedesktop.org
>>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
>>>> restrictions for NV12
>>>>
>>>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
>>>>> As per display WA 1106, to avoid corruption issues
>>>>> NV12 plane height needs to be multiplier of 4 We expect the src
>>>>> dimensions to be multiplier of 4 We fail the case where src width or
>>>>> height is not multiple of 4 for NV12.
>>>>> We also set the scaler destination height and width to be multiple
>>>>> of 4. Without this, pipe fifo underruns were seen on APL and KBL. We
>>>>> also skip src trunction/adjustments for NV12 case and handle the
>>>>> sizes directly in skl_update_plane
>>>>>
>>>>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
>>>>>  2 files changed, 18 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>>>> b/drivers/gpu/drm/i915/intel_drv.h
>>>>> index 9c58da0..a1f718d 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>>> @@ -159,6 +159,8 @@
>>>>>  #define INTEL_I2C_BUS_DVO 1
>>>>>  #define INTEL_I2C_BUS_SDVO 2
>>>>>
>>>>> +#define MULT4(x) ((x + 3) & ~0x03)
>>>>> +
>>>>>  /* these are outputs from the chip - integrated only
>>>>>     external chips are via DVO or SDVO output */  enum
>>>>> intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>>>> index d5dad44..b2292dd 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
>>>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>>>>>  	unsigned long irqflags;
>>>>>
>>>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
>>>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
>>>>> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
>>>>> +		return;
>>>>> +	}
>>>> Also on a side note, don't add DRM_DEBUG_KMS in update_plane or
>>>> disable_plane calls. It can take an arbitrary amount of time,
>>>> resulting in an atomic update failure.
>>> Sure, thank you. These are anyway duplicate checks. I will remove them.
>>> The main one is done in intel_framebuffer_init as suggested by you.
>> They're not duplicate, they're done for related reasons:
>> 1. It doesn't make sense to allow creation of a framebuffer with an invalid
>> width/height if the full width/height cannot be used.
>> 2. The checks in skl_update_scaler are the ones that will reject even showing
>> a subset with invalid width, which can cause the fifo underruns.
> If we don’t keep dest width and height as mult of 4, fifo underruns are seen.
> Shall we reject those also? Then it will be safe. As per my observation,
> We have only two options. Either change it to mult of 4 or reject it :(
> Please suggest.
Reject non multiples of 4. Userspace can adapt. :)

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Wednesday, April 4, 2018 2:02 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 04-04-18 om 10:29 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Wednesday, April 4, 2018 1:40 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >> restrictions for NV12
> >>
> >> Op 04-04-18 om 10:06 schreef Srinivas, Vidya:
> >>>> -----Original Message-----
> >>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >>>> Sent: Wednesday, April 4, 2018 1:33 PM
> >>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>> gfx@lists.freedesktop.org
> >>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >>>> restrictions for NV12
> >>>>
> >>>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> >>>>> As per display WA 1106, to avoid corruption issues
> >>>>> NV12 plane height needs to be multiplier of 4 We expect the src
> >>>>> dimensions to be multiplier of 4 We fail the case where src width
> >>>>> or height is not multiple of 4 for NV12.
> >>>>> We also set the scaler destination height and width to be multiple
> >>>>> of 4. Without this, pipe fifo underruns were seen on APL and KBL.
> >>>>> We also skip src trunction/adjustments for NV12 case and handle
> >>>>> the sizes directly in skl_update_plane
> >>>>>
> >>>>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>>>> ---
> >>>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >>>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >>>>>  2 files changed, 18 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >>>>> b/drivers/gpu/drm/i915/intel_drv.h
> >>>>> index 9c58da0..a1f718d 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>>>> @@ -159,6 +159,8 @@
> >>>>>  #define INTEL_I2C_BUS_DVO 1
> >>>>>  #define INTEL_I2C_BUS_SDVO 2
> >>>>>
> >>>>> +#define MULT4(x) ((x + 3) & ~0x03)
> >>>>> +
> >>>>>  /* these are outputs from the chip - integrated only
> >>>>>     external chips are via DVO or SDVO output */  enum
> >>>>> intel_output_type { diff --git
> >>>>> a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> index d5dad44..b2292dd 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >>>>>  	unsigned long irqflags;
> >>>>>
> >>>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> >>>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> >>>>> +		DRM_DEBUG_KMS("NV12: src dimensions not
> valid\n");
> >>>>> +		return;
> >>>>> +	}
> >>>> Also on a side note, don't add DRM_DEBUG_KMS in update_plane or
> >>>> disable_plane calls. It can take an arbitrary amount of time,
> >>>> resulting in an atomic update failure.
> >>> Sure, thank you. These are anyway duplicate checks. I will remove them.
> >>> The main one is done in intel_framebuffer_init as suggested by you.
> >> They're not duplicate, they're done for related reasons:
> >> 1. It doesn't make sense to allow creation of a framebuffer with an
> >> invalid width/height if the full width/height cannot be used.
> >> 2. The checks in skl_update_scaler are the ones that will reject even
> >> showing a subset with invalid width, which can cause the fifo underruns.
> > If we don’t keep dest width and height as mult of 4, fifo underruns are
> seen.
> > Shall we reject those also? Then it will be safe. As per my
> > observation, We have only two options. Either change it to mult of 4
> > or reject it :( Please suggest.
> Reject non multiples of 4. Userspace can adapt. :)

:) okay. Thank you. I will update the patch.

Regards
Vidya

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Maarten Lankhorst]

Op 04-04-18 om 10:04 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Wednesday, April 4, 2018 1:28 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
>> for NV12
>>
>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
>>> As per display WA 1106, to avoid corruption issues
>>> NV12 plane height needs to be multiplier of 4 We expect the src
>>> dimensions to be multiplier of 4 We fail the case where src width or
>>> height is not multiple of 4 for NV12.
>>> We also set the scaler destination height and width to be multiple of
>>> 4. Without this, pipe fifo underruns were seen on APL and KBL. We also
>>> skip src trunction/adjustments for NV12 case and handle the sizes
>>> directly in skl_update_plane
>>>
>>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
>>>  2 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>> b/drivers/gpu/drm/i915/intel_drv.h
>>> index 9c58da0..a1f718d 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -159,6 +159,8 @@
>>>  #define INTEL_I2C_BUS_DVO 1
>>>  #define INTEL_I2C_BUS_SDVO 2
>>>
>>> +#define MULT4(x) ((x + 3) & ~0x03)
>>> +
>>>  /* these are outputs from the chip - integrated only
>>>     external chips are via DVO or SDVO output */  enum
>>> intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>> index d5dad44..b2292dd 100644
>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>>>  	unsigned long irqflags;
>>>
>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
>>> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
>>> +		return;
>>> +	}
>>> +
>> You can't do this check in skl_update_plane, it's way too late. It should be
>> rejected in the plane check with -EINVAL, or perhaps in skl_update_scaler.
> Have done it right from intel_framebuffer_init onwards, have done it in skl_update_scaler also
> In fact it will get rejected in framebuffer init and all these are duplicate checks, can remove them.
>>>  	/* Sizes are 0 based */
>>>  	src_w--;
>>>  	src_h--;
>>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
>>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
>> scaler->mode);
>>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
>> << 16) | crtc_y);
>>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
>>> -
>>> +		if (fb->format->format == DRM_FORMAT_NV12)
>>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>> +				      (MULT4(crtc_w) << 16) |
>> MULT4(crtc_h));
>> See the comment above, sizes are 0 based. This will add 1 to the size, so the
>> size is always 1 more than requested.
>> I don't think it would pass plane CRC tests..
> When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920 back.
> If we don’t do this, I see fifo underruns. The destination width and height
> If not mult of 4, I am seeing underruns.
What you see as 1920 is 1921, which should be underrunning even more and is out of FB bounds.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Wednesday, April 4, 2018 2:18 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 04-04-18 om 10:04 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Wednesday, April 4, 2018 1:28 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >> restrictions for NV12
> >>
> >> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> >>> As per display WA 1106, to avoid corruption issues
> >>> NV12 plane height needs to be multiplier of 4 We expect the src
> >>> dimensions to be multiplier of 4 We fail the case where src width or
> >>> height is not multiple of 4 for NV12.
> >>> We also set the scaler destination height and width to be multiple
> >>> of 4. Without this, pipe fifo underruns were seen on APL and KBL. We
> >>> also skip src trunction/adjustments for NV12 case and handle the
> >>> sizes directly in skl_update_plane
> >>>
> >>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >>>  2 files changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >>> b/drivers/gpu/drm/i915/intel_drv.h
> >>> index 9c58da0..a1f718d 100644
> >>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>> @@ -159,6 +159,8 @@
> >>>  #define INTEL_I2C_BUS_DVO 1
> >>>  #define INTEL_I2C_BUS_SDVO 2
> >>>
> >>> +#define MULT4(x) ((x + 3) & ~0x03)
> >>> +
> >>>  /* these are outputs from the chip - integrated only
> >>>     external chips are via DVO or SDVO output */  enum
> >>> intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>> index d5dad44..b2292dd 100644
> >>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >>>  	unsigned long irqflags;
> >>>
> >>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> >>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> >>> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
> >>> +		return;
> >>> +	}
> >>> +
> >> You can't do this check in skl_update_plane, it's way too late. It
> >> should be rejected in the plane check with -EINVAL, or perhaps in
> skl_update_scaler.
> > Have done it right from intel_framebuffer_init onwards, have done it
> > in skl_update_scaler also In fact it will get rejected in framebuffer init and
> all these are duplicate checks, can remove them.
> >>>  	/* Sizes are 0 based */
> >>>  	src_w--;
> >>>  	src_h--;
> >>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> >> scaler->mode);
> >>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> >>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> >> << 16) | crtc_y);
> >>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >>> -
> >>> +		if (fb->format->format == DRM_FORMAT_NV12)
> >>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>> +				      (MULT4(crtc_w) << 16) |
> >> MULT4(crtc_h));
> >> See the comment above, sizes are 0 based. This will add 1 to the
> >> size, so the size is always 1 more than requested.
> >> I don't think it would pass plane CRC tests..
> > When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920 back.
> > If we don’t do this, I see fifo underruns. The destination width and
> > height If not mult of 4, I am seeing underruns.
> What you see as 1920 is 1921, which should be underrunning even more and
> is out of FB bounds.
Sorry, I gave a wrong example here. It doesn’t happen when we scale it to full resolution.
It happened during other testing when the scaled dest width or height was not
multiple of 4.
 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Maarten Lankhorst]

Op 04-04-18 om 10:51 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Wednesday, April 4, 2018 2:18 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
>> for NV12
>>
>> Op 04-04-18 om 10:04 schreef Srinivas, Vidya:
>>>> -----Original Message-----
>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>> Sent: Wednesday, April 4, 2018 1:28 PM
>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>> gfx@lists.freedesktop.org
>>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
>>>> restrictions for NV12
>>>>
>>>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
>>>>> As per display WA 1106, to avoid corruption issues
>>>>> NV12 plane height needs to be multiplier of 4 We expect the src
>>>>> dimensions to be multiplier of 4 We fail the case where src width or
>>>>> height is not multiple of 4 for NV12.
>>>>> We also set the scaler destination height and width to be multiple
>>>>> of 4. Without this, pipe fifo underruns were seen on APL and KBL. We
>>>>> also skip src trunction/adjustments for NV12 case and handle the
>>>>> sizes directly in skl_update_plane
>>>>>
>>>>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
>>>>>  2 files changed, 18 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>>>> b/drivers/gpu/drm/i915/intel_drv.h
>>>>> index 9c58da0..a1f718d 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>>> @@ -159,6 +159,8 @@
>>>>>  #define INTEL_I2C_BUS_DVO 1
>>>>>  #define INTEL_I2C_BUS_SDVO 2
>>>>>
>>>>> +#define MULT4(x) ((x + 3) & ~0x03)
>>>>> +
>>>>>  /* these are outputs from the chip - integrated only
>>>>>     external chips are via DVO or SDVO output */  enum
>>>>> intel_output_type { diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>>>> index d5dad44..b2292dd 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
>>>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>>>>>  	unsigned long irqflags;
>>>>>
>>>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
>>>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
>>>>> +		DRM_DEBUG_KMS("NV12: src dimensions not valid\n");
>>>>> +		return;
>>>>> +	}
>>>>> +
>>>> You can't do this check in skl_update_plane, it's way too late. It
>>>> should be rejected in the plane check with -EINVAL, or perhaps in
>> skl_update_scaler.
>>> Have done it right from intel_framebuffer_init onwards, have done it
>>> in skl_update_scaler also In fact it will get rejected in framebuffer init and
>> all these are duplicate checks, can remove them.
>>>>>  	/* Sizes are 0 based */
>>>>>  	src_w--;
>>>>>  	src_h--;
>>>>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
>>>>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
>>>> scaler->mode);
>>>>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>>>>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
>>>> << 16) | crtc_y);
>>>>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>>>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
>>>>> -
>>>>> +		if (fb->format->format == DRM_FORMAT_NV12)
>>>>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>>>> +				      (MULT4(crtc_w) << 16) |
>>>> MULT4(crtc_h));
>>>> See the comment above, sizes are 0 based. This will add 1 to the
>>>> size, so the size is always 1 more than requested.
>>>> I don't think it would pass plane CRC tests..
>>> When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920 back.
>>> If we don’t do this, I see fifo underruns. The destination width and
>>> height If not mult of 4, I am seeing underruns.
>> What you see as 1920 is 1921, which should be underrunning even more and
>> is out of FB bounds.
> Sorry, I gave a wrong example here. It doesn’t happen when we scale it to full resolution.
> It happened during other testing when the scaled dest width or height was not
> multiple of 4.
>  

We could reject it, but odd that crtc w/h matters. I didn't expect that. :)

Rejecting with -EINVAL is the best way to go, along with documentation in the form of tests that we handle this case correctly.

~Maarten

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Wednesday, April 4, 2018 2:37 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 04-04-18 om 10:51 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Wednesday, April 4, 2018 2:18 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >> restrictions for NV12
> >>
> >> Op 04-04-18 om 10:04 schreef Srinivas, Vidya:
> >>>> -----Original Message-----
> >>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >>>> Sent: Wednesday, April 4, 2018 1:28 PM
> >>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>> gfx@lists.freedesktop.org
> >>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >>>> restrictions for NV12
> >>>>
> >>>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> >>>>> As per display WA 1106, to avoid corruption issues
> >>>>> NV12 plane height needs to be multiplier of 4 We expect the src
> >>>>> dimensions to be multiplier of 4 We fail the case where src width
> >>>>> or height is not multiple of 4 for NV12.
> >>>>> We also set the scaler destination height and width to be multiple
> >>>>> of 4. Without this, pipe fifo underruns were seen on APL and KBL.
> >>>>> We also skip src trunction/adjustments for NV12 case and handle
> >>>>> the sizes directly in skl_update_plane
> >>>>>
> >>>>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>>>> ---
> >>>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >>>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >>>>>  2 files changed, 18 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >>>>> b/drivers/gpu/drm/i915/intel_drv.h
> >>>>> index 9c58da0..a1f718d 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>>>> @@ -159,6 +159,8 @@
> >>>>>  #define INTEL_I2C_BUS_DVO 1
> >>>>>  #define INTEL_I2C_BUS_SDVO 2
> >>>>>
> >>>>> +#define MULT4(x) ((x + 3) & ~0x03)
> >>>>> +
> >>>>>  /* these are outputs from the chip - integrated only
> >>>>>     external chips are via DVO or SDVO output */  enum
> >>>>> intel_output_type { diff --git
> >>>>> a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> index d5dad44..b2292dd 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
> >>>>>  	unsigned long irqflags;
> >>>>>
> >>>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> >>>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> >>>>> +		DRM_DEBUG_KMS("NV12: src dimensions not
> valid\n");
> >>>>> +		return;
> >>>>> +	}
> >>>>> +
> >>>> You can't do this check in skl_update_plane, it's way too late. It
> >>>> should be rejected in the plane check with -EINVAL, or perhaps in
> >> skl_update_scaler.
> >>> Have done it right from intel_framebuffer_init onwards, have done it
> >>> in skl_update_scaler also In fact it will get rejected in
> >>> framebuffer init and
> >> all these are duplicate checks, can remove them.
> >>>>>  	/* Sizes are 0 based */
> >>>>>  	src_w--;
> >>>>>  	src_h--;
> >>>>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
> >>>>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> >>>> scaler->mode);
> >>>>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> >>>>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> >>>> << 16) | crtc_y);
> >>>>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>>>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >>>>> -
> >>>>> +		if (fb->format->format == DRM_FORMAT_NV12)
> >>>>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe,
> scaler_id),
> >>>>> +				      (MULT4(crtc_w) << 16) |
> >>>> MULT4(crtc_h));
> >>>> See the comment above, sizes are 0 based. This will add 1 to the
> >>>> size, so the size is always 1 more than requested.
> >>>> I don't think it would pass plane CRC tests..
> >>> When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920
> back.
> >>> If we don’t do this, I see fifo underruns. The destination width and
> >>> height If not mult of 4, I am seeing underruns.
> >> What you see as 1920 is 1921, which should be underrunning even more
> >> and is out of FB bounds.
> > Sorry, I gave a wrong example here. It doesn’t happen when we scale it to
> full resolution.
> > It happened during other testing when the scaled dest width or height
> > was not multiple of 4.
> >
> 
> We could reject it, but odd that crtc w/h matters. I didn't expect that. :)
> 
> Rejecting with -EINVAL is the best way to go, along with documentation in
> the form of tests that we handle this case correctly.

Hi Maarten,

Completely understand and agree. With these fifo underruns, I have tried 
too many tests and encountered those fifo underruns under diff set of
experiments. I remember seeing them when src width and height were not
x4. When I kept src dimensions x4, I hit underruns during destination testing. But right now, I tried
to reproduce the same - it isn’t happening on my APL. So, I have removed
all restrictions from the series https://patchwork.freedesktop.org/series/38919/ 
(sent to trybot only for now). I have just retained the src height to be min 16 which
Is mentioned in bspec. Now since we have i-g-t 16x16 merged and the only change for
NV12 majorly for underruns is we skip the truncations that were happening in intel_check_sprite
We will get CI results shortly. Due to too many experiments done at my end, I overloaded myself with data
and get confused - when I hit which issue. Apologies for the same.
PS: I removed the src restrictions too  in this series. Because during clipping clamping tests, the fb src
widthxheight generated is like 257x159. This gets rejected from KMD as its not x4.

Thanks
Vidya

> 
> ~Maarten

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Maarten Lankhorst]

Op 05-04-18 om 07:18 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Wednesday, April 4, 2018 2:37 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
>> for NV12
>>
>> Op 04-04-18 om 10:51 schreef Srinivas, Vidya:
>>>> -----Original Message-----
>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>> Sent: Wednesday, April 4, 2018 2:18 PM
>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>> gfx@lists.freedesktop.org
>>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
>>>> restrictions for NV12
>>>>
>>>> Op 04-04-18 om 10:04 schreef Srinivas, Vidya:
>>>>>> -----Original Message-----
>>>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>>>> Sent: Wednesday, April 4, 2018 1:28 PM
>>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>>>> gfx@lists.freedesktop.org
>>>>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
>>>>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
>>>>>> restrictions for NV12
>>>>>>
>>>>>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
>>>>>>> As per display WA 1106, to avoid corruption issues
>>>>>>> NV12 plane height needs to be multiplier of 4 We expect the src
>>>>>>> dimensions to be multiplier of 4 We fail the case where src width
>>>>>>> or height is not multiple of 4 for NV12.
>>>>>>> We also set the scaler destination height and width to be multiple
>>>>>>> of 4. Without this, pipe fifo underruns were seen on APL and KBL.
>>>>>>> We also skip src trunction/adjustments for NV12 case and handle
>>>>>>> the sizes directly in skl_update_plane
>>>>>>>
>>>>>>> Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>>>>>> ---
>>>>>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
>>>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
>>>>>>>  2 files changed, 18 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>>>>>> b/drivers/gpu/drm/i915/intel_drv.h
>>>>>>> index 9c58da0..a1f718d 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>>>>>> @@ -159,6 +159,8 @@
>>>>>>>  #define INTEL_I2C_BUS_DVO 1
>>>>>>>  #define INTEL_I2C_BUS_SDVO 2
>>>>>>>
>>>>>>> +#define MULT4(x) ((x + 3) & ~0x03)
>>>>>>> +
>>>>>>>  /* these are outputs from the chip - integrated only
>>>>>>>     external chips are via DVO or SDVO output */  enum
>>>>>>> intel_output_type { diff --git
>>>>>>> a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>> index d5dad44..b2292dd 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane *plane,
>>>>>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>>>>>>>  	unsigned long irqflags;
>>>>>>>
>>>>>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
>>>>>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
>>>>>>> +		DRM_DEBUG_KMS("NV12: src dimensions not
>> valid\n");
>>>>>>> +		return;
>>>>>>> +	}
>>>>>>> +
>>>>>> You can't do this check in skl_update_plane, it's way too late. It
>>>>>> should be rejected in the plane check with -EINVAL, or perhaps in
>>>> skl_update_scaler.
>>>>> Have done it right from intel_framebuffer_init onwards, have done it
>>>>> in skl_update_scaler also In fact it will get rejected in
>>>>> framebuffer init and
>>>> all these are duplicate checks, can remove them.
>>>>>>>  	/* Sizes are 0 based */
>>>>>>>  	src_w--;
>>>>>>>  	src_h--;
>>>>>>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane *plane,
>>>>>>>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
>>>>>> scaler->mode);
>>>>>>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>>>>>>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
>>>>>> << 16) | crtc_y);
>>>>>>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
>>>>>>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
>>>>>>> -
>>>>>>> +		if (fb->format->format == DRM_FORMAT_NV12)
>>>>>>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe,
>> scaler_id),
>>>>>>> +				      (MULT4(crtc_w) << 16) |
>>>>>> MULT4(crtc_h));
>>>>>> See the comment above, sizes are 0 based. This will add 1 to the
>>>>>> size, so the size is always 1 more than requested.
>>>>>> I don't think it would pass plane CRC tests..
>>>>> When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920
>> back.
>>>>> If we don’t do this, I see fifo underruns. The destination width and
>>>>> height If not mult of 4, I am seeing underruns.
>>>> What you see as 1920 is 1921, which should be underrunning even more
>>>> and is out of FB bounds.
>>> Sorry, I gave a wrong example here. It doesn’t happen when we scale it to
>> full resolution.
>>> It happened during other testing when the scaled dest width or height
>>> was not multiple of 4.
>>>
>> We could reject it, but odd that crtc w/h matters. I didn't expect that. :)
>>
>> Rejecting with -EINVAL is the best way to go, along with documentation in
>> the form of tests that we handle this case correctly.
> Hi Maarten,
>
> Completely understand and agree. With these fifo underruns, I have tried 
> too many tests and encountered those fifo underruns under diff set of
> experiments. I remember seeing them when src width and height were not
> x4. When I kept src dimensions x4, I hit underruns during destination testing. But right now, I tried
> to reproduce the same - it isn’t happening on my APL. So, I have removed
> all restrictions from the series https://patchwork.freedesktop.org/series/38919/ 
> (sent to trybot only for now). I have just retained the src height to be min 16 which
> Is mentioned in bspec. Now since we have i-g-t 16x16 merged and the only change for
> NV12 majorly for underruns is we skip the truncations that were happening in intel_check_sprite
> We will get CI results shortly. Due to too many experiments done at my end, I overloaded myself with data
> and get confused - when I hit which issue. Apologies for the same.
> PS: I removed the src restrictions too  in this series. Because during clipping clamping tests, the fb src
> widthxheight generated is like 257x159. This gets rejected from KMD as its not x4.
I've done testing and had very reliable underruns with height not a multiple of 2, as could be seen in the earlier kms_nv12 series. Clipping/clamping tests should be fixed at least not to get into such a bad situation for NV12, and take alignment into account.

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

Re: [PATCH v19 18/18] drm/i915: Set src size restrictions for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Thursday, April 5, 2018 3:32 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size restrictions
> for NV12
> 
> Op 05-04-18 om 07:18 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Wednesday, April 4, 2018 2:37 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >> restrictions for NV12
> >>
> >> Op 04-04-18 om 10:51 schreef Srinivas, Vidya:
> >>>> -----Original Message-----
> >>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >>>> Sent: Wednesday, April 4, 2018 2:18 PM
> >>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>> gfx@lists.freedesktop.org
> >>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >>>> restrictions for NV12
> >>>>
> >>>> Op 04-04-18 om 10:04 schreef Srinivas, Vidya:
> >>>>>> -----Original Message-----
> >>>>>> From: Maarten Lankhorst
> >>>>>> [mailto:maarten.lankhorst@linux.intel.com]
> >>>>>> Sent: Wednesday, April 4, 2018 1:28 PM
> >>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>>>> gfx@lists.freedesktop.org
> >>>>>> Cc: Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >>>>>> Subject: Re: [Intel-gfx] [PATCH v19 18/18] drm/i915: Set src size
> >>>>>> restrictions for NV12
> >>>>>>
> >>>>>> Op 02-04-18 om 11:51 schreef Vidya Srinivas:
> >>>>>>> As per display WA 1106, to avoid corruption issues
> >>>>>>> NV12 plane height needs to be multiplier of 4 We expect the src
> >>>>>>> dimensions to be multiplier of 4 We fail the case where src
> >>>>>>> width or height is not multiple of 4 for NV12.
> >>>>>>> We also set the scaler destination height and width to be
> >>>>>>> multiple of 4. Without this, pipe fifo underruns were seen on APL
> and KBL.
> >>>>>>> We also skip src trunction/adjustments for NV12 case and handle
> >>>>>>> the sizes directly in skl_update_plane
> >>>>>>>
> >>>>>>> Credits-to: Maarten Lankhorst
> >>>>>>> <maarten.lankhorst@linux.intel.com>
> >>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>>>>>> ---
> >>>>>>>  drivers/gpu/drm/i915/intel_drv.h    |  2 ++
> >>>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 19 ++++++++++++++++---
> >>>>>>>  2 files changed, 18 insertions(+), 3 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >>>>>>> b/drivers/gpu/drm/i915/intel_drv.h
> >>>>>>> index 9c58da0..a1f718d 100644
> >>>>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>>>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>>>>>> @@ -159,6 +159,8 @@
> >>>>>>>  #define INTEL_I2C_BUS_DVO 1
> >>>>>>>  #define INTEL_I2C_BUS_SDVO 2
> >>>>>>>
> >>>>>>> +#define MULT4(x) ((x + 3) & ~0x03)
> >>>>>>> +
> >>>>>>>  /* these are outputs from the chip - integrated only
> >>>>>>>     external chips are via DVO or SDVO output */  enum
> >>>>>>> intel_output_type { diff --git
> >>>>>>> a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>> index d5dad44..b2292dd 100644
> >>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>> @@ -255,6 +255,12 @@ skl_update_plane(struct intel_plane
> *plane,
> >>>>>>>  	uint32_t src_h = drm_rect_height(&plane_state->base.src)
> >> 16;
> >>>>>>>  	unsigned long irqflags;
> >>>>>>>
> >>>>>>> +	if (fb->format->format == DRM_FORMAT_NV12 &&
> >>>>>>> +		((src_h % 4) != 0 || (src_w % 4) != 0)) {
> >>>>>>> +		DRM_DEBUG_KMS("NV12: src dimensions not
> >> valid\n");
> >>>>>>> +		return;
> >>>>>>> +	}
> >>>>>>> +
> >>>>>> You can't do this check in skl_update_plane, it's way too late.
> >>>>>> It should be rejected in the plane check with -EINVAL, or perhaps
> >>>>>> in
> >>>> skl_update_scaler.
> >>>>> Have done it right from intel_framebuffer_init onwards, have done
> >>>>> it in skl_update_scaler also In fact it will get rejected in
> >>>>> framebuffer init and
> >>>> all these are duplicate checks, can remove them.
> >>>>>>>  	/* Sizes are 0 based */
> >>>>>>>  	src_w--;
> >>>>>>>  	src_h--;
> >>>>>>> @@ -292,9 +298,12 @@ skl_update_plane(struct intel_plane
> *plane,
> >>>>>>>  			      PS_SCALER_EN |
> PS_PLANE_SEL(plane_id) |
> >>>>>> scaler->mode);
> >>>>>>>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe,
> scaler_id), 0);
> >>>>>>>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id),
> (crtc_x
> >>>>>> << 16) | crtc_y);
> >>>>>>> -		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >>>>>>> -			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> >>>>>>> -
> >>>>>>> +		if (fb->format->format == DRM_FORMAT_NV12)
> >>>>>>> +			I915_WRITE_FW(SKL_PS_WIN_SZ(pipe,
> >> scaler_id),
> >>>>>>> +				      (MULT4(crtc_w) << 16) |
> >>>>>> MULT4(crtc_h));
> >>>>>> See the comment above, sizes are 0 based. This will add 1 to the
> >>>>>> size, so the size is always 1 more than requested.
> >>>>>> I don't think it would pass plane CRC tests..
> >>>>> When I align it to mult of 4, 1919 (which was 1920-1) becomes 1920
> >> back.
> >>>>> If we don’t do this, I see fifo underruns. The destination width
> >>>>> and height If not mult of 4, I am seeing underruns.
> >>>> What you see as 1920 is 1921, which should be underrunning even
> >>>> more and is out of FB bounds.
> >>> Sorry, I gave a wrong example here. It doesn’t happen when we scale
> >>> it to
> >> full resolution.
> >>> It happened during other testing when the scaled dest width or
> >>> height was not multiple of 4.
> >>>
> >> We could reject it, but odd that crtc w/h matters. I didn't expect
> >> that. :)
> >>
> >> Rejecting with -EINVAL is the best way to go, along with
> >> documentation in the form of tests that we handle this case correctly.
> > Hi Maarten,
> >
> > Completely understand and agree. With these fifo underruns, I have
> > tried too many tests and encountered those fifo underruns under diff
> > set of experiments. I remember seeing them when src width and height
> > were not x4. When I kept src dimensions x4, I hit underruns during
> > destination testing. But right now, I tried to reproduce the same - it
> > isn’t happening on my APL. So, I have removed all restrictions from
> > the series https://patchwork.freedesktop.org/series/38919/
> > (sent to trybot only for now). I have just retained the src height to
> > be min 16 which Is mentioned in bspec. Now since we have i-g-t 16x16
> > merged and the only change for
> > NV12 majorly for underruns is we skip the truncations that were
> > happening in intel_check_sprite We will get CI results shortly. Due to
> > too many experiments done at my end, I overloaded myself with data and
> get confused - when I hit which issue. Apologies for the same.
> > PS: I removed the src restrictions too  in this series. Because during
> > clipping clamping tests, the fb src widthxheight generated is like 257x159.
> This gets rejected from KMD as its not x4.
> I've done testing and had very reliable underruns with height not a multiple
> of 2, as could be seen in the earlier kms_nv12 series. Clipping/clamping
> tests should be fixed at least not to get into such a bad situation for NV12,
> and take alignment into account.

Sure, will address the same. Thank you.

Regards
Vidya

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