diff mbox series

[1/6] drm/modes: add drm_mode_print() to dump mode in drm_printer

Message ID 54199d36993bfb00e29cc059ab9a215495405a99.1709843865.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm: debug logging improvements | expand

Commit Message

Jani Nikula March 7, 2024, 8:39 p.m. UTC
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(+)

Comments

Thomas Zimmermann April 5, 2024, 8:45 a.m. UTC | #1
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);
Ville Syrjala April 5, 2024, 6:45 p.m. UTC | #2
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.
Jani Nikula April 8, 2024, 9:24 a.m. UTC | #3
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 mbox series

Patch

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);