Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Josh Boyer <jwboyer@fedoraproject.org>
Cc: "Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	"Michel Dänzer" <michel.daenzer@amd.com>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Daniel Vetter" <daniel.vetter@intel.com>
Subject: Re: [Intel-gfx] [PATCH] drm/vblank: WARN_ON nested use of drm_vblank_on/off
Date: Mon, 22 Jun 2015 16:45:08 +0300	[thread overview]
Message-ID: <87pp4n26xn.fsf@intel.com> (raw)
In-Reply-To: <CA+5PVA6PEP2ZTNkEAnoFVTDotaT_++aFAyxhqWPZ79bQR=1czA@mail.gmail.com>

On Mon, 22 Jun 2015, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Mon, Jun 22, 2015 at 9:27 AM, Jani Nikula
> <jani.nikula@linux.intel.com> wrote:
>> On Mon, 22 Jun 2015, Josh Boyer <jwboyer@fedoraproject.org> wrote:
>>> On Mon, Jun 22, 2015 at 8:02 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>>> We should never nest these since in theory kms drivers should know
>>>> when a pipe is on/off and call the corresponding enable/disable
>>>> functions for the vblank helper code only once. But for historical
>>>> reasons (the shared-with-ums version of this code in modeset_pre/post
>>>> needed to be able to cope with silly userspace that lost track of
>>>> things) we still have this bit of "robustness" around.
>>>>
>>>> Enforce this with a WARN_ON, preparing to eventually rip out this
>>>> special handling.
>>>
>>> This doesn't really provide any context in the WARN_ON itself.  It
>>> will just result in a splat that looks like a kernel oops, and end
>>> users and distribution maintainers will be left scratching their
>>> heads.
>>>
>>> Could this be done with a printk warning instead, or could you at
>>> least provide a pr_warn statement to help people understand why their
>>> machine has an oops splat?
>>
>> FWIW i915_drv.h has
>>
>> #define WARN_ON(x) WARN((x), "WARN_ON(" #x ")")
>>
>> which makes it a little better...
>
> Only a little though, and only in i915.  This is in the generic DRM
> code, isn't it?

You're absolutely right, sorry. Agreed with your complaint.

BR,
Jani.


-- 
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:[~2015-06-22 13:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-22 12:02 [PATCH] drm/vblank: WARN_ON nested use of drm_vblank_on/off Daniel Vetter
2015-06-22 13:12 ` Josh Boyer
2015-06-22 13:27   ` Jani Nikula
2015-06-22 13:36     ` Josh Boyer
2015-06-22 13:45       ` Jani Nikula [this message]
2015-06-26  9:27 ` Tomeu Vizoso

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=87pp4n26xn.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jwboyer@fedoraproject.org \
    --cc=michel.daenzer@amd.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox