From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/3] [VPG HSW-A] drm/i915: Add aspect ratio in drm_display_mode Date: Thu, 15 Aug 2013 10:06:42 +0300 Message-ID: <20130815070642.GC7159@intel.com> References: <1376542743-25845-1-git-send-email-vandana.kannan@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id D8E3FE5C5D for ; Thu, 15 Aug 2013 00:06:45 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1376542743-25845-1-git-send-email-vandana.kannan@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: vandana.kannan@intel.com Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 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_mode= s[] =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 -- = Ville Syrj=E4l=E4 Intel OTC