Re: [RFC 06/28] drm/i915: Convert i915_gem_init_swizzling to intel_gt

Chris Wilson
2019-06-14T09:59:24Z

Quoting Tvrtko Ursulin (2019-06-14 10:42:11)
>
> On 14/06/2019 10:24, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-06-14 10:06:41)
> >>
> >> On 13/06/2019 14:49, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-06-13 14:35:17)
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Start using the newly introduced struct intel_gt to fuse together correct
> >>>> logical init flow with uncore for more removal of implicit dev_priv in
> >>>> mmio access.
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>
> >>> Looks fine, I might move it again later next to the fence registers, or
> >>> at least pull this and the detection into its own intel_gt_swizzling.c
> >>>
> >>> Hmm, now that I said that, does that seem like a reasonable thing to do
> >>> right away, see i915_gem_fence_regs.c for the swizzle probe?
> >>
> >> Is swizzling global for the memory controller or applicable only for
> >> fenced regions?
> >
> > As far as my understanding goes, it used to be only for fenced regions
> > when the gpu was the gmch, but completely migrated to the memory
> > controller around snb (with the ring architecture, the GPU was just
> > another client). This coincides with swizzling becoming defunct as part
> > of tiling. To further muddy the pictures, all the time the memory
> > controller is interleaving across the channels. I am pretty certain
> > around gen3 this was explicitly controlled by the GPU for its pages, but
> > by gen5 this is transparent to the GPU. (See the issues with L-shaped
> > memory configurations where the magic was not hidden from the GPU.)
> >
> > So, aiui, for our world view tiling and swizzling are complicit.
>
> Hmm.. looking at the code complicit but still seem separate. So I could
> be more easily convinced the swizzling code does not actually belong in
> i915_gem_fence_reg.c and we should maybe have intel_gt_swizzle.h|c.

gem_fence_reg -> gt/ as well I think, except for the i915_gem_object
bitmaps. We can put the bitmaps into gem/i915_gem_tiling?
-Chris