Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754648AbdGJUm3 (ORCPT ); Mon, 10 Jul 2017 16:42:29 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:53929 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752158AbdGJUm2 (ORCPT ); Mon, 10 Jul 2017 16:42:28 -0400 From: Laurent Pinchart To: Jose Abreu Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Kieran Bingham , Laurent Pinchart , Archit Taneja , Andrzej Hajda , Mark Yao , Carlos Palminha , Heiko =?ISO-8859-1?Q?St=FCbner?= Subject: Re: [PATCH v2] drm: bridge: synopsys/dw-hdmi: Provide default configuration function for HDMI 2.0 PHY Date: Mon, 10 Jul 2017 23:42:30 +0300 Message-ID: <3742691.WXjVyA7RAG@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.34-gentoo; KDE/4.14.32; x86_64; ; ) In-Reply-To: <185ccf7d4473fa557044732402ca20b3d4007952.1498209896.git.joabreu@synopsys.com> References: <185ccf7d4473fa557044732402ca20b3d4007952.1498209896.git.joabreu@synopsys.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v6AKgXde013747 Content-Length: 3839 Lines: 96 Hi Jose, On Friday 23 Jun 2017 10:36:44 Jose Abreu wrote: > Currently HDMI 2.0 PHYs do not have a default configuration function. > > As *some* of the HDMI 2.0 PHYs have the same register layout as the 3D > PHYs we can provide the same default configuration function for both > and still let user overwrite this with custom configuration function > if needed. > > If, for some reason, the PHY is custom or has a register different > register layout then custom configuration function *must* be provided > in order for the system to work correctly. As we prefer the pdata > provided configuration function over the internal one this change > will not make any impact in custom platforms. > > This patch is based on today's drm-misc-next branch. > > Signed-off-by: Jose Abreu > Tested-by: Mark Yao > Cc: Kieran Bingham > Cc: Laurent Pinchart > Cc: Archit Taneja > Cc: Andrzej Hajda > Cc: Mark Yao > Cc: Carlos Palminha > Cc: Heiko St?bner > > Changes in v2: > - Rebased and refrased commit message > --- > > Hi All, > > There as been a little confusion about dw-hdmi phys so I will expand a > little bit here so that I can base my decision about this patch and why > does it only works in some platforms. > > First, if you read dw-hdmi.c code, you will see that there is an > identification register for the phy type being used. Unfortunatelly, this > only states the phy type and not the phy version. > > Second, we have many HDMI 2.0 phys (so, same phy type: 0xf3) but, as you may > have guessed, HW team decided to change regbank between some versions. > > Third and last, each phy in a SoC has unique characteristics, so each phy > (event if they are the same version) will have different PLL configuration > parameters. > > Given all this I managed to conclude that Mark's phy is still an HDMI 2.0 > phy but with the same register layout as previous 3D PHY's. I found at > least 2 phys with the same register layout and only 1 phy which has a > different layout, so I think majority wins here and we should let the > default configuration function for HDMI 2.0 phys be the same one as the 3D. > > Short story: There is no way to correctly identify, at runtime, the phy > version being used by the controller so we can't provide a default > configuration function. :'-( > We can, however assume that most of the HDMI 2.0 phys will have the 3D > layout BUT each developer must confirm that the layout in its SoC is the > expected one and if not, provide a custom configuration function. This seems fair to me, so Reviewed-by: Laurent Pinchart However, if the PHY version can't be identified at runtime, I wonder whether it would make sense to add it to the PHY DT node. The driver could then pick the right configuration function accordingly. SoC-specific parameters (such as PLL parameters) would still be passed through platform glue code, but all the rest could be moved to the core driver. > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead1124..10c8d8c 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -2170,6 +2170,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, > .name = "Vendor PHY", -- Regards, Laurent Pinchart