[PATCH 0/8] Fixed16.16 wrapper cleanup & wm optimization

[PATCH 0/8] Fixed16.16 wrapper cleanup & wm optimization

[Mahesh Kumar]

This series Include patches for:
	clean fixed16.16 wrappers
	optimize wm calculation code
	enable/Implement trans wm calculation
	create a DebugFS entry for IPC status

Mahesh Kumar (8):
  drm/i915: Fixed point fixed16 wrapper cleanup
  drm/i915/skl+: Optimize WM calculation
  drm/i915/gen10: Calculate and enable transition WM
  drm/i915/glk: IPC linetime watermark workaround for GLK
  drm/i915/cnl: Extend WM workaround with IPC for CNL
  drm/i915/gen9+: Add has_ipc flag in device info structure
  drm/i915/bxt+: Enable IPC support
  drm/i915/skl+: debugfs entry to control IPC

 drivers/gpu/drm/i915/i915_debugfs.c  |  73 +++++++++-
 drivers/gpu/drm/i915/i915_drv.c      |   4 +-
 drivers/gpu/drm/i915/i915_drv.h      |  32 +++--
 drivers/gpu/drm/i915/i915_pci.c      |   4 +
 drivers/gpu/drm/i915/i915_reg.h      |   1 +
 drivers/gpu/drm/i915/intel_display.c |   1 +
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_pm.c      | 262 +++++++++++++++++++++++------------
 8 files changed, 283 insertions(+), 96 deletions(-)

-- 
2.13.0

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

[PATCH 1/8] drm/i915: Fixed point fixed16 wrapper cleanup

[Mahesh Kumar]

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

As per suggestion from Jani, cleanup the code. Cleanup includes
 - Instead of left shifting & check, compare with U32/16_MAX
 - Use typecast instead of clamp_t

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c6fab08a2e6..96edced67e10 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -126,7 +126,7 @@ static inline uint_fixed_16_16_t u32_to_fixed16(uint32_t val)
 {
 	uint_fixed_16_16_t fp;
 
-	WARN_ON(val >> 16);
+	WARN_ON(val > U16_MAX);
 
 	fp.val = val << 16;
 	return fp;
@@ -163,8 +163,8 @@ static inline uint_fixed_16_16_t max_fixed16(uint_fixed_16_16_t max1,
 static inline uint_fixed_16_16_t clamp_u64_to_fixed16(uint64_t val)
 {
 	uint_fixed_16_16_t fp;
-	WARN_ON(val >> 32);
-	fp.val = clamp_t(uint32_t, val, 0, ~0);
+	WARN_ON(val > U32_MAX);
+	fp.val = (uint32_t) val;
 	return fp;
 }
 
@@ -181,8 +181,8 @@ static inline uint32_t mul_round_up_u32_fixed16(uint32_t val,
 
 	intermediate_val = (uint64_t) val * mul.val;
 	intermediate_val = DIV_ROUND_UP_ULL(intermediate_val, 1 << 16);
-	WARN_ON(intermediate_val >> 32);
-	return clamp_t(uint32_t, intermediate_val, 0, ~0);
+	WARN_ON(intermediate_val > U32_MAX);
+	return (uint32_t) intermediate_val;
 }
 
 static inline uint_fixed_16_16_t mul_fixed16(uint_fixed_16_16_t val,
@@ -211,8 +211,8 @@ static inline uint32_t div_round_up_u32_fixed16(uint32_t val,
 
 	interm_val = (uint64_t)val << 16;
 	interm_val = DIV_ROUND_UP_ULL(interm_val, d.val);
-	WARN_ON(interm_val >> 32);
-	return clamp_t(uint32_t, interm_val, 0, ~0);
+	WARN_ON(interm_val > U32_MAX);
+	return (uint32_t) interm_val;
 }
 
 static inline uint_fixed_16_16_t mul_u32_fixed16(uint32_t val,
-- 
2.13.0

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

[PATCH 2/8] drm/i915/skl+: Optimize WM calculation

[Mahesh Kumar]

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

Plane configuration parameters doesn't change for each WM-level
calculation. Currently we compute same parameters 8 times for each
wm-level.
This patch optimizes it by calculating these parameters in beginning
& reuse during each level-wm calculation.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  13 +++
 drivers/gpu/drm/i915/intel_pm.c | 179 ++++++++++++++++++++++------------------
 2 files changed, 111 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 96edced67e10..62ba18d6717f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1790,6 +1790,19 @@ struct skl_wm_level {
 	uint8_t plane_res_l;
 };
 
+/* Stores plane specific WM parameters */
+struct skl_wm_params{
+	bool x_tiled, y_tiled;
+	uint32_t width;
+	uint8_t cpp;
+	uint32_t plane_pixel_rate;
+	uint32_t y_min_scanlines;
+	uint32_t plane_bytes_per_line;
+	uint_fixed_16_16_t plane_blocks_per_line;
+	uint_fixed_16_16_t y_tile_minimum;
+	uint32_t linetime_us;
+};
+
 /*
  * This struct helps tracking the state needed for runtime PM, which puts the
  * device in PCI D3 state. Notice that when this happens, nothing on the
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ee2a349cfe68..b2bd65847d9b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4341,128 +4341,135 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate,
 					    downscale_amount);
 }
 
-static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
-				struct intel_crtc_state *cstate,
-				const struct intel_plane_state *intel_pstate,
-				uint16_t ddb_allocation,
-				int level,
-				uint16_t *out_blocks, /* out */
-				uint8_t *out_lines, /* out */
-				bool *enabled /* out */)
+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 intel_plane *plane = to_intel_plane(intel_pstate->base.plane);
 	const struct drm_plane_state *pstate = &intel_pstate->base;
 	const struct drm_framebuffer *fb = pstate->fb;
-	uint32_t latency = dev_priv->wm.skl_latency[level];
-	uint_fixed_16_16_t method1, method2;
-	uint_fixed_16_16_t plane_blocks_per_line;
-	uint_fixed_16_16_t selected_result;
 	uint32_t interm_pbpl;
-	uint32_t plane_bytes_per_line;
-	uint32_t res_blocks, res_lines;
-	uint8_t cpp;
-	uint32_t width = 0;
-	uint32_t plane_pixel_rate;
-	uint_fixed_16_16_t y_tile_minimum;
-	uint32_t y_min_scanlines;
 	struct intel_atomic_state *state =
 		to_intel_atomic_state(cstate->base.state);
 	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
-	bool y_tiled, x_tiled;
 
-	if (latency == 0 ||
-	    !intel_wm_plane_visible(cstate, intel_pstate)) {
-		*enabled = false;
+	if (!intel_wm_plane_visible(cstate, intel_pstate))
 		return 0;
-	}
 
-	y_tiled = fb->modifier == I915_FORMAT_MOD_Y_TILED ||
-		  fb->modifier == I915_FORMAT_MOD_Yf_TILED;
-	x_tiled = fb->modifier == I915_FORMAT_MOD_X_TILED;
-
-	/* Display WA #1141: kbl,cfl */
-	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
-	    dev_priv->ipc_enabled)
-		latency += 4;
-
-	if (apply_memory_bw_wa && x_tiled)
-		latency += 15;
+	wp->y_tiled = fb->modifier == I915_FORMAT_MOD_Y_TILED ||
+		      fb->modifier == I915_FORMAT_MOD_Yf_TILED;
+	wp->x_tiled = fb->modifier == I915_FORMAT_MOD_X_TILED;
 
 	if (plane->id == PLANE_CURSOR) {
-		width = intel_pstate->base.crtc_w;
+		wp->width = intel_pstate->base.crtc_w;
 	} else {
 		/*
 		 * Src coordinates are already rotated by 270 degrees for
 		 * the 90/270 degree plane rotation cases (to match the
 		 * GTT mapping), hence no need to account for rotation here.
 		 */
-		width = drm_rect_width(&intel_pstate->base.src) >> 16;
+		wp->width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	}
 
-	cpp = (fb->format->format == DRM_FORMAT_NV12) ? fb->format->cpp[1] :
-							fb->format->cpp[0];
-	plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate, intel_pstate);
+	wp->cpp = (fb->format->format == DRM_FORMAT_NV12) ? fb->format->cpp[1] :
+							    fb->format->cpp[0];
+	wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
+							     intel_pstate);
 
 	if (drm_rotation_90_or_270(pstate->rotation)) {
-
-		switch (cpp) {
+		switch (wp->cpp) {
 		case 1:
-			y_min_scanlines = 16;
+			wp->y_min_scanlines = 16;
 			break;
 		case 2:
-			y_min_scanlines = 8;
+			wp->y_min_scanlines = 8;
 			break;
 		case 4:
-			y_min_scanlines = 4;
+			wp->y_min_scanlines = 4;
 			break;
 		default:
-			MISSING_CASE(cpp);
+			MISSING_CASE(wp->cpp);
 			return -EINVAL;
 		}
 	} else {
-		y_min_scanlines = 4;
+		wp->y_min_scanlines = 4;
 	}
 
 	if (apply_memory_bw_wa)
-		y_min_scanlines *= 2;
-
-	plane_bytes_per_line = width * cpp;
-	if (y_tiled) {
-		interm_pbpl = DIV_ROUND_UP(plane_bytes_per_line *
-					   y_min_scanlines, 512);
-		plane_blocks_per_line = div_fixed16(interm_pbpl,
-							y_min_scanlines);
-	} else if (x_tiled) {
-		interm_pbpl = DIV_ROUND_UP(plane_bytes_per_line, 512);
-		plane_blocks_per_line = u32_to_fixed16(interm_pbpl);
+		wp->y_min_scanlines *= 2;
+
+	wp->plane_bytes_per_line = wp->width * wp->cpp;
+	if (wp->y_tiled) {
+		interm_pbpl = DIV_ROUND_UP(wp->plane_bytes_per_line *
+					   wp->y_min_scanlines, 512);
+		wp->plane_blocks_per_line = div_fixed16(interm_pbpl,
+							wp->y_min_scanlines);
+	} else if (wp->x_tiled) {
+		interm_pbpl = DIV_ROUND_UP(wp->plane_bytes_per_line, 512);
+		wp->plane_blocks_per_line = u32_to_fixed16(interm_pbpl);
 	} else {
-		interm_pbpl = DIV_ROUND_UP(plane_bytes_per_line, 512) + 1;
-		plane_blocks_per_line = u32_to_fixed16(interm_pbpl);
+		interm_pbpl = DIV_ROUND_UP(wp->plane_bytes_per_line, 512) + 1;
+		wp->plane_blocks_per_line = u32_to_fixed16(interm_pbpl);
 	}
 
-	method1 = skl_wm_method1(plane_pixel_rate, cpp, latency);
-	method2 = skl_wm_method2(plane_pixel_rate,
+	wp->y_tile_minimum = mul_u32_fixed16(wp->y_min_scanlines,
+					     wp->plane_blocks_per_line);
+	wp->linetime_us = fixed16_to_u32_round_up(
+					intel_get_linetime_us(cstate));
+	return 0;
+}
+
+static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
+				struct intel_crtc_state *cstate,
+				const struct intel_plane_state *intel_pstate,
+				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 */)
+{
+	const struct drm_plane_state *pstate = &intel_pstate->base;
+	uint32_t latency = dev_priv->wm.skl_latency[level];
+	uint_fixed_16_16_t method1, method2;
+	uint_fixed_16_16_t selected_result;
+	uint32_t res_blocks, res_lines;
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(cstate->base.state);
+	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
+
+	if (latency == 0 ||
+	    !intel_wm_plane_visible(cstate, intel_pstate)) {
+		*enabled = false;
+		return 0;
+	}
+
+	/* Display WA #1141: kbl,cfl */
+	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
+	    dev_priv->ipc_enabled)
+		latency += 4;
+
+	if (apply_memory_bw_wa && wp->x_tiled)
+		latency += 15;
+
+	method1 = skl_wm_method1(wp->plane_pixel_rate, wp->cpp, latency);
+	method2 = skl_wm_method2(wp->plane_pixel_rate,
 				 cstate->base.adjusted_mode.crtc_htotal,
 				 latency,
-				 plane_blocks_per_line);
-
-	y_tile_minimum = mul_u32_fixed16(y_min_scanlines,
-					 plane_blocks_per_line);
+				 wp->plane_blocks_per_line);
 
-	if (y_tiled) {
-		selected_result = max_fixed16(method2, y_tile_minimum);
+	if (wp->y_tiled) {
+		selected_result = max_fixed16(method2, wp->y_tile_minimum);
 	} else {
-		uint32_t linetime_us;
-
-		linetime_us = fixed16_to_u32_round_up(
-				intel_get_linetime_us(cstate));
-		if ((cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1) &&
-		    (plane_bytes_per_line / 512 < 1))
+		if ((wp->cpp * cstate->base.adjusted_mode.crtc_htotal / 512 < 1)
+		    && (wp->plane_bytes_per_line / 512 < 1))
 			selected_result = method2;
 		else if ((ddb_allocation && ddb_allocation /
-			fixed16_to_u32_round_up(plane_blocks_per_line)) >= 1)
+			fixed16_to_u32_round_up(wp->plane_blocks_per_line)) >= 1)
 			selected_result = min_fixed16(method1, method2);
-		else if (latency >= linetime_us)
+		else if (latency >= wp->linetime_us)
 			selected_result = min_fixed16(method1, method2);
 		else
 			selected_result = method1;
@@ -4470,12 +4477,13 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 
 	res_blocks = fixed16_to_u32_round_up(selected_result) + 1;
 	res_lines = div_round_up_fixed16(selected_result,
-					 plane_blocks_per_line);
+					 wp->plane_blocks_per_line);
 
 	if (level >= 1 && level <= 7) {
-		if (y_tiled) {
-			res_blocks += fixed16_to_u32_round_up(y_tile_minimum);
-			res_lines += y_min_scanlines;
+		if (wp->y_tiled) {
+			res_blocks += fixed16_to_u32_round_up(
+							wp->y_tile_minimum);
+			res_lines += wp->y_min_scanlines;
 		} else {
 			res_blocks++;
 		}
@@ -4513,6 +4521,7 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 		      struct skl_ddb_allocation *ddb,
 		      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 intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
@@ -4536,6 +4545,7 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
 					   intel_pstate,
 					   ddb_blocks,
 					   level,
+					   wm_params,
 					   &result->plane_res_b,
 					   &result->plane_res_l,
 					   &result->plane_en);
@@ -4600,11 +4610,18 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
 		const struct intel_plane_state *intel_pstate =
 						to_intel_plane_state(pstate);
 		enum plane_id plane_id = to_intel_plane(plane)->id;
+		struct skl_wm_params wm_params;
 
 		wm = &pipe_wm->planes[plane_id];
+		memset(&wm_params, 0, sizeof(struct skl_wm_params));
+
+		ret = skl_compute_plane_wm_params(dev_priv, cstate,
+						  intel_pstate, &wm_params);
+		if (ret)
+			return ret;
 
 		ret = skl_compute_wm_levels(dev_priv, ddb, cstate,
-					    intel_pstate, wm);
+					    intel_pstate, &wm_params, wm);
 		if (ret)
 			return ret;
 		skl_compute_transition_wm(cstate, &wm->trans_wm);
-- 
2.13.0

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

[PATCH 3/8] drm/i915/gen10: Calculate and enable transition WM

[Mahesh Kumar]

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

GEN > 9 require transition WM to be programmed if IPC is enabled.
This patch calculates & enable transition WM for supported platforms.
If transition WM is enabled, Plane read requests are sent at high
priority until filling above the transition watermark, then the
requests are sent at lower priority until dropping below the level-0 WM.
The lower priority requests allow other memory clients to have better
memory access.

transition minimum is the minimum amount needed for trans_wm to work to
ensure  the demote does not happen before enough data has been read to
meet the level 0 watermark requirements.

transition amount is configurable value. Higher values will
tend to cause longer periods of high priority reads followed by longer
periods of lower priority reads. Tuning to lower values will tend to
cause shorter periods of high and lower priority reads.

Keeping transition amount to 10 in this patch, as suggested by HW team.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b2bd65847d9b..9a2ed1b734d5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4579,12 +4579,55 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate)
 }
 
 static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
+				      struct skl_wm_params *wp,
+				      struct skl_wm_level *wm_l0,
+				      uint16_t ddb_allocation,
 				      struct skl_wm_level *trans_wm /* out */)
 {
+	struct drm_device *dev = cstate->base.crtc->dev;
+	const struct drm_i915_private *dev_priv = to_i915(dev);
+	uint16_t trans_min, trans_y_tile_min;
+	uint16_t trans_amount = 10; /* This is configurable amount */
+	uint16_t trans_offset_b, res_blocks;
+
 	if (!cstate->base.active)
+		goto exit;
+
+	/* Transition WM are not recommended by HW team for GEN9 */
+	if (INTEL_GEN(dev_priv) <= 9)
+		goto exit;
+
+	/* Transition WM don't make any sense if ipc is disabled */
+	if (!dev_priv->ipc_enabled)
+		goto exit;
+
+	if (INTEL_GEN(dev_priv) >= 10)
+		trans_min = 4;
+
+	trans_offset_b = trans_min + trans_amount;
+	trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
+							wp->y_tile_minimum);
+
+	if (wp->y_tiled) {
+		res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
+				trans_offset_b;
+	} else {
+		res_blocks = wm_l0->plane_res_b + trans_offset_b;
+	}
+
+	res_blocks += 1;
+
+	/* WA BUG:1938466 add one block for non y-tile planes */
+	if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
+		res_blocks += 1;
+
+	if (res_blocks < ddb_allocation) {
+		trans_wm->plane_res_b = res_blocks;
+		trans_wm->plane_en = true;
 		return;
+	}
 
-	/* Until we know more, just disable transition WMs */
+exit:
 	trans_wm->plane_en = false;
 }
 
@@ -4611,8 +4654,11 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
 						to_intel_plane_state(pstate);
 		enum plane_id plane_id = to_intel_plane(plane)->id;
 		struct skl_wm_params wm_params;
+		enum pipe pipe = to_intel_crtc(cstate->base.crtc)->pipe;
+		uint16_t ddb_blocks;
 
 		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,
@@ -4624,7 +4670,8 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
 					    intel_pstate, &wm_params, wm);
 		if (ret)
 			return ret;
-		skl_compute_transition_wm(cstate, &wm->trans_wm);
+		skl_compute_transition_wm(cstate, &wm_params, &wm->wm[0],
+					  ddb_blocks, &wm->trans_wm);
 	}
 	pipe_wm->linetime = skl_compute_linetime_wm(cstate);
 
-- 
2.13.0

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

[PATCH 4/8] drm/i915/glk: IPC linetime watermark workaround for GLK

[Mahesh Kumar]

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

IF IPC is enabled LINETIME_WM value should be half of calculated value
 line time = ROUNDDOWN(1/2 * Calculated Line Time)

Earlier code was rounding-up the value, But updated Bspec says we should
take the ROUNDDOWN. This patch corrects that as well.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9a2ed1b734d5..dac0fcf8b54e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4571,9 +4571,10 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate)
 
 	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
 
-	/* Display WA #1135: bxt. */
-	if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled)
-		linetime_wm = DIV_ROUND_UP(linetime_wm, 2);
+	/* Display WA #1135: bxt:ALL GLK:ALL */
+	if ((IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv)) &&
+	    dev_priv->ipc_enabled)
+		linetime_wm /= 2;
 
 	return linetime_wm;
 }
-- 
2.13.0

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

[PATCH 5/8] drm/i915/cnl: Extend WM workaround with IPC for CNL

[Mahesh Kumar]

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

CNL:A & CNL:B have same workaround as KBL to increase wm level latency
by 4us if IPC is enabled.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dac0fcf8b54e..f2794d29f217 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4447,7 +4447,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	}
 
 	/* Display WA #1141: kbl,cfl */
-	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
+	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) ||
+	    IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) &&
 	    dev_priv->ipc_enabled)
 		latency += 4;
 
-- 
2.13.0

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

[PATCH 6/8] drm/i915/gen9+: Add has_ipc flag in device info structure

[Mahesh Kumar]

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

New Isochronous Priority Control (IPC) capability is introduced in newer
GEN platforms. This patch adds a device info flag to indicate if platform
supports IPC. Patch also sets this flag in supported platforms.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h | 5 ++++-
 drivers/gpu/drm/i915/i915_pci.c | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 62ba18d6717f..47745e3de841 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -784,7 +784,8 @@ struct intel_csr {
 	func(cursor_needs_physical); \
 	func(hws_needs_physical); \
 	func(overlay_needs_physical); \
-	func(supports_tv);
+	func(supports_tv); \
+	func(has_ipc);
 
 struct sseu_dev_info {
 	u8 slice_mask;
@@ -3011,6 +3012,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define HAS_RUNTIME_PM(dev_priv) ((dev_priv)->info.has_runtime_pm)
 #define HAS_64BIT_RELOC(dev_priv) ((dev_priv)->info.has_64bit_reloc)
 
+#define HAS_IPC(dev_priv)		 ((dev_priv)->info.has_ipc)
+
 /*
  * For now, anything with a GuC requires uCode loading, and then supports
  * command submission once loaded. But these are logically independent
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index a1e6b696bcfa..0dbdfe0f32a5 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -390,6 +390,7 @@ static const struct intel_device_info intel_skylake_gt3_info = {
 	.has_full_ppgtt = 1, \
 	.has_full_48bit_ppgtt = 1, \
 	.has_reset_engine = 1, \
+	.has_ipc = 1, \
 	GEN_DEFAULT_PIPEOFFSETS, \
 	IVB_CURSOR_OFFSETS, \
 	BDW_COLORS
@@ -413,6 +414,7 @@ static const struct intel_device_info intel_geminilake_info = {
 	.platform = INTEL_KABYLAKE, \
 	.has_csr = 1, \
 	.has_guc = 1, \
+	.has_ipc = 1, \
 	.ddb_size = 896
 
 static const struct intel_device_info intel_kabylake_info = {
@@ -431,6 +433,7 @@ static const struct intel_device_info intel_kabylake_gt3_info = {
 	.platform = INTEL_COFFEELAKE, \
 	.has_csr = 1, \
 	.has_guc = 1, \
+	.has_ipc = 1, \
 	.ddb_size = 896
 
 static const struct intel_device_info intel_coffeelake_info = {
@@ -449,6 +452,7 @@ static const struct intel_device_info intel_cannonlake_info = {
 	.gen = 10,
 	.ddb_size = 1024,
 	.has_csr = 1,
+	.has_ipc = 1,
 	.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
 };
 
-- 
2.13.0

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

[PATCH 7/8] drm/i915/bxt+: Enable IPC support

[Mahesh Kumar]

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

This patch adds IPC support. This patch also enables IPC in all supported
platforms based on has_ipc flag.
IPC (Isochronous Priority Control) is the hardware feature, which
dynamically controls the memory read priority of Display.

When IPC is enabled, plane read requests are sent at high priority until
filling above the transition watermark, then the requests are sent at
lower priority until dropping below the level 0 watermark.
The lower priority requests allow other memory clients to have better
memory access. When IPC is disabled, all plane read requests are sent at
high priority.

Changes since V1:
 - Remove commandline parameter to disable ipc
 - Address Paulo's comments
Changes since V2:
 - Address review comments
 - Set ipc_enabled flag
Changes since V3:
 - move ipc_enabled flag assignment inside intel_ipc_enable function
Changes since V4:
 - Re-enable IPC after suspend/resume
Changes since V5:
 - Enable IPC for all gen >=9 except SKL
Changes since V6:
 - fix commit msg
 - after resume program IPC based on SW state.
Changes since V7:
 - Modify IPC support check based on HAS_IPC macro (suggested by Chris)

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c      |  4 +++-
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_pm.c      | 24 ++++++++++++++++++++++++
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d310d8245dca..c69958b55ee2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1338,7 +1338,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	intel_runtime_pm_enable(dev_priv);
 
-	dev_priv->ipc_enabled = false;
+	intel_init_ipc(dev_priv);
 
 	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG))
 		DRM_INFO("DRM_I915_DEBUG enabled\n");
@@ -2602,6 +2602,8 @@ static int intel_runtime_resume(struct device *kdev)
 	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
 		intel_hpd_init(dev_priv);
 
+	intel_enable_ipc(dev_priv);
+
 	enable_rpm_wakeref_asserts(dev_priv);
 
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c712d01f92ab..4ceda0fc1baf 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6728,6 +6728,7 @@ enum {
 #define  DISP_FBC_WM_DIS		(1<<15)
 #define DISP_ARB_CTL2	_MMIO(0x45004)
 #define  DISP_DATA_PARTITION_5_6	(1<<6)
+#define  DISP_IPC_ENABLE		(1<<3)
 #define DBUF_CTL	_MMIO(0x45008)
 #define  DBUF_POWER_REQUEST		(1<<31)
 #define  DBUF_POWER_STATE		(1<<30)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0ed454a23e34..0a5d6ba64925 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15733,6 +15733,7 @@ void intel_display_resume(struct drm_device *dev)
 	if (!ret)
 		ret = __intel_display_resume(dev, state, &ctx);
 
+	intel_enable_ipc(dev_priv);
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0902d7cb48d9..7b0e834a61f9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1883,6 +1883,8 @@ bool ilk_disable_lp_wm(struct drm_device *dev);
 int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
 int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
 				  struct intel_crtc_state *cstate);
+void intel_init_ipc(struct drm_i915_private *dev_priv);
+void intel_enable_ipc(struct drm_i915_private *dev_priv);
 static inline int intel_enable_rc6(void)
 {
 	return i915.enable_rc6;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f2794d29f217..70f59df09e75 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5772,6 +5772,30 @@ void intel_update_watermarks(struct intel_crtc *crtc)
 		dev_priv->display.update_wm(crtc);
 }
 
+void intel_enable_ipc(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+
+	val = I915_READ(DISP_ARB_CTL2);
+
+	if (dev_priv->ipc_enabled)
+		val |= DISP_IPC_ENABLE;
+	else
+		val &= ~DISP_IPC_ENABLE;
+
+	I915_WRITE(DISP_ARB_CTL2, val);
+}
+
+void intel_init_ipc(struct drm_i915_private *dev_priv)
+{
+	dev_priv->ipc_enabled = false;
+	if (!HAS_IPC(dev_priv))
+		return;
+
+	dev_priv->ipc_enabled = true;
+	intel_enable_ipc(dev_priv);
+}
+
 /*
  * Lock protecting IPS related data structures
  */
-- 
2.13.0

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

[PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC

[Mahesh Kumar]

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

This patch creates an entry in debugfs to check the status of IPC.
This can also be used to enable/disable IPC in supported platforms.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 73 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ef75c1a6119..368f64de0fdc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
 	return 0;
 }
 
+static int i915_ipc_status_show(struct seq_file *m, void *data)
+{
+	struct drm_i915_private *dev_priv = m->private;
+
+	seq_printf(m, "Isochronous Priority Control: %s\n",
+			enableddisabled(dev_priv->ipc_enabled));
+	return 0;
+}
+
+static int i915_ipc_status_open(struct inode *inode, struct file *file)
+{
+	struct drm_i915_private *dev_priv = inode->i_private;
+
+	if (HAS_IPC(dev_priv))
+		return -ENODEV;
+
+	return single_open(file, i915_ipc_status_show, dev_priv);
+}
+
+static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
+				     size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct drm_i915_private *dev_priv = m->private;
+	char *newline;
+	char tmp[16];
+	bool enable;
+
+	if (HAS_IPC(dev_priv))
+		return -ENODEV;
+
+	if (len >= sizeof(tmp))
+		return -EINVAL;
+
+	if (copy_from_user(tmp, ubuf, len))
+		return -EFAULT;
+
+	tmp[len] = '\0';
+
+	/* Strip newline, if any */
+	newline = strchr(tmp, '\n');
+	if (newline)
+		*newline = '\0';
+
+	if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
+	    strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
+		enable = false;
+	else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
+	    strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
+		enable = true;
+	else
+		return -EINVAL;
+
+	intel_runtime_pm_get(dev_priv);
+	dev_priv->ipc_enabled = enable;
+	intel_enable_ipc(dev_priv);
+	intel_runtime_pm_put(dev_priv);
+
+	return len;
+}
+
+static const struct file_operations i915_ipc_status_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_ipc_status_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_ipc_status_write
+};
+
 static int i915_ddb_info(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4929,7 +4999,8 @@ static const struct i915_debugfs_files {
 	{"i915_dp_test_type", &i915_displayport_test_type_fops},
 	{"i915_dp_test_active", &i915_displayport_test_active_fops},
 	{"i915_guc_log_control", &i915_guc_log_control_fops},
-	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
+	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
+	{"i915_ipc_status", &i915_ipc_status_fops}
 };
 
 int i915_debugfs_register(struct drm_i915_private *dev_priv)
-- 
2.13.0

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

✓ Fi.CI.BAT: success for Fixed16.16 wrapper cleanup & wm optimization (rev6)

[Patchwork]

== Series Details ==

Series: Fixed16.16 wrapper cleanup & wm optimization (rev6)
URL   : https://patchwork.freedesktop.org/series/25692/
State : success

== Summary ==

Series 25692v6 Fixed16.16 wrapper cleanup & wm optimization
https://patchwork.freedesktop.org/api/1.0/series/25692/revisions/6/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) k.org#196399
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900) fdo#101705 +1

k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:439s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:429s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:358s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:535s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:504s
fi-byt-j1900     total:279  pass:255  dwarn:0   dfail:0   fail:0   skip:24  time:492s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:490s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:598s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:443s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:499s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:571s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:578s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:563s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:453s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:586s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:467s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:475s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:441s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:463s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:548s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:399s

10de1e17faaab452782e5a1baffd1b30a639a261 drm-tip: 2017y-07m-18d-10h-08m-42s UTC integration manifest
e6dea33 drm/i915/skl+: debugfs entry to control IPC
38552712 drm/i915/bxt+: Enable IPC support
1564688 drm/i915/gen9+: Add has_ipc flag in device info structure
fac25d3 drm/i915/cnl: Extend WM workaround with IPC for CNL
c99e00c drm/i915/glk: IPC linetime watermark workaround for GLK
c549418 drm/i915/gen10: Calculate and enable transition WM
a180ccf drm/i915/skl+: Optimize WM calculation
10e25f6 drm/i915: Fixed point fixed16 wrapper cleanup

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5218/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/8] drm/i915/gen10: Calculate and enable transition WM

[Maarten Lankhorst]

Op 18-07-17 om 14:49 schreef Mahesh Kumar:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>
> GEN > 9 require transition WM to be programmed if IPC is enabled.
> This patch calculates & enable transition WM for supported platforms.
> If transition WM is enabled, Plane read requests are sent at high
> priority until filling above the transition watermark, then the
> requests are sent at lower priority until dropping below the level-0 WM.
> The lower priority requests allow other memory clients to have better
> memory access.
>
> transition minimum is the minimum amount needed for trans_wm to work to
> ensure  the demote does not happen before enough data has been read to
> meet the level 0 watermark requirements.
>
> transition amount is configurable value. Higher values will
> tend to cause longer periods of high priority reads followed by longer
> periods of lower priority reads. Tuning to lower values will tend to
> cause shorter periods of high and lower priority reads.
>
> Keeping transition amount to 10 in this patch, as suggested by HW team.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b2bd65847d9b..9a2ed1b734d5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4579,12 +4579,55 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate)
>  }
>  
>  static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
> +				      struct skl_wm_params *wp,
> +				      struct skl_wm_level *wm_l0,
> +				      uint16_t ddb_allocation,
>  				      struct skl_wm_level *trans_wm /* out */)
>  {
> +	struct drm_device *dev = cstate->base.crtc->dev;
> +	const struct drm_i915_private *dev_priv = to_i915(dev);
> +	uint16_t trans_min, trans_y_tile_min;
> +	uint16_t trans_amount = 10; /* This is configurable amount */
should probably be a const since it's a tweakable.
> +	uint16_t trans_offset_b, res_blocks;
> +
>  	if (!cstate->base.active)
> +		goto exit;
> +
> +	/* Transition WM are not recommended by HW team for GEN9 */
> +	if (INTEL_GEN(dev_priv) <= 9)
> +		goto exit;
> +
> +	/* Transition WM don't make any sense if ipc is disabled */
> +	if (!dev_priv->ipc_enabled)
> +		goto exit;
> +
> +	if (INTEL_GEN(dev_priv) >= 10)
> +		trans_min = 4;
> +
> +	trans_offset_b = trans_min + trans_amount;
> +	trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
> +							wp->y_tile_minimum);
> +
> +	if (wp->y_tiled) {
> +		res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
> +				trans_offset_b;
Perhaps only calculate trans_y_tile_min if y-tiled?
> +	} else {
> +		res_blocks = wm_l0->plane_res_b + trans_offset_b;
> +	}
> +
> +	res_blocks += 1;
> +
> +	/* WA BUG:1938466 add one block for non y-tile planes */
> +	if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
> +		res_blocks += 1;
Could be added to the !y_tiled branch?

Patch looks sane though, and seems to match bspec exactly. I couldn't find the value for trans_amount, but I'll take your word it's the right one. :)

With the minor fixes:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC

[Maarten Lankhorst]

Op 18-07-17 om 14:49 schreef Mahesh Kumar:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>
> This patch creates an entry in debugfs to check the status of IPC.
> This can also be used to enable/disable IPC in supported platforms.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 73 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2ef75c1a6119..368f64de0fdc 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
>  	return 0;
>  }
>  
> +static int i915_ipc_status_show(struct seq_file *m, void *data)
> +{
> +	struct drm_i915_private *dev_priv = m->private;
> +
> +	seq_printf(m, "Isochronous Priority Control: %s\n",
> +			enableddisabled(dev_priv->ipc_enabled));
> +	return 0;
> +}
> +
> +static int i915_ipc_status_open(struct inode *inode, struct file *file)
> +{
> +	struct drm_i915_private *dev_priv = inode->i_private;
> +
> +	if (HAS_IPC(dev_priv))
> +		return -ENODEV;
I take it you didn't test this version of the patch? :p
> +
> +	return single_open(file, i915_ipc_status_show, dev_priv);
> +}
> +
> +static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
> +				     size_t len, loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct drm_i915_private *dev_priv = m->private;
> +	char *newline;
> +	char tmp[16];
> +	bool enable;
> +
> +	if (HAS_IPC(dev_priv))
> +		return -ENODEV;
Again, though I would remove this check since you already test in open().
> +	if (len >= sizeof(tmp))
> +		return -EINVAL;
> +
> +	if (copy_from_user(tmp, ubuf, len))
> +		return -EFAULT;
> +
> +	tmp[len] = '\0';
> +
> +	/* Strip newline, if any */
> +	newline = strchr(tmp, '\n');
> +	if (newline)
> +		*newline = '\0';
> +
> +	if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
> +	    strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
> +		enable = false;
> +	else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
> +	    strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
> +		enable = true;
> +	else
> +		return -EINVAL;
Maybe replace with kstrtobool_from_user, and use yesno for ipc_enabled in show()? That way you won't have to do all these special cases here. :)
> +
> +	intel_runtime_pm_get(dev_priv);
> +	dev_priv->ipc_enabled = enable;
> +	intel_enable_ipc(dev_priv);
> +	intel_runtime_pm_put(dev_priv);
> +
> +	return len;
> +}
> +
> +static const struct file_operations i915_ipc_status_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_ipc_status_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_ipc_status_write
> +};
> +
>  static int i915_ddb_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -4929,7 +4999,8 @@ static const struct i915_debugfs_files {
>  	{"i915_dp_test_type", &i915_displayport_test_type_fops},
>  	{"i915_dp_test_active", &i915_displayport_test_active_fops},
>  	{"i915_guc_log_control", &i915_guc_log_control_fops},
> -	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
> +	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> +	{"i915_ipc_status", &i915_ipc_status_fops}
>  };
>  
>  int i915_debugfs_register(struct drm_i915_private *dev_priv)


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

Re: [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC

[Maarten Lankhorst]

Op 18-07-17 om 14:49 schreef Mahesh Kumar:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>
> This patch creates an entry in debugfs to check the status of IPC.
> This can also be used to enable/disable IPC in supported platforms.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 73 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2ef75c1a6119..368f64de0fdc 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
>  	return 0;
>  }
>  
> +static int i915_ipc_status_show(struct seq_file *m, void *data)
> +{
> +	struct drm_i915_private *dev_priv = m->private;
> +
> +	seq_printf(m, "Isochronous Priority Control: %s\n",
> +			enableddisabled(dev_priv->ipc_enabled));
> +	return 0;
> +}
> +
> +static int i915_ipc_status_open(struct inode *inode, struct file *file)
> +{
> +	struct drm_i915_private *dev_priv = inode->i_private;
> +
> +	if (HAS_IPC(dev_priv))
> +		return -ENODEV;
> +
> +	return single_open(file, i915_ipc_status_show, dev_priv);
> +}
> +
> +static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
> +				     size_t len, loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct drm_i915_private *dev_priv = m->private;
> +	char *newline;
> +	char tmp[16];
> +	bool enable;
> +
> +	if (HAS_IPC(dev_priv))
> +		return -ENODEV;
> +
> +	if (len >= sizeof(tmp))
> +		return -EINVAL;
> +
> +	if (copy_from_user(tmp, ubuf, len))
> +		return -EFAULT;
> +
> +	tmp[len] = '\0';
> +
> +	/* Strip newline, if any */
> +	newline = strchr(tmp, '\n');
> +	if (newline)
> +		*newline = '\0';
> +
> +	if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
> +	    strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
> +		enable = false;
> +	else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
> +	    strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
> +		enable = true;
> +	else
> +		return -EINVAL;
> +
> +	intel_runtime_pm_get(dev_priv);
> +	dev_priv->ipc_enabled = enable;
> +	intel_enable_ipc(dev_priv);
> +	intel_runtime_pm_put(dev_priv);
Hm, intel_enable_ipc should take a bool on whether to enable ipc.

Forcefully enabling IPC here might give weird effects until the next plane update. The transition watermarks are not updated yet until the next commit. This could probably be handled here, but that would be overkill..

Perhaps add a drm_info or something about it when setting enable = true, when it was previously false?
> +
> +	return len;
> +}
> +
> +static const struct file_operations i915_ipc_status_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_ipc_status_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_ipc_status_write
> +};
> +
>  static int i915_ddb_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -4929,7 +4999,8 @@ static const struct i915_debugfs_files {
>  	{"i915_dp_test_type", &i915_displayport_test_type_fops},
>  	{"i915_dp_test_active", &i915_displayport_test_active_fops},
>  	{"i915_guc_log_control", &i915_guc_log_control_fops},
> -	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
> +	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> +	{"i915_ipc_status", &i915_ipc_status_fops}
>  };
>  
>  int i915_debugfs_register(struct drm_i915_private *dev_priv)


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

Re: [PATCH 7/8] drm/i915/bxt+: Enable IPC support

[Maarten Lankhorst]

Op 18-07-17 om 14:49 schreef Mahesh Kumar:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>
> This patch adds IPC support. This patch also enables IPC in all supported
> platforms based on has_ipc flag.
> IPC (Isochronous Priority Control) is the hardware feature, which
> dynamically controls the memory read priority of Display.
>
> When IPC is enabled, plane read requests are sent at high priority until
> filling above the transition watermark, then the requests are sent at
> lower priority until dropping below the level 0 watermark.
> The lower priority requests allow other memory clients to have better
> memory access. When IPC is disabled, all plane read requests are sent at
> high priority.
>
> Changes since V1:
>  - Remove commandline parameter to disable ipc
>  - Address Paulo's comments
> Changes since V2:
>  - Address review comments
>  - Set ipc_enabled flag
> Changes since V3:
>  - move ipc_enabled flag assignment inside intel_ipc_enable function
> Changes since V4:
>  - Re-enable IPC after suspend/resume
> Changes since V5:
>  - Enable IPC for all gen >=9 except SKL
> Changes since V6:
>  - fix commit msg
>  - after resume program IPC based on SW state.
> Changes since V7:
>  - Modify IPC support check based on HAS_IPC macro (suggested by Chris)
Much better, for all patches without comments..

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC

[Mahesh Kumar]

Hi,

Thanks for review.


On Wednesday 09 August 2017 03:03 PM, Maarten Lankhorst wrote:
> Op 18-07-17 om 14:49 schreef Mahesh Kumar:
>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>
>> This patch creates an entry in debugfs to check the status of IPC.
>> This can also be used to enable/disable IPC in supported platforms.
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 73 ++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 72 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 2ef75c1a6119..368f64de0fdc 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
>>   	return 0;
>>   }
>>   
>> +static int i915_ipc_status_show(struct seq_file *m, void *data)
>> +{
>> +	struct drm_i915_private *dev_priv = m->private;
>> +
>> +	seq_printf(m, "Isochronous Priority Control: %s\n",
>> +			enableddisabled(dev_priv->ipc_enabled));
>> +	return 0;
>> +}
>> +
>> +static int i915_ipc_status_open(struct inode *inode, struct file *file)
>> +{
>> +	struct drm_i915_private *dev_priv = inode->i_private;
>> +
>> +	if (HAS_IPC(dev_priv))
>> +		return -ENODEV;
> I take it you didn't test this version of the patch? :p
hmm, I switched HAS_IPC macro in this version of patch only. will fix 
this. thanks for catching it.
>> +
>> +	return single_open(file, i915_ipc_status_show, dev_priv);
>> +}
>> +
>> +static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
>> +				     size_t len, loff_t *offp)
>> +{
>> +	struct seq_file *m = file->private_data;
>> +	struct drm_i915_private *dev_priv = m->private;
>> +	char *newline;
>> +	char tmp[16];
>> +	bool enable;
>> +
>> +	if (HAS_IPC(dev_priv))
>> +		return -ENODEV;
> Again, though I would remove this check since you already test in open().
ok, will remove this check.
>> +	if (len >= sizeof(tmp))
>> +		return -EINVAL;
>> +
>> +	if (copy_from_user(tmp, ubuf, len))
>> +		return -EFAULT;
>> +
>> +	tmp[len] = '\0';
>> +
>> +	/* Strip newline, if any */
>> +	newline = strchr(tmp, '\n');
>> +	if (newline)
>> +		*newline = '\0';
>> +
>> +	if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
>> +	    strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
>> +		enable = false;
>> +	else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
>> +	    strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
>> +		enable = true;
>> +	else
>> +		return -EINVAL;
> Maybe replace with kstrtobool_from_user, and use yesno for ipc_enabled in show()? That way you won't have to do all these special cases here. :)
kstrtobool_from_user sounds good, will use this macro :)

