[Intel-gfx] [PATCH 2/5] drm/i915: preserve swizzle settings if necessary v3

[Daniel Vetter]

On Tue, Jun 10, 2014 at 12:45:38PM -0700, Jesse Barnes wrote:
> On Tue, 10 Jun 2014 21:33:27 +0200
> Daniel Vetter <daniel at ffwll.ch> wrote:
>
> > On Tue, Jun 10, 2014 at 7:27 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > > Yes, that's what I do below... I even added it to the changelog:
> > > http://patchwork.freedesktop.org/patch/27223/
> > >
> > > Did you miss the later hunk in intel_display.c?
> > >
> > > What we try to do here is enable swizzling if possible, which we can do
> > > if no inherited fbs are tiled.
> > >
> > > So I think I've done exactly what you repeated above, and documented
> > > it.  So you're going to need to repeat it with different words so I can
> > > understand, if I'm still missing something.
> >
> > In swizzle_detect:
> >
> > ...
> >
> > if (GEN6+) {
> > if (preserve_bios_swizzle) {
> > if (I915_READ(DISP_ARB_CTL) & DISP_TILE_SURFACE_SWIZZLING) {
> > swizzle_x = I915_BIT_6_SWIZZLE_9_10;
> > ...
> > } else {
> > swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> > ...
> > }
> > } else {
> > /* existing/old logic to decide about swizzling */
> > }
> > }
> >
> > ...
> >
> > Plus no shortcut in i915_gem_init_swizzling. Personally I'd also just use
> > a small helper function to compute preserve_bios_swizzle instead of
> > storing it in dev_priv (since we will only use it at exactly one place),
> > but that's a pure style preference.
>
> Doesn't this amount to the same thing?  I.e. we enable it if possible,
> otherwise just report it as unswizzled?  So you're just wanting a style
> change here?

So presuming I read your code correctly there's two issues:

- The first thing you check in swizzle_detect is the hw swizzle bit in
  DSP_ARB. If that's not set you force unswizzled. Since most BIOS don't
  bother to set this (they use an untiled buffer after all) this means
  you've effectively disabled swizzling on almost all machines. The goal
  of keeping the old logic was that we actually want to enable swizzling
  in some situations.

- If you have a machine which uses tiled framebuffers and enables
  swizzling in the BIOS your code will a) drop the swizzle setup in
  gem_init_hw, breaking resume b) not set the swizzle settings correctly
  in swizzle_detect, breaking swap in/out and pwrite/pread. Not sure such
  a machine exists, but still.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch