On Thu, Mar 21, 2013 at 11:12 PM, Paulo Zanoni <przanoni at gmail.com> wrote: > I don't understand your way of thinking: your argument is that "this > is difficult to maintain", but then you're proposing the addition of a > new i915_read, which IMHO is way much more complex and feels like a > hackish solution to the problem. Every patch to the "normal" > i915_read#x will have to also patch i915_read_nocheck#x. And on patch > 0014, which you also suggested to use the "I915_READ_NOCHECK" > function, our code will be relying on the _coincidence_ that when we > read a register that does not exist, we read 0. I really don't think > this is a sane approach. I915_READ_NOCHECK already exists, it's called I915_READ_NOTRACE ;-) We'd lose the tracepoint when just using that, but imo that's no too horrible for debug checks. Wrt the assert_pipe check relying on the fact that the hw returns 0, I'm totally ok on relying on that. If that ever changes in a future product, we can fix it. > Our hardware is getting much more complex with respect to these reads, > and I think we should never read ever read from a register that does > not exist or is powered off. I'm open to more suggestions on how to > write this patch, but I really think we shouldn't read registers that > don't exist or are powered off. The "much more complex" part is actually why I've looked into different options for the _debug_ functions. I agree with you for all the real display code. But debug code (especially the gpu hang stuff) tends to not be run too often, hence I'm leaning more towards a "just do it, I know what I'm doing" kind of approach. Currently it's fairly simple, but with future hw we'll have more chances to screw up and not dump a few regs we actually want. So I just want to avoid getting a bug report where the first thing we have to do is patch up the reg dumper (and we can't really test this any other way than by human inspection of a dump). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
