Re: [Intel-gfx] [PATCH] drm/vblank: WARN_ON nested use of drm_vblank_on/off

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