All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment
@ 2019-12-27 23:51 Imre Deak
  2019-12-27 23:51 ` [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned Imre Deak
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Imre Deak @ 2019-12-27 23:51 UTC (permalink / raw)
  To: intel-gfx

At least one framebuffer plane on TGL - the UV plane of YUV semiplanar
FBs - requires a non-power-of-2 alignment, so add support for this. This
new alignment restriction applies only to an offset within an FB, so the
GEM buffer itself containing the FB must still be power-of-2 aligned.
Add a check for this (in practice plane 0, since the plane 0 offset must
be 0).

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 22 +++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 624ba9be7293..d8970198c77e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2194,6 +2194,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 		return ERR_PTR(-EINVAL);
 
 	alignment = intel_surf_alignment(fb, 0);
+	WARN_ON(!is_power_of_2(alignment));
 
 	/* Note that the w/a also requires 64 PTE of padding following the
 	 * bo. We currently fill all unused PTE with the shadow page and so
@@ -2432,9 +2433,6 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
 	unsigned int cpp = fb->format->cpp[color_plane];
 	u32 offset, offset_aligned;
 
-	if (alignment)
-		alignment--;
-
 	if (!is_surface_linear(fb, color_plane)) {
 		unsigned int tile_size, tile_width, tile_height;
 		unsigned int tile_rows, tiles, pitch_tiles;
@@ -2456,17 +2454,24 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
 		*x %= tile_width;
 
 		offset = (tile_rows * pitch_tiles + tiles) * tile_size;
-		offset_aligned = offset & ~alignment;
+
+		offset_aligned = offset;
+		if (alignment)
+			offset_aligned = rounddown(offset_aligned, alignment);
 
 		intel_adjust_tile_offset(x, y, tile_width, tile_height,
 					 tile_size, pitch_tiles,
 					 offset, offset_aligned);
 	} else {
 		offset = *y * pitch + *x * cpp;
-		offset_aligned = offset & ~alignment;
-
-		*y = (offset & alignment) / pitch;
-		*x = ((offset & alignment) - *y * pitch) / cpp;
+		offset_aligned = offset;
+		if (alignment) {
+			offset_aligned = rounddown(offset_aligned, alignment);
+			*y = (offset % alignment) / pitch;
+			*x = ((offset % alignment) - *y * pitch) / cpp;
+		} else {
+			*y = *x = 0;
+		}
 	}
 
 	return offset_aligned;
@@ -3738,6 +3743,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 	offset = intel_plane_compute_aligned_offset(&x, &y, plane_state, 0);
 	alignment = intel_surf_alignment(fb, 0);
+	WARN_ON(!is_power_of_2(alignment));
 
 	/*
 	 * AUX surface offset is specified as the distance from the
-- 
2.23.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned
  2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
@ 2019-12-27 23:51 ` Imre Deak
  2019-12-30 10:53   ` Chris Wilson
  2019-12-27 23:51 ` [Intel-gfx] [PATCH 3/3] drm/i915: Add debug message for FB plane[0].offset!=0 error Imre Deak
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Imre Deak @ 2019-12-27 23:51 UTC (permalink / raw)
  To: intel-gfx

Currently the start address of a UV plane in a semiplanar YUV FB is tile
size (4kB) aligned. I noticed, that enforcing only this alignment leads
oddly to random memory corruptions on TGL while scanning out Y-tiled
FBs. This issue can be easily reproduced with a UV plane that is not
aligned to the plane's tile row size.

Some experiments showed the correct alignment to be tile row size
indeed. This also makes sense, since the de-tiling fence created for the
object - with its own stride and so "left" and "right" edge - applies to
all the planes in the FB, so each tile row of all planes should be tile
row aligned.

In fact BSpec requires this alignment since SKL. On SKL we may enforce
this due to the AUX plane x,y coords check, but on ICL and TGL we don't.
For now enforce this only on TGL; I can follow up with any necessary
change for ICL after more tests.

BSpec requires a stricter alignment for linear UV planes too (kind of a
tile row alignment), but it's unclear whether that's really needed
(couldn't be explained with the de-tiling fence as above) and enforcing
that could break existing user space; so avoid that too for now until
more tests.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 36 ++++++++++++++++++--
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index d8970198c77e..de382bba9a91 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1995,6 +1995,13 @@ intel_format_info_is_yuv_semiplanar(const struct drm_format_info *info,
 	       info->num_planes == (is_ccs_modifier(modifier) ? 4 : 2);
 }
 
+static bool is_semiplanar_uv_plane(const struct drm_framebuffer *fb,
+				   int color_plane)
+{
+	return intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier) &&
+	       color_plane == 1;
+}
+
 static unsigned int
 intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
 {
@@ -2069,6 +2076,16 @@ static void intel_tile_dims(const struct drm_framebuffer *fb, int color_plane,
 	*tile_height = intel_tile_height(fb, color_plane);
 }
 
+static unsigned int intel_tile_row_size(const struct drm_framebuffer *fb,
+					int color_plane)
+{
+	unsigned int tile_width, tile_height;
+
+	intel_tile_dims(fb, color_plane, &tile_width, &tile_height);
+
+	return fb->pitches[color_plane] * tile_height;
+}
+
 unsigned int
 intel_fb_align_height(const struct drm_framebuffer *fb,
 		      int color_plane, unsigned int height)
@@ -2143,7 +2160,8 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
 	struct drm_i915_private *dev_priv = to_i915(fb->dev);
 
 	/* AUX_DIST needs only 4K alignment */
-	if (is_aux_plane(fb, color_plane))
+	if ((INTEL_GEN(dev_priv) < 12 && is_aux_plane(fb, color_plane)) ||
+	    is_ccs_plane(fb, color_plane))
 		return 4096;
 
 	switch (fb->modifier) {
@@ -2158,6 +2176,10 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
 	case I915_FORMAT_MOD_Y_TILED_CCS:
 	case I915_FORMAT_MOD_Yf_TILED_CCS:
 	case I915_FORMAT_MOD_Y_TILED:
+		if (INTEL_GEN(dev_priv) >= 12 &&
+		    is_semiplanar_uv_plane(fb, color_plane))
+			return intel_tile_row_size(fb, color_plane);
+		/* Fall-through */
 	case I915_FORMAT_MOD_Yf_TILED:
 		return 1 * 1024 * 1024;
 	default:
@@ -2504,9 +2526,17 @@ static int intel_fb_offset_to_xy(int *x, int *y,
 {
 	struct drm_i915_private *dev_priv = to_i915(fb->dev);
 	unsigned int height;
+	u32 alignment;
+
+	if (INTEL_GEN(dev_priv) >= 12 &&
+	    is_semiplanar_uv_plane(fb, color_plane))
+		alignment = intel_tile_row_size(fb, color_plane);
+	else if (fb->modifier != DRM_FORMAT_MOD_LINEAR)
+		alignment = intel_tile_size(dev_priv);
+	else
+		alignment = 0;
 
-	if (fb->modifier != DRM_FORMAT_MOD_LINEAR &&
-	    fb->offsets[color_plane] % intel_tile_size(dev_priv)) {
+	if (alignment != 0 && fb->offsets[color_plane] % alignment) {
 		DRM_DEBUG_KMS("Misaligned offset 0x%08x for color plane %d\n",
 			      fb->offsets[color_plane], color_plane);
 		return -EINVAL;
-- 
2.23.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH 3/3] drm/i915: Add debug message for FB plane[0].offset!=0 error
  2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
  2019-12-27 23:51 ` [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned Imre Deak
@ 2019-12-27 23:51 ` Imre Deak
  2019-12-28  0:32 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Patchwork
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Imre Deak @ 2019-12-27 23:51 UTC (permalink / raw)
  To: intel-gfx

Print a debug message if the FB plane[0] offset is not 0 as expected, to
help understainding an add FB IOCTL fail.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index de382bba9a91..3d2f7e12dcc1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16723,8 +16723,11 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 	}
 
 	/* FIXME need to adjust LINOFF/TILEOFF accordingly. */
-	if (mode_cmd->offsets[0] != 0)
+	if (mode_cmd->offsets[0] != 0) {
+		DRM_DEBUG_KMS("plane 0 offset (0x%08x) must be 0\n",
+			      mode_cmd->offsets[0]);
 		goto err;
+	}
 
 	drm_helper_mode_fill_fb_struct(&dev_priv->drm, fb, mode_cmd);
 
-- 
2.23.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Add support for non-power-of-2 FB plane alignment
  2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
  2019-12-27 23:51 ` [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned Imre Deak
  2019-12-27 23:51 ` [Intel-gfx] [PATCH 3/3] drm/i915: Add debug message for FB plane[0].offset!=0 error Imre Deak
@ 2019-12-28  0:32 ` Patchwork
  2019-12-28  0:47 ` [Intel-gfx] [PATCH v2 1/3] " Imre Deak
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-12-28  0:32 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Add support for non-power-of-2 FB plane alignment
URL   : https://patchwork.freedesktop.org/series/71444/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7644 -> Patchwork_15934
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15934 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15934, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15934:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_busy@basic-flip-pipe-a:
    - fi-ivb-3770:        [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-ivb-3770/igt@kms_busy@basic-flip-pipe-a.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-ivb-3770/igt@kms_busy@basic-flip-pipe-a.html
    - fi-ilk-650:         NOTRUN -> [DMESG-WARN][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-ilk-650/igt@kms_busy@basic-flip-pipe-a.html
    - fi-snb-2600:        NOTRUN -> [DMESG-WARN][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-snb-2600/igt@kms_busy@basic-flip-pipe-a.html
    - fi-bsw-kefka:       [PASS][5] -> [DMESG-WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bsw-kefka/igt@kms_busy@basic-flip-pipe-a.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bsw-kefka/igt@kms_busy@basic-flip-pipe-a.html
    - fi-hsw-peppy:       NOTRUN -> [DMESG-WARN][7]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-hsw-peppy/igt@kms_busy@basic-flip-pipe-a.html
    - fi-bdw-5557u:       [PASS][8] -> [DMESG-WARN][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bdw-5557u/igt@kms_busy@basic-flip-pipe-a.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bdw-5557u/igt@kms_busy@basic-flip-pipe-a.html
    - fi-snb-2520m:       [PASS][10] -> [DMESG-WARN][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-snb-2520m/igt@kms_busy@basic-flip-pipe-a.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-snb-2520m/igt@kms_busy@basic-flip-pipe-a.html
    - fi-hsw-4770:        [PASS][12] -> [DMESG-WARN][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-hsw-4770/igt@kms_busy@basic-flip-pipe-a.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-hsw-4770/igt@kms_busy@basic-flip-pipe-a.html
    - fi-byt-n2820:       [PASS][14] -> [DMESG-WARN][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-byt-n2820/igt@kms_busy@basic-flip-pipe-a.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-byt-n2820/igt@kms_busy@basic-flip-pipe-a.html
    - fi-elk-e7500:       [PASS][16] -> [DMESG-WARN][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-elk-e7500/igt@kms_busy@basic-flip-pipe-a.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-elk-e7500/igt@kms_busy@basic-flip-pipe-a.html
    - fi-byt-j1900:       NOTRUN -> [DMESG-WARN][18]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-byt-j1900/igt@kms_busy@basic-flip-pipe-a.html

  * igt@kms_busy@basic-flip-pipe-c:
    - fi-bsw-n3050:       [PASS][19] -> [DMESG-WARN][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bsw-n3050/igt@kms_busy@basic-flip-pipe-c.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bsw-n3050/igt@kms_busy@basic-flip-pipe-c.html

  * igt@runner@aborted:
    - fi-pnv-d510:        NOTRUN -> [FAIL][21]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-pnv-d510/igt@runner@aborted.html
    - fi-hsw-peppy:       NOTRUN -> [FAIL][22]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-hsw-peppy/igt@runner@aborted.html
    - fi-gdg-551:         NOTRUN -> [FAIL][23]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-gdg-551/igt@runner@aborted.html
    - fi-snb-2520m:       NOTRUN -> [FAIL][24]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-snb-2520m/igt@runner@aborted.html
    - fi-byt-n2820:       NOTRUN -> [FAIL][25]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-byt-n2820/igt@runner@aborted.html
    - fi-hsw-4770:        NOTRUN -> [FAIL][26]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-hsw-4770/igt@runner@aborted.html
    - fi-snb-2600:        NOTRUN -> [FAIL][27]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-snb-2600/igt@runner@aborted.html
    - fi-ivb-3770:        NOTRUN -> [FAIL][28]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-ivb-3770/igt@runner@aborted.html
    - fi-byt-j1900:       NOTRUN -> [FAIL][29]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-byt-j1900/igt@runner@aborted.html
    - fi-blb-e6850:       NOTRUN -> [FAIL][30]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-blb-e6850/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_15934 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-6700k2:      [PASS][31] -> [INCOMPLETE][32] ([i915#671])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-6700k2/igt@i915_module_load@reload-with-fault-injection.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-skl-6700k2/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-guc:         [PASS][33] -> [SKIP][34] ([fdo#109271])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_blt:
    - fi-bsw-nick:        [PASS][35] -> [DMESG-FAIL][36] ([i915#723])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bsw-nick/igt@i915_selftest@live_blt.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bsw-nick/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8700k:       [PASS][37] -> [INCOMPLETE][38] ([i915#424])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
    - fi-cfl-guc:         [PASS][39] -> [INCOMPLETE][40] ([fdo#106070] / [i915#424])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-cfl-guc/igt@i915_selftest@live_gem_contexts.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-cfl-guc/igt@i915_selftest@live_gem_contexts.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-6770hq:      [INCOMPLETE][41] ([i915#671]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-skl-6770hq/igt@i915_module_load@reload-with-fault-injection.html
    - fi-skl-lmem:        [INCOMPLETE][43] ([i915#671]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6600u:       [DMESG-WARN][45] ([i915#889]) -> [PASS][46] +13 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_client:
    - fi-skl-6600u:       [DMESG-WARN][47] -> [PASS][48] +9 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-6600u/igt@i915_selftest@live_client.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-skl-6600u/igt@i915_selftest@live_client.html

  * igt@i915_selftest@live_gt_engines:
    - fi-bxt-dsi:         [DMESG-FAIL][49] ([i915#889]) -> [PASS][50] +3 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bxt-dsi/igt@i915_selftest@live_gt_engines.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bxt-dsi/igt@i915_selftest@live_gt_engines.html

  * igt@i915_selftest@live_gt_lrc:
    - fi-skl-6600u:       [DMESG-FAIL][51] ([i915#889]) -> [PASS][52] +3 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-6600u/igt@i915_selftest@live_gt_lrc.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-skl-6600u/igt@i915_selftest@live_gt_lrc.html

  * igt@i915_selftest@live_hugepages:
    - fi-bxt-dsi:         [DMESG-WARN][53] -> [PASS][54] +9 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bxt-dsi/igt@i915_selftest@live_hugepages.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bxt-dsi/igt@i915_selftest@live_hugepages.html

  * igt@i915_selftest@live_mman:
    - fi-bxt-dsi:         [DMESG-WARN][55] ([i915#889]) -> [PASS][56] +13 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bxt-dsi/igt@i915_selftest@live_mman.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bxt-dsi/igt@i915_selftest@live_mman.html

  * igt@i915_selftest@live_reset:
    - fi-skl-6600u:       [DMESG-FAIL][57] -> [PASS][58] +3 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-6600u/igt@i915_selftest@live_reset.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-skl-6600u/igt@i915_selftest@live_reset.html
    - fi-bxt-dsi:         [DMESG-FAIL][59] -> [PASS][60] +3 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bxt-dsi/igt@i915_selftest@live_reset.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-bxt-dsi/igt@i915_selftest@live_reset.html

  
#### Warnings ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][61] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][62] ([i915#62] / [i915#92]) +3 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-edid:
    - fi-kbl-x1275:       [DMESG-WARN][63] ([i915#62] / [i915#92]) -> [DMESG-WARN][64] ([i915#62] / [i915#92] / [i915#95]) +4 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html

  
  [fdo#106070]: https://bugs.freedesktop.org/show_bug.cgi?id=106070
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#671]: https://gitlab.freedesktop.org/drm/intel/issues/671
  [i915#723]: https://gitlab.freedesktop.org/drm/intel/issues/723
  [i915#889]: https://gitlab.freedesktop.org/drm/intel/issues/889
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (42 -> 42)
------------------------------

  Additional (8): fi-byt-j1900 fi-hsw-peppy fi-ilk-650 fi-gdg-551 fi-kbl-8809g fi-blb-e6850 fi-kbl-r fi-snb-2600 
  Missing    (8): fi-hsw-4770r fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7644 -> Patchwork_15934

  CI-20190529: 20190529
  CI_DRM_7644: 103086964d18ac22a8e333b9ea7649f67c468f7b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5352: 0586d205f651674e575351c2d5a7d0760716c9f1 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15934: 80df0ef0833e4d109e47cbf329160cbcb2dd1645 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

80df0ef0833e drm/i915: Add debug message for FB plane[0].offset!=0 error
0f0022b981cb drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned
ed691a592743 drm/i915: Add support for non-power-of-2 FB plane alignment

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15934/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH v2 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment
  2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
                   ` (2 preceding siblings ...)
  2019-12-28  0:32 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Patchwork
@ 2019-12-28  0:47 ` Imre Deak
  2019-12-28  1:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Add support for non-power-of-2 FB plane alignment (rev2) Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Imre Deak @ 2019-12-28  0:47 UTC (permalink / raw)
  To: intel-gfx

At least one framebuffer plane on TGL - the UV plane of YUV semiplanar
FBs - requires a non-power-of-2 alignment, so add support for this. This
new alignment restriction applies only to an offset within an FB, so the
GEM buffer itself containing the FB must still be power-of-2 aligned.
Add a check for this (in practice plane 0, since the plane 0 offset must
be 0).

v2:
- Fix WARN check for alignment=0.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 22 +++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 624ba9be7293..3242185a106d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2194,6 +2194,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 		return ERR_PTR(-EINVAL);
 
 	alignment = intel_surf_alignment(fb, 0);
+	WARN_ON(alignment && !is_power_of_2(alignment));
 
 	/* Note that the w/a also requires 64 PTE of padding following the
 	 * bo. We currently fill all unused PTE with the shadow page and so
@@ -2432,9 +2433,6 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
 	unsigned int cpp = fb->format->cpp[color_plane];
 	u32 offset, offset_aligned;
 
-	if (alignment)
-		alignment--;
-
 	if (!is_surface_linear(fb, color_plane)) {
 		unsigned int tile_size, tile_width, tile_height;
 		unsigned int tile_rows, tiles, pitch_tiles;
@@ -2456,17 +2454,24 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
 		*x %= tile_width;
 
 		offset = (tile_rows * pitch_tiles + tiles) * tile_size;
-		offset_aligned = offset & ~alignment;
+
+		offset_aligned = offset;
+		if (alignment)
+			offset_aligned = rounddown(offset_aligned, alignment);
 
 		intel_adjust_tile_offset(x, y, tile_width, tile_height,
 					 tile_size, pitch_tiles,
 					 offset, offset_aligned);
 	} else {
 		offset = *y * pitch + *x * cpp;
-		offset_aligned = offset & ~alignment;
-
-		*y = (offset & alignment) / pitch;
-		*x = ((offset & alignment) - *y * pitch) / cpp;
+		offset_aligned = offset;
+		if (alignment) {
+			offset_aligned = rounddown(offset_aligned, alignment);
+			*y = (offset % alignment) / pitch;
+			*x = ((offset % alignment) - *y * pitch) / cpp;
+		} else {
+			*y = *x = 0;
+		}
 	}
 
 	return offset_aligned;
@@ -3738,6 +3743,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 	offset = intel_plane_compute_aligned_offset(&x, &y, plane_state, 0);
 	alignment = intel_surf_alignment(fb, 0);
+	WARN_ON(alignment && !is_power_of_2(alignment));
 
 	/*
 	 * AUX surface offset is specified as the distance from the
-- 
2.23.1

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Add support for non-power-of-2 FB plane alignment (rev2)
  2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
                   ` (3 preceding siblings ...)
  2019-12-28  0:47 ` [Intel-gfx] [PATCH v2 1/3] " Imre Deak
@ 2019-12-28  1:22 ` Patchwork
  2019-12-28  2:41 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2019-12-30 10:48 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Chris Wilson
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-12-28  1:22 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Add support for non-power-of-2 FB plane alignment (rev2)
URL   : https://patchwork.freedesktop.org/series/71444/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7644 -> Patchwork_15935
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/index.html

Known issues
------------

  Here are the changes found in Patchwork_15935 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_tiled_blits@basic:
    - fi-byt-n2820:       [PASS][1] -> [FAIL][2] ([i915#832])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-byt-n2820/igt@gem_tiled_blits@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/fi-byt-n2820/igt@gem_tiled_blits@basic.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-6700k2:      [PASS][3] -> [INCOMPLETE][4] ([i915#671])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-6700k2/igt@i915_module_load@reload-with-fault-injection.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/fi-skl-6700k2/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_blt:
    - fi-ivb-3770:        [PASS][5] -> [DMESG-FAIL][6] ([i915#725])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-ivb-3770/igt@i915_selftest@live_blt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/fi-ivb-3770/igt@i915_selftest@live_blt.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-lmem:        [INCOMPLETE][7] ([i915#671]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html

  
#### Warnings ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-bxt-dsi:         [DMESG-WARN][9] ([i915#889]) -> [INCOMPLETE][10] ([fdo#103927])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-bxt-dsi/igt@i915_module_load@reload-with-fault-injection.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/fi-bxt-dsi/igt@i915_module_load@reload-with-fault-injection.html

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-kbl-x1275:       [DMESG-WARN][11] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][12] ([i915#62] / [i915#92]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html

  * igt@kms_force_connector_basic@force-edid:
    - fi-kbl-x1275:       [DMESG-WARN][13] ([i915#62] / [i915#92]) -> [DMESG-WARN][14] ([i915#62] / [i915#92] / [i915#95]) +5 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html

  
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#671]: https://gitlab.freedesktop.org/drm/intel/issues/671
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#832]: https://gitlab.freedesktop.org/drm/intel/issues/832
  [i915#889]: https://gitlab.freedesktop.org/drm/intel/issues/889
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (42 -> 43)
------------------------------

  Additional (8): fi-byt-j1900 fi-hsw-peppy fi-ilk-650 fi-gdg-551 fi-kbl-8809g fi-blb-e6850 fi-kbl-r fi-snb-2600 
  Missing    (7): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus fi-byt-clapper fi-skl-6600u 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7644 -> Patchwork_15935

  CI-20190529: 20190529
  CI_DRM_7644: 103086964d18ac22a8e333b9ea7649f67c468f7b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5352: 0586d205f651674e575351c2d5a7d0760716c9f1 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15935: 0da4bd71dd5acb05648abadf42113d8d9cbaaf38 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0da4bd71dd5a drm/i915: Add debug message for FB plane[0].offset!=0 error
7a13dc270b8a drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned
58f02cd86f46 drm/i915: Add support for non-power-of-2 FB plane alignment

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v2,1/3] drm/i915: Add support for non-power-of-2 FB plane alignment (rev2)
  2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
                   ` (4 preceding siblings ...)
  2019-12-28  1:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Add support for non-power-of-2 FB plane alignment (rev2) Patchwork
@ 2019-12-28  2:41 ` Patchwork
  2019-12-30 10:48 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Chris Wilson
  6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-12-28  2:41 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Add support for non-power-of-2 FB plane alignment (rev2)
URL   : https://patchwork.freedesktop.org/series/71444/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7644_full -> Patchwork_15935_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_15935_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vcs1-none:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#109276] / [fdo#112080]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb4/igt@gem_ctx_isolation@vcs1-none.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb5/igt@gem_ctx_isolation@vcs1-none.html

  * igt@gem_ctx_persistence@vecs0-mixed-process:
    - shard-apl:          [PASS][3] -> [FAIL][4] ([i915#679])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-apl2/igt@gem_ctx_persistence@vecs0-mixed-process.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-apl8/igt@gem_ctx_persistence@vecs0-mixed-process.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#110854])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb2/igt@gem_exec_balancer@smoke.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb7/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#109276]) +10 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb7/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#112146]) +8 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb6/igt@gem_exec_schedule@reorder-wide-bsd.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb2/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([i915#454])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb5/igt@i915_pm_dc@dc6-psr.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb2/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@system-suspend-modeset:
    - shard-skl:          [PASS][13] -> [INCOMPLETE][14] ([i915#151] / [i915#69])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-skl8/igt@i915_pm_rpm@system-suspend-modeset.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-skl3/igt@i915_pm_rpm@system-suspend-modeset.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([i915#54]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-skl1/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-apl:          [PASS][17] -> [DMESG-WARN][18] ([i915#180])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-apl3/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-apl8/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_legacy@all-pipes-torture-move:
    - shard-glk:          [PASS][19] -> [DMESG-WARN][20] ([i915#128])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-glk2/igt@kms_cursor_legacy@all-pipes-torture-move.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-glk6/igt@kms_cursor_legacy@all-pipes-torture-move.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([IGT#5])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-skl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-skl:          [PASS][23] -> [DMESG-WARN][24] ([i915#109])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-skl2/igt@kms_plane@pixel-format-pipe-c-planes.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-skl8/igt@kms_plane@pixel-format-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109441]) +2 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb3/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@perf_pmu@init-busy-vcs1:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#112080]) +7 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb2/igt@perf_pmu@init-busy-vcs1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb5/igt@perf_pmu@init-busy-vcs1.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@bcs0-mixed-process:
    - shard-apl:          [FAIL][31] ([i915#679]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-apl6/igt@gem_ctx_persistence@bcs0-mixed-process.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-apl7/igt@gem_ctx_persistence@bcs0-mixed-process.html

  * igt@gem_ctx_persistence@rcs0-mixed-process:
    - shard-skl:          [FAIL][33] ([i915#679]) -> [PASS][34] +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-skl1/igt@gem_ctx_persistence@rcs0-mixed-process.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-skl4/igt@gem_ctx_persistence@rcs0-mixed-process.html

  * igt@gem_ctx_persistence@vcs1-mixed-process:
    - shard-iclb:         [SKIP][35] ([fdo#109276] / [fdo#112080]) -> [PASS][36] +3 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb8/igt@gem_ctx_persistence@vcs1-mixed-process.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb1/igt@gem_ctx_persistence@vcs1-mixed-process.html

  * igt@gem_eio@kms:
    - shard-glk:          [INCOMPLETE][37] ([i915#58] / [k.org#198133]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-glk2/igt@gem_eio@kms.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-glk9/igt@gem_eio@kms.html

  * igt@gem_eio@reset-stress:
    - shard-snb:          [FAIL][39] ([i915#232]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-snb7/igt@gem_eio@reset-stress.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-snb6/igt@gem_eio@reset-stress.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [SKIP][41] ([fdo#112146]) -> [PASS][42] +4 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb4/igt@gem_exec_schedule@in-order-bsd.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb8/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive:
    - shard-apl:          [TIMEOUT][43] ([i915#530]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-apl3/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-apl6/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x85-sliding:
    - shard-skl:          [FAIL][45] ([i915#54]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-skl7/igt@kms_cursor_crc@pipe-b-cursor-256x85-sliding.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-skl2/igt@kms_cursor_crc@pipe-b-cursor-256x85-sliding.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][47] ([i915#79]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-kbl:          [DMESG-WARN][49] ([i915#180]) -> [PASS][50] +3 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52] +3 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [SKIP][53] ([fdo#109441]) -> [PASS][54] +2 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb6/igt@kms_psr@psr2_cursor_plane_move.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@perf_pmu@busy-accuracy-2-vcs1:
    - shard-iclb:         [SKIP][55] ([fdo#112080]) -> [PASS][56] +13 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb8/igt@perf_pmu@busy-accuracy-2-vcs1.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb4/igt@perf_pmu@busy-accuracy-2-vcs1.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][57] ([fdo#109276]) -> [PASS][58] +24 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb6/igt@prime_busy@hang-bsd2.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb4/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][59] ([IGT#28]) -> [SKIP][60] ([fdo#109276] / [fdo#112080])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_ctx_isolation@vcs1-nonpriv-switch:
    - shard-iclb:         [SKIP][61] ([fdo#109276] / [fdo#112080]) -> [FAIL][62] ([IGT#28])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-iclb6/igt@gem_ctx_isolation@vcs1-nonpriv-switch.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-iclb4/igt@gem_ctx_isolation@vcs1-nonpriv-switch.html

  * igt@gem_tiled_blits@interruptible:
    - shard-hsw:          [FAIL][63] ([i915#832]) -> [FAIL][64] ([i915#818])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7644/shard-hsw8/igt@gem_tiled_blits@interruptible.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/shard-hsw1/igt@gem_tiled_blits@interruptible.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [IGT#28]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/28
  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#128]: https://gitlab.freedesktop.org/drm/intel/issues/128
  [i915#151]: https://gitlab.freedesktop.org/drm/intel/issues/151
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#232]: https://gitlab.freedesktop.org/drm/intel/issues/232
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#530]: https://gitlab.freedesktop.org/drm/intel/issues/530
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818
  [i915#832]: https://gitlab.freedesktop.org/drm/intel/issues/832
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7644 -> Patchwork_15935

  CI-20190529: 20190529
  CI_DRM_7644: 103086964d18ac22a8e333b9ea7649f67c468f7b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5352: 0586d205f651674e575351c2d5a7d0760716c9f1 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15935: 0da4bd71dd5acb05648abadf42113d8d9cbaaf38 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15935/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment
  2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
                   ` (5 preceding siblings ...)
  2019-12-28  2:41 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2019-12-30 10:48 ` Chris Wilson
  2019-12-30 18:39   ` Imre Deak
  6 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2019-12-30 10:48 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

Quoting Imre Deak (2019-12-27 23:51:45)
> At least one framebuffer plane on TGL - the UV plane of YUV semiplanar
> FBs - requires a non-power-of-2 alignment, so add support for this. This
> new alignment restriction applies only to an offset within an FB, so the
> GEM buffer itself containing the FB must still be power-of-2 aligned.

It's worth talking about virtual memory alignment (in the GGTT) here and
not the physical alignment of the backing store. The buffer itself plays
no part here.

> Add a check for this (in practice plane 0, since the plane 0 offset must
> be 0).
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 22 +++++++++++++-------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 624ba9be7293..d8970198c77e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2194,6 +2194,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>                 return ERR_PTR(-EINVAL);
>  
>         alignment = intel_surf_alignment(fb, 0);
> +       WARN_ON(!is_power_of_2(alignment));

Handle the error, if you are going to the trouble to warn, add the
return as well.

>  
>         /* Note that the w/a also requires 64 PTE of padding following the
>          * bo. We currently fill all unused PTE with the shadow page and so
> @@ -2432,9 +2433,6 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
>         unsigned int cpp = fb->format->cpp[color_plane];
>         u32 offset, offset_aligned;
>  
> -       if (alignment)
> -               alignment--;
> -
>         if (!is_surface_linear(fb, color_plane)) {
>                 unsigned int tile_size, tile_width, tile_height;
>                 unsigned int tile_rows, tiles, pitch_tiles;
> @@ -2456,17 +2454,24 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
>                 *x %= tile_width;
>  
>                 offset = (tile_rows * pitch_tiles + tiles) * tile_size;
> -               offset_aligned = offset & ~alignment;
> +
> +               offset_aligned = offset;
> +               if (alignment)
> +                       offset_aligned = rounddown(offset_aligned, alignment);
>  
>                 intel_adjust_tile_offset(x, y, tile_width, tile_height,
>                                          tile_size, pitch_tiles,
>                                          offset, offset_aligned);
>         } else {
>                 offset = *y * pitch + *x * cpp;
> -               offset_aligned = offset & ~alignment;
> -
> -               *y = (offset & alignment) / pitch;
> -               *x = ((offset & alignment) - *y * pitch) / cpp;
> +               offset_aligned = offset;
> +               if (alignment) {
> +                       offset_aligned = rounddown(offset_aligned, alignment);
> +                       *y = (offset % alignment) / pitch;
> +                       *x = ((offset % alignment) - *y * pitch) / cpp;
> +               } else {
> +                       *y = *x = 0;
> +               }
>         }
>  
>         return offset_aligned;
> @@ -3738,6 +3743,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>         intel_add_fb_offsets(&x, &y, plane_state, 0);
>         offset = intel_plane_compute_aligned_offset(&x, &y, plane_state, 0);
>         alignment = intel_surf_alignment(fb, 0);
> +       WARN_ON(!is_power_of_2(alignment));

The other two are expected to handle !is_pot...

I would strongly suggest handling the WARNs, or else you may as well bug
out for the programming error.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned
  2019-12-27 23:51 ` [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned Imre Deak
@ 2019-12-30 10:53   ` Chris Wilson
  2019-12-30 19:20     ` Imre Deak
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2019-12-30 10:53 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

Quoting Imre Deak (2019-12-27 23:51:46)
> Currently the start address of a UV plane in a semiplanar YUV FB is tile
> size (4kB) aligned. I noticed, that enforcing only this alignment leads
> oddly to random memory corruptions on TGL while scanning out Y-tiled
> FBs. This issue can be easily reproduced with a UV plane that is not
> aligned to the plane's tile row size.
> 
> Some experiments showed the correct alignment to be tile row size
> indeed. This also makes sense, since the de-tiling fence created for the

One would expect the implicit fence to follow the fence detiling HW
logic, which does not require tile-row alignment, just 4096 alignment
(since gen4). That suggests this logic is peculiar to the display engine.

> object - with its own stride and so "left" and "right" edge - applies to
> all the planes in the FB, so each tile row of all planes should be tile
> row aligned.
> 
> In fact BSpec requires this alignment since SKL. On SKL we may enforce
> this due to the AUX plane x,y coords check, but on ICL and TGL we don't.
> For now enforce this only on TGL; I can follow up with any necessary
> change for ICL after more tests.

Considering the severity of the error, I strongly suggest we fix all
suspected machines and Cc:stable.

> BSpec requires a stricter alignment for linear UV planes too (kind of a
> tile row alignment), but it's unclear whether that's really needed
> (couldn't be explained with the de-tiling fence as above) and enforcing
> that could break existing user space; so avoid that too for now until
> more tests.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> +static unsigned int intel_tile_row_size(const struct drm_framebuffer *fb,
> +                                       int color_plane)
> +{
> +       unsigned int tile_width, tile_height;
> +
> +       intel_tile_dims(fb, color_plane, &tile_width, &tile_height);
> +
> +       return fb->pitches[color_plane] * tile_height;

Ok, that is tile_row_size.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment
  2019-12-30 10:48 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Chris Wilson
@ 2019-12-30 18:39   ` Imre Deak
  0 siblings, 0 replies; 12+ messages in thread
From: Imre Deak @ 2019-12-30 18:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Mon, Dec 30, 2019 at 10:48:31AM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2019-12-27 23:51:45)
> > At least one framebuffer plane on TGL - the UV plane of YUV semiplanar
> > FBs - requires a non-power-of-2 alignment, so add support for this. This
> > new alignment restriction applies only to an offset within an FB, so the
> > GEM buffer itself containing the FB must still be power-of-2 aligned.
> 
> It's worth talking about virtual memory alignment (in the GGTT) here and
> not the physical alignment of the backing store. The buffer itself plays
> no part here.

Yes, this new restriction is about the GGTT mapping and display
specific. In fact other engines have other restrictions when
reading/writing the same YUV surfaces - for instance via a PPGTT map.
And yes, the page physical addresses can be anything.

Will improve the commit log.

> > Add a check for this (in practice plane 0, since the plane 0 offset must
> > be 0).
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 22 +++++++++++++-------
> >  1 file changed, 14 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 624ba9be7293..d8970198c77e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -2194,6 +2194,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> >                 return ERR_PTR(-EINVAL);
> >  
> >         alignment = intel_surf_alignment(fb, 0);
> > +       WARN_ON(!is_power_of_2(alignment));
> 
> Handle the error, if you are going to the trouble to warn, add the
> return as well.

Ok.

> >  
> >         /* Note that the w/a also requires 64 PTE of padding following the
> >          * bo. We currently fill all unused PTE with the shadow page and so
> > @@ -2432,9 +2433,6 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
> >         unsigned int cpp = fb->format->cpp[color_plane];
> >         u32 offset, offset_aligned;
> >  
> > -       if (alignment)
> > -               alignment--;
> > -
> >         if (!is_surface_linear(fb, color_plane)) {
> >                 unsigned int tile_size, tile_width, tile_height;
> >                 unsigned int tile_rows, tiles, pitch_tiles;
> > @@ -2456,17 +2454,24 @@ static u32 intel_compute_aligned_offset(struct drm_i915_private *dev_priv,
> >                 *x %= tile_width;
> >  
> >                 offset = (tile_rows * pitch_tiles + tiles) * tile_size;
> > -               offset_aligned = offset & ~alignment;
> > +
> > +               offset_aligned = offset;
> > +               if (alignment)
> > +                       offset_aligned = rounddown(offset_aligned, alignment);
> >  
> >                 intel_adjust_tile_offset(x, y, tile_width, tile_height,
> >                                          tile_size, pitch_tiles,
> >                                          offset, offset_aligned);
> >         } else {
> >                 offset = *y * pitch + *x * cpp;
> > -               offset_aligned = offset & ~alignment;
> > -
> > -               *y = (offset & alignment) / pitch;
> > -               *x = ((offset & alignment) - *y * pitch) / cpp;
> > +               offset_aligned = offset;
> > +               if (alignment) {
> > +                       offset_aligned = rounddown(offset_aligned, alignment);
> > +                       *y = (offset % alignment) / pitch;
> > +                       *x = ((offset % alignment) - *y * pitch) / cpp;
> > +               } else {
> > +                       *y = *x = 0;
> > +               }
> >         }
> >  
> >         return offset_aligned;
> > @@ -3738,6 +3743,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >         intel_add_fb_offsets(&x, &y, plane_state, 0);
> >         offset = intel_plane_compute_aligned_offset(&x, &y, plane_state, 0);
> >         alignment = intel_surf_alignment(fb, 0);
> > +       WARN_ON(!is_power_of_2(alignment));
> 
> The other two are expected to handle !is_pot...

Not sure what the alignment of the address we write to the main surface
address register should be, the spec doesn't say anything about that. I
assume now that not wrapping around the right edge of the detiler fence
we add is enough (which is also checked in intel_fill_fb_info()).

> I would strongly suggest handling the WARNs, or else you may as well bug
> out for the programming error.

Ok, will change those.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned
  2019-12-30 10:53   ` Chris Wilson
@ 2019-12-30 19:20     ` Imre Deak
  2019-12-30 20:43       ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Imre Deak @ 2019-12-30 19:20 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Mon, Dec 30, 2019 at 10:53:21AM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2019-12-27 23:51:46)
> > Currently the start address of a UV plane in a semiplanar YUV FB is tile
> > size (4kB) aligned. I noticed, that enforcing only this alignment leads
> > oddly to random memory corruptions on TGL while scanning out Y-tiled
> > FBs. This issue can be easily reproduced with a UV plane that is not
> > aligned to the plane's tile row size.
> > 
> > Some experiments showed the correct alignment to be tile row size
> > indeed. This also makes sense, since the de-tiling fence created for the
> 
> One would expect the implicit fence to follow the fence detiling HW
> logic, which does not require tile-row alignment, just 4096 alignment
> (since gen4). That suggests this logic is peculiar to the display engine.

Not sure what's an implicit fence, I only considered the fence we add
(which is added at offset 0 for the whole object via the set_tiling
IOCTL).  But I suppose you mean the display would use its own fence even
if we don't add one (since an explicit fence for display is not needed
for a while now). Not sure what's the exact reason for the spec's
tile-row alignment restriction for UV surface addresses, maybe it's
required both by an implicit or explicit fence. Note that I haven't seen
the memory corruption if I didn't add the explicit fence for the object,
but I haven't checked if there are other problems even then, once I
noticed the spec alignment requirement.

To me what's logical as a rule is to not wrap around the right edge of
the fence (implicit or explicit) by programming a display surface base
address and an x offset. That would also mean that the stride of FB
surfaces (for planar YUV surfaces all surface stride's being the same)
match the stride of an implicit or explicit fence. Not sure if we
enforce all these.

> > object - with its own stride and so "left" and "right" edge - applies to
> > all the planes in the FB, so each tile row of all planes should be tile
> > row aligned.
> > 
> > In fact BSpec requires this alignment since SKL. On SKL we may enforce
> > this due to the AUX plane x,y coords check, but on ICL and TGL we don't.
> > For now enforce this only on TGL; I can follow up with any necessary
> > change for ICL after more tests.
> 
> Considering the severity of the error, I strongly suggest we fix all
> suspected machines and Cc:stable.

Ok, will add Cc now to this one, but would follow up with other machines
after more empirical proofs.

> > BSpec requires a stricter alignment for linear UV planes too (kind of a
> > tile row alignment), but it's unclear whether that's really needed
> > (couldn't be explained with the de-tiling fence as above) and enforcing
> > that could break existing user space; so avoid that too for now until
> > more tests.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > +static unsigned int intel_tile_row_size(const struct drm_framebuffer *fb,
> > +                                       int color_plane)
> > +{
> > +       unsigned int tile_width, tile_height;
> > +
> > +       intel_tile_dims(fb, color_plane, &tile_width, &tile_height);
> > +
> > +       return fb->pitches[color_plane] * tile_height;
> 
> Ok, that is tile_row_size.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned
  2019-12-30 19:20     ` Imre Deak
@ 2019-12-30 20:43       ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-12-30 20:43 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

Quoting Imre Deak (2019-12-30 19:20:36)
> On Mon, Dec 30, 2019 at 10:53:21AM +0000, Chris Wilson wrote:
> > Quoting Imre Deak (2019-12-27 23:51:46)
> > > Currently the start address of a UV plane in a semiplanar YUV FB is tile
> > > size (4kB) aligned. I noticed, that enforcing only this alignment leads
> > > oddly to random memory corruptions on TGL while scanning out Y-tiled
> > > FBs. This issue can be easily reproduced with a UV plane that is not
> > > aligned to the plane's tile row size.
> > > 
> > > Some experiments showed the correct alignment to be tile row size
> > > indeed. This also makes sense, since the de-tiling fence created for the
> > 
> > One would expect the implicit fence to follow the fence detiling HW
> > logic, which does not require tile-row alignment, just 4096 alignment
> > (since gen4). That suggests this logic is peculiar to the display engine.
> 
> Not sure what's an implicit fence, I only considered the fence we add
> (which is added at offset 0 for the whole object via the set_tiling
> IOCTL).  But I suppose you mean the display would use its own fence even
> if we don't add one (since an explicit fence for display is not needed
> for a while now). Not sure what's the exact reason for the spec's
> tile-row alignment restriction for UV surface addresses, maybe it's
> required both by an implicit or explicit fence. Note that I haven't seen
> the memory corruption if I didn't add the explicit fence for the object,
> but I haven't checked if there are other problems even then, once I
> noticed the spec alignment requirement.
> 
> To me what's logical as a rule is to not wrap around the right edge of
> the fence (implicit or explicit) by programming a display surface base
> address and an x offset. That would also mean that the stride of FB
> surfaces (for planar YUV surfaces all surface stride's being the same)
> match the stride of an implicit or explicit fence. Not sure if we
> enforce all these.
> 
> > > object - with its own stride and so "left" and "right" edge - applies to
> > > all the planes in the FB, so each tile row of all planes should be tile
> > > row aligned.
> > > 
> > > In fact BSpec requires this alignment since SKL. On SKL we may enforce
> > > this due to the AUX plane x,y coords check, but on ICL and TGL we don't.
> > > For now enforce this only on TGL; I can follow up with any necessary
> > > change for ICL after more tests.
> > 
> > Considering the severity of the error, I strongly suggest we fix all
> > suspected machines and Cc:stable.
> 
> Ok, will add Cc now to this one, but would follow up with other machines
> after more empirical proofs.

Personally, I would do it in one patch. The downside of imposing a
stricter alignment is increased fragmentation (and perhaps the
occasional bit of rebinding), which should be of little concern.

If you can figure out the meaning of the rule, please do add it to the
changelog and comments,
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-12-30 20:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-27 23:51 [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Imre Deak
2019-12-27 23:51 ` [Intel-gfx] [PATCH 2/3] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned Imre Deak
2019-12-30 10:53   ` Chris Wilson
2019-12-30 19:20     ` Imre Deak
2019-12-30 20:43       ` Chris Wilson
2019-12-27 23:51 ` [Intel-gfx] [PATCH 3/3] drm/i915: Add debug message for FB plane[0].offset!=0 error Imre Deak
2019-12-28  0:32 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Patchwork
2019-12-28  0:47 ` [Intel-gfx] [PATCH v2 1/3] " Imre Deak
2019-12-28  1:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Add support for non-power-of-2 FB plane alignment (rev2) Patchwork
2019-12-28  2:41 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2019-12-30 10:48 ` [Intel-gfx] [PATCH 1/3] drm/i915: Add support for non-power-of-2 FB plane alignment Chris Wilson
2019-12-30 18:39   ` Imre Deak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.