Re: [Intel-gfx] [RFC 34/60] drm/i915: introduce kernel blitter_context
+Chris Wilson On Mon, Aug 3, 2020 at 12:59 PM Lucas De Marchi wrote: > > On Fri, Jul 10, 2020 at 5:00 AM Matthew Auld wrote: > > > > We may be without a context to perform various internal blitter > > operations, for example when performing object migration. Piggybacking > > off the kernel_context is probably a bad idea, since it has other uses. > > > > Signed-off-by: Matthew Auld > > Cc: Joonas Lahtinen > > Cc: Abdiel Janulgue > > --- > > drivers/gpu/drm/i915/gt/intel_engine_cs.c| 30 +--- > > drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 + > > 2 files changed, 27 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > index dd1a42c4d344..1df94e82550f 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > > @@ -792,6 +792,7 @@ create_kernel_context(struct intel_engine_cs *engine) > > int err; > > > > ce = intel_context_create(engine); > > + > > if (IS_ERR(ce)) > > return ce; > > > > @@ -845,16 +846,32 @@ static int engine_init_common(struct intel_engine_cs > > *engine) > > return PTR_ERR(ce); > > > > ret = measure_breadcrumb_dw(ce); > > - if (ret < 0) > > - goto err_context; > > + if (ret < 0) { > > + intel_context_put(ce); > > I think it's easier to follow the code if the error handling is in one > place. Since you have to put the context. > And since create_kernel_context() pins it, don't we have to > intel_context_unpin() like we are doing > in intel_engine_cleanup_common()? Which would also mean to probably > factor out a `destroy_kernel_context()` > to always do it, and call from here and from intel_engine_cleanup_common(). We actually had a destroy_kernel_context() and the unpin, but that got dropped by e6ba76480299 ("drm/i915: Remove i915->kernel_context") . Wouldn't we hit a GEM_BUG_ON() in the destroy function if we don't unpin it ? Lucas De Marchi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 34/60] drm/i915: introduce kernel blitter_context
On Fri, Jul 10, 2020 at 5:00 AM Matthew Auld wrote: > > We may be without a context to perform various internal blitter > operations, for example when performing object migration. Piggybacking > off the kernel_context is probably a bad idea, since it has other uses. > > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Abdiel Janulgue > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c| 30 +--- > drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 + > 2 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index dd1a42c4d344..1df94e82550f 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -792,6 +792,7 @@ create_kernel_context(struct intel_engine_cs *engine) > int err; > > ce = intel_context_create(engine); > + > if (IS_ERR(ce)) > return ce; > > @@ -845,16 +846,32 @@ static int engine_init_common(struct intel_engine_cs > *engine) > return PTR_ERR(ce); > > ret = measure_breadcrumb_dw(ce); > - if (ret < 0) > - goto err_context; > + if (ret < 0) { > + intel_context_put(ce); I think it's easier to follow the code if the error handling is in one place. Since you have to put the context. And since create_kernel_context() pins it, don't we have to intel_context_unpin() like we are doing in intel_engine_cleanup_common()? Which would also mean to probably factor out a `destroy_kernel_context()` to always do it, and call from here and from intel_engine_cleanup_common(). Lucas De Marchi > + return ret; > + } > > engine->emit_fini_breadcrumb_dw = ret; > engine->kernel_context = ce; > > + /* > +* The blitter context is used to quickly memset or migrate objects > +* in local memory, so it has to always be available. > +*/ > + if (engine->class == COPY_ENGINE_CLASS) { > + ce = create_kernel_context(engine); > + if (IS_ERR(ce)) { > + ret = PTR_ERR(ce); > + goto err_kernel_context; > + } > + > + engine->blitter_context = ce; > + } > + > return 0; > > -err_context: > - intel_context_put(ce); > +err_kernel_context: > + intel_context_put(engine->kernel_context); > return ret; > } > > @@ -910,6 +927,11 @@ void intel_engine_cleanup_common(struct intel_engine_cs > *engine) > if (engine->default_state) > fput(engine->default_state); > > + if (engine->blitter_context) { > + intel_context_unpin(engine->blitter_context); > + intel_context_put(engine->blitter_context); > + } > + > if (engine->kernel_context) { > intel_context_unpin(engine->kernel_context); > intel_context_put(engine->kernel_context); > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h > b/drivers/gpu/drm/i915/gt/intel_engine_types.h > index 490af81bd6f3..3b2d9ed7670f 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > @@ -342,6 +342,7 @@ struct intel_engine_cs { > struct llist_head barrier_tasks; > > struct intel_context *kernel_context; /* pinned */ > + struct intel_context *blitter_context; /* pinned; exists for BCS only > */ > > intel_engine_mask_t saturated; /* submitting semaphores too late? */ > > -- > 2.26.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Lucas De Marchi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 34/60] drm/i915: introduce kernel blitter_context
We may be without a context to perform various internal blitter operations, for example when performing object migration. Piggybacking off the kernel_context is probably a bad idea, since it has other uses. Signed-off-by: Matthew Auld Cc: Joonas Lahtinen Cc: Abdiel Janulgue --- drivers/gpu/drm/i915/gt/intel_engine_cs.c| 30 +--- drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 + 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index dd1a42c4d344..1df94e82550f 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -792,6 +792,7 @@ create_kernel_context(struct intel_engine_cs *engine) int err; ce = intel_context_create(engine); + if (IS_ERR(ce)) return ce; @@ -845,16 +846,32 @@ static int engine_init_common(struct intel_engine_cs *engine) return PTR_ERR(ce); ret = measure_breadcrumb_dw(ce); - if (ret < 0) - goto err_context; + if (ret < 0) { + intel_context_put(ce); + return ret; + } engine->emit_fini_breadcrumb_dw = ret; engine->kernel_context = ce; + /* +* The blitter context is used to quickly memset or migrate objects +* in local memory, so it has to always be available. +*/ + if (engine->class == COPY_ENGINE_CLASS) { + ce = create_kernel_context(engine); + if (IS_ERR(ce)) { + ret = PTR_ERR(ce); + goto err_kernel_context; + } + + engine->blitter_context = ce; + } + return 0; -err_context: - intel_context_put(ce); +err_kernel_context: + intel_context_put(engine->kernel_context); return ret; } @@ -910,6 +927,11 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine) if (engine->default_state) fput(engine->default_state); + if (engine->blitter_context) { + intel_context_unpin(engine->blitter_context); + intel_context_put(engine->blitter_context); + } + if (engine->kernel_context) { intel_context_unpin(engine->kernel_context); intel_context_put(engine->kernel_context); diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 490af81bd6f3..3b2d9ed7670f 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -342,6 +342,7 @@ struct intel_engine_cs { struct llist_head barrier_tasks; struct intel_context *kernel_context; /* pinned */ + struct intel_context *blitter_context; /* pinned; exists for BCS only */ intel_engine_mask_t saturated; /* submitting semaphores too late? */ -- 2.26.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx