[Intel-gfx] [RFC 34/60] drm/i915: introduce kernel blitter_context

[Lucas De Marchi]

+Chris Wilson

On Mon, Aug 3, 2020 at 12:59 PM Lucas De Marchi
<lucas.de.marchi at gmail.com> wrote:
>
> On Fri, Jul 10, 2020 at 5:00 AM Matthew Auld <matthew.auld at intel.com> 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 <matthew.auld at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> > ---
> >  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