* [PATCH] drm/i915/gtt: Relax pd_used assertion
@ 2019-08-20 14:12 Chris Wilson
2019-08-20 14:25 ` Mika Kuoppala
2019-08-20 18:29 ` ✗ Fi.CI.BAT: failure for " Patchwork
0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2019-08-20 14:12 UTC (permalink / raw
To: intel-gfx
The current assertion tries to make sure that we do not over count the
number of used PDE inside a page directory -- that is with an array of
512 pde, we do not expect more than 512 elements used! However, our
assertion has to take into account that as we pin an element into the
page directory, the caller first pins the page directory so the usage
count is one higher. However, this should be one extra pin per thread,
and the upper bound is that we may have one thread for each entry.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e48df11a19fb..9435d184ddf2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -771,7 +771,8 @@ __set_pd_entry(struct i915_page_directory * const pd,
struct i915_page_dma * const to,
u64 (*encode)(const dma_addr_t, const enum i915_cache_level))
{
- GEM_BUG_ON(atomic_read(px_used(pd)) > ARRAY_SIZE(pd->entry));
+ /* Each thread pre-pins the pd, and we may have a thread per each pde */
+ GEM_BUG_ON(atomic_read(px_used(pd)) > 2 * ARRAY_SIZE(pd->entry));
atomic_inc(px_used(pd));
pd->entry[idx] = to;
--
2.23.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/gtt: Relax pd_used assertion
2019-08-20 14:12 [PATCH] drm/i915/gtt: Relax pd_used assertion Chris Wilson
@ 2019-08-20 14:25 ` Mika Kuoppala
2019-08-20 14:28 ` Chris Wilson
2019-08-20 18:29 ` ✗ Fi.CI.BAT: failure for " Patchwork
1 sibling, 1 reply; 4+ messages in thread
From: Mika Kuoppala @ 2019-08-20 14:25 UTC (permalink / raw
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> The current assertion tries to make sure that we do not over count the
> number of used PDE inside a page directory -- that is with an array of
> 512 pde, we do not expect more than 512 elements used! However, our
> assertion has to take into account that as we pin an element into the
> page directory, the caller first pins the page directory so the usage
> count is one higher. However, this should be one extra pin per thread,
> and the upper bound is that we may have one thread for each entry.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index e48df11a19fb..9435d184ddf2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -771,7 +771,8 @@ __set_pd_entry(struct i915_page_directory * const pd,
> struct i915_page_dma * const to,
> u64 (*encode)(const dma_addr_t, const enum i915_cache_level))
> {
> - GEM_BUG_ON(atomic_read(px_used(pd)) > ARRAY_SIZE(pd->entry));
> + /* Each thread pre-pins the pd, and we may have a thread per each pde */
> + GEM_BUG_ON(atomic_read(px_used(pd)) > 2 * ARRAY_SIZE(pd->entry));
When I saw +1 wrt array_size that should have rang some bells between
my ears. I did increase it to +1 for the upper pinning but
the parallelism escaped me and no more bells were rung.
from irc 'the upper page directory' could be added to the commit msg
and to the comment to emphasize why this happen leaf like.
Thanks,
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>
> atomic_inc(px_used(pd));
> pd->entry[idx] = to;
> --
> 2.23.0.rc1
_______________________________________________
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: [PATCH] drm/i915/gtt: Relax pd_used assertion
2019-08-20 14:25 ` Mika Kuoppala
@ 2019-08-20 14:28 ` Chris Wilson
0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2019-08-20 14:28 UTC (permalink / raw
To: Mika Kuoppala, intel-gfx
Quoting Mika Kuoppala (2019-08-20 15:25:50)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
>
> > The current assertion tries to make sure that we do not over count the
> > number of used PDE inside a page directory -- that is with an array of
> > 512 pde, we do not expect more than 512 elements used! However, our
> > assertion has to take into account that as we pin an element into the
> > page directory, the caller first pins the page directory so the usage
> > count is one higher. However, this should be one extra pin per thread,
> > and the upper bound is that we may have one thread for each entry.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index e48df11a19fb..9435d184ddf2 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -771,7 +771,8 @@ __set_pd_entry(struct i915_page_directory * const pd,
> > struct i915_page_dma * const to,
> > u64 (*encode)(const dma_addr_t, const enum i915_cache_level))
> > {
> > - GEM_BUG_ON(atomic_read(px_used(pd)) > ARRAY_SIZE(pd->entry));
> > + /* Each thread pre-pins the pd, and we may have a thread per each pde */
> > + GEM_BUG_ON(atomic_read(px_used(pd)) > 2 * ARRAY_SIZE(pd->entry));
>
> When I saw +1 wrt array_size that should have rang some bells between
> my ears. I did increase it to +1 for the upper pinning but
> the parallelism escaped me and no more bells were rung.
It completely escaped me and I had the reason to make sure this worked
with multiple threads!
Thanks for the review,
-Chris
_______________________________________________
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
* ✗ Fi.CI.BAT: failure for drm/i915/gtt: Relax pd_used assertion
2019-08-20 14:12 [PATCH] drm/i915/gtt: Relax pd_used assertion Chris Wilson
2019-08-20 14:25 ` Mika Kuoppala
@ 2019-08-20 18:29 ` Patchwork
1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2019-08-20 18:29 UTC (permalink / raw
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gtt: Relax pd_used assertion
URL : https://patchwork.freedesktop.org/series/65484/
State : failure
== Summary ==
Applying: drm/i915/gtt: Relax pd_used assertion
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/i915_gem_gtt.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_gem_gtt.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_gem_gtt.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915/gtt: Relax pd_used assertion
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
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
end of thread, other threads:[~2019-08-20 18:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-20 14:12 [PATCH] drm/i915/gtt: Relax pd_used assertion Chris Wilson
2019-08-20 14:25 ` Mika Kuoppala
2019-08-20 14:28 ` Chris Wilson
2019-08-20 18:29 ` ✗ Fi.CI.BAT: failure for " 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.