-Mahesh
>> +
>> +	intel_runtime_pm_get(dev_priv);
>> +	dev_priv->ipc_enabled = enable;
>> +	intel_enable_ipc(dev_priv);
>> +	intel_runtime_pm_put(dev_priv);
>> +
>> +	return len;
>> +}
>> +
>> +static const struct file_operations i915_ipc_status_fops = {
>> +	.owner = THIS_MODULE,
>> +	.open = i915_ipc_status_open,
>> +	.read = seq_read,
>> +	.llseek = seq_lseek,
>> +	.release = single_release,
>> +	.write = i915_ipc_status_write
>> +};
>> +
>>   static int i915_ddb_info(struct seq_file *m, void *unused)
>>   {
>>   	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> @@ -4929,7 +4999,8 @@ static const struct i915_debugfs_files {
>>   	{"i915_dp_test_type", &i915_displayport_test_type_fops},
>>   	{"i915_dp_test_active", &i915_displayport_test_active_fops},
>>   	{"i915_guc_log_control", &i915_guc_log_control_fops},
>> -	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
>> +	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
>> +	{"i915_ipc_status", &i915_ipc_status_fops}
>>   };
>>   
>>   int i915_debugfs_register(struct drm_i915_private *dev_priv)
>

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

Re: [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC

[Mahesh Kumar]

Hi,


On Wednesday 09 August 2017 03:16 PM, Maarten Lankhorst wrote:
> Op 18-07-17 om 14:49 schreef Mahesh Kumar:
>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>
>> This patch creates an entry in debugfs to check the status of IPC.
>> This can also be used to enable/disable IPC in supported platforms.
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 73 ++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 72 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 2ef75c1a6119..368f64de0fdc 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
>>   	return 0;
>>   }
>>   
>> +static int i915_ipc_status_show(struct seq_file *m, void *data)
>> +{
>> +	struct drm_i915_private *dev_priv = m->private;
>> +
>> +	seq_printf(m, "Isochronous Priority Control: %s\n",
>> +			enableddisabled(dev_priv->ipc_enabled));
>> +	return 0;
>> +}
>> +
>> +static int i915_ipc_status_open(struct inode *inode, struct file *file)
>> +{
>> +	struct drm_i915_private *dev_priv = inode->i_private;
>> +
>> +	if (HAS_IPC(dev_priv))
>> +		return -ENODEV;
>> +
>> +	return single_open(file, i915_ipc_status_show, dev_priv);
>> +}
>> +
>> +static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
>> +				     size_t len, loff_t *offp)
>> +{
>> +	struct seq_file *m = file->private_data;
>> +	struct drm_i915_private *dev_priv = m->private;
>> +	char *newline;
>> +	char tmp[16];
>> +	bool enable;
>> +
>> +	if (HAS_IPC(dev_priv))
>> +		return -ENODEV;
>> +
>> +	if (len >= sizeof(tmp))
>> +		return -EINVAL;
>> +
>> +	if (copy_from_user(tmp, ubuf, len))
>> +		return -EFAULT;
>> +
>> +	tmp[len] = '\0';
>> +
>> +	/* Strip newline, if any */
>> +	newline = strchr(tmp, '\n');
>> +	if (newline)
>> +		*newline = '\0';
>> +
>> +	if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
>> +	    strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
>> +		enable = false;
>> +	else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
>> +	    strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
>> +		enable = true;
>> +	else
>> +		return -EINVAL;
>> +
>> +	intel_runtime_pm_get(dev_priv);
>> +	dev_priv->ipc_enabled = enable;
>> +	intel_enable_ipc(dev_priv);
>> +	intel_runtime_pm_put(dev_priv);
> Hm, intel_enable_ipc should take a bool on whether to enable ipc.
intel_enable_ipc takes decision to enable/disable ipc based on 
dev_priv->ipc_enabled value. which is anyway input to function.
>
> Forcefully enabling IPC here might give weird effects until the next plane update. The transition watermarks are not updated yet until the next commit. This could probably be handled here, but that would be overkill..
yes, agree, but I don't wanted to call a commit internally. Will add a 
drm_info when changing IPC disable => enable as suggested.
-Mahesh
>
> Perhaps add a drm_info or something about it when setting enable = true, when it was previously false?
>> +
>> +	return len;
>> +}
>> +
>> +static const struct file_operations i915_ipc_status_fops = {
>> +	.owner = THIS_MODULE,
>> +	.open = i915_ipc_status_open,
>> +	.read = seq_read,
>> +	.llseek = seq_lseek,
>> +	.release = single_release,
>> +	.write = i915_ipc_status_write
>> +};
>> +
>>   static int i915_ddb_info(struct seq_file *m, void *unused)
>>   {
>>   	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> @@ -4929,7 +4999,8 @@ static const struct i915_debugfs_files {
>>   	{"i915_dp_test_type", &i915_displayport_test_type_fops},
>>   	{"i915_dp_test_active", &i915_displayport_test_active_fops},
>>   	{"i915_guc_log_control", &i915_guc_log_control_fops},
>> -	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}
>> +	{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
>> +	{"i915_ipc_status", &i915_ipc_status_fops}
>>   };
>>   
>>   int i915_debugfs_register(struct drm_i915_private *dev_priv)
>

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

Re: [PATCH 8/8] drm/i915/skl+: debugfs entry to control IPC

[Maarten Lankhorst]

Op 10-08-17 om 10:56 schreef Mahesh Kumar:
> Hi,
>
>
> On Wednesday 09 August 2017 03:16 PM, Maarten Lankhorst wrote:
>> Op 18-07-17 om 14:49 schreef Mahesh Kumar:
>>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>>
>>> This patch creates an entry in debugfs to check the status of IPC.
>>> This can also be used to enable/disable IPC in supported platforms.
>>>
>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_debugfs.c | 73 ++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 72 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 2ef75c1a6119..368f64de0fdc 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3592,6 +3592,76 @@ static int i915_wa_registers(struct seq_file *m, void *unused)
>>>       return 0;
>>>   }
>>>   +static int i915_ipc_status_show(struct seq_file *m, void *data)
>>> +{
>>> +    struct drm_i915_private *dev_priv = m->private;
>>> +
>>> +    seq_printf(m, "Isochronous Priority Control: %s\n",
>>> +            enableddisabled(dev_priv->ipc_enabled));
>>> +    return 0;
>>> +}
>>> +
>>> +static int i915_ipc_status_open(struct inode *inode, struct file *file)
>>> +{
>>> +    struct drm_i915_private *dev_priv = inode->i_private;
>>> +
>>> +    if (HAS_IPC(dev_priv))
>>> +        return -ENODEV;
>>> +
>>> +    return single_open(file, i915_ipc_status_show, dev_priv);
>>> +}
>>> +
>>> +static ssize_t i915_ipc_status_write(struct file *file, const char __user *ubuf,
>>> +                     size_t len, loff_t *offp)
>>> +{
>>> +    struct seq_file *m = file->private_data;
>>> +    struct drm_i915_private *dev_priv = m->private;
>>> +    char *newline;
>>> +    char tmp[16];
>>> +    bool enable;
>>> +
>>> +    if (HAS_IPC(dev_priv))
>>> +        return -ENODEV;
>>> +
>>> +    if (len >= sizeof(tmp))
>>> +        return -EINVAL;
>>> +
>>> +    if (copy_from_user(tmp, ubuf, len))
>>> +        return -EFAULT;
>>> +
>>> +    tmp[len] = '\0';
>>> +
>>> +    /* Strip newline, if any */
>>> +    newline = strchr(tmp, '\n');
>>> +    if (newline)
>>> +        *newline = '\0';
>>> +
>>> +    if (strcmp(tmp, "0") == 0 || strcmp(tmp, "disable") == 0 ||
>>> +        strcmp(tmp, "off") == 0 || strcmp(tmp, "dis") == 0)
>>> +        enable = false;
>>> +    else if (strcmp(tmp, "1") == 0 || strcmp(tmp, "enable") == 0 ||
>>> +        strcmp(tmp, "on") == 0 || strcmp(tmp, "en") == 0)
>>> +        enable = true;
>>> +    else
>>> +        return -EINVAL;
>>> +
>>> +    intel_runtime_pm_get(dev_priv);
>>> +    dev_priv->ipc_enabled = enable;
>>> +    intel_enable_ipc(dev_priv);
>>> +    intel_runtime_pm_put(dev_priv);
>> Hm, intel_enable_ipc should take a bool on whether to enable ipc.
> intel_enable_ipc takes decision to enable/disable ipc based on dev_priv->ipc_enabled value. which is anyway input to function.
>>
>> Forcefully enabling IPC here might give weird effects until the next plane update. The transition watermarks are not updated yet until the next commit. This could probably be handled here, but that would be overkill..
> yes, agree, but I don't wanted to call a commit internally. Will add a drm_info when changing IPC disable => enable as suggested. 

True, perhaps we could set dev_priv->wm.distrust_bios_wm to force recalculation next time?

~Maarten

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

[PATCH 7/8] drm/i915/bxt+: Enable IPC support

[Mahesh Kumar]

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

This patch adds IPC support. This patch also enables IPC in all supported
platforms based on has_ipc flag.
IPC (Isochronous Priority Control) is the hardware feature, which
dynamically controls the memory read priority of Display.

When IPC is enabled, plane read requests are sent at high priority until
filling above the transition watermark, then the requests are sent at
lower priority until dropping below the level 0 watermark.
The lower priority requests allow other memory clients to have better
memory access. When IPC is disabled, all plane read requests are sent at
high priority.

Changes since V1:
 - Remove commandline parameter to disable ipc
 - Address Paulo's comments
Changes since V2:
 - Address review comments
 - Set ipc_enabled flag
Changes since V3:
 - move ipc_enabled flag assignment inside intel_ipc_enable function
Changes since V4:
 - Re-enable IPC after suspend/resume
Changes since V5:
 - Enable IPC for all gen >=9 except SKL
Changes since V6:
 - fix commit msg
 - after resume program IPC based on SW state.
Changes since V7:
 - Modify IPC support check based on HAS_IPC macro (suggested by Chris)

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |  4 +++-
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_pm.c      | 24 ++++++++++++++++++++++++
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 43100229613c..f655973bcb26 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1340,7 +1340,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	intel_runtime_pm_enable(dev_priv);
 
-	dev_priv->ipc_enabled = false;
+	intel_init_ipc(dev_priv);
 
 	if (IS_ENABLED(CONFIG_DRM_I915_DEBUG))
 		DRM_INFO("DRM_I915_DEBUG enabled\n");
@@ -2602,6 +2602,8 @@ static int intel_runtime_resume(struct device *kdev)
 	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
 		intel_hpd_init(dev_priv);
 
+	intel_enable_ipc(dev_priv);
+
 	enable_rpm_wakeref_asserts(dev_priv);
 
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ed7cd9ee2c2a..7240c0a3641c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6934,6 +6934,7 @@ enum {
 #define  DISP_FBC_WM_DIS		(1<<15)
 #define DISP_ARB_CTL2	_MMIO(0x45004)
 #define  DISP_DATA_PARTITION_5_6	(1<<6)
+#define  DISP_IPC_ENABLE		(1<<3)
 #define DBUF_CTL	_MMIO(0x45008)
 #define  DBUF_POWER_REQUEST		(1<<31)
 #define  DBUF_POWER_STATE		(1<<30)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0e93ec201fe3..e11bb3b430a2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15164,6 +15164,7 @@ void intel_display_resume(struct drm_device *dev)
 	if (!ret)
 		ret = __intel_display_resume(dev, state, &ctx);
 
+	intel_enable_ipc(dev_priv);
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fa47285918f4..a17c602319eb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1867,6 +1867,8 @@ bool ilk_disable_lp_wm(struct drm_device *dev);
 int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
 int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
 				  struct intel_crtc_state *cstate);
+void intel_init_ipc(struct drm_i915_private *dev_priv);
+void intel_enable_ipc(struct drm_i915_private *dev_priv);
 static inline int intel_enable_rc6(void)
 {
 	return i915.enable_rc6;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aee1a387a65a..8d7af30ed0b0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5818,6 +5818,30 @@ void intel_update_watermarks(struct intel_crtc *crtc)
 		dev_priv->display.update_wm(crtc);
 }
 
+void intel_enable_ipc(struct drm_i915_private *dev_priv)
+{
+	u32 val;
+
+	val = I915_READ(DISP_ARB_CTL2);
+
+	if (dev_priv->ipc_enabled)
+		val |= DISP_IPC_ENABLE;
+	else
+		val &= ~DISP_IPC_ENABLE;
+
+	I915_WRITE(DISP_ARB_CTL2, val);
+}
+
+void intel_init_ipc(struct drm_i915_private *dev_priv)
+{
+	dev_priv->ipc_enabled = false;
+	if (!HAS_IPC(dev_priv))
+		return;
+
+	dev_priv->ipc_enabled = true;
+	intel_enable_ipc(dev_priv);
+}
+
 /*
  * Lock protecting IPS related data structures
  */
-- 
2.13.0

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