From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vandana Kannan Subject: Re: [PATCH 1/3] [VPG HSW-A] drm/i915: Add aspect ratio in drm_display_mode Date: Wed, 18 Dec 2013 18:32:40 +0530 Message-ID: <52B19CF0.1000707@intel.com> References: <1376542743-25845-1-git-send-email-vandana.kannan@intel.com> <20130815070642.GC7159@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 4998F105829 for ; Wed, 18 Dec 2013 05:03:48 -0800 (PST) In-Reply-To: <20130815070642.GC7159@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: =?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Aug-15-2013 12:36 PM, Ville Syrj=E4l=E4 wrote: > On Thu, Aug 15, 2013 at 10:29:01AM +0530, vandana.kannan@intel.com wrote: >> From: vkannan >> >> Mode is the video format, which is the information the sink needs to >> properly display an image. a complete definition of video format includes >> video timing, picture aspect ratio, color space, quantization range, >> component depth. >> >> video format timing may be associated with more than 1 video format >> for example, 720x480p formatted in the 4:3 Picture Aspect Ratio or a >> 720x480p formatted in the 16:9 Picture Aspect Ratio. >> >> For HDMI compliance, a set of CEA modes are tested (CEA 861-D table 3). >> This list has 64 modes. When one of the modes are set, the corresponding >> fields should show up correctly (as mentioned in Table 3 of CEA spec). >> For picture aspect ratio, if the mode is found from the CEA mode list, >> the corresponding PAR is sent as part of infoframe. If the mode to be set >> is not part of the CEA mode list, PAR is calculated from resolution. >> >> CEA modes have a specific picture aspect ratio. Adding this field >> as part of drm_display_mode. This is required to be sent as part of AVI >> infoframe for HDMI compliance. > = >> >> Signed-off-by: Vandana Kannan >> --- >> drivers/gpu/drm/drm_edid.c | 374 ++++++++++++++++++------= ------- >> drivers/gpu/drm/gma500/oaktrail_lvds.c | 14 +- >> drivers/gpu/drm/gma500/psb_intel_sdvo.c | 38 ++-- >> drivers/gpu/drm/i915/intel_display.c | 3 +- >> drivers/gpu/drm/i915/intel_sdvo.c | 38 ++-- >> include/drm/drm_crtc.h | 7 +- >> 6 files changed, 265 insertions(+), 209 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index e8d17ee..83e2fda 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -135,378 +135,379 @@ static const struct drm_display_mode drm_dmt_mod= es[] =3D { >> /* 640x350@85Hz */ >> { DRM_MODE("640x350", DRM_MODE_TYPE_DRIVER, 31500, 640, 672, >> 736, 832, 0, 350, 382, 385, 445, 0, >> - DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC) }, >> + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC, 0) }, > >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h >> index 9779ea1..07c0d58 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -30,6 +30,7 @@ >> #include >> #include >> #include >> +#include >> #include >> = >> #include >> @@ -115,12 +116,14 @@ enum drm_mode_status { >> #define DRM_MODE_TYPE_CLOCK_CRTC_C (DRM_MODE_TYPE_CLOCK_C | \ >> DRM_MODE_TYPE_CRTC_C) >> = >> -#define DRM_MODE(nm, t, c, hd, hss, hse, ht, hsk, vd, vss, vse, vt, vs,= f) \ >> +#define DRM_MODE(nm, t, c, hd, hss, hse, ht, hsk, vd, vss, vse, vt, vs,= f, \ >> + ar) \ >> .name =3D nm, .status =3D 0, .type =3D (t), .clock =3D (c), \ >> .hdisplay =3D (hd), .hsync_start =3D (hss), .hsync_end =3D (hse), \ >> .htotal =3D (ht), .hskew =3D (hsk), .vdisplay =3D (vd), \ >> .vsync_start =3D (vss), .vsync_end =3D (vse), .vtotal =3D (vt), \ >> .vscan =3D (vs), .flags =3D (f), \ >> + .picture_aspect_ratio =3D (ar), \ >> .base.type =3D DRM_MODE_OBJECT_MODE >> = >> #define CRTC_INTERLACE_HALVE_V 0x1 /* halve V values for interlacing */ >> @@ -177,6 +180,8 @@ struct drm_display_mode { >> = >> int vrefresh; /* in Hz */ >> int hsync; /* in kHz */ >> + >> + enum hdmi_picture_aspect picture_aspect_ratio; >> }; > = > I'm not sure we want to bloat drm_display_mode with additional > junk for the sake of CEA modes. We could perhaps just have a > separate VIC indexed array for the aspect ratio information. > = > At the very least we don't want to modify DRM_MODE() since that makes > this patch needlessly large, and makes DRM_MODE() even harder to decode > for humans. Instead you can just use c99 initializers and do it like so: > = > { DRM_MODE(...), > .picture_aspect_ratio =3D x }, > = >> = >> enum drm_connector_status { >> -- = >> 1.7.9.5 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > = I am working on making changes such that cea_modes looks like { DRM_MODE(...), .picture_aspect_ratio =3D x }, Will push the patch soon. Please let me know if there are any concerns. -Vandana