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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

-- 
Ville Syrjälä
Intel