Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756490AbdCUHw0 (ORCPT ); Tue, 21 Mar 2017 03:52:26 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:49802 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755783AbdCUHvq (ORCPT ); Tue, 21 Mar 2017 03:51:46 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 094A7609F7 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=architt@codeaurora.org Subject: Re: [PATCH] drm: bridge: dw-hdmi: add HDMI vendor specific infoframe config To: Andrzej Hajda , Nickey Yang References: <1489990249-29152-1-git-send-email-nickey.yang@rock-chips.com> <9cf73d37-78bb-dab8-9c48-5a951fb41c75@samsung.com> Cc: airlied@linux.ie, laurent.pinchart+renesas@ideasonboard.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, dianders@chromium.org, linux-rockchip@lists.infradead.org, joabreu@synopsys.com, rmk+kernel@arm.linux.org.uk, andy.yan@rock-chips.com, vladimir_zapolskiy@mentor.com, zhengyang@rock-chips.com From: Archit Taneja Message-ID: <10305fb6-4777-6a1b-c9c0-e12f76ee1e42@codeaurora.org> Date: Tue, 21 Mar 2017 13:21:37 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <9cf73d37-78bb-dab8-9c48-5a951fb41c75@samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4232 Lines: 123 On 03/21/2017 01:17 PM, Andrzej Hajda wrote: > On 20.03.2017 07:10, Nickey Yang wrote: >> Vendor specific infoframe is mandatory for 4K2K resolution. >> Without this, the HDMI protocol compliance fails. >> >> Signed-off-by: Nickey Yang >> --- >> drivers/gpu/drm/bridge/dw-hdmi.c | 50 ++++++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/bridge/dw-hdmi.h | 4 ++++ >> 2 files changed, 54 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c >> index 9a9ec27..79e2e48 100644 >> --- a/drivers/gpu/drm/bridge/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c >> @@ -1195,6 +1195,55 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode) >> hdmi_writeb(hdmi, (frame.right_bar >> 8) & 0xff, HDMI_FC_AVISRB1); >> } >> >> +static void hdmi_config_vendor_specific_infoframe(struct dw_hdmi *hdmi, >> + struct drm_display_mode *mode) >> +{ >> + struct hdmi_vendor_infoframe frame; >> + u8 buffer[10]; >> + ssize_t err; >> + >> + err = drm_hdmi_vendor_infoframe_from_display_mode(&frame, mode); >> + if (err) { >> + dev_err(hdmi->dev, >> + "Failed to get vendor infoframe from mode: %zd\n", err); >> + return; >> + } >> + >> + err = hdmi_vendor_infoframe_pack(&frame, buffer, sizeof(buffer)); >> + if (!err) { >> + dev_err(hdmi->dev, "Failed to pack vendor infoframe: %zd\n", >> + err); >> + return; >> + } > > AFAIK function returns number of packed bytes or -EINVAL so the check is > incorrect, and -EINVAL means that there is no need to generate vendor > infoframe, so definitely not dev_err. I queued this to drm-misc-next before you posted this. We can have a follow up patch to fix this (since we don't rebase drm-misc). Thanks, Archit > >> + hdmi_modb(hdmi, 0 << HDMI_FC_DATAUTO0_VSD_OFFSET, >> + HDMI_FC_DATAUTO0_VSD_MASK, HDMI_FC_DATAUTO0); > > I see no gain in constructs (0 << HDMI_FC_DATAUTO0_VSD_OFFSET), > additionally mask and offset are redundant (mask == 1 << offset), but it > seems you follow existing practice, so I do not oppose :) > > Regards > Andrzej > >> + >> + /* Set the length of HDMI vendor specific InfoFrame payload */ >> + hdmi_writeb(hdmi, buffer[2], HDMI_FC_VSDSIZE); >> + >> + /* Set 24bit IEEE Registration Identifier */ >> + hdmi_writeb(hdmi, buffer[4], HDMI_FC_VSDIEEEID0); >> + hdmi_writeb(hdmi, buffer[5], HDMI_FC_VSDIEEEID1); >> + hdmi_writeb(hdmi, buffer[6], HDMI_FC_VSDIEEEID2); >> + >> + /* Set HDMI_Video_Format and HDMI_VIC/3D_Structure */ >> + hdmi_writeb(hdmi, buffer[7], HDMI_FC_VSDPAYLOAD0); >> + hdmi_writeb(hdmi, buffer[8], HDMI_FC_VSDPAYLOAD1); >> + >> + if (frame.s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF) >> + hdmi_writeb(hdmi, buffer[9], HDMI_FC_VSDPAYLOAD2); >> + >> + /* Packet frame interpolation */ >> + hdmi_writeb(hdmi, 1, HDMI_FC_DATAUTO1); >> + >> + /* Auto packets per frame and line spacing */ >> + hdmi_writeb(hdmi, 0x11, HDMI_FC_DATAUTO2); >> + >> + /* Configures the Frame Composer On RDRB mode */ >> + hdmi_modb(hdmi, 1 << HDMI_FC_DATAUTO0_VSD_OFFSET, >> + HDMI_FC_DATAUTO0_VSD_MASK, HDMI_FC_DATAUTO0); >> +} >> + >> static void hdmi_av_composer(struct dw_hdmi *hdmi, >> const struct drm_display_mode *mode) >> { >> @@ -1446,6 +1495,7 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode) >> >> /* HDMI Initialization Step F - Configure AVI InfoFrame */ >> hdmi_config_AVI(hdmi, mode); >> + hdmi_config_vendor_specific_infoframe(hdmi, mode); >> } else { >> dev_dbg(hdmi->dev, "%s DVI mode\n", __func__); >> } >> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h b/drivers/gpu/drm/bridge/dw-hdmi.h >> index 325b0b8..c59f87e 100644 >> --- a/drivers/gpu/drm/bridge/dw-hdmi.h >> +++ b/drivers/gpu/drm/bridge/dw-hdmi.h >> @@ -854,6 +854,10 @@ enum { >> HDMI_FC_DBGFORCE_FORCEAUDIO = 0x10, >> HDMI_FC_DBGFORCE_FORCEVIDEO = 0x1, >> >> +/* FC_DATAUTO0 field values */ >> + HDMI_FC_DATAUTO0_VSD_MASK = 0x08, >> + HDMI_FC_DATAUTO0_VSD_OFFSET = 3, >> + >> /* PHY_CONF0 field values */ >> HDMI_PHY_CONF0_PDZ_MASK = 0x80, >> HDMI_PHY_CONF0_PDZ_OFFSET = 7, > > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project