From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH] drm/exynos: set the active aspect ratio as per mode Date: Wed, 12 Mar 2014 16:19:10 +0100 Message-ID: <53207AEE.7030202@samsung.com> References: <1394423086-21383-1-git-send-email-s.shirish@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by gabe.freedesktop.org (Postfix) with ESMTP id 18671FA97E for ; Wed, 12 Mar 2014 08:19:29 -0700 (PDT) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0N2B003A6X8D9C20@mailout1.w1.samsung.com> for dri-devel@lists.freedesktop.org; Wed, 12 Mar 2014 15:19:25 +0000 (GMT) In-reply-to: <1394423086-21383-1-git-send-email-s.shirish@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Shirish S , dri-devel@lists.freedesktop.org, inki.dae@samsung.com Cc: shirish@chromium.org List-Id: dri-devel@lists.freedesktop.org Hi Shirish, On 10.03.2014 04:44, Shirish S wrote: > now that the drm_display_mode also provides aspect > ratio for all resolutions, this patch adds its usage > to set the active aspect ratio of AVI info frame > packets as per CEA-861-D standard's Table 9. > > This is also needed to abide by the 7-27 > compliance test of HDMI. > > Signed-off-by: Shirish S > --- > drivers/gpu/drm/exynos/exynos_hdmi.c | 33 +++++++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c > index c021ddc..8aca52a 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -53,8 +53,6 @@ > /* AVI header and aspect ratio */ > #define HDMI_AVI_VERSION 0x02 > #define HDMI_AVI_LENGTH 0x0D > -#define AVI_PIC_ASPECT_RATIO_16_9 (2 << 4) > -#define AVI_SAME_AS_PIC_ASPECT_RATIO 8 > > /* AUI header info */ > #define HDMI_AUI_VERSION 0x01 > @@ -65,6 +63,12 @@ enum hdmi_type { > HDMI_TYPE14, > }; > > +enum active_aspect_ratio { > + AVI_SAME_AS_PIC_ASPECT_RATIO = 8, > + AVI_4_3_Center_RATIO, > + AVI_16_9_Center_RATIO, > +}; > + CodingStyle: Please define these using preprocessor macros, since they are bitfield values. Also coding style implies using uppercase for constants. > struct hdmi_resources { > struct clk *hdmi; > struct clk *sclk_hdmi; > @@ -162,6 +166,7 @@ struct hdmi_v14_conf { > struct hdmi_conf_regs { > int pixel_clock; > int cea_video_id; > + enum hdmi_picture_aspect aspect_ratio; > union { > struct hdmi_v13_conf v13_conf; > struct hdmi_v14_conf v14_conf; > @@ -668,7 +673,6 @@ static void hdmi_reg_infoframe(struct hdmi_context *hdata, > { > u32 hdr_sum; > u8 chksum; > - u32 aspect_ratio; > u32 mod; > u32 vic; > > @@ -697,10 +701,25 @@ static void hdmi_reg_infoframe(struct hdmi_context *hdata, > AVI_ACTIVE_FORMAT_VALID | > AVI_UNDERSCANNED_DISPLAY_VALID); > > - aspect_ratio = AVI_PIC_ASPECT_RATIO_16_9; > - > - hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), aspect_ratio | > + /* > + * Set the aspect ratio as per the mode, mentioned in > + * Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D Standard > + */ > + switch (hdata->mode_conf.aspect_ratio) { > + case HDMI_PICTURE_ASPECT_4_3: > + hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), aspect_ratio | > + AVI_4_3_Center_RATIO); CodingStyle: Please keep wrapped function calls aligned using tabs at least to the opening parenthesis. > + break; > + case HDMI_PICTURE_ASPECT_16_9: > + hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), aspect_ratio | > + AVI_16_9_Center_RATIO); Ditto. > + break; > + case HDMI_PICTURE_ASPECT_NONE: > + default: > + hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(2), aspect_ratio | > AVI_SAME_AS_PIC_ASPECT_RATIO); Ditto. Best regards, Tomasz