[PATCH 00/16] Adding NV12 support

[PATCH 00/16] Adding 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/

Update from last rev:
	Rebased the series.
	Review comments by Maarten are addressed in this series.
	NV12 enabled for Gen10.

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 (1):
  drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg

 drivers/gpu/drm/i915/i915_drv.h      |   9 +-
 drivers/gpu/drm/i915/i915_reg.h      |   7 +
 drivers/gpu/drm/i915/intel_atomic.c  |   8 +-
 drivers/gpu/drm/i915/intel_display.c |  51 +++-
 drivers/gpu/drm/i915/intel_drv.h     |   9 +-
 drivers/gpu/drm/i915/intel_pm.c      | 435 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_sprite.c  |  16 +-
 7 files changed, 349 insertions(+), 186 deletions(-)

-- 
2.7.4

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

[PATCH 01/16] 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

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  | 14 +++++++-------
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d6b5ac2..d73e0a2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1444,7 +1444,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;
 };
@@ -2139,7 +2139,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 468ec1e..61d9946 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -480,7 +480,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 eb68abf..0cd5e95 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5042,8 +5042,8 @@ 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,
+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],
@@ -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] 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

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 d73e0a2..35d1663 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1440,8 +1440,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 61d9946..35ee715 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -595,6 +595,7 @@ struct intel_pipe_wm {
 struct skl_plane_wm {
 	struct skl_wm_level wm[8];
 	struct skl_wm_level trans_wm;
+	bool is_nv12;
 };
 
 struct skl_pipe_wm {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0cd5e95..1546bc8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4012,9 +4012,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;
@@ -4028,9 +4028,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;
 
 	/*
@@ -4041,19 +4041,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);
@@ -4066,8 +4061,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;
@@ -4083,17 +4078,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;
 	}
@@ -4102,8 +4097,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);
@@ -4114,8 +4108,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 */
@@ -4134,15 +4128,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) {
@@ -4170,7 +4161,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;
@@ -4185,7 +4176,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);
@@ -4203,17 +4194,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;
@@ -4228,7 +4219,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
@@ -4238,7 +4229,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) {
@@ -4260,14 +4251,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;
@@ -4291,21 +4282,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,
+		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;
@@ -4433,8 +4423,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);
 
@@ -4661,6 +4650,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_nv12 = 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_nv12) {
+		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]);
+			PLANE_NV12_BUF_CFG(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 03/16] 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.

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 60ba5bb..e3a6a7f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2626,6 +2626,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 04/16] drm/i915/skl+: support verification of DDB HW state for NV12

[Vidya Srinivas]

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

NV12 formats have two registers for DDB. Verify both the registers for
NV12 during verify_wm_state.

v2: Addressed review comments by Maarten.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@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      | 50 ++++++++++++++++++++++++++++--------
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e3a6a7f..5fc9255 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2621,7 +2621,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 35ee715..ed33840 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1595,6 +1595,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 5bff004..4c9a811 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3828,6 +3828,43 @@ 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, 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 */)
 {
@@ -3844,16 +3881,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 05/16] 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.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@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  | 54 ++++++++++++++++++++++++++++++++--------
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cca5414..778dc5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1459,6 +1459,7 @@ struct skl_wm_level {
 struct skl_wm_params {
 	bool x_tiled, y_tiled;
 	bool rc_surface;
+	bool is_nv12;
 	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 ed33840..65dac21 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -594,6 +594,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_nv12;
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c9a811..b3d1bf7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4421,7 +4421,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_num)
 {
 	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
 	const struct drm_plane_state *pstate = &intel_pstate->base;
@@ -4434,6 +4434,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_num == 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 ||
@@ -4441,6 +4447,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_nv12 = fb->format->format == DRM_FORMAT_NV12;
 
 	if (plane->id == PLANE_CURSOR) {
 		wp->width = intel_pstate->base.crtc_w;
@@ -4453,7 +4460,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_num == 1 && wp->is_nv12)
+		wp->width /= 2;
+
+	wp->cpp = fb->format->cpp[plane_num];
 	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_num)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_plane *plane = intel_pstate->base.plane;
@@ -4657,15 +4668,20 @@ 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 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]);
+	if (plane_num == 0)
+		ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
+	else
+		ddb_blocks = skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]);
 
 	for (level = 0; level <= max_level; level++) {
-		struct skl_wm_level *result = &wm->wm[level];
+		struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
+							  &wm->wm[level];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4680,9 +4696,6 @@ 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_nv12 = true;
-
 	return 0;
 }
 
@@ -4791,20 +4804,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 */
+		if (wm_params.is_nv12) {
+			memset(&wm_params, 0, sizeof(struct skl_wm_params));
+			wm->is_nv12 = 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 06/16] drm/i915/skl+: pass skl_wm_level struct to wm compute func

[Vidya Srinivas]

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

This will reduce number of arguments required to be passed in
skl_compute_plane_wm function.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@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 b3d1bf7..07fc084 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4531,9 +4531,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];
@@ -4547,7 +4545,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;
 	}
 
@@ -4627,7 +4625,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 
 	if (res_blocks >= ddb_allocation || res_lines > 31 ||
 	    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,
 		}
 	}
 
-	*out_blocks = res_blocks;
-	*out_lines = res_lines;
-	*enabled = true;
+	result->plane_res_b = res_blocks;
+	result->plane_res_l = res_lines;
+	result->plane_en = true;
 
 	return 0;
 }
@@ -4689,9 +4687,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 07/16] 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.
Render decompression requires level WM to be as high as wm level-0.
This patch fulfils both the requirements.

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 07fc084..d9801bf 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4531,6 +4531,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;
@@ -4598,6 +4599,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.
+		 * 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) {
@@ -4680,6 +4690,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_num ? &wm->uv_wm[level] :
 							  &wm->wm[level];
+		struct skl_wm_level *result_prev;
+
+		if (level)
+			result_prev = plane_num ? &wm->uv_wm[level - 1] :
+						  &wm->wm[level - 1];
+		else
+			result_prev = plane_num ? &wm->uv_wm[0] : &wm->wm[0];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4687,6 +4704,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 08/16] 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.

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 d9801bf..c37d014 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4654,6 +4654,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_nv12 && (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;
+	}
+
 	result->plane_res_b = res_blocks;
 	result->plane_res_l = res_lines;
 	result->plane_en = true;
-- 
2.7.4

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

[PATCH 09/16] 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.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@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 c37d014..477eaea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5058,69 +5058,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;
@@ -5182,23 +5129,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
@@ -5209,14 +5156,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 10/16] 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.

Tested-by: Clinton Taylor <clinton.a.taylor@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     | 1 +
 drivers/gpu/drm/i915/intel_atomic.c | 8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e9c79b5..18be7be 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6705,6 +6705,7 @@ enum {
 #define PS_SCALER_MODE_MASK (3 << 28)
 #define PS_SCALER_MODE_DYN  (0 << 28)
 #define PS_SCALER_MODE_HQ  (1 << 28)
+#define PS_SCALER_MODE_NV12 (2 << 28)
 #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 d452c32..196427a 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -327,8 +327,12 @@ 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) {
+			scaler_state->scalers[*scaler_id].mode =
+				PS_SCALER_MODE_NV12;
 		} 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 11/16] 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.

Tested-by: Clinton Taylor <clinton.a.taylor@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_drv.h    | 1 +
 drivers/gpu/drm/i915/intel_sprite.c | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 65dac21..d4c027a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2032,6 +2032,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 3be22c0..9e31be2 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -41,14 +41,14 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
-static bool
-format_is_yuv(uint32_t format)
+bool intel_format_is_yuv(uint32_t format)
 {
 	switch (format) {
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
 	case DRM_FORMAT_YVYU:
+	case DRM_FORMAT_NV12:
 		return true;
 	default:
 		return false;
@@ -352,7 +352,7 @@ chv_update_csc(struct intel_plane *plane, uint32_t format)
 	enum plane_id plane_id = plane->id;
 
 	/* Seems RGB data bypasses the CSC always */
-	if (!format_is_yuv(format))
+	if (!intel_format_is_yuv(format))
 		return;
 
 	/*
@@ -979,7 +979,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 		src_y = src->y1 >> 16;
 		src_h = drm_rect_height(src) >> 16;
 
-		if (format_is_yuv(fb->format->format)) {
+		if (intel_format_is_yuv(fb->format->format)) {
 			src_x &= ~1;
 			src_w &= ~1;
 
-- 
2.7.4

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

[PATCH 12/16] 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)

Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 33 +++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 drivers/gpu/drm/i915/intel_sprite.c  |  3 ++-
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5fc9255..74757d0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3435,6 +3435,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);
 	}
@@ -4658,7 +4660,8 @@ 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,
+		  uint32_t pixel_format)
 {
 	struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
@@ -4674,7 +4677,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	 * the 90/270 degree plane rotation cases (to match the
 	 * GTT mapping), hence no need to account for rotation here.
 	 */
-	need_scaling = src_w != dst_w || src_h != dst_h;
+	need_scaling = src_w != dst_w || src_h != dst_h ||
+				   (pixel_format == DRM_FORMAT_NV12);
 
 	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
 		need_scaling = true;
@@ -4753,7 +4757,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
 	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
 		&state->scaler_state.scaler_id,
 		state->pipe_src_w, state->pipe_src_h,
-		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
+		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay, 0);
 }
 
 /**
@@ -4783,7 +4787,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 ? fb->format->format : 0);
 
 	if (ret || plane_state->scaler_id < 0)
 		return ret;
@@ -4809,6 +4814,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",
@@ -12758,11 +12764,12 @@ 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;
@@ -12784,8 +12791,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;
 }
@@ -12807,7 +12816,11 @@ intel_check_primary_plane(struct intel_plane *plane,
 		/* 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);
+			max_scale = skl_max_scale(to_intel_crtc(crtc),
+						crtc_state,
+						state->base.fb ?
+						state->base.fb->format->format :
+						0);
 		}
 		can_position = true;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d4c027a..111f1a4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1580,7 +1580,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 9e31be2..f2e144b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -897,7 +897,8 @@ intel_check_sprite_plane(struct intel_plane *plane,
 		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,
+						fb->format->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 13/16] 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.

Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 74757d0..24248c7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = {
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
 };
 
 static const uint64_t skl_format_modifiers_noccs[] = {
@@ -13233,6 +13234,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		intel_primary_formats = skl_primary_formats;
 		num_formats = ARRAY_SIZE(skl_primary_formats);
 
+		if (INTEL_GEN(dev_priv) == 9 && (pipe == PIPE_C))
+			num_formats -= 1;
+
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
 			modifiers = skl_format_modifiers_ccs;
 		else
-- 
2.7.4

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

[PATCH 14/16] 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.

Tested-by: Clinton Taylor <clinton.a.taylor@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 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index f2e144b..f359b22 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1168,6 +1168,7 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
 };
 
 static const uint64_t skl_plane_format_modifiers_noccs[] = {
@@ -1366,6 +1367,10 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
 
+		if (INTEL_GEN(dev_priv) <= 10 && ((plane != 0) ||
+			(pipe == PIPE_C)))
+			num_plane_formats -= 1;
+
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
 			modifiers = skl_plane_format_modifiers_ccs;
 		else
-- 
2.7.4

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

[PATCH 15/16] 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

Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 24248c7..b4a5e05 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14040,6 +14040,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 			goto err;
 		}
 		break;
+	case DRM_FORMAT_NV12:
+		if (INTEL_GEN(dev_priv) < 9) {
+			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));
-- 
2.7.4

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

[PATCH 16/16] 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.

Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 6 ++++++
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 18be7be..f813406 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6426,6 +6426,12 @@ enum {
 #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
 #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30)
 #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23)
+#define PLANE_COLOR_CSC_MASK	(0x7 << 17)
+#define PLANE_COLOR_CSC_MODE_BYPASS	(0 << 17)
+#define PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709	(1 << 17)
+#define PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709	(2 << 17)
+#define PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020	(3 << 17)
+#define PLANE_COLOR_CSC_MODE_RGB709_TO_RGB2020	(4 << 17)
 #define   PLANE_COLOR_PLANE_GAMMA_DISABLE	(1 << 13)
 #define   PLANE_COLOR_ALPHA_MASK		(0x3 << 4)
 #define   PLANE_COLOR_ALPHA_DISABLE		(0 << 4)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b4a5e05..d34adda 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3577,6 +3577,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
 	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
 	plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
+	if (intel_format_is_yuv(fb->format->format))
+		plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
 
 	return plane_color_ctl;
 }
-- 
2.7.4

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

Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>
> Add support of recognizing DRM_FORMAT_NV12 from plane_format
> register value.
>
> 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 60ba5bb..e3a6a7f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2626,6 +2626,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;
I dont think this is correct, the case PLANE_CTL_FORMAT_NV12 is defined 
as (1 << 24) but when I check bspec definition, 24th bit is set for 
P010/12/16 formats. AFAIK NV12 is 8 bit format whereas P0xx formats are 
10/12/16 bit formats (they both are YCBCR 4:2:0 of course). This means 
we have mixed NV12 format with P0xx formats. When I checked the 
definition of DRM_FORMAT_NV12, I am not sure if that's intended for 
this. Ville, I saw that the DRM_FORMAT_NV12 definition was added by you, 
can you please comment if this is the right usage ?

- Shashank
>   	default:
>   	case PLANE_CTL_FORMAT_XRGB_8888:
>   		if (rgb_order) {

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

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

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>
> NV12 formats have two registers for DDB. Verify both the registers for
> NV12 during verify_wm_state.
The commit message can add a little more details about how we are 
planning to program. You can pick the direct line from Bspec definition 
about how the UV buffer allocation should happen and how the Y buffer 
allocation.
>
> v2: Addressed review comments by Maarten.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@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      | 50 ++++++++++++++++++++++++++++--------
>   3 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e3a6a7f..5fc9255 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2621,7 +2621,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 35ee715..ed33840 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1595,6 +1595,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 5bff004..4c9a811 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3828,6 +3828,43 @@ 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 */)
Is it me, or each of the input variables need one more space for 
alignment ?
> +{
> +	u32 val, val2 = 0;
> +	int fourcc, pixel_format;
> +
> +	/* Cursor doesn't support NV12, 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
please continue the curly brace for else case too
- Shashank
> +		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 */)
>   {
> @@ -3844,16 +3881,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);
>   	}

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

Re: [PATCH 05/16] drm/i915/skl+: NV12 related changes for WM

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@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  | 54 ++++++++++++++++++++++++++++++++--------
>   3 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cca5414..778dc5a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1459,6 +1459,7 @@ struct skl_wm_level {
>   struct skl_wm_params {
>   	bool x_tiled, y_tiled;
>   	bool rc_surface;
> +	bool is_nv12;
>   	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 ed33840..65dac21 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -594,6 +594,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_nv12;
>   };
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4c9a811..b3d1bf7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4421,7 +4421,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_num)
Is this plane_num intended for 'no of planes' or 'plane number' ? from 
the usage below looks like 'no of planes' but I could be wrong.
>   {
>   	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
>   	const struct drm_plane_state *pstate = &intel_pstate->base;
> @@ -4434,6 +4434,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_num == 1 && fb->format->format != DRM_FORMAT_NV12) {
> +		DRM_DEBUG_KMS("Non NV12 format have single plane\n");
> +		return -EINVAL;
> +	}
The comment says only NV12 format can have two planes, but if format != 
NV12 && plane_num == 1, why is this an error ? Also the debug message 
says "Non NV12 format have single plane". Shouldn't it ?
Looks like you are expecting plane_number = 0, 1 for NV12 planes, and 
when plane_num is non-zero, you expect it to be NV12 only ?
> +
>   	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 ||
> @@ -4441,6 +4447,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_nv12 = fb->format->format == DRM_FORMAT_NV12;
>   
>   	if (plane->id == PLANE_CURSOR) {
>   		wp->width = intel_pstate->base.crtc_w;
> @@ -4453,7 +4460,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_num == 1 && wp->is_nv12)
> +		wp->width /= 2;
> +
> +	wp->cpp = fb->format->cpp[plane_num];
>   	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_num)
Same question here
>   {
>   	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
>   	struct drm_plane *plane = intel_pstate->base.plane;
> @@ -4657,15 +4668,20 @@ 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 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]);
> +	if (plane_num == 0)
> +		ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
> +	else
> +		ddb_blocks = skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]);
>   
>   	for (level = 0; level <= max_level; level++) {
> -		struct skl_wm_level *result = &wm->wm[level];
> +		struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
> +							  &wm->wm[level];
It would look neat if both above condition as well as this one follows 
the same pattern, if() else() Or (cond)? () : ().
>   
>   		ret = skl_compute_plane_wm(dev_priv,
>   					   cstate,
> @@ -4680,9 +4696,6 @@ 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_nv12 = true;
> -
>   	return 0;
>   }
>   
> @@ -4791,20 +4804,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);
This is confusing that you refer to 0 as plane_num = 1 and 1 as 
plane_num = 2.
>   		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 */
> +		if (wm_params.is_nv12) {
> +			memset(&wm_params, 0, sizeof(struct skl_wm_params));
> +			wm->is_nv12 = true;
> +
> +			ret = skl_compute_plane_wm_params(dev_priv, cstate,
> +							  intel_pstate,
> +							  &wm_params, 1);
Same here, there should be a better way to do this.

- Shashank
> +			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;

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

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

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>
> This will reduce number of arguments required to be passed in
> skl_compute_plane_wm function.
The commit message can be like "This patch is passes skl_wm_level 
structure itself to watermark computation function (instead of its 
internal parameters). It reduces number of arguments required to be passed"
With or without this change,
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, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b3d1bf7..07fc084 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4531,9 +4531,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];
> @@ -4547,7 +4545,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;
>   	}
>   
> @@ -4627,7 +4625,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>   
>   	if (res_blocks >= ddb_allocation || res_lines > 31 ||
>   	    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,
>   		}
>   	}
>   
> -	*out_blocks = res_blocks;
> -	*out_lines = res_lines;
> -	*enabled = true;
> +	result->plane_res_b = res_blocks;
> +	result->plane_res_l = res_lines;
> +	result->plane_en = true;
>   
>   	return 0;
>   }
> @@ -4689,9 +4687,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;
>   	}

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

Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc

[Srinivas, Vidya]

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Wednesday, February 7, 2018 9:22 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Kumar, Mahesh1 <mahesh1.kumar@intel.com>
> Subject: Re: [PATCH 03/16] drm/i915/skl+: add NV12 in
> skl_format_to_fourcc
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> > From: Mahesh Kumar <mahesh1.kumar@intel.com>
> >
> > Add support of recognizing DRM_FORMAT_NV12 from plane_format
> register
> > value.
> >
> > 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 60ba5bb..e3a6a7f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2626,6 +2626,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;
> I dont think this is correct, the case PLANE_CTL_FORMAT_NV12 is defined as
> (1 << 24) but when I check bspec definition, 24th bit is set for
> P010/12/16 formats. AFAIK NV12 is 8 bit format whereas P0xx formats are
> 10/12/16 bit formats (they both are YCBCR 4:2:0 of course). This means we
> have mixed NV12 format with P0xx formats. When I checked the definition
> of DRM_FORMAT_NV12, I am not sure if that's intended for this. Ville, I saw
> that the DRM_FORMAT_NV12 definition was added by you, can you please
> comment if this is the right usage ?
> 

Upto Gen10 24-27 bits of PLANE_CTL will be used for format. ICL onwards 23rd bit
is also used. PLANE_CTL_FORMAT_MASK has been defined in i915_reg.h
and mapping will be same if 23rd bit is 0. For NV12, 1<< 24 thus holds good
for all Gen. 

> >   	default:
> >   	case PLANE_CTL_FORMAT_XRGB_8888:
> >   		if (rgb_order) {

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

Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc

[Sharma, Shashank]

On 2/8/2018 8:50 AM, Srinivas, Vidya wrote:
>
>> -----Original Message-----
>> From: Sharma, Shashank
>> Sent: Wednesday, February 7, 2018 9:22 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
>> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
>> Kumar, Mahesh1 <mahesh1.kumar@intel.com>
>> Subject: Re: [PATCH 03/16] drm/i915/skl+: add NV12 in
>> skl_format_to_fourcc
>>
>> Regards
>>
>> Shashank
>>
>>
>> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
>>> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>>>
>>> Add support of recognizing DRM_FORMAT_NV12 from plane_format
>> register
>>> value.
>>>
>>> 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 60ba5bb..e3a6a7f 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -2626,6 +2626,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;
>> I dont think this is correct, the case PLANE_CTL_FORMAT_NV12 is defined as
>> (1 << 24) but when I check bspec definition, 24th bit is set for
>> P010/12/16 formats. AFAIK NV12 is 8 bit format whereas P0xx formats are
>> 10/12/16 bit formats (they both are YCBCR 4:2:0 of course). This means we
>> have mixed NV12 format with P0xx formats. When I checked the definition
>> of DRM_FORMAT_NV12, I am not sure if that's intended for this. Ville, I saw
>> that the DRM_FORMAT_NV12 definition was added by you, can you please
>> comment if this is the right usage ?
>>
> Upto Gen10 24-27 bits of PLANE_CTL will be used for format. ICL onwards 23rd bit
> is also used. PLANE_CTL_FORMAT_MASK has been defined in i915_reg.h
> and mapping will be same if 23rd bit is 0. For NV12, 1<< 24 thus holds good
> for all Gen.
That's not my point. What I want to say is, as per bspec (1 << 24) is 
for P010/P012/P016 formats, not NV12. NV12 is 8 bit YCBCR 4:2:0 format 
whereas P010/012/016 are 10,12 and 16 bit YCBCR 4:2:0 formats. So I was 
not sure if that should be called NV12 and hence I was not sure if we 
should return DRM_FORMAT_NV12 for the same.
- Shashank
>>>    	default:
>>>    	case PLANE_CTL_FORMAT_XRGB_8888:
>>>    		if (rgb_order) {

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

Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc

[Sharma, Shashank]

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

Regards

Shashank


On 2/8/2018 10:02 AM, Sharma, Shashank wrote:
>
>
> On 2/8/2018 8:50 AM, Srinivas, Vidya wrote:
>>
>>> -----Original Message-----
>>> From: Sharma, Shashank
>>> Sent: Wednesday, February 7, 2018 9:22 PM
>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>> gfx@lists.freedesktop.org
>>> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
>>> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
>>> Kumar, Mahesh1 <mahesh1.kumar@intel.com>
>>> Subject: Re: [PATCH 03/16] drm/i915/skl+: add NV12 in
>>> skl_format_to_fourcc
>>>
>>> Regards
>>>
>>> Shashank
>>>
>>>
>>> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
>>>> From: Mahesh Kumar <mahesh1.kumar@intel.com>
>>>>
>>>> Add support of recognizing DRM_FORMAT_NV12 from plane_format
>>> register
>>>> value.
>>>>
>>>> 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 60ba5bb..e3a6a7f 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -2626,6 +2626,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;
>>> I dont think this is correct, the case PLANE_CTL_FORMAT_NV12 is 
>>> defined as
>>> (1 << 24) but when I check bspec definition, 24th bit is set for
>>> P010/12/16 formats. AFAIK NV12 is 8 bit format whereas P0xx formats are
>>> 10/12/16 bit formats (they both are YCBCR 4:2:0 of course). This 
>>> means we
>>> have mixed NV12 format with P0xx formats. When I checked the definition
>>> of DRM_FORMAT_NV12, I am not sure if that's intended for this. 
>>> Ville, I saw
>>> that the DRM_FORMAT_NV12 definition was added by you, can you please
>>> comment if this is the right usage ?
>>>
>> Upto Gen10 24-27 bits of PLANE_CTL will be used for format. ICL 
>> onwards 23rd bit
>> is also used. PLANE_CTL_FORMAT_MASK has been defined in i915_reg.h
>> and mapping will be same if 23rd bit is 0. For NV12, 1<< 24 thus 
>> holds good
>> for all Gen.
> That's not my point. What I want to say is, as per bspec (1 << 24) is 
> for P010/P012/P016 formats, not NV12. NV12 is 8 bit YCBCR 4:2:0 format 
> whereas P010/012/016 are 10,12 and 16 bit YCBCR 4:2:0 formats. So I 
> was not sure if that should be called NV12 and hence I was not sure if 
> we should return DRM_FORMAT_NV12 for the same.
> - Shashank
What I mean to say here is, the check (1 << 24) is not good enough for 
all NV12, as you have to use it as if (mask == YCBCR_420_FORMAT_NV12) 
not (mask & YCBCR_420_FORMAT_NV12).
But if that's how its intended to be used for future addition of 
P010/012/016,  then I guess you can bypass this comment, but please make 
sure you differentiate NV12 with those formats.
- Shashank
>>>>        default:
>>>>        case PLANE_CTL_FORMAT_XRGB_8888:
>>>>            if (rgb_order) {
>


[-- Attachment #1.2: Type: text/html, Size: 42793 bytes --]

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

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

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

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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.
this line / statement can be more clear, "DDB value to be as high as 
level below "  what is level below ? are we talking about C state level 
or DDB level ?
> Render decompression requires level WM to be as high as wm level-0.
> This patch fulfils both the requirements.
>
> 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 07fc084..d9801bf 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4531,6 +4531,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;
> @@ -4598,6 +4599,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.
what is level below ? latency level or you mean "one level below the 
current level" ? Please reformat this comment.
> +		 * 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) {
> @@ -4680,6 +4690,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_num ? &wm->uv_wm[level] :
>   							  &wm->wm[level];
> +		struct skl_wm_level *result_prev;
> +
> +		if (level)
> +			result_prev = plane_num ? &wm->uv_wm[level - 1] :
> +						  &wm->wm[level - 1];
> +		else
> +			result_prev = plane_num ? &wm->uv_wm[0] : &wm->wm[0];
>   
>   		ret = skl_compute_plane_wm(dev_priv,
>   					   cstate,
> @@ -4687,6 +4704,7 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
>   					   ddb_blocks,
>   					   level,
>   					   wm_params,
> +					   result_prev,
>   					   result);
>   		if (ret)
>   			return ret;

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

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

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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.
>
> 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 d9801bf..c37d014 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4654,6 +4654,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_nv12 && (level >= 1) &&
> +		(IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> +		 IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))) {
Seeing that this WAR is applicable for all GEN9 and CNL some rev ids, Is 
this applicable for GLK too ?
- Shashank
> +		result->plane_en = false;
> +		return 0;
> +	}
> +
>   	result->plane_res_b = res_blocks;
>   	result->plane_res_l = res_lines;
>   	result->plane_en = true;

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

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

[Kumar, Mahesh]

Hi,


On 2/8/2018 2:01 PM, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
>> 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.
>>
>> 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 d9801bf..c37d014 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4654,6 +4654,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_nv12 && (level >= 1) &&
>> +        (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
>> +         IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))) {
> Seeing that this WAR is applicable for all GEN9 and CNL some rev ids, 
> Is this applicable for GLK too ?
Not really for all GEN-9, it's valid for SKL/BXT-ALL & CNL some id, so 
it doesn't seems to be applicable for GLK.

-Mahesh
> - Shashank
>> +        result->plane_en = false;
>> +        return 0;
>> +    }
>> +
>>       result->plane_res_b = res_blocks;
>>       result->plane_res_l = res_lines;
>>       result->plane_en = true;
>

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

Re: [PATCH 10/16] drm/i915: Set scaler mode for NV12

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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.
I am not sure if the same code is applicable for all GEN > 9, I am 
seeing a different bit definition for GEN > 10 for bit 29:28
> Comments under v8 is not valid anymore.
>
> Tested-by: Clinton Taylor <clinton.a.taylor@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     | 1 +
>   drivers/gpu/drm/i915/intel_atomic.c | 8 ++++++--
>   2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e9c79b5..18be7be 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6705,6 +6705,7 @@ enum {
>   #define PS_SCALER_MODE_MASK (3 << 28)
>   #define PS_SCALER_MODE_DYN  (0 << 28)
>   #define PS_SCALER_MODE_HQ  (1 << 28)
> +#define PS_SCALER_MODE_NV12 (2 << 28)
>   #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 d452c32..196427a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -327,8 +327,12 @@ 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) &&
Same here, this might not be applicable for (GEN > 10) due to different 
bit definition of 29:28.
> +			plane_state && plane_state->base.fb &&
> +			plane_state->base.fb->format->format ==
> +			DRM_FORMAT_NV12) {
Above alignment should be aligned to first line (INTEL_GEN())
> +			scaler_state->scalers[*scaler_id].mode =
> +				PS_SCALER_MODE_NV12;
>   		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
>   			/*
>   			 * when only 1 scaler is in use on either pipe A or B,

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

Re: [PATCH 11/16] drm/i915: Update format_is_yuv() to include NV12

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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.
>
> Tested-by: Clinton Taylor <clinton.a.taylor@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_drv.h    | 1 +
>   drivers/gpu/drm/i915/intel_sprite.c | 8 ++++----
>   2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 65dac21..d4c027a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2032,6 +2032,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 3be22c0..9e31be2 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -41,14 +41,14 @@
>   #include <drm/i915_drm.h>
>   #include "i915_drv.h"
>   
> -static bool
> -format_is_yuv(uint32_t format)
> +bool intel_format_is_yuv(uint32_t format)
>   {
>   	switch (format) {
>   	case DRM_FORMAT_YUYV:
>   	case DRM_FORMAT_UYVY:
>   	case DRM_FORMAT_VYUY:
>   	case DRM_FORMAT_YVYU:
> +	case DRM_FORMAT_NV12:
>   		return true;
>   	default:
>   		return false;
> @@ -352,7 +352,7 @@ chv_update_csc(struct intel_plane *plane, uint32_t format)
>   	enum plane_id plane_id = plane->id;
>   
>   	/* Seems RGB data bypasses the CSC always */
> -	if (!format_is_yuv(format))
> +	if (!intel_format_is_yuv(format))
This assumption that CSC does only limited_range to full_range 
conversion is only correct until we enable plane level gamut mapping / 
generic color conversion. It would be great if we keep it like:
chv_update_csc() {
     if (intel_format_is_yuv()) {
         /* limited_range -> full_range code here */
     }
     /* So that generic CSC/GM code can be added here */
}

But this should not really a blocker for this patch,
>   		return;
>   
>   	/*
> @@ -979,7 +979,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
>   		src_y = src->y1 >> 16;
>   		src_h = drm_rect_height(src) >> 16;
>   
> -		if (format_is_yuv(fb->format->format)) {
> +		if (intel_format_is_yuv(fb->format->format)) {
>   			src_x &= ~1;
>   			src_w &= ~1;
>   
so with or without above comment addressed:
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/16] drm/i915: Upscale scaler max scale for NV12

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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)
>
> Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 33 +++++++++++++++++++++++----------
>   drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
>   drivers/gpu/drm/i915/intel_sprite.c  |  3 ++-
>   3 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5fc9255..74757d0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3435,6 +3435,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);
>   	}
> @@ -4658,7 +4660,8 @@ 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,
> +		  uint32_t pixel_format)
>   {
>   	struct intel_crtc_scaler_state *scaler_state =
>   		&crtc_state->scaler_state;
> @@ -4674,7 +4677,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>   	 * the 90/270 degree plane rotation cases (to match the
>   	 * GTT mapping), hence no need to account for rotation here.
>   	 */
> -	need_scaling = src_w != dst_w || src_h != dst_h;
> +	need_scaling = src_w != dst_w || src_h != dst_h ||
> +				   (pixel_format == DRM_FORMAT_NV12);
This line needs to be aligned to above line, after = , also no need for 
braces
>   
>   	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
>   		need_scaling = true;
> @@ -4753,7 +4757,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
>   	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
>   		&state->scaler_state.scaler_id,
>   		state->pipe_src_w, state->pipe_src_h,
> -		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
> +		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay, 0);
Why 0 ? shouldn't we send actual current pixel format ? Or if you are 
just looking for pixel format is NV12 or not, you can make this variable 
a bool and name it is_nv12 or is_yuv420
>   }
>   
>   /**
> @@ -4783,7 +4787,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 ? fb->format->format : 0);
Again, sending 0 doesn't look logical to me
>   
>   	if (ret || plane_state->scaler_id < 0)
>   		return ret;
> @@ -4809,6 +4814,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",
> @@ -12758,11 +12764,12 @@ 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)
this line should be aligned to the '(' from above line
>   {
>   	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;
> @@ -12784,8 +12791,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;
>   }
> @@ -12807,7 +12816,11 @@ intel_check_primary_plane(struct intel_plane *plane,
>   		/* 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);
> +			max_scale = skl_max_scale(to_intel_crtc(crtc),
> +						crtc_state,
> +						state->base.fb ?
> +						state->base.fb->format->format :
> +						0);
All these parameters should be aligned to the first opening brace '('
>   		}
>   		can_position = true;
>   	}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d4c027a..111f1a4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1580,7 +1580,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);
Align to above '('
>   
>   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 9e31be2..f2e144b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -897,7 +897,8 @@ intel_check_sprite_plane(struct intel_plane *plane,
>   		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,
> +						fb->format->format);
Alignment
>   		} else {
>   			can_scale = 0;
>   			min_scale = DRM_PLANE_HELPER_NO_SCALING;
- Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/16] drm/i915: Add NV12 as supported format for primary plane

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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.
>
> Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 74757d0..24248c7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = {
>   	DRM_FORMAT_YVYU,
>   	DRM_FORMAT_UYVY,
>   	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
>   };
>   
>   static const uint64_t skl_format_modifiers_noccs[] = {
> @@ -13233,6 +13234,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>   		intel_primary_formats = skl_primary_formats;
>   		num_formats = ARRAY_SIZE(skl_primary_formats);
>   
> +		if (INTEL_GEN(dev_priv) == 9 && (pipe == PIPE_C))
> +			num_formats -= 1;
> +
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>   		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
>   			modifiers = skl_format_modifiers_ccs;
>   		else

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

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

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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.
>
> Tested-by: Clinton Taylor <clinton.a.taylor@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 | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index f2e144b..f359b22 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1168,6 +1168,7 @@ static uint32_t skl_plane_formats[] = {
>   	DRM_FORMAT_YVYU,
>   	DRM_FORMAT_UYVY,
>   	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
>   };
>   
>   static const uint64_t skl_plane_format_modifiers_noccs[] = {
> @@ -1366,6 +1367,10 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>   		plane_formats = skl_plane_formats;
>   		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
>   
> +		if (INTEL_GEN(dev_priv) <= 10 && ((plane != 0) ||
braces for (plane != 0) not required, also does this means GLK/CNL can't 
support NV12 on sprite planes ?
> +			(pipe == PIPE_C)))
braces for (pipe == PIPE_C) not required
> +			num_plane_formats -= 1;
> +
>   		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
>   			modifiers = skl_plane_format_modifiers_ccs;
>   		else

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

Re: [PATCH 15/16] drm/i915: Add NV12 support to intel_framebuffer_init

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> 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
>
> Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 24248c7..b4a5e05 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14040,6 +14040,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>   			goto err;
>   		}
>   		break;
> +	case DRM_FORMAT_NV12:
> +		if (INTEL_GEN(dev_priv) < 9) {
> +			DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> +		      drm_get_format_name(mode_cmd->pixel_format,
alignment
> +				&format_name));
same here
> +			goto err;
> +		}
> +		break;
>   	default:
>   		DRM_DEBUG_KMS("unsupported pixel format: %s\n",
>   			      drm_get_format_name(mode_cmd->pixel_format, &format_name));

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

Re: [PATCH 16/16] drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> If the fb format is YUV, enable the plane CSC mode bits
> for the conversion.
>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h      | 6 ++++++
>   drivers/gpu/drm/i915/intel_display.c | 2 ++
>   2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 18be7be..f813406 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6426,6 +6426,12 @@ enum {
>   #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
>   #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30)
>   #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23)
> +#define PLANE_COLOR_CSC_MASK	(0x7 << 17)
The bit definition should be aligned to previous definition, with once 
space offset. Applies to all bits.
> +#define PLANE_COLOR_CSC_MODE_BYPASS	(0 << 17)
> +#define PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709	(1 << 17)
> +#define PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709	(2 << 17)
> +#define PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020	(3 << 17)
> +#define PLANE_COLOR_CSC_MODE_RGB709_TO_RGB2020	(4 << 17)
>   #define   PLANE_COLOR_PLANE_GAMMA_DISABLE	(1 << 13)
>   #define   PLANE_COLOR_ALPHA_MASK		(0x3 << 4)
>   #define   PLANE_COLOR_ALPHA_DISABLE		(0 << 4)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b4a5e05..d34adda 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3577,6 +3577,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>   	plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
>   	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
>   	plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
> +	if (intel_format_is_yuv(fb->format->format))
> +		plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
I guess this is with the assumption that both the gamut are in 709 
format, need not to be. But this should be taken care by plane CSC 
property patch, so ok for now.
- Shashank
>   
>   	return plane_color_ctl;
>   }

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

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

[Srinivas, Vidya]

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 4:21 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Konduru, Chandra <chandra.konduru@intel.com>; Maiti, Nabendu Bikash
> <nabendu.bikash.maiti@intel.com>
> Subject: Re: [PATCH 14/16] drm/i915: Add NV12 as supported format for
> sprite plane
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> > 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.
> >
> > Tested-by: Clinton Taylor <clinton.a.taylor@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 | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index f2e144b..f359b22 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1168,6 +1168,7 @@ static uint32_t skl_plane_formats[] = {
> >   	DRM_FORMAT_YVYU,
> >   	DRM_FORMAT_UYVY,
> >   	DRM_FORMAT_VYUY,
> > +	DRM_FORMAT_NV12,
> >   };
> >
> >   static const uint64_t skl_plane_format_modifiers_noccs[] = { @@
> > -1366,6 +1367,10 @@ intel_sprite_plane_create(struct drm_i915_private
> *dev_priv,
> >   		plane_formats = skl_plane_formats;
> >   		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> >
> > +		if (INTEL_GEN(dev_priv) <= 10 && ((plane != 0) ||
> braces for (plane != 0) not required, also does this means GLK/CNL can't
> support NV12 on sprite planes ?

Gen9 and Gen10 support NV12 only on primary and the first sprite plane.
Regarding braces - will fix it. Thank u

> > +			(pipe == PIPE_C)))
> braces for (pipe == PIPE_C) not required
> > +			num_plane_formats -= 1;
> > +
> >   		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 +
> plane))
> >   			modifiers = skl_plane_format_modifiers_ccs;
> >   		else

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

Re: [PATCH 12/16] drm/i915: Upscale scaler max scale for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 3:16 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Konduru, Chandra <chandra.konduru@intel.com>; Maiti, Nabendu Bikash
> <nabendu.bikash.maiti@intel.com>
> Subject: Re: [PATCH 12/16] drm/i915: Upscale scaler max scale for NV12
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> > 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)
> >
> > Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 33 +++++++++++++++++++++++---
> -------
> >   drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
> >   drivers/gpu/drm/i915/intel_sprite.c  |  3 ++-
> >   3 files changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 5fc9255..74757d0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3435,6 +3435,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);
> >   	}
> > @@ -4658,7 +4660,8 @@ 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,
> > +		  uint32_t pixel_format)
> >   {
> >   	struct intel_crtc_scaler_state *scaler_state =
> >   		&crtc_state->scaler_state;
> > @@ -4674,7 +4677,8 @@ skl_update_scaler(struct intel_crtc_state
> *crtc_state, bool force_detach,
> >   	 * the 90/270 degree plane rotation cases (to match the
> >   	 * GTT mapping), hence no need to account for rotation here.
> >   	 */
> > -	need_scaling = src_w != dst_w || src_h != dst_h;
> > +	need_scaling = src_w != dst_w || src_h != dst_h ||
> > +				   (pixel_format == DRM_FORMAT_NV12);
> This line needs to be aligned to above line, after = , also no need for braces
> >
> >   	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
> >   		need_scaling = true;
> > @@ -4753,7 +4757,7 @@ int skl_update_scaler_crtc(struct
> intel_crtc_state *state)
> >   	return skl_update_scaler(state, !state->base.active,
> SKL_CRTC_INDEX,
> >   		&state->scaler_state.scaler_id,
> >   		state->pipe_src_w, state->pipe_src_h,
> > -		adjusted_mode->crtc_hdisplay, adjusted_mode-
> >crtc_vdisplay);
> > +		adjusted_mode->crtc_hdisplay, adjusted_mode-
> >crtc_vdisplay, 0);
> Why 0 ? shouldn't we send actual current pixel format ? Or if you are just
> looking for pixel format is NV12 or not, you can make this variable a bool
> and name it is_nv12 or is_yuv420
> >   }

Will address this. Thank u.

> >
> >   /**
> > @@ -4783,7 +4787,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 ? fb->format->format : 0);
> Again, sending 0 doesn't look logical to me
> >
> >   	if (ret || plane_state->scaler_id < 0)
> >   		return ret;
> > @@ -4809,6 +4814,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", @@ -12758,11 +12764,12 @@ 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)
> this line should be aligned to the '(' from above line
> >   {
> >   	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; @@ -12784,8
> +12791,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;
> >   }
> > @@ -12807,7 +12816,11 @@ intel_check_primary_plane(struct
> intel_plane *plane,
> >   		/* 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);
> > +			max_scale = skl_max_scale(to_intel_crtc(crtc),
> > +						crtc_state,
> > +						state->base.fb ?
> > +						state->base.fb->format-
> >format :
> > +						0);
> All these parameters should be aligned to the first opening brace '('
> >   		}
> >   		can_position = true;
> >   	}
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index d4c027a..111f1a4 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1580,7 +1580,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);
> Align to above '('
> >
> >   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 9e31be2..f2e144b 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -897,7 +897,8 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >   		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,
> > +						fb->format->format);
> Alignment
Sorry about all the alignment issues. Looks like my editor had some problem
Will fix them. Thank u.
> >   		} else {
> >   			can_scale = 0;
> >   			min_scale = DRM_PLANE_HELPER_NO_SCALING;
> - Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/16] drm/i915: Set scaler mode for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 2:35 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Konduru, Chandra <chandra.konduru@intel.com>; Maiti, Nabendu Bikash
> <nabendu.bikash.maiti@intel.com>
> Subject: Re: [PATCH 10/16] drm/i915: Set scaler mode for NV12
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> > 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.
> I am not sure if the same code is applicable for all GEN > 9, I am seeing a
> different bit definition for GEN > 10 for bit 29:28
> > Comments under v8 is not valid anymore.
> >
> > Tested-by: Clinton Taylor <clinton.a.taylor@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     | 1 +
> >   drivers/gpu/drm/i915/intel_atomic.c | 8 ++++++--
> >   2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index e9c79b5..18be7be 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6705,6 +6705,7 @@ enum {
> >   #define PS_SCALER_MODE_MASK (3 << 28)
> >   #define PS_SCALER_MODE_DYN  (0 << 28)
> >   #define PS_SCALER_MODE_HQ  (1 << 28)
> > +#define PS_SCALER_MODE_NV12 (2 << 28)
> >   #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 d452c32..196427a 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -327,8 +327,12 @@ 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) &&
> Same here, this might not be applicable for (GEN > 10) due to different bit
> definition of 29:28.

Thank u. Will recheck and limit to Gen 9 and 10.

> > +			plane_state && plane_state->base.fb &&
> > +			plane_state->base.fb->format->format ==
> > +			DRM_FORMAT_NV12) {
> Above alignment should be aligned to first line (INTEL_GEN())
> > +			scaler_state->scalers[*scaler_id].mode =
> > +				PS_SCALER_MODE_NV12;
> >   		} else if (num_scalers_need == 1 && intel_crtc->pipe !=
> PIPE_C) {
> >   			/*
> >   			 * when only 1 scaler is in use on either pipe A or B,

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

Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc

[Srinivas, Vidya]

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



From: Sharma, Shashank
Sent: Thursday, February 8, 2018 12:17 PM
To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org
Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>; Kumar, Mahesh1 <mahesh1.kumar@intel.com>
Subject: Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc


Regards

Shashank

On 2/8/2018 10:02 AM, Sharma, Shashank wrote:


On 2/8/2018 8:50 AM, Srinivas, Vidya wrote:



-----Original Message-----
From: Sharma, Shashank
Sent: Wednesday, February 7, 2018 9:22 PM
To: Srinivas, Vidya <vidya.srinivas@intel.com><mailto:vidya.srinivas@intel.com>; intel-
gfx@lists.freedesktop.org<mailto:gfx@lists.freedesktop.org>
Cc: maarten.lankhorst@linux.intel.com<mailto:maarten.lankhorst@linux.intel.com>; Kamath, Sunil
<sunil.kamath@intel.com><mailto:sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com><mailto:uma.shankar@intel.com>;
Kumar, Mahesh1 <mahesh1.kumar@intel.com><mailto:mahesh1.kumar@intel.com>
Subject: Re: [PATCH 03/16] drm/i915/skl+: add NV12 in
skl_format_to_fourcc

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:

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

Add support of recognizing DRM_FORMAT_NV12 from plane_format
register

value.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com><mailto: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 60ba5bb..e3a6a7f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2626,6 +2626,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;
I dont think this is correct, the case PLANE_CTL_FORMAT_NV12 is defined as
(1 << 24) but when I check bspec definition, 24th bit is set for
P010/12/16 formats. AFAIK NV12 is 8 bit format whereas P0xx formats are
10/12/16 bit formats (they both are YCBCR 4:2:0 of course). This means we
have mixed NV12 format with P0xx formats. When I checked the definition
of DRM_FORMAT_NV12, I am not sure if that's intended for this. Ville, I saw
that the DRM_FORMAT_NV12 definition was added by you, can you please
comment if this is the right usage ?
Upto Gen10 24-27 bits of PLANE_CTL will be used for format. ICL onwards 23rd bit
is also used. PLANE_CTL_FORMAT_MASK has been defined in i915_reg.h
and mapping will be same if 23rd bit is 0. For NV12, 1<< 24 thus holds good
for all Gen.
That's not my point. What I want to say is, as per bspec (1 << 24) is for P010/P012/P016 formats, not NV12. NV12 is 8 bit YCBCR 4:2:0 format whereas P010/012/016 are 10,12 and 16 bit YCBCR 4:2:0 formats. So I was not sure if that should be called NV12 and hence I was not sure if we should return DRM_FORMAT_NV12 for the same.
- Shashank
What I mean to say here is, the check (1 << 24) is not good enough for all NV12, as you have to use it as if (mask == YCBCR_420_FORMAT_NV12) not (mask & YCBCR_420_FORMAT_NV12).
But if that's how its intended to be used for future addition of P010/012/016,  then I guess you can bypass this comment, but please make sure you differentiate NV12 with those formats.
- Shashank

Sure thank u, will take care when P0xx gets added.
       default:
       case PLANE_CTL_FORMAT_XRGB_8888:
           if (rgb_order) {



[-- Attachment #1.2: Type: text/html, Size: 8859 bytes --]

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

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

Re: [PATCH 12/16] drm/i915: Upscale scaler max scale for NV12

[Srinivas, Vidya]

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 3:16 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Konduru, Chandra <chandra.konduru@intel.com>; Maiti, Nabendu Bikash
> <nabendu.bikash.maiti@intel.com>
> Subject: Re: [PATCH 12/16] drm/i915: Upscale scaler max scale for NV12
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
> > 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)
> >
> > Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 33 +++++++++++++++++++++++---
> -------
> >   drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
> >   drivers/gpu/drm/i915/intel_sprite.c  |  3 ++-
> >   3 files changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 5fc9255..74757d0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3435,6 +3435,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);
> >   	}
> > @@ -4658,7 +4660,8 @@ 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,
> > +		  uint32_t pixel_format)
> >   {
> >   	struct intel_crtc_scaler_state *scaler_state =
> >   		&crtc_state->scaler_state;
> > @@ -4674,7 +4677,8 @@ skl_update_scaler(struct intel_crtc_state
> *crtc_state, bool force_detach,
> >   	 * the 90/270 degree plane rotation cases (to match the
> >   	 * GTT mapping), hence no need to account for rotation here.
> >   	 */
> > -	need_scaling = src_w != dst_w || src_h != dst_h;
> > +	need_scaling = src_w != dst_w || src_h != dst_h ||
> > +				   (pixel_format == DRM_FORMAT_NV12);
> This line needs to be aligned to above line, after = , also no need for braces
> >
> >   	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
> >   		need_scaling = true;
> > @@ -4753,7 +4757,7 @@ int skl_update_scaler_crtc(struct
> intel_crtc_state *state)
> >   	return skl_update_scaler(state, !state->base.active,
> SKL_CRTC_INDEX,
> >   		&state->scaler_state.scaler_id,
> >   		state->pipe_src_w, state->pipe_src_h,
> > -		adjusted_mode->crtc_hdisplay, adjusted_mode-
> >crtc_vdisplay);
> > +		adjusted_mode->crtc_hdisplay, adjusted_mode-
> >crtc_vdisplay, 0);
> Why 0 ? shouldn't we send actual current pixel format ? Or if you are just
> looking for pixel format is NV12 or not, you can make this variable a bool
> and name it is_nv12 or is_yuv420
> >   }
Thank you. We don't check for pixel format for update scaler during crtc check..
We need it only for plane check. we don't want to derive the fb and its format
here. So 0 was passed. Will see if I can modify to make more sense here.
> >
> >   /**
> > @@ -4783,7 +4787,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 ? fb->format->format : 0);
> Again, sending 0 doesn't look logical to me
Thank you. Agree, will change this.
> >
> >   	if (ret || plane_state->scaler_id < 0)
> >   		return ret;
> > @@ -4809,6 +4814,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", @@ -12758,11 +12764,12 @@ 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)
> this line should be aligned to the '(' from above line
> >   {
> >   	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; @@ -12784,8
> +12791,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;
> >   }
> > @@ -12807,7 +12816,11 @@ intel_check_primary_plane(struct
> intel_plane *plane,
> >   		/* 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);
> > +			max_scale = skl_max_scale(to_intel_crtc(crtc),
> > +						crtc_state,
> > +						state->base.fb ?
> > +						state->base.fb->format-
> >format :
> > +						0);
> All these parameters should be aligned to the first opening brace '('
> >   		}
> >   		can_position = true;
> >   	}
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index d4c027a..111f1a4 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1580,7 +1580,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);
> Align to above '('
> >
> >   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 9e31be2..f2e144b 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -897,7 +897,8 @@ intel_check_sprite_plane(struct intel_plane
> *plane,
> >   		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,
> > +						fb->format->format);
> Alignment
> >   		} else {
> >   			can_scale = 0;
> >   			min_scale = DRM_PLANE_HELPER_NO_SCALING;
> - Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/16] drm/i915/skl+: NV12 related changes for WM

[Kumar, Mahesh]

Hi,


On 2/7/2018 10:12 PM, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 2/6/2018 6:28 PM, Vidya Srinivas wrote:
>> 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.
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@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  | 54 
>> ++++++++++++++++++++++++++++++++--------
>>   3 files changed, 45 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index cca5414..778dc5a 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1459,6 +1459,7 @@ struct skl_wm_level {
>>   struct skl_wm_params {
>>       bool x_tiled, y_tiled;
>>       bool rc_surface;
>> +    bool is_nv12;
>>       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 ed33840..65dac21 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -594,6 +594,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_nv12;
>>   };
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index 4c9a811..b3d1bf7 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4421,7 +4421,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_num)
> Is this plane_num intended for 'no of planes' or 'plane number' ? from 
> the usage below looks like 'no of planes' but I could be wrong.
it's plane-number.
>>   {
>>       struct intel_plane *plane = 
>> to_intel_plane(intel_pstate->base.plane);
>>       const struct drm_plane_state *pstate = &intel_pstate->base;
>> @@ -4434,6 +4434,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_num == 1 && fb->format->format != DRM_FORMAT_NV12) {
>> +        DRM_DEBUG_KMS("Non NV12 format have single plane\n");
>> +        return -EINVAL;
>> +    }
> The comment says only NV12 format can have two planes, but if format 
> != NV12 && plane_num == 1, why is this an error ? Also the debug 
> message says "Non NV12 format have single plane". Shouldn't it ?
> Looks like you are expecting plane_number = 0, 1 for NV12 planes, and 
> when plane_num is non-zero, you expect it to be NV12 only ?
yes, non-planar formats will have single plane, that's why if format != 
NV12 & plane_num == 1 it's an error.
>
>> +
>>       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 ||
>> @@ -4441,6 +4447,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_nv12 = fb->format->format == DRM_FORMAT_NV12;
>>         if (plane->id == PLANE_CURSOR) {
>>           wp->width = intel_pstate->base.crtc_w;
>> @@ -4453,7 +4460,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_num == 1 && wp->is_nv12)
>> +        wp->width /= 2;
>> +
>> +    wp->cpp = fb->format->cpp[plane_num];
>>       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_num)
> Same question here
>>   {
>>       struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
>>       struct drm_plane *plane = intel_pstate->base.plane;
>> @@ -4657,15 +4668,20 @@ 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 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]);
>> +    if (plane_num == 0)
>> +        ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
>> +    else
>> +        ddb_blocks = 
>> skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]);
>>         for (level = 0; level <= max_level; level++) {
>> -        struct skl_wm_level *result = &wm->wm[level];
>> +        struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
>> +                              &wm->wm[level];
> It would look neat if both above condition as well as this one follows 
> the same pattern, if() else() Or (cond)? () : ().
agree.
>>             ret = skl_compute_plane_wm(dev_priv,
>>                          cstate,
>> @@ -4680,9 +4696,6 @@ 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_nv12 = true;
>> -
>>       return 0;
>>   }
>>   @@ -4791,20 +4804,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);
> This is confusing that you refer to 0 as plane_num = 1 and 1 as 
> plane_num = 2.
index start from zero for plane_num as well :), same thing is used in 
drm_fourcc.h file while defining planar formats.

-Mahesh
>>           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 */
>> +        if (wm_params.is_nv12) {
>> +            memset(&wm_params, 0, sizeof(struct skl_wm_params));
>> +            wm->is_nv12 = true;
>> +
>> +            ret = skl_compute_plane_wm_params(dev_priv, cstate,
>> +                              intel_pstate,
>> +                              &wm_params, 1);
> Same here, there should be a better way to do this.
>
> - Shashank
>> +            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;
>

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

Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc

[Srinivas, Vidya]

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



From: Sharma, Shashank
Sent: Thursday, February 8, 2018 12:17 PM
To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org
Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>; Kumar, Mahesh1 <mahesh1.kumar@intel.com>
Subject: Re: [PATCH 03/16] drm/i915/skl+: add NV12 in skl_format_to_fourcc


Regards

Shashank

On 2/8/2018 10:02 AM, Sharma, Shashank wrote:


On 2/8/2018 8:50 AM, Srinivas, Vidya wrote:



-----Original Message-----
From: Sharma, Shashank
Sent: Wednesday, February 7, 2018 9:22 PM
To: Srinivas, Vidya <vidya.srinivas@intel.com><mailto:vidya.srinivas@intel.com>; intel-
gfx@lists.freedesktop.org<mailto:gfx@lists.freedesktop.org>
Cc: maarten.lankhorst@linux.intel.com<mailto:maarten.lankhorst@linux.intel.com>; Kamath, Sunil
<sunil.kamath@intel.com><mailto:sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com><mailto:uma.shankar@intel.com>;
Kumar, Mahesh1 <mahesh1.kumar@intel.com><mailto:mahesh1.kumar@intel.com>
Subject: Re: [PATCH 03/16] drm/i915/skl+: add NV12 in
skl_format_to_fourcc

Regards

Shashank


On 2/6/2018 6:28 PM, Vidya Srinivas wrote:

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

Add support of recognizing DRM_FORMAT_NV12 from plane_format
register

value.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com><mailto: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 60ba5bb..e3a6a7f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2626,6 +2626,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;
I dont think this is correct, the case PLANE_CTL_FORMAT_NV12 is defined as
(1 << 24) but when I check bspec definition, 24th bit is set for
P010/12/16 formats. AFAIK NV12 is 8 bit format whereas P0xx formats are
10/12/16 bit formats (they both are YCBCR 4:2:0 of course). This means we
have mixed NV12 format with P0xx formats. When I checked the definition
of DRM_FORMAT_NV12, I am not sure if that's intended for this. Ville, I saw
that the DRM_FORMAT_NV12 definition was added by you, can you please
comment if this is the right usage ?
Upto Gen10 24-27 bits of PLANE_CTL will be used for format. ICL onwards 23rd bit
is also used. PLANE_CTL_FORMAT_MASK has been defined in i915_reg.h
and mapping will be same if 23rd bit is 0. For NV12, 1<< 24 thus holds good
for all Gen.
That's not my point. What I want to say is, as per bspec (1 << 24) is for P010/P012/P016 formats, not NV12. NV12 is 8 bit YCBCR 4:2:0 format whereas P010/012/016 are 10,12 and 16 bit YCBCR 4:2:0 formats. So I was not sure if that should be called NV12 and hence I was not sure if we should return DRM_FORMAT_NV12 for the same.
- Shashank
What I mean to say here is, the check (1 << 24) is not good enough for all NV12, as you have to use it as if (mask == YCBCR_420_FORMAT_NV12) not (mask & YCBCR_420_FORMAT_NV12).
But if that's how its intended to be used for future addition of P010/012/016,  then I guess you can bypass this comment, but please make sure you differentiate NV12 with those formats.
- Shashank

Thank you. For now, I will keep the patch as is. Will take care of your comment when we add P0xx.

Regards
Vidya
       default:
       case PLANE_CTL_FORMAT_XRGB_8888:
           if (rgb_order) {



[-- Attachment #1.2: Type: text/html, Size: 9143 bytes --]

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

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

[PATCH 05/16] 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

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 81ab25d..c64c846 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1471,6 +1471,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 d5a6242..d2f58cc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -597,6 +597,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 1095e20..1545ead 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 05/16] 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

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 8967cc1..590e5f5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1464,6 +1464,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 f13decc..6402d6d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -594,6 +594,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 3eeb66d..bfe64a9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4422,7 +4422,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;
@@ -4435,6 +4435,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 ||
@@ -4442,6 +4448,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;
@@ -4454,7 +4461,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);
 
@@ -4652,7 +4662,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;
@@ -4660,15 +4671,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,
@@ -4794,20 +4809,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

Re: [PATCH 05/16] drm/i915/skl+: NV12 related changes for WM

[Sharma, Shashank]

Ideal case would have been an enum for nv12 planes with y and uv 
parameters, to indicate in code, but I will at least prefer the 
plane_num variable should be replaced by plane_id, as plane_number 0 and 
1 confuses.

With that change, feel free to use:

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

Regards
Shashank
On 2/14/2018 10:27 AM, Vidya Srinivas wrote:
> 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.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@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  | 55 +++++++++++++++++++++++++++++++---------
>   3 files changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b9914bb..9a9a645 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1463,6 +1463,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 f13decc..6402d6d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -594,6 +594,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 bc0e2bf..2a0cc0b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4422,7 +4422,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_num)
>   {
>   	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
>   	const struct drm_plane_state *pstate = &intel_pstate->base;
> @@ -4435,6 +4435,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_num == 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 ||
> @@ -4442,6 +4448,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;
> @@ -4454,7 +4461,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_num == 1 && wp->is_planar)
> +		wp->width /= 2;
> +
> +	wp->cpp = fb->format->cpp[plane_num];
>   	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
>   							     intel_pstate);
>   
> @@ -4652,7 +4662,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_num)
>   {
>   	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
>   	struct drm_plane *plane = intel_pstate->base.plane;
> @@ -4660,15 +4671,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 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_num ?
> +		     skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]) :
> +		     skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
>   
>   	for (level = 0; level <= max_level; level++) {
> -		struct skl_wm_level *result = &wm->wm[level];
> +		struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
> +							  &wm->wm[level];
>   
>   		ret = skl_compute_plane_wm(dev_priv,
>   					   cstate,
> @@ -4683,9 +4698,6 @@ 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_nv12 = true;
> -
>   	return 0;
>   }
>   
> @@ -4794,20 +4806,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;
> @@ -4862,7 +4893,7 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
>   		return skl_ddb_entry_write(dev_priv,
>   					   PLANE_BUF_CFG(pipe, plane_id),
>   					   &ddb->plane[pipe][plane_id]);
> -	if (wm->is_nv12) {
> +	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,

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

[PATCH 05/16] 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.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@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  | 55 +++++++++++++++++++++++++++++++---------
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b9914bb..9a9a645 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1463,6 +1463,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 f13decc..6402d6d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -594,6 +594,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 bc0e2bf..2a0cc0b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4422,7 +4422,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_num)
 {
 	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
 	const struct drm_plane_state *pstate = &intel_pstate->base;
@@ -4435,6 +4435,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_num == 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 ||
@@ -4442,6 +4448,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;
@@ -4454,7 +4461,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_num == 1 && wp->is_planar)
+		wp->width /= 2;
+
+	wp->cpp = fb->format->cpp[plane_num];
 	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
 							     intel_pstate);
 
@@ -4652,7 +4662,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_num)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_plane *plane = intel_pstate->base.plane;
@@ -4660,15 +4671,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 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_num ?
+		     skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]) :
+		     skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
 
 	for (level = 0; level <= max_level; level++) {
-		struct skl_wm_level *result = &wm->wm[level];
+		struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
+							  &wm->wm[level];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4683,9 +4698,6 @@ 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_nv12 = true;
-
 	return 0;
 }
 
@@ -4794,20 +4806,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;
@@ -4862,7 +4893,7 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
 		return skl_ddb_entry_write(dev_priv,
 					   PLANE_BUF_CFG(pipe, plane_id),
 					   &ddb->plane[pipe][plane_id]);
-	if (wm->is_nv12) {
+	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,
-- 
2.7.4

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

[PATCH 05/16] 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.

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  | 54 +++++++++++++++++++++++++++++++---------
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cb339ec..b8e064c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1461,6 +1461,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 4ebafdb..1f9fa87 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -594,6 +594,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 c525652..a77d572 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4422,7 +4422,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_num)
 {
 	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
 	const struct drm_plane_state *pstate = &intel_pstate->base;
@@ -4435,6 +4435,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_num == 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 ||
@@ -4442,6 +4448,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;
@@ -4454,7 +4461,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_num == 1 && wp->is_planar)
+		wp->width /= 2;
+
+	wp->cpp = fb->format->cpp[plane_num];
 	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
 							     intel_pstate);
 
@@ -4652,7 +4662,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_num)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_plane *plane = intel_pstate->base.plane;
@@ -4660,15 +4671,18 @@ 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 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_num ? skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]) :
+				 skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
 
 	for (level = 0; level <= max_level; level++) {
-		struct skl_wm_level *result = &wm->wm[level];
+		struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
+							  &wm->wm[level];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4683,9 +4697,6 @@ 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_nv12 = true;
-
 	return 0;
 }
 
@@ -4794,20 +4805,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;
@@ -4862,7 +4892,7 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
 		return skl_ddb_entry_write(dev_priv,
 					   PLANE_BUF_CFG(pipe, plane_id),
 					   &ddb->plane[pipe][plane_id]);
-	if (wm->is_nv12) {
+	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,
-- 
2.7.4

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

[PATCH 05/16] 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.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@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  | 54 ++++++++++++++++++++++++++++++++--------
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cca5414..778dc5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1459,6 +1459,7 @@ struct skl_wm_level {
 struct skl_wm_params {
 	bool x_tiled, y_tiled;
 	bool rc_surface;
+	bool is_nv12;
 	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 ed33840..65dac21 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -594,6 +594,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_nv12;
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c9a811..b3d1bf7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4421,7 +4421,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_num)
 {
 	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
 	const struct drm_plane_state *pstate = &intel_pstate->base;
@@ -4434,6 +4434,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_num == 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 ||
@@ -4441,6 +4447,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_nv12 = fb->format->format == DRM_FORMAT_NV12;
 
 	if (plane->id == PLANE_CURSOR) {
 		wp->width = intel_pstate->base.crtc_w;
@@ -4453,7 +4460,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_num == 1 && wp->is_nv12)
+		wp->width /= 2;
+
+	wp->cpp = fb->format->cpp[plane_num];
 	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_num)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_plane *plane = intel_pstate->base.plane;
@@ -4657,15 +4668,20 @@ 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 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]);
+	if (plane_num == 0)
+		ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
+	else
+		ddb_blocks = skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]);
 
 	for (level = 0; level <= max_level; level++) {
-		struct skl_wm_level *result = &wm->wm[level];
+		struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
+							  &wm->wm[level];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4680,9 +4696,6 @@ 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_nv12 = true;
-
 	return 0;
 }
 
@@ -4791,20 +4804,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 */
+		if (wm_params.is_nv12) {
+			memset(&wm_params, 0, sizeof(struct skl_wm_params));
+			wm->is_nv12 = 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 05/16] 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.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@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  | 54 ++++++++++++++++++++++++++++++++--------
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index df91904..c7abe91 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1452,6 +1452,7 @@ struct skl_wm_level {
 struct skl_wm_params {
 	bool x_tiled, y_tiled;
 	bool rc_surface;
+	bool is_nv12;
 	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 8473434..7644ca7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -591,6 +591,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_nv12;
 };
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e790679..f79787b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4413,7 +4413,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_num)
 {
 	struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
 	const struct drm_plane_state *pstate = &intel_pstate->base;
@@ -4426,6 +4426,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_num == 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 ||
@@ -4433,6 +4439,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_nv12 = fb->format->format == DRM_FORMAT_NV12;
 
 	if (plane->id == PLANE_CURSOR) {
 		wp->width = intel_pstate->base.crtc_w;
@@ -4445,7 +4452,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_num == 1 && wp->is_nv12)
+		wp->width /= 2;
+
+	wp->cpp = fb->format->cpp[plane_num];
 	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
 							     intel_pstate);
 
@@ -4606,7 +4616,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_num)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct drm_plane *plane = intel_pstate->base.plane;
@@ -4614,15 +4625,20 @@ 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 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]);
+	if (plane_num == 0)
+		ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]);
+	else
+		ddb_blocks = skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]);
 
 	for (level = 0; level <= max_level; level++) {
-		struct skl_wm_level *result = &wm->wm[level];
+		struct skl_wm_level *result = plane_num ? &wm->uv_wm[level] :
+							  &wm->wm[level];
 
 		ret = skl_compute_plane_wm(dev_priv,
 					   cstate,
@@ -4637,9 +4653,6 @@ 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_nv12 = true;
-
 	return 0;
 }
 
@@ -4748,20 +4761,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 */
+		if (wm_params.is_nv12) {
+			memset(&wm_params, 0, sizeof(struct skl_wm_params));
+			wm->is_nv12 = 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