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