[PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Imre Deak]

As described in the WA on GLK and CNL planes on the right edge of the
screen that have less than 4 pixels visible from the beginning of the
plane to the edge of the screen can cause FIFO underflow and display
corruption.

On GLK/CNL I could trigger the problem only if the plane was at the same
time also aligned to the top edge of the screen (after clipping) and
there were exactly 2 pixels visible from the start of the plane to the
right edge of the screen (so couldn't trigger it with 1 or 3 pixels
visible). Nevertheless, to be sure, I also applied the WA for these cases.

I also couldn't see any problem with the cursor plane and later Art
confirmed that it's not affected, so the WA is applied only for the
other plane types.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 221e3a183d36..3d931b652795 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2917,14 +2917,19 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
 	return true;
 }
 
-static int skl_check_main_surface(struct intel_plane_state *plane_state)
+static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
+				  struct intel_plane_state *plane_state)
 {
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
 	int w = drm_rect_width(&plane_state->base.src) >> 16;
 	int h = drm_rect_height(&plane_state->base.src) >> 16;
+	int dst_x = plane_state->base.dst.x1;
+	int pipe_src_w = crtc_state->pipe_src_w;
 	int max_width = skl_max_plane_width(fb, 0, rotation);
 	int max_height = 4096;
 	u32 alignment, offset, aux_offset = plane_state->aux.offset;
@@ -2935,6 +2940,20 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 		return -EINVAL;
 	}
 
+	/*
+	 * Display WA #1175: cnl,glk
+	 * Planes other than the cursor may cause FIFO underflow and display
+	 * corruption if starting less than 4 pixels from the right edge of
+	 * the screen.
+	 */
+	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
+	    dst_x > pipe_src_w - 4) {
+		DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
+			      dst_x,
+			      0, pipe_src_w - 4);
+		return -EINVAL;
+	}
+
 	intel_add_fb_offsets(&x, &y, plane_state, 0);
 	offset = intel_compute_tile_offset(&x, &y, plane_state, 0);
 	alignment = intel_surf_alignment(fb, 0);
@@ -3067,7 +3086,8 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
-int skl_check_plane_surface(struct intel_plane_state *plane_state)
+int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
+			    struct intel_plane_state *plane_state)
 {
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	unsigned int rotation = plane_state->base.rotation;
@@ -3107,7 +3127,7 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
 		plane_state->aux.y = 0;
 	}
 
-	ret = skl_check_main_surface(plane_state);
+	ret = skl_check_main_surface(crtc_state, plane_state);
 	if (ret)
 		return ret;
 
@@ -12766,7 +12786,7 @@ intel_check_primary_plane(struct intel_plane *plane,
 		return 0;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
-		ret = skl_check_plane_surface(state);
+		ret = skl_check_plane_surface(crtc_state, state);
 		if (ret)
 			return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 731dc36d7129..5e23da04837e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1588,7 +1588,8 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
 		  const struct intel_plane_state *plane_state);
 u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
 		     unsigned int rotation);
-int skl_check_plane_surface(struct intel_plane_state *plane_state);
+int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
+			    struct intel_plane_state *plane_state);
 int i9xx_check_plane_surface(struct intel_plane_state *plane_state);
 
 /* intel_csr.c */
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index dd485f59eb1d..b8b617ba0c0c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1027,7 +1027,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 	dst->y2 = crtc_y + crtc_h;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
-		ret = skl_check_plane_surface(state);
+		ret = skl_check_plane_surface(crtc_state, state);
 		if (ret)
 			return ret;
 
-- 
2.13.2

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

[PATCH 2/2] drm/i915: Add WA for planes ending close to left screen edge

[Imre Deak]

While running the kms_plane clipping test I noticed a similar problem to
the one described in Display WA #1175. In this case, similarly for
planes other than the cursor, with 1 or 3 pixels visible from the left
edge of the screen to the end of the plane and an odd plane X offset
used for clipping causes the same kind of underflow and display
corruption as described for WA #1175. Fix this in a similar way as that
WA rejecting planes ending <4 pixels from the left screen edge.

Testcase: igt/kms_plane/plane-clipping-pipe-*-planes
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3d931b652795..ab04ef471921 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2945,12 +2945,16 @@ static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
 	 * Planes other than the cursor may cause FIFO underflow and display
 	 * corruption if starting less than 4 pixels from the right edge of
 	 * the screen.
+	 * Besides the above WA fix the similar problem, where planes other
+	 * than the cursor ending less than 4 pixels from the left edge of the
+	 * screen may cause FIFO underflow and display corruption.
 	 */
 	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
-	    dst_x > pipe_src_w - 4) {
-		DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
-			      dst_x,
-			      0, pipe_src_w - 4);
+	    (dst_x + w < 4 || dst_x > pipe_src_w - 4)) {
+		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
+			      dst_x + w < 4 ? "end" : "start",
+			      dst_x + w < 4 ? dst_x + w : dst_x,
+			      4, pipe_src_w - 4);
 		return -EINVAL;
 	}
 
-- 
2.13.2

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

Re: [PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Chris Wilson]

Quoting Imre Deak (2018-01-12 14:54:36)
> As described in the WA on GLK and CNL planes on the right edge of the
> screen that have less than 4 pixels visible from the beginning of the
> plane to the edge of the screen can cause FIFO underflow and display
> corruption.
> 
> On GLK/CNL I could trigger the problem only if the plane was at the same
> time also aligned to the top edge of the screen (after clipping) and
> there were exactly 2 pixels visible from the start of the plane to the
> right edge of the screen (so couldn't trigger it with 1 or 3 pixels
> visible). Nevertheless, to be sure, I also applied the WA for these cases.
> 
> I also couldn't see any problem with the cursor plane and later Art
> confirmed that it's not affected, so the WA is applied only for the
> other plane types.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
>  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
>  3 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 221e3a183d36..3d931b652795 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2917,14 +2917,19 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
>         return true;
>  }
>  
> -static int skl_check_main_surface(struct intel_plane_state *plane_state)
> +static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
> +                                 struct intel_plane_state *plane_state)
>  {
> +       struct drm_i915_private *dev_priv =
> +               to_i915(plane_state->base.plane->dev);
>         const struct drm_framebuffer *fb = plane_state->base.fb;
>         unsigned int rotation = plane_state->base.rotation;
>         int x = plane_state->base.src.x1 >> 16;
>         int y = plane_state->base.src.y1 >> 16;
>         int w = drm_rect_width(&plane_state->base.src) >> 16;
>         int h = drm_rect_height(&plane_state->base.src) >> 16;
> +       int dst_x = plane_state->base.dst.x1;
> +       int pipe_src_w = crtc_state->pipe_src_w;
>         int max_width = skl_max_plane_width(fb, 0, rotation);
>         int max_height = 4096;
>         u32 alignment, offset, aux_offset = plane_state->aux.offset;
> @@ -2935,6 +2940,20 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>                 return -EINVAL;
>         }
>  
> +       /*
> +        * Display WA #1175: cnl,glk
> +        * Planes other than the cursor may cause FIFO underflow and display
> +        * corruption if starting less than 4 pixels from the right edge of
> +        * the screen.
> +        */
> +       if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +           dst_x > pipe_src_w - 4) {
> +               DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
> +                             dst_x,
> +                             0, pipe_src_w - 4);

You are rejecting user input, so this should be DRM_DEBUG() (or whatever
the future user channel will be).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Chris Wilson]

Quoting Imre Deak (2018-01-12 14:54:36)
> As described in the WA on GLK and CNL planes on the right edge of the
> screen that have less than 4 pixels visible from the beginning of the
> plane to the edge of the screen can cause FIFO underflow and display
> corruption.
> 
> On GLK/CNL I could trigger the problem only if the plane was at the same
> time also aligned to the top edge of the screen (after clipping) and
> there were exactly 2 pixels visible from the start of the plane to the
> right edge of the screen (so couldn't trigger it with 1 or 3 pixels
> visible). Nevertheless, to be sure, I also applied the WA for these cases.
> 
> I also couldn't see any problem with the cursor plane and later Art
> confirmed that it's not affected, so the WA is applied only for the
> other plane types.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> +       /*
> +        * Display WA #1175: cnl,glk
> +        * Planes other than the cursor may cause FIFO underflow and display
> +        * corruption if starting less than 4 pixels from the right edge of
> +        * the screen.
> +        */
> +       if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +           dst_x > pipe_src_w - 4) {
> +               DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
> +                             dst_x,
> +                             0, pipe_src_w - 4);
> +               return -EINVAL;

Should this be -ERANGE or -ENOSPC?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge
URL   : https://patchwork.freedesktop.org/series/36408/
State : success

== Summary ==

Series 36408v1 series starting with [1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge
https://patchwork.freedesktop.org/api/1.0/series/36408/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:425s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:427s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:491s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:279s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:483s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:480s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:462s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:275s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:510s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:394s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:401s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:412s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:453s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:412s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:465s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:500s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:501s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:583s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:437s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:532s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:496s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:492s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:524s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:396s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:575s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:477s

353fa2d3affffef324005ed2553da6eb174a2f0b drm-tip: 2018y-01m-12d-09h-21m-50s UTC integration manifest
0459db82b73b drm/i915: Add WA for planes ending close to left screen edge
cb1f35d93de0 drm/i915: Add display WA #1175 for planes ending close to right screen edge

== Logs ==

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

✗ Fi.CI.IGT: warning for series starting with [1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge
URL   : https://patchwork.freedesktop.org/series/36408/
State : warning

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623 +1
        Subgroup fbc-1p-primscrn-pri-indfb-draw-pwrite:
                fail       -> PASS       (shard-snb) fdo#103167
        Subgroup fbc-rgb101010-draw-mmap-gtt:
                pass       -> SKIP       (shard-snb)
Test gem_eio:
        Subgroup in-flight-contexts:
                dmesg-warn -> PASS       (shard-snb) fdo#104058 +1
Test gem_wait:
        Subgroup write-busy-bsd:
                skip       -> PASS       (shard-snb)
Test gem_pwrite_snooped:
                fail       -> PASS       (shard-snb) fdo#104600
Test perf_pmu:
        Subgroup busy-check-all-rcs0:
                skip       -> PASS       (shard-snb)
Test gem_exec_parallel:
        Subgroup default-fds:
                skip       -> PASS       (shard-snb)
Test gem_partial_pwrite_pread:
        Subgroup write:
                skip       -> PASS       (shard-snb)
Test prime_vgem:
        Subgroup fence-wait-blt:
                skip       -> PASS       (shard-snb)
Test kms_flip:
        Subgroup blt-wf_vblank-vs-modeset-interruptible:
                skip       -> PASS       (shard-snb) fdo#104218 +1
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                pass       -> INCOMPLETE (shard-hsw) fdo#104152
Test kms_rotation_crc:
        Subgroup cursor-rotation-180:
                pass       -> SKIP       (shard-snb)
Test kms_cursor_crc:
        Subgroup cursor-128x128-sliding:
                pass       -> SKIP       (shard-snb)

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#104600 https://bugs.freedesktop.org/show_bug.cgi?id=104600
fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218
fdo#104152 https://bugs.freedesktop.org/show_bug.cgi?id=104152

shard-hsw        total:2641 pass:1500 dwarn:1   dfail:0   fail:10  skip:1129 time:8880s
shard-snb        total:2713 pass:1306 dwarn:1   dfail:0   fail:12  skip:1394 time:7895s
Blacklisted hosts:
shard-apl        total:2637 pass:1640 dwarn:1   dfail:1   fail:24  skip:971 time:12916s
shard-kbl        total:2713 pass:1808 dwarn:1   dfail:0   fail:26  skip:878 time:10504s

== Logs ==

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

Re: [PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Imre Deak]

On Fri, Jan 12, 2018 at 03:01:59PM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2018-01-12 14:54:36)
> > As described in the WA on GLK and CNL planes on the right edge of the
> > screen that have less than 4 pixels visible from the beginning of the
> > plane to the edge of the screen can cause FIFO underflow and display
> > corruption.
> > 
> > On GLK/CNL I could trigger the problem only if the plane was at the same
> > time also aligned to the top edge of the screen (after clipping) and
> > there were exactly 2 pixels visible from the start of the plane to the
> > right edge of the screen (so couldn't trigger it with 1 or 3 pixels
> > visible). Nevertheless, to be sure, I also applied the WA for these cases.
> > 
> > I also couldn't see any problem with the cursor plane and later Art
> > confirmed that it's not affected, so the WA is applied only for the
> > other plane types.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++----
> >  drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
> >  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
> >  3 files changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 221e3a183d36..3d931b652795 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2917,14 +2917,19 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
> >         return true;
> >  }
> >  
> > -static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > +static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
> > +                                 struct intel_plane_state *plane_state)
> >  {
> > +       struct drm_i915_private *dev_priv =
> > +               to_i915(plane_state->base.plane->dev);
> >         const struct drm_framebuffer *fb = plane_state->base.fb;
> >         unsigned int rotation = plane_state->base.rotation;
> >         int x = plane_state->base.src.x1 >> 16;
> >         int y = plane_state->base.src.y1 >> 16;
> >         int w = drm_rect_width(&plane_state->base.src) >> 16;
> >         int h = drm_rect_height(&plane_state->base.src) >> 16;
> > +       int dst_x = plane_state->base.dst.x1;
> > +       int pipe_src_w = crtc_state->pipe_src_w;
> >         int max_width = skl_max_plane_width(fb, 0, rotation);
> >         int max_height = 4096;
> >         u32 alignment, offset, aux_offset = plane_state->aux.offset;
> > @@ -2935,6 +2940,20 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> >                 return -EINVAL;
> >         }
> >  
> > +       /*
> > +        * Display WA #1175: cnl,glk
> > +        * Planes other than the cursor may cause FIFO underflow and display
> > +        * corruption if starting less than 4 pixels from the right edge of
> > +        * the screen.
> > +        */
> > +       if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > +           dst_x > pipe_src_w - 4) {
> > +               DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
> > +                             dst_x,
> > +                             0, pipe_src_w - 4);
> 
> You are rejecting user input, so this should be DRM_DEBUG() (or whatever
> the future user channel will be).

What's the rational for a user channel? Not having to build the rest of
debugging stuff, or a simpler user interface?

The rest of places failing due to user input on this path also use
DRM_DEBUG_KMS(), so we could also convert all as a follow-up.

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

Re: [PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Imre Deak]

On Fri, Jan 12, 2018 at 03:13:19PM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2018-01-12 14:54:36)
> > As described in the WA on GLK and CNL planes on the right edge of the
> > screen that have less than 4 pixels visible from the beginning of the
> > plane to the edge of the screen can cause FIFO underflow and display
> > corruption.
> > 
> > On GLK/CNL I could trigger the problem only if the plane was at the same
> > time also aligned to the top edge of the screen (after clipping) and
> > there were exactly 2 pixels visible from the start of the plane to the
> > right edge of the screen (so couldn't trigger it with 1 or 3 pixels
> > visible). Nevertheless, to be sure, I also applied the WA for these cases.
> > 
> > I also couldn't see any problem with the cursor plane and later Art
> > confirmed that it's not affected, so the WA is applied only for the
> > other plane types.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > +       /*
> > +        * Display WA #1175: cnl,glk
> > +        * Planes other than the cursor may cause FIFO underflow and display
> > +        * corruption if starting less than 4 pixels from the right edge of
> > +        * the screen.
> > +        */
> > +       if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > +           dst_x > pipe_src_w - 4) {
> > +               DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
> > +                             dst_x,
> > +                             0, pipe_src_w - 4);
> > +               return -EINVAL;
> 
> Should this be -ERANGE or -ENOSPC?

I don't mind changing it to -ERANGE. (I suppose it'd be used by user
space as detection of platforms with the problem, falling back to
something else.)

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

Re: [PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Chris Wilson]

Quoting Imre Deak (2018-01-15 13:20:37)
> On Fri, Jan 12, 2018 at 03:01:59PM +0000, Chris Wilson wrote:
> > Quoting Imre Deak (2018-01-12 14:54:36)
> > > As described in the WA on GLK and CNL planes on the right edge of the
> > > screen that have less than 4 pixels visible from the beginning of the
> > > plane to the edge of the screen can cause FIFO underflow and display
> > > corruption.
> > > 
> > > On GLK/CNL I could trigger the problem only if the plane was at the same
> > > time also aligned to the top edge of the screen (after clipping) and
> > > there were exactly 2 pixels visible from the start of the plane to the
> > > right edge of the screen (so couldn't trigger it with 1 or 3 pixels
> > > visible). Nevertheless, to be sure, I also applied the WA for these cases.
> > > 
> > > I also couldn't see any problem with the cursor plane and later Art
> > > confirmed that it's not affected, so the WA is applied only for the
> > > other plane types.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++----
> > >  drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
> > >  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
> > >  3 files changed, 27 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 221e3a183d36..3d931b652795 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2917,14 +2917,19 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
> > >         return true;
> > >  }
> > >  
> > > -static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > > +static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
> > > +                                 struct intel_plane_state *plane_state)
> > >  {
> > > +       struct drm_i915_private *dev_priv =
> > > +               to_i915(plane_state->base.plane->dev);
> > >         const struct drm_framebuffer *fb = plane_state->base.fb;
> > >         unsigned int rotation = plane_state->base.rotation;
> > >         int x = plane_state->base.src.x1 >> 16;
> > >         int y = plane_state->base.src.y1 >> 16;
> > >         int w = drm_rect_width(&plane_state->base.src) >> 16;
> > >         int h = drm_rect_height(&plane_state->base.src) >> 16;
> > > +       int dst_x = plane_state->base.dst.x1;
> > > +       int pipe_src_w = crtc_state->pipe_src_w;
> > >         int max_width = skl_max_plane_width(fb, 0, rotation);
> > >         int max_height = 4096;
> > >         u32 alignment, offset, aux_offset = plane_state->aux.offset;
> > > @@ -2935,6 +2940,20 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > >                 return -EINVAL;
> > >         }
> > >  
> > > +       /*
> > > +        * Display WA #1175: cnl,glk
> > > +        * Planes other than the cursor may cause FIFO underflow and display
> > > +        * corruption if starting less than 4 pixels from the right edge of
> > > +        * the screen.
> > > +        */
> > > +       if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > +           dst_x > pipe_src_w - 4) {
> > > +               DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
> > > +                             dst_x,
> > > +                             0, pipe_src_w - 4);
> > 
> > You are rejecting user input, so this should be DRM_DEBUG() (or whatever
> > the future user channel will be).
> 
> What's the rational for a user channel? Not having to build the rest of
> debugging stuff, or a simpler user interface?

So that the right information goes to the right user. The only person
who should be able to see such error messages is the person making the
mistake (we're leaking information about the caller into a general
purpose message log). Plus we really want some other means for getting
accurate error messages about mistakes in using the ABI back to the user
without having to use dmesg.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Add display WA #1175 for planes ending close to right screen edge

