All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>, Rob Clark <robdclark@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [RFC] drm/i915: tame the chattermouth
Date: Fri, 12 Dec 2014 12:57:40 +0200	[thread overview]
Message-ID: <87k31x6spn.fsf@intel.com> (raw)
In-Reply-To: <20141212080158.GE22904@nuc-i3427.alporthouse.com>

On Fri, 12 Dec 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Dec 12, 2014 at 08:17:23AM +0100, Daniel Vetter wrote:
>> On Thu, Dec 11, 2014 at 06:18:12PM -0500, Rob Clark wrote:
>> > Many distro's have mechanism in place to collect and automatically file
>> > bugs for failed WARN()s.  And since every third line in i915 is a WARN()

Come on Rob, that's not necessary.

>> > it generates quite a lot of noise which is somewhat disconcerting to the
>> > end user.
>> > 
>> > Separate out the internal hw-is-in-the-state-I-expected checks into
>> > I915_WARN()s and allow configuration via i915.verbose_checks module
>> > param about whether this will generate a full blown stacktrace or just
>> > DRM_ERROR().
>> > 
>> > Signed-off-by: Rob Clark <robdclark@gmail.com>
>> 
>> Yeah I guess makes sense, although I still claim that these are as much
>> "we've lost track of shit" bugs as when a refcount underflows or a pointer
>> is NULL when it shouldn't. But I also agree that we've done a stellar job
>> this year at not locking at these bugs, so meh.
>
> How about making the state checker WARNs conditional on drm.debug&2? The
> premise is that they are often tantalising unhelpful without the debug
> log, so only show them when we have both?

As I suggested in an internal mail just days ago, make the checks emit a
DRM_ERROR normally (but do log something!), and a WARN if drm.debug &
DRM_UT_KMS (or DRM_UT_DRIVER like Chris suggests) holds.

I object to adding new kernel parameters for this. We have enough, and
our bug fixing efforts really don't need another round of "oh please
*also* enable this new parameter we added".

BR,
Jani.


> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2014-12-12 10:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-11 23:18 [RFC] drm/i915: tame the chattermouth Rob Clark
2014-12-12  7:17 ` Daniel Vetter
2014-12-12  8:01   ` Chris Wilson
2014-12-12 10:57     ` Jani Nikula [this message]
2014-12-12 13:14       ` Rob Clark

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=87k31x6spn.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=robdclark@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.