From patchwork Tue Mar 31 09:02:20 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yakir Yang X-Patchwork-Id: 6137131 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id B59649F54E for ; Wed, 1 Apr 2015 02:03:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 37B74201FE for ; Wed, 1 Apr 2015 02:03:03 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 84C92201BC for ; Wed, 1 Apr 2015 02:03:00 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A4C49720DF; Tue, 31 Mar 2015 19:02:53 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from regular1.263xmail.com (regular1.263xmail.com [211.150.99.135]) by gabe.freedesktop.org (Postfix) with ESMTP id 723216E0EC for ; Tue, 31 Mar 2015 02:01:49 -0700 (PDT) Received: from ykk?rock-chips.com (unknown [192.168.167.87]) by regular1.263xmail.com (Postfix) with SMTP id EC41C19815; Tue, 31 Mar 2015 17:01:34 +0800 (CST) X-263anti-spam: KSV:0;BIG:0;ABS:1;DNS:0;ATT:0;SPF:S; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ADDR-CHECKED: 0 Received: from [192.168.2.106] (localhost.localdomain [127.0.0.1]) by smtp.263.net (Postfix) with ESMTP id 436484570; Tue, 31 Mar 2015 17:01:34 +0800 (CST) X-RL-SENDER: ykk@rock-chips.com X-FST-TO: airlied@linux.ie X-SENDER-IP: 103.46.141.26 X-LOGIN-NAME: ykk@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-SENDER: ykk@rock-chips.com X-DNS-TYPE: 0 Received: from [192.168.2.106] (unknown [103.46.141.26]) by smtp.263.net (Postfix) whith ESMTP id 18152QU3UC8; Tue, 31 Mar 2015 17:01:36 +0800 (CST) Message-ID: <551A629C.5010306@rock-chips.com> Date: Tue, 31 Mar 2015 05:02:20 -0400 From: Yang Kuankuan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Russell King , alsa-devel@alsa-project.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH RFC 02/11] drm: bridge/dw_hdmi: use drm_hdmi_avi_infoframe_from_display_mode() References: <20150330193911.GM24899@n2100.arm.linux.org.uk> In-Reply-To: X-Mailman-Approved-At: Tue, 31 Mar 2015 19:02:52 -0700 Cc: Fabio Estevam , Mark Brown X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,HTML_MESSAGE, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Russell, On 03/30/2015 03:40 PM, Russell King wrote: > Use drm_hdmi_avi_infoframe_from_display_mode() to compose the AVI > frame. Very interesting, I am also preparing the drm_hdmi_avi_infoframe_from_display_mode patches to upstream, seems it is unnecessary now :) Besides, as you are going an dw_hdmi cleanups, I want to point another bugs that relate to the HDMI CTS test. There are somethings wrong with General Control Pack, as for now the encoder color depth is 8-bit packing mode, so the color depth only support 24 bits per pixel video, In this case the CD filed in GCP should set to "Color Depth Not indicated". In the end we should keep the *csc_color_depth(HDMI_CSC_SCALE)* & *color_depth(HDMI_VP_PR_CD)* to zero, code should modify like this GCP would test pass: } else if (hdmi_data->enc_color_depth == 10) { color_depth = 5; Best regards. Yakir Yang > Signed-off-by: Russell King > --- > drivers/gpu/drm/bridge/dw_hdmi.c | 126 +++++++++++++++++++++------------------ > 1 file changed, 67 insertions(+), 59 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c > index 49df6c8c4ea8..44c63883db19 100644 > --- a/drivers/gpu/drm/bridge/dw_hdmi.c > +++ b/drivers/gpu/drm/bridge/dw_hdmi.c > @@ -919,74 +919,79 @@ static void hdmi_tx_hdcp_config(struct dw_hdmi *hdmi) > HDMI_A_HDCPCFG1_ENCRYPTIONDISABLE_MASK, HDMI_A_HDCPCFG1); > } > > -static void hdmi_config_AVI(struct dw_hdmi *hdmi) > +static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode) > { > - u8 val, pix_fmt, under_scan; > - u8 act_ratio, coded_ratio, colorimetry, ext_colorimetry; > - bool aspect_16_9; > + struct hdmi_avi_infoframe frame; > + u8 val; > > - aspect_16_9 = false; /* FIXME */ > + /* Initialise info frame from DRM mode */ > + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); > > - /* AVI Data Byte 1 */ > if (hdmi->hdmi_data.enc_out_format == YCBCR444) > - pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_YCBCR444; > + frame.colorspace = HDMI_COLORSPACE_YUV444; > else if (hdmi->hdmi_data.enc_out_format == YCBCR422_8BITS) > - pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_YCBCR422; > + frame.colorspace = HDMI_COLORSPACE_YUV422; > else > - pix_fmt = HDMI_FC_AVICONF0_PIX_FMT_RGB; > - > - under_scan = HDMI_FC_AVICONF0_SCAN_INFO_NODATA; > - > - /* > - * Active format identification data is present in the AVI InfoFrame. > - * Under scan info, no bar data > - */ > - val = pix_fmt | under_scan | > - HDMI_FC_AVICONF0_ACTIVE_FMT_INFO_PRESENT | > - HDMI_FC_AVICONF0_BAR_DATA_NO_DATA; > - > - hdmi_writeb(hdmi, val, HDMI_FC_AVICONF0); > - > - /* AVI Data Byte 2 -Set the Aspect Ratio */ > - if (aspect_16_9) { > - act_ratio = HDMI_FC_AVICONF1_ACTIVE_ASPECT_RATIO_16_9; > - coded_ratio = HDMI_FC_AVICONF1_CODED_ASPECT_RATIO_16_9; > - } else { > - act_ratio = HDMI_FC_AVICONF1_ACTIVE_ASPECT_RATIO_4_3; > - coded_ratio = HDMI_FC_AVICONF1_CODED_ASPECT_RATIO_4_3; > - } > + frame.colorspace = HDMI_COLORSPACE_RGB; > > /* Set up colorimetry */ > if (hdmi->hdmi_data.enc_out_format == XVYCC444) { > - colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_EXTENDED_INFO; > + frame.colorimetry = HDMI_COLORIMETRY_EXTENDED; > if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601) > - ext_colorimetry = > - HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601; > + frame.extended_colorimetry = > + HDMI_EXTENDED_COLORIMETRY_XV_YCC_601; > else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/ > - ext_colorimetry = > - HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC709; > + frame.extended_colorimetry = > + HDMI_EXTENDED_COLORIMETRY_XV_YCC_709; > } else if (hdmi->hdmi_data.enc_out_format != RGB) { > if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601) > - colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_SMPTE; > + frame.colorimetry = HDMI_COLORIMETRY_ITU_601; > else /*hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_709*/ > - colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_ITUR; > - ext_colorimetry = HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601; > + frame.colorimetry = HDMI_COLORIMETRY_ITU_709; > + frame.extended_colorimetry = HDMI_EXTENDED_COLORIMETRY_XV_YCC_601; > } else { /* Carries no data */ > - colorimetry = HDMI_FC_AVICONF1_COLORIMETRY_NO_DATA; > - ext_colorimetry = HDMI_FC_AVICONF2_EXT_COLORIMETRY_XVYCC601; > + frame.colorimetry = HDMI_COLORIMETRY_NONE; > + frame.extended_colorimetry = HDMI_EXTENDED_COLORIMETRY_XV_YCC_601; > } > > - val = colorimetry | coded_ratio | act_ratio; > + frame.scan_mode = HDMI_SCAN_MODE_NONE; > + > + /* > + * The Designware IP uses a different byte format from standard > + * AVI info frames, though generally the bits are in the correct > + * bytes. > + */ > + > + /* > + * AVI data byte 1 differences: Colorspace in bits 4,5 rather than 5,6, > + * active aspect present in bit 6 rather than 4. > + */ > + val = (frame.colorspace & 3) << 4 | (frame.scan_mode & 0x3); > + if (frame.active_aspect & 15) > + val |= HDMI_FC_AVICONF0_ACTIVE_FMT_INFO_PRESENT; > + if (frame.top_bar || frame.bottom_bar) > + val |= HDMI_FC_AVICONF0_BAR_DATA_HORIZ_BAR; > + if (frame.left_bar || frame.right_bar) > + val |= HDMI_FC_AVICONF0_BAR_DATA_VERT_BAR; > + hdmi_writeb(hdmi, val, HDMI_FC_AVICONF0); > + > + /* AVI data byte 2 differences: none */ > + val = ((frame.colorimetry & 0x3) << 6) | > + ((frame.picture_aspect & 0x3) << 4) | > + (frame.active_aspect & 0xf); > hdmi_writeb(hdmi, val, HDMI_FC_AVICONF1); > > - /* AVI Data Byte 3 */ > - val = HDMI_FC_AVICONF2_IT_CONTENT_NO_DATA | ext_colorimetry | > - HDMI_FC_AVICONF2_RGB_QUANT_DEFAULT | > - HDMI_FC_AVICONF2_SCALING_NONE; > + /* AVI data byte 3 differences: none */ > + val = ((frame.extended_colorimetry & 0x7) << 4) | > + ((frame.quantization_range & 0x3) << 2) | > + (frame.nups & 0x3); > + if (frame.itc) > + val |= HDMI_FC_AVICONF2_IT_CONTENT_VALID; > hdmi_writeb(hdmi, val, HDMI_FC_AVICONF2); > > - /* AVI Data Byte 4 */ > - hdmi_writeb(hdmi, hdmi->vic, HDMI_FC_AVIVID); > + /* AVI data byte 4 differences: none */ > + val = frame.video_code & 0x7f; > + hdmi_writeb(hdmi, val, HDMI_FC_AVIVID); > > /* AVI Data Byte 5- set up input and output pixel repetition */ > val = (((hdmi->hdmi_data.video_mode.mpixelrepetitioninput + 1) << > @@ -997,20 +1002,23 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi) > HDMI_FC_PRCONF_OUTPUT_PR_FACTOR_MASK); > hdmi_writeb(hdmi, val, HDMI_FC_PRCONF); > > - /* IT Content and quantization range = don't care */ > - val = HDMI_FC_AVICONF3_IT_CONTENT_TYPE_GRAPHICS | > - HDMI_FC_AVICONF3_QUANT_RANGE_LIMITED; > + /* > + * AVI data byte 5 differences: content type in 0,1 rather than 4,5, > + * ycc range in bits 2,3 rather than 6,7 > + */ > + val = ((frame.ycc_quantization_range & 0x3) << 2) | > + (frame.content_type & 0x3); > hdmi_writeb(hdmi, val, HDMI_FC_AVICONF3); > > /* AVI Data Bytes 6-13 */ > - hdmi_writeb(hdmi, 0, HDMI_FC_AVIETB0); > - hdmi_writeb(hdmi, 0, HDMI_FC_AVIETB1); > - hdmi_writeb(hdmi, 0, HDMI_FC_AVISBB0); > - hdmi_writeb(hdmi, 0, HDMI_FC_AVISBB1); > - hdmi_writeb(hdmi, 0, HDMI_FC_AVIELB0); > - hdmi_writeb(hdmi, 0, HDMI_FC_AVIELB1); > - hdmi_writeb(hdmi, 0, HDMI_FC_AVISRB0); > - hdmi_writeb(hdmi, 0, HDMI_FC_AVISRB1); > + hdmi_writeb(hdmi, frame.top_bar & 0xff, HDMI_FC_AVIETB0); > + hdmi_writeb(hdmi, (frame.top_bar >> 8) & 0xff, HDMI_FC_AVIETB1); > + hdmi_writeb(hdmi, frame.bottom_bar & 0xff, HDMI_FC_AVISBB0); > + hdmi_writeb(hdmi, (frame.bottom_bar >> 8) & 0xff, HDMI_FC_AVISBB1); > + hdmi_writeb(hdmi, frame.left_bar & 0xff, HDMI_FC_AVIELB0); > + hdmi_writeb(hdmi, (frame.left_bar >> 8) & 0xff, HDMI_FC_AVIELB1); > + hdmi_writeb(hdmi, frame.right_bar & 0xff, HDMI_FC_AVISRB0); > + hdmi_writeb(hdmi, (frame.right_bar >> 8) & 0xff, HDMI_FC_AVISRB1); > } > > static void hdmi_av_composer(struct dw_hdmi *hdmi, > @@ -1244,7 +1252,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode) > hdmi_enable_audio_clk(hdmi); > > /* HDMI Initialization Step F - Configure AVI InfoFrame */ > - hdmi_config_AVI(hdmi); > + hdmi_config_AVI(hdmi, mode); > } > > hdmi_video_packetize(hdmi); diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index bf5be679..c5d9b16 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -695,7 +695,7 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi) if (!hdmi_data->enc_color_depth) { output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS; } else if (hdmi_data->enc_color_depth == 8) { - color_depth = 4; + color_depth = 0; output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;