[Imre Deak]

On Mon, Jan 15, 2018 at 01:26:48PM +0000, Chris Wilson wrote:
> Quoting Imre Deak (2018-01-15 13:20:37)
> > On Fri, Jan 12, 2018 at 03:01:59PM +0000, Chris Wilson wrote:
> > > Quoting Imre Deak (2018-01-12 14:54:36)
> > > > As described in the WA on GLK and CNL planes on the right edge of the
> > > > screen that have less than 4 pixels visible from the beginning of the
> > > > plane to the edge of the screen can cause FIFO underflow and display
> > > > corruption.
> > > > 
> > > > On GLK/CNL I could trigger the problem only if the plane was at the same
> > > > time also aligned to the top edge of the screen (after clipping) and
> > > > there were exactly 2 pixels visible from the start of the plane to the
> > > > right edge of the screen (so couldn't trigger it with 1 or 3 pixels
> > > > visible). Nevertheless, to be sure, I also applied the WA for these cases.
> > > > 
> > > > I also couldn't see any problem with the cursor plane and later Art
> > > > confirmed that it's not affected, so the WA is applied only for the
> > > > other plane types.
> > > > 
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++----
> > > >  drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
> > > >  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
> > > >  3 files changed, 27 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 221e3a183d36..3d931b652795 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -2917,14 +2917,19 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
> > > >         return true;
> > > >  }
> > > >  
> > > > -static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > > > +static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
> > > > +                                 struct intel_plane_state *plane_state)
> > > >  {
> > > > +       struct drm_i915_private *dev_priv =
> > > > +               to_i915(plane_state->base.plane->dev);
> > > >         const struct drm_framebuffer *fb = plane_state->base.fb;
> > > >         unsigned int rotation = plane_state->base.rotation;
> > > >         int x = plane_state->base.src.x1 >> 16;
> > > >         int y = plane_state->base.src.y1 >> 16;
> > > >         int w = drm_rect_width(&plane_state->base.src) >> 16;
> > > >         int h = drm_rect_height(&plane_state->base.src) >> 16;
> > > > +       int dst_x = plane_state->base.dst.x1;
> > > > +       int pipe_src_w = crtc_state->pipe_src_w;
> > > >         int max_width = skl_max_plane_width(fb, 0, rotation);
> > > >         int max_height = 4096;
> > > >         u32 alignment, offset, aux_offset = plane_state->aux.offset;
> > > > @@ -2935,6 +2940,20 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> > > >                 return -EINVAL;
> > > >         }
> > > >  
> > > > +       /*
> > > > +        * Display WA #1175: cnl,glk
> > > > +        * Planes other than the cursor may cause FIFO underflow and display
> > > > +        * corruption if starting less than 4 pixels from the right edge of
> > > > +        * the screen.
> > > > +        */
> > > > +       if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> > > > +           dst_x > pipe_src_w - 4) {
> > > > +               DRM_DEBUG_KMS("requested plane X start position %d invalid (valid range %d-%d)\n",
> > > > +                             dst_x,
> > > > +                             0, pipe_src_w - 4);
> > > 
> > > You are rejecting user input, so this should be DRM_DEBUG() (or whatever
> > > the future user channel will be).
> > 
> > What's the rational for a user channel? Not having to build the rest of
> > debugging stuff, or a simpler user interface?
> 
> So that the right information goes to the right user. The only person
> who should be able to see such error messages is the person making the
> mistake (we're leaking information about the caller into a general
> purpose message log). Plus we really want some other means for getting
> accurate error messages about mistakes in using the ABI back to the user
> without having to use dmesg.

Ok, sounds reasonable, but I think the conversion should be done for all
places as a follow-up. At least I can't see how changing the above to
DRM_DEBUG alone would help now, as I understand you'd need some per-process
log buffer in addition.

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