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: Sun, 17 Mar 2013 21:46:37 +0100 Message-ID: <20130317204637.GP9021@phenom.ffwll.local> References: <1362611003-4823-1-git-send-email-przanoni@gmail.com> <1362611003-4823-11-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f51.google.com (mail-ee0-f51.google.com [74.125.83.51]) by gabe.freedesktop.org (Postfix) with ESMTP id 438AAE5D01 for ; Sun, 17 Mar 2013 13:43:52 -0700 (PDT) Received: by mail-ee0-f51.google.com with SMTP id d17so2276001eek.38 for ; Sun, 17 Mar 2013 13:43:51 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1362611003-4823-11-git-send-email-przanoni@gmail.com> 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 Wed, Mar 06, 2013 at 08:03:17PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > > This solves many "unclaimed register" messages when the power well is > down and we get a GPU hang. > > Also print the power well register and each pipe's CPU transcoder on > the error state to allow proper interpratation of the registers. And > kzalloc the error state structure since we may not read some of the > registers (to avoid the "unclaimed register" messages). > > Signed-off-by: Paulo Zanoni Ok, I've thought some more about all and you're all going to hate me. Essentially I don't like that we have rather invasive patches for a minor feature with _very_ delicate limits as to what hits the debug yelling and what doesn't. Furthermore the debug feature isn't that clear-cut since interrupts get in the way (and we cant fix that since that would also disable underrun interrupts, which we want). And the audio driver also randomly inteferes. Now where it's justified that a register doesn't exist I don't mind merging the patches - it's always good to have accurate documentation of what's really used by the hw. But for debug features I'm really uneasy with runtime checks, since it might very well be that those are wrong. Hence I vote for a I915_READ_NOCHECK to avoid all this madness. This one will never check for errors, but silently clear them after a read. Note that we don't need a I915_WRITE_NOCHECK, and imo that's good since writing random crap could indeed be a serious bug. So I'll punt on this patch since imo it's too messy for future maintenance. Same for the other debug patches (e.g. the assert_pipe_enabled check). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch