* [PATCH] drm/i915: Increment composite fence seqno
@ 2021-12-14 19:59 Matthew Brost
2021-12-14 20:17 ` [Intel-gfx] " Jani Nikula
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Matthew Brost @ 2021-12-14 19:59 UTC (permalink / raw)
To: intel-gfx, dri-devel; +Cc: daniele.ceraolospurio, john.c.harrison
Increment composite fence seqno on each fence creation.
Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf")
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 2213f7b613da..96cf8361b017 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -3113,7 +3113,7 @@ eb_composite_fence_create(struct i915_execbuffer *eb, int out_fence_fd)
fence_array = dma_fence_array_create(eb->num_batches,
fences,
eb->context->parallel.fence_context,
- eb->context->parallel.seqno,
+ eb->context->parallel.seqno++,
false);
if (!fence_array) {
kfree(fences);
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Increment composite fence seqno
2021-12-14 20:17 ` [Intel-gfx] " Jani Nikula
@ 2021-12-14 20:15 ` Matthew Brost
0 siblings, 0 replies; 5+ messages in thread
From: Matthew Brost @ 2021-12-14 20:15 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dri-devel
On Tue, Dec 14, 2021 at 10:17:48PM +0200, Jani Nikula wrote:
> On Tue, 14 Dec 2021, Matthew Brost <matthew.brost@intel.com> wrote:
> > Increment composite fence seqno on each fence creation.
> >
> > Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf")
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index 2213f7b613da..96cf8361b017 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -3113,7 +3113,7 @@ eb_composite_fence_create(struct i915_execbuffer *eb, int out_fence_fd)
> > fence_array = dma_fence_array_create(eb->num_batches,
> > fences,
> > eb->context->parallel.fence_context,
> > - eb->context->parallel.seqno,
> > + eb->context->parallel.seqno++,
> > false);
> > if (!fence_array) {
> > kfree(fences);
>
> I have no idea what's going on, but the feeling I get from "code smells"
> just in this small snippet is that the seqno++ does not take the error
> path here into account.
>
It does not take the error path into account, but it completely fine to
skip seqno numbers. As long as next valid seqno is greater than the last
valid seqno we should be fine.
Matt
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Increment composite fence seqno
2021-12-14 19:59 [PATCH] drm/i915: Increment composite fence seqno Matthew Brost
@ 2021-12-14 20:17 ` Jani Nikula
2021-12-14 20:15 ` Matthew Brost
2021-12-23 21:32 ` John Harrison
2021-12-29 11:29 ` [Intel-gfx] " Jani Nikula
2 siblings, 1 reply; 5+ messages in thread
From: Jani Nikula @ 2021-12-14 20:17 UTC (permalink / raw)
To: Matthew Brost, intel-gfx, dri-devel
On Tue, 14 Dec 2021, Matthew Brost <matthew.brost@intel.com> wrote:
> Increment composite fence seqno on each fence creation.
>
> Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf")
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 2213f7b613da..96cf8361b017 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -3113,7 +3113,7 @@ eb_composite_fence_create(struct i915_execbuffer *eb, int out_fence_fd)
> fence_array = dma_fence_array_create(eb->num_batches,
> fences,
> eb->context->parallel.fence_context,
> - eb->context->parallel.seqno,
> + eb->context->parallel.seqno++,
> false);
> if (!fence_array) {
> kfree(fences);
I have no idea what's going on, but the feeling I get from "code smells"
just in this small snippet is that the seqno++ does not take the error
path here into account.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Increment composite fence seqno
2021-12-14 19:59 [PATCH] drm/i915: Increment composite fence seqno Matthew Brost
2021-12-14 20:17 ` [Intel-gfx] " Jani Nikula
@ 2021-12-23 21:32 ` John Harrison
2021-12-29 11:29 ` [Intel-gfx] " Jani Nikula
2 siblings, 0 replies; 5+ messages in thread
From: John Harrison @ 2021-12-23 21:32 UTC (permalink / raw)
To: Matthew Brost, intel-gfx, dri-devel; +Cc: daniele.ceraolospurio
On 12/14/2021 11:59, Matthew Brost wrote:
> Increment composite fence seqno on each fence creation.
>
> Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf")
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 2213f7b613da..96cf8361b017 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -3113,7 +3113,7 @@ eb_composite_fence_create(struct i915_execbuffer *eb, int out_fence_fd)
> fence_array = dma_fence_array_create(eb->num_batches,
> fences,
> eb->context->parallel.fence_context,
> - eb->context->parallel.seqno,
> + eb->context->parallel.seqno++,
As is, this change looks good. So:
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
However, just spotted that dma_fence_array_create() takes the seqno
value as an 'unsigned' even though it passes it on to an underlying
dma-fence as a u64 (and all other dma-fence code uses u64 seqnos). That
should probably be fixed (and our context::parallel::seqno changed from
u32 to u64).
John.
> false);
> if (!fence_array) {
> kfree(fences);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Increment composite fence seqno
2021-12-14 19:59 [PATCH] drm/i915: Increment composite fence seqno Matthew Brost
2021-12-14 20:17 ` [Intel-gfx] " Jani Nikula
2021-12-23 21:32 ` John Harrison
@ 2021-12-29 11:29 ` Jani Nikula
2 siblings, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2021-12-29 11:29 UTC (permalink / raw)
To: Matthew Brost, intel-gfx, dri-devel
On Tue, 14 Dec 2021, Matthew Brost <matthew.brost@intel.com> wrote:
> Increment composite fence seqno on each fence creation.
For future reference, this commit message is not enough. Both the
subject line and the commit message just repeat what I can see from the
code, i.e. *what* is being done, but there is not a hint on the *why*
here.
BR,
Jani.
>
> Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf")
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 2213f7b613da..96cf8361b017 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -3113,7 +3113,7 @@ eb_composite_fence_create(struct i915_execbuffer *eb, int out_fence_fd)
> fence_array = dma_fence_array_create(eb->num_batches,
> fences,
> eb->context->parallel.fence_context,
> - eb->context->parallel.seqno,
> + eb->context->parallel.seqno++,
> false);
> if (!fence_array) {
> kfree(fences);
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-29 11:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-14 19:59 [PATCH] drm/i915: Increment composite fence seqno Matthew Brost
2021-12-14 20:17 ` [Intel-gfx] " Jani Nikula
2021-12-14 20:15 ` Matthew Brost
2021-12-23 21:32 ` John Harrison
2021-12-29 11:29 ` [Intel-gfx] " Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox