diff mbox series

[1/2] drm: Pretty print mode flags

Message ID 20190620185049.8974-1-ville.syrjala@linux.intel.com (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show
Series [1/2] drm: Pretty print mode flags | expand

Commit Message

Ville Syrjala June 20, 2019, 6:50 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Decode the mode flags when printing the modeline so that I
no longer have to decode the hex number myself.

To do this neatly I made the caller provide a temporary
on stack buffer where we can produce the results. I choce 64
bytes as a reasonable size for this. The worst case I think
is > 100 bytes but that kind of mode would be nonsense anyway
so I figured correct decoding isn't as important in such
cases.

Cc: Russell King <linux@armlinux.org.uk>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Cc: linux-amlogic@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/armada/armada_crtc.c          |   3 +-
 drivers/gpu/drm/drm_atomic.c                  |   3 +-
 drivers/gpu/drm/drm_modes.c                   | 116 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_debugfs.c           |   3 +-
 drivers/gpu/drm/meson/meson_dw_hdmi.c         |   3 +-
 drivers/gpu/drm/meson/meson_venc.c            |   4 +-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c     |   3 +-
 .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c  |   3 +-
 .../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c  |   3 +-
 .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c |   3 +-
 .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c  |   4 +-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |   3 +-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c  |   3 +-
 drivers/gpu/drm/msm/dsi/dsi_manager.c         |   3 +-
 drivers/gpu/drm/msm/edp/edp_bridge.c          |   3 +-
 drivers/gpu/drm/omapdrm/omap_connector.c      |   5 +-
 drivers/gpu/drm/omapdrm/omap_crtc.c           |   3 +-
 drivers/gpu/drm/panel/panel-ronbo-rb070d30.c  |   3 +-
 drivers/gpu/drm/sti/sti_crtc.c                |   3 +-
 include/drm/drm_modes.h                       |  14 ++-
 20 files changed, 165 insertions(+), 23 deletions(-)

Comments

Sam Ravnborg June 20, 2019, 8:25 p.m. UTC | #1
Hi Ville.

On Thu, Jun 20, 2019 at 09:50:48PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Decode the mode flags when printing the modeline so that I
> no longer have to decode the hex number myself.
You are extending the current way to print mode flags,
but I would anyway like to point out a different approach.

When I need to print a fourcc code it is as simple as:

{
	struct drm_format_name_buf fbuf;

	printk("My format: %s\n", drm_get_format_name(format, &fbuf);
}

This way to handle this feels more straightforward
than the current approach used for modes.

Maybe bikeshedding, as your mileage may vary.

A middle ground could be to introduce a struct for the buf so we know
the callers do it right.

Most of the code would be the same, but all call sites would need to be
updated.
What do you think?

	Sam


> 
> To do this neatly I made the caller provide a temporary
> on stack buffer where we can produce the results. I choce 64
> bytes as a reasonable size for this. The worst case I think
> is > 100 bytes but that kind of mode would be nonsense anyway
> so I figured correct decoding isn't as important in such
> cases.
> 
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/armada/armada_crtc.c          |   3 +-
>  drivers/gpu/drm/drm_atomic.c                  |   3 +-
>  drivers/gpu/drm/drm_modes.c                   | 116 +++++++++++++++++-
>  drivers/gpu/drm/i915/i915_debugfs.c           |   3 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c         |   3 +-
>  drivers/gpu/drm/meson/meson_venc.c            |   4 +-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c     |   3 +-
>  .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c  |   3 +-
>  .../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c  |   3 +-
>  .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c |   3 +-
>  .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c  |   4 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |   3 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c  |   3 +-
>  drivers/gpu/drm/msm/dsi/dsi_manager.c         |   3 +-
>  drivers/gpu/drm/msm/edp/edp_bridge.c          |   3 +-
>  drivers/gpu/drm/omapdrm/omap_connector.c      |   5 +-
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |   3 +-
>  drivers/gpu/drm/panel/panel-ronbo-rb070d30.c  |   3 +-
>  drivers/gpu/drm/sti/sti_crtc.c                |   3 +-
>  include/drm/drm_modes.h                       |  14 ++-
>  20 files changed, 165 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> index ba4a3fab7745..ce9335682bd2 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -262,6 +262,7 @@ static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	unsigned long flags;
>  	unsigned i;
>  	bool interlaced = !!(adj->flags & DRM_MODE_FLAG_INTERLACE);
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	i = 0;
>  	rm = adj->crtc_hsync_start - adj->crtc_hdisplay;
> @@ -270,7 +271,7 @@ static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	tm = adj->crtc_vtotal - adj->crtc_vsync_end;
>  
>  	DRM_DEBUG_KMS("[CRTC:%d:%s] mode " DRM_MODE_FMT "\n",
> -		      crtc->base.id, crtc->name, DRM_MODE_ARG(adj));
> +		      crtc->base.id, crtc->name, DRM_MODE_ARG(adj, buf));
>  	DRM_DEBUG_KMS("lm %d rm %d tm %d bm %d\n", lm, rm, tm, bm);
>  
>  	/* Now compute the divider for real */
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 419381abbdd1..81caf91fbd72 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -380,6 +380,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p,
>  		const struct drm_crtc_state *state)
>  {
>  	struct drm_crtc *crtc = state->crtc;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
>  	drm_printf(p, "\tenable=%d\n", state->enable);
> @@ -393,7 +394,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p,
>  	drm_printf(p, "\tplane_mask=%x\n", state->plane_mask);
>  	drm_printf(p, "\tconnector_mask=%x\n", state->connector_mask);
>  	drm_printf(p, "\tencoder_mask=%x\n", state->encoder_mask);
> -	drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode));
> +	drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode, buf));
>  
>  	if (crtc->funcs->atomic_print_state)
>  		crtc->funcs->atomic_print_state(p, state);
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 57e6408288c8..3d15c600295a 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -45,6 +45,118 @@
>  
>  #include "drm_crtc_internal.h"
>  
> +static char *snprint_cont(char *buf, int *len,
> +			  const char *str, bool last)
> +{
> +	int r;
> +
> +	r = snprintf(buf, *len, "%s%s", str, last ? "" : ",");
> +	if (r >= *len)
> +		return buf;
> +
> +	*len -= r;
> +	buf += r;
> +
> +	return buf;
> +}
> +
> +#define MODE_STR(x) { .name = #x, .flag = DRM_MODE_FLAG_ ## x, }
> +
> +static const struct {
> +	const char *name;
> +	u32 flag;
> +} mode_flags[] = {
> +	MODE_STR(PHSYNC),
> +	MODE_STR(NHSYNC),
> +	MODE_STR(PVSYNC),
> +	MODE_STR(NVSYNC),
> +	MODE_STR(INTERLACE),
> +	MODE_STR(CSYNC),
> +	MODE_STR(PCSYNC),
> +	MODE_STR(NCSYNC),
> +	MODE_STR(DBLSCAN),
> +	MODE_STR(HSKEW),
> +	MODE_STR(DBLCLK),
> +	MODE_STR(CLKDIV2),
> +};
> +
> +#undef MODE_STR
> +#define MODE_STR(x) [DRM_MODE_FLAG_3D_ ## x >> 14] = #x
> +
> +static const char * const stereo_flags[] = {
> +	MODE_STR(NONE),
> +	MODE_STR(FRAME_PACKING),
> +	MODE_STR(FIELD_ALTERNATIVE),
> +	MODE_STR(LINE_ALTERNATIVE),
> +	MODE_STR(SIDE_BY_SIDE_FULL),
> +	MODE_STR(L_DEPTH),
> +	MODE_STR(L_DEPTH_GFX_GFX_DEPTH),
> +	MODE_STR(TOP_AND_BOTTOM),
> +	MODE_STR(SIDE_BY_SIDE_HALF),
> +};
> +
> +#undef MODE_STR
> +#define MODE_STR(x) [DRM_MODE_FLAG_PIC_AR_ ## x >> 19] = #x
> +
> +static const char * const aspect_flags[] = {
> +	MODE_STR(NONE),
> +	MODE_STR(4_3),
> +	MODE_STR(16_9),
> +	MODE_STR(64_27),
> +	MODE_STR(256_135),
> +};
> +
> +#undef MODE_STR
> +
> +const char *drm_get_mode_flags_name(char *buf, int len, u32 flags)
> +{
> +	char *ptr = buf;
> +	int i;
> +
> +	if (len == 0)
> +		return buf;
> +
> +	buf[0] = '\0';
> +
> +	if (flags & DRM_MODE_FLAG_3D_MASK) {
> +		int stereo = (flags & DRM_MODE_FLAG_3D_MASK) >> 14;
> +
> +		if (stereo < ARRAY_SIZE(stereo_flags)) {
> +			flags &= ~DRM_MODE_FLAG_3D_MASK;
> +			ptr = snprint_cont(ptr, &len,
> +					   stereo_flags[stereo], !flags);
> +		}
> +	}
> +
> +	if (flags & DRM_MODE_FLAG_PIC_AR_MASK) {
> +		int aspect = (flags & DRM_MODE_FLAG_PIC_AR_MASK) >> 19;
> +
> +		if (aspect < ARRAY_SIZE(aspect_flags)) {
> +			flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
> +			ptr = snprint_cont(ptr, &len,
> +					   aspect_flags[aspect], !flags);
> +		}
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(mode_flags); i++) {
> +		u32 flag = mode_flags[i].flag;
> +
> +		if ((flags & flag) == 0)
> +			continue;
> +
> +		flags &= ~flag;
> +
> +		ptr = snprint_cont(ptr, &len,
> +				   mode_flags[i].name, !flags);
> +	}
> +
> +	if (flags)
> +		ptr = snprint_cont(ptr, &len, "?", true);
> +
> +	return buf;
> +}
> +EXPORT_SYMBOL(drm_get_mode_flags_name);
> +
>  /**
>   * drm_mode_debug_printmodeline - print a mode to dmesg
>   * @mode: mode to print
> @@ -53,7 +165,9 @@
>   */
>  void drm_mode_debug_printmodeline(const struct drm_display_mode *mode)
>  {
> -	DRM_DEBUG_KMS("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> +
> +	DRM_DEBUG_KMS("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
>  }
>  EXPORT_SYMBOL(drm_mode_debug_printmodeline);
>  
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 62cf34db9280..18a3ff8e1461 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2539,12 +2539,13 @@ static int i915_dmc_info(struct seq_file *m, void *unused)
>  static void intel_seq_print_mode(struct seq_file *m, int tabs,
>  				 struct drm_display_mode *mode)
>  {
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  	int i;
>  
>  	for (i = 0; i < tabs; i++)
>  		seq_putc(m, '\t');
>  
> -	seq_printf(m, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> +	seq_printf(m, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
>  }
>  
>  static void intel_encoder_info(struct seq_file *m,
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index df3f9ddd2234..30e53a043ba6 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -610,13 +610,14 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
>  		   const struct drm_display_mode *mode)
>  {
>  	struct meson_drm *priv = connector->dev->dev_private;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  	unsigned int vclk_freq;
>  	unsigned int venc_freq;
>  	unsigned int hdmi_freq;
>  	int vic = drm_match_cea_mode(mode);
>  	enum drm_mode_status status;
>  
> -	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> +	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
>  
>  	/* If sink max TMDS clock, we reject the mode */
>  	if (connector->display_info.max_tmds_clock &&
> diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
> index 7b7a0d8d737c..09acbc06f9f3 100644
> --- a/drivers/gpu/drm/meson/meson_venc.c
> +++ b/drivers/gpu/drm/meson/meson_venc.c
> @@ -987,9 +987,11 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
>  	if (meson_venc_hdmi_supported_vic(vic)) {
>  		vmode = meson_venc_hdmi_get_vic_vmode(vic);
>  		if (!vmode) {
> +			char buf[DRM_MODE_FLAGS_BUF_LEN];
> +
>  			dev_err(priv->dev, "%s: Fatal Error, unsupported mode "
>  				DRM_MODE_FMT "\n", __func__,
> -				DRM_MODE_ARG(mode));
> +				DRM_MODE_ARG(mode, buf));
>  			return;
>  		}
>  	} else {
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> index 0cfd4c06b610..f68d9f74b0e4 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> @@ -238,6 +238,7 @@ static void mdp4_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	enum mdp4_dma dma = mdp4_crtc->dma;
>  	int ovlp = mdp4_crtc->ovlp;
>  	struct drm_display_mode *mode;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	if (WARN_ON(!crtc->state))
>  		return;
> @@ -245,7 +246,7 @@ static void mdp4_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	mode = &crtc->state->adjusted_mode;
>  
>  	DBG("%s: set mode: " DRM_MODE_FMT,
> -			mdp4_crtc->name, DRM_MODE_ARG(mode));
> +	    mdp4_crtc->name, DRM_MODE_ARG(mode, buf));
>  
>  	mdp4_write(mdp4_kms, REG_MDP4_DMA_SRC_SIZE(dma),
>  			MDP4_DMA_SRC_SIZE_WIDTH(mode->hdisplay) |
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
> index caa39b4621e3..2e0dca4d2484 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
> @@ -55,10 +55,11 @@ static void mdp4_dsi_encoder_mode_set(struct drm_encoder *encoder,
>  	uint32_t dsi_hsync_skew, vsync_period, vsync_len, ctrl_pol;
>  	uint32_t display_v_start, display_v_end;
>  	uint32_t hsync_start_x, hsync_end_x;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	mode = adjusted_mode;
>  
> -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
>  
>  	ctrl_pol = 0;
>  	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
> index 259d51971401..e88ac070a672 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
> @@ -101,10 +101,11 @@ static void mdp4_dtv_encoder_mode_set(struct drm_encoder *encoder,
>  	uint32_t dtv_hsync_skew, vsync_period, vsync_len, ctrl_pol;
>  	uint32_t display_v_start, display_v_end;
>  	uint32_t hsync_start_x, hsync_end_x;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	mode = adjusted_mode;
>  
> -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
>  
>  	mdp4_dtv_encoder->pixclock = mode->clock * 1000;
>  
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> index df6f9803a1d7..99bdae9c945b 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> @@ -270,10 +270,11 @@ static void mdp4_lcdc_encoder_mode_set(struct drm_encoder *encoder,
>  	uint32_t lcdc_hsync_skew, vsync_period, vsync_len, ctrl_pol;
>  	uint32_t display_v_start, display_v_end;
>  	uint32_t hsync_start_x, hsync_end_x;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	mode = adjusted_mode;
>  
> -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
>  
>  	mdp4_lcdc_encoder->pixclock = mode->clock * 1000;
>  
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> index eeef41fcd4e1..6bffbebee8bb 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> @@ -124,9 +124,11 @@ void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder,
>  			       struct drm_display_mode *mode,
>  			       struct drm_display_mode *adjusted_mode)
>  {
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> +
>  	mode = adjusted_mode;
>  
> -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
>  	pingpong_tearcheck_setup(encoder, mode);
>  	mdp5_crtc_set_pipeline(encoder->crtc);
>  }
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index c3751c95b452..888a25d1da8b 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -378,13 +378,14 @@ static void mdp5_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	u32 mixer_width, val;
>  	unsigned long flags;
>  	struct drm_display_mode *mode;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	if (WARN_ON(!crtc->state))
>  		return;
>  
>  	mode = &crtc->state->adjusted_mode;
>  
> -	DBG("%s: set mode: " DRM_MODE_FMT, crtc->name, DRM_MODE_ARG(mode));
> +	DBG("%s: set mode: " DRM_MODE_FMT, crtc->name, DRM_MODE_ARG(mode, buf));
>  
>  	mixer_width = mode->hdisplay;
>  	if (r_mixer)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
> index 820a62c40063..809118bb6965 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
> @@ -115,10 +115,11 @@ static void mdp5_vid_encoder_mode_set(struct drm_encoder *encoder,
>  	uint32_t hsync_start_x, hsync_end_x;
>  	uint32_t format = 0x2100;
>  	unsigned long flags;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	mode = adjusted_mode;
>  
> -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
>  
>  	ctrl_pol = 0;
>  
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index ec6cb0f7f206..1bf2f503b84b 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -527,8 +527,9 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge,
>  	struct msm_dsi *other_dsi = dsi_mgr_get_other_dsi(id);
>  	struct mipi_dsi_host *host = msm_dsi->host;
>  	bool is_dual_dsi = IS_DUAL_DSI();
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
> -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
>  
>  	if (is_dual_dsi && !IS_MASTER_DSI_LINK(id))
>  		return;
> diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
> index 2950bba4aca9..0844345862ef 100644
> --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> @@ -51,8 +51,9 @@ static void edp_bridge_mode_set(struct drm_bridge *bridge,
>  	struct drm_connector *connector;
>  	struct edp_bridge *edp_bridge = to_edp_bridge(bridge);
>  	struct msm_edp *edp = edp_bridge->edp;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
> -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
>  
>  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>  		if ((connector->encoder != NULL) &&
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index 5967283934e1..4ce29288c70e 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -276,6 +276,7 @@ static enum drm_mode_status omap_connector_mode_valid(struct drm_connector *conn
>  	struct omap_connector *omap_connector = to_omap_connector(connector);
>  	struct drm_display_mode new_mode = { { 0 } };
>  	enum drm_mode_status status;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	status = omap_connector_mode_fixup(omap_connector->output, mode,
>  					   &new_mode);
> @@ -288,8 +289,8 @@ static enum drm_mode_status omap_connector_mode_valid(struct drm_connector *conn
>  
>  done:
>  	DBG("connector: mode %s: " DRM_MODE_FMT,
> -			(status == MODE_OK) ? "valid" : "invalid",
> -			DRM_MODE_ARG(mode));
> +	    (status == MODE_OK) ? "valid" : "invalid",
> +	    DRM_MODE_ARG(mode, buf));
>  
>  	return status;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index d61215494617..221459d6abe9 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -553,9 +553,10 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  {
>  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>  	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	DBG("%s: set mode: " DRM_MODE_FMT,
> -	    omap_crtc->name, DRM_MODE_ARG(mode));
> +	    omap_crtc->name, DRM_MODE_ARG(mode, buf));
>  
>  	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
>  }
> diff --git a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
> index 3c15764f0c03..468ebdca94f4 100644
> --- a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
> +++ b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
> @@ -126,12 +126,13 @@ static int rb070d30_panel_get_modes(struct drm_panel *panel)
>  	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
>  	struct drm_display_mode *mode;
>  	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	mode = drm_mode_duplicate(panel->drm, &default_mode);
>  	if (!mode) {
>  		DRM_DEV_ERROR(&ctx->dsi->dev,
>  			      "Failed to add mode " DRM_MODE_FMT "\n",
> -			      DRM_MODE_ARG(&default_mode));
> +			      DRM_MODE_ARG(&default_mode, buf));
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c
> index dc64fbfc4e61..bc9602f519d7 100644
> --- a/drivers/gpu/drm/sti/sti_crtc.c
> +++ b/drivers/gpu/drm/sti/sti_crtc.c
> @@ -54,11 +54,12 @@ sti_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode)
>  	struct sti_compositor *compo = dev_get_drvdata(dev);
>  	struct clk *compo_clk, *pix_clk;
>  	int rate = mode->clock * 1000;
> +	char buf[DRM_MODE_FLAGS_BUF_LEN];
>  
>  	DRM_DEBUG_KMS("CRTC:%d (%s) mode: (%s)\n",
>  		      crtc->base.id, sti_mixer_to_str(mixer), mode->name);
>  
> -	DRM_DEBUG_KMS(DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> +	DRM_DEBUG_KMS(DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
>  
>  	if (mixer->id == STI_MIXER_MAIN) {
>  		compo_clk = compo->clk_compo_main;
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 083f16747369..3962dbf82100 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -428,20 +428,27 @@ struct drm_display_mode {
>  	struct list_head export_head;
>  };
>  
> +/**
> + * DRM_MODE_FLAGS_BUF_LEN - reasonable size for the buffer passed to DRM_MODE_ARG()
> + */
> +#define DRM_MODE_FLAGS_BUF_LEN 64
> +
>  /**
>   * DRM_MODE_FMT - printf string for &struct drm_display_mode
>   */
> -#define DRM_MODE_FMT    "\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x"
> +#define DRM_MODE_FMT    "\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s"
>  
>  /**
>   * DRM_MODE_ARG - printf arguments for &struct drm_display_mode
>   * @m: display mode
> + * @b: buffer for temporary string
>   */
> -#define DRM_MODE_ARG(m) \
> +#define DRM_MODE_ARG(m, b) \
>  	(m)->name, (m)->vrefresh, (m)->clock, \
>  	(m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \
>  	(m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \
> -	(m)->type, (m)->flags
> +	(m)->type, (m)->flags, \
> +	drm_get_mode_flags_name(b, sizeof(b), (m)->flags)
>  
>  #define obj_to_mode(x) container_of(x, struct drm_display_mode, base)
>  
> @@ -542,5 +549,6 @@ drm_mode_parse_command_line_for_connector(const char *mode_option,
>  struct drm_display_mode *
>  drm_mode_create_from_cmdline_mode(struct drm_device *dev,
>  				  struct drm_cmdline_mode *cmd);
> +const char *drm_get_mode_flags_name(char *buf, int len, u32 flags);
>  
>  #endif /* __DRM_MODES_H__ */
> -- 
> 2.21.0
Ville Syrjala June 24, 2019, 12:37 p.m. UTC | #2
On Thu, Jun 20, 2019 at 10:25:42PM +0200, Sam Ravnborg wrote:
> Hi Ville.
> 
> On Thu, Jun 20, 2019 at 09:50:48PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Decode the mode flags when printing the modeline so that I
> > no longer have to decode the hex number myself.
> You are extending the current way to print mode flags,
> but I would anyway like to point out a different approach.
> 
> When I need to print a fourcc code it is as simple as:
> 
> {
> 	struct drm_format_name_buf fbuf;
> 
> 	printk("My format: %s\n", drm_get_format_name(format, &fbuf);
> }
> 
> This way to handle this feels more straightforward
> than the current approach used for modes.
> 
> Maybe bikeshedding, as your mileage may vary.

You're suggesting to print the whole mode to a temp buffer, and then
print that with just %s? I thought about that, but it would waste
even more stack so didn't think it was all that great.

> 
> A middle ground could be to introduce a struct for the buf so we know
> the callers do it right.

I'm not a fan of that struct approach because it forces you to use
a dedicated buffer for the thing. With just a raw char[] you can
print to anything, so it could be used more like strcat() as well.
Also I think it makes it more clear how much stack space you're
blowing away. And it looks a bit like snprintf() so it's more
clear what's happening on. Although in this case that last point
is kinda lost due to the macro thing.

> 
> Most of the code would be the same, but all call sites would need to be
> updated.
> What do you think?
> 
> 	Sam
> 
> 
> > 
> > To do this neatly I made the caller provide a temporary
> > on stack buffer where we can produce the results. I choce 64
> > bytes as a reasonable size for this. The worst case I think
> > is > 100 bytes but that kind of mode would be nonsense anyway
> > so I figured correct decoding isn't as important in such
> > cases.
> > 
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > Cc: Vincent Abriou <vincent.abriou@st.com>
> > Cc: linux-amlogic@lists.infradead.org
> > Cc: linux-arm-msm@vger.kernel.org
> > Cc: freedreno@lists.freedesktop.org
> > Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/armada/armada_crtc.c          |   3 +-
> >  drivers/gpu/drm/drm_atomic.c                  |   3 +-
> >  drivers/gpu/drm/drm_modes.c                   | 116 +++++++++++++++++-
> >  drivers/gpu/drm/i915/i915_debugfs.c           |   3 +-
> >  drivers/gpu/drm/meson/meson_dw_hdmi.c         |   3 +-
> >  drivers/gpu/drm/meson/meson_venc.c            |   4 +-
> >  drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c     |   3 +-
> >  .../gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c  |   3 +-
> >  .../gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c  |   3 +-
> >  .../gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c |   3 +-
> >  .../gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c  |   4 +-
> >  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |   3 +-
> >  drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c  |   3 +-
> >  drivers/gpu/drm/msm/dsi/dsi_manager.c         |   3 +-
> >  drivers/gpu/drm/msm/edp/edp_bridge.c          |   3 +-
> >  drivers/gpu/drm/omapdrm/omap_connector.c      |   5 +-
> >  drivers/gpu/drm/omapdrm/omap_crtc.c           |   3 +-
> >  drivers/gpu/drm/panel/panel-ronbo-rb070d30.c  |   3 +-
> >  drivers/gpu/drm/sti/sti_crtc.c                |   3 +-
> >  include/drm/drm_modes.h                       |  14 ++-
> >  20 files changed, 165 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> > index ba4a3fab7745..ce9335682bd2 100644
> > --- a/drivers/gpu/drm/armada/armada_crtc.c
> > +++ b/drivers/gpu/drm/armada/armada_crtc.c
> > @@ -262,6 +262,7 @@ static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	unsigned long flags;
> >  	unsigned i;
> >  	bool interlaced = !!(adj->flags & DRM_MODE_FLAG_INTERLACE);
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	i = 0;
> >  	rm = adj->crtc_hsync_start - adj->crtc_hdisplay;
> > @@ -270,7 +271,7 @@ static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	tm = adj->crtc_vtotal - adj->crtc_vsync_end;
> >  
> >  	DRM_DEBUG_KMS("[CRTC:%d:%s] mode " DRM_MODE_FMT "\n",
> > -		      crtc->base.id, crtc->name, DRM_MODE_ARG(adj));
> > +		      crtc->base.id, crtc->name, DRM_MODE_ARG(adj, buf));
> >  	DRM_DEBUG_KMS("lm %d rm %d tm %d bm %d\n", lm, rm, tm, bm);
> >  
> >  	/* Now compute the divider for real */
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 419381abbdd1..81caf91fbd72 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -380,6 +380,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p,
> >  		const struct drm_crtc_state *state)
> >  {
> >  	struct drm_crtc *crtc = state->crtc;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
> >  	drm_printf(p, "\tenable=%d\n", state->enable);
> > @@ -393,7 +394,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p,
> >  	drm_printf(p, "\tplane_mask=%x\n", state->plane_mask);
> >  	drm_printf(p, "\tconnector_mask=%x\n", state->connector_mask);
> >  	drm_printf(p, "\tencoder_mask=%x\n", state->encoder_mask);
> > -	drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode));
> > +	drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode, buf));
> >  
> >  	if (crtc->funcs->atomic_print_state)
> >  		crtc->funcs->atomic_print_state(p, state);
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 57e6408288c8..3d15c600295a 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -45,6 +45,118 @@
> >  
> >  #include "drm_crtc_internal.h"
> >  
> > +static char *snprint_cont(char *buf, int *len,
> > +			  const char *str, bool last)
> > +{
> > +	int r;
> > +
> > +	r = snprintf(buf, *len, "%s%s", str, last ? "" : ",");
> > +	if (r >= *len)
> > +		return buf;
> > +
> > +	*len -= r;
> > +	buf += r;
> > +
> > +	return buf;
> > +}
> > +
> > +#define MODE_STR(x) { .name = #x, .flag = DRM_MODE_FLAG_ ## x, }
> > +
> > +static const struct {
> > +	const char *name;
> > +	u32 flag;
> > +} mode_flags[] = {
> > +	MODE_STR(PHSYNC),
> > +	MODE_STR(NHSYNC),
> > +	MODE_STR(PVSYNC),
> > +	MODE_STR(NVSYNC),
> > +	MODE_STR(INTERLACE),
> > +	MODE_STR(CSYNC),
> > +	MODE_STR(PCSYNC),
> > +	MODE_STR(NCSYNC),
> > +	MODE_STR(DBLSCAN),
> > +	MODE_STR(HSKEW),
> > +	MODE_STR(DBLCLK),
> > +	MODE_STR(CLKDIV2),
> > +};
> > +
> > +#undef MODE_STR
> > +#define MODE_STR(x) [DRM_MODE_FLAG_3D_ ## x >> 14] = #x
> > +
> > +static const char * const stereo_flags[] = {
> > +	MODE_STR(NONE),
> > +	MODE_STR(FRAME_PACKING),
> > +	MODE_STR(FIELD_ALTERNATIVE),
> > +	MODE_STR(LINE_ALTERNATIVE),
> > +	MODE_STR(SIDE_BY_SIDE_FULL),
> > +	MODE_STR(L_DEPTH),
> > +	MODE_STR(L_DEPTH_GFX_GFX_DEPTH),
> > +	MODE_STR(TOP_AND_BOTTOM),
> > +	MODE_STR(SIDE_BY_SIDE_HALF),
> > +};
> > +
> > +#undef MODE_STR
> > +#define MODE_STR(x) [DRM_MODE_FLAG_PIC_AR_ ## x >> 19] = #x
> > +
> > +static const char * const aspect_flags[] = {
> > +	MODE_STR(NONE),
> > +	MODE_STR(4_3),
> > +	MODE_STR(16_9),
> > +	MODE_STR(64_27),
> > +	MODE_STR(256_135),
> > +};
> > +
> > +#undef MODE_STR
> > +
> > +const char *drm_get_mode_flags_name(char *buf, int len, u32 flags)
> > +{
> > +	char *ptr = buf;
> > +	int i;
> > +
> > +	if (len == 0)
> > +		return buf;
> > +
> > +	buf[0] = '\0';
> > +
> > +	if (flags & DRM_MODE_FLAG_3D_MASK) {
> > +		int stereo = (flags & DRM_MODE_FLAG_3D_MASK) >> 14;
> > +
> > +		if (stereo < ARRAY_SIZE(stereo_flags)) {
> > +			flags &= ~DRM_MODE_FLAG_3D_MASK;
> > +			ptr = snprint_cont(ptr, &len,
> > +					   stereo_flags[stereo], !flags);
> > +		}
> > +	}
> > +
> > +	if (flags & DRM_MODE_FLAG_PIC_AR_MASK) {
> > +		int aspect = (flags & DRM_MODE_FLAG_PIC_AR_MASK) >> 19;
> > +
> > +		if (aspect < ARRAY_SIZE(aspect_flags)) {
> > +			flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
> > +			ptr = snprint_cont(ptr, &len,
> > +					   aspect_flags[aspect], !flags);
> > +		}
> > +	}
> > +
> > +	for (i = 0; i < ARRAY_SIZE(mode_flags); i++) {
> > +		u32 flag = mode_flags[i].flag;
> > +
> > +		if ((flags & flag) == 0)
> > +			continue;
> > +
> > +		flags &= ~flag;
> > +
> > +		ptr = snprint_cont(ptr, &len,
> > +				   mode_flags[i].name, !flags);
> > +	}
> > +
> > +	if (flags)
> > +		ptr = snprint_cont(ptr, &len, "?", true);
> > +
> > +	return buf;
> > +}
> > +EXPORT_SYMBOL(drm_get_mode_flags_name);
> > +
> >  /**
> >   * drm_mode_debug_printmodeline - print a mode to dmesg
> >   * @mode: mode to print
> > @@ -53,7 +165,9 @@
> >   */
> >  void drm_mode_debug_printmodeline(const struct drm_display_mode *mode)
> >  {
> > -	DRM_DEBUG_KMS("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> > +
> > +	DRM_DEBUG_KMS("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
> >  }
> >  EXPORT_SYMBOL(drm_mode_debug_printmodeline);
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 62cf34db9280..18a3ff8e1461 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2539,12 +2539,13 @@ static int i915_dmc_info(struct seq_file *m, void *unused)
> >  static void intel_seq_print_mode(struct seq_file *m, int tabs,
> >  				 struct drm_display_mode *mode)
> >  {
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  	int i;
> >  
> >  	for (i = 0; i < tabs; i++)
> >  		seq_putc(m, '\t');
> >  
> > -	seq_printf(m, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> > +	seq_printf(m, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
> >  }
> >  
> >  static void intel_encoder_info(struct seq_file *m,
> > diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> > index df3f9ddd2234..30e53a043ba6 100644
> > --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> > +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> > @@ -610,13 +610,14 @@ dw_hdmi_mode_valid(struct drm_connector *connector,
> >  		   const struct drm_display_mode *mode)
> >  {
> >  	struct meson_drm *priv = connector->dev->dev_private;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  	unsigned int vclk_freq;
> >  	unsigned int venc_freq;
> >  	unsigned int hdmi_freq;
> >  	int vic = drm_match_cea_mode(mode);
> >  	enum drm_mode_status status;
> >  
> > -	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> > +	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
> >  
> >  	/* If sink max TMDS clock, we reject the mode */
> >  	if (connector->display_info.max_tmds_clock &&
> > diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
> > index 7b7a0d8d737c..09acbc06f9f3 100644
> > --- a/drivers/gpu/drm/meson/meson_venc.c
> > +++ b/drivers/gpu/drm/meson/meson_venc.c
> > @@ -987,9 +987,11 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
> >  	if (meson_venc_hdmi_supported_vic(vic)) {
> >  		vmode = meson_venc_hdmi_get_vic_vmode(vic);
> >  		if (!vmode) {
> > +			char buf[DRM_MODE_FLAGS_BUF_LEN];
> > +
> >  			dev_err(priv->dev, "%s: Fatal Error, unsupported mode "
> >  				DRM_MODE_FMT "\n", __func__,
> > -				DRM_MODE_ARG(mode));
> > +				DRM_MODE_ARG(mode, buf));
> >  			return;
> >  		}
> >  	} else {
> > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> > index 0cfd4c06b610..f68d9f74b0e4 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> > @@ -238,6 +238,7 @@ static void mdp4_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	enum mdp4_dma dma = mdp4_crtc->dma;
> >  	int ovlp = mdp4_crtc->ovlp;
> >  	struct drm_display_mode *mode;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	if (WARN_ON(!crtc->state))
> >  		return;
> > @@ -245,7 +246,7 @@ static void mdp4_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	mode = &crtc->state->adjusted_mode;
> >  
> >  	DBG("%s: set mode: " DRM_MODE_FMT,
> > -			mdp4_crtc->name, DRM_MODE_ARG(mode));
> > +	    mdp4_crtc->name, DRM_MODE_ARG(mode, buf));
> >  
> >  	mdp4_write(mdp4_kms, REG_MDP4_DMA_SRC_SIZE(dma),
> >  			MDP4_DMA_SRC_SIZE_WIDTH(mode->hdisplay) |
> > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
> > index caa39b4621e3..2e0dca4d2484 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
> > @@ -55,10 +55,11 @@ static void mdp4_dsi_encoder_mode_set(struct drm_encoder *encoder,
> >  	uint32_t dsi_hsync_skew, vsync_period, vsync_len, ctrl_pol;
> >  	uint32_t display_v_start, display_v_end;
> >  	uint32_t hsync_start_x, hsync_end_x;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	mode = adjusted_mode;
> >  
> > -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> > +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
> >  
> >  	ctrl_pol = 0;
> >  	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
> > index 259d51971401..e88ac070a672 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
> > @@ -101,10 +101,11 @@ static void mdp4_dtv_encoder_mode_set(struct drm_encoder *encoder,
> >  	uint32_t dtv_hsync_skew, vsync_period, vsync_len, ctrl_pol;
> >  	uint32_t display_v_start, display_v_end;
> >  	uint32_t hsync_start_x, hsync_end_x;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	mode = adjusted_mode;
> >  
> > -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> > +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
> >  
> >  	mdp4_dtv_encoder->pixclock = mode->clock * 1000;
> >  
> > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> > index df6f9803a1d7..99bdae9c945b 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> > @@ -270,10 +270,11 @@ static void mdp4_lcdc_encoder_mode_set(struct drm_encoder *encoder,
> >  	uint32_t lcdc_hsync_skew, vsync_period, vsync_len, ctrl_pol;
> >  	uint32_t display_v_start, display_v_end;
> >  	uint32_t hsync_start_x, hsync_end_x;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	mode = adjusted_mode;
> >  
> > -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> > +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
> >  
> >  	mdp4_lcdc_encoder->pixclock = mode->clock * 1000;
> >  
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> > index eeef41fcd4e1..6bffbebee8bb 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
> > @@ -124,9 +124,11 @@ void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder,
> >  			       struct drm_display_mode *mode,
> >  			       struct drm_display_mode *adjusted_mode)
> >  {
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> > +
> >  	mode = adjusted_mode;
> >  
> > -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> > +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
> >  	pingpong_tearcheck_setup(encoder, mode);
> >  	mdp5_crtc_set_pipeline(encoder->crtc);
> >  }
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> > index c3751c95b452..888a25d1da8b 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> > @@ -378,13 +378,14 @@ static void mdp5_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  	u32 mixer_width, val;
> >  	unsigned long flags;
> >  	struct drm_display_mode *mode;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	if (WARN_ON(!crtc->state))
> >  		return;
> >  
> >  	mode = &crtc->state->adjusted_mode;
> >  
> > -	DBG("%s: set mode: " DRM_MODE_FMT, crtc->name, DRM_MODE_ARG(mode));
> > +	DBG("%s: set mode: " DRM_MODE_FMT, crtc->name, DRM_MODE_ARG(mode, buf));
> >  
> >  	mixer_width = mode->hdisplay;
> >  	if (r_mixer)
> > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
> > index 820a62c40063..809118bb6965 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
> > @@ -115,10 +115,11 @@ static void mdp5_vid_encoder_mode_set(struct drm_encoder *encoder,
> >  	uint32_t hsync_start_x, hsync_end_x;
> >  	uint32_t format = 0x2100;
> >  	unsigned long flags;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	mode = adjusted_mode;
> >  
> > -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> > +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
> >  
> >  	ctrl_pol = 0;
> >  
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > index ec6cb0f7f206..1bf2f503b84b 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -527,8 +527,9 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge,
> >  	struct msm_dsi *other_dsi = dsi_mgr_get_other_dsi(id);
> >  	struct mipi_dsi_host *host = msm_dsi->host;
> >  	bool is_dual_dsi = IS_DUAL_DSI();
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> > -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> > +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
> >  
> >  	if (is_dual_dsi && !IS_MASTER_DSI_LINK(id))
> >  		return;
> > diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > index 2950bba4aca9..0844345862ef 100644
> > --- a/drivers/gpu/drm/msm/edp/edp_bridge.c
> > +++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
> > @@ -51,8 +51,9 @@ static void edp_bridge_mode_set(struct drm_bridge *bridge,
> >  	struct drm_connector *connector;
> >  	struct edp_bridge *edp_bridge = to_edp_bridge(bridge);
> >  	struct msm_edp *edp = edp_bridge->edp;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> > -	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
> > +	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
> >  
> >  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> >  		if ((connector->encoder != NULL) &&
> > diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> > index 5967283934e1..4ce29288c70e 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> > @@ -276,6 +276,7 @@ static enum drm_mode_status omap_connector_mode_valid(struct drm_connector *conn
> >  	struct omap_connector *omap_connector = to_omap_connector(connector);
> >  	struct drm_display_mode new_mode = { { 0 } };
> >  	enum drm_mode_status status;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	status = omap_connector_mode_fixup(omap_connector->output, mode,
> >  					   &new_mode);
> > @@ -288,8 +289,8 @@ static enum drm_mode_status omap_connector_mode_valid(struct drm_connector *conn
> >  
> >  done:
> >  	DBG("connector: mode %s: " DRM_MODE_FMT,
> > -			(status == MODE_OK) ? "valid" : "invalid",
> > -			DRM_MODE_ARG(mode));
> > +	    (status == MODE_OK) ? "valid" : "invalid",
> > +	    DRM_MODE_ARG(mode, buf));
> >  
> >  	return status;
> >  }
> > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> > index d61215494617..221459d6abe9 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> > @@ -553,9 +553,10 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >  {
> >  	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> >  	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	DBG("%s: set mode: " DRM_MODE_FMT,
> > -	    omap_crtc->name, DRM_MODE_ARG(mode));
> > +	    omap_crtc->name, DRM_MODE_ARG(mode, buf));
> >  
> >  	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
> >  }
> > diff --git a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
> > index 3c15764f0c03..468ebdca94f4 100644
> > --- a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
> > +++ b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
> > @@ -126,12 +126,13 @@ static int rb070d30_panel_get_modes(struct drm_panel *panel)
> >  	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
> >  	struct drm_display_mode *mode;
> >  	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	mode = drm_mode_duplicate(panel->drm, &default_mode);
> >  	if (!mode) {
> >  		DRM_DEV_ERROR(&ctx->dsi->dev,
> >  			      "Failed to add mode " DRM_MODE_FMT "\n",
> > -			      DRM_MODE_ARG(&default_mode));
> > +			      DRM_MODE_ARG(&default_mode, buf));
> >  		return -EINVAL;
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c
> > index dc64fbfc4e61..bc9602f519d7 100644
> > --- a/drivers/gpu/drm/sti/sti_crtc.c
> > +++ b/drivers/gpu/drm/sti/sti_crtc.c
> > @@ -54,11 +54,12 @@ sti_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode)
> >  	struct sti_compositor *compo = dev_get_drvdata(dev);
> >  	struct clk *compo_clk, *pix_clk;
> >  	int rate = mode->clock * 1000;
> > +	char buf[DRM_MODE_FLAGS_BUF_LEN];
> >  
> >  	DRM_DEBUG_KMS("CRTC:%d (%s) mode: (%s)\n",
> >  		      crtc->base.id, sti_mixer_to_str(mixer), mode->name);
> >  
> > -	DRM_DEBUG_KMS(DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> > +	DRM_DEBUG_KMS(DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
> >  
> >  	if (mixer->id == STI_MIXER_MAIN) {
> >  		compo_clk = compo->clk_compo_main;
> > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> > index 083f16747369..3962dbf82100 100644
> > --- a/include/drm/drm_modes.h
> > +++ b/include/drm/drm_modes.h
> > @@ -428,20 +428,27 @@ struct drm_display_mode {
> >  	struct list_head export_head;
> >  };
> >  
> > +/**
> > + * DRM_MODE_FLAGS_BUF_LEN - reasonable size for the buffer passed to DRM_MODE_ARG()
> > + */
> > +#define DRM_MODE_FLAGS_BUF_LEN 64
> > +
> >  /**
> >   * DRM_MODE_FMT - printf string for &struct drm_display_mode
> >   */
> > -#define DRM_MODE_FMT    "\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x"
> > +#define DRM_MODE_FMT    "\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s"
> >  
> >  /**
> >   * DRM_MODE_ARG - printf arguments for &struct drm_display_mode
> >   * @m: display mode
> > + * @b: buffer for temporary string
> >   */
> > -#define DRM_MODE_ARG(m) \
> > +#define DRM_MODE_ARG(m, b) \
> >  	(m)->name, (m)->vrefresh, (m)->clock, \
> >  	(m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \
> >  	(m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \
> > -	(m)->type, (m)->flags
> > +	(m)->type, (m)->flags, \
> > +	drm_get_mode_flags_name(b, sizeof(b), (m)->flags)
> >  
> >  #define obj_to_mode(x) container_of(x, struct drm_display_mode, base)
> >  
> > @@ -542,5 +549,6 @@ drm_mode_parse_command_line_for_connector(const char *mode_option,
> >  struct drm_display_mode *
> >  drm_mode_create_from_cmdline_mode(struct drm_device *dev,
> >  				  struct drm_cmdline_mode *cmd);
> > +const char *drm_get_mode_flags_name(char *buf, int len, u32 flags);
> >  
> >  #endif /* __DRM_MODES_H__ */
> > -- 
> > 2.21.0
diff mbox series

Patch

diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index ba4a3fab7745..ce9335682bd2 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -262,6 +262,7 @@  static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	unsigned long flags;
 	unsigned i;
 	bool interlaced = !!(adj->flags & DRM_MODE_FLAG_INTERLACE);
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	i = 0;
 	rm = adj->crtc_hsync_start - adj->crtc_hdisplay;
@@ -270,7 +271,7 @@  static void armada_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	tm = adj->crtc_vtotal - adj->crtc_vsync_end;
 
 	DRM_DEBUG_KMS("[CRTC:%d:%s] mode " DRM_MODE_FMT "\n",
-		      crtc->base.id, crtc->name, DRM_MODE_ARG(adj));
+		      crtc->base.id, crtc->name, DRM_MODE_ARG(adj, buf));
 	DRM_DEBUG_KMS("lm %d rm %d tm %d bm %d\n", lm, rm, tm, bm);
 
 	/* Now compute the divider for real */
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 419381abbdd1..81caf91fbd72 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -380,6 +380,7 @@  static void drm_atomic_crtc_print_state(struct drm_printer *p,
 		const struct drm_crtc_state *state)
 {
 	struct drm_crtc *crtc = state->crtc;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	drm_printf(p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
 	drm_printf(p, "\tenable=%d\n", state->enable);
@@ -393,7 +394,7 @@  static void drm_atomic_crtc_print_state(struct drm_printer *p,
 	drm_printf(p, "\tplane_mask=%x\n", state->plane_mask);
 	drm_printf(p, "\tconnector_mask=%x\n", state->connector_mask);
 	drm_printf(p, "\tencoder_mask=%x\n", state->encoder_mask);
-	drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode));
+	drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode, buf));
 
 	if (crtc->funcs->atomic_print_state)
 		crtc->funcs->atomic_print_state(p, state);
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 57e6408288c8..3d15c600295a 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -45,6 +45,118 @@ 
 
 #include "drm_crtc_internal.h"
 
+static char *snprint_cont(char *buf, int *len,
+			  const char *str, bool last)
+{
+	int r;
+
+	r = snprintf(buf, *len, "%s%s", str, last ? "" : ",");
+	if (r >= *len)
+		return buf;
+
+	*len -= r;
+	buf += r;
+
+	return buf;
+}
+
+#define MODE_STR(x) { .name = #x, .flag = DRM_MODE_FLAG_ ## x, }
+
+static const struct {
+	const char *name;
+	u32 flag;
+} mode_flags[] = {
+	MODE_STR(PHSYNC),
+	MODE_STR(NHSYNC),
+	MODE_STR(PVSYNC),
+	MODE_STR(NVSYNC),
+	MODE_STR(INTERLACE),
+	MODE_STR(CSYNC),
+	MODE_STR(PCSYNC),
+	MODE_STR(NCSYNC),
+	MODE_STR(DBLSCAN),
+	MODE_STR(HSKEW),
+	MODE_STR(DBLCLK),
+	MODE_STR(CLKDIV2),
+};
+
+#undef MODE_STR
+#define MODE_STR(x) [DRM_MODE_FLAG_3D_ ## x >> 14] = #x
+
+static const char * const stereo_flags[] = {
+	MODE_STR(NONE),
+	MODE_STR(FRAME_PACKING),
+	MODE_STR(FIELD_ALTERNATIVE),
+	MODE_STR(LINE_ALTERNATIVE),
+	MODE_STR(SIDE_BY_SIDE_FULL),
+	MODE_STR(L_DEPTH),
+	MODE_STR(L_DEPTH_GFX_GFX_DEPTH),
+	MODE_STR(TOP_AND_BOTTOM),
+	MODE_STR(SIDE_BY_SIDE_HALF),
+};
+
+#undef MODE_STR
+#define MODE_STR(x) [DRM_MODE_FLAG_PIC_AR_ ## x >> 19] = #x
+
+static const char * const aspect_flags[] = {
+	MODE_STR(NONE),
+	MODE_STR(4_3),
+	MODE_STR(16_9),
+	MODE_STR(64_27),
+	MODE_STR(256_135),
+};
+
+#undef MODE_STR
+
+const char *drm_get_mode_flags_name(char *buf, int len, u32 flags)
+{
+	char *ptr = buf;
+	int i;
+
+	if (len == 0)
+		return buf;
+
+	buf[0] = '\0';
+
+	if (flags & DRM_MODE_FLAG_3D_MASK) {
+		int stereo = (flags & DRM_MODE_FLAG_3D_MASK) >> 14;
+
+		if (stereo < ARRAY_SIZE(stereo_flags)) {
+			flags &= ~DRM_MODE_FLAG_3D_MASK;
+			ptr = snprint_cont(ptr, &len,
+					   stereo_flags[stereo], !flags);
+		}
+	}
+
+	if (flags & DRM_MODE_FLAG_PIC_AR_MASK) {
+		int aspect = (flags & DRM_MODE_FLAG_PIC_AR_MASK) >> 19;
+
+		if (aspect < ARRAY_SIZE(aspect_flags)) {
+			flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+			ptr = snprint_cont(ptr, &len,
+					   aspect_flags[aspect], !flags);
+		}
+	}
+
+	for (i = 0; i < ARRAY_SIZE(mode_flags); i++) {
+		u32 flag = mode_flags[i].flag;
+
+		if ((flags & flag) == 0)
+			continue;
+
+		flags &= ~flag;
+
+		ptr = snprint_cont(ptr, &len,
+				   mode_flags[i].name, !flags);
+	}
+
+	if (flags)
+		ptr = snprint_cont(ptr, &len, "?", true);
+
+	return buf;
+}
+EXPORT_SYMBOL(drm_get_mode_flags_name);
+
 /**
  * drm_mode_debug_printmodeline - print a mode to dmesg
  * @mode: mode to print
@@ -53,7 +165,9 @@ 
  */
 void drm_mode_debug_printmodeline(const struct drm_display_mode *mode)
 {
-	DRM_DEBUG_KMS("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
+
+	DRM_DEBUG_KMS("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
 }
 EXPORT_SYMBOL(drm_mode_debug_printmodeline);
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 62cf34db9280..18a3ff8e1461 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2539,12 +2539,13 @@  static int i915_dmc_info(struct seq_file *m, void *unused)
 static void intel_seq_print_mode(struct seq_file *m, int tabs,
 				 struct drm_display_mode *mode)
 {
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 	int i;
 
 	for (i = 0; i < tabs; i++)
 		seq_putc(m, '\t');
 
-	seq_printf(m, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
+	seq_printf(m, DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
 }
 
 static void intel_encoder_info(struct seq_file *m,
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index df3f9ddd2234..30e53a043ba6 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -610,13 +610,14 @@  dw_hdmi_mode_valid(struct drm_connector *connector,
 		   const struct drm_display_mode *mode)
 {
 	struct meson_drm *priv = connector->dev->dev_private;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 	unsigned int vclk_freq;
 	unsigned int venc_freq;
 	unsigned int hdmi_freq;
 	int vic = drm_match_cea_mode(mode);
 	enum drm_mode_status status;
 
-	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
+	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
 
 	/* If sink max TMDS clock, we reject the mode */
 	if (connector->display_info.max_tmds_clock &&
diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
index 7b7a0d8d737c..09acbc06f9f3 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -987,9 +987,11 @@  void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
 	if (meson_venc_hdmi_supported_vic(vic)) {
 		vmode = meson_venc_hdmi_get_vic_vmode(vic);
 		if (!vmode) {
+			char buf[DRM_MODE_FLAGS_BUF_LEN];
+
 			dev_err(priv->dev, "%s: Fatal Error, unsupported mode "
 				DRM_MODE_FMT "\n", __func__,
-				DRM_MODE_ARG(mode));
+				DRM_MODE_ARG(mode, buf));
 			return;
 		}
 	} else {
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 0cfd4c06b610..f68d9f74b0e4 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -238,6 +238,7 @@  static void mdp4_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	enum mdp4_dma dma = mdp4_crtc->dma;
 	int ovlp = mdp4_crtc->ovlp;
 	struct drm_display_mode *mode;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	if (WARN_ON(!crtc->state))
 		return;
@@ -245,7 +246,7 @@  static void mdp4_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	mode = &crtc->state->adjusted_mode;
 
 	DBG("%s: set mode: " DRM_MODE_FMT,
-			mdp4_crtc->name, DRM_MODE_ARG(mode));
+	    mdp4_crtc->name, DRM_MODE_ARG(mode, buf));
 
 	mdp4_write(mdp4_kms, REG_MDP4_DMA_SRC_SIZE(dma),
 			MDP4_DMA_SRC_SIZE_WIDTH(mode->hdisplay) |
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
index caa39b4621e3..2e0dca4d2484 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
@@ -55,10 +55,11 @@  static void mdp4_dsi_encoder_mode_set(struct drm_encoder *encoder,
 	uint32_t dsi_hsync_skew, vsync_period, vsync_len, ctrl_pol;
 	uint32_t display_v_start, display_v_end;
 	uint32_t hsync_start_x, hsync_end_x;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	mode = adjusted_mode;
 
-	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
+	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
 
 	ctrl_pol = 0;
 	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
index 259d51971401..e88ac070a672 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dtv_encoder.c
@@ -101,10 +101,11 @@  static void mdp4_dtv_encoder_mode_set(struct drm_encoder *encoder,
 	uint32_t dtv_hsync_skew, vsync_period, vsync_len, ctrl_pol;
 	uint32_t display_v_start, display_v_end;
 	uint32_t hsync_start_x, hsync_end_x;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	mode = adjusted_mode;
 
-	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
+	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
 
 	mdp4_dtv_encoder->pixclock = mode->clock * 1000;
 
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
index df6f9803a1d7..99bdae9c945b 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
@@ -270,10 +270,11 @@  static void mdp4_lcdc_encoder_mode_set(struct drm_encoder *encoder,
 	uint32_t lcdc_hsync_skew, vsync_period, vsync_len, ctrl_pol;
 	uint32_t display_v_start, display_v_end;
 	uint32_t hsync_start_x, hsync_end_x;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	mode = adjusted_mode;
 
-	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
+	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
 
 	mdp4_lcdc_encoder->pixclock = mode->clock * 1000;
 
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
index eeef41fcd4e1..6bffbebee8bb 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cmd_encoder.c
@@ -124,9 +124,11 @@  void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder,
 			       struct drm_display_mode *mode,
 			       struct drm_display_mode *adjusted_mode)
 {
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
+
 	mode = adjusted_mode;
 
-	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
+	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
 	pingpong_tearcheck_setup(encoder, mode);
 	mdp5_crtc_set_pipeline(encoder->crtc);
 }
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index c3751c95b452..888a25d1da8b 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -378,13 +378,14 @@  static void mdp5_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	u32 mixer_width, val;
 	unsigned long flags;
 	struct drm_display_mode *mode;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	if (WARN_ON(!crtc->state))
 		return;
 
 	mode = &crtc->state->adjusted_mode;
 
-	DBG("%s: set mode: " DRM_MODE_FMT, crtc->name, DRM_MODE_ARG(mode));
+	DBG("%s: set mode: " DRM_MODE_FMT, crtc->name, DRM_MODE_ARG(mode, buf));
 
 	mixer_width = mode->hdisplay;
 	if (r_mixer)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
index 820a62c40063..809118bb6965 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
@@ -115,10 +115,11 @@  static void mdp5_vid_encoder_mode_set(struct drm_encoder *encoder,
 	uint32_t hsync_start_x, hsync_end_x;
 	uint32_t format = 0x2100;
 	unsigned long flags;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	mode = adjusted_mode;
 
-	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
+	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
 
 	ctrl_pol = 0;
 
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index ec6cb0f7f206..1bf2f503b84b 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -527,8 +527,9 @@  static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge,
 	struct msm_dsi *other_dsi = dsi_mgr_get_other_dsi(id);
 	struct mipi_dsi_host *host = msm_dsi->host;
 	bool is_dual_dsi = IS_DUAL_DSI();
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
-	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
+	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
 
 	if (is_dual_dsi && !IS_MASTER_DSI_LINK(id))
 		return;
diff --git a/drivers/gpu/drm/msm/edp/edp_bridge.c b/drivers/gpu/drm/msm/edp/edp_bridge.c
index 2950bba4aca9..0844345862ef 100644
--- a/drivers/gpu/drm/msm/edp/edp_bridge.c
+++ b/drivers/gpu/drm/msm/edp/edp_bridge.c
@@ -51,8 +51,9 @@  static void edp_bridge_mode_set(struct drm_bridge *bridge,
 	struct drm_connector *connector;
 	struct edp_bridge *edp_bridge = to_edp_bridge(bridge);
 	struct msm_edp *edp = edp_bridge->edp;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
-	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode));
+	DBG("set mode: " DRM_MODE_FMT, DRM_MODE_ARG(mode, buf));
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		if ((connector->encoder != NULL) &&
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 5967283934e1..4ce29288c70e 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -276,6 +276,7 @@  static enum drm_mode_status omap_connector_mode_valid(struct drm_connector *conn
 	struct omap_connector *omap_connector = to_omap_connector(connector);
 	struct drm_display_mode new_mode = { { 0 } };
 	enum drm_mode_status status;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	status = omap_connector_mode_fixup(omap_connector->output, mode,
 					   &new_mode);
@@ -288,8 +289,8 @@  static enum drm_mode_status omap_connector_mode_valid(struct drm_connector *conn
 
 done:
 	DBG("connector: mode %s: " DRM_MODE_FMT,
-			(status == MODE_OK) ? "valid" : "invalid",
-			DRM_MODE_ARG(mode));
+	    (status == MODE_OK) ? "valid" : "invalid",
+	    DRM_MODE_ARG(mode, buf));
 
 	return status;
 }
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index d61215494617..221459d6abe9 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -553,9 +553,10 @@  static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	DBG("%s: set mode: " DRM_MODE_FMT,
-	    omap_crtc->name, DRM_MODE_ARG(mode));
+	    omap_crtc->name, DRM_MODE_ARG(mode, buf));
 
 	drm_display_mode_to_videomode(mode, &omap_crtc->vm);
 }
diff --git a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
index 3c15764f0c03..468ebdca94f4 100644
--- a/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
+++ b/drivers/gpu/drm/panel/panel-ronbo-rb070d30.c
@@ -126,12 +126,13 @@  static int rb070d30_panel_get_modes(struct drm_panel *panel)
 	struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
 	struct drm_display_mode *mode;
 	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	mode = drm_mode_duplicate(panel->drm, &default_mode);
 	if (!mode) {
 		DRM_DEV_ERROR(&ctx->dsi->dev,
 			      "Failed to add mode " DRM_MODE_FMT "\n",
-			      DRM_MODE_ARG(&default_mode));
+			      DRM_MODE_ARG(&default_mode, buf));
 		return -EINVAL;
 	}
 
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c
index dc64fbfc4e61..bc9602f519d7 100644
--- a/drivers/gpu/drm/sti/sti_crtc.c
+++ b/drivers/gpu/drm/sti/sti_crtc.c
@@ -54,11 +54,12 @@  sti_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode)
 	struct sti_compositor *compo = dev_get_drvdata(dev);
 	struct clk *compo_clk, *pix_clk;
 	int rate = mode->clock * 1000;
+	char buf[DRM_MODE_FLAGS_BUF_LEN];
 
 	DRM_DEBUG_KMS("CRTC:%d (%s) mode: (%s)\n",
 		      crtc->base.id, sti_mixer_to_str(mixer), mode->name);
 
-	DRM_DEBUG_KMS(DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
+	DRM_DEBUG_KMS(DRM_MODE_FMT "\n", DRM_MODE_ARG(mode, buf));
 
 	if (mixer->id == STI_MIXER_MAIN) {
 		compo_clk = compo->clk_compo_main;
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 083f16747369..3962dbf82100 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -428,20 +428,27 @@  struct drm_display_mode {
 	struct list_head export_head;
 };
 
+/**
+ * DRM_MODE_FLAGS_BUF_LEN - reasonable size for the buffer passed to DRM_MODE_ARG()
+ */
+#define DRM_MODE_FLAGS_BUF_LEN 64
+
 /**
  * DRM_MODE_FMT - printf string for &struct drm_display_mode
  */
-#define DRM_MODE_FMT    "\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x"
+#define DRM_MODE_FMT    "\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s"
 
 /**
  * DRM_MODE_ARG - printf arguments for &struct drm_display_mode
  * @m: display mode
+ * @b: buffer for temporary string
  */
-#define DRM_MODE_ARG(m) \
+#define DRM_MODE_ARG(m, b) \
 	(m)->name, (m)->vrefresh, (m)->clock, \
 	(m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \
 	(m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \
-	(m)->type, (m)->flags
+	(m)->type, (m)->flags, \
+	drm_get_mode_flags_name(b, sizeof(b), (m)->flags)
 
 #define obj_to_mode(x) container_of(x, struct drm_display_mode, base)
 
@@ -542,5 +549,6 @@  drm_mode_parse_command_line_for_connector(const char *mode_option,
 struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd);
+const char *drm_get_mode_flags_name(char *buf, int len, u32 flags);
 
 #endif /* __DRM_MODES_H__ */