[PATCH 0/7] Misc WM fixes and Arbitrated Display Bandwidth WA for SKL

[PATCH 0/7] Misc WM fixes and Arbitrated Display Bandwidth WA for SKL

[Shobhit Kumar]

Hi,
This series add a set of updates to the WM calculation and also enables
arbitrated display bandwidth based WA. Some of these patches do overlap
with Matts work but we wanted to send them out as we have them in our
internal testing for early review. Most likley some of them can be
superceded by patches from Matt, or can be re-used if deemed necessary.

Especially "drm/i915/skl+: Use fb size for relative data rate calculation"
this already addresses some of Ville's comment on similar patch from Matt.

Regards
Shobhit

Kumar, Mahesh (6):
  drm/i915/skl+: Use proper bytes_per_pixel during WM calculation
  drm/i915/skl+: Use fb size for relative data rate calculation
  drm/i915/skl+: calculate ddb minimum allocation
  drm/i915/skl+: calculate plane pixel rate.
  drm/i915/skl+: Use scaling amount for plane data rate calculation
  drm/i915/skl: WA for watermark calculation based on Arbitrated Display
    BW

Shobhit Kumar (1):
  drm/i915: Add support to parse DMI table and get platform memory info

 drivers/gpu/drm/i915/i915_dma.c  |  19 +++
 drivers/gpu/drm/i915/i915_drv.h  |  15 ++
 drivers/gpu/drm/i915/intel_drv.h |   2 +
 drivers/gpu/drm/i915/intel_pm.c  | 294 +++++++++++++++++++++++++++++++++++++--
 4 files changed, 316 insertions(+), 14 deletions(-)

-- 
2.4.3

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

[PATCH 1/7] drm/i915/skl+: Use proper bytes_per_pixel during WM calculation

[Shobhit Kumar]

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

Don't always use bytes_per_pixel using y_plane=0, instead use it
according to pixel format. If NV12 use y_plane eqal to 1

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9df9e9a..68f21b9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3185,7 +3185,9 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	if (latency == 0 || !cstate->base.active || !fb)
 		return false;
 
-	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
+	bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
+				drm_format_plane_cpp(fb->pixel_format, 1) :
+				drm_format_plane_cpp(fb->pixel_format, 0);
 	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
 				 bytes_per_pixel,
 				 latency);
-- 
2.4.3

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

[PATCH 2/7] drm/i915/skl+: Use fb size for relative data rate calculation

[Shobhit Kumar]

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

Use FB size for relative data rate calculation. don't always use
pipe source width & height.
adjust height & width according to rotation.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 68f21b9..d33c4ff 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2928,24 +2928,33 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 			     int y)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
 	struct drm_framebuffer *fb = pstate->fb;
+	uint32_t width = 0, height = 0;
+
+	if (drm_rect_width(&intel_pstate->src)) {
+		width = drm_rect_width(&intel_pstate->src) >> 16;
+		height = drm_rect_height(&intel_pstate->src) >> 16;
+	} else {
+		width = intel_crtc->config->pipe_src_w;
+		height = intel_crtc->config->pipe_src_h;
+	}
+
+	if (intel_rotation_90_or_270(pstate->rotation))
+		swap(width, height);
 
 	/* for planar format */
 	if (fb->pixel_format == DRM_FORMAT_NV12) {
 		if (y)  /* y-plane data rate */
-			return intel_crtc->config->pipe_src_w *
-				intel_crtc->config->pipe_src_h *
+			return width * height *
 				drm_format_plane_cpp(fb->pixel_format, 0);
 		else    /* uv-plane data rate */
-			return (intel_crtc->config->pipe_src_w/2) *
-				(intel_crtc->config->pipe_src_h/2) *
+			return (width / 2) * (height / 2) *
 				drm_format_plane_cpp(fb->pixel_format, 1);
 	}
 
 	/* for packed formats */
-	return intel_crtc->config->pipe_src_w *
-		intel_crtc->config->pipe_src_h *
-		drm_format_plane_cpp(fb->pixel_format, 0);
+	return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
 }
 
 /*
@@ -3181,10 +3190,25 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	uint32_t res_blocks, res_lines;
 	uint32_t selected_result;
 	uint8_t bytes_per_pixel;
+	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+	struct intel_plane_state *intel_pstate = to_intel_plane_state(plane->state);
+	uint32_t width = 0, height = 0;
 
 	if (latency == 0 || !cstate->base.active || !fb)
 		return false;
 
+	if (drm_rect_width(&intel_pstate->src)) {
+		width = drm_rect_width(&intel_pstate->src) >> 16;
+		height = drm_rect_height(&intel_pstate->src) >> 16;
+	} else {
+		width = intel_crtc->config->pipe_src_w;
+		height = intel_crtc->config->pipe_src_h;
+	}
+
+	if (intel_rotation_90_or_270(plane->state->rotation))
+		swap(width, height);
+
+	/* for planar format */
 	bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
 				drm_format_plane_cpp(fb->pixel_format, 1) :
 				drm_format_plane_cpp(fb->pixel_format, 0);
@@ -3193,12 +3217,12 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				 latency);
 	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
 				 cstate->base.adjusted_mode.crtc_htotal,
-				 cstate->pipe_src_w,
+				 width,
 				 bytes_per_pixel,
 				 fb->modifier[0],
 				 latency);
 
-	plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
+	plane_bytes_per_line = width * bytes_per_pixel;
 	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 
 	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
-- 
2.4.3

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

[PATCH 3/7] drm/i915/skl+: calculate ddb minimum allocation

[Shobhit Kumar]

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

don't always use 8 ddb as minimum, instead calculate using proper
algorithm.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d33c4ff..64b39ec 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2994,6 +2994,59 @@ skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
 	return total_data_rate;
 }
 
+static uint16_t
+skl_ddb_min_alloc(const struct intel_crtc *crtc,
+		const struct drm_plane *plane, int y)
+{
+	struct drm_framebuffer *fb = plane->state->fb;
+	struct intel_plane_state *pstate = to_intel_plane_state(plane->state);
+	uint16_t min_alloc;
+	uint32_t src_w, src_h;
+
+	/* For packed formats, no y-plane, return 0 */
+	if (y && !(fb->pixel_format == DRM_FORMAT_NV12))
+		return 0;
+
+	if (drm_rect_width(&pstate->src)) {
+		src_w = drm_rect_width(&pstate->src) >> 16;
+		src_h = drm_rect_height(&pstate->src) >> 16;
+	} else {
+		src_w = crtc->config->pipe_src_w;
+		src_h = crtc->config->pipe_src_h;
+	}
+
+	if (intel_rotation_90_or_270(plane->state->rotation))
+		swap(src_w, src_h);
+
+	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
+		uint32_t min_scanlines = 8;
+		uint8_t bytes_per_pixel =
+			y ? drm_format_plane_cpp(fb->pixel_format, 1) :
+				drm_format_plane_cpp(fb->pixel_format, 0);
+
+		if (intel_rotation_90_or_270(plane->state->rotation)) {
+			switch (bytes_per_pixel) {
+			case 1:
+				min_scanlines = 32;
+				break;
+			case 2:
+				min_scanlines = 16;
+				break;
+			case 8:
+				WARN(1, "Unsupported pixel depth for rotation");
+			}
+		}
+		min_alloc = DIV_ROUND_UP((4 * src_w / (y ? 1 : 2) *
+				bytes_per_pixel), 512) * min_scanlines/4 + 3;
+	} else {
+		min_alloc = 8;
+	}
+
+	return min_alloc;
+}
+
+
 static void
 skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		      struct skl_ddb_allocation *ddb /* out */)
@@ -3038,9 +3091,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		if (plane->type == DRM_PLANE_TYPE_CURSOR)
 			continue;
 
-		minimum[id] = 8;
+		minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 0);
 		alloc_size -= minimum[id];
-		y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
+		y_minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 1);
 		alloc_size -= y_minimum[id];
 	}
 
-- 
2.4.3

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

[PATCH 4/7] drm/i915/skl+: calculate plane pixel rate.

[Shobhit Kumar]

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

Don't use pipe pixel rate for plane pixel rate.
Calculate plane pixel according to formula

adjusted plane_pixel_rate = adjusted pipe_pixel_rate * downscale ammount

downscale amount = max[1, src_h/dst_h] * max[1, src_w/dst_w]
if 90/270 rotation use rotated width & height

Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  2 +
 drivers/gpu/drm/i915/intel_pm.c  | 95 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 059b46e..49f237e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -661,6 +661,8 @@ struct intel_plane_wm_parameters {
 	u64 tiling;
 	unsigned int rotation;
 	uint16_t fifo_size;
+	 /* Stores the adjusted plane pixel rate for WM calculation */
+	uint32_t plane_pixel_rate;
 };
 
 struct intel_plane {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 64b39ec..ffcc56a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2841,6 +2841,54 @@ skl_wm_plane_id(const struct intel_plane *plane)
 	}
 }
 
+/*
+ * This function takes drm_plane_state as input
+ * and decides the downscale amount according to the formula
+ *
+ * downscale amount = Max[1, Horizontal source size / Horizontal dest size]
+ *
+ * Return value is multiplied by 1000 to retain fractional part
+ * Caller should take care of dividing & Rounding off the value
+ */
+static uint32_t
+skl_plane_downscale_amount(const struct intel_plane *intel_plane)
+{
+	struct drm_plane_state *pstate = intel_plane->base.state;
+	struct intel_crtc *crtc = to_intel_crtc(pstate->crtc);
+	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
+	uint32_t downscale_h, downscale_w;
+	uint32_t src_w, src_h, dst_w, dst_h, tmp;
+
+	if (drm_rect_width(&intel_pstate->src)) {
+		src_w = drm_rect_width(&intel_pstate->src) >> 16;
+		src_h = drm_rect_height(&intel_pstate->src) >> 16;
+	} else {
+		src_w = crtc->config->pipe_src_w;
+		src_h = crtc->config->pipe_src_h;
+	}
+
+	dst_w = drm_rect_width(&intel_pstate->dst);
+	dst_h = drm_rect_height(&intel_pstate->dst);
+
+	if (intel_rotation_90_or_270(pstate->rotation))
+		swap(dst_w, dst_h);
+
+	/* If destination height & wight are zero return amount as unity */
+	if (dst_w == 0 || dst_h == 0)
+		return 1000;
+
+	/* Multiply by 1000 for precision */
+	tmp = (1000 * src_h) / dst_h;
+	downscale_h = max_t(uint32_t, 1000, tmp);
+
+	tmp = (1000 * src_w) / dst_w;
+	downscale_w = max_t(uint32_t, 1000, tmp);
+
+	/* Reducing precision to 3 decimal places */
+	return DIV_ROUND_UP(downscale_h * downscale_w, 1000);
+}
+
+
 static void
 skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 				   const struct intel_crtc_state *cstate,
@@ -3265,10 +3313,10 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
 				drm_format_plane_cpp(fb->pixel_format, 1) :
 				drm_format_plane_cpp(fb->pixel_format, 0);
-	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
+	method1 = skl_wm_method1(intel_plane->wm.plane_pixel_rate,
 				 bytes_per_pixel,
 				 latency);
-	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
+	method2 = skl_wm_method2(intel_plane->wm.plane_pixel_rate,
 				 cstate->base.adjusted_mode.crtc_htotal,
 				 width,
 				 bytes_per_pixel,
@@ -3709,6 +3757,46 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
 	}
 }
 
+static uint32_t
+skl_plane_pixel_rate(struct intel_crtc_state *cstate, struct intel_plane *plane)
+{
+	uint32_t adjusted_pixel_rate;
+	uint32_t downscale_amount;
+
+	/*
+	 * adjusted plane pixel rate = adjusted pipe pixel rate
+	 * Plane pixel rate = adjusted plane pixel rate * plane down scale
+	 * amount
+	 */
+	adjusted_pixel_rate = skl_pipe_pixel_rate(cstate);
+	downscale_amount = skl_plane_downscale_amount(plane);
+
+	return DIV_ROUND_UP(adjusted_pixel_rate * downscale_amount,
+			1000);
+}
+
+static void skl_set_plane_pixel_rate(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+	struct intel_plane *intel_plane = NULL;
+	struct drm_device *dev = crtc->dev;
+
+	if (!intel_crtc->active)
+		return;
+	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+		struct drm_plane *plane = &intel_plane->base;
+		struct drm_framebuffer *fb = plane->state->fb;
+
+		if (fb == NULL)
+			continue;
+
+		intel_plane->wm.plane_pixel_rate = skl_plane_pixel_rate(cstate,
+				intel_plane);
+	}
+
+}
+
 static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
 {
 	watermarks->wm_linetime[pipe] = 0;
@@ -3744,6 +3832,9 @@ static void skl_update_wm(struct drm_crtc *crtc)
 
 	skl_clear_wm(results, intel_crtc->pipe);
 
+	/* Calculate plane pixel rate for each plane in advance */
+	skl_set_plane_pixel_rate(crtc);
+
 	if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
 		return;
 
-- 
2.4.3

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

[PATCH 5/7] drm/i915/skl+: Use scaling amount for plane data rate calculation

[Shobhit Kumar]

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

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ffcc56a..dc08494 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2978,6 +2978,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
 	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
 	struct drm_framebuffer *fb = pstate->fb;
+	struct intel_plane *intel_plane = to_intel_plane(pstate->plane);
+	uint32_t down_scale_amount, data_rate;
 	uint32_t width = 0, height = 0;
 
 	if (drm_rect_width(&intel_pstate->src)) {
@@ -2994,15 +2996,19 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 	/* for planar format */
 	if (fb->pixel_format == DRM_FORMAT_NV12) {
 		if (y)  /* y-plane data rate */
-			return width * height *
+			data_rate = width * height *
 				drm_format_plane_cpp(fb->pixel_format, 0);
 		else    /* uv-plane data rate */
-			return (width / 2) * (height / 2) *
+			data_rate = (width / 2) * (height / 2) *
 				drm_format_plane_cpp(fb->pixel_format, 1);
 	}
 
 	/* for packed formats */
-	return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
+	data_rate = width * height * drm_format_plane_cpp(fb->pixel_format, 0);
+	down_scale_amount = skl_plane_downscale_amount(intel_plane);
+
+	return DIV_ROUND_UP((data_rate * down_scale_amount), 1000);
+
 }
 
 /*
-- 
2.4.3

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

[PATCH 6/7] drm/i915: Add support to parse DMI table and get platform memory info

[Shobhit Kumar]

This is needed for WM computation workaround for arbitrated display
bandwidth.

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a0f5659..03c3131 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -49,6 +49,7 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/oom.h>
+#include <linux/dmi.h>
 
 
 static int i915_getparam(struct drm_device *dev, void *data,
@@ -855,6 +856,21 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void dmi_decode_memory_info(const struct dmi_header *hdr, void *priv)
+{
+	struct drm_i915_private *dev_priv = (struct drm_i915_private *) priv;
+	const u8 *data = (const u8 *) hdr;
+
+	if (hdr->type == 17 && hdr->length > 0x17) {
+
+		/* Found a memory channel */
+		dev_priv->dmi.mem_channel++;
+
+		/* Get the speed */
+		dev_priv->dmi.mem_speed = (uint16_t) (*((uint16_t *)(data + 0x15)));
+	}
+}
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
@@ -882,6 +898,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 
+	/* walk the dmi device table for getting platform memory information */
+	dmi_walk(dmi_decode_memory_info, (void *) dev_priv);
+
 	/* Setup the write-once "constant" device info */
 	device_info = (struct intel_device_info *)&dev_priv->info;
 	memcpy(device_info, info, sizeof(dev_priv->info));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 104bd18..f588993 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1962,6 +1962,12 @@ struct drm_i915_private {
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
 	 */
+
+	/* DMI data for memory bandwidth calculation */
+	struct {
+		uint16_t mem_channel;
+		uint16_t mem_speed;
+	} dmi;
 };
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
-- 
2.4.3

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

[PATCH 7/7] drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW

[Shobhit Kumar]

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

 If the arbitary display bandwidth is > 60% of memory bandwith, for
 x-tile we should increase latency at all levels by 15us.

 If the arbitary dsplay bandwidth is greater than 20% of memory bandwith
 in case of y-tile  being enabled, double the scan lines

v2: Update the commit message to explain the WA (shobhit)

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  9 +++++
 drivers/gpu/drm/i915/intel_pm.c | 86 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f588993..3c914a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1620,6 +1620,12 @@ enum intel_pipe_crc_source {
 	INTEL_PIPE_CRC_SOURCE_MAX,
 };
 
+enum watermark_memory_wa {
+	WATERMARK_WA_NONE,
+	WATERMARK_WA_X_TILED,
+	WATERMARK_WA_Y_TILED,
+};
+
 struct intel_pipe_crc_entry {
 	uint32_t frame;
 	uint32_t crc[5];
@@ -1915,6 +1921,9 @@ struct drm_i915_private {
 		/* Committed wm config */
 		struct intel_wm_config config;
 
+		/* This stores if WaterMark memory workaround is needed */
+		enum watermark_memory_wa mem_wa;
+
 		/*
 		 * The skl_wm_values structure is a bit too big for stack
 		 * allocation, so we keep the staging struct where we store
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dc08494..fb59f4e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3304,6 +3304,11 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	if (latency == 0 || !cstate->base.active || !fb)
 		return false;
 
+	if (dev_priv->wm.mem_wa != WATERMARK_WA_NONE) {
+		if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
+			latency += 15;
+	}
+
 	if (drm_rect_width(&intel_pstate->src)) {
 		width = drm_rect_width(&intel_pstate->src) >> 16;
 		height = drm_rect_height(&intel_pstate->src) >> 16;
@@ -3352,6 +3357,9 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				WARN(1, "Unsupported pixel depth for rotation");
 			}
 		}
+		if (dev_priv->wm.mem_wa == WATERMARK_WA_Y_TILED)
+			min_scanlines *= 2;
+
 		y_tile_minimum = plane_blocks_per_line * min_scanlines;
 		selected_result = max(method2, y_tile_minimum);
 	} else {
@@ -3803,6 +3811,83 @@ static void skl_set_plane_pixel_rate(struct drm_crtc *crtc)
 
 }
 
+static void
+skl_set_display_memory_wa(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = NULL;
+	struct intel_plane *intel_plane = NULL;
+	uint32_t num_active_crtc = 0;
+	uint64_t max_pixel_rate_pipe = 0;
+	uint64_t display_bw = 0, available_bw = 0;
+	bool y_tile_enabled = false;
+	int memory_portion = 0;
+
+	for_each_intel_crtc(dev, intel_crtc) {
+		uint64_t max_pixel_rate_plane = 0;
+		uint64_t pipe_bw;
+		uint32_t num_active_plane = 0;
+		const struct intel_crtc_state *cstate = NULL;
+
+		if (!intel_crtc->active)
+			continue;
+		cstate = to_intel_crtc_state(intel_crtc->base.state);
+		num_active_crtc++;
+
+		for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+			struct drm_plane *plane = &intel_plane->base;
+			struct drm_framebuffer *fb = plane->state->fb;
+			uint64_t plane_bw, interm_bw = 10000000;
+
+			if (fb == NULL)
+				continue;
+			if (plane->type == DRM_PLANE_TYPE_CURSOR)
+				continue;
+			num_active_plane++;
+
+			if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED)
+				y_tile_enabled = true;
+
+			/*
+			 * planeBW = pixel_rate(MHz) * BPP * plane downscale
+			 *		amount * pipe downscale amount;
+			 *
+			 * skl_pipe_pixel_rate return adjusted value according
+			 * to downscaling  amount
+			 * pixel rate is in KHz & downscale factor is multiplied
+			 * by 1000, so devide by 1000*1000
+			 */
+			interm_bw = skl_pipe_pixel_rate(cstate) *
+				drm_format_plane_cpp(fb->pixel_format, 0) *
+				skl_plane_downscale_amount(intel_plane);
+
+			if (fb->pixel_format == DRM_FORMAT_NV12)
+				interm_bw += skl_pipe_pixel_rate(cstate) *
+				drm_format_plane_cpp(fb->pixel_format, 1) *
+				skl_plane_downscale_amount(intel_plane);
+
+			plane_bw = DIV_ROUND_UP(interm_bw, (uint64_t) (1000 *
+						1000));
+			max_pixel_rate_plane = max(max_pixel_rate_plane,
+					plane_bw);
+		}
+		pipe_bw = max_pixel_rate_plane * num_active_plane;
+		max_pixel_rate_pipe = max(max_pixel_rate_pipe, pipe_bw);
+	}
+	display_bw = max_pixel_rate_pipe * num_active_crtc;
+
+
+	available_bw = dev_priv->dmi.mem_channel * dev_priv->dmi.mem_speed * 8;
+	memory_portion = DIV_ROUND_UP((display_bw * 100), available_bw);
+
+	if (y_tile_enabled && (memory_portion >= 20))
+		dev_priv->wm.mem_wa = WATERMARK_WA_Y_TILED;
+	else if (memory_portion >= 60)
+		dev_priv->wm.mem_wa = WATERMARK_WA_X_TILED;
+	else
+		dev_priv->wm.mem_wa = WATERMARK_WA_NONE;
+}
+
 static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
 {
 	watermarks->wm_linetime[pipe] = 0;
@@ -3840,6 +3925,7 @@ static void skl_update_wm(struct drm_crtc *crtc)
 
 	/* Calculate plane pixel rate for each plane in advance */
 	skl_set_plane_pixel_rate(crtc);
+	skl_set_display_memory_wa(dev);
 
 	if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
 		return;
-- 
2.4.3

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on 058740f8fced6851aeda34f366f5330322cd585f drm-intel-nightly: 2016y-01m-13d-17h-07m-44s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                dmesg-warn -> PASS       (bdw-nuci7)
                pass       -> DMESG-WARN (bdw-ultra)

bdw-nuci7        total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:138  pass:131  dwarn:1   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:141  pass:100  dwarn:4   dfail:0   fail:0   skip:37 
ivb-t430s        total:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1186/

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

Re: [PATCH 7/7] drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

Hi Mahesh,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20160114]
[cannot apply to v4.4]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Shobhit-Kumar/Misc-WM-fixes-and-Arbitrated-Display-Bandwidth-WA-for-SKL/20160114-200444
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-defconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `skl_update_wm':
>> intel_pm.c:(.text+0xdcbfb): undefined reference to `__udivdi3'
   intel_pm.c:(.text+0xdccb7): undefined reference to `__udivdi3'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23967 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 7/7] drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

Hi Mahesh,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20160114]
[cannot apply to v4.4]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Shobhit-Kumar/Misc-WM-fixes-and-Arbitrated-Display-Bandwidth-WA-for-SKL/20160114-200444
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-c0-01142249 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 27362 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 7/7] drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW

[Damien Lespiau]

On Thu, Jan 14, 2016 at 11:30:31PM +0800, kbuild test robot wrote:
> Hi Mahesh,
> 
> [auto build test ERROR on drm-intel/for-linux-next]
> [also build test ERROR on next-20160114]
> [cannot apply to v4.4]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Shobhit-Kumar/Misc-WM-fixes-and-Arbitrated-Display-Bandwidth-WA-for-SKL/20160114-200444
> base:   git://anongit.freedesktop.org/drm-intel for-linux-next
> config: i386-defconfig (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/built-in.o: In function `skl_update_wm':
> >> intel_pm.c:(.text+0xdcbfb): undefined reference to `__udivdi3'
>    intel_pm.c:(.text+0xdccb7): undefined reference to `__udivdi3'

In case you wonder, compiling for x86 32 bits, this is most likely
because DIV_ROUND_UP() uses a stray '/' operator and you use it with 64
bit values, which will make gcc use a run-time helper function that
isn't part of the kernel.

You need to use DIV_ROUND_UP_ULL(), making sure the second parameter is
32 bits only.

HTH,

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

Re: [PATCH 1/7] drm/i915/skl+: Use proper bytes_per_pixel during WM calculation

[Matt Roper]

On Thu, Jan 14, 2016 at 05:32:42PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> Don't always use bytes_per_pixel using y_plane=0, instead use it
> according to pixel format. If NV12 use y_plane eqal to 1
> 
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>

The second parameter to drm_format_plane_cpp() is the plane index
(0 => Y = 1bpp, 1 => UV = 2bpp), so I think passing 0 actually was what
we wanted, right?


Matt

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9df9e9a..68f21b9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3185,7 +3185,9 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	if (latency == 0 || !cstate->base.active || !fb)
>  		return false;
>  
> -	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
> +	bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
> +				drm_format_plane_cpp(fb->pixel_format, 1) :
> +				drm_format_plane_cpp(fb->pixel_format, 0);
>  	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
>  				 bytes_per_pixel,
>  				 latency);
> -- 
> 2.4.3
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/i915/skl+: Use fb size for relative data rate calculation

[Matt Roper]

On Thu, Jan 14, 2016 at 05:32:43PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> Use FB size for relative data rate calculation. don't always use

Minor wording nitpick:  it's not really the FB size you're using here
but rather the plane size.  The FB itself may be larger than the plane
and largely offscreen.

> pipe source width & height.
> adjust height & width according to rotation.
> 
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 42 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 68f21b9..d33c4ff 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2928,24 +2928,33 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  			     int y)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> +	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
>  	struct drm_framebuffer *fb = pstate->fb;
> +	uint32_t width = 0, height = 0;
> +
> +	if (drm_rect_width(&intel_pstate->src)) {
> +		width = drm_rect_width(&intel_pstate->src) >> 16;
> +		height = drm_rect_height(&intel_pstate->src) >> 16;
> +	} else {
> +		width = intel_crtc->config->pipe_src_w;
> +		height = intel_crtc->config->pipe_src_h;

The else block would mean the plane is invisible (either off or fully
clipped), right?  Shouldn't our relative data rate be 0 in that case
(instead of max possible)?


> +	}
> +
> +	if (intel_rotation_90_or_270(pstate->rotation))
> +		swap(width, height);
>  
>  	/* for planar format */
>  	if (fb->pixel_format == DRM_FORMAT_NV12) {
>  		if (y)  /* y-plane data rate */
> -			return intel_crtc->config->pipe_src_w *
> -				intel_crtc->config->pipe_src_h *
> +			return width * height *
>  				drm_format_plane_cpp(fb->pixel_format, 0);
>  		else    /* uv-plane data rate */
> -			return (intel_crtc->config->pipe_src_w/2) *
> -				(intel_crtc->config->pipe_src_h/2) *
> +			return (width / 2) * (height / 2) *
>  				drm_format_plane_cpp(fb->pixel_format, 1);
>  	}
>  
>  	/* for packed formats */
> -	return intel_crtc->config->pipe_src_w *
> -		intel_crtc->config->pipe_src_h *
> -		drm_format_plane_cpp(fb->pixel_format, 0);
> +	return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
>  }
>  
>  /*
> @@ -3181,10 +3190,25 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	uint32_t res_blocks, res_lines;
>  	uint32_t selected_result;
>  	uint8_t bytes_per_pixel;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> +	struct intel_plane_state *intel_pstate = to_intel_plane_state(plane->state);
> +	uint32_t width = 0, height = 0;
>  
>  	if (latency == 0 || !cstate->base.active || !fb)
>  		return false;
>  
> +	if (drm_rect_width(&intel_pstate->src)) {
> +		width = drm_rect_width(&intel_pstate->src) >> 16;
> +		height = drm_rect_height(&intel_pstate->src) >> 16;
> +	} else {
> +		width = intel_crtc->config->pipe_src_w;
> +		height = intel_crtc->config->pipe_src_h;
> +	}

We're already bailing above this on !fb; shouldn't we do the same on a
plane that has a valid fb but is disabled due to being fully clipped
(which I think is the only thing that will trigger the else block here)?


Matt

> +
> +	if (intel_rotation_90_or_270(plane->state->rotation))
> +		swap(width, height);
> +
> +	/* for planar format */
>  	bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
>  				drm_format_plane_cpp(fb->pixel_format, 1) :
>  				drm_format_plane_cpp(fb->pixel_format, 0);
> @@ -3193,12 +3217,12 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  				 latency);
>  	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
>  				 cstate->base.adjusted_mode.crtc_htotal,
> -				 cstate->pipe_src_w,
> +				 width,
>  				 bytes_per_pixel,
>  				 fb->modifier[0],
>  				 latency);
>  
> -	plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
> +	plane_bytes_per_line = width * bytes_per_pixel;
>  	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
>  
>  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> -- 
> 2.4.3
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915/skl+: calculate ddb minimum allocation

[Matt Roper]

On Thu, Jan 14, 2016 at 05:32:44PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> don't always use 8 ddb as minimum, instead calculate using proper
> algorithm.
> 
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 57 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d33c4ff..64b39ec 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2994,6 +2994,59 @@ skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
>  	return total_data_rate;
>  }
>  
> +static uint16_t
> +skl_ddb_min_alloc(const struct intel_crtc *crtc,
> +		const struct drm_plane *plane, int y)
> +{
> +	struct drm_framebuffer *fb = plane->state->fb;
> +	struct intel_plane_state *pstate = to_intel_plane_state(plane->state);
> +	uint16_t min_alloc;
> +	uint32_t src_w, src_h;
> +
> +	/* For packed formats, no y-plane, return 0 */
> +	if (y && !(fb->pixel_format == DRM_FORMAT_NV12))
> +		return 0;

Your general logic all looks fine, but I'd move the simple/common case
of !tile-y up here and immediately return the constant 8 to make it a
bit more obvious.  Then the remaining code below will only apply to
tile-y formats and you can even remove a level of nesting farther down.

> +
> +	if (drm_rect_width(&pstate->src)) {
> +		src_w = drm_rect_width(&pstate->src) >> 16;
> +		src_h = drm_rect_height(&pstate->src) >> 16;
> +	} else {
> +		src_w = crtc->config->pipe_src_w;
> +		src_h = crtc->config->pipe_src_h;
> +	}
> +
> +	if (intel_rotation_90_or_270(plane->state->rotation))
> +		swap(src_w, src_h);
> +
> +	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> +	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
> +		uint32_t min_scanlines = 8;
> +		uint8_t bytes_per_pixel =
> +			y ? drm_format_plane_cpp(fb->pixel_format, 1) :
> +				drm_format_plane_cpp(fb->pixel_format, 0);

Isn't this backwards?  As noted on an earlier patch, the second
parameter isn't a "is y" boolean, but rather a "which plane number"
integer.  For NV12, Y=0, UV=1.

> +
> +		if (intel_rotation_90_or_270(plane->state->rotation)) {
> +			switch (bytes_per_pixel) {
> +			case 1:
> +				min_scanlines = 32;
> +				break;
> +			case 2:
> +				min_scanlines = 16;
> +				break;
> +			case 8:
> +				WARN(1, "Unsupported pixel depth for rotation");
> +			}

Can we simplify this to a min_scalines = 32/bpp instead of the switch statement?


Matt

> +		}
> +		min_alloc = DIV_ROUND_UP((4 * src_w / (y ? 1 : 2) *
> +				bytes_per_pixel), 512) * min_scanlines/4 + 3;
> +	} else {
> +		min_alloc = 8;
> +	}
> +
> +	return min_alloc;
> +}
> +
> +
>  static void
>  skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  		      struct skl_ddb_allocation *ddb /* out */)
> @@ -3038,9 +3091,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  		if (plane->type == DRM_PLANE_TYPE_CURSOR)
>  			continue;
>  
> -		minimum[id] = 8;
> +		minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 0);
>  		alloc_size -= minimum[id];
> -		y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
> +		y_minimum[id] = skl_ddb_min_alloc(intel_crtc, plane, 1);
>  		alloc_size -= y_minimum[id];
>  	}
>  
> -- 
> 2.4.3
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915/skl+: calculate plane pixel rate.

[Matt Roper]

On Thu, Jan 14, 2016 at 05:32:45PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> Don't use pipe pixel rate for plane pixel rate.
> Calculate plane pixel according to formula
> 
> adjusted plane_pixel_rate = adjusted pipe_pixel_rate * downscale ammount
> 
> downscale amount = max[1, src_h/dst_h] * max[1, src_w/dst_w]
> if 90/270 rotation use rotated width & height
> 
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  drivers/gpu/drm/i915/intel_pm.c  | 95 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 95 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 059b46e..49f237e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -661,6 +661,8 @@ struct intel_plane_wm_parameters {
>  	u64 tiling;
>  	unsigned int rotation;
>  	uint16_t fifo_size;
> +	 /* Stores the adjusted plane pixel rate for WM calculation */
> +	uint32_t plane_pixel_rate;
>  };
>  
>  struct intel_plane {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 64b39ec..ffcc56a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2841,6 +2841,54 @@ skl_wm_plane_id(const struct intel_plane *plane)
>  	}
>  }
>  
> +/*
> + * This function takes drm_plane_state as input
> + * and decides the downscale amount according to the formula
> + *
> + * downscale amount = Max[1, Horizontal source size / Horizontal dest size]
> + *
> + * Return value is multiplied by 1000 to retain fractional part
> + * Caller should take care of dividing & Rounding off the value

Would it be simpler to just continue using 16.16 fixed point format?
Also, are there any helpers in drm_rect.c that we can use to simplify
this at all?

> + */
> +static uint32_t
> +skl_plane_downscale_amount(const struct intel_plane *intel_plane)
> +{
> +	struct drm_plane_state *pstate = intel_plane->base.state;
> +	struct intel_crtc *crtc = to_intel_crtc(pstate->crtc);
> +	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> +	uint32_t downscale_h, downscale_w;
> +	uint32_t src_w, src_h, dst_w, dst_h, tmp;
> +
> +	if (drm_rect_width(&intel_pstate->src)) {
> +		src_w = drm_rect_width(&intel_pstate->src) >> 16;
> +		src_h = drm_rect_height(&intel_pstate->src) >> 16;
> +	} else {
> +		src_w = crtc->config->pipe_src_w;
> +		src_h = crtc->config->pipe_src_h;
> +	}
> +
> +	dst_w = drm_rect_width(&intel_pstate->dst);
> +	dst_h = drm_rect_height(&intel_pstate->dst);
> +
> +	if (intel_rotation_90_or_270(pstate->rotation))
> +		swap(dst_w, dst_h);
> +
> +	/* If destination height & wight are zero return amount as unity */
> +	if (dst_w == 0 || dst_h == 0)
> +		return 1000;
> +
> +	/* Multiply by 1000 for precision */
> +	tmp = (1000 * src_h) / dst_h;
> +	downscale_h = max_t(uint32_t, 1000, tmp);
> +
> +	tmp = (1000 * src_w) / dst_w;
> +	downscale_w = max_t(uint32_t, 1000, tmp);
> +
> +	/* Reducing precision to 3 decimal places */
> +	return DIV_ROUND_UP(downscale_h * downscale_w, 1000);
> +}
> +
> +
>  static void
>  skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
>  				   const struct intel_crtc_state *cstate,
> @@ -3265,10 +3313,10 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
>  				drm_format_plane_cpp(fb->pixel_format, 1) :
>  				drm_format_plane_cpp(fb->pixel_format, 0);
> -	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
> +	method1 = skl_wm_method1(intel_plane->wm.plane_pixel_rate,
>  				 bytes_per_pixel,
>  				 latency);
> -	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
> +	method2 = skl_wm_method2(intel_plane->wm.plane_pixel_rate,
>  				 cstate->base.adjusted_mode.crtc_htotal,
>  				 width,
>  				 bytes_per_pixel,
> @@ -3709,6 +3757,46 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
>  	}
>  }
>  
> +static uint32_t
> +skl_plane_pixel_rate(struct intel_crtc_state *cstate, struct intel_plane *plane)
> +{
> +	uint32_t adjusted_pixel_rate;
> +	uint32_t downscale_amount;
> +
> +	/*
> +	 * adjusted plane pixel rate = adjusted pipe pixel rate
> +	 * Plane pixel rate = adjusted plane pixel rate * plane down scale
> +	 * amount
> +	 */
> +	adjusted_pixel_rate = skl_pipe_pixel_rate(cstate);
> +	downscale_amount = skl_plane_downscale_amount(plane);
> +
> +	return DIV_ROUND_UP(adjusted_pixel_rate * downscale_amount,
> +			1000);
> +}
> +
> +static void skl_set_plane_pixel_rate(struct drm_crtc *crtc)
> +{
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> +	struct intel_plane *intel_plane = NULL;
> +	struct drm_device *dev = crtc->dev;
> +
> +	if (!intel_crtc->active)
> +		return;
> +	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> +		struct drm_plane *plane = &intel_plane->base;
> +		struct drm_framebuffer *fb = plane->state->fb;
> +
> +		if (fb == NULL)
> +			continue;

Maybe change this to a test of intel_plane_state->visible instead so
that we're not bothering with this for planes that actually do have an
FB assigned but are fully clipped?

Then you'll also be able to eliminate the 'else' block of the non-zero
width test farther up.


Matt

> +
> +		intel_plane->wm.plane_pixel_rate = skl_plane_pixel_rate(cstate,
> +				intel_plane);
> +	}
> +
> +}
> +
>  static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
>  {
>  	watermarks->wm_linetime[pipe] = 0;
> @@ -3744,6 +3832,9 @@ static void skl_update_wm(struct drm_crtc *crtc)
>  
>  	skl_clear_wm(results, intel_crtc->pipe);
>  
> +	/* Calculate plane pixel rate for each plane in advance */
> +	skl_set_plane_pixel_rate(crtc);
> +
>  	if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
>  		return;
>  
> -- 
> 2.4.3
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915/skl+: Use scaling amount for plane data rate calculation

[Matt Roper]

On Thu, Jan 14, 2016 at 05:32:46PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ffcc56a..dc08494 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2978,6 +2978,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
>  	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
>  	struct drm_framebuffer *fb = pstate->fb;
> +	struct intel_plane *intel_plane = to_intel_plane(pstate->plane);
> +	uint32_t down_scale_amount, data_rate;
>  	uint32_t width = 0, height = 0;
>  
>  	if (drm_rect_width(&intel_pstate->src)) {
> @@ -2994,15 +2996,19 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  	/* for planar format */
>  	if (fb->pixel_format == DRM_FORMAT_NV12) {
>  		if (y)  /* y-plane data rate */
> -			return width * height *
> +			data_rate = width * height *
>  				drm_format_plane_cpp(fb->pixel_format, 0);
>  		else    /* uv-plane data rate */
> -			return (width / 2) * (height / 2) *
> +			data_rate = (width / 2) * (height / 2) *
>  				drm_format_plane_cpp(fb->pixel_format, 1);
>  	}
>  
>  	/* for packed formats */
> -	return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
> +	data_rate = width * height * drm_format_plane_cpp(fb->pixel_format, 0);

This needs to be moved into an 'else' block now, otherwise you'll
clobber the NV12 values you calculated above.


Matt

> +	down_scale_amount = skl_plane_downscale_amount(intel_plane);
> +
> +	return DIV_ROUND_UP((data_rate * down_scale_amount), 1000);
> +
>  }
>  
>  /*
> -- 
> 2.4.3
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915: Add support to parse DMI table and get platform memory info

[Matt Roper]

On Thu, Jan 14, 2016 at 05:32:47PM +0530, Shobhit Kumar wrote:
> This is needed for WM computation workaround for arbitrated display
> bandwidth.
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 19 +++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index a0f5659..03c3131 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -49,6 +49,7 @@
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/oom.h>
> +#include <linux/dmi.h>
>  
>  
>  static int i915_getparam(struct drm_device *dev, void *data,
> @@ -855,6 +856,21 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> +static void dmi_decode_memory_info(const struct dmi_header *hdr, void *priv)
> +{
> +	struct drm_i915_private *dev_priv = (struct drm_i915_private *) priv;
> +	const u8 *data = (const u8 *) hdr;
> +
> +	if (hdr->type == 17 && hdr->length > 0x17) {

I think there are some constants in include/linux/dmi.h that we can use
instead of magic numbers.  DMI_ENTRY_MEM_DEVICE.

I'm not familiar with any of this DMI stuff; it seems like there are
functions elsewhere in the kernel (e.g., decode_dclk() in
drivers/edac/i7core_edac.c) that decode this into a memdev_dmi_entry and
then do some more robust/paranoid checking to determine the actual
speed.  Unfortunately it's static to that file and not directly callable
by us.

I wonder if it would make sense to refactor that structure and decoding
out into the general DMI files and export them for module use so that we
could just call those functions.


Matt

> +
> +		/* Found a memory channel */
> +		dev_priv->dmi.mem_channel++;
> +
> +		/* Get the speed */
> +		dev_priv->dmi.mem_speed = (uint16_t) (*((uint16_t *)(data + 0x15)));
> +	}
> +}
> +
>  /**
>   * i915_driver_load - setup chip and create an initial config
>   * @dev: DRM device
> @@ -882,6 +898,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	dev->dev_private = dev_priv;
>  	dev_priv->dev = dev;
>  
> +	/* walk the dmi device table for getting platform memory information */
> +	dmi_walk(dmi_decode_memory_info, (void *) dev_priv);
> +
>  	/* Setup the write-once "constant" device info */
>  	device_info = (struct intel_device_info *)&dev_priv->info;
>  	memcpy(device_info, info, sizeof(dev_priv->info));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 104bd18..f588993 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1962,6 +1962,12 @@ struct drm_i915_private {
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> +
> +	/* DMI data for memory bandwidth calculation */
> +	struct {
> +		uint16_t mem_channel;
> +		uint16_t mem_speed;
> +	} dmi;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> -- 
> 2.4.3
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/7] Misc WM fixes and Arbitrated Display Bandwidth WA for SKL

[Matt Roper]

On Thu, Jan 14, 2016 at 05:32:41PM +0530, Shobhit Kumar wrote:
> Hi,
> This series add a set of updates to the WM calculation and also enables
> arbitrated display bandwidth based WA. Some of these patches do overlap
> with Matts work but we wanted to send them out as we have them in our
> internal testing for early review. Most likley some of them can be
> superceded by patches from Matt, or can be re-used if deemed necessary.

I've left some feedback on your patches, but it was all pretty minor;
they look pretty good overall.  So I think we should polish up this set
and merge it; I've got more SKL WM work coming (more atomic-y watermark
calculation and programming) but I'll hold off on that until your series
lands so that I don't cause too much thrash.

Thanks.


Matt

> 
> Especially "drm/i915/skl+: Use fb size for relative data rate calculation"
> this already addresses some of Ville's comment on similar patch from Matt.
> 
> Regards
> Shobhit
> 
> Kumar, Mahesh (6):
>   drm/i915/skl+: Use proper bytes_per_pixel during WM calculation
>   drm/i915/skl+: Use fb size for relative data rate calculation
>   drm/i915/skl+: calculate ddb minimum allocation
>   drm/i915/skl+: calculate plane pixel rate.
>   drm/i915/skl+: Use scaling amount for plane data rate calculation
>   drm/i915/skl: WA for watermark calculation based on Arbitrated Display
>     BW
> 
> Shobhit Kumar (1):
>   drm/i915: Add support to parse DMI table and get platform memory info
> 
>  drivers/gpu/drm/i915/i915_dma.c  |  19 +++
>  drivers/gpu/drm/i915/i915_drv.h  |  15 ++
>  drivers/gpu/drm/i915/intel_drv.h |   2 +
>  drivers/gpu/drm/i915/intel_pm.c  | 294 +++++++++++++++++++++++++++++++++++++--
>  4 files changed, 316 insertions(+), 14 deletions(-)
> 
> -- 
> 2.4.3
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/7] Misc WM fixes and Arbitrated Display Bandwidth WA for SKL

[Kumar, Shobhit]

On 01/15/2016 07:18 AM, Matt Roper wrote:
> On Thu, Jan 14, 2016 at 05:32:41PM +0530, Shobhit Kumar wrote:
>> Hi,
>> This series add a set of updates to the WM calculation and also enables
>> arbitrated display bandwidth based WA. Some of these patches do overlap
>> with Matts work but we wanted to send them out as we have them in our
>> internal testing for early review. Most likley some of them can be
>> superceded by patches from Matt, or can be re-used if deemed necessary.
>
> I've left some feedback on your patches, but it was all pretty minor;
> they look pretty good overall.  So I think we should polish up this set
> and merge it; I've got more SKL WM work coming (more atomic-y watermark
> calculation and programming) but I'll hold off on that until your series
> lands so that I don't cause too much thrash.

Thanks Matt for the quick review. I will get the comments addressed and 
final patches uploaded quickly.

Regards
Shobhit

>
> Thanks.
>
>
> Matt
>
>>
>> Especially "drm/i915/skl+: Use fb size for relative data rate calculation"
>> this already addresses some of Ville's comment on similar patch from Matt.
>>
>> Regards
>> Shobhit
>>
>> Kumar, Mahesh (6):
>>    drm/i915/skl+: Use proper bytes_per_pixel during WM calculation
>>    drm/i915/skl+: Use fb size for relative data rate calculation
>>    drm/i915/skl+: calculate ddb minimum allocation
>>    drm/i915/skl+: calculate plane pixel rate.
>>    drm/i915/skl+: Use scaling amount for plane data rate calculation
>>    drm/i915/skl: WA for watermark calculation based on Arbitrated Display
>>      BW
>>
>> Shobhit Kumar (1):
>>    drm/i915: Add support to parse DMI table and get platform memory info
>>
>>   drivers/gpu/drm/i915/i915_dma.c  |  19 +++
>>   drivers/gpu/drm/i915/i915_drv.h  |  15 ++
>>   drivers/gpu/drm/i915/intel_drv.h |   2 +
>>   drivers/gpu/drm/i915/intel_pm.c  | 294 +++++++++++++++++++++++++++++++++++++--
>>   4 files changed, 316 insertions(+), 14 deletions(-)
>>
>> --
>> 2.4.3
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915/skl+: Use proper bytes_per_pixel during WM calculation

[Kumar, Shobhit]

On 01/15/2016 12:37 AM, Matt Roper wrote:
> On Thu, Jan 14, 2016 at 05:32:42PM +0530, Shobhit Kumar wrote:
>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>
>> Don't always use bytes_per_pixel using y_plane=0, instead use it
>> according to pixel format. If NV12 use y_plane eqal to 1
>>
>> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
>
> The second parameter to drm_format_plane_cpp() is the plane index
> (0 => Y = 1bpp, 1 => UV = 2bpp), so I think passing 0 actually was what
> we wanted, right?
>

Yes, I guess it was an oversight. Might have to test again for NV12 but 
for now we can drop this patch.

Regards
Shobhit

>
> Matt
>
>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 9df9e9a..68f21b9 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3185,7 +3185,9 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>>   	if (latency == 0 || !cstate->base.active || !fb)
>>   		return false;
>>
>> -	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
>> +	bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
>> +				drm_format_plane_cpp(fb->pixel_format, 1) :
>> +				drm_format_plane_cpp(fb->pixel_format, 0);
>>   	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
>>   				 bytes_per_pixel,
>>   				 latency);
>> --
>> 2.4.3
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/7] Misc WM fixes and Arbitrated Display Bandwidth WA for SKL

[Shobhit Kumar]

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

On Fri, Jan 15, 2016 at 10:32 AM, Kumar, Shobhit <
shobhit.kumar@linux.intel.com> wrote:

> On 01/15/2016 07:18 AM, Matt Roper wrote:
>
>> On Thu, Jan 14, 2016 at 05:32:41PM +0530, Shobhit Kumar wrote:
>>
>>> Hi,
>>> This series add a set of updates to the WM calculation and also enables
>>> arbitrated display bandwidth based WA. Some of these patches do overlap
>>> with Matts work but we wanted to send them out as we have them in our
>>> internal testing for early review. Most likley some of them can be
>>> superceded by patches from Matt, or can be re-used if deemed necessary.
>>>
>>
>> I've left some feedback on your patches, but it was all pretty minor;
>> they look pretty good overall.  So I think we should polish up this set
>> and merge it; I've got more SKL WM work coming (more atomic-y watermark
>> calculation and programming) but I'll hold off on that until your series
>> lands so that I don't cause too much thrash.
>>
>
> Thanks Matt for the quick review. I will get the comments addressed and
> final patches uploaded quickly.
>

We have the updated patches ready. Just completing some testing. Should be
posting sometime this week.

Regards
Shobhit


>
> Regards
> Shobhit
>
>
>> Thanks.
>>
>>
>> Matt
>>
>>
>>> Especially "drm/i915/skl+: Use fb size for relative data rate
>>> calculation"
>>> this already addresses some of Ville's comment on similar patch from
>>> Matt.
>>>
>>> Regards
>>> Shobhit
>>>
>>> Kumar, Mahesh (6):
>>>    drm/i915/skl+: Use proper bytes_per_pixel during WM calculation
>>>    drm/i915/skl+: Use fb size for relative data rate calculation
>>>    drm/i915/skl+: calculate ddb minimum allocation
>>>    drm/i915/skl+: calculate plane pixel rate.
>>>    drm/i915/skl+: Use scaling amount for plane data rate calculation
>>>    drm/i915/skl: WA for watermark calculation based on Arbitrated Display
>>>      BW
>>>
>>> Shobhit Kumar (1):
>>>    drm/i915: Add support to parse DMI table and get platform memory info
>>>
>>>   drivers/gpu/drm/i915/i915_dma.c  |  19 +++
>>>   drivers/gpu/drm/i915/i915_drv.h  |  15 ++
>>>   drivers/gpu/drm/i915/intel_drv.h |   2 +
>>>   drivers/gpu/drm/i915/intel_pm.c  | 294
>>> +++++++++++++++++++++++++++++++++++++--
>>>   4 files changed, 316 insertions(+), 14 deletions(-)
>>>
>>> --
>>> 2.4.3
>>>
>>>
>> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

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

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

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

Re: [PATCH 6/7] drm/i915: Add support to parse DMI table and get platform memory info

[Kumar, Shobhit]

On 01/15/2016 07:14 AM, Matt Roper wrote:
> On Thu, Jan 14, 2016 at 05:32:47PM +0530, Shobhit Kumar wrote:
>> This is needed for WM computation workaround for arbitrated display
>> bandwidth.
>>
>> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_dma.c | 19 +++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index a0f5659..03c3131 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -49,6 +49,7 @@
>>   #include <linux/pm.h>
>>   #include <linux/pm_runtime.h>
>>   #include <linux/oom.h>
>> +#include <linux/dmi.h>
>>
>>
>>   static int i915_getparam(struct drm_device *dev, void *data,
>> @@ -855,6 +856,21 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv)
>>   	}
>>   }
>>
>> +static void dmi_decode_memory_info(const struct dmi_header *hdr, void *priv)
>> +{
>> +	struct drm_i915_private *dev_priv = (struct drm_i915_private *) priv;
>> +	const u8 *data = (const u8 *) hdr;
>> +
>> +	if (hdr->type == 17 && hdr->length > 0x17) {
>
> I think there are some constants in include/linux/dmi.h that we can use
> instead of magic numbers.  DMI_ENTRY_MEM_DEVICE.
>

Yeah will do that.

> I'm not familiar with any of this DMI stuff; it seems like there are
> functions elsewhere in the kernel (e.g., decode_dclk() in
> drivers/edac/i7core_edac.c) that decode this into a memdev_dmi_entry and
> then do some more robust/paranoid checking to determine the actual
> speed.  Unfortunately it's static to that file and not directly callable
> by us.
>
> I wonder if it would make sense to refactor that structure and decoding
> out into the general DMI files and export them for module use so that we
> could just call those functions.
>

Perhaps we can but that will take some time. I will add paranoid parsing 
as done in edac with minimal #defined offsets for now. Think that should 
suffice. At next level we can work on exporting new functionality from 
the DMI files.

Regards
Shobhit

>
> Matt
>
>> +
>> +		/* Found a memory channel */
>> +		dev_priv->dmi.mem_channel++;
>> +
>> +		/* Get the speed */
>> +		dev_priv->dmi.mem_speed = (uint16_t) (*((uint16_t *)(data + 0x15)));
>> +	}
>> +}
>> +
>>   /**
>>    * i915_driver_load - setup chip and create an initial config
>>    * @dev: DRM device
>> @@ -882,6 +898,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>>   	dev->dev_private = dev_priv;
>>   	dev_priv->dev = dev;
>>
>> +	/* walk the dmi device table for getting platform memory information */
>> +	dmi_walk(dmi_decode_memory_info, (void *) dev_priv);
>> +
>>   	/* Setup the write-once "constant" device info */
>>   	device_info = (struct intel_device_info *)&dev_priv->info;
>>   	memcpy(device_info, info, sizeof(dev_priv->info));
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 104bd18..f588993 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1962,6 +1962,12 @@ struct drm_i915_private {
>>   	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>>   	 * will be rejected. Instead look for a better place.
>>   	 */
>> +
>> +	/* DMI data for memory bandwidth calculation */
>> +	struct {
>> +		uint16_t mem_channel;
>> +		uint16_t mem_speed;
>> +	} dmi;
>>   };
>>
>>   static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
>> --
>> 2.4.3
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx