All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 10/15] drm/i915: check the power well when capturing error state
Date: Sun, 17 Mar 2013 21:51:29 +0100	[thread overview]
Message-ID: <20130317205129.GR9021@phenom.ffwll.local> (raw)
In-Reply-To: <20130317204637.GP9021@phenom.ffwll.local>

On Sun, Mar 17, 2013 at 09:46:37PM +0100, Daniel Vetter wrote:
> On Wed, Mar 06, 2013 at 08:03:17PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > 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 <paulo.r.zanoni@intel.com>
> 
> 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).

Forgotten to add: I like the powerwell related additions to the error
state, but that's imo a separate patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2013-03-17 20:48 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-06 23:03 [PATCH 00/15] More "unclaimed register" fixes Paulo Zanoni
2013-03-06 23:03 ` [PATCH 01/15] drm/i915: only disable DDI sound if intel_crtc->eld_vld Paulo Zanoni
2013-03-07  9:31   ` Ville Syrjälä
2013-03-17 20:17     ` Daniel Vetter
2013-03-17 20:23     ` Daniel Vetter
2013-03-20 22:03       ` Paulo Zanoni
2013-03-20 22:24         ` Daniel Vetter
2013-03-22 17:11   ` [PATCH 2/7] " Paulo Zanoni
2013-03-22 18:11     ` Ville Syrjälä
2013-03-06 23:03 ` [PATCH 02/15] drm/i915: disable sound first on intel_disable_ddi Paulo Zanoni
2013-03-15 18:24   ` Ben Widawsky
2013-03-17 20:25     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 03/15] drm/i915: add intel_power_well_is_down Paulo Zanoni
2013-03-06 23:26   ` Daniel Vetter
2013-03-06 23:31     ` Daniel Vetter
2013-03-15 18:31     ` Ben Widawsky
2013-03-20 22:21       ` Paulo Zanoni
2013-03-22 17:14   ` [PATCH 3/7] drm/i915: add intel_using_power_well Paulo Zanoni
2013-04-17  9:04     ` Daniel Vetter
2013-04-17 12:27       ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 04/15] drm/i915: don't touch the PF regs if the power well is down Paulo Zanoni
2013-03-06 23:28   ` Daniel Vetter
2013-03-15 18:41     ` Ben Widawsky
2013-03-22 17:16   ` [PATCH 4/7] " Paulo Zanoni
2013-04-12 20:23     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 05/15] drm/i915: capture the correct cursor registers on IVB Paulo Zanoni
2013-03-07  9:34   ` Ville Syrjälä
2013-03-15 18:45     ` Ben Widawsky
2013-03-17 20:26       ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 06/15] drm/i915: there's no DSPSIZE register on gen4+ Paulo Zanoni
2013-03-07  9:38   ` Ville Syrjälä
2013-03-15 19:04     ` Ben Widawsky
2013-03-15 19:08       ` Ben Widawsky
2013-03-17 20:29         ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 07/15] drm/i915: there's no DSPADDR register on Haswell Paulo Zanoni
2013-03-15 19:10   ` Ben Widawsky
2013-03-17 20:33     ` Daniel Vetter
2013-03-20 18:01       ` Paulo Zanoni
2013-03-20 21:44         ` Daniel Vetter
2013-03-22 17:19   ` [PATCH 5/7] drm/i915: fix DSPADDR Gen check Paulo Zanoni
2013-03-23 12:33     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 08/15] drm/i915: remove DSPPOS register Paulo Zanoni
2013-03-07  9:43   ` Ville Syrjälä
2013-03-17 20:39     ` Daniel Vetter
2013-03-15 19:13   ` Ben Widawsky
2013-03-22 17:20   ` [PATCH 6/7] drm/i915: there's no DSPPOS register on gen4+ Paulo Zanoni
2013-03-23 12:33     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 09/15] drm/i915: there's no PIPESTAT on Gen5+ Paulo Zanoni
2013-03-06 23:22   ` Daniel Vetter
2013-03-07  9:19     ` Ville Syrjälä
2013-03-22 17:24   ` [PATCH 7/7] drm/i915: there's no PIPESTAT on HAS_PCH_SPLIT platforms Paulo Zanoni
2013-03-23 12:33     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 10/15] drm/i915: check the power well when capturing error state Paulo Zanoni
2013-03-15 19:22   ` Ben Widawsky
2013-03-17 20:46   ` Daniel Vetter
2013-03-17 20:51     ` Daniel Vetter [this message]
2013-03-21 22:12     ` Paulo Zanoni
2013-03-22  8:45       ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 11/15] drm/i915: add HAS_POWER_WELL Paulo Zanoni
2013-03-07 17:14   ` Jesse Barnes
2013-03-17 20:49     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 12/15] drm/i915: reorganize intel_lvds_supported Paulo Zanoni
2013-03-15 20:57   ` Ben Widawsky
2013-03-17 20:52     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 13/15] drm/i915: don't save/restore PCH_LVDS on LPT Paulo Zanoni
2013-03-15 21:04   ` Ben Widawsky
2013-03-17 20:54     ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 14/15] drm/i915: check the power well on i915_pipe_enabled Paulo Zanoni
2013-03-15 21:06   ` Ben Widawsky
2013-03-17 20:55   ` Daniel Vetter
2013-03-06 23:03 ` [PATCH 15/15] drm/i915: add missing space in error message Paulo Zanoni
2013-03-15 21:10   ` Ben Widawsky
2013-03-17 20:57     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130317205129.GR9021@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    --cc=przanoni@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.