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

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

[Shobhit Kumar]

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>
---
 drivers/gpu/drm/i915/intel_pm.c | 42 ++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 20bf854..d55e5d0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2868,25 +2868,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);
 }
 
 /*
@@ -2905,9 +2908,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;
 
@@ -2965,8 +2967,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;
 
@@ -2992,7 +2995,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;
@@ -3116,28 +3119,37 @@ 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 bytes_per_pixel;
+	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);
+
 	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
 	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
 				 bytes_per_pixel,
 				 latency);
 	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
 				 cstate->base.adjusted_mode.crtc_htotal,
-				 cstate->pipe_src_w,
+				 width,
 				 bytes_per_pixel,
 				 fb->modifier[0],
 				 latency);
 
-	plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
+	plane_bytes_per_line = width * bytes_per_pixel;
 	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
 
 	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
-- 
2.5.0

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

[v2 2/6] drm/i915/skl+: calculate ddb minimum allocation

[Shobhit Kumar]

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

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

v2: optimizations as per Matt's comments.

Cc: matthew.d.roper@intel.com
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@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 d55e5d0..708f329 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2928,6 +2928,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 (y && !fb && !(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 */)
@@ -2964,7 +3009,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)
@@ -2973,9 +3017,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.5.0

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

[v2 3/6] drm/i915/skl+: calculate plane pixel rate

[Shobhit Kumar]

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

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

adjusted plane_pixel_rate = adjusted pipe_pixel_rate * downscale ammount

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

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

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

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bc97012..bb2b1c7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -638,6 +638,8 @@ struct intel_plane_wm_parameters {
 	u64 tiling;
 	unsigned int rotation;
 	uint16_t fifo_size;
+	/* Stores the adjusted plane pixel rate for WM calculation for SKL+ */
+	uint32_t plane_pixel_rate;
 };
 
 struct intel_plane {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 708f329..40fff09 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2782,6 +2782,48 @@ skl_wm_plane_id(const struct intel_plane *plane)
 	}
 }
 
+/*
+ * This function takes drm_plane_state as input
+ * and decides the downscale amount according to the formula
+ *
+ * downscale amount = Max[1, Horizontal source size / Horizontal dest size]
+ *
+ * Return value is multiplied by 1000 to retain fractional part
+ * Caller should take care of dividing & Rounding off the value
+ */
+static uint32_t
+skl_plane_downscale_amount(const struct intel_plane *intel_plane)
+{
+	struct drm_plane_state *pstate = intel_plane->base.state;
+	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
+	uint32_t downscale_h, downscale_w;
+	uint32_t src_w, src_h, dst_w, dst_h, tmp;
+
+	/* If plane not visible return amount as unity */
+	if (!intel_pstate->visible)
+		return 1000;
+
+	src_w = drm_rect_width(&intel_pstate->src) >> 16;
+	src_h = drm_rect_height(&intel_pstate->src) >> 16;
+
+	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);
+
+	/* Multiply by 1000 for precision */
+	tmp = (1000 * src_h) / dst_h;
+	downscale_h = max_t(uint32_t, 1000, tmp);
+
+	tmp = (1000 * src_w) / dst_w;
+	downscale_w = max_t(uint32_t, 1000, tmp);
+
+	/* Reducing precision to 3 decimal places */
+	return DIV_ROUND_UP(downscale_h * downscale_w, 1000);
+}
+
+
 static void
 skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 				   const struct intel_crtc_state *cstate,
@@ -3183,10 +3225,10 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 		swap(width, height);
 
 	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
-	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
+	method1 = skl_wm_method1(intel_plane->wm.plane_pixel_rate,
 				 bytes_per_pixel,
 				 latency);
-	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
+	method2 = skl_wm_method2(intel_plane->wm.plane_pixel_rate,
 				 cstate->base.adjusted_mode.crtc_htotal,
 				 width,
 				 bytes_per_pixel,
@@ -3627,6 +3669,45 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
 	}
 }
 
+static uint32_t
+skl_plane_pixel_rate(struct intel_crtc_state *cstate, struct intel_plane *plane)
+{
+	uint32_t adjusted_pixel_rate;
+	uint32_t downscale_amount;
+
+	/*
+	 * adjusted plane pixel rate = adjusted pipe pixel rate
+	 * Plane pixel rate = adjusted plane pixel rate * plane down scale
+	 * amount
+	 */
+	adjusted_pixel_rate = skl_pipe_pixel_rate(cstate);
+	downscale_amount = skl_plane_downscale_amount(plane);
+
+	return DIV_ROUND_UP(adjusted_pixel_rate * downscale_amount,
+			1000);
+}
+
+static void skl_set_plane_pixel_rate(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+	struct intel_plane *intel_plane = NULL;
+	struct drm_device *dev = crtc->dev;
+
+	if (!intel_crtc->active)
+		return;
+	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+		struct drm_plane *plane = &intel_plane->base;
+
+		if (!to_intel_plane_state(plane->state)->visible)
+			continue;
+
+		intel_plane->wm.plane_pixel_rate = skl_plane_pixel_rate(cstate,
+				intel_plane);
+	}
+
+}
+
 static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
 {
 	watermarks->wm_linetime[pipe] = 0;
@@ -3662,6 +3743,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.5.0

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

[v2 4/6] drm/i915/skl+: Use scaling amount for plane data rate calculation

[Shobhit Kumar]

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.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 40fff09..a9f9396 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2912,6 +2912,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;
@@ -2923,15 +2925,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);
+
+	down_scale_amount = skl_plane_downscale_amount(intel_plane);
+
+	return DIV_ROUND_UP((data_rate * down_scale_amount), 1000);
 
-	/* for packed formats */
-	return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
 }
 
 /*
-- 
2.5.0

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

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

[Shobhit Kumar]

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

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

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

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d70d96f..320143b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -49,6 +49,7 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/oom.h>
+#include <linux/dmi.h>
 
 
 static int i915_getparam(struct drm_device *dev, void *data,
@@ -855,6 +856,49 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void dmi_decode_memory_info(const struct dmi_header *hdr, void *priv)
+{
+	struct drm_i915_private *dev_priv = (struct drm_i915_private *) priv;
+	const u8 *data = (const u8 *) hdr;
+	uint16_t size, mem_speed;
+
+#define DMI_CONF_MEM_SPEED_OFFSET	0x20
+#define DMI_MEM_SPEED_OFFSET		0x15
+#define DMI_MEM_SIZE_OFFSET		0x0C
+
+	if (hdr->type == DMI_ENTRY_MEM_DEVICE) {
+		/* Found a memory channel ? */
+		size = (uint16_t) (*((uint16_t *)(data + DMI_MEM_SIZE_OFFSET)));
+		if (size == 0)
+			return;
+
+		dev_priv->dmi.mem_channel++;
+
+		/* Get the speed */
+		if (hdr->length > DMI_CONF_MEM_SPEED_OFFSET)
+			mem_speed =
+				(uint16_t) (*((uint16_t *)(data + DMI_CONF_MEM_SPEED_OFFSET)));
+		else if (hdr->length > DMI_MEM_SPEED_OFFSET)
+			mem_speed =
+				(uint16_t) (*((uint16_t *)(data + DMI_MEM_SPEED_OFFSET)));
+		else
+			mem_speed = -1;
+
+		/*
+		 * Check all channels have same speed
+		 * else mark speed as invalid
+		 */
+		if (dev_priv->dmi.mem_speed == 0) {
+			if (mem_speed > 0)
+				dev_priv->dmi.mem_speed = mem_speed;
+			else
+				dev_priv->dmi.mem_speed = -1;
+		} else if (dev_priv->dmi.mem_speed > 0 &&
+					dev_priv->dmi.mem_speed != mem_speed)
+			dev_priv->dmi.mem_speed = -1;
+	}
+}
+
 /**
  * i915_driver_load - setup chip and create an initial config
  * @dev: DRM device
@@ -882,6 +926,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 
+	/* walk the dmi device table for getting platform memory information */
+	dmi_walk(dmi_decode_memory_info, (void *) dev_priv);
+
 	/* Setup the write-once "constant" device info */
 	device_info = (struct intel_device_info *)&dev_priv->info;
 	memcpy(device_info, info, sizeof(dev_priv->info));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 211af53..b040e7a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1968,6 +1968,12 @@ struct drm_i915_private {
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
 	 */
+
+	/* DMI data for memory bandwidth calculation */
+	struct {
+		uint16_t mem_channel;
+		int16_t mem_speed;
+	} dmi;
 };
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
-- 
2.5.0

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

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

[Shobhit Kumar]

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

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

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

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

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)

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>
---
 drivers/gpu/drm/i915/i915_drv.h |  9 ++++
 drivers/gpu/drm/i915/intel_pm.c | 92 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b040e7a..450e6d3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1624,6 +1624,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];
@@ -1926,6 +1932,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 a9f9396..da5ff34 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3225,6 +3225,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;
 
@@ -3265,6 +3270,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 {
@@ -3715,6 +3723,89 @@ 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.mem_channel == 0 || dev_priv->dmi.mem_speed == -1) {
+		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;
+			uint64_t plane_bw, interm_bw = 10000000;
+
+			if (fb == NULL)
+				continue;
+			if (plane->type == DRM_PLANE_TYPE_CURSOR)
+				continue;
+			num_active_plane++;
+
+			if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED)
+				y_tile_enabled = true;
+
+			/*
+			 * planeBW = pixel_rate(MHz) * BPP * plane downscale
+			 *		amount * pipe downscale amount;
+			 *
+			 * skl_pipe_pixel_rate return adjusted value according
+			 * to downscaling  amount
+			 * pixel rate is in KHz & downscale factor is multiplied
+			 * by 1000, so devide by 1000*1000
+			 */
+			interm_bw = skl_pipe_pixel_rate(cstate) *
+				drm_format_plane_cpp(fb->pixel_format, 0) *
+				skl_plane_downscale_amount(intel_plane);
+
+			if (fb->pixel_format == DRM_FORMAT_NV12)
+				interm_bw += skl_pipe_pixel_rate(cstate) *
+				drm_format_plane_cpp(fb->pixel_format, 1) *
+				skl_plane_downscale_amount(intel_plane);
+
+			plane_bw = DIV_ROUND_UP(interm_bw, (uint64_t) (1000 *
+						1000));
+			max_pixel_rate_plane = max(max_pixel_rate_plane,
+					plane_bw);
+		}
+		pipe_bw = max_pixel_rate_plane * num_active_plane;
+		max_pixel_rate_pipe = max(max_pixel_rate_pipe, pipe_bw);
+	}
+	display_bw = max_pixel_rate_pipe * num_active_crtc;
+
+	available_bw = dev_priv->dmi.mem_channel * dev_priv->dmi.mem_speed * 8;
+
+	memory_portion = DIV_ROUND_UP_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;
@@ -3752,6 +3843,7 @@ static void skl_update_wm(struct drm_crtc *crtc)
 
 	/* Calculate plane pixel rate for each plane in advance */
 	skl_set_plane_pixel_rate(crtc);
+	skl_set_display_memory_wa(dev);
 
 	if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
 		return;
-- 
2.5.0

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

Re: [v2 1/6] drm/i915/skl+: Use plane size for relative data rate calculation

[Kumar, Shobhit]

On 01/27/2016 09:39 PM, Shobhit Kumar wrote:
> 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.
>

Matt, request you to have a re-look at this series.

Regards
Shobhit

> Cc: matthew.d.roper@intel.com
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 42 ++++++++++++++++++++++++++---------------
>   1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 20bf854..d55e5d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2868,25 +2868,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);
>   }
>
>   /*
> @@ -2905,9 +2908,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;
>
> @@ -2965,8 +2967,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;
>
> @@ -2992,7 +2995,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;
> @@ -3116,28 +3119,37 @@ 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 bytes_per_pixel;
> +	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);
> +
>   	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
>   	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
>   				 bytes_per_pixel,
>   				 latency);
>   	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
>   				 cstate->base.adjusted_mode.crtc_htotal,
> -				 cstate->pipe_src_w,
> +				 width,
>   				 bytes_per_pixel,
>   				 fb->modifier[0],
>   				 latency);
>
> -	plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
> +	plane_bytes_per_line = width * bytes_per_pixel;
>   	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
>
>   	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v2 1/6] drm/i915/skl+: Use plane size for relative data rate calculation

[Matt Roper]

On Wed, Jan 27, 2016 at 09:39:58PM +0530, Shobhit Kumar wrote:
> 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>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 42 ++++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 20bf854..d55e5d0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2868,25 +2868,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);
>  }
>  
>  /*
> @@ -2905,9 +2908,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;
>  
> @@ -2965,8 +2967,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;
>  
> @@ -2992,7 +2995,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;
> @@ -3116,28 +3119,37 @@ 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 bytes_per_pixel;
> +	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);
> +
>  	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
>  	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
>  				 bytes_per_pixel,
>  				 latency);
>  	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
>  				 cstate->base.adjusted_mode.crtc_htotal,
> -				 cstate->pipe_src_w,
> +				 width,
>  				 bytes_per_pixel,
>  				 fb->modifier[0],
>  				 latency);
>  
> -	plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
> +	plane_bytes_per_line = width * bytes_per_pixel;
>  	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
>  
>  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> -- 
> 2.5.0
> 

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

Re: [v2 2/6] drm/i915/skl+: calculate ddb minimum allocation

[Matt Roper]

On Wed, Jan 27, 2016 at 09:39:59PM +0530, Shobhit Kumar wrote:
> 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.
> 
> Cc: matthew.d.roper@intel.com
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@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 d55e5d0..708f329 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2928,6 +2928,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 (y && !fb && !(fb->pixel_format == DRM_FORMAT_NV12))

I think you meant

        if (!fb || (y && fb->pixel_format != DRM_FORMAT_NV12))

right?

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

Minor nitpick, but it might be more readable to use != instead of
pulling the ! outside of the comparison.  On my first quick read I
missed the inversions.


Matt

> +
> +	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 */)
> @@ -2964,7 +3009,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)
> @@ -2973,9 +3017,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.5.0
> 

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

Re: [v2 2/6] drm/i915/skl+: calculate ddb minimum allocation

[Kumar, Shobhit]

On 02/05/2016 07:59 PM, Matt Roper wrote:
> On Wed, Jan 27, 2016 at 09:39:59PM +0530, Shobhit Kumar wrote:
>> 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.
>>
>> Cc: matthew.d.roper@intel.com
>> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@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 d55e5d0..708f329 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2928,6 +2928,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 (y && !fb && !(fb->pixel_format == DRM_FORMAT_NV12))
>
> I think you meant
>
>          if (!fb || (y && fb->pixel_format != DRM_FORMAT_NV12))
>
> right?
>

Yeah, looks like I made a blunder while testing and adding some fb 
checks in few places. This is one place it got messed up.

>> +		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;
>
> Minor nitpick, but it might be more readable to use != instead of
> pulling the ! outside of the comparison.  On my first quick read I
> missed the inversions.

Yeah will do.

>
>
> Matt
>
>> +
>> +	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 */)
>> @@ -2964,7 +3009,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)
>> @@ -2973,9 +3017,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.5.0
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v2 3/6] drm/i915/skl+: calculate plane pixel rate

[Matt Roper]

On Wed, Jan 27, 2016 at 09:40:00PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> Don't use pipe pixel rate for plane pixel rate. Calculate plane pixel according
> to formula
> 
> adjusted plane_pixel_rate = adjusted pipe_pixel_rate * downscale ammount
> 
> downscale amount = max[1, src_h/dst_h] * max[1, src_w/dst_w]
> if 90/270 rotation use rotated width & height
> 
> v2: use intel_plane_state->visible instead of (fb == NULL) as per Matt's
>     comment.
> 
> Cc: matthew.d.roper@intel.com
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  drivers/gpu/drm/i915/intel_pm.c  | 88 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bc97012..bb2b1c7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -638,6 +638,8 @@ struct intel_plane_wm_parameters {
>  	u64 tiling;
>  	unsigned int rotation;
>  	uint16_t fifo_size;
> +	/* Stores the adjusted plane pixel rate for WM calculation for SKL+ */
> +	uint32_t plane_pixel_rate;
>  };
>  
>  struct intel_plane {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 708f329..40fff09 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2782,6 +2782,48 @@ skl_wm_plane_id(const struct intel_plane *plane)
>  	}
>  }
>  
> +/*
> + * This function takes drm_plane_state as input
> + * and decides the downscale amount according to the formula
> + *
> + * downscale amount = Max[1, Horizontal source size / Horizontal dest size]
> + *
> + * Return value is multiplied by 1000 to retain fractional part
> + * Caller should take care of dividing & Rounding off the value
> + */
> +static uint32_t
> +skl_plane_downscale_amount(const struct intel_plane *intel_plane)
> +{
> +	struct drm_plane_state *pstate = intel_plane->base.state;
> +	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> +	uint32_t downscale_h, downscale_w;
> +	uint32_t src_w, src_h, dst_w, dst_h, tmp;
> +
> +	/* If plane not visible return amount as unity */
> +	if (!intel_pstate->visible)
> +		return 1000;
> +
> +	src_w = drm_rect_width(&intel_pstate->src) >> 16;
> +	src_h = drm_rect_height(&intel_pstate->src) >> 16;
> +
> +	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);
> +
> +	/* Multiply by 1000 for precision */
> +	tmp = (1000 * src_h) / dst_h;
> +	downscale_h = max_t(uint32_t, 1000, tmp);
> +
> +	tmp = (1000 * src_w) / dst_w;
> +	downscale_w = max_t(uint32_t, 1000, tmp);
> +
> +	/* Reducing precision to 3 decimal places */
> +	return DIV_ROUND_UP(downscale_h * downscale_w, 1000);
> +}

I think I mentioned it on my earlier review, but it feels like it would
be simpler/more consistent to just continue using 16.16 binary fixed
point instead of switching over to decimal fixed point (and maybe call
drm_rect_calc_[hv]scale to calculate the scaling).  Is there a specific
need to switch?

> +
> +
>  static void
>  skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
>  				   const struct intel_crtc_state *cstate,
> @@ -3183,10 +3225,10 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  		swap(width, height);
>  
>  	bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
> -	method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
> +	method1 = skl_wm_method1(intel_plane->wm.plane_pixel_rate,
>  				 bytes_per_pixel,
>  				 latency);
> -	method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
> +	method2 = skl_wm_method2(intel_plane->wm.plane_pixel_rate,
>  				 cstate->base.adjusted_mode.crtc_htotal,
>  				 width,
>  				 bytes_per_pixel,
> @@ -3627,6 +3669,45 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
>  	}
>  }
>  
> +static uint32_t
> +skl_plane_pixel_rate(struct intel_crtc_state *cstate, struct intel_plane *plane)
> +{
> +	uint32_t adjusted_pixel_rate;
> +	uint32_t downscale_amount;
> +
> +	/*
> +	 * adjusted plane pixel rate = adjusted pipe pixel rate
> +	 * Plane pixel rate = adjusted plane pixel rate * plane down scale
> +	 * amount
> +	 */
> +	adjusted_pixel_rate = skl_pipe_pixel_rate(cstate);
> +	downscale_amount = skl_plane_downscale_amount(plane);
> +
> +	return DIV_ROUND_UP(adjusted_pixel_rate * downscale_amount,
> +			1000);
> +}
> +
> +static void skl_set_plane_pixel_rate(struct drm_crtc *crtc)
> +{
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> +	struct intel_plane *intel_plane = NULL;
> +	struct drm_device *dev = crtc->dev;
> +
> +	if (!intel_crtc->active)
> +		return;
> +	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> +		struct drm_plane *plane = &intel_plane->base;
> +
> +		if (!to_intel_plane_state(plane->state)->visible)
> +			continue;
> +
> +		intel_plane->wm.plane_pixel_rate = skl_plane_pixel_rate(cstate,
> +				intel_plane);

Can we move this to intel_plane_state instead?  We've eliminated all
usage of intel_plane->wm from ILK and SKL watermarks, so that structure
is only used on VLV today.


Matt

> +	}
> +
> +}
> +
>  static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
>  {
>  	watermarks->wm_linetime[pipe] = 0;
> @@ -3662,6 +3743,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.5.0
> 

-- 
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: [v2 4/6] drm/i915/skl+: Use scaling amount for plane data rate calculation

[Matt Roper]

On Wed, Jan 27, 2016 at 09:40:01PM +0530, Shobhit Kumar wrote:
> 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.
> 
> Cc: matthew.d.roper@intel.com
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 40fff09..a9f9396 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2912,6 +2912,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;
> @@ -2923,15 +2925,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);

According to the coding style, I believe we're supposed to use braces
for both branches if either one of them needs braces.

Aside from that,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

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

-- 
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: [v2 5/6] drm/i915: Add support to parse DMI table and get platform memory info

[Matt Roper]

On Wed, Jan 27, 2016 at 09:40:02PM +0530, Shobhit Kumar wrote:
> 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
> 
> Cc: matthew.d.roper@intel.com
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 47 +++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index d70d96f..320143b 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -49,6 +49,7 @@
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/oom.h>
> +#include <linux/dmi.h>
>  
>  
>  static int i915_getparam(struct drm_device *dev, void *data,
> @@ -855,6 +856,49 @@ static void intel_init_dpio(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> +static void dmi_decode_memory_info(const struct dmi_header *hdr, void *priv)
> +{
> +	struct drm_i915_private *dev_priv = (struct drm_i915_private *) priv;
> +	const u8 *data = (const u8 *) hdr;
> +	uint16_t size, mem_speed;
> +
> +#define DMI_CONF_MEM_SPEED_OFFSET	0x20
> +#define DMI_MEM_SPEED_OFFSET		0x15
> +#define DMI_MEM_SIZE_OFFSET		0x0C
> +
> +	if (hdr->type == DMI_ENTRY_MEM_DEVICE) {
> +		/* Found a memory channel ? */
> +		size = (uint16_t) (*((uint16_t *)(data + DMI_MEM_SIZE_OFFSET)));

It might be nicer/cleaner to copy over the memdev_dmi_entry struct from
drivers/edac/i7core_edac.c and cast the data pointer into that to avoid
all the pointer arithmetic.  But all of your calculations look correct
to me, so probably not a huge deal either way.

> +		if (size == 0)
> +			return;
> +
> +		dev_priv->dmi.mem_channel++;
> +
> +		/* Get the speed */
> +		if (hdr->length > DMI_CONF_MEM_SPEED_OFFSET)
> +			mem_speed =
> +				(uint16_t) (*((uint16_t *)(data + DMI_CONF_MEM_SPEED_OFFSET)));
> +		else if (hdr->length > DMI_MEM_SPEED_OFFSET)
> +			mem_speed =
> +				(uint16_t) (*((uint16_t *)(data + DMI_MEM_SPEED_OFFSET)));

I think we have more layers of casting here than necessary?

> +		else
> +			mem_speed = -1;

mem_speed is a uint, so down below it's actually going to pass the
mem_speed > 0 tests, which I don't think was your intent.


Matt

> +
> +		/*
> +		 * Check all channels have same speed
> +		 * else mark speed as invalid
> +		 */
> +		if (dev_priv->dmi.mem_speed == 0) {
> +			if (mem_speed > 0)
> +				dev_priv->dmi.mem_speed = mem_speed;
> +			else
> +				dev_priv->dmi.mem_speed = -1;
> +		} else if (dev_priv->dmi.mem_speed > 0 &&
> +					dev_priv->dmi.mem_speed != mem_speed)
> +			dev_priv->dmi.mem_speed = -1;
> +	}
> +}
> +
>  /**
>   * i915_driver_load - setup chip and create an initial config
>   * @dev: DRM device
> @@ -882,6 +926,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	dev->dev_private = dev_priv;
>  	dev_priv->dev = dev;
>  
> +	/* walk the dmi device table for getting platform memory information */
> +	dmi_walk(dmi_decode_memory_info, (void *) dev_priv);
> +
>  	/* Setup the write-once "constant" device info */
>  	device_info = (struct intel_device_info *)&dev_priv->info;
>  	memcpy(device_info, info, sizeof(dev_priv->info));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 211af53..b040e7a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1968,6 +1968,12 @@ struct drm_i915_private {
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> +
> +	/* DMI data for memory bandwidth calculation */
> +	struct {
> +		uint16_t mem_channel;
> +		int16_t mem_speed;
> +	} dmi;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> -- 
> 2.5.0
> 

-- 
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: [v2 4/6] drm/i915/skl+: Use scaling amount for plane data rate calculation

[Daniel Vetter]

On Wed, Feb 10, 2016 at 11:39:41AM -0800, Matt Roper wrote:
> On Wed, Jan 27, 2016 at 09:40:01PM +0530, Shobhit Kumar wrote:
> > 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.
> > 
> > Cc: matthew.d.roper@intel.com
> > Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 40fff09..a9f9396 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2912,6 +2912,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;
> > @@ -2923,15 +2925,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);
> 
> According to the coding style, I believe we're supposed to use braces
> for both branches if either one of them needs braces.

Yes, and definitely when the other branch spans more than 1 line.
-Daniel

> 
> Aside from that,
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> 
> > +
> > +	down_scale_amount = skl_plane_downscale_amount(intel_plane);
> > +
> > +	return DIV_ROUND_UP((data_rate * down_scale_amount), 1000);
> >  
> > -	/* for packed formats */
> > -	return width * height * drm_format_plane_cpp(fb->pixel_format, 0);
> >  }
> >  
> >  /*
> > -- 
> > 2.5.0
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx