All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
@ 2018-03-21 14:23 Katarzyna Dec
  2018-03-21 14:56 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Katarzyna Dec @ 2018-03-21 14:23 UTC (permalink / raw)
  To: intel-gfx

During debugging gpgpu_fill test on various platforms, I found out few
things that can affect newer gens:
  Seting number of threads in TS in gen8_fill_interface_descriptor to 1.
This field was omitted in earlier platforms (apparently without any side
effects). Not setting it for newer platforms results in GPU hang.
  Adding pipeline selection mask to gen9_gpgpu_fillfunc, which is
necessary from SKL.
  Changes gen7_emit_interface_descriptor_load to gen8 version.

Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/gpgpu_fill.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
index 4d98643d..e7a1edaa 100644
--- a/lib/gpgpu_fill.c
+++ b/lib/gpgpu_fill.c
@@ -336,6 +336,8 @@ gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *
 	idd->desc5.constant_urb_entry_read_offset = 0;
 	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
 
+	idd->desc6.num_threads_in_tg = 1;
+
 	return offset;
 }
 
@@ -790,12 +792,13 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	batch->ptr = batch->buffer;
 
 	/* GPGPU pipeline */
-	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
+	OUT_BATCH(GEN7_PIPELINE_SELECT | GEN9_PIPELINE_SELECTION_MASK |
+	PIPELINE_SELECT_GPGPU);
 
 	gen9_emit_state_base_address(batch);
 	gen8_emit_vfe_state_gpgpu(batch);
 	gen7_emit_curbe_load(batch, curbe_buffer);
-	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
+	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
 	gen8_emit_gpgpu_walk(batch, x, y, width, height);
 
 	OUT_BATCH(MI_BATCH_BUFFER_END);
-- 
2.14.3

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

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

* ✓ Fi.CI.BAT: success for lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
  2018-03-21 14:23 [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function Katarzyna Dec
@ 2018-03-21 14:56 ` Patchwork
  2018-03-21 18:43 ` ✓ Fi.CI.IGT: " Patchwork
  2018-03-26 21:28   ` [igt-dev] " Daniele Ceraolo Spurio
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-03-21 14:56 UTC (permalink / raw)
  To: Katarzyna Dec; +Cc: intel-gfx

== Series Details ==

Series: lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
URL   : https://patchwork.freedesktop.org/series/40378/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
ddc4ffb00e389a4dd584e8055eadf283dff69db5 lib: Don't fail if plane IN_FORMATS not present

with latest DRM-Tip kernel build CI_DRM_3960
54f3d9c6882c drm-tip: 2018y-03m-21d-14h-06m-24s UTC integration manifest

No testlist changes.

---- Known issues:

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> FAIL       (fi-cfl-s2) fdo#103481

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

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:434s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:544s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:516s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:517s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:518s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:505s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:414s
fi-cfl-s2        total:285  pass:258  dwarn:0   dfail:0   fail:1   skip:26  time:563s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:510s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:519s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:581s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:421s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:316s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:400s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:473s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:431s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:472s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:466s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:521s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:650s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:441s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:532s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:489s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:430s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:446s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:577s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:403s

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
  2018-03-21 14:23 [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function Katarzyna Dec
  2018-03-21 14:56 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-03-21 18:43 ` Patchwork
  2018-03-26 21:28   ` [igt-dev] " Daniele Ceraolo Spurio
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-03-21 18:43 UTC (permalink / raw)
  To: Katarzyna Dec; +Cc: intel-gfx

== Series Details ==

Series: lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
URL   : https://patchwork.freedesktop.org/series/40378/
State : success

== Summary ==

---- Possible new issues:

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                skip       -> PASS       (shard-snb)
Test kms_vblank:
        Subgroup pipe-a-ts-continuation-idle-hang:
                skip       -> PASS       (shard-snb)

---- Known issues:

Test kms_flip:
        Subgroup 2x-dpms-vs-vblank-race:
                fail       -> PASS       (shard-hsw) fdo#103060 +2
        Subgroup 2x-flip-vs-expired-vblank:
                pass       -> FAIL       (shard-hsw) fdo#102887
        Subgroup blocking-wf_vblank:
                pass       -> FAIL       (shard-hsw) fdo#100368 +2
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
                fail       -> PASS       (shard-apl) fdo#101623 +1
Test kms_rotation_crc:
        Subgroup sprite-rotation-180:
                fail       -> PASS       (shard-snb) fdo#103925
Test kms_vblank:
        Subgroup pipe-a-accuracy-idle:
                pass       -> FAIL       (shard-hsw) fdo#102583

fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583

shard-apl        total:3478 pass:1814 dwarn:1   dfail:0   fail:7   skip:1655 time:13144s
shard-hsw        total:3478 pass:1763 dwarn:1   dfail:0   fail:6   skip:1707 time:11887s
shard-snb        total:3478 pass:1358 dwarn:1   dfail:0   fail:2   skip:2117 time:7264s
Blacklisted hosts:
shard-kbl        total:3432 pass:1913 dwarn:1   dfail:0   fail:9   skip:1508 time:9403s

== Logs ==

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

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

* Re: [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
  2018-03-21 14:23 [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function Katarzyna Dec
@ 2018-03-26 21:28   ` Daniele Ceraolo Spurio
  2018-03-21 18:43 ` ✓ Fi.CI.IGT: " Patchwork
  2018-03-26 21:28   ` [igt-dev] " Daniele Ceraolo Spurio
  2 siblings, 0 replies; 7+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-03-26 21:28 UTC (permalink / raw)
  To: Katarzyna Dec, intel-gfx; +Cc: igt-dev

+ igt-dev

On 21/03/18 07:23, Katarzyna Dec wrote:
> During debugging gpgpu_fill test on various platforms, I found out few
> things that can affect newer gens:

this is slightly confusing, as you start with saying that you've found 
something but then below you start with what you've changed. I'd reword 
to make it clearer what were the issues and how they've been fixed.

>    Seting number of threads in TS in gen8_fill_interface_descriptor to 1.
> This field was omitted in earlier platforms (apparently without any side
> effects). Not setting it for newer platforms results in GPU hang.
>    Adding pipeline selection mask to gen9_gpgpu_fillfunc, which is
> necessary from SKL.
>    Changes gen7_emit_interface_descriptor_load to gen8 version.

This last point applies to the gen9 fillfunc only. I'd also mention that 
the gen8 interface descriptor applies to gen9+ as well for extra clarity.

> 
> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   lib/gpgpu_fill.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> index 4d98643d..e7a1edaa 100644
> --- a/lib/gpgpu_fill.c
> +++ b/lib/gpgpu_fill.c
> @@ -336,6 +336,8 @@ gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *
>   	idd->desc5.constant_urb_entry_read_offset = 0;
>   	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
>   
> +	idd->desc6.num_threads_in_tg = 1;
> +

1 seems like a safe value which applies to all platforms and it doesn't 
change execution time (gem_gpgpu_fill still completes in ~0.2s on SKL 
with this change applied) so it seems reasonable to me to use it, but 
it'd be nice to get an ack by someone more experienced with the gpgpu 
pipeline.

>   	return offset;
>   }
>   
> @@ -790,12 +792,13 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = batch->buffer;
>   
>   	/* GPGPU pipeline */
> -	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
> +	OUT_BATCH(GEN7_PIPELINE_SELECT | GEN9_PIPELINE_SELECTION_MASK |
> +	PIPELINE_SELECT_GPGPU);

need more indent here.

Apart from the various nitpicks the change looks good to me.

Daniele

>   
>   	gen9_emit_state_base_address(batch);
>   	gen8_emit_vfe_state_gpgpu(batch);
>   	gen7_emit_curbe_load(batch, curbe_buffer);
> -	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> +	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
>   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
>   
>   	OUT_BATCH(MI_BATCH_BUFFER_END);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
@ 2018-03-26 21:28   ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 7+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-03-26 21:28 UTC (permalink / raw)
  To: Katarzyna Dec, intel-gfx; +Cc: igt-dev

+ igt-dev

On 21/03/18 07:23, Katarzyna Dec wrote:
> During debugging gpgpu_fill test on various platforms, I found out few
> things that can affect newer gens:

this is slightly confusing, as you start with saying that you've found 
something but then below you start with what you've changed. I'd reword 
to make it clearer what were the issues and how they've been fixed.

>    Seting number of threads in TS in gen8_fill_interface_descriptor to 1.
> This field was omitted in earlier platforms (apparently without any side
> effects). Not setting it for newer platforms results in GPU hang.
>    Adding pipeline selection mask to gen9_gpgpu_fillfunc, which is
> necessary from SKL.
>    Changes gen7_emit_interface_descriptor_load to gen8 version.

This last point applies to the gen9 fillfunc only. I'd also mention that 
the gen8 interface descriptor applies to gen9+ as well for extra clarity.

> 
> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   lib/gpgpu_fill.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> index 4d98643d..e7a1edaa 100644
> --- a/lib/gpgpu_fill.c
> +++ b/lib/gpgpu_fill.c
> @@ -336,6 +336,8 @@ gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *
>   	idd->desc5.constant_urb_entry_read_offset = 0;
>   	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
>   
> +	idd->desc6.num_threads_in_tg = 1;
> +

1 seems like a safe value which applies to all platforms and it doesn't 
change execution time (gem_gpgpu_fill still completes in ~0.2s on SKL 
with this change applied) so it seems reasonable to me to use it, but 
it'd be nice to get an ack by someone more experienced with the gpgpu 
pipeline.

>   	return offset;
>   }
>   
> @@ -790,12 +792,13 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = batch->buffer;
>   
>   	/* GPGPU pipeline */
> -	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
> +	OUT_BATCH(GEN7_PIPELINE_SELECT | GEN9_PIPELINE_SELECTION_MASK |
> +	PIPELINE_SELECT_GPGPU);

need more indent here.

Apart from the various nitpicks the change looks good to me.

Daniele

>   
>   	gen9_emit_state_base_address(batch);
>   	gen8_emit_vfe_state_gpgpu(batch);
>   	gen7_emit_curbe_load(batch, curbe_buffer);
> -	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> +	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
>   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
>   
>   	OUT_BATCH(MI_BATCH_BUFFER_END);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
  2018-03-26 21:28   ` [igt-dev] " Daniele Ceraolo Spurio
@ 2018-03-27  7:43     ` Katarzyna Dec
  -1 siblings, 0 replies; 7+ messages in thread
From: Katarzyna Dec @ 2018-03-27  7:43 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: igt-dev, intel-gfx

On Mon, Mar 26, 2018 at 02:28:48PM -0700, Daniele Ceraolo Spurio wrote:
> + igt-dev
> 
> On 21/03/18 07:23, Katarzyna Dec wrote:
> > During debugging gpgpu_fill test on various platforms, I found out few
> > things that can affect newer gens:
> 
> this is slightly confusing, as you start with saying that you've found
> something but then below you start with what you've changed. I'd reword to
> make it clearer what were the issues and how they've been fixed.
>
Actually, this changes will be included in refactoring series and I will
explain everything (I hope) in there.

> >    Seting number of threads in TS in gen8_fill_interface_descriptor to 1.
> > This field was omitted in earlier platforms (apparently without any side
> > effects). Not setting it for newer platforms results in GPU hang.
> >    Adding pipeline selection mask to gen9_gpgpu_fillfunc, which is
> > necessary from SKL.
> >    Changes gen7_emit_interface_descriptor_load to gen8 version.
> 
> This last point applies to the gen9 fillfunc only. I'd also mention that the
> gen8 interface descriptor applies to gen9+ as well for extra clarity.
>
Gen9+ functions will be based on Gen9, so this change will be included there
there as well.
> > 
> > Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> > Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> >   lib/gpgpu_fill.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> > index 4d98643d..e7a1edaa 100644
> > --- a/lib/gpgpu_fill.c
> > +++ b/lib/gpgpu_fill.c
> > @@ -336,6 +336,8 @@ gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *
> >   	idd->desc5.constant_urb_entry_read_offset = 0;
> >   	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
> > +	idd->desc6.num_threads_in_tg = 1;
> > +
> 
> 1 seems like a safe value which applies to all platforms and it doesn't
> change execution time (gem_gpgpu_fill still completes in ~0.2s on SKL with
> this change applied) so it seems reasonable to me to use it, but it'd be
> nice to get an ack by someone more experienced with the gpgpu pipeline.
> 
Missing seting number of threads in thread group seems like a 'bug' here.
According to documentation from BDW this field cannot be set to 0. We cannot
be sure that it is not zero unless we set it explicitly.

> >   	return offset;
> >   }
> > @@ -790,12 +792,13 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   	batch->ptr = batch->buffer;
> >   	/* GPGPU pipeline */
> > -	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
> > +	OUT_BATCH(GEN7_PIPELINE_SELECT | GEN9_PIPELINE_SELECTION_MASK |
> > +	PIPELINE_SELECT_GPGPU);
> 
> need more indent here.
> 
> Apart from the various nitpicks the change looks good to me.
> 
> Daniele
> 
Refactoring of these libraries will be a good oportunity to adjust code style :)

Thanks for important feedback!
Kasia
> >   	gen9_emit_state_base_address(batch);
> >   	gen8_emit_vfe_state_gpgpu(batch);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> > -	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> > +	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> >   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
> >   	OUT_BATCH(MI_BATCH_BUFFER_END);
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function
@ 2018-03-27  7:43     ` Katarzyna Dec
  0 siblings, 0 replies; 7+ messages in thread
From: Katarzyna Dec @ 2018-03-27  7:43 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: igt-dev, intel-gfx

On Mon, Mar 26, 2018 at 02:28:48PM -0700, Daniele Ceraolo Spurio wrote:
> + igt-dev
> 
> On 21/03/18 07:23, Katarzyna Dec wrote:
> > During debugging gpgpu_fill test on various platforms, I found out few
> > things that can affect newer gens:
> 
> this is slightly confusing, as you start with saying that you've found
> something but then below you start with what you've changed. I'd reword to
> make it clearer what were the issues and how they've been fixed.
>
Actually, this changes will be included in refactoring series and I will
explain everything (I hope) in there.

> >    Seting number of threads in TS in gen8_fill_interface_descriptor to 1.
> > This field was omitted in earlier platforms (apparently without any side
> > effects). Not setting it for newer platforms results in GPU hang.
> >    Adding pipeline selection mask to gen9_gpgpu_fillfunc, which is
> > necessary from SKL.
> >    Changes gen7_emit_interface_descriptor_load to gen8 version.
> 
> This last point applies to the gen9 fillfunc only. I'd also mention that the
> gen8 interface descriptor applies to gen9+ as well for extra clarity.
>
Gen9+ functions will be based on Gen9, so this change will be included there
there as well.
> > 
> > Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> > Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> >   lib/gpgpu_fill.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> > index 4d98643d..e7a1edaa 100644
> > --- a/lib/gpgpu_fill.c
> > +++ b/lib/gpgpu_fill.c
> > @@ -336,6 +336,8 @@ gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *
> >   	idd->desc5.constant_urb_entry_read_offset = 0;
> >   	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
> > +	idd->desc6.num_threads_in_tg = 1;
> > +
> 
> 1 seems like a safe value which applies to all platforms and it doesn't
> change execution time (gem_gpgpu_fill still completes in ~0.2s on SKL with
> this change applied) so it seems reasonable to me to use it, but it'd be
> nice to get an ack by someone more experienced with the gpgpu pipeline.
> 
Missing seting number of threads in thread group seems like a 'bug' here.
According to documentation from BDW this field cannot be set to 0. We cannot
be sure that it is not zero unless we set it explicitly.

> >   	return offset;
> >   }
> > @@ -790,12 +792,13 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   	batch->ptr = batch->buffer;
> >   	/* GPGPU pipeline */
> > -	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
> > +	OUT_BATCH(GEN7_PIPELINE_SELECT | GEN9_PIPELINE_SELECTION_MASK |
> > +	PIPELINE_SELECT_GPGPU);
> 
> need more indent here.
> 
> Apart from the various nitpicks the change looks good to me.
> 
> Daniele
> 
Refactoring of these libraries will be a good oportunity to adjust code style :)

Thanks for important feedback!
Kasia
> >   	gen9_emit_state_base_address(batch);
> >   	gen8_emit_vfe_state_gpgpu(batch);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> > -	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> > +	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> >   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
> >   	OUT_BATCH(MI_BATCH_BUFFER_END);
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-03-27  7:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 14:23 [PATCH i-g-t] lib/gpgpu_fill: Adding missing configuration parameters for gpgpu_fill function Katarzyna Dec
2018-03-21 14:56 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-03-21 18:43 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-26 21:28 ` [PATCH i-g-t] " Daniele Ceraolo Spurio
2018-03-26 21:28   ` [igt-dev] " Daniele Ceraolo Spurio
2018-03-27  7:43   ` Katarzyna Dec
2018-03-27  7:43     ` [Intel-gfx] " Katarzyna Dec

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.