On Thu, Dec 10, 2020 at 09:00:53PM +0000, Chris Wilson wrote:
> Quoting Matthew Brost (2020-12-10 19:28:06)
> > On Thu, Dec 10, 2020 at 08:02:37AM +0000, Chris Wilson wrote:
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_timeline_types.h b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
> > > index f187c5aac11c..32c51425a0c4 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_timeline_types.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
> > > @@ -20,6 +20,12 @@ struct i915_syncmap;
> > > struct intel_gt;
> > > struct intel_timeline_hwsp;
> > >
> > > +enum intel_timeline_mode {
> > > + INTEL_TIMELINE_ABSOLUTE = 0,
> > > + INTEL_TIMELINE_CONTEXT = BIT(0),
> > > + INTEL_TIMELINE_GLOBAL = BIT(1),
> > > +};
> > > +
> >
> > Not sure I like these names.
> >
> > How about:
> > INTEL_TIMELINE_ABSOLUTE_GGTT
> > INTEL_TIMELINE_RELATIVE_PPGTT
> > INTEL_TIMELINE_RELATIVE_GGTT
>
> They are all in the GGTT, including the ppHWSP.
>
Ah, got it. The 'MI_FLUSH_DW_USE_GTT' in a later patch threw me off. I
see now that it is picking between global status page and per-process
page in that case.
> One is relative to the context, the other relative to the engine.
>
> INTEL_TIMELINE_ABSOLUTE
> INTEL_TIMELINE_RELATIVE_CONTEXT
> INTEL_TIMELINE_RELATIVE_ENGINE
>
I like these names better.
> > Also not convinced we need the 'RELATIVE' modes. See my comments in 'Use
> > indices for writing into relative'.
>
> It saves extra allocations for when we don't (e.g. gen8, and other
> contexts where we know we will never require disposable slots), and
> there's a strong incentive to not use absolute addressing with GVT
Understand using the status page to save on allocations.
I could see relative addressing helping with GVT.
With the name nits:
Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
