* [PATCH] drm/i915/gt: Limit VFE threads based on GT
@ 2020-10-16 17:54 Chris Wilson
2020-11-10 0:32 ` [Intel-gfx] " rwright
2021-01-07 19:50 ` Rodrigo Vivi
0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2020-10-16 17:54 UTC (permalink / raw)
To: intel-gfx
Cc: Chris Wilson, Mika Kuoppala, Prathap Kumar Valsan,
Akeem G Abodunrin, Balestrieri Francesco, Bloomfield Jon, stable
MEDIA_STATE_VFE only accepts the 'maximum number of threads' in the
range [0, n-1] where n is #EU * (#threads/EU) with the number of threads
based on plaform and the number of EU based on the number of slices and
subslices. This is a fixed number per platform/gt, so appropriately
limit the number of threads we spawn to match the device.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2024
Fixes: 47f8253d2b89 ("drm/i915/gen7: Clear all EU/L3 residual contexts")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Cc: Balestrieri Francesco <francesco.balestrieri@intel.com>
Cc: Bloomfield Jon <jon.bloomfield@intel.com>
Cc: <stable@vger.kernel.org> # v5.7+
---
drivers/gpu/drm/i915/gt/gen7_renderclear.c | 35 +++++++++++++++-------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
index d93d85cd3027..f3b8fea6226e 100644
--- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
+++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
@@ -7,8 +7,6 @@
#include "i915_drv.h"
#include "intel_gpu_commands.h"
-#define MAX_URB_ENTRIES 64
-#define STATE_SIZE (4 * 1024)
#define GT3_INLINE_DATA_DELAYS 0x1E00
#define batch_advance(Y, CS) GEM_BUG_ON((Y)->end != (CS))
@@ -34,8 +32,7 @@ struct batch_chunk {
};
struct batch_vals {
- u32 max_primitives;
- u32 max_urb_entries;
+ u32 max_primitives; /* == number of VFE threads */
u32 cmd_size;
u32 state_size;
u32 state_start;
@@ -50,18 +47,35 @@ static void
batch_get_defaults(struct drm_i915_private *i915, struct batch_vals *bv)
{
if (IS_HASWELL(i915)) {
- bv->max_primitives = 280;
- bv->max_urb_entries = MAX_URB_ENTRIES;
+ switch (INTEL_INFO(i915)->gt) {
+ default:
+ case 1:
+ bv->max_primitives = 70;
+ break;
+ case 2:
+ bv->max_primitives = 140;
+ break;
+ case 3:
+ bv->max_primitives = 280;
+ break;
+ }
bv->surface_height = 16 * 16;
bv->surface_width = 32 * 2 * 16;
} else {
- bv->max_primitives = 128;
- bv->max_urb_entries = MAX_URB_ENTRIES / 2;
+ switch (INTEL_INFO(i915)->gt) {
+ default:
+ case 1: /* including vlv */
+ bv->max_primitives = 36;
+ break;
+ case 2:
+ bv->max_primitives = 128;
+ break;
+ }
bv->surface_height = 16 * 8;
bv->surface_width = 32 * 16;
}
bv->cmd_size = bv->max_primitives * 4096;
- bv->state_size = STATE_SIZE;
+ bv->state_size = SZ_4K;
bv->state_start = bv->cmd_size;
bv->batch_size = bv->cmd_size + bv->state_size;
bv->scratch_size = bv->surface_height * bv->surface_width;
@@ -244,7 +258,6 @@ gen7_emit_vfe_state(struct batch_chunk *batch,
u32 urb_size, u32 curbe_size,
u32 mode)
{
- u32 urb_entries = bv->max_urb_entries;
u32 threads = bv->max_primitives - 1;
u32 *cs = batch_alloc_items(batch, 32, 8);
@@ -254,7 +267,7 @@ gen7_emit_vfe_state(struct batch_chunk *batch,
*cs++ = 0;
/* number of threads & urb entries for GPGPU vs Media Mode */
- *cs++ = threads << 16 | urb_entries << 8 | mode << 2;
+ *cs++ = threads << 16 | 1 << 8 | mode << 2;
*cs++ = 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Limit VFE threads based on GT
2020-10-16 17:54 [PATCH] drm/i915/gt: Limit VFE threads based on GT Chris Wilson
@ 2020-11-10 0:32 ` rwright
2021-01-07 19:50 ` Rodrigo Vivi
1 sibling, 0 replies; 4+ messages in thread
From: rwright @ 2020-11-10 0:32 UTC (permalink / raw)
To: chris
Cc: mika.kuoppala, prathap.kumar.valsan, akeem.g.abodunrin,
francesco.balestrieri, jon.bloomfield, stable, intel-gfx
On Fri, Oct 16, 2020 at 06:54:11PM +0100, Chris Wilson wrote:
> MEDIA_STATE_VFE only accepts the 'maximum number of threads' in the
> range [0, n-1] where n is #EU * (#threads/EU) with the number of threads
> based on plaform and the number of EU based on the number of slices and
> subslices. This is a fixed number per platform/gt, so appropriately
> limit the number of threads we spawn to match the device.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2024
> Fixes: 47f8253d2b89 ("drm/i915/gen7: Clear all EU/L3 residual contexts")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Prathap Kumar Valsan <prathap.kumar.valsan at intel.com>
> Cc: Akeem G Abodunrin <akeem.g.abodunrin at intel.com>
> Cc: Balestrieri Francesco <francesco.balestrieri at intel.com>
> Cc: Bloomfield Jon <jon.bloomfield at intel.com>
> Cc: <stable at vger.kernel.org> # v5.7+
> ---
> ...
I tested this patch and found that it prevents the GPU hang I had
reported on the HP Pavilion Mini 300-020 in
https://gitlab.freedesktop.org/drm/intel/-/issues/2413.
In more detail: I built linux-next at tag next-20201106 without
the patch, and booted the result on an Ubuntu 20.04 base system. As
expected, I observed the hang that I had previously reported as soon as
Cinnnamon started when I entered graphical.target.
I then applied this patch - that being the only change to my kernel -
and I was able to boot to graphical.target 5 times consecutively without
any GPU hang.
You may add my endorsements:
Tested-by: Randy Wright <rwright@hpe.com>
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2413
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2106
--
Randy Wright Hewlett Packard Enterprise
Phone: (970) 898-0998 Mail: rwright@hpe.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Limit VFE threads based on GT
2020-10-16 17:54 [PATCH] drm/i915/gt: Limit VFE threads based on GT Chris Wilson
2020-11-10 0:32 ` [Intel-gfx] " rwright
@ 2021-01-07 19:50 ` Rodrigo Vivi
2021-01-07 22:04 ` Chris Wilson
1 sibling, 1 reply; 4+ messages in thread
From: Rodrigo Vivi @ 2021-01-07 19:50 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, stable
On Fri, Oct 16, 2020 at 06:54:11PM +0100, Chris Wilson wrote:
> MEDIA_STATE_VFE only accepts the 'maximum number of threads' in the
> range [0, n-1] where n is #EU * (#threads/EU) with the number of threads
> based on plaform and the number of EU based on the number of slices and
> subslices. This is a fixed number per platform/gt, so appropriately
> limit the number of threads we spawn to match the device.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2024
we need to get this closed...
> Fixes: 47f8253d2b89 ("drm/i915/gen7: Clear all EU/L3 residual contexts")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> Cc: Balestrieri Francesco <francesco.balestrieri@intel.com>
> Cc: Bloomfield Jon <jon.bloomfield@intel.com>
> Cc: <stable@vger.kernel.org> # v5.7+
> ---
> drivers/gpu/drm/i915/gt/gen7_renderclear.c | 35 +++++++++++++++-------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> index d93d85cd3027..f3b8fea6226e 100644
> --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> @@ -7,8 +7,6 @@
> #include "i915_drv.h"
> #include "intel_gpu_commands.h"
>
> -#define MAX_URB_ENTRIES 64
> -#define STATE_SIZE (4 * 1024)
> #define GT3_INLINE_DATA_DELAYS 0x1E00
> #define batch_advance(Y, CS) GEM_BUG_ON((Y)->end != (CS))
>
> @@ -34,8 +32,7 @@ struct batch_chunk {
> };
>
> struct batch_vals {
> - u32 max_primitives;
> - u32 max_urb_entries;
> + u32 max_primitives; /* == number of VFE threads */
> u32 cmd_size;
> u32 state_size;
> u32 state_start;
> @@ -50,18 +47,35 @@ static void
> batch_get_defaults(struct drm_i915_private *i915, struct batch_vals *bv)
> {
> if (IS_HASWELL(i915)) {
> - bv->max_primitives = 280;
> - bv->max_urb_entries = MAX_URB_ENTRIES;
> + switch (INTEL_INFO(i915)->gt) {
> + default:
> + case 1:
> + bv->max_primitives = 70;
> + break;
> + case 2:
> + bv->max_primitives = 140;
> + break;
> + case 3:
> + bv->max_primitives = 280;
> + break;
> + }
> bv->surface_height = 16 * 16;
> bv->surface_width = 32 * 2 * 16;
> } else {
> - bv->max_primitives = 128;
> - bv->max_urb_entries = MAX_URB_ENTRIES / 2;
> + switch (INTEL_INFO(i915)->gt) {
> + default:
> + case 1: /* including vlv */
> + bv->max_primitives = 36;
> + break;
> + case 2:
> + bv->max_primitives = 128;
> + break;
> + }
> bv->surface_height = 16 * 8;
> bv->surface_width = 32 * 16;
> }
> bv->cmd_size = bv->max_primitives * 4096;
> - bv->state_size = STATE_SIZE;
> + bv->state_size = SZ_4K;
> bv->state_start = bv->cmd_size;
> bv->batch_size = bv->cmd_size + bv->state_size;
> bv->scratch_size = bv->surface_height * bv->surface_width;
> @@ -244,7 +258,6 @@ gen7_emit_vfe_state(struct batch_chunk *batch,
> u32 urb_size, u32 curbe_size,
> u32 mode)
> {
> - u32 urb_entries = bv->max_urb_entries;
> u32 threads = bv->max_primitives - 1;
> u32 *cs = batch_alloc_items(batch, 32, 8);
>
> @@ -254,7 +267,7 @@ gen7_emit_vfe_state(struct batch_chunk *batch,
> *cs++ = 0;
>
> /* number of threads & urb entries for GPGPU vs Media Mode */
> - *cs++ = threads << 16 | urb_entries << 8 | mode << 2;
> + *cs++ = threads << 16 | 1 << 8 | mode << 2;
why urb_entries = 1 ?
the range is 0,64 and 0,128 depending on the sku.
in general there's a min of 32 URBs
>
> *cs++ = 0;
>
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/gt: Limit VFE threads based on GT
2021-01-07 19:50 ` Rodrigo Vivi
@ 2021-01-07 22:04 ` Chris Wilson
0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2021-01-07 22:04 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, stable
Quoting Rodrigo Vivi (2021-01-07 19:50:37)
> On Fri, Oct 16, 2020 at 06:54:11PM +0100, Chris Wilson wrote:
> > MEDIA_STATE_VFE only accepts the 'maximum number of threads' in the
> > range [0, n-1] where n is #EU * (#threads/EU) with the number of threads
> > based on plaform and the number of EU based on the number of slices and
> > subslices. This is a fixed number per platform/gt, so appropriately
> > limit the number of threads we spawn to match the device.
> >
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2024
>
> we need to get this closed...
Unfortunately this failed the validation test. And as that test is still
not in CI, I cannot say why. My vote would be to remove the
clear_residuals until it works on all target platforms. Plus we clearly
need a hsw-gt1 in CI.
> > bv->scratch_size = bv->surface_height * bv->surface_width;
> > @@ -244,7 +258,6 @@ gen7_emit_vfe_state(struct batch_chunk *batch,
> > u32 urb_size, u32 curbe_size,
> > u32 mode)
> > {
> > - u32 urb_entries = bv->max_urb_entries;
> > u32 threads = bv->max_primitives - 1;
> > u32 *cs = batch_alloc_items(batch, 32, 8);
> >
> > @@ -254,7 +267,7 @@ gen7_emit_vfe_state(struct batch_chunk *batch,
> > *cs++ = 0;
> >
> > /* number of threads & urb entries for GPGPU vs Media Mode */
> > - *cs++ = threads << 16 | urb_entries << 8 | mode << 2;
> > + *cs++ = threads << 16 | 1 << 8 | mode << 2;
>
> why urb_entries = 1 ?
We only used a single entry. There was no measurable benefit from
assigning more entries, and the importance of any side effects from doing
so unknown.
> the range is 0,64 and 0,128 depending on the sku.
>
> in general there's a min of 32 URBs
Don't forget num_entries * entry_size must fit within the URB
allocation/allotment.
-Chris
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-07 22:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16 17:54 [PATCH] drm/i915/gt: Limit VFE threads based on GT Chris Wilson
2020-11-10 0:32 ` [Intel-gfx] " rwright
2021-01-07 19:50 ` Rodrigo Vivi
2021-01-07 22:04 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).