On Mon, 29 Jul 2019, Sam Ravnborg sam@ravnborg.org wrote:
Hi Christian.
On Mon, Jul 29, 2019 at 03:28:15PM +0000, Koenig, Christian wrote:
Am 29.07.19 um 16:35 schrieb Sam Ravnborg:
Even then it so useless (which drm driver is this message for???) that I want to remove them all :(
Yeah, agree. I mean it is nice if the core drm functions use a prefix for debug output.
But I actually don't see the point for individual drivers.
We should all migrate to the versions with device...
Just to do an xkdc.com/927 I have considered:
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, ...))
Why not get completely rid of those and just use dev_err, dev_warn, pr_err, pr_warn etc?
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.
Frankly, I've actually ended up thinking about adding our own, short i915 wrappers for our needs. :(
BR, Jani.
And then we can prefix every logging with driver name and device name.
The idea is to make a thin layer on top of the existing pr_xxx() functions. So not a full subsystem, only a wrapper on top of what we already have.
Anyway, idle talk only. We need patches and sample output if we should discuss more.
Sam
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx