On Tue, May 06, 2014 at 02:47:40PM +0100, Chris Wilson wrote: > Why does this work? It is neither the most minimal batch, nor the > maximal. Which state is truly required? It looks like cargo-culted > Chinese.
I'll have to echo this. It's really not obvious why this is needed. If you look at the render engine power context for instance, it's just a list of registers. So if we do: - init_clock_gating() (bad name!) - enable_rc6() The render power context should contain the W/A we setup. Would we do: - enable_rc6() -> enter rc6 -> power context save - init_clock_gating() -> exit rc6 -> power context restore We'd end up restoring the reset value of the registers we touch in init_clock_gating() (or the values after BIOS really), ie may not contain all W/As and hang? Note that init_clock_gating() is not the only place where we touch the registers that are part of the power context(s), we do need to ensure rc6 is only enabled after we setup those registers. It could also be that something else than saving/restoring the power contexts is happening at rc6 entry/exit, but the documentation is rather sparse here and so we need to try talking to the hardware engineers again. So yes very much feels like cargo culting. I'd be nice to really understand what's happening. Now, a rather pragmatic approach would be to take those patches if they actually paper over an issue, but the Tested-by: tags are not legion, Mika didn't reproduce the issue on his BDW (IIRC) and Ben was saying Kristen didn't confirm it was these exact patches that were solving hangs for her (If I understood correctly on the call). I do have to point out that it's a lot of code to review and rather full of details, ie, we'll get it wrong-ish (but not enough to break anything, hopefully). -- Damien _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx