From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart 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> References: <185ccf7d4473fa557044732402ca20b3d4007952.1498209896.git.joabreu@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [IPv6:2001:4b98:dc2:45:216:3eff:febb:480d]) by gabe.freedesktop.org (Postfix) with ESMTPS id D343B6E23F for ; Mon, 10 Jul 2017 20:42:28 +0000 (UTC) In-Reply-To: <185ccf7d4473fa557044732402ca20b3d4007952.1498209896.git.joabreu@synopsys.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jose Abreu Cc: Laurent Pinchart , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Carlos Palminha , Kieran Bingham List-Id: dri-devel@lists.freedesktop.org SGkgSm9zZSwKCk9uIEZyaWRheSAyMyBKdW4gMjAxNyAxMDozNjo0NCBKb3NlIEFicmV1IHdyb3Rl Ogo+IEN1cnJlbnRseSBIRE1JIDIuMCBQSFlzIGRvIG5vdCBoYXZlIGEgZGVmYXVsdCBjb25maWd1 cmF0aW9uIGZ1bmN0aW9uLgo+IAo+IEFzICpzb21lKiBvZiB0aGUgSERNSSAyLjAgUEhZcyBoYXZl IHRoZSBzYW1lIHJlZ2lzdGVyIGxheW91dCBhcyB0aGUgM0QKPiBQSFlzIHdlIGNhbiBwcm92aWRl IHRoZSBzYW1lIGRlZmF1bHQgY29uZmlndXJhdGlvbiBmdW5jdGlvbiBmb3IgYm90aAo+IGFuZCBz dGlsbCBsZXQgdXNlciBvdmVyd3JpdGUgdGhpcyB3aXRoIGN1c3RvbSBjb25maWd1cmF0aW9uIGZ1 bmN0aW9uCj4gaWYgbmVlZGVkLgo+IAo+IElmLCBmb3Igc29tZSByZWFzb24sIHRoZSBQSFkgaXMg Y3VzdG9tIG9yIGhhcyBhIHJlZ2lzdGVyIGRpZmZlcmVudAo+IHJlZ2lzdGVyIGxheW91dCB0aGVu IGN1c3RvbSBjb25maWd1cmF0aW9uIGZ1bmN0aW9uICptdXN0KiBiZSBwcm92aWRlZAo+IGluIG9y ZGVyIGZvciB0aGUgc3lzdGVtIHRvIHdvcmsgY29ycmVjdGx5LiBBcyB3ZSBwcmVmZXIgdGhlIHBk YXRhCj4gcHJvdmlkZWQgY29uZmlndXJhdGlvbiBmdW5jdGlvbiBvdmVyIHRoZSBpbnRlcm5hbCBv bmUgdGhpcyBjaGFuZ2UKPiB3aWxsIG5vdCBtYWtlIGFueSBpbXBhY3QgaW4gY3VzdG9tIHBsYXRm b3Jtcy4KPiAKPiBUaGlzIHBhdGNoIGlzIGJhc2VkIG9uIHRvZGF5J3MgZHJtLW1pc2MtbmV4dCBi cmFuY2guCj4gCj4gU2lnbmVkLW9mZi1ieTogSm9zZSBBYnJldSA8am9hYnJldUBzeW5vcHN5cy5j b20+Cj4gVGVzdGVkLWJ5OiBNYXJrIFlhbyA8bWFyay55YW9Acm9jay1jaGlwcy5jb20+Cj4gQ2M6 IEtpZXJhbiBCaW5naGFtIDxraWVyYW4uYmluZ2hhbStyZW5lc2FzQGlkZWFzb25ib2FyZC5jb20+ Cj4gQ2M6IExhdXJlbnQgUGluY2hhcnQgPGxhdXJlbnQucGluY2hhcnQrcmVuZXNhc0BpZGVhc29u Ym9hcmQuY29tPgo+IENjOiBBcmNoaXQgVGFuZWphIDxhcmNoaXR0QGNvZGVhdXJvcmEub3JnPgo+ IENjOiBBbmRyemVqIEhhamRhIDxhLmhhamRhQHNhbXN1bmcuY29tPgo+IENjOiBNYXJrIFlhbyA8 bWFyay55YW9Acm9jay1jaGlwcy5jb20+Cj4gQ2M6IENhcmxvcyBQYWxtaW5oYSA8cGFsbWluaGFA c3lub3BzeXMuY29tPgo+IENjOiBIZWlrbyBTdMO8Ym5lciA8aGVpa29Ac250ZWNoLmRlPgo+IAo+ IENoYW5nZXMgaW4gdjI6Cj4gCS0gUmViYXNlZCBhbmQgcmVmcmFzZWQgY29tbWl0IG1lc3NhZ2UK PiAtLS0KPiAKPiBIaSBBbGwsCj4gCj4gVGhlcmUgYXMgYmVlbiBhIGxpdHRsZSBjb25mdXNpb24g YWJvdXQgZHctaGRtaSBwaHlzIHNvIEkgd2lsbCBleHBhbmQgYQo+IGxpdHRsZSBiaXQgaGVyZSBz byB0aGF0IEkgY2FuIGJhc2UgbXkgZGVjaXNpb24gYWJvdXQgdGhpcyBwYXRjaCBhbmQgd2h5Cj4g ZG9lcyBpdCBvbmx5IHdvcmtzIGluIHNvbWUgcGxhdGZvcm1zLgo+IAo+IEZpcnN0LCBpZiB5b3Ug cmVhZCBkdy1oZG1pLmMgY29kZSwgeW91IHdpbGwgc2VlIHRoYXQgdGhlcmUgaXMgYW4KPiBpZGVu dGlmaWNhdGlvbiByZWdpc3RlciBmb3IgdGhlIHBoeSB0eXBlIGJlaW5nIHVzZWQuIFVuZm9ydHVu YXRlbGx5LCB0aGlzCj4gb25seSBzdGF0ZXMgdGhlIHBoeSB0eXBlIGFuZCBub3QgdGhlIHBoeSB2 ZXJzaW9uLgo+IAo+IFNlY29uZCwgd2UgaGF2ZSBtYW55IEhETUkgMi4wIHBoeXMgKHNvLCBzYW1l IHBoeSB0eXBlOiAweGYzKSBidXQsIGFzIHlvdSBtYXkKPiBoYXZlIGd1ZXNzZWQsIEhXIHRlYW0g ZGVjaWRlZCB0byBjaGFuZ2UgcmVnYmFuayBiZXR3ZWVuIHNvbWUgdmVyc2lvbnMuCj4gCj4gVGhp cmQgYW5kIGxhc3QsIGVhY2ggcGh5IGluIGEgU29DIGhhcyB1bmlxdWUgY2hhcmFjdGVyaXN0aWNz LCBzbyBlYWNoIHBoeQo+IChldmVudCBpZiB0aGV5IGFyZSB0aGUgc2FtZSB2ZXJzaW9uKSB3aWxs IGhhdmUgZGlmZmVyZW50IFBMTCBjb25maWd1cmF0aW9uCj4gcGFyYW1ldGVycy4KPiAKPiBHaXZl biBhbGwgdGhpcyBJIG1hbmFnZWQgdG8gY29uY2x1ZGUgdGhhdCBNYXJrJ3MgcGh5IGlzIHN0aWxs IGFuIEhETUkgMi4wCj4gcGh5IGJ1dCB3aXRoIHRoZSBzYW1lIHJlZ2lzdGVyIGxheW91dCBhcyBw cmV2aW91cyAzRCBQSFkncy4gSSBmb3VuZCBhdAo+IGxlYXN0IDIgcGh5cyB3aXRoIHRoZSBzYW1l IHJlZ2lzdGVyIGxheW91dCBhbmQgb25seSAxIHBoeSB3aGljaCBoYXMgYQo+IGRpZmZlcmVudCBs YXlvdXQsIHNvIEkgdGhpbmsgbWFqb3JpdHkgd2lucyBoZXJlIGFuZCB3ZSBzaG91bGQgbGV0IHRo ZQo+IGRlZmF1bHQgY29uZmlndXJhdGlvbiBmdW5jdGlvbiBmb3IgSERNSSAyLjAgcGh5cyBiZSB0 aGUgc2FtZSBvbmUgYXMgdGhlIDNELgo+IAo+IFNob3J0IHN0b3J5OiBUaGVyZSBpcyBubyB3YXkg dG8gY29ycmVjdGx5IGlkZW50aWZ5LCBhdCBydW50aW1lLCB0aGUgcGh5Cj4gdmVyc2lvbiBiZWlu ZyB1c2VkIGJ5IHRoZSBjb250cm9sbGVyIHNvIHdlIGNhbid0IHByb3ZpZGUgYSBkZWZhdWx0Cj4g Y29uZmlndXJhdGlvbiBmdW5jdGlvbi4KCjonLSgKCj4gV2UgY2FuLCBob3dldmVyIGFzc3VtZSB0 aGF0IG1vc3Qgb2YgdGhlIEhETUkgMi4wIHBoeXMgd2lsbCBoYXZlIHRoZSAzRAo+IGxheW91dCBC VVQgZWFjaCBkZXZlbG9wZXIgbXVzdCBjb25maXJtIHRoYXQgdGhlIGxheW91dCBpbiBpdHMgU29D IGlzIHRoZQo+IGV4cGVjdGVkIG9uZSBhbmQgaWYgbm90LCBwcm92aWRlIGEgY3VzdG9tIGNvbmZp Z3VyYXRpb24gZnVuY3Rpb24uCgpUaGlzIHNlZW1zIGZhaXIgdG8gbWUsIHNvCgpSZXZpZXdlZC1i eTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBpZGVhc29uYm9hcmQuY29tPgoK SG93ZXZlciwgaWYgdGhlIFBIWSB2ZXJzaW9uIGNhbid0IGJlIGlkZW50aWZpZWQgYXQgcnVudGlt ZSwgSSB3b25kZXIgd2hldGhlciAKaXQgd291bGQgbWFrZSBzZW5zZSB0byBhZGQgaXQgdG8gdGhl IFBIWSBEVCBub2RlLiBUaGUgZHJpdmVyIGNvdWxkIHRoZW4gcGljayAKdGhlIHJpZ2h0IGNvbmZp Z3VyYXRpb24gZnVuY3Rpb24gYWNjb3JkaW5nbHkuIFNvQy1zcGVjaWZpYyBwYXJhbWV0ZXJzIChz dWNoIGFzIApQTEwgcGFyYW1ldGVycykgd291bGQgc3RpbGwgYmUgcGFzc2VkIHRocm91Z2ggcGxh dGZvcm0gZ2x1ZSBjb2RlLCBidXQgYWxsIHRoZSAKcmVzdCBjb3VsZCBiZSBtb3ZlZCB0byB0aGUg Y29yZSBkcml2ZXIuCgo+ICBkcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9wc3lzL2R3LWhkbWku YyB8IDEgKwo+ICAxIGZpbGUgY2hhbmdlZCwgMSBpbnNlcnRpb24oKykKPiAKPiBkaWZmIC0tZ2l0 IGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMKPiBiL2RyaXZlcnMv Z3B1L2RybS9icmlkZ2Uvc3lub3BzeXMvZHctaGRtaS5jIGluZGV4IGVhZDExMjQuLjEwYzhkOGMg MTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMK PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9wc3lzL2R3LWhkbWkuYwo+IEBAIC0y MTcwLDYgKzIxNzAsNyBAQCBzdGF0aWMgaXJxcmV0dXJuX3QgZHdfaGRtaV9pcnEoaW50IGlycSwg dm9pZCAqZGV2X2lkKQo+ICAJCS5uYW1lID0gIkRXQyBIRE1JIDIuMCBUWCBQSFkiLAo+ICAJCS5n ZW4gPSAyLAo+ICAJCS5oYXNfc3ZzcmV0ID0gdHJ1ZSwKPiArCQkuY29uZmlndXJlID0gaGRtaV9w aHlfY29uZmlndXJlX2R3Y19oZG1pXzNkX3R4LAo+ICAJfSwgewo+ICAJCS50eXBlID0gRFdfSERN SV9QSFlfVkVORE9SX1BIWSwKPiAgCQkubmFtZSA9ICJWZW5kb3IgUEhZIiwKCi0tIApSZWdhcmRz LAoKTGF1cmVudCBQaW5jaGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRl c2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8v ZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 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 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