All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm: Fix example comment of format modifier blob
@ 2017-08-31 19:52 Gabriel Krisman Bertazi
  2017-08-31 19:52 ` [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap Gabriel Krisman Bertazi
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Gabriel Krisman Bertazi @ 2017-08-31 19:52 UTC (permalink / raw)
  To: intel-gfx; +Cc: ben

To represent formats 98-102, the supported formats mask must be
0x7c00000000 and not 0x3c00000000.

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
---
 include/uapi/drm/drm_mode.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 54fc38c3c3f1..34b6bb34b002 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -749,9 +749,9 @@ struct drm_format_modifier {
 	 * If the number formats grew to 128, and formats 98-102 are
 	 * supported with the modifier:
 	 *
-	 * 0x0000003c00000000 0000000000000000
+	 * 0x0000007c00000000 0000000000000000
 	 *		  ^
-	 *		  |__offset = 64, formats = 0x3c00000000
+	 *		  |__offset = 64, formats = 0x7c00000000
 	 *
 	 */
 	__u64 formats;
-- 
2.11.0

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

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

* [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-08-31 19:52 [PATCH 1/2] drm: Fix example comment of format modifier blob Gabriel Krisman Bertazi
@ 2017-08-31 19:52 ` Gabriel Krisman Bertazi
  2017-08-31 20:06   ` Ben Widawsky
                     ` (2 more replies)
  2017-08-31 20:00 ` [PATCH 1/2] drm: Fix example comment of format modifier blob Ben Widawsky
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 14+ messages in thread
From: Gabriel Krisman Bertazi @ 2017-08-31 19:52 UTC (permalink / raw)
  To: intel-gfx; +Cc: ben

With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
succeeds.

Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b28f076f98bc..ff1ed67a9eff 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13989,6 +13989,11 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 			DRM_DEBUG_KMS("RC supported only with RGB8888 formats\n");
 			goto err;
 		}
+
+		if (mode_cmd->offsets[1] < mode_cmd->pitches[0]) {
+			DRM_DEBUG_KMS("CCS and color buffers overlap\n");
+			return -EINVAL;
+		}
 		/* fall through */
 	case I915_FORMAT_MOD_Y_TILED:
 	case I915_FORMAT_MOD_Yf_TILED:
-- 
2.11.0

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

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

* Re: [PATCH 1/2] drm: Fix example comment of format modifier blob
  2017-08-31 19:52 [PATCH 1/2] drm: Fix example comment of format modifier blob Gabriel Krisman Bertazi
  2017-08-31 19:52 ` [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap Gabriel Krisman Bertazi
@ 2017-08-31 20:00 ` Ben Widawsky
  2017-09-04  8:24   ` Daniel Vetter
  2017-08-31 20:22 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
  2017-08-31 23:37 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2017-08-31 20:00 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: intel-gfx

On 17-08-31 16:52:14, Gabriel Krisman Bertazi wrote:
>To represent formats 98-102, the supported formats mask must be
>0x7c00000000 and not 0x3c00000000.
>
>Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
>---
> include/uapi/drm/drm_mode.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>index 54fc38c3c3f1..34b6bb34b002 100644
>--- a/include/uapi/drm/drm_mode.h
>+++ b/include/uapi/drm/drm_mode.h
>@@ -749,9 +749,9 @@ struct drm_format_modifier {
> 	 * If the number formats grew to 128, and formats 98-102 are
> 	 * supported with the modifier:
> 	 *
>-	 * 0x0000003c00000000 0000000000000000
>+	 * 0x0000007c00000000 0000000000000000
> 	 *		  ^
>-	 *		  |__offset = 64, formats = 0x3c00000000
>+	 *		  |__offset = 64, formats = 0x7c00000000
> 	 *
> 	 */
> 	__u64 formats;
>-- 

>>> hex(0x1f << 98)
'0x7c000000000000000000000000'
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-08-31 19:52 ` [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap Gabriel Krisman Bertazi
@ 2017-08-31 20:06   ` Ben Widawsky
  2017-09-01  4:35     ` Gabriel Krisman Bertazi
  2017-09-04  8:21   ` Daniel Vetter
  2017-09-04 16:37   ` Ville Syrjälä
  2 siblings, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2017-08-31 20:06 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: intel-gfx

On 17-08-31 16:52:15, Gabriel Krisman Bertazi wrote:
>With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
>succeeds.
>
>Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
>---
> drivers/gpu/drm/i915/intel_display.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index b28f076f98bc..ff1ed67a9eff 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -13989,6 +13989,11 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> 			DRM_DEBUG_KMS("RC supported only with RGB8888 formats\n");
> 			goto err;
> 		}
>+
>+		if (mode_cmd->offsets[1] < mode_cmd->pitches[0]) {
>+			DRM_DEBUG_KMS("CCS and color buffers overlap\n");
>+			return -EINVAL;
>+		}

This check doesn't look nearly strict enough to determine overlap. All it's
checking is that the aux buffer isn't in the first row of the main buffer.
Second of all, while today our requirement is that the aux buffer always come
after the main buffer, that may not be the case forever.

> 		/* fall through */
> 	case I915_FORMAT_MOD_Y_TILED:
> 	case I915_FORMAT_MOD_Yf_TILED:
>-- 
>2.11.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm: Fix example comment of format modifier blob
  2017-08-31 19:52 [PATCH 1/2] drm: Fix example comment of format modifier blob Gabriel Krisman Bertazi
  2017-08-31 19:52 ` [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap Gabriel Krisman Bertazi
  2017-08-31 20:00 ` [PATCH 1/2] drm: Fix example comment of format modifier blob Ben Widawsky
@ 2017-08-31 20:22 ` Patchwork
  2017-08-31 23:37 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-08-31 20:22 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm: Fix example comment of format modifier blob
URL   : https://patchwork.freedesktop.org/series/29627/
State : success

== Summary ==

Series 29627v1 series starting with [1/2] drm: Fix example comment of format modifier blob
https://patchwork.freedesktop.org/api/1.0/series/29627/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
        Subgroup basic-flip-after-cursor-varying-size:
                fail       -> PASS       (fi-hsw-4770) fdo#102402 +1

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

fi-bdw-5557u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:455s
fi-bdw-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:448s
fi-blb-e6850     total:288  pass:224  dwarn:1   dfail:0   fail:0   skip:63  time:357s
fi-bsw-n3050     total:288  pass:243  dwarn:0   dfail:0   fail:0   skip:45  time:561s
fi-bwr-2160      total:288  pass:184  dwarn:0   dfail:0   fail:0   skip:104 time:253s
fi-bxt-j4205     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:523s
fi-byt-j1900     total:288  pass:254  dwarn:1   dfail:0   fail:0   skip:33  time:522s
fi-byt-n2820     total:288  pass:250  dwarn:1   dfail:0   fail:0   skip:37  time:521s
fi-elk-e7500     total:288  pass:230  dwarn:0   dfail:0   fail:0   skip:58  time:440s
fi-glk-2a        total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:610s
fi-hsw-4770      total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:460s
fi-hsw-4770r     total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:426s
fi-ilk-650       total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:424s
fi-ivb-3520m     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:499s
fi-ivb-3770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:477s
fi-kbl-7500u     total:288  pass:264  dwarn:1   dfail:0   fail:0   skip:23  time:512s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:601s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:607s
fi-pnv-d510      total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:529s
fi-skl-6260u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:473s
fi-skl-6700k     total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:536s
fi-skl-6770hq    total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:489s
fi-skl-gvtdvm    total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:22  time:444s
fi-skl-x1585l    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:495s
fi-snb-2520m     total:288  pass:251  dwarn:0   dfail:0   fail:0   skip:37  time:551s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:2   skip:38  time:404s

ccf4ca2d93383fe1a234aba83df9c21400216433 drm-tip: 2017y-08m-31d-18h-37m-46s UTC integration manifest
ead24675c00f drm/i915: Fail addfb ioctl if color and CCS buffers overlap
2371d5210288 drm: Fix example comment of format modifier blob

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for series starting with [1/2] drm: Fix example comment of format modifier blob
  2017-08-31 19:52 [PATCH 1/2] drm: Fix example comment of format modifier blob Gabriel Krisman Bertazi
                   ` (2 preceding siblings ...)
  2017-08-31 20:22 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2017-08-31 23:37 ` Patchwork
  3 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-08-31 23:37 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm: Fix example comment of format modifier blob
URL   : https://patchwork.freedesktop.org/series/29627/
State : success

== Summary ==

shard-hsw        total:2265 pass:1232 dwarn:0   dfail:0   fail:17  skip:1016 time:9651s

== Logs ==

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

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-08-31 20:06   ` Ben Widawsky
@ 2017-09-01  4:35     ` Gabriel Krisman Bertazi
  0 siblings, 0 replies; 14+ messages in thread
From: Gabriel Krisman Bertazi @ 2017-09-01  4:35 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

Ben Widawsky <ben@bwidawsk.net> writes:

> On 17-08-31 16:52:15, Gabriel Krisman Bertazi wrote:
>>With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
>>succeeds.
>>
>>Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
>>---
>> drivers/gpu/drm/i915/intel_display.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>index b28f076f98bc..ff1ed67a9eff 100644
>>--- a/drivers/gpu/drm/i915/intel_display.c
>>+++ b/drivers/gpu/drm/i915/intel_display.c
>>@@ -13989,6 +13989,11 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>> 			DRM_DEBUG_KMS("RC supported only with RGB8888 formats\n");
>> 			goto err;
>> 		}
>>+
>>+		if (mode_cmd->offsets[1] < mode_cmd->pitches[0]) {
>>+			DRM_DEBUG_KMS("CCS and color buffers overlap\n");
>>+			return -EINVAL;
>>+		}
>
> This check doesn't look nearly strict enough to determine overlap. All it's
> checking is that the aux buffer isn't in the first row of the main buffer.
> Second of all, while today our requirement is that the aux buffer always come
> after the main buffer, that may not be the case forever.

Right.  Thanks for catching it.  I'll re-spin with a more strict (and
correct) test for overlapping.

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

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-08-31 19:52 ` [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap Gabriel Krisman Bertazi
  2017-08-31 20:06   ` Ben Widawsky
@ 2017-09-04  8:21   ` Daniel Vetter
  2017-09-04 16:37   ` Ville Syrjälä
  2 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2017-09-04  8:21 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: intel-gfx, ben

On Thu, Aug 31, 2017 at 04:52:15PM -0300, Gabriel Krisman Bertazi wrote:
> With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
> succeeds.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>

Do we have igts for this? If so, please add a Testcase: line.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b28f076f98bc..ff1ed67a9eff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13989,6 +13989,11 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>  			DRM_DEBUG_KMS("RC supported only with RGB8888 formats\n");
>  			goto err;
>  		}
> +
> +		if (mode_cmd->offsets[1] < mode_cmd->pitches[0]) {
> +			DRM_DEBUG_KMS("CCS and color buffers overlap\n");
> +			return -EINVAL;
> +		}
>  		/* fall through */
>  	case I915_FORMAT_MOD_Y_TILED:
>  	case I915_FORMAT_MOD_Yf_TILED:
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH 1/2] drm: Fix example comment of format modifier blob
  2017-08-31 20:00 ` [PATCH 1/2] drm: Fix example comment of format modifier blob Ben Widawsky
@ 2017-09-04  8:24   ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2017-09-04  8:24 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

On Thu, Aug 31, 2017 at 01:00:35PM -0700, Ben Widawsky wrote:
> On 17-08-31 16:52:14, Gabriel Krisman Bertazi wrote:
> > To represent formats 98-102, the supported formats mask must be
> > 0x7c00000000 and not 0x3c00000000.
> > 
> > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
> > ---
> > include/uapi/drm/drm_mode.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 54fc38c3c3f1..34b6bb34b002 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -749,9 +749,9 @@ struct drm_format_modifier {
> > 	 * If the number formats grew to 128, and formats 98-102 are
> > 	 * supported with the modifier:
> > 	 *
> > -	 * 0x0000003c00000000 0000000000000000
> > +	 * 0x0000007c00000000 0000000000000000
> > 	 *		  ^
> > -	 *		  |__offset = 64, formats = 0x3c00000000
> > +	 *		  |__offset = 64, formats = 0x7c00000000
> > 	 *
> > 	 */
> > 	__u64 formats;
> > -- 
> 
> > > > hex(0x1f << 98)
> '0x7c000000000000000000000000'
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

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

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-08-31 19:52 ` [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap Gabriel Krisman Bertazi
  2017-08-31 20:06   ` Ben Widawsky
  2017-09-04  8:21   ` Daniel Vetter
@ 2017-09-04 16:37   ` Ville Syrjälä
  2017-09-05  7:36     ` Daniel Stone
  2 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2017-09-04 16:37 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: intel-gfx, ben

On Thu, Aug 31, 2017 at 04:52:15PM -0300, Gabriel Krisman Bertazi wrote:
> With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
> succeeds.

I don't think we actually want to reject overlap. I had a patch for that
years ago, but I decided to drop it because people might want to
interleave the planes in some interesting ways. Making the overlap
check accurate enough to allow that would be to total overkill. So IMO
it's perfectly fine to let the user shoot himself in the foot if they
mess up the offsets.

> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b28f076f98bc..ff1ed67a9eff 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13989,6 +13989,11 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>  			DRM_DEBUG_KMS("RC supported only with RGB8888 formats\n");
>  			goto err;
>  		}
> +
> +		if (mode_cmd->offsets[1] < mode_cmd->pitches[0]) {
> +			DRM_DEBUG_KMS("CCS and color buffers overlap\n");
> +			return -EINVAL;
> +		}
>  		/* fall through */
>  	case I915_FORMAT_MOD_Y_TILED:
>  	case I915_FORMAT_MOD_Yf_TILED:
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-09-04 16:37   ` Ville Syrjälä
@ 2017-09-05  7:36     ` Daniel Stone
  2017-09-07 12:19       ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Stone @ 2017-09-05  7:36 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Ben Widawsky

Hi Ville,

On 4 September 2017 at 17:37, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Aug 31, 2017 at 04:52:15PM -0300, Gabriel Krisman Bertazi wrote:
>> With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
>> succeeds.
>
> I don't think we actually want to reject overlap. I had a patch for that
> years ago, but I decided to drop it because people might want to
> interleave the planes in some interesting ways. Making the overlap
> check accurate enough to allow that would be to total overkill. So IMO
> it's perfectly fine to let the user shoot himself in the foot if they
> mess up the offsets.

Is that actually supported by any hardware renderer? If not, maybe the
check should only be enabled for generations who support it.

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

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-09-05  7:36     ` Daniel Stone
@ 2017-09-07 12:19       ` Ville Syrjälä
  2017-09-22  5:25         ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2017-09-07 12:19 UTC (permalink / raw)
  To: Daniel Stone; +Cc: intel-gfx, Ben Widawsky

On Tue, Sep 05, 2017 at 08:36:54AM +0100, Daniel Stone wrote:
> Hi Ville,
> 
> On 4 September 2017 at 17:37, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Aug 31, 2017 at 04:52:15PM -0300, Gabriel Krisman Bertazi wrote:
> >> With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
> >> succeeds.
> >
> > I don't think we actually want to reject overlap. I had a patch for that
> > years ago, but I decided to drop it because people might want to
> > interleave the planes in some interesting ways. Making the overlap
> > check accurate enough to allow that would be to total overkill. So IMO
> > it's perfectly fine to let the user shoot himself in the foot if they
> > mess up the offsets.
> 
> Is that actually supported by any hardware renderer? If not, maybe the
> check should only be enabled for generations who support it.

Not sure I understand the question. You can program your offsets/strides
any which way you want,

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

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-09-07 12:19       ` Ville Syrjälä
@ 2017-09-22  5:25         ` Gabriel Krisman Bertazi
  2017-09-22 11:54           ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Gabriel Krisman Bertazi @ 2017-09-22  5:25 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Ben Widawsky

Ville Syrjälä <ville.syrjala@linux.intel.com> writes:

> On Tue, Sep 05, 2017 at 08:36:54AM +0100, Daniel Stone wrote:
>> Hi Ville,
>> 
>> On 4 September 2017 at 17:37, Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>> > On Thu, Aug 31, 2017 at 04:52:15PM -0300, Gabriel Krisman Bertazi wrote:
>> >> With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
>> >> succeeds.
>> >
>> > I don't think we actually want to reject overlap. I had a patch for that
>> > years ago, but I decided to drop it because people might want to
>> > interleave the planes in some interesting ways. Making the overlap
>> > check accurate enough to allow that would be to total overkill. So IMO
>> > it's perfectly fine to let the user shoot himself in the foot if they
>> > mess up the offsets.
>> 
>> Is that actually supported by any hardware renderer? If not, maybe the
>> check should only be enabled for generations who support it.
>
> Not sure I understand the question. You can program your offsets/strides
> any which way you want,

Hi Ville,

Sorry for the delay.  Reviewing documentation at [1] I see the
following, in page 177, regarding CCS structure:
"The CCS is always placed after the main surface and is 4K page
aligned".  Sorry if I am missing something, but I believe this indicates
we can reject any overlapping between the main buffer and CCS. Can you
explain why not?

[1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12-display.pdf

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

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

* Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
  2017-09-22  5:25         ` Gabriel Krisman Bertazi
@ 2017-09-22 11:54           ` Ville Syrjälä
  0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2017-09-22 11:54 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: intel-gfx, Ben Widawsky

On Fri, Sep 22, 2017 at 02:25:19AM -0300, Gabriel Krisman Bertazi wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> 
> > On Tue, Sep 05, 2017 at 08:36:54AM +0100, Daniel Stone wrote:
> >> Hi Ville,
> >> 
> >> On 4 September 2017 at 17:37, Ville Syrjälä
> >> <ville.syrjala@linux.intel.com> wrote:
> >> > On Thu, Aug 31, 2017 at 04:52:15PM -0300, Gabriel Krisman Bertazi wrote:
> >> >> With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
> >> >> succeeds.
> >> >
> >> > I don't think we actually want to reject overlap. I had a patch for that
> >> > years ago, but I decided to drop it because people might want to
> >> > interleave the planes in some interesting ways. Making the overlap
> >> > check accurate enough to allow that would be to total overkill. So IMO
> >> > it's perfectly fine to let the user shoot himself in the foot if they
> >> > mess up the offsets.
> >> 
> >> Is that actually supported by any hardware renderer? If not, maybe the
> >> check should only be enabled for generations who support it.
> >
> > Not sure I understand the question. You can program your offsets/strides
> > any which way you want,
> 
> Hi Ville,
> 
> Sorry for the delay.  Reviewing documentation at [1] I see the
> following, in page 177, regarding CCS structure:
> "The CCS is always placed after the main surface and is 4K page
> aligned".

The hardware doesn't really have such a requirement. The only thing it
cares about is that AUX_DIST must be positive which means the AUX
surface always starts >= plane SURF. That doesn't imply that the two
can't potentially overlap, and actually doesn't even imply that we
couldn't place the aux surface below the main surface (we can do that as
long as we are able to adjust the plane SURF and AUX_DIST appropriately).

But the main point is that the user can still ess up the AUX plane
offset in other ways, so trying to protect them against this one way
of messing up is rather pointless.

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

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

end of thread, other threads:[~2017-09-22 11:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-31 19:52 [PATCH 1/2] drm: Fix example comment of format modifier blob Gabriel Krisman Bertazi
2017-08-31 19:52 ` [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap Gabriel Krisman Bertazi
2017-08-31 20:06   ` Ben Widawsky
2017-09-01  4:35     ` Gabriel Krisman Bertazi
2017-09-04  8:21   ` Daniel Vetter
2017-09-04 16:37   ` Ville Syrjälä
2017-09-05  7:36     ` Daniel Stone
2017-09-07 12:19       ` Ville Syrjälä
2017-09-22  5:25         ` Gabriel Krisman Bertazi
2017-09-22 11:54           ` Ville Syrjälä
2017-08-31 20:00 ` [PATCH 1/2] drm: Fix example comment of format modifier blob Ben Widawsky
2017-09-04  8:24   ` Daniel Vetter
2017-08-31 20:22 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2017-08-31 23:37 ` ✓ Fi.CI.IGT: " Patchwork

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.