Re: [Intel-gfx] [PATCH v1 02/11] drm: drop uapi dependency from drm_print.h

From: Sam Ravnborg <sam@ravnborg.org>
To: Jani Nikula <jani.nikula@intel.com>
Cc: Boris Brezillon <bbrezillon@kernel.org>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	David Airlie <airlied@linux.ie>,
	"linux-mediatek@lists.infradead.org"
	<linux-mediatek@lists.infradead.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Sean Paul <seanpaul@chromium.org>,
	Thierry Reding <treding@nvidia.com>,
	"Koenig, Christian" <Christian.Koenig@amd.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [Intel-gfx] [PATCH v1 02/11] drm: drop uapi dependency from drm_print.h
Date: Fri, 2 Aug 2019 17:28:55 +0200	[thread overview]
Message-ID: <20190802152855.GA5626@ravnborg.org> (raw)
In-Reply-To: <87o917lluk.fsf@intel.com>

Hi Jani.

> >> I mean is it useful to have this extra printing subsystem in DRM while 
> >> the standard Linux one actually does a better job?
> > The added functionality of drm_xxx_err would be to keep the current
> > drm.debug=0x1f filtering on the command-line.
> > I do not think we can do this with the standard logging.
> 
> Correct. I'd love the benefits of dynamic debug, but there's no support
> for the kind of message categories that we do with drm.debug.
> 
> I've contemplated switching i915 over to using the variants where you
> pass the device, but I really really don't like the idea of:
> 
> - 	DRM_DEBUG_KMS("hello\n");
> + 	DRM_DEV_DEBUG_KMS(i915->drm.dev, "hello\n");
> 
> Where i915 is our private wrapper for drm_device. I think it's just too
> much ugly uppercase boilerplate, and a large portion of the debugs would
> have to span at least an extra line after that.
> 
> I'd also very much prefer passing just struct *drm_device instead of
> struct *device. In that, I think the needs of the few have prevailed
> over the needs of the many. I'd definitely prefer drm_err(const struct
> drm_device *drm, ...) and friends over the current ones.

This is from my notes:

"
drm_err(const struct drm_device *drm, ...)
drm_info(const struct drm_device *drm, ...)

drm_kms_err(const struct drm_device *drm, ...)
drm_kms_info(const struct drm_device *drm, ...))

etc.

Then we could fish out relevant info from the drm device and present
this nicely.

For the cases where no drm_device is available the fallback is:
drm_dev_err(const struct drm_device *drm, ...)
drm_dev_info(const struct drm_device *drm, ...))

We could modify the existing UPPERCASE macros to be placeholders for
the more reader friendly lower cases variants.
"

So we seems to be aligned here.
In other words - if you throw time after this then try to add
the new logging variants to drm_print.h for the benefit of all.
The we can maybe later do some mass conversion if we want to get rid
of some of the older logging systems.

I have not yet found time/energy to throw after this myself.

	Sam

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel