On Thu, Jun 20, 2019 at 11:46 AM Ville Syrjala
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Currently the logs show nothing about the mode's aspect ratio.
> Include that information in the normal mode dump.
>
> Cc: Ilia Mirkin <imirkin@xxxxxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/video/hdmi.c | 3 ++-
> include/drm/drm_modes.h | 4 ++--
> include/linux/hdmi.h | 3 +++
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index b939bc28d886..bc593fe1c268 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum hdmi_colorimetry colorimetry)
> return "Invalid";
> }
>
> -static const char *
> +const char *
> hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect)
> {
> switch (picture_aspect) {
> @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect)
> }
> return "Invalid";
> }
> +EXPORT_SYMBOL(hdmi_picture_aspect_get_name);
So this will return "No Data" most of the time (since the DRM_CAP
won't be enabled)? This will look awkward, esp since the person seeing
this print will have no idea what "No Data" is referring to.
>
> static const char *
> hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect)
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 083f16747369..2b1809c74fbe 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -431,7 +431,7 @@ struct drm_display_mode {
> /**
> * 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
> @@ -441,7 +441,7 @@ struct drm_display_mode {
> (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, hdmi_picture_aspect_get_name((m)->picture_aspect_ratio)
Flags are printed as a literal integer value. Given that AR stuff is
fairly esoteric, I might just print an integer here too. (Why was
picture_aspect_ratio not stuffed into ->flags in the first place?)
>
> #define obj_to_mode(x) container_of(x, struct drm_display_mode, base)
>
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 9918a6c910c5..de7cbe385dba 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame,
> void hdmi_infoframe_log(const char *level, struct device *dev,
> const union hdmi_infoframe *frame);
>
> +const char *
> +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect);
> +
> #endif /* _DRM_HDMI_H */
> --
> 2.21.0
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
