| Message ID | 54199d36993bfb00e29cc059ab9a215495405a99.1709843865.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: debug logging improvements | expand |
Hi Am 07.03.24 um 21:39 schrieb Jani Nikula: > Add a printer based function for dumping the modeline, so it's not > limited to KMS debug. > > Note: The printed output intentionally does not have the "Modeline" > prefix. Prefix, if any, is for the caller to decide when initializing > drm_printer. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/drm_modes.c | 13 +++++++++++++ > include/drm/drm_modes.h | 2 ++ > 2 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index c4f88c3a93b7..711750ab57c7 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -49,6 +49,19 @@ > > #include "drm_crtc_internal.h" > > +/** > + * drm_mode_print - print a mode to drm printer > + * @p: drm printer > + * @mode: mode to print > + * > + * Write @mode description to struct drm_printer @p. > + */ > +void drm_mode_print(struct drm_printer *p, const struct drm_display_mode *mode) Could this be a printf function with a trailing format string as final argument? The printed mode could then be part of another string instead of just at the end of it. Best regards Thomas > +{ > + drm_printf(p, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode)); > +} > +EXPORT_SYMBOL(drm_mode_print); > + > /** > * drm_mode_debug_printmodeline - print a mode to dmesg > * @mode: mode to print > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h > index b9bb92e4b029..10c45014fbff 100644 > --- a/include/drm/drm_modes.h > +++ b/include/drm/drm_modes.h > @@ -32,6 +32,7 @@ > #include <drm/drm_mode_object.h> > #include <drm/drm_connector.h> > > +struct drm_printer; > struct videomode; > > /* > @@ -460,6 +461,7 @@ int drm_mode_convert_umode(struct drm_device *dev, > 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_print(struct drm_printer *p, const struct drm_display_mode *mode); > void drm_mode_debug_printmodeline(const struct drm_display_mode *mode); > bool drm_mode_is_420_only(const struct drm_display_info *display, > const struct drm_display_mode *mode);
On Fri, Apr 05, 2024 at 10:45:42AM +0200, Thomas Zimmermann wrote: > Hi > > Am 07.03.24 um 21:39 schrieb Jani Nikula: > > Add a printer based function for dumping the modeline, so it's not > > limited to KMS debug. > > > > Note: The printed output intentionally does not have the "Modeline" > > prefix. Prefix, if any, is for the caller to decide when initializing > > drm_printer. > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > drivers/gpu/drm/drm_modes.c | 13 +++++++++++++ > > include/drm/drm_modes.h | 2 ++ > > 2 files changed, 15 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index c4f88c3a93b7..711750ab57c7 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -49,6 +49,19 @@ > > > > #include "drm_crtc_internal.h" > > > > +/** > > + * drm_mode_print - print a mode to drm printer > > + * @p: drm printer > > + * @mode: mode to print > > + * > > + * Write @mode description to struct drm_printer @p. > > + */ > > +void drm_mode_print(struct drm_printer *p, const struct drm_display_mode *mode) > > Could this be a printf function with a trailing format string as final > argument? The printed mode could then be part of another string instead > of just at the end of it. All this seems pretty much redundant. We already have DRM_MODE_FMT/ARGS so people can include the mode in any kind of format string they want. I would just nuke drm_mode_print() and all its ilk and let people format things themselves.
On Fri, 05 Apr 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Apr 05, 2024 at 10:45:42AM +0200, Thomas Zimmermann wrote: >> Am 07.03.24 um 21:39 schrieb Jani Nikula: >> > +void drm_mode_print(struct drm_printer *p, const struct drm_display_mode *mode) >> >> Could this be a printf function with a trailing format string as final >> argument? The printed mode could then be part of another string instead >> of just at the end of it. > > All this seems pretty much redundant. We already have > DRM_MODE_FMT/ARGS so people can include the mode in any > kind of format string they want. > > I would just nuke drm_mode_print() and all its ilk and > let people format things themselves. Fair enough. Did just that in v2, for all the non-driver stuff anyway. BR, Jani.
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index c4f88c3a93b7..711750ab57c7 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -49,6 +49,19 @@ #include "drm_crtc_internal.h" +/** + * drm_mode_print - print a mode to drm printer + * @p: drm printer + * @mode: mode to print + * + * Write @mode description to struct drm_printer @p. + */ +void drm_mode_print(struct drm_printer *p, const struct drm_display_mode *mode) +{ + drm_printf(p, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode)); +} +EXPORT_SYMBOL(drm_mode_print); + /** * drm_mode_debug_printmodeline - print a mode to dmesg * @mode: mode to print diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index b9bb92e4b029..10c45014fbff 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -32,6 +32,7 @@ #include <drm/drm_mode_object.h> #include <drm/drm_connector.h> +struct drm_printer; struct videomode; /* @@ -460,6 +461,7 @@ int drm_mode_convert_umode(struct drm_device *dev, 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_print(struct drm_printer *p, const struct drm_display_mode *mode); void drm_mode_debug_printmodeline(const struct drm_display_mode *mode); bool drm_mode_is_420_only(const struct drm_display_info *display, const struct drm_display_mode *mode);
Add a printer based function for dumping the modeline, so it's not limited to KMS debug. Note: The printed output intentionally does not have the "Modeline" prefix. Prefix, if any, is for the caller to decide when initializing drm_printer. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/drm_modes.c | 13 +++++++++++++ include/drm/drm_modes.h | 2 ++ 2 files changed, 15 insertions(+)