<html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> Hi Mika, Thanks for the review. Please find my comments inline: <div class="moz-cite-prefix">On 2/12/2018 8:04 PM, Kahola, Mika wrote: </div> <blockquote cite="mid:1518446057.7484.5.camel@intel.com" type="cite"> <pre wrap="">On Wed, 2018-01-24 at 18:20 +0530, Nautiyal, Ankit K wrote: </pre> <blockquote type="cite"> <pre wrap="">From: Ankit Nautiyal <a class="moz-txt-link-rfc2396E" href="mailto:ankit.k.nautiyal@intel.com"><ankit.k.nautiyal@intel.com></a> This patch adds the support to print the aspect ratio of the modes (if provided) along with other mode information. Signed-off-by: Ankit Nautiyal <a class="moz-txt-link-rfc2396E" href="mailto:ankit.k.nautiyal@intel.com"><ankit.k.nautiyal@intel.com></a> --- lib/igt_kms.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index eb57f4a..585f94d 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -56,6 +56,14 @@ #include "igt_sysfs.h" #include "sw_sync.h" +#ifndef DRM_MODE_FLAG_PIC_AR_64_27 +#define DRM_MODE_FLAG_PIC_AR_64_27 (3<<19) +#endif + +#ifndef DRM_MODE_FLAG_PIC_AR_256_135 +#define DRM_MODE_FLAG_PIC_AR_256_135 (4<<19) +#endif + </pre> </blockquote> <pre wrap="">Shouldn't these be defined in /include/uapi/drm/drm_mode.h? Although, I wasn't able to find these definitions from that file. Do we have a patch under review in drm to fill this gap?</pre> </blockquote> Yes you are right. These macros are introduced in the drm-devel patch-series to add aspect-ratio support in drm layer. <a class="moz-txt-link-freetext" href="https://patchwork.freedesktop.org/series/33984/">https://patchwork.freedesktop.org/series/33984/</a> which is under review. When the kernel patch gets r-b, I will remove these macros from here and submit next patchset for this series. Regards, Ankit <blockquote cite="mid:1518446057.7484.5.camel@intel.com" type="cite"> <pre wrap=""> Otherwise, the patch looks good. Acked-by: Mika Kahola <a class="moz-txt-link-rfc2396E" href="mailto:mika.kahola@intel.com"><mika.kahola@intel.com></a> </pre> <blockquote type="cite"> <pre wrap=""> /** * SECTION:igt_kms * @short_description: Kernel modesetting support library @@ -491,6 +499,22 @@ static const char *mode_stereo_name(const drmModeModeInfo *mode) } } +static const char *mode_aspect_name(const drmModeModeInfo *mode) +{ + switch (mode->flags & DRM_MODE_FLAG_PIC_AR_MASK) { + case DRM_MODE_FLAG_PIC_AR_4_3: + return "4:3"; + case DRM_MODE_FLAG_PIC_AR_16_9: + return "16:9"; + case DRM_MODE_FLAG_PIC_AR_64_27: + return "64:27"; + case DRM_MODE_FLAG_PIC_AR_256_135: + return "256:135"; + default: + return NULL; + } +} + /** * kmstest_dump_mode: * @mode: libdrm mode structure @@ -500,8 +524,9 @@ static const char *mode_stereo_name(const drmModeModeInfo *mode) void kmstest_dump_mode(drmModeModeInfo *mode) { const char *stereo = mode_stereo_name(mode); + const char *aspect_ratio = mode_aspect_name(mode); - igt_info(" %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d%s%s%s\n", + igt_info(" %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d%s%s%s %s%s%s\n", mode->name, mode->vrefresh, mode->hdisplay, mode->hsync_start, mode->hsync_end, mode->htotal, @@ -509,7 +534,9 @@ void kmstest_dump_mode(drmModeModeInfo *mode) mode->vsync_end, mode->vtotal, mode->flags, mode->type, mode->clock, stereo ? " (3D:" : "", - stereo ? stereo : "", stereo ? ")" : ""); + stereo ? stereo : "", stereo ? ")" : "", + aspect_ratio ? " (Pixel Aspect Ratio:" : "", + aspect_ratio ? aspect_ratio : "", aspect_ratio ? ")" : ""); } /** </pre> </blockquote> </blockquote> </body> </html>