From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 10/15] drm/i915: check the power well when capturing error state Date: Fri, 22 Mar 2013 09:45:38 +0100 Message-ID: References: <1362611003-4823-1-git-send-email-przanoni@gmail.com> <1362611003-4823-11-git-send-email-przanoni@gmail.com> <20130317204637.GP9021@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ie0-f169.google.com (mail-ie0-f169.google.com [209.85.223.169]) by gabe.freedesktop.org (Postfix) with ESMTP id B00ECE5F4E for ; Fri, 22 Mar 2013 01:45:39 -0700 (PDT) Received: by mail-ie0-f169.google.com with SMTP id qd14so3103532ieb.0 for ; Fri, 22 Mar 2013 01:45:39 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Thu, Mar 21, 2013 at 11:12 PM, Paulo Zanoni 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