| Message ID | 1461751622-26927-2-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 27 Apr 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Purpose is to enable drivers to print out just the mode > string with their own formatting. Some alternatives that preserve the use of a single printk(), avoiding garbled console output due to races (as discussed on intel-gfx in reply to the cover letter [1]): 1) Simply add a prefix string parameter to use in drm_mode_debug_printmodeline(). Really easy and covers most needs. Maybe wrap this in a macro to use the caller's function name. 2) Model drm_mode_debug_printmodeline() after drm_ut_debug_printk(), adding a mode parameter. Maybe wrap this in a macro to use the caller's function name. 3) Add char *drm_mode_line(mode) that kmallocs a mode line string, or a drm_mode_line(mode, buf, bufsize) that prints the mode string to a statically allocated buffer. BR, Jani. [1] http://mid.gmane.org/87fuu7jp8x.fsf@intel.com > > Existing drm_mode_debug_printmodeline calls the new > helper and the format is unchanged in this patch. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/drm_modes.c | 29 ++++++++++++++++++++++------- > include/drm/drm_modes.h | 1 + > 2 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 7def3d58da18..fd4795e2c8db 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -42,6 +42,24 @@ > #include "drm_crtc_internal.h" > > /** > + * drm_mode_debug_printmodeline_raw - print a mode raw string to dmesg > + * @mode: mode to print > + * > + * Describe @mode using DRM_DEBUG without the loglevel, drm prefix and newline. > + */ > +void drm_mode_debug_printmodeline_raw(const struct drm_display_mode *mode) > +{ > + if (drm_debug & DRM_UT_KMS) > + printk("%d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x", > + mode->base.id, mode->name, mode->vrefresh, mode->clock, > + mode->hdisplay, mode->hsync_start, > + mode->hsync_end, mode->htotal, > + mode->vdisplay, mode->vsync_start, > + mode->vsync_end, mode->vtotal, mode->type, mode->flags); > +} > +EXPORT_SYMBOL(drm_mode_debug_printmodeline_raw); > + > +/** > * drm_mode_debug_printmodeline - print a mode to dmesg > * @mode: mode to print > * > @@ -49,13 +67,10 @@ > */ > void drm_mode_debug_printmodeline(const struct drm_display_mode *mode) > { > - DRM_DEBUG_KMS("Modeline %d:\"%s\" %d %d %d %d %d %d %d %d %d %d " > - "0x%x 0x%x\n", > - mode->base.id, mode->name, mode->vrefresh, mode->clock, > - mode->hdisplay, mode->hsync_start, > - mode->hsync_end, mode->htotal, > - mode->vdisplay, mode->vsync_start, > - mode->vsync_end, mode->vtotal, mode->type, mode->flags); > + DRM_DEBUG_KMS("Modeline "); > + drm_mode_debug_printmodeline_raw(mode); > + if (drm_debug & DRM_UT_KMS) > + printk("\n"); > } > EXPORT_SYMBOL(drm_mode_debug_printmodeline); > > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h > index 625966a906f2..9bb0cd06b80e 100644 > --- a/include/drm/drm_modes.h > +++ b/include/drm/drm_modes.h > @@ -437,6 +437,7 @@ int drm_mode_convert_umode(struct drm_display_mode *out, > const struct drm_mode_modeinfo *in); > void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode); > void drm_mode_debug_printmodeline(const struct drm_display_mode *mode); > +void drm_mode_debug_printmodeline_raw(const struct drm_display_mode *mode); > > struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, > int hdisplay, int vdisplay, int vrefresh,
On 27/04/16 13:35, Jani Nikula wrote: > On Wed, 27 Apr 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Purpose is to enable drivers to print out just the mode >> string with their own formatting. > > Some alternatives that preserve the use of a single printk(), avoiding > garbled console output due to races (as discussed on intel-gfx in reply > to the cover letter [1]): > > 1) Simply add a prefix string parameter to use in > drm_mode_debug_printmodeline(). Really easy and covers most > needs. Maybe wrap this in a macro to use the caller's function name. > > 2) Model drm_mode_debug_printmodeline() after drm_ut_debug_printk(), > adding a mode parameter. Maybe wrap this in a macro to use the > caller's function name. This sounds good to me... > 3) Add char *drm_mode_line(mode) that kmallocs a mode line string, or a > drm_mode_line(mode, buf, bufsize) that prints the mode string to a > statically allocated buffer. ...but it only solved the modeline part of the story. Unless something like 3), which I specifically wanted to avoid. String handling etc.. best to be avoided in general if possible and more so for debug code only. Any potential bug in those is best avoided if they do not exist. And some of them log external input so even more so. Something like debug_start/debug_print/debug_end would solve all that but that would be bigger and core. I'll try to do 2) and see what to do with the rest... Regards, Tvrtko
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 7def3d58da18..fd4795e2c8db 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -42,6 +42,24 @@ #include "drm_crtc_internal.h" /** + * drm_mode_debug_printmodeline_raw - print a mode raw string to dmesg + * @mode: mode to print + * + * Describe @mode using DRM_DEBUG without the loglevel, drm prefix and newline. + */ +void drm_mode_debug_printmodeline_raw(const struct drm_display_mode *mode) +{ + if (drm_debug & DRM_UT_KMS) + printk("%d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x", + mode->base.id, mode->name, mode->vrefresh, mode->clock, + mode->hdisplay, mode->hsync_start, + mode->hsync_end, mode->htotal, + mode->vdisplay, mode->vsync_start, + mode->vsync_end, mode->vtotal, mode->type, mode->flags); +} +EXPORT_SYMBOL(drm_mode_debug_printmodeline_raw); + +/** * drm_mode_debug_printmodeline - print a mode to dmesg * @mode: mode to print * @@ -49,13 +67,10 @@ */ void drm_mode_debug_printmodeline(const struct drm_display_mode *mode) { - DRM_DEBUG_KMS("Modeline %d:\"%s\" %d %d %d %d %d %d %d %d %d %d " - "0x%x 0x%x\n", - mode->base.id, mode->name, mode->vrefresh, mode->clock, - mode->hdisplay, mode->hsync_start, - mode->hsync_end, mode->htotal, - mode->vdisplay, mode->vsync_start, - mode->vsync_end, mode->vtotal, mode->type, mode->flags); + DRM_DEBUG_KMS("Modeline "); + drm_mode_debug_printmodeline_raw(mode); + if (drm_debug & DRM_UT_KMS) + printk("\n"); } EXPORT_SYMBOL(drm_mode_debug_printmodeline); diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 625966a906f2..9bb0cd06b80e 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -437,6 +437,7 @@ int drm_mode_convert_umode(struct drm_display_mode *out, const struct drm_mode_modeinfo *in); void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode); void drm_mode_debug_printmodeline(const struct drm_display_mode *mode); +void drm_mode_debug_printmodeline_raw(const struct drm_display_mode *mode); struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,