From patchwork Wed Apr 26 11:04:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jose Abreu X-Patchwork-Id: 9702229 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 13559603F8 for ; Thu, 27 Apr 2017 00:07:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 039851FF15 for ; Thu, 27 Apr 2017 00:07:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EB956283FE; Thu, 27 Apr 2017 00:07:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3BD981FF15 for ; Thu, 27 Apr 2017 00:07:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CE9756E627; Thu, 27 Apr 2017 00:07:01 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtprelay.synopsys.com (us01smtprelay-2.synopsys.com [198.182.47.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id D5EEF6E3F0 for ; Wed, 26 Apr 2017 11:04:50 +0000 (UTC) Received: from mailhost.synopsys.com (mailhost1.synopsys.com [10.12.238.239]) by smtprelay.synopsys.com (Postfix) with ESMTP id 85E7D24E1F47; Wed, 26 Apr 2017 04:04:50 -0700 (PDT) Received: from mailhost.synopsys.com (localhost [127.0.0.1]) by mailhost.synopsys.com (Postfix) with ESMTP id 65F41F83; Wed, 26 Apr 2017 04:04:50 -0700 (PDT) Received: from US01WEHTC3.internal.synopsys.com (us01wehtc3.internal.synopsys.com [10.15.84.232]) by mailhost.synopsys.com (Postfix) with ESMTP id 38EB9E93; Wed, 26 Apr 2017 04:04:42 -0700 (PDT) Received: from DE02WEHTCA.internal.synopsys.com (10.225.19.92) by US01WEHTC3.internal.synopsys.com (10.15.84.232) with Microsoft SMTP Server (TLS) id 14.3.266.1; Wed, 26 Apr 2017 04:04:42 -0700 Received: from DE02WEHTCB.internal.synopsys.com (10.225.19.94) by DE02WEHTCA.internal.synopsys.com (10.225.19.92) with Microsoft SMTP Server (TLS) id 14.3.266.1; Wed, 26 Apr 2017 13:04:39 +0200 Received: from [10.0.2.15] (10.107.19.44) by DE02WEHTCB.internal.synopsys.com (10.225.19.80) with Microsoft SMTP Server (TLS) id 14.3.266.1; Wed, 26 Apr 2017 13:04:39 +0200 Subject: Re: [PATCH v5 07/10] drm: bridge: dw-hdmi: Add support for custom PHY configuration To: , References: <20170303172007.26541-1-laurent.pinchart+renesas@ideasonboard.com> <20170303172007.26541-8-laurent.pinchart+renesas@ideasonboard.com> <4f1a49ee-dd29-e60c-4b92-788a355d3e88@rock-chips.com> From: Jose Abreu Message-ID: Date: Wed, 26 Apr 2017 12:04:38 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <4f1a49ee-dd29-e60c-4b92-788a355d3e88@rock-chips.com> X-Originating-IP: [10.107.19.44] X-Mailman-Approved-At: Thu, 27 Apr 2017 00:07:00 +0000 Cc: =?UTF-8?B?5p2o5oKm5Lmm?= , =?UTF-8?B?6JSh5p6r?= , "xhc@rock-chips.com" 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-Virus-Scanned: ClamAV using ClamSMTP Hi, On 24-04-2017 08:46, 郑阳 wrote: > Hi, Laurent Pinchart: > > 在 2017年03月04日 01:20, Laurent Pinchart 写道: >> From: Kieran Bingham >> >> The DWC HDMI TX controller interfaces with a companion PHY. While >> Synopsys provides multiple standard PHYs, SoC vendors can also >> integrate >> a custom PHY. >> >> Modularize PHY configuration to support vendor PHYs through >> platform >> data. The existing PHY configuration code was originally >> written to >> support the DWC HDMI 3D TX PHY, and seems to be compatible >> with the DWC >> MLP PHY. The HDMI 2.0 PHY will require a separate configuration >> function. >> >> Signed-off-by: Kieran Bingham >> >> Signed-off-by: Laurent Pinchart >> >> Tested-by: Neil Armstrong >> Reviewed-by: Jose Abreu >> --- >> Changes since v1: >> >> - Check pdata->phy_configure in hdmi_phy_configure() to avoid >> dereferencing NULL pointer. >> --- >> drivers/gpu/drm/bridge/dw-hdmi.c | 109 >> ++++++++++++++++++++++++++------------- >> include/drm/bridge/dw_hdmi.h | 7 +++ >> 2 files changed, 81 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c >> b/drivers/gpu/drm/bridge/dw-hdmi.c >> index cb2703862be2..b835d81bb471 100644 >> --- a/drivers/gpu/drm/bridge/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c >> @@ -118,6 +118,9 @@ struct dw_hdmi_phy_data { >> const char *name; >> unsigned int gen; >> bool has_svsret; >> + int (*configure)(struct dw_hdmi *hdmi, >> + const struct dw_hdmi_plat_data *pdata, >> + unsigned long mpixelclock); >> }; >> struct dw_hdmi { >> @@ -860,8 +863,8 @@ static bool hdmi_phy_wait_i2c_done(struct >> dw_hdmi *hdmi, int msec) >> return true; >> } >> -static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, >> unsigned short data, >> - unsigned char addr) >> +void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned >> short data, >> + unsigned char addr) >> { >> hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0); >> hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR); >> @@ -873,6 +876,7 @@ static void hdmi_phy_i2c_write(struct >> dw_hdmi *hdmi, unsigned short data, >> HDMI_PHY_I2CM_OPERATION_ADDR); >> hdmi_phy_wait_i2c_done(hdmi, 1000); >> } >> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write); >> static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi >> *hdmi, bool enable) >> { >> @@ -993,37 +997,67 @@ static int dw_hdmi_phy_power_on(struct >> dw_hdmi *hdmi) >> return 0; >> } >> -static int hdmi_phy_configure(struct dw_hdmi *hdmi) >> +/* >> + * PHY configuration function for the DWC HDMI 3D TX PHY. >> Based on the available >> + * information the DWC MHL PHY has the same register layout >> and is thus also >> + * supported by this function. >> + */ >> +static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct dw_hdmi >> *hdmi, >> + const struct dw_hdmi_plat_data *pdata, >> + unsigned long mpixelclock) >> { >> - const struct dw_hdmi_phy_data *phy = hdmi->phy.data; >> - const struct dw_hdmi_plat_data *pdata = hdmi->plat_data; >> const struct dw_hdmi_mpll_config *mpll_config = >> pdata->mpll_cfg; >> const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr; >> const struct dw_hdmi_phy_config *phy_config = >> pdata->phy_config; >> /* PLL/MPLL Cfg - always match on final entry */ >> for (; mpll_config->mpixelclock != ~0UL; mpll_config++) >> - if (hdmi->hdmi_data.video_mode.mpixelclock <= >> - mpll_config->mpixelclock) >> + if (mpixelclock <= mpll_config->mpixelclock) >> break; >> for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++) >> - if (hdmi->hdmi_data.video_mode.mpixelclock <= >> - curr_ctrl->mpixelclock) >> + if (mpixelclock <= curr_ctrl->mpixelclock) >> break; >> for (; phy_config->mpixelclock != ~0UL; phy_config++) >> - if (hdmi->hdmi_data.video_mode.mpixelclock <= >> - phy_config->mpixelclock) >> + if (mpixelclock <= phy_config->mpixelclock) >> break; >> if (mpll_config->mpixelclock == ~0UL || >> curr_ctrl->mpixelclock == ~0UL || >> - phy_config->mpixelclock == ~0UL) { >> - dev_err(hdmi->dev, "Pixel clock %d - unsupported by >> HDMI\n", >> - hdmi->hdmi_data.video_mode.mpixelclock); >> + phy_config->mpixelclock == ~0UL) >> return -EINVAL; >> - } >> + >> + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce, >> + HDMI_3D_TX_PHY_CPCE_CTRL); >> + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp, >> + HDMI_3D_TX_PHY_GMPCTRL); >> + dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0], >> + HDMI_3D_TX_PHY_CURRCTRL); >> + >> + dw_hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL); >> + dw_hdmi_phy_i2c_write(hdmi, >> HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK, >> + HDMI_3D_TX_PHY_MSM_CTRL); >> + >> + dw_hdmi_phy_i2c_write(hdmi, phy_config->term, >> HDMI_3D_TX_PHY_TXTERM); >> + dw_hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr, >> + HDMI_3D_TX_PHY_CKSYMTXCTRL); >> + dw_hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr, >> + HDMI_3D_TX_PHY_VLEVCTRL); >> + >> + /* Override and disable clock termination. */ >> + dw_hdmi_phy_i2c_write(hdmi, >> HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE, >> + HDMI_3D_TX_PHY_CKCALCTRL); >> + >> + return 0; >> +} >> + >> +static int hdmi_phy_configure(struct dw_hdmi *hdmi) >> +{ >> + const struct dw_hdmi_phy_data *phy = hdmi->phy.data; >> + const struct dw_hdmi_plat_data *pdata = hdmi->plat_data; >> + unsigned long mpixelclock = >> hdmi->hdmi_data.video_mode.mpixelclock; >> + int ret; >> dw_hdmi_phy_power_off(hdmi); >> @@ -1042,26 +1076,16 @@ static int hdmi_phy_configure(struct >> dw_hdmi *hdmi) >> HDMI_PHY_I2CM_SLAVE_ADDR); >> hdmi_phy_test_clear(hdmi, 0); >> - hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce, >> - HDMI_3D_TX_PHY_CPCE_CTRL); >> - hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp, >> - HDMI_3D_TX_PHY_GMPCTRL); >> - hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0], >> - HDMI_3D_TX_PHY_CURRCTRL); >> - >> - hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL); >> - hdmi_phy_i2c_write(hdmi, >> HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK, >> - HDMI_3D_TX_PHY_MSM_CTRL); >> - >> - hdmi_phy_i2c_write(hdmi, phy_config->term, >> HDMI_3D_TX_PHY_TXTERM); >> - hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr, >> - HDMI_3D_TX_PHY_CKSYMTXCTRL); >> - hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr, >> - HDMI_3D_TX_PHY_VLEVCTRL); >> - >> - /* Override and disable clock termination. */ >> - hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE, >> - HDMI_3D_TX_PHY_CKCALCTRL); >> + /* Write to the PHY as configured by the platform */ >> + if (pdata->configure_phy) >> + ret = pdata->configure_phy(hdmi, pdata, mpixelclock); >> + else >> + ret = phy->configure(hdmi, pdata, mpixelclock); >> + if (ret) { >> + dev_err(hdmi->dev, "PHY configuration failed (clock >> %lu)\n", >> + mpixelclock); >> + return ret; >> + } >> return dw_hdmi_phy_power_on(hdmi); >> } >> @@ -1895,24 +1919,31 @@ static const struct dw_hdmi_phy_data >> dw_hdmi_phys[] = { >> .name = "DWC MHL PHY + HEAC PHY", >> .gen = 2, >> .has_svsret = true, >> + .configure = hdmi_phy_configure_dwc_hdmi_3d_tx, >> }, { >> .type = DW_HDMI_PHY_DWC_MHL_PHY, >> .name = "DWC MHL PHY", >> .gen = 2, >> .has_svsret = true, >> + .configure = hdmi_phy_configure_dwc_hdmi_3d_tx, >> }, { >> .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC, >> .name = "DWC HDMI 3D TX PHY + HEAC PHY", >> .gen = 2, >> + .configure = hdmi_phy_configure_dwc_hdmi_3d_tx, >> }, { >> .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY, >> .name = "DWC HDMI 3D TX PHY", >> .gen = 2, >> + .configure = hdmi_phy_configure_dwc_hdmi_3d_tx, >> }, { >> .type = DW_HDMI_PHY_DWC_HDMI20_TX_PHY, >> .name = "DWC HDMI 2.0 TX PHY", >> .gen = 2, >> .has_svsret = true, > After this commit, the dw-hdmi on RK3368/RK3399 run into the > "DWC HDMI 2.0 TX PHY requires platform support error". > the phy-type of RK3368/RK3399 is 0xf3, and phy register layout is > compatible with DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC. > Is here missing a default phy configure function? If this has the same register layout then this patch should solve the problem: ---------------------------------------------------------------- .name = "Vendor PHY", ---------------------------------------------------------------- Please test and let me know if its ok. This shouldn't impact other platforms which supply custom configuration function by pdata as the check for pdata configure() is done before checking the internal configure(). Best regards, Jose Miguel Abreu >> + }, { >> + .type = DW_HDMI_PHY_VENDOR_PHY, >> + .name = "Vendor PHY", >> } >> }; >> @@ -1943,6 +1974,14 @@ static int dw_hdmi_detect_phy(struct >> dw_hdmi *hdmi) >> hdmi->phy.ops = &dw_hdmi_synopsys_phy_ops; >> hdmi->phy.name = dw_hdmi_phys[i].name; >> hdmi->phy.data = (void *)&dw_hdmi_phys[i]; >> + >> + if (!dw_hdmi_phys[i].configure && >> + !hdmi->plat_data->configure_phy) { >> + dev_err(hdmi->dev, "%s requires platform >> support\n", >> + hdmi->phy.name); >> + return -ENODEV; >> + } >> + >> return 0; >> } >> } >> diff --git a/include/drm/bridge/dw_hdmi.h >> b/include/drm/bridge/dw_hdmi.h >> index 0f583ca7e66e..dd330259a3dc 100644 >> --- a/include/drm/bridge/dw_hdmi.h >> +++ b/include/drm/bridge/dw_hdmi.h >> @@ -78,6 +78,9 @@ struct dw_hdmi_plat_data { >> const struct dw_hdmi_mpll_config *mpll_cfg; >> const struct dw_hdmi_curr_ctrl *cur_ctr; >> const struct dw_hdmi_phy_config *phy_config; >> + int (*configure_phy)(struct dw_hdmi *hdmi, >> + const struct dw_hdmi_plat_data *pdata, >> + unsigned long mpixelclock); >> }; >> int dw_hdmi_probe(struct platform_device *pdev, >> @@ -91,4 +94,8 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi >> *hdmi, unsigned int rate); >> void dw_hdmi_audio_enable(struct dw_hdmi *hdmi); >> void dw_hdmi_audio_disable(struct dw_hdmi *hdmi); >> +/* PHY configuration */ >> +void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned >> short data, >> + unsigned char addr); >> + >> #endif /* __IMX_HDMI_H__ */ > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=cjy3Og8_l5PxS4qa2_RekJB_Bi_J4sVXR1zH24rY7ng&s=bH0i9klWoHnYe5d9wM4Ei2u_2bkGUeZJZV5J7BsBCqE&e= diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 4e1f54a..ff96864 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2159,6 +2159,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) .name = "DWC HDMI 2.0 TX PHY", .gen = 2, .has_svsret = true, + .configure = hdmi_phy_configure_dwc_hdmi_3d_tx, }, { .type = DW_HDMI_PHY_VENDOR_PHY,