From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Laurent Pinchart To: Jose Abreu Cc: dri-devel@lists.freedesktop.org, Fabio Estevam , Laurent Pinchart , Russell King - ARM Linux , Kieran Bingham , linux-renesas-soc@vger.kernel.org, Ulrich Hecht , Andy Yan , Vladimir Zapolskiy Subject: Re: [PATCH v2 17/29] drm: bridge: dw-hdmi: Refactor PHY power handling Date: Thu, 02 Mar 2017 00:47:50 +0200 Message-ID: <2035754.xySSvQpCjl@avalon> In-Reply-To: <2b176eff-2fe9-2daf-b9e8-0dfb116235cc@synopsys.com> References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <7271033.6cEIpZMQXV@avalon> <2b176eff-2fe9-2daf-b9e8-0dfb116235cc@synopsys.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" List-ID: Hi Jose, On Wednesday 01 Mar 2017 16:25:46 Jose Abreu wrote: > On 01-03-2017 11:09, Laurent Pinchart wrote: > > On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote: > >> On 05-01-2017 12:29, Laurent Pinchart wrote: > >>> On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: > >>>> On 20-12-2016 11:45, Russell King - ARM Linux wrote: > >>>>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > >>>>>> Instead of spreading version-dependent PHY power handling code > >>>>>> around, group it in two functions to power the PHY on and off and use > >>>>>> them through the driver. > >>>>>> > >>>>>> Powering off the PHY at the beginning of the setup phase is currently > >>>>>> split in two locations for first and second generation PHYs, group > >>>>>> all the operations in the dw_hdmi_phy_init() function. > >>>>> > >>>>> This changes the behaviour of the driver. > >>>>> > >>>>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > >>>>>> +{ > >>>>>> + if (hdmi->phy->gen == 1) { > >>>>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>>> + } else { > >>>>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>>>> + } > >>>>>> +} > >>>>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>>>> *hdmi) > >>>>>> if (!hdmi->phy_enabled) > >>>>>> return; > >>>>>> > >>>>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>>> - > >>>>>> + dw_hdmi_phy_power_off(hdmi); > >>>>> > >>>>> This makes dw_hdmi_phy_disable() power down a gen2 phy. > >>>>> > >>>>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > >>>>> gen2 phy. I've been carrying this change for a while, which I've had > >>>>> to revert (and finally expunge), as it causes problems on iMX6: > >>>>> > >>>>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>>> *hdmi) > >>>>> if (!hdmi->phy_enabled) > >>>>> return; > >>>>> > >>>>> + /* Actually set the phy into low power mode */ > >>>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>>> + > >>>>> + /* FIXME: We should wait for TX_READY to be deasserted */ > >>>>> + > >>>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>>> + > >>>>> + /* This appears to have no effect on iMX6 */ > >>>>> dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>> dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>> > >>>>> So, I think your change here will cause problems for iMX6. > >>>>> > >>>>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > >>>>> bouncing when the PHY is powered down. I can't promise when I'll be > >>>>> able to check for that again. > >>>> > >>>> Indeed TX_READY must be low before asserting pddq. > >>> > >>> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY > >>> output signal, but there seems to be no HDMI TX register from which its > >>> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register > >>> through I2C ? How long is the PHY expected to take to set TX_READY to 0 > >>> ? > >> > >> TX_READY can be read from register 0x1A of phy, BIT(2) (through > >> I2C). Not sure if same offset for all phys though. > > > > I have been told that another option is to poll the TX_PHY_LOCK bit in the > > phy_stat0 register. That would be a simpler solution that doesn't require > > I2C access. Could you confirm (or disconfirm) this ? > > Yes (In our implementation we have TX_PHY_LOCK wired up to > TX_READY that comes from phy. But if using a custom phy or an > unusual implementation then this may not hold). Thank you for the information. I've submitted a v4 that poll the TX_PHY_LOCK bit. With the patch series applied the vendor PHY handling code already delegates PHY power control to the glue code, there's thus no issue there. For platforms using a Synopsys PHY, all the ones we have to support today have TX_READY wired to the PHY lock signal so we should be good as well there. If that doesn't remain true in the future we'll fix it. > >>>> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense > >>>> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, > >>>> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and > >>>> consequently entering power down mode) you can enable again > >>>> enhpdrxsense. > >>> > >>> Thanks, I'll give it a try. -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 17/29] drm: bridge: dw-hdmi: Refactor PHY power handling Date: Thu, 02 Mar 2017 00:47:50 +0200 Message-ID: <2035754.xySSvQpCjl@avalon> References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <7271033.6cEIpZMQXV@avalon> <2b176eff-2fe9-2daf-b9e8-0dfb116235cc@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 71EA26E20C for ; Wed, 1 Mar 2017 22:47:15 +0000 (UTC) In-Reply-To: <2b176eff-2fe9-2daf-b9e8-0dfb116235cc@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: Fabio Estevam , Laurent Pinchart , Russell King - ARM Linux , dri-devel@lists.freedesktop.org, Kieran Bingham , linux-renesas-soc@vger.kernel.org, Ulrich Hecht , Andy Yan , Vladimir Zapolskiy List-Id: dri-devel@lists.freedesktop.org SGkgSm9zZSwKCk9uIFdlZG5lc2RheSAwMSBNYXIgMjAxNyAxNjoyNTo0NiBKb3NlIEFicmV1IHdy b3RlOgo+IE9uIDAxLTAzLTIwMTcgMTE6MDksIExhdXJlbnQgUGluY2hhcnQgd3JvdGU6Cj4gPiBP biBUaHVyc2RheSAwNSBKYW4gMjAxNyAxNTowNjo0OSBKb3NlIEFicmV1IHdyb3RlOgo+ID4+IE9u IDA1LTAxLTIwMTcgMTI6MjksIExhdXJlbnQgUGluY2hhcnQgd3JvdGU6Cj4gPj4+IE9uIFR1ZXNk YXkgMjAgRGVjIDIwMTYgMTI6MTc6MjMgSm9zZSBBYnJldSB3cm90ZToKPiA+Pj4+IE9uIDIwLTEy LTIwMTYgMTE6NDUsIFJ1c3NlbGwgS2luZyAtIEFSTSBMaW51eCB3cm90ZToKPiA+Pj4+PiBPbiBU dWUsIERlYyAyMCwgMjAxNiBhdCAwMzozMzo0OEFNICswMjAwLCBMYXVyZW50IFBpbmNoYXJ0IHdy b3RlOgo+ID4+Pj4+PiBJbnN0ZWFkIG9mIHNwcmVhZGluZyB2ZXJzaW9uLWRlcGVuZGVudCBQSFkg cG93ZXIgaGFuZGxpbmcgY29kZQo+ID4+Pj4+PiBhcm91bmQsIGdyb3VwIGl0IGluIHR3byBmdW5j dGlvbnMgdG8gcG93ZXIgdGhlIFBIWSBvbiBhbmQgb2ZmIGFuZCB1c2UKPiA+Pj4+Pj4gdGhlbSB0 aHJvdWdoIHRoZSBkcml2ZXIuCj4gPj4+Pj4+IAo+ID4+Pj4+PiBQb3dlcmluZyBvZmYgdGhlIFBI WSBhdCB0aGUgYmVnaW5uaW5nIG9mIHRoZSBzZXR1cCBwaGFzZSBpcyBjdXJyZW50bHkKPiA+Pj4+ Pj4gc3BsaXQgaW4gdHdvIGxvY2F0aW9ucyBmb3IgZmlyc3QgYW5kIHNlY29uZCBnZW5lcmF0aW9u IFBIWXMsIGdyb3VwCj4gPj4+Pj4+IGFsbCB0aGUgb3BlcmF0aW9ucyBpbiB0aGUgZHdfaGRtaV9w aHlfaW5pdCgpIGZ1bmN0aW9uLgo+ID4+Pj4+IAo+ID4+Pj4+IFRoaXMgY2hhbmdlcyB0aGUgYmVo YXZpb3VyIG9mIHRoZSBkcml2ZXIuCj4gPj4+Pj4gCj4gPj4+Pj4+ICtzdGF0aWMgdm9pZCBkd19o ZG1pX3BoeV9wb3dlcl9vZmYoc3RydWN0IGR3X2hkbWkgKmhkbWkpCj4gPj4+Pj4+ICt7Cj4gPj4+ Pj4+ICsJaWYgKGhkbWktPnBoeS0+Z2VuID09IDEpIHsKPiA+Pj4+Pj4gKwkJZHdfaGRtaV9waHlf ZW5hYmxlX3RtZHMoaGRtaSwgMCk7Cj4gPj4+Pj4+ICsJCWR3X2hkbWlfcGh5X2VuYWJsZV9wb3dl cmRvd24oaGRtaSwgdHJ1ZSk7Cj4gPj4+Pj4+ICsJfSBlbHNlIHsKPiA+Pj4+Pj4gKwkJZHdfaGRt aV9waHlfZ2VuMl90eHB3cm9uKGhkbWksIDApOwo+ID4+Pj4+PiArCQlkd19oZG1pX3BoeV9nZW4y X3BkZHEoaGRtaSwgMSk7Cj4gPj4+Pj4+ICsJfQo+ID4+Pj4+PiArfQo+ID4+Pj4+PiBAQCAtMTI5 MCw5ICsxMzAyLDcgQEAgc3RhdGljIHZvaWQgZHdfaGRtaV9waHlfZGlzYWJsZShzdHJ1Y3QgZHdf aGRtaQo+ID4+Pj4+PiAqaGRtaSkKPiA+Pj4+Pj4gIAlpZiAoIWhkbWktPnBoeV9lbmFibGVkKQo+ ID4+Pj4+PiAgCQlyZXR1cm47Cj4gPj4+Pj4+IAo+ID4+Pj4+PiAtCWR3X2hkbWlfcGh5X2VuYWJs ZV90bWRzKGhkbWksIDApOwo+ID4+Pj4+PiAtCWR3X2hkbWlfcGh5X2VuYWJsZV9wb3dlcmRvd24o aGRtaSwgdHJ1ZSk7Cj4gPj4+Pj4+IC0KPiA+Pj4+Pj4gKwlkd19oZG1pX3BoeV9wb3dlcl9vZmYo aGRtaSk7Cj4gPj4+Pj4gCj4gPj4+Pj4gVGhpcyBtYWtlcyBkd19oZG1pX3BoeV9kaXNhYmxlKCkg cG93ZXIgZG93biBhIGdlbjIgcGh5Lgo+ID4+Pj4+IAo+ID4+Pj4+IFRoZSBpTVg2IGhhcyBhIERX X0hETUlfUEhZX0RXQ19IRE1JXzNEX1RYX1BIWSBwaHksIHdoaWNoIHlvdSBsaXN0IGFzIGEKPiA+ Pj4+PiBnZW4yIHBoeS4gIEkndmUgYmVlbiBjYXJyeWluZyB0aGlzIGNoYW5nZSBmb3IgYSB3aGls ZSwgd2hpY2ggSSd2ZSBoYWQKPiA+Pj4+PiB0byByZXZlcnQgKGFuZCBmaW5hbGx5IGV4cHVuZ2Up LCBhcyBpdCBjYXVzZXMgcHJvYmxlbXMgb24gaU1YNjoKPiA+Pj4+PiAKPiA+Pj4+PiBAQCAtMTEx Miw2ICsxMTEyLDE0IEBAIHN0YXRpYyB2b2lkIGR3X2hkbWlfcGh5X2Rpc2FibGUoc3RydWN0IGR3 X2hkbWkKPiA+Pj4+PiAqaGRtaSkKPiA+Pj4+PiAgICAgICAgIGlmICghaGRtaS0+cGh5X2VuYWJs ZWQpCj4gPj4+Pj4gICAgICAgICAgICAgICAgIHJldHVybjsKPiA+Pj4+PiAKPiA+Pj4+PiArICAg ICAgIC8qIEFjdHVhbGx5IHNldCB0aGUgcGh5IGludG8gbG93IHBvd2VyIG1vZGUgKi8KPiA+Pj4+ PiArICAgICAgIGR3X2hkbWlfcGh5X2dlbjJfdHhwd3JvbihoZG1pLCAwKTsKPiA+Pj4+PiArCj4g Pj4+Pj4gKyAgICAgICAvKiBGSVhNRTogV2Ugc2hvdWxkIHdhaXQgZm9yIFRYX1JFQURZIHRvIGJl IGRlYXNzZXJ0ZWQgKi8KPiA+Pj4+PiArCj4gPj4+Pj4gKyAgICAgICBkd19oZG1pX3BoeV9nZW4y X3BkZHEoaGRtaSwgMSk7Cj4gPj4+Pj4gKwo+ID4+Pj4+ICsgICAgICAgLyogVGhpcyBhcHBlYXJz IHRvIGhhdmUgbm8gZWZmZWN0IG9uIGlNWDYgKi8KPiA+Pj4+PiAgICAgICAgIGR3X2hkbWlfcGh5 X2VuYWJsZV90bWRzKGhkbWksIDApOwo+ID4+Pj4+ICAgICAgICAgZHdfaGRtaV9waHlfZW5hYmxl X3Bvd2VyZG93bihoZG1pLCB0cnVlKTsKPiA+Pj4+PiAKPiA+Pj4+PiBTbywgSSB0aGluayB5b3Vy IGNoYW5nZSBoZXJlIHdpbGwgY2F1c2UgcHJvYmxlbXMgZm9yIGlNWDYuCj4gPj4+Pj4gCj4gPj4+ Pj4gRnJvbSB3aGF0IEkgcmVtZW1iZXIsIGl0IHNlZW1zIHRoYXQgaU1YNiBoYXMgaXNzdWVzIHdp dGggUlhTRU5TRS9IUEQKPiA+Pj4+PiBib3VuY2luZyB3aGVuIHRoZSBQSFkgaXMgcG93ZXJlZCBk b3duLiAgSSBjYW4ndCBwcm9taXNlIHdoZW4gSSdsbCBiZQo+ID4+Pj4+IGFibGUgdG8gY2hlY2sg Zm9yIHRoYXQgYWdhaW4uCj4gPj4+PiAKPiA+Pj4+IEluZGVlZCBUWF9SRUFEWSBtdXN0IGJlIGxv dyBiZWZvcmUgYXNzZXJ0aW5nIHBkZHEuCj4gPj4+IAo+ID4+PiBUaGUgVFhfUkVBRFkgc2lnbmFs IGlzIGRvY3VtZW50ZWQgaW4gdGhlIGkuTVg2IGRhdGFzaGVldCBhcyBiZWluZyBhIFBIWQo+ID4+ PiBvdXRwdXQgc2lnbmFsLCBidXQgdGhlcmUgc2VlbXMgdG8gYmUgbm8gSERNSSBUWCByZWdpc3Rl ciBmcm9tIHdoaWNoIGl0cwo+ID4+PiBzdGF0ZSBjYW4gYmUgcmVhZC4gRG8gSSBuZWVkIHRvIHBv bGwgdGhlIEhETUlfUEhZX1BUUlBUX0VOQkwgcmVnaXN0ZXIKPiA+Pj4gdGhyb3VnaCBJMkMgPyBI b3cgbG9uZyBpcyB0aGUgUEhZIGV4cGVjdGVkIHRvIHRha2UgdG8gc2V0IFRYX1JFQURZIHRvIDAK PiA+Pj4gPwo+ID4+IAo+ID4+IFRYX1JFQURZIGNhbiBiZSByZWFkIGZyb20gcmVnaXN0ZXIgMHgx QSBvZiBwaHksIEJJVCgyKSAodGhyb3VnaAo+ID4+IEkyQykuIE5vdCBzdXJlIGlmIHNhbWUgb2Zm c2V0IGZvciBhbGwgcGh5cyB0aG91Z2guCj4gPiAKPiA+IEkgaGF2ZSBiZWVuIHRvbGQgdGhhdCBh bm90aGVyIG9wdGlvbiBpcyB0byBwb2xsIHRoZSBUWF9QSFlfTE9DSyBiaXQgaW4gdGhlCj4gPiBw aHlfc3RhdDAgcmVnaXN0ZXIuIFRoYXQgd291bGQgYmUgYSBzaW1wbGVyIHNvbHV0aW9uIHRoYXQg ZG9lc24ndCByZXF1aXJlCj4gPiBJMkMgYWNjZXNzLiBDb3VsZCB5b3UgY29uZmlybSAob3IgZGlz Y29uZmlybSkgdGhpcyA/Cj4gCj4gWWVzIChJbiBvdXIgaW1wbGVtZW50YXRpb24gd2UgaGF2ZSBU WF9QSFlfTE9DSyB3aXJlZCB1cCB0bwo+IFRYX1JFQURZIHRoYXQgY29tZXMgZnJvbSBwaHkuIEJ1 dCBpZiB1c2luZyBhIGN1c3RvbSBwaHkgb3IgYW4KPiB1bnVzdWFsIGltcGxlbWVudGF0aW9uIHRo ZW4gdGhpcyBtYXkgbm90IGhvbGQpLgoKVGhhbmsgeW91IGZvciB0aGUgaW5mb3JtYXRpb24uIEkn dmUgc3VibWl0dGVkIGEgdjQgdGhhdCBwb2xsIHRoZSBUWF9QSFlfTE9DSyAKYml0LiAgV2l0aCB0 aGUgcGF0Y2ggc2VyaWVzIGFwcGxpZWQgdGhlIHZlbmRvciBQSFkgaGFuZGxpbmcgY29kZSBhbHJl YWR5IApkZWxlZ2F0ZXMgUEhZIHBvd2VyIGNvbnRyb2wgdG8gdGhlIGdsdWUgY29kZSwgdGhlcmUn cyB0aHVzIG5vIGlzc3VlIHRoZXJlLiBGb3IgCnBsYXRmb3JtcyB1c2luZyBhIFN5bm9wc3lzIFBI WSwgYWxsIHRoZSBvbmVzIHdlIGhhdmUgdG8gc3VwcG9ydCB0b2RheSBoYXZlIApUWF9SRUFEWSB3 aXJlZCB0byB0aGUgUEhZIGxvY2sgc2lnbmFsIHNvIHdlIHNob3VsZCBiZSBnb29kIGFzIHdlbGwg dGhlcmUuIElmIAp0aGF0IGRvZXNuJ3QgcmVtYWluIHRydWUgaW4gdGhlIGZ1dHVyZSB3ZSdsbCBm aXggaXQuCgo+ID4+Pj4gSFBEIGFuZCBSWFNFTlNFIHNob3VsZCB3b3JrIGluIHBvd2VyIGRvd24g bW9kZSBieSBlbmFibGluZyBlbmhwZHJ4c2Vuc2UKPiA+Pj4+IGJpdCBpbiBwaHlfY29uZjAgQlVU IHRvIGVudGVyIHBvd2VyIGRvd24geW91IG11c3QgZGlzYWJsZSBUWF9QV1JPTiwKPiA+Pj4+IGVu aHBkcnhzZW5zZSBhbmQgZW5hYmxlIFBERFEgYW5kIFBIWV9SRVNFVC4gT25seSBhZnRlciBkb2lu ZyB0aGlzIChhbmQKPiA+Pj4+IGNvbnNlcXVlbnRseSBlbnRlcmluZyBwb3dlciBkb3duIG1vZGUp IHlvdSBjYW4gZW5hYmxlIGFnYWluCj4gPj4+PiBlbmhwZHJ4c2Vuc2UuCj4gPj4+IAo+ID4+PiBU aGFua3MsIEknbGwgZ2l2ZSBpdCBhIHRyeS4KCi0tIApSZWdhcmRzLAoKTGF1cmVudCBQaW5jaGFy dAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRl dmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==