On Wed, 11 Jun 2014 11:23:26 +0200 Daniel Vetter <daniel@xxxxxxxx> wrote: > 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@xxxxxxxx> wrote: > > > > > On Tue, Jun 10, 2014 at 7:27 PM, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> 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. Ah damn, I had been thinking that bit_6_swizzle was the runtime call, and that the init_swizzle call would go ahead and set it up properly. I see that's too late though. > - 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. This would affect krh's MBA, which is why I wanted testing here... anyway I'll spin a new one and ask krh to test again. -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
