[PATCH 0/8] SKL WM fixes and Arbitrated Display Bandwidth WA

[PATCH 0/8] SKL WM fixes and Arbitrated Display Bandwidth WA

[Matt Roper]

This series of patches was mostly written by Mahesh and Shobhit; the last
iteration posted was by Shobhit here:

        https://patchwork.freedesktop.org/series/2881/

I believe they've been busy recently and haven't had time to revisit this work,
so I've rebased their patches to the latest upstream tree, incorporated the
review feedback I previously gave, and added a couple more fixes on top.

Kumar, Mahesh (5):
  drm/i915/skl+: Use plane size for relative data rate calculation
  drm/i915/skl+: calculate ddb minimum allocation (v3)
  drm/i915/skl+: calculate plane pixel rate (v3)
  drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)
  drm/i915/skl: WA for watermark calculation based on Arbitrated Display
    BW (v4)

Matt Roper (2):
  drm/i915/gen9: Hold wm_mutex around SKL watermark updates
  dmi: Move memdev_dmi_entry definition to dmi.h

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

 drivers/edac/ghes_edac.c         |  26 ----
 drivers/edac/i7core_edac.c       |  25 ----
 drivers/gpu/drm/i915/i915_dma.c  |  34 +++++
 drivers/gpu/drm/i915/i915_drv.h  |  16 +++
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 drivers/gpu/drm/i915/intel_pm.c  | 292 ++++++++++++++++++++++++++++++++++++---
 include/linux/dmi.h              |  25 ++++
 7 files changed, 347 insertions(+), 74 deletions(-)

-- 
2.1.4

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

[PATCH 1/8] drm/i915/skl+: Use plane size for relative data rate calculation

[Matt Roper]

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

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

v2: Address Matt's comments.
    Use intel_plane_state->visible to avoid divide-by-zero error.
    Where FB was present but not visible so causing total data rate to
    be zero, hence divide-by-zero.

Cc: matthew.d.roper@intel.com
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 45 ++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f65e841..9e88e0c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2934,25 +2934,28 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 			     const struct drm_plane_state *pstate,
 			     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;
+
+	width = drm_rect_width(&intel_pstate->src) >> 16;
+	height = drm_rect_height(&intel_pstate->src) >> 16;
+
+	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);
 }
 
 /*
@@ -2971,9 +2974,8 @@ skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
 	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
 		const struct drm_plane_state *pstate = intel_plane->base.state;
 
-		if (pstate->fb == NULL)
+		if (!to_intel_plane_state(pstate)->visible)
 			continue;
-
 		if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
 			continue;
 
@@ -3031,8 +3033,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		struct drm_framebuffer *fb = plane->state->fb;
 		int id = skl_wm_plane_id(intel_plane);
 
-		if (fb == NULL)
+		if (!to_intel_plane_state(plane->state)->visible)
 			continue;
+
 		if (plane->type == DRM_PLANE_TYPE_CURSOR)
 			continue;
 
@@ -3058,7 +3061,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		uint16_t plane_blocks, y_plane_blocks = 0;
 		int id = skl_wm_plane_id(intel_plane);
 
-		if (pstate->fb == NULL)
+		if (!to_intel_plane_state(pstate)->visible)
 			continue;
 		if (plane->type == DRM_PLANE_TYPE_CURSOR)
 			continue;
@@ -3181,26 +3184,36 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 {
 	struct drm_plane *plane = &intel_plane->base;
 	struct drm_framebuffer *fb = plane->state->fb;
+	struct intel_plane_state *intel_pstate =
+					to_intel_plane_state(plane->state);
 	uint32_t latency = dev_priv->wm.skl_latency[level];
 	uint32_t method1, method2;
 	uint32_t plane_bytes_per_line, plane_blocks_per_line;
 	uint32_t res_blocks, res_lines;
 	uint32_t selected_result;
 	uint8_t cpp;
+	uint32_t width = 0, height = 0;
 
-	if (latency == 0 || !cstate->base.active || !fb)
+	if (latency == 0 || !cstate->base.active || !intel_pstate->visible)
 		return false;
 
+	width = drm_rect_width(&intel_pstate->src) >> 16;
+	height = drm_rect_height(&intel_pstate->src) >> 16;
+
+	if (intel_rotation_90_or_270(plane->state->rotation))
+		swap(width, height);
+
 	cpp = drm_format_plane_cpp(fb->pixel_format, 0);
 	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
 				 cpp, latency);
 	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
 				 cstate->base.adjusted_mode.crtc_htotal,
-				 cstate->pipe_src_w,
-				 cpp, fb->modifier[0],
+				 width,
+				 cpp,
+				 fb->modifier[0],
 				 latency);
 
-	plane_bytes_per_line = cstate->pipe_src_w * cpp;
+	plane_bytes_per_line = width * cpp;
 	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 
 	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
-- 
2.1.4

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

[PATCH 2/8] drm/i915/skl+: calculate ddb minimum allocation (v3)

[Matt Roper]

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

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

v2: optimizations as per Matt's comments.

v3 (by Matt):
 - Fix boolean logic for !fb test in skl_ddb_min_alloc()
 - Adjust negative tiling format comparisons in skl_ddb_min_alloc() to
   improve readability.

Cc: matthew.d.roper@intel.com
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9e88e0c..d8ab9f1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2994,6 +2994,51 @@ 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);
+	uint32_t src_w, src_h;
+	uint32_t min_scanlines = 8;
+	uint8_t bytes_per_pixel;
+
+	/* For packed formats, no y-plane, return 0 */
+	if (!fb || (y && fb->pixel_format != DRM_FORMAT_NV12))
+		return 0;
+
+	/* For Non Y-tile return 8-blocks */
+	if (fb->modifier[0] != I915_FORMAT_MOD_Y_TILED &&
+	    fb->modifier[0] != I915_FORMAT_MOD_Yf_TILED)
+		return 8;
+
+	src_w = drm_rect_width(&pstate->src) >> 16;
+	src_h = drm_rect_height(&pstate->src) >> 16;
+
+	if (intel_rotation_90_or_270(plane->state->rotation))
+		swap(src_w, src_h);
+
+	bytes_per_pixel = y ? drm_format_plane_cpp(fb->pixel_format, 0) :
+		drm_format_plane_cpp(fb->pixel_format, 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");
+		}
+	}
+
+	return DIV_ROUND_UP((4 * src_w / (y ? 1 : 2) * bytes_per_pixel), 512) *
+							min_scanlines/4 + 3;
+}
+
 static void
 skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 		      struct skl_ddb_allocation *ddb /* out */)
@@ -3030,7 +3075,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
 	/* 1. Allocate the mininum required blocks for each active plane */
 	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;
 		int id = skl_wm_plane_id(intel_plane);
 
 		if (!to_intel_plane_state(plane->state)->visible)
@@ -3039,9 +3083,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.1.4

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

[PATCH 3/8] drm/i915/skl+: calculate plane pixel rate (v3)

[Matt Roper]

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

v2: use intel_plane_state->visible instead of (fb == NULL) as per Matt's
    comment.

v3 (by Matt):
 - Keep downscale amount in 16.16 fixed point rather than converting to
   decimal fixed point.
 - Store adjusted plane pixel rate in plane state instead of the plane
   parameters structure that we no longer use.

Cc: matthew.d.roper@intel.com
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |  3 ++
 drivers/gpu/drm/i915/intel_pm.c  | 85 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3daf1e3..f022911 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -334,6 +334,9 @@ struct intel_plane_state {
 
 	/* async flip related structures */
 	struct drm_i915_gem_request *wait_req;
+
+	/* Stores the adjusted plane pixel rate for WM calculation for SKL+ */
+	uint32_t plane_pixel_rate;
 };
 
 struct intel_initial_plane_config {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d8ab9f1..e828bde 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -26,6 +26,7 @@
  */
 
 #include <linux/cpufreq.h>
+#include <drm/drm_plane_helper.h>
 #include "i915_drv.h"
 #include "intel_drv.h"
 #include "../../../platform/x86/intel_ips.h"
@@ -2843,6 +2844,43 @@ 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 provided in 16.16 fixed point form 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_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;
+
+	/* If plane not visible return amount as unity */
+	if (!intel_pstate->visible)
+		return DRM_PLANE_HELPER_NO_SCALING;
+
+	/* n.b., src is 16.16 fixed point, dst is whole integer */
+	src_w = drm_rect_width(&intel_pstate->src);
+	src_h = drm_rect_height(&intel_pstate->src);
+	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);
+
+	downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
+	downscale_w = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
+
+	/* Provide result in 16.16 fixed point */
+	return (uint64_t)downscale_w * downscale_h >> 16;
+}
+
+
 static void
 skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 				   const struct intel_crtc_state *cstate,
@@ -3248,9 +3286,9 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 		swap(width, height);
 
 	cpp = drm_format_plane_cpp(fb->pixel_format, 0);
-	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
+	method1 = skl_wm_method1(intel_pstate->plane_pixel_rate,
 				 cpp, latency);
-	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
+	method2 = skl_wm_method2(intel_pstate->plane_pixel_rate,
 				 cstate->base.adjusted_mode.crtc_htotal,
 				 width,
 				 cpp,
@@ -3691,6 +3729,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)
+{
+	uint64_t adjusted_pixel_rate;
+	uint64_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 (uint32_t)(adjusted_pixel_rate * downscale_amount >> 16);
+}
+
+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 intel_plane_state *pstate =
+			to_intel_plane_state(plane->state);
+
+		if (!pstate->visible)
+			continue;
+
+		pstate->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;
@@ -3726,6 +3804,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.1.4

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

[PATCH 4/8] drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)

[Matt Roper]

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

if downscaling is enabled plane data rate increases according to scaling
amount. take scaling amount under consideration while calculating plane
data rate

v2: Address Matt's comments, where data rate was overridden because of
missing else.

v3 (by Matt):
 - Add braces to 'else' branch to match kernel coding style
 - Adjust final calculation now that skl_plane_downscale_amount()
   returns 16.16 fixed point value instead of a decimal fixed point

Cc: matthew.d.roper@intel.com
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
Reviewed-by(v2): Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e828bde..041db5d3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2974,6 +2974,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 {
 	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;
 
 	width = drm_rect_width(&intel_pstate->src) >> 16;
@@ -2985,15 +2987,20 @@ 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);
+	} else {
+		/* for packed formats */
+		data_rate = width * height *
+			drm_format_plane_cpp(fb->pixel_format, 0);
 	}
 
-	/* for packed formats */
-	return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
+	down_scale_amount = skl_plane_downscale_amount(intel_plane);
+
+	return data_rate * down_scale_amount >> 16;
 }
 
 /*
-- 
2.1.4

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

[PATCH 5/8] drm/i915/gen9: Hold wm_mutex around SKL watermark updates

[Matt Roper]

SKL watermark code uses a field in dev_priv to hold the watermark
results being calculated.  If independent commits are submitted against
disjoint CRTC's, the watermark updates could race and clobber each
other's usage of dev_priv->wm.skl_results.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 041db5d3..29d37d3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3805,6 +3805,7 @@ static void skl_update_wm(struct drm_crtc *crtc)
 	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
 	struct skl_pipe_wm *pipe_wm = &cstate->wm.optimal.skl;
 
+	mutex_lock(&dev_priv->wm.wm_mutex);
 
 	/* Clear all dirty flags */
 	memset(results->dirty, 0, sizeof(bool) * I915_MAX_PIPES);
@@ -3815,7 +3816,7 @@ static void skl_update_wm(struct drm_crtc *crtc)
 	skl_set_plane_pixel_rate(crtc);
 
 	if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
-		return;
+		goto out;
 
 	skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
 	results->dirty[intel_crtc->pipe] = true;
@@ -3826,6 +3827,9 @@ static void skl_update_wm(struct drm_crtc *crtc)
 
 	/* store the new configuration */
 	dev_priv->wm.skl_hw = *results;
+
+out:
+	mutex_unlock(&dev_priv->wm.wm_mutex);
 }
 
 static void ilk_compute_wm_config(struct drm_device *dev,
-- 
2.1.4

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

[PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h

[Matt Roper]

A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
decoding DMI memory device entries.  Move the structure definition to
dmi.h so that it can be shared between those drivers and also other
parts of the kernel; the i915 graphics driver is going to need to use
this structure soon as well.

Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: linux-edac@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/edac/ghes_edac.c   | 26 --------------------------
 drivers/edac/i7core_edac.c | 25 -------------------------
 include/linux/dmi.h        | 25 +++++++++++++++++++++++++
 3 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index e3fa439..429be79 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -35,32 +35,6 @@ static DEFINE_MUTEX(ghes_edac_lock);
 static int ghes_edac_mc_num;
 
 
-/* Memory Device - Type 17 of SMBIOS spec */
-struct memdev_dmi_entry {
-	u8 type;
-	u8 length;
-	u16 handle;
-	u16 phys_mem_array_handle;
-	u16 mem_err_info_handle;
-	u16 total_width;
-	u16 data_width;
-	u16 size;
-	u8 form_factor;
-	u8 device_set;
-	u8 device_locator;
-	u8 bank_locator;
-	u8 memory_type;
-	u16 type_detail;
-	u16 speed;
-	u8 manufacturer;
-	u8 serial_number;
-	u8 asset_tag;
-	u8 part_number;
-	u8 attributes;
-	u32 extended_size;
-	u16 conf_mem_clk_speed;
-} __attribute__((__packed__));
-
 struct ghes_edac_dimm_fill {
 	struct mem_ctl_info *mci;
 	unsigned count;
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 01087a3..b11d3be 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -1906,31 +1906,6 @@ static struct notifier_block i7_mce_dec = {
 	.notifier_call	= i7core_mce_check_error,
 };
 
-struct memdev_dmi_entry {
-	u8 type;
-	u8 length;
-	u16 handle;
-	u16 phys_mem_array_handle;
-	u16 mem_err_info_handle;
-	u16 total_width;
-	u16 data_width;
-	u16 size;
-	u8 form;
-	u8 device_set;
-	u8 device_locator;
-	u8 bank_locator;
-	u8 memory_type;
-	u16 type_detail;
-	u16 speed;
-	u8 manufacturer;
-	u8 serial_number;
-	u8 asset_tag;
-	u8 part_number;
-	u8 attributes;
-	u32 extended_size;
-	u16 conf_mem_clk_speed;
-} __attribute__((__packed__));
-
 
 /*
  * Decode the DRAM Clock Frequency, be paranoid, make sure that all
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index 5e9c74c..1eb2136 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -78,6 +78,31 @@ struct dmi_header {
 	u16 handle;
 } __packed;
 
+struct memdev_dmi_entry {
+	u8 type;
+	u8 length;
+	u16 handle;
+	u16 phys_mem_array_handle;
+	u16 mem_err_info_handle;
+	u16 total_width;
+	u16 data_width;
+	u16 size;
+	u8 form;
+	u8 device_set;
+	u8 device_locator;
+	u8 bank_locator;
+	u8 memory_type;
+	u16 type_detail;
+	u16 speed;
+	u8 manufacturer;
+	u8 serial_number;
+	u8 asset_tag;
+	u8 part_number;
+	u8 attributes;
+	u32 extended_size;
+	u16 conf_mem_clk_speed;
+} __attribute__((__packed__));
+
 struct dmi_device {
 	struct list_head list;
 	int type;
-- 
2.1.4

[PATCH 7/8] drm/i915: Add support to parse DMI table and get platform memory info (v3)

[Matt Roper]

From: Shobhit Kumar <shobhit.kumar@intel.com>

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

v2: Address Matt's review comments
    - Be more paranoid while dmi decoding
    - Also add support for decoding speed from configured memory speed
      if availble in DMI memory entry

v3 (by Matt):
 - Use memdev_dmi_entry from dmi.h
 - Don't try to use/compare negative numbers in unsigned types

Cc: matthew.d.roper@intel.com
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  7 +++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4aa3db6..4259afe 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,
@@ -973,6 +974,33 @@ static void i915_mmio_cleanup(struct drm_device *dev)
 	pci_iounmap(dev->pdev, dev_priv->regs);
 }
 
+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 struct memdev_dmi_entry *memdev =
+		(const struct memdev_dmi_entry *)hdr;
+	uint16_t mem_speed = 0;
+
+	if (hdr->type != DMI_ENTRY_MEM_DEVICE)
+		return;
+
+	/* Get the speed */
+	if (hdr->length > offsetof(struct memdev_dmi_entry, conf_mem_clk_speed))
+		mem_speed = memdev->conf_mem_clk_speed;
+	else if (hdr->length > offsetof(struct memdev_dmi_entry, speed))
+		mem_speed = memdev->speed;
+	else
+		return;
+
+	dev_priv->dmi.mem_channel++;
+
+	/* All channels are expected to have same the speed */
+	if (dev_priv->dmi.mem_speed == 0)
+		dev_priv->dmi.mem_speed = mem_speed;
+	else if (mem_speed != dev_priv->dmi.mem_speed)
+		dev_priv->dmi.valid = false;
+}
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
@@ -1000,6 +1028,12 @@ 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 */
+	dev_priv->dmi.valid = true;
+	dmi_walk(dmi_decode_memory_info, dev_priv);
+	if (!dev_priv->dmi.mem_speed)
+		dev_priv->dmi.valid = false;
+
 	/* 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 f37ac12..a2a7d8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2022,6 +2022,13 @@ 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 {
+		bool valid;
+		uint16_t mem_channel;
+		int16_t mem_speed;
+	} dmi;
 };
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
-- 
2.1.4

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

[PATCH 8/8] drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW (v4)

[Matt Roper]

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)

v3: - Address Damien's comment, use DIV_ROUND_UP_ULL macro
    - Check both mem_speed and mem_channel to be valid before applying
      WA(shobhit)

v4 (by Matt):
 - Adjust calculations now that skl_plane_downscale_amount() returns
   a 16.16 fixed point value
 - Use pstate->visible instead of pstate->fb != NULL for determining
   whether a plane is active

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a2a7d8d..63b621a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1669,6 +1669,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];
@@ -1973,6 +1979,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 29d37d3..c1700af 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3286,6 +3286,11 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	if (latency == 0 || !cstate->base.active || !intel_pstate->visible)
 		return false;
 
+	if (dev_priv->wm.mem_wa != WATERMARK_WA_NONE) {
+		if (fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
+			latency += 15;
+	}
+
 	width = drm_rect_width(&intel_pstate->src) >> 16;
 	height = drm_rect_height(&intel_pstate->src) >> 16;
 
@@ -3325,6 +3330,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 {
@@ -3776,6 +3784,94 @@ 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;
+
+	/* Verify that we got proper memory information */
+	if (!dev_priv->dmi.valid) {
+		dev_priv->wm.mem_wa = WATERMARK_WA_NONE;
+		return;
+	}
+
+	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;
+			struct intel_plane_state *intel_pstate =
+				to_intel_plane_state(plane->state);
+			uint64_t plane_bw, interm_bw;
+
+			if (!intel_pstate->visible)
+				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 returns the adjusted value
+			 * according to the downscaling amount
+			 *
+			 * pixel rate is in KHz so divide it by 1000
+			 *
+			 * downscale factor is in 16.16 fixed point so shift
+			 * the result accordingly
+			 */
+			interm_bw = skl_pipe_pixel_rate(cstate)/1000 *
+				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)/1000 *
+					drm_format_plane_cpp(fb->pixel_format, 1) *
+					skl_plane_downscale_amount(intel_plane);
+
+			plane_bw = interm_bw >> 16;
+			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_ULL((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;
@@ -3814,6 +3910,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))
 		goto out;
-- 
2.1.4

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

Re: [PATCH 0/8] SKL WM fixes and Arbitrated Display Bandwidth WA

[Kumar, Shobhit]

On 03/08/2016 06:35 AM, Matt Roper wrote:
> This series of patches was mostly written by Mahesh and Shobhit; the last
> iteration posted was by Shobhit here:
>
>          https://patchwork.freedesktop.org/series/2881/
>
> I believe they've been busy recently and haven't had time to revisit this work,
> so I've rebased their patches to the latest upstream tree, incorporated the
> review feedback I previously gave, and added a couple more fixes on top.

Sorry Matt to have fallen behind on this one. Yes been occupied with too 
many things. Thanks a ton for taking care of these. Really appreciate it.

Regards
Shobhit

>
> Kumar, Mahesh (5):
>    drm/i915/skl+: Use plane size for relative data rate calculation
>    drm/i915/skl+: calculate ddb minimum allocation (v3)
>    drm/i915/skl+: calculate plane pixel rate (v3)
>    drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)
>    drm/i915/skl: WA for watermark calculation based on Arbitrated Display
>      BW (v4)
>
> Matt Roper (2):
>    drm/i915/gen9: Hold wm_mutex around SKL watermark updates
>    dmi: Move memdev_dmi_entry definition to dmi.h
>
> Shobhit Kumar (1):
>    drm/i915: Add support to parse DMI table and get platform memory info
>      (v3)
>
>   drivers/edac/ghes_edac.c         |  26 ----
>   drivers/edac/i7core_edac.c       |  25 ----
>   drivers/gpu/drm/i915/i915_dma.c  |  34 +++++
>   drivers/gpu/drm/i915/i915_drv.h  |  16 +++
>   drivers/gpu/drm/i915/intel_drv.h |   3 +
>   drivers/gpu/drm/i915/intel_pm.c  | 292 ++++++++++++++++++++++++++++++++++++---
>   include/linux/dmi.h              |  25 ++++
>   7 files changed, 347 insertions(+), 74 deletions(-)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for SKL WM fixes and Arbitrated Display Bandwidth WA

[Patchwork]

== Series Details ==

Series: SKL WM fixes and Arbitrated Display Bandwidth WA
URL   : https://patchwork.freedesktop.org/series/4197/
State : failure

== Summary ==

Series 4197v1 SKL WM fixes and Arbitrated Display Bandwidth WA
http://patchwork.freedesktop.org/api/1.0/series/4197/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (snb-x220t)
                fail       -> PASS       (hsw-brixbox)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (snb-dellxps)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (snb-dellxps)

bdw-nuci7        total:183  pass:172  dwarn:0   dfail:0   fail:0   skip:11 
bdw-ultra        total:183  pass:164  dwarn:1   dfail:0   fail:0   skip:18 
byt-nuc          total:183  pass:152  dwarn:0   dfail:0   fail:0   skip:31 
hsw-brixbox      total:183  pass:164  dwarn:0   dfail:0   fail:0   skip:19 
ivb-t430s        total:183  pass:162  dwarn:0   dfail:0   fail:0   skip:21 
skl-i5k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
snb-dellxps      total:183  pass:153  dwarn:1   dfail:0   fail:0   skip:29 
snb-x220t        total:183  pass:152  dwarn:1   dfail:1   fail:1   skip:28 

Results at /archive/results/CI_IGT_test/Patchwork_1538/

dd7b01270c8b2048c35b3c336431c50c05a07718 drm-intel-nightly: 2016y-03m-07d-18h-03m-57s UTC integration manifest
8ee9026664f53824d577ec28b163519a351d9be8 drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW (v4)
f850425dc359060444b2a58afd44ddc777b3cc8a drm/i915: Add support to parse DMI table and get platform memory info (v3)
7e53a583d7222594350e917a601534553633fb80 dmi: Move memdev_dmi_entry definition to dmi.h
34e0962ebf34cfb08d03720fed14af805115d84a drm/i915/gen9: Hold wm_mutex around SKL watermark updates
e48f2732b5a8134b762d1ecd4163e95179a3db07 drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)
b67e2f25abb08e4514fbfe97d0592b72e5a7cdf0 drm/i915/skl+: calculate plane pixel rate (v3)
139d4a2f666bbc33fa27a2e5876e9234e5c14621 drm/i915/skl+: calculate ddb minimum allocation (v3)
f4b9bb02668a64965cff10d33881b6803bbb88aa drm/i915/skl+: Use plane size for relative data rate calculation

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

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h

[Jean Delvare]

Hi Matt,

On Mon,  7 Mar 2016 17:05:44 -0800, Matt Roper wrote:
> A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> decoding DMI memory device entries.  Move the structure definition to
> dmi.h so that it can be shared between those drivers and also other
> parts of the kernel; the i915 graphics driver is going to need to use
> this structure soon as well.

Looks good in principle, a few suggestions inline below:

> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: linux-edac@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/edac/ghes_edac.c   | 26 --------------------------
>  drivers/edac/i7core_edac.c | 25 -------------------------
>  include/linux/dmi.h        | 25 +++++++++++++++++++++++++
>  3 files changed, 25 insertions(+), 51 deletions(-)
> (...)
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index 5e9c74c..1eb2136 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -78,6 +78,31 @@ struct dmi_header {
>  	u16 handle;
>  } __packed;
>  
> +struct memdev_dmi_entry {

As a member of the API of the dmi subsystem, this symbol should start
with dmi_. What about dmi_entry_memdev?

> +	u8 type;
> +	u8 length;
> +	u16 handle;
> +	u16 phys_mem_array_handle;
> +	u16 mem_err_info_handle;
> +	u16 total_width;
> +	u16 data_width;
> +	u16 size;
> +	u8 form;
> +	u8 device_set;
> +	u8 device_locator;
> +	u8 bank_locator;
> +	u8 memory_type;
> +	u16 type_detail;
> +	u16 speed;
> +	u8 manufacturer;
> +	u8 serial_number;
> +	u8 asset_tag;
> +	u8 part_number;
> +	u8 attributes;
> +	u32 extended_size;
> +	u16 conf_mem_clk_speed;
> +} __attribute__((__packed__));

dmi_header is declared with __packed instead. I would appreciate
consistency within this header file, so please use __packed too.

> +
>  struct dmi_device {
>  	struct list_head list;
>  	int type;

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h

[Jean Delvare]

Hi Matt,

On Mon,  7 Mar 2016 17:05:44 -0800, Matt Roper wrote:
> A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> decoding DMI memory device entries.  Move the structure definition to
> dmi.h so that it can be shared between those drivers and also other
> parts of the kernel; the i915 graphics driver is going to need to use
> this structure soon as well.

Looks good in principle, a few suggestions inline below:

> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: linux-edac@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/edac/ghes_edac.c   | 26 --------------------------
>  drivers/edac/i7core_edac.c | 25 -------------------------
>  include/linux/dmi.h        | 25 +++++++++++++++++++++++++
>  3 files changed, 25 insertions(+), 51 deletions(-)
> (...)
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index 5e9c74c..1eb2136 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -78,6 +78,31 @@ struct dmi_header {
>  	u16 handle;
>  } __packed;
>  
> +struct memdev_dmi_entry {

As a member of the API of the dmi subsystem, this symbol should start
with dmi_. What about dmi_entry_memdev?

> +	u8 type;
> +	u8 length;
> +	u16 handle;
> +	u16 phys_mem_array_handle;
> +	u16 mem_err_info_handle;
> +	u16 total_width;
> +	u16 data_width;
> +	u16 size;
> +	u8 form;
> +	u8 device_set;
> +	u8 device_locator;
> +	u8 bank_locator;
> +	u8 memory_type;
> +	u16 type_detail;
> +	u16 speed;
> +	u8 manufacturer;
> +	u8 serial_number;
> +	u8 asset_tag;
> +	u8 part_number;
> +	u8 attributes;
> +	u32 extended_size;
> +	u16 conf_mem_clk_speed;
> +} __attribute__((__packed__));

dmi_header is declared with __packed instead. I would appreciate
consistency within this header file, so please use __packed too.

> +
>  struct dmi_device {
>  	struct list_head list;
>  	int type;

Thanks,
-- 
Jean Delvare
SUSE L3 Support
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Matt Roper]

A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
decoding DMI memory device entries.  Move the structure definition to
dmi.h so that it can be shared between those drivers and also other
parts of the kernel; the i915 graphics driver is going to need to use
this structure soon as well.  As part of this move we rename the
structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
'dmi' prefix.

v2:
 - Rename structure to dmi_entry_memdev.  (Jean)
 - Use __packed instead of __attribute__((__packed__)) for consistency
   with the rest of the dmi.h header.  (Jean)

Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: linux-edac@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/edac/ghes_edac.c   | 28 +--------------------------
 drivers/edac/i7core_edac.c | 47 +++++++++++-----------------------------------
 include/linux/dmi.h        | 25 ++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 63 deletions(-)

diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index e3fa439..39535bb 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -35,32 +35,6 @@ static DEFINE_MUTEX(ghes_edac_lock);
 static int ghes_edac_mc_num;
 
 
-/* Memory Device - Type 17 of SMBIOS spec */
-struct memdev_dmi_entry {
-	u8 type;
-	u8 length;
-	u16 handle;
-	u16 phys_mem_array_handle;
-	u16 mem_err_info_handle;
-	u16 total_width;
-	u16 data_width;
-	u16 size;
-	u8 form_factor;
-	u8 device_set;
-	u8 device_locator;
-	u8 bank_locator;
-	u8 memory_type;
-	u16 type_detail;
-	u16 speed;
-	u8 manufacturer;
-	u8 serial_number;
-	u8 asset_tag;
-	u8 part_number;
-	u8 attributes;
-	u32 extended_size;
-	u16 conf_mem_clk_speed;
-} __attribute__((__packed__));
-
 struct ghes_edac_dimm_fill {
 	struct mem_ctl_info *mci;
 	unsigned count;
@@ -80,7 +54,7 @@ static void ghes_edac_dmidecode(const struct dmi_header *dh, void *arg)
 	struct mem_ctl_info *mci = dimm_fill->mci;
 
 	if (dh->type == DMI_ENTRY_MEM_DEVICE) {
-		struct memdev_dmi_entry *entry = (struct memdev_dmi_entry *)dh;
+		struct dmi_entry_memdev *entry = (struct dmi_entry_memdev *)dh;
 		struct dimm_info *dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms,
 						       mci->n_layers,
 						       dimm_fill->count, 0, 0);
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 01087a3..fbfb06f 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -1906,31 +1906,6 @@ static struct notifier_block i7_mce_dec = {
 	.notifier_call	= i7core_mce_check_error,
 };
 
-struct memdev_dmi_entry {
-	u8 type;
-	u8 length;
-	u16 handle;
-	u16 phys_mem_array_handle;
-	u16 mem_err_info_handle;
-	u16 total_width;
-	u16 data_width;
-	u16 size;
-	u8 form;
-	u8 device_set;
-	u8 device_locator;
-	u8 bank_locator;
-	u8 memory_type;
-	u16 type_detail;
-	u16 speed;
-	u8 manufacturer;
-	u8 serial_number;
-	u8 asset_tag;
-	u8 part_number;
-	u8 attributes;
-	u32 extended_size;
-	u16 conf_mem_clk_speed;
-} __attribute__((__packed__));
-
 
 /*
  * Decode the DRAM Clock Frequency, be paranoid, make sure that all
@@ -1946,28 +1921,28 @@ static void decode_dclk(const struct dmi_header *dh, void *_dclk_freq)
 		return;
 
 	if (dh->type == DMI_ENTRY_MEM_DEVICE) {
-		struct memdev_dmi_entry *memdev_dmi_entry =
-			(struct memdev_dmi_entry *)dh;
+		struct dmi_entry_memdev *dmi_entry_memdev =
+			(struct dmi_entry_memdev *)dh;
 		unsigned long conf_mem_clk_speed_offset =
-			(unsigned long)&memdev_dmi_entry->conf_mem_clk_speed -
-			(unsigned long)&memdev_dmi_entry->type;
+			(unsigned long)&dmi_entry_memdev->conf_mem_clk_speed -
+			(unsigned long)&dmi_entry_memdev->type;
 		unsigned long speed_offset =
-			(unsigned long)&memdev_dmi_entry->speed -
-			(unsigned long)&memdev_dmi_entry->type;
+			(unsigned long)&dmi_entry_memdev->speed -
+			(unsigned long)&dmi_entry_memdev->type;
 
 		/* Check that a DIMM is present */
-		if (memdev_dmi_entry->size == 0)
+		if (dmi_entry_memdev->size == 0)
 			return;
 
 		/*
 		 * Pick the configured speed if it's available, otherwise
 		 * pick the DIMM speed, or we don't have a speed.
 		 */
-		if (memdev_dmi_entry->length > conf_mem_clk_speed_offset) {
+		if (dmi_entry_memdev->length > conf_mem_clk_speed_offset) {
 			dmi_mem_clk_speed =
-				memdev_dmi_entry->conf_mem_clk_speed;
-		} else if (memdev_dmi_entry->length > speed_offset) {
-			dmi_mem_clk_speed = memdev_dmi_entry->speed;
+				dmi_entry_memdev->conf_mem_clk_speed;
+		} else if (dmi_entry_memdev->length > speed_offset) {
+			dmi_mem_clk_speed = dmi_entry_memdev->speed;
 		} else {
 			*dclk_freq = -1;
 			return;
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index 5e9c74c..60dcc31 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -78,6 +78,31 @@ struct dmi_header {
 	u16 handle;
 } __packed;
 
+struct dmi_entry_memdev {
+	u8 type;
+	u8 length;
+	u16 handle;
+	u16 phys_mem_array_handle;
+	u16 mem_err_info_handle;
+	u16 total_width;
+	u16 data_width;
+	u16 size;
+	u8 form;
+	u8 device_set;
+	u8 device_locator;
+	u8 bank_locator;
+	u8 memory_type;
+	u16 type_detail;
+	u16 speed;
+	u8 manufacturer;
+	u8 serial_number;
+	u8 asset_tag;
+	u8 part_number;
+	u8 attributes;
+	u32 extended_size;
+	u16 conf_mem_clk_speed;
+} __packed;
+
 struct dmi_device {
 	struct list_head list;
 	int type;
-- 
2.1.4

[PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Matt Roper]

A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
decoding DMI memory device entries.  Move the structure definition to
dmi.h so that it can be shared between those drivers and also other
parts of the kernel; the i915 graphics driver is going to need to use
this structure soon as well.  As part of this move we rename the
structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
'dmi' prefix.

v2:
 - Rename structure to dmi_entry_memdev.  (Jean)
 - Use __packed instead of __attribute__((__packed__)) for consistency
   with the rest of the dmi.h header.  (Jean)

Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: linux-edac@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/edac/ghes_edac.c   | 28 +--------------------------
 drivers/edac/i7core_edac.c | 47 +++++++++++-----------------------------------
 include/linux/dmi.h        | 25 ++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 63 deletions(-)

diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index e3fa439..39535bb 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -35,32 +35,6 @@ static DEFINE_MUTEX(ghes_edac_lock);
 static int ghes_edac_mc_num;
 
 
-/* Memory Device - Type 17 of SMBIOS spec */
-struct memdev_dmi_entry {
-	u8 type;
-	u8 length;
-	u16 handle;
-	u16 phys_mem_array_handle;
-	u16 mem_err_info_handle;
-	u16 total_width;
-	u16 data_width;
-	u16 size;
-	u8 form_factor;
-	u8 device_set;
-	u8 device_locator;
-	u8 bank_locator;
-	u8 memory_type;
-	u16 type_detail;
-	u16 speed;
-	u8 manufacturer;
-	u8 serial_number;
-	u8 asset_tag;
-	u8 part_number;
-	u8 attributes;
-	u32 extended_size;
-	u16 conf_mem_clk_speed;
-} __attribute__((__packed__));
-
 struct ghes_edac_dimm_fill {
 	struct mem_ctl_info *mci;
 	unsigned count;
@@ -80,7 +54,7 @@ static void ghes_edac_dmidecode(const struct dmi_header *dh, void *arg)
 	struct mem_ctl_info *mci = dimm_fill->mci;
 
 	if (dh->type == DMI_ENTRY_MEM_DEVICE) {
-		struct memdev_dmi_entry *entry = (struct memdev_dmi_entry *)dh;
+		struct dmi_entry_memdev *entry = (struct dmi_entry_memdev *)dh;
 		struct dimm_info *dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms,
 						       mci->n_layers,
 						       dimm_fill->count, 0, 0);
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 01087a3..fbfb06f 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -1906,31 +1906,6 @@ static struct notifier_block i7_mce_dec = {
 	.notifier_call	= i7core_mce_check_error,
 };
 
-struct memdev_dmi_entry {
-	u8 type;
-	u8 length;
-	u16 handle;
-	u16 phys_mem_array_handle;
-	u16 mem_err_info_handle;
-	u16 total_width;
-	u16 data_width;
-	u16 size;
-	u8 form;
-	u8 device_set;
-	u8 device_locator;
-	u8 bank_locator;
-	u8 memory_type;
-	u16 type_detail;
-	u16 speed;
-	u8 manufacturer;
-	u8 serial_number;
-	u8 asset_tag;
-	u8 part_number;
-	u8 attributes;
-	u32 extended_size;
-	u16 conf_mem_clk_speed;
-} __attribute__((__packed__));
-
 
 /*
  * Decode the DRAM Clock Frequency, be paranoid, make sure that all
@@ -1946,28 +1921,28 @@ static void decode_dclk(const struct dmi_header *dh, void *_dclk_freq)
 		return;
 
 	if (dh->type == DMI_ENTRY_MEM_DEVICE) {
-		struct memdev_dmi_entry *memdev_dmi_entry =
-			(struct memdev_dmi_entry *)dh;
+		struct dmi_entry_memdev *dmi_entry_memdev =
+			(struct dmi_entry_memdev *)dh;
 		unsigned long conf_mem_clk_speed_offset =
-			(unsigned long)&memdev_dmi_entry->conf_mem_clk_speed -
-			(unsigned long)&memdev_dmi_entry->type;
+			(unsigned long)&dmi_entry_memdev->conf_mem_clk_speed -
+			(unsigned long)&dmi_entry_memdev->type;
 		unsigned long speed_offset =
-			(unsigned long)&memdev_dmi_entry->speed -
-			(unsigned long)&memdev_dmi_entry->type;
+			(unsigned long)&dmi_entry_memdev->speed -
+			(unsigned long)&dmi_entry_memdev->type;
 
 		/* Check that a DIMM is present */
-		if (memdev_dmi_entry->size == 0)
+		if (dmi_entry_memdev->size == 0)
 			return;
 
 		/*
 		 * Pick the configured speed if it's available, otherwise
 		 * pick the DIMM speed, or we don't have a speed.
 		 */
-		if (memdev_dmi_entry->length > conf_mem_clk_speed_offset) {
+		if (dmi_entry_memdev->length > conf_mem_clk_speed_offset) {
 			dmi_mem_clk_speed =
-				memdev_dmi_entry->conf_mem_clk_speed;
-		} else if (memdev_dmi_entry->length > speed_offset) {
-			dmi_mem_clk_speed = memdev_dmi_entry->speed;
+				dmi_entry_memdev->conf_mem_clk_speed;
+		} else if (dmi_entry_memdev->length > speed_offset) {
+			dmi_mem_clk_speed = dmi_entry_memdev->speed;
 		} else {
 			*dclk_freq = -1;
 			return;
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index 5e9c74c..60dcc31 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -78,6 +78,31 @@ struct dmi_header {
 	u16 handle;
 } __packed;
 
+struct dmi_entry_memdev {
+	u8 type;
+	u8 length;
+	u16 handle;
+	u16 phys_mem_array_handle;
+	u16 mem_err_info_handle;
+	u16 total_width;
+	u16 data_width;
+	u16 size;
+	u8 form;
+	u8 device_set;
+	u8 device_locator;
+	u8 bank_locator;
+	u8 memory_type;
+	u16 type_detail;
+	u16 speed;
+	u8 manufacturer;
+	u8 serial_number;
+	u8 asset_tag;
+	u8 part_number;
+	u8 attributes;
+	u32 extended_size;
+	u16 conf_mem_clk_speed;
+} __packed;
+
 struct dmi_device {
 	struct list_head list;
 	int type;
-- 
2.1.4

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

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

[Ville Syrjälä]

On Mon, Mar 07, 2016 at 05:05:45PM -0800, Matt Roper wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> This is needed for WM computation workaround for arbitrated display
> bandwidth.
> 
> v2: Address Matt's review comments
>     - Be more paranoid while dmi decoding
>     - Also add support for decoding speed from configured memory speed
>       if availble in DMI memory entry
> 
> v3 (by Matt):
>  - Use memdev_dmi_entry from dmi.h
>  - Don't try to use/compare negative numbers in unsigned types
> 
> Cc: matthew.d.roper@intel.com
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 34 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  7 +++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4aa3db6..4259afe 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,
> @@ -973,6 +974,33 @@ static void i915_mmio_cleanup(struct drm_device *dev)
>  	pci_iounmap(dev->pdev, dev_priv->regs);
>  }
>  
> +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 struct memdev_dmi_entry *memdev =
> +		(const struct memdev_dmi_entry *)hdr;
> +	uint16_t mem_speed = 0;
> +
> +	if (hdr->type != DMI_ENTRY_MEM_DEVICE)
> +		return;
> +
> +	/* Get the speed */
> +	if (hdr->length > offsetof(struct memdev_dmi_entry, conf_mem_clk_speed))
> +		mem_speed = memdev->conf_mem_clk_speed;
> +	else if (hdr->length > offsetof(struct memdev_dmi_entry, speed))
> +		mem_speed = memdev->speed;
> +	else
> +		return;
> +
> +	dev_priv->dmi.mem_channel++;
> +
> +	/* All channels are expected to have same the speed */
> +	if (dev_priv->dmi.mem_speed == 0)
> +		dev_priv->dmi.mem_speed = mem_speed;
> +	else if (mem_speed != dev_priv->dmi.mem_speed)
> +		dev_priv->dmi.valid = false;
> +}

Dunno if this was covered already, but trusting DMI for this feels
rather fragile to me. Isn't there some way to get the relevant
information from the hardware itself?

> +
>  /**
>   * i915_driver_load - setup chip and create an initial config
>   * @dev: DRM device
> @@ -1000,6 +1028,12 @@ 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 */
> +	dev_priv->dmi.valid = true;
> +	dmi_walk(dmi_decode_memory_info, dev_priv);
> +	if (!dev_priv->dmi.mem_speed)
> +		dev_priv->dmi.valid = false;
> +
>  	/* 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 f37ac12..a2a7d8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2022,6 +2022,13 @@ 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 {
> +		bool valid;
> +		uint16_t mem_channel;
> +		int16_t mem_speed;
> +	} dmi;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: ✗ Fi.CI.BAT: failure for SKL WM fixes and Arbitrated Display Bandwidth WA

[Matt Roper]

On Tue, Mar 08, 2016 at 07:57:12AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: SKL WM fixes and Arbitrated Display Bandwidth WA
> URL   : https://patchwork.freedesktop.org/series/4197/
> State : failure
> 
> == Summary ==
> 
> Series 4197v1 SKL WM fixes and Arbitrated Display Bandwidth WA
> http://patchwork.freedesktop.org/api/1.0/series/4197/revisions/1/mbox/
> 
> Test kms_flip:
>         Subgroup basic-flip-vs-modeset:
>                 pass       -> DMESG-WARN (bdw-ultra)

There's a WARN() here from

        https://bugs.freedesktop.org/show_bug.cgi?id=94349

There's also a bunch of ACPI parsing spam here that I'm not sure what
the cause is:

    ...
    [  387.572173] ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.H_EC._Q32] (Node ffff8802446b59e0), AE_AML_OPERAND_TYPE (20160108/psparse-542)
    [  388.619096] ACPI Error: Cannot assign type [Notify] to [RegionField] (must be type Int/Str/Buf) (20160108/exstoren-127)
    [  388.619169] ACPI Error: Method parse/execution failed [\PNOT] (Node ffff880244711700), AE_AML_OPERAND_TYPE (20160108/psparse-542)
    [  388.619248] ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.H_EC._Q32] (Node ffff8802446b59e0), AE_AML_OPERAND_TYPE (20160108/psparse-542)
    [  389.626344] ACPI Error: Cannot assign type [Notify] to [RegionField] (must be type Int/Str/Buf) (20160108/exstoren-127)
    [  389.626376] ACPI Error: Method parse/execution failed [\PNOT] (Node ffff880244711700), AE_AML_OPERAND_TYPE (20160108/psparse-542)
    [  389.626412] ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.H_EC._Q32] (Node ffff8802446b59e0), AE_AML_OPERAND_TYPE (20160108/psparse-542)
    [  390.669061] ACPI Error: Cannot assign type [Notify] to [RegionField] (must be type Int/Str/Buf) (20160108/exstoren-127)
    ...

The same has happened at least once before in CI on this machine
(CI_DRM_1126), so I don't think it's related to graphics or my patch
series here.


>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> FAIL       (snb-x220t)

https://bugs.freedesktop.org/show_bug.cgi?id=94294


>                 fail       -> PASS       (hsw-brixbox)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 pass       -> DMESG-WARN (snb-dellxps)

https://bugs.freedesktop.org/show_bug.cgi?id=94349 again


Matt

>         Subgroup basic-rte:
>                 dmesg-warn -> PASS       (snb-dellxps)
> 
> bdw-nuci7        total:183  pass:172  dwarn:0   dfail:0   fail:0   skip:11 
> bdw-ultra        total:183  pass:164  dwarn:1   dfail:0   fail:0   skip:18 
> byt-nuc          total:183  pass:152  dwarn:0   dfail:0   fail:0   skip:31 
> hsw-brixbox      total:183  pass:164  dwarn:0   dfail:0   fail:0   skip:19 
> ivb-t430s        total:183  pass:162  dwarn:0   dfail:0   fail:0   skip:21 
> skl-i5k-2        total:183  pass:163  dwarn:0   dfail:0   fail:0   skip:20 
> snb-dellxps      total:183  pass:153  dwarn:1   dfail:0   fail:0   skip:29 
> snb-x220t        total:183  pass:152  dwarn:1   dfail:1   fail:1   skip:28 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1538/
> 
> dd7b01270c8b2048c35b3c336431c50c05a07718 drm-intel-nightly: 2016y-03m-07d-18h-03m-57s UTC integration manifest
> 8ee9026664f53824d577ec28b163519a351d9be8 drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW (v4)
> f850425dc359060444b2a58afd44ddc777b3cc8a drm/i915: Add support to parse DMI table and get platform memory info (v3)
> 7e53a583d7222594350e917a601534553633fb80 dmi: Move memdev_dmi_entry definition to dmi.h
> 34e0962ebf34cfb08d03720fed14af805115d84a drm/i915/gen9: Hold wm_mutex around SKL watermark updates
> e48f2732b5a8134b762d1ecd4163e95179a3db07 drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)
> b67e2f25abb08e4514fbfe97d0592b72e5a7cdf0 drm/i915/skl+: calculate plane pixel rate (v3)
> 139d4a2f666bbc33fa27a2e5876e9234e5c14621 drm/i915/skl+: calculate ddb minimum allocation (v3)
> f4b9bb02668a64965cff10d33881b6803bbb88aa drm/i915/skl+: Use plane size for relative data rate calculation
> 

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

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

[Matt Roper]

On Tue, Mar 08, 2016 at 08:49:23PM +0200, Ville Syrjälä wrote:
> On Mon, Mar 07, 2016 at 05:05:45PM -0800, Matt Roper wrote:
> > From: Shobhit Kumar <shobhit.kumar@intel.com>
> > 
> > This is needed for WM computation workaround for arbitrated display
> > bandwidth.
> > 
> > v2: Address Matt's review comments
> >     - Be more paranoid while dmi decoding
> >     - Also add support for decoding speed from configured memory speed
> >       if availble in DMI memory entry
> > 
> > v3 (by Matt):
> >  - Use memdev_dmi_entry from dmi.h
> >  - Don't try to use/compare negative numbers in unsigned types
> > 
> > Cc: matthew.d.roper@intel.com
> > Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c | 34 ++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_drv.h |  7 +++++++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 4aa3db6..4259afe 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,
> > @@ -973,6 +974,33 @@ static void i915_mmio_cleanup(struct drm_device *dev)
> >  	pci_iounmap(dev->pdev, dev_priv->regs);
> >  }
> >  
> > +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 struct memdev_dmi_entry *memdev =
> > +		(const struct memdev_dmi_entry *)hdr;
> > +	uint16_t mem_speed = 0;
> > +
> > +	if (hdr->type != DMI_ENTRY_MEM_DEVICE)
> > +		return;
> > +
> > +	/* Get the speed */
> > +	if (hdr->length > offsetof(struct memdev_dmi_entry, conf_mem_clk_speed))
> > +		mem_speed = memdev->conf_mem_clk_speed;
> > +	else if (hdr->length > offsetof(struct memdev_dmi_entry, speed))
> > +		mem_speed = memdev->speed;
> > +	else
> > +		return;
> > +
> > +	dev_priv->dmi.mem_channel++;
> > +
> > +	/* All channels are expected to have same the speed */
> > +	if (dev_priv->dmi.mem_speed == 0)
> > +		dev_priv->dmi.mem_speed = mem_speed;
> > +	else if (mem_speed != dev_priv->dmi.mem_speed)
> > +		dev_priv->dmi.valid = false;
> > +}
> 
> Dunno if this was covered already, but trusting DMI for this feels
> rather fragile to me. Isn't there some way to get the relevant
> information from the hardware itself?

Not sure.  You're right that it's fragile though; I find that the DMI
never provides valid memdev information on the BXT board I'm working
with, so the workaround always winds up disabled for me.  Not sure if
Shobhit has any other ideas for how we could get valid memory
information.


Matt

> 
> > +
> >  /**
> >   * i915_driver_load - setup chip and create an initial config
> >   * @dev: DRM device
> > @@ -1000,6 +1028,12 @@ 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 */
> > +	dev_priv->dmi.valid = true;
> > +	dmi_walk(dmi_decode_memory_info, dev_priv);
> > +	if (!dev_priv->dmi.mem_speed)
> > +		dev_priv->dmi.valid = false;
> > +
> >  	/* 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 f37ac12..a2a7d8d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2022,6 +2022,13 @@ 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 {
> > +		bool valid;
> > +		uint16_t mem_channel;
> > +		int16_t mem_speed;
> > +	} dmi;
> >  };
> >  
> >  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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

[PATCH 7/8] drm/i915: Add support to parse DMI table and get platform memory info (v4)

[Matt Roper]

From: Shobhit Kumar <shobhit.kumar@intel.com>

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

v2: Address Matt's review comments
    - Be more paranoid while dmi decoding
    - Also add support for decoding speed from configured memory speed
      if availble in DMI memory entry

v3 (by Matt):
 - Use memdev_dmi_entry from dmi.h
 - Don't try to use/compare negative numbers in unsigned types

v4:
 - memdev_dmi_entry is now called dmi_entry_memdev

Cc: matthew.d.roper@intel.com
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  7 +++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4aa3db6..2f06c93 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,
@@ -973,6 +974,33 @@ static void i915_mmio_cleanup(struct drm_device *dev)
 	pci_iounmap(dev->pdev, dev_priv->regs);
 }
 
+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 struct dmi_entry_memdev *memdev =
+		(const struct dmi_entry_memdev *)hdr;
+	uint16_t mem_speed = 0;
+
+	if (hdr->type != DMI_ENTRY_MEM_DEVICE)
+		return;
+
+	/* Get the speed */
+	if (hdr->length > offsetof(struct dmi_entry_memdev, conf_mem_clk_speed))
+		mem_speed = memdev->conf_mem_clk_speed;
+	else if (hdr->length > offsetof(struct dmi_entry_memdev, speed))
+		mem_speed = memdev->speed;
+	else
+		return;
+
+	dev_priv->dmi.mem_channel++;
+
+	/* All channels are expected to have same the speed */
+	if (dev_priv->dmi.mem_speed == 0)
+		dev_priv->dmi.mem_speed = mem_speed;
+	else if (mem_speed != dev_priv->dmi.mem_speed)
+		dev_priv->dmi.valid = false;
+}
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
@@ -1000,6 +1028,12 @@ 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 */
+	dev_priv->dmi.valid = true;
+	dmi_walk(dmi_decode_memory_info, dev_priv);
+	if (!dev_priv->dmi.mem_speed)
+		dev_priv->dmi.valid = false;
+
 	/* 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 f37ac12..a2a7d8d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2022,6 +2022,13 @@ 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 {
+		bool valid;
+		uint16_t mem_channel;
+		int16_t mem_speed;
+	} dmi;
 };
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
-- 
2.1.4

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

✗ Fi.CI.BAT: failure for SKL WM fixes and Arbitrated Display Bandwidth WA (rev3)

[Patchwork]

== Series Details ==

Series: SKL WM fixes and Arbitrated Display Bandwidth WA (rev3)
URL   : https://patchwork.freedesktop.org/series/4197/
State : failure

== Summary ==

Series 4197v3 SKL WM fixes and Arbitrated Display Bandwidth WA
http://patchwork.freedesktop.org/api/1.0/series/4197/revisions/3/mbox/

Test gem_ringfill:
        Subgroup basic-default-s3:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (bdw-ultra)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (snb-x220t)
                pass       -> DMESG-WARN (hsw-gt2)
        Subgroup basic-plain-flip:
                dmesg-warn -> PASS       (hsw-gt2)
                pass       -> DMESG-WARN (hsw-brixbox)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b:
                dmesg-warn -> PASS       (hsw-gt2)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (snb-dellxps)
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (snb-x220t)
                dmesg-warn -> PASS       (snb-dellxps)
                dmesg-warn -> PASS       (bsw-nuc-2)

bdw-nuci7        total:186  pass:174  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:186  pass:167  dwarn:0   dfail:0   fail:0   skip:19 
bsw-nuc-2        total:186  pass:150  dwarn:1   dfail:0   fail:0   skip:35 
byt-nuc          total:186  pass:154  dwarn:0   dfail:0   fail:0   skip:32 
hsw-brixbox      total:186  pass:165  dwarn:1   dfail:0   fail:0   skip:20 
hsw-gt2          total:186  pass:170  dwarn:1   dfail:0   fail:0   skip:15 
ilk-hp8440p      total:186  pass:127  dwarn:0   dfail:0   fail:0   skip:59 
ivb-t430s        total:186  pass:164  dwarn:0   dfail:0   fail:0   skip:22 
skl-i5k-2        total:186  pass:165  dwarn:0   dfail:0   fail:0   skip:21 
skl-i7k-2        total:186  pass:165  dwarn:0   dfail:0   fail:0   skip:21 
snb-dellxps      total:186  pass:155  dwarn:1   dfail:0   fail:0   skip:30 
snb-x220t        total:186  pass:155  dwarn:0   dfail:1   fail:1   skip:29 

Results at /archive/results/CI_IGT_test/Patchwork_1543/

b519bbd9633eca6bc8e8e80588b48bcee447c330 drm-intel-nightly: 2016y-03m-08d-13h-00m-35s UTC integration manifest
cb4b3400c2f6ef7e8125fe3288cc2e99afbfdf66 drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW (v4)
ce5681608146d12d038c8b1a31b0cdcb18d6b111 drm/i915: Add support to parse DMI table and get platform memory info (v4)
fedc1fec775c675d648f93e8d183556a429c9b2e dmi: Move memdev_dmi_entry definition to dmi.h (v2)
8aaa3cf1f7ff6c8c333267ebadb50c5d674748b4 drm/i915/gen9: Hold wm_mutex around SKL watermark updates
c411070f9507d5d0e8afa48eef877244516afb77 drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)
b3344d515c43874f62653df3a0ee3390e7acd8c5 drm/i915/skl+: calculate plane pixel rate (v3)
95cfa8a08ec8be3f0756bf7ff05742e3831e3dcb drm/i915/skl+: calculate ddb minimum allocation (v3)
d4d4801dfb6d3b754b6bf98634512e005d2c0337 drm/i915/skl+: Use plane size for relative data rate calculation

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

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Jean Delvare]

Hi Matt,

On Tue,  8 Mar 2016 10:32:37 -0800, Matt Roper wrote:
> A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> decoding DMI memory device entries.  Move the structure definition to
> dmi.h so that it can be shared between those drivers and also other
> parts of the kernel; the i915 graphics driver is going to need to use
> this structure soon as well.  As part of this move we rename the
> structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
> 'dmi' prefix.
> 
> v2:
>  - Rename structure to dmi_entry_memdev.  (Jean)
>  - Use __packed instead of __attribute__((__packed__)) for consistency
>    with the rest of the dmi.h header.  (Jean)

Looks better. One more comment inline below:

> (...)
> diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
> index 01087a3..fbfb06f 100644
> --- a/drivers/edac/i7core_edac.c
> +++ b/drivers/edac/i7core_edac.c
> (...)
> @@ -1946,28 +1921,28 @@ static void decode_dclk(const struct dmi_header *dh, void *_dclk_freq)
>  		return;
>  
>  	if (dh->type == DMI_ENTRY_MEM_DEVICE) {
> -		struct memdev_dmi_entry *memdev_dmi_entry =
> -			(struct memdev_dmi_entry *)dh;
> +		struct dmi_entry_memdev *dmi_entry_memdev =
> +			(struct dmi_entry_memdev *)dh;
>  		unsigned long conf_mem_clk_speed_offset =
> -			(unsigned long)&memdev_dmi_entry->conf_mem_clk_speed -
> -			(unsigned long)&memdev_dmi_entry->type;
> +			(unsigned long)&dmi_entry_memdev->conf_mem_clk_speed -
> +			(unsigned long)&dmi_entry_memdev->type;
>  		unsigned long speed_offset =
> -			(unsigned long)&memdev_dmi_entry->speed -
> -			(unsigned long)&memdev_dmi_entry->type;
> +			(unsigned long)&dmi_entry_memdev->speed -
> +			(unsigned long)&dmi_entry_memdev->type;
>  
>  		/* Check that a DIMM is present */
> -		if (memdev_dmi_entry->size == 0)
> +		if (dmi_entry_memdev->size == 0)
>  			return;
>  
>  		/*
>  		 * Pick the configured speed if it's available, otherwise
>  		 * pick the DIMM speed, or we don't have a speed.
>  		 */
> -		if (memdev_dmi_entry->length > conf_mem_clk_speed_offset) {
> +		if (dmi_entry_memdev->length > conf_mem_clk_speed_offset) {
>  			dmi_mem_clk_speed =
> -				memdev_dmi_entry->conf_mem_clk_speed;
> -		} else if (memdev_dmi_entry->length > speed_offset) {
> -			dmi_mem_clk_speed = memdev_dmi_entry->speed;
> +				dmi_entry_memdev->conf_mem_clk_speed;
> +		} else if (dmi_entry_memdev->length > speed_offset) {
> +			dmi_mem_clk_speed = dmi_entry_memdev->speed;
>  		} else {
>  			*dclk_freq = -1;
>  			return;

You do not need all of these changes. memdev_dmi_entry was both the
structure name and the variable name in the original code (something I
usually avoid, but C allows it.) Just because you renamed the structure
doesn't mean you have to rename the variable.

Other than that, looks good to me.

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 4/8] drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)

[Sripada, Radhakrishna]

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

This commit introduced a divide-by-zero crash on plugging in an external display to the system. Below is the crash.

      [  122.320882] divide error: 0000 [#1] PREEMPT SMP
      [  122.320893] Modules linked in: rfcomm i2c_dev uinput snd_soc_hdac_hdmi snd_soc_dmic aesni_intel aes_x86_64 glue_helper lrw gf128mul ablk_helper cryptd btusb uvcvideo
       snd_soc_skl snd_soc_skl_ipc snd_hda_ext_core videobuf2_vmalloc snd_hda_core btrtl videobuf2_memops btbcm videobuf2_v4l2 btintel videobuf2_core bluetooth snd_soc_ssm456
      7 fuse cfg80211 nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables r8152 mii joydev snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device ppp_asyn
      c ppp_generic slhc tun
      [  122.321004] CPU: 3 PID: 9307 Comm: DrmThread Tainted: G     U          4.6.0-rc3-00039-g11f59d6 #420
      [  122.321008] Hardware name: xxxxxxxxxxxxx, BIOS xxxxx.7820.64.0 03/17/2016
      [  122.321011] task: ffff880071c58e40 ti: ffff880272afc000 task.ti: ffff880272afc000
      [  122.321014] RIP: 0010:[<ffffffffac93c3b8>]  [<ffffffffac93c3b8>] skl_update_pipe_wm+0x384/0x814
      [  122.321022] RSP: 0018:ffff880272affa18  EFLAGS: 00010202
      [  122.321025] RAX: 0000000000000000 RBX: ffff8802750a01be RCX: 0000000000010000
      [  122.321028] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 00000000750a01c6
      [  122.321031] RBP: ffff880272affad0 R08: ffff88026c6f7780 R09: ffff8802750a5000
      [  122.321033] R10: ffff880274aea800 R11: 00000000000005a0 R12: ffff880274aea7fc
      [  122.321036] R13: 0000000000000000 R14: ffff880274add000 R15: ffff880269571000
      [  122.321039] FS:  00007f8b04a30700(0000) GS:ffff88027ed80000(0000) knlGS:0000000000000000
      [  122.321042] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      [  122.321045] CR2: 00007f97a0f33000 CR3: 0000000075b08000 CR4: 00000000003406e0
      [  122.321047] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
      [  122.321050] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
      [  122.321053] Stack:
      [  122.321056]  ffff880274a81598 0000000000000001 ffff880272affa80 ffffffffac9b6973
      [  122.321066]  ffff8802750a5000 ffff8802750a5000 ffff88026c6f7780 ffff88026c6f7780
      [  122.321077]  ffff8802750a34e8 0000000000000004 0000000000000000 00000000000001ae
      [  122.321087] Call Trace:
      [  122.321092]  [<ffffffffac9b6973>] ? intel_dp_aux_transfer+0x169/0x1d8
      [  122.321096]  [<ffffffffac93cbbe>] skl_update_wm+0xf8/0x652
      [  122.321108]  [<ffffffffac9790eb>] ? gen9_write32+0x1ba/0x1cc
      [  122.321112]  [<ffffffffac93debc>] intel_update_watermarks+0x1e/0x20
      [  122.321119]  [<ffffffffac994f98>] haswell_crtc_enable+0x359/0x786
      [  122.321123]  [<ffffffffac9984b1>] intel_atomic_commit+0x85e/0xff3
      [  122.321127]  [<ffffffffac925f75>] ? drm_atomic_check_only+0x3f0/0x55c
      [  122.321130]  [<ffffffffac92612e>] drm_atomic_commit+0x4d/0x52
      [  122.321135]  [<ffffffffac907e06>] drm_atomic_helper_set_config+0x53/0x9e
      [  122.321138]  [<ffffffffac9183f1>] drm_mode_set_config_internal+0x5e/0xf8
      [  122.321142]  [<ffffffffac91c4b1>] drm_mode_setcrtc+0x3e3/0x465
      [  122.321145]  [<ffffffffac90f60d>] drm_ioctl+0x274/0x3ce
      [  122.321149]  [<ffffffffac91c0ce>] ? drm_mode_setplane+0x158/0x158
      [  122.321153]  [<ffffffffac6bf6b6>] ? seccomp_phase1+0xf2/0x1ca
      [  122.321170]  [<ffffffffac714b8f>] ? remove_vma+0x60/0x68
      [  122.321179]  [<ffffffffac73dde6>] vfs_ioctl+0x18/0x34
      [  122.321187]  [<ffffffffac73e362>] do_vfs_ioctl+0x4a2/0x4c5
      [  122.321199]  [<ffffffffac746c18>] ? __fget+0x77/0x83
      [  122.321203]  [<ffffffffac73e3dc>] SyS_ioctl+0x57/0x79
      [  122.321207]  [<ffffffffac6017a0>] do_syscall_64+0x4c/0x5b
      [  122.321212]  [<ffffffffacc735fc>] entry_SYSCALL64_slow_path+0x25/0x25
      [  122.321215] Code: f6 4c 89 c7 e8 28 d0 ff ff 89 c0 31 d2 89 df 48 0f af 45 a0 66 42 03 7c 6d c0 48 8b 75 a8 4c 8b 45 80 4c 01 ee 66 41 89 5c b4 0c <48> f7 75 98 8d 1c 07 66 41 89 5c b4 0e 49 8b 40 10 81 b8 8c 00
      [  122.321481] RIP  [<ffffffffac93c3b8>] skl_update_pipe_wm+0x384/0x814
      [  122.321488]  RSP <ffff880272affa18>
      [  122.321491] ---[ end trace a66fed013d3c5bdf ]---
      [  122.321978] Kernel panic - not syncing: Fatal exception
      [  122.321999] Kernel Offset: 0x2b600000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
      [  122.324445] ACPI MEMORY or I/O RESET_REG.

Thanks,
RK
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Matt Roper
Sent: Monday, March 07, 2016 5:06 PM
To: intel-gfx@lists.freedesktop.org
Cc: Kumar, Shobhit
Subject: [Intel-gfx] [PATCH 4/8] drm/i915/skl+: Use scaling amount for plane data rate calculation (v3)

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

if downscaling is enabled plane data rate increases according to scaling amount. take scaling amount under consideration while calculating plane data rate

v2: Address Matt's comments, where data rate was overridden because of missing else.

v3 (by Matt):
 - Add braces to 'else' branch to match kernel coding style
 - Adjust final calculation now that skl_plane_downscale_amount()
   returns 16.16 fixed point value instead of a decimal fixed point

Cc: matthew.d.roper@intel.com<mailto:matthew.d.roper@intel.com>
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com<mailto:mahesh1.kumar@intel.com>>
Reviewed-by(v2): Matt Roper <matthew.d.roper@intel.com<mailto:matthew.d.roper@intel.com>>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com<mailto:matthew.d.roper@intel.com>>
---
 drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e828bde..041db5d3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2974,6 +2974,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,  {
        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;

        width = drm_rect_width(&intel_pstate->src) >> 16; @@ -2985,15 +2987,20 @@ 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);
+       } else {
+               /* for packed formats */
+               data_rate = width * height *
+                       drm_format_plane_cpp(fb->pixel_format, 0);
        }

-       /* for packed formats */
-       return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
+       down_scale_amount = skl_plane_downscale_amount(intel_plane);
+
+       return data_rate * down_scale_amount >> 16;
 }

 /*
--
2.1.4

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


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

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

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

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Jean Delvare]

Hi Matt, Mauro,

On Thu, 17 Mar 2016 15:18:20 +0100, Jean Delvare wrote:
> On Tue,  8 Mar 2016 10:32:37 -0800, Matt Roper wrote:
> > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> > decoding DMI memory device entries.  Move the structure definition to
> > dmi.h so that it can be shared between those drivers and also other
> > parts of the kernel; the i915 graphics driver is going to need to use
> > this structure soon as well.  As part of this move we rename the
> > structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
> > 'dmi' prefix.
> > 
> > v2:
> >  - Rename structure to dmi_entry_memdev.  (Jean)
> >  - Use __packed instead of __attribute__((__packed__)) for consistency
> >    with the rest of the dmi.h header.  (Jean)  
> 
> Looks better. (...)

What happened to this patch? I never received v3. Is it sill needed?

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Jean Delvare]

Hi Matt, Mauro,

On Thu, 17 Mar 2016 15:18:20 +0100, Jean Delvare wrote:
> On Tue,  8 Mar 2016 10:32:37 -0800, Matt Roper wrote:
> > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> > decoding DMI memory device entries.  Move the structure definition to
> > dmi.h so that it can be shared between those drivers and also other
> > parts of the kernel; the i915 graphics driver is going to need to use
> > this structure soon as well.  As part of this move we rename the
> > structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
> > 'dmi' prefix.
> > 
> > v2:
> >  - Rename structure to dmi_entry_memdev.  (Jean)
> >  - Use __packed instead of __attribute__((__packed__)) for consistency
> >    with the rest of the dmi.h header.  (Jean)  
> 
> Looks better. (...)

What happened to this patch? I never received v3. Is it sill needed?

-- 
Jean Delvare
SUSE L3 Support
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Matt Roper]

On Mon, Jul 31, 2017 at 10:36:05AM +0200, Jean Delvare wrote:
> Hi Matt, Mauro,
> 
> On Thu, 17 Mar 2016 15:18:20 +0100, Jean Delvare wrote:
> > On Tue,  8 Mar 2016 10:32:37 -0800, Matt Roper wrote:
> > > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> > > decoding DMI memory device entries.  Move the structure definition to
> > > dmi.h so that it can be shared between those drivers and also other
> > > parts of the kernel; the i915 graphics driver is going to need to use
> > > this structure soon as well.  As part of this move we rename the
> > > structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
> > > 'dmi' prefix.
> > > 
> > > v2:
> > >  - Rename structure to dmi_entry_memdev.  (Jean)
> > >  - Use __packed instead of __attribute__((__packed__)) for consistency
> > >    with the rest of the dmi.h header.  (Jean)  
> > 
> > Looks better. (...)
> 
> What happened to this patch? I never received v3. Is it sill needed?

We ended up going a different direction in the graphics driver and wound
up not needing access to this structure.  If there's still interest in
the general refactoring here, let me know and I can incorporate your
last feedback and respin a v3.


Matt

> -- 
> Jean Delvare
> SUSE L3 Support

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Matt Roper]

On Mon, Jul 31, 2017 at 10:36:05AM +0200, Jean Delvare wrote:
> Hi Matt, Mauro,
> 
> On Thu, 17 Mar 2016 15:18:20 +0100, Jean Delvare wrote:
> > On Tue,  8 Mar 2016 10:32:37 -0800, Matt Roper wrote:
> > > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> > > decoding DMI memory device entries.  Move the structure definition to
> > > dmi.h so that it can be shared between those drivers and also other
> > > parts of the kernel; the i915 graphics driver is going to need to use
> > > this structure soon as well.  As part of this move we rename the
> > > structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
> > > 'dmi' prefix.
> > > 
> > > v2:
> > >  - Rename structure to dmi_entry_memdev.  (Jean)
> > >  - Use __packed instead of __attribute__((__packed__)) for consistency
> > >    with the rest of the dmi.h header.  (Jean)  
> > 
> > Looks better. (...)
> 
> What happened to this patch? I never received v3. Is it sill needed?

We ended up going a different direction in the graphics driver and wound
up not needing access to this structure.  If there's still interest in
the general refactoring here, let me know and I can incorporate your
last feedback and respin a v3.


Matt

> -- 
> Jean Delvare
> SUSE L3 Support

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

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Jean Delvare]

On mer., 2017-08-09 at 09:18 -0700, Matt Roper wrote:
> On Mon, Jul 31, 2017 at 10:36:05AM +0200, Jean Delvare wrote:
> > 
> > Hi Matt, Mauro,
> > 
> > On Thu, 17 Mar 2016 15:18:20 +0100, Jean Delvare wrote:
> > > 
> > > On Tue,  8 Mar 2016 10:32:37 -0800, Matt Roper wrote:
> > > > 
> > > > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> > > > decoding DMI memory device entries.  Move the structure definition to
> > > > dmi.h so that it can be shared between those drivers and also other
> > > > parts of the kernel; the i915 graphics driver is going to need to use
> > > > this structure soon as well.  As part of this move we rename the
> > > > structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
> > > > 'dmi' prefix.
> > > > 
> > > > v2:
> > > >  - Rename structure to dmi_entry_memdev.  (Jean)
> > > >  - Use __packed instead of __attribute__((__packed__)) for consistency
> > > >    with the rest of the dmi.h header.  (Jean)  
> > > 
> > > Looks better. (...)
> > 
> > What happened to this patch? I never received v3. Is it sill needed?
> 
> We ended up going a different direction in the graphics driver and wound
> up not needing access to this structure.  If there's still interest in
> the general refactoring here, let me know and I can incorporate your
> last feedback and respin a v3.

No, if you don't need it anymore I'll just drop it.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH 6/8] dmi: Move memdev_dmi_entry definition to dmi.h (v2)

[Jean Delvare]

On mer., 2017-08-09 at 09:18 -0700, Matt Roper wrote:
> On Mon, Jul 31, 2017 at 10:36:05AM +0200, Jean Delvare wrote:
> > 
> > Hi Matt, Mauro,
> > 
> > On Thu, 17 Mar 2016 15:18:20 +0100, Jean Delvare wrote:
> > > 
> > > On Tue,  8 Mar 2016 10:32:37 -0800, Matt Roper wrote:
> > > > 
> > > > A couple of the EDAC drivers have a nice memdev_dmi_entry structure for
> > > > decoding DMI memory device entries.  Move the structure definition to
> > > > dmi.h so that it can be shared between those drivers and also other
> > > > parts of the kernel; the i915 graphics driver is going to need to use
> > > > this structure soon as well.  As part of this move we rename the
> > > > structure s/memdev_dmi_entry/dmi_entry_memdev/ to ensure it has a proper
> > > > 'dmi' prefix.
> > > > 
> > > > v2:
> > > >  - Rename structure to dmi_entry_memdev.  (Jean)
> > > >  - Use __packed instead of __attribute__((__packed__)) for consistency
> > > >    with the rest of the dmi.h header.  (Jean)  
> > > 
> > > Looks better. (...)
> > 
> > What happened to this patch? I never received v3. Is it sill needed?
> 
> We ended up going a different direction in the graphics driver and wound
> up not needing access to this structure.  If there's still interest in
> the general refactoring here, let me know and I can incorporate your
> last feedback and respin a v3.

No, if you don't need it anymore I'll just drop it.

Thanks,
-- 
Jean Delvare
SUSE L3 Support
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx