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, 05 Jan 2017 17:33:58 +0200 Message-ID: <21211642.vKbn0SAu3E@avalon> In-Reply-To: References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <5812310.k87Vx0mP1E@avalon> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" List-ID: Hi Jose, 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). That's what I thought, I'll poll that then. Do you have any idea how long it's supposed to take, to set an appropriate timeout ? > Not sure if same offset for all phys though. Most probably not, it would be too easy :-) I'll investigate (which will likely include lots of guesswork). If you can find any information about that (and especially about the MHL and HDMI 2.0 PHYs) that would be very appreciated, as I don't have access to any documentation that mentions a TX_READY bit for those. -- 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, 05 Jan 2017 17:33:58 +0200 Message-ID: <21211642.vKbn0SAu3E@avalon> References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <5812310.k87Vx0mP1E@avalon> 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 C53766E89F for ; Thu, 5 Jan 2017 15:33:54 +0000 (UTC) In-Reply-To: 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 SGkgSm9zZSwKCk9uIFRodXJzZGF5IDA1IEphbiAyMDE3IDE1OjA2OjQ5IEpvc2UgQWJyZXUgd3Jv dGU6Cj4gT24gMDUtMDEtMjAxNyAxMjoyOSwgTGF1cmVudCBQaW5jaGFydCB3cm90ZToKPiA+IE9u IFR1ZXNkYXkgMjAgRGVjIDIwMTYgMTI6MTc6MjMgSm9zZSBBYnJldSB3cm90ZToKPiA+PiBPbiAy MC0xMi0yMDE2IDExOjQ1LCBSdXNzZWxsIEtpbmcgLSBBUk0gTGludXggd3JvdGU6Cj4gPj4+IE9u IFR1ZSwgRGVjIDIwLCAyMDE2IGF0IDAzOjMzOjQ4QU0gKzAyMDAsIExhdXJlbnQgUGluY2hhcnQg d3JvdGU6Cj4gPj4+PiBJbnN0ZWFkIG9mIHNwcmVhZGluZyB2ZXJzaW9uLWRlcGVuZGVudCBQSFkg cG93ZXIgaGFuZGxpbmcgY29kZSBhcm91bmQsCj4gPj4+PiBncm91cCBpdCBpbiB0d28gZnVuY3Rp b25zIHRvIHBvd2VyIHRoZSBQSFkgb24gYW5kIG9mZiBhbmQgdXNlIHRoZW0KPiA+Pj4+IHRocm91 Z2ggdGhlIGRyaXZlci4KPiA+Pj4+IAo+ID4+Pj4gUG93ZXJpbmcgb2ZmIHRoZSBQSFkgYXQgdGhl IGJlZ2lubmluZyBvZiB0aGUgc2V0dXAgcGhhc2UgaXMgY3VycmVudGx5Cj4gPj4+PiBzcGxpdCBp biB0d28gbG9jYXRpb25zIGZvciBmaXJzdCBhbmQgc2Vjb25kIGdlbmVyYXRpb24gUEhZcywgZ3Jv dXAgYWxsCj4gPj4+PiB0aGUgb3BlcmF0aW9ucyBpbiB0aGUgZHdfaGRtaV9waHlfaW5pdCgpIGZ1 bmN0aW9uLgo+ID4+PiAKPiA+Pj4gVGhpcyBjaGFuZ2VzIHRoZSBiZWhhdmlvdXIgb2YgdGhlIGRy aXZlci4KPiA+Pj4gCj4gPj4+PiArc3RhdGljIHZvaWQgZHdfaGRtaV9waHlfcG93ZXJfb2ZmKHN0 cnVjdCBkd19oZG1pICpoZG1pKQo+ID4+Pj4gK3sKPiA+Pj4+ICsJaWYgKGhkbWktPnBoeS0+Z2Vu ID09IDEpIHsKPiA+Pj4+ICsJCWR3X2hkbWlfcGh5X2VuYWJsZV90bWRzKGhkbWksIDApOwo+ID4+ Pj4gKwkJZHdfaGRtaV9waHlfZW5hYmxlX3Bvd2VyZG93bihoZG1pLCB0cnVlKTsKPiA+Pj4+ICsJ fSBlbHNlIHsKPiA+Pj4+ICsJCWR3X2hkbWlfcGh5X2dlbjJfdHhwd3JvbihoZG1pLCAwKTsKPiA+ Pj4+ICsJCWR3X2hkbWlfcGh5X2dlbjJfcGRkcShoZG1pLCAxKTsKPiA+Pj4+ICsJfQo+ID4+Pj4g K30KPiA+Pj4+IEBAIC0xMjkwLDkgKzEzMDIsNyBAQCBzdGF0aWMgdm9pZCBkd19oZG1pX3BoeV9k aXNhYmxlKHN0cnVjdCBkd19oZG1pCj4gPj4+PiAqaGRtaSkKPiA+Pj4+IAo+ID4+Pj4gIAlpZiAo IWhkbWktPnBoeV9lbmFibGVkKQo+ID4+Pj4gIAkKPiA+Pj4+ICAJCXJldHVybjsKPiA+Pj4+IAo+ ID4+Pj4gLQlkd19oZG1pX3BoeV9lbmFibGVfdG1kcyhoZG1pLCAwKTsKPiA+Pj4+IC0JZHdfaGRt aV9waHlfZW5hYmxlX3Bvd2VyZG93bihoZG1pLCB0cnVlKTsKPiA+Pj4+IC0KPiA+Pj4+ICsJZHdf aGRtaV9waHlfcG93ZXJfb2ZmKGhkbWkpOwo+ID4+PiAKPiA+Pj4gVGhpcyBtYWtlcyBkd19oZG1p X3BoeV9kaXNhYmxlKCkgcG93ZXIgZG93biBhIGdlbjIgcGh5Lgo+ID4+PiAKPiA+Pj4gVGhlIGlN WDYgaGFzIGEgRFdfSERNSV9QSFlfRFdDX0hETUlfM0RfVFhfUEhZIHBoeSwgd2hpY2ggeW91IGxp c3QgYXMgYQo+ID4+PiBnZW4yIHBoeS4gIEkndmUgYmVlbiBjYXJyeWluZyB0aGlzIGNoYW5nZSBm b3IgYSB3aGlsZSwgd2hpY2ggSSd2ZSBoYWQKPiA+Pj4gdG8gcmV2ZXJ0IChhbmQgZmluYWxseSBl eHB1bmdlKSwgYXMgaXQgY2F1c2VzIHByb2JsZW1zIG9uIGlNWDY6Cj4gPj4+IAo+ID4+PiBAQCAt MTExMiw2ICsxMTEyLDE0IEBAIHN0YXRpYyB2b2lkIGR3X2hkbWlfcGh5X2Rpc2FibGUoc3RydWN0 IGR3X2hkbWkKPiA+Pj4gKmhkbWkpPgo+ID4+PiAKPiA+Pj4gICAgICAgICBpZiAoIWhkbWktPnBo eV9lbmFibGVkKQo+ID4+PiAgICAgICAgIAo+ID4+PiAgICAgICAgICAgICAgICAgcmV0dXJuOwo+ ID4+PiAKPiA+Pj4gKyAgICAgICAvKiBBY3R1YWxseSBzZXQgdGhlIHBoeSBpbnRvIGxvdyBwb3dl ciBtb2RlICovCj4gPj4+ICsgICAgICAgZHdfaGRtaV9waHlfZ2VuMl90eHB3cm9uKGhkbWksIDAp Owo+ID4+PiArCj4gPj4+ICsgICAgICAgLyogRklYTUU6IFdlIHNob3VsZCB3YWl0IGZvciBUWF9S RUFEWSB0byBiZSBkZWFzc2VydGVkICovCj4gPj4+ICsKPiA+Pj4gKyAgICAgICBkd19oZG1pX3Bo eV9nZW4yX3BkZHEoaGRtaSwgMSk7Cj4gPj4+ICsKPiA+Pj4gKyAgICAgICAvKiBUaGlzIGFwcGVh cnMgdG8gaGF2ZSBubyBlZmZlY3Qgb24gaU1YNiAqLwo+ID4+PiAKPiA+Pj4gICAgICAgICBkd19o ZG1pX3BoeV9lbmFibGVfdG1kcyhoZG1pLCAwKTsKPiA+Pj4gICAgICAgICBkd19oZG1pX3BoeV9l bmFibGVfcG93ZXJkb3duKGhkbWksIHRydWUpOwo+ID4+PiAKPiA+Pj4gU28sIEkgdGhpbmsgeW91 ciBjaGFuZ2UgaGVyZSB3aWxsIGNhdXNlIHByb2JsZW1zIGZvciBpTVg2Lgo+ID4+PiAKPiA+Pj4g RnJvbSB3aGF0IEkgcmVtZW1iZXIsIGl0IHNlZW1zIHRoYXQgaU1YNiBoYXMgaXNzdWVzIHdpdGgg UlhTRU5TRS9IUEQKPiA+Pj4gYm91bmNpbmcgd2hlbiB0aGUgUEhZIGlzIHBvd2VyZWQgZG93bi4g IEkgY2FuJ3QgcHJvbWlzZSB3aGVuIEknbGwgYmUKPiA+Pj4gYWJsZSB0byBjaGVjayBmb3IgdGhh dCBhZ2Fpbi4KPiA+PiAKPiA+PiBJbmRlZWQgVFhfUkVBRFkgbXVzdCBiZSBsb3cgYmVmb3JlIGFz c2VydGluZyBwZGRxLgo+ID4gCj4gPiBUaGUgVFhfUkVBRFkgc2lnbmFsIGlzIGRvY3VtZW50ZWQg aW4gdGhlIGkuTVg2IGRhdGFzaGVldCBhcyBiZWluZyBhIFBIWQo+ID4gb3V0cHV0IHNpZ25hbCwg YnV0IHRoZXJlIHNlZW1zIHRvIGJlIG5vIEhETUkgVFggcmVnaXN0ZXIgZnJvbSB3aGljaCBpdHMK PiA+IHN0YXRlIGNhbiBiZSByZWFkLiBEbyBJIG5lZWQgdG8gcG9sbCB0aGUgSERNSV9QSFlfUFRS UFRfRU5CTCByZWdpc3Rlcgo+ID4gdGhyb3VnaCBJMkMgPyBIb3cgbG9uZyBpcyB0aGUgUEhZIGV4 cGVjdGVkIHRvIHRha2UgdG8gc2V0IFRYX1JFQURZIHRvIDAgPwo+IAo+IFRYX1JFQURZIGNhbiBi ZSByZWFkIGZyb20gcmVnaXN0ZXIgMHgxQSBvZiBwaHksIEJJVCgyKSAodGhyb3VnaAo+IEkyQyku CgpUaGF0J3Mgd2hhdCBJIHRob3VnaHQsIEknbGwgcG9sbCB0aGF0IHRoZW4uIERvIHlvdSBoYXZl IGFueSBpZGVhIGhvdyBsb25nIGl0J3MgCnN1cHBvc2VkIHRvIHRha2UsIHRvIHNldCBhbiBhcHBy b3ByaWF0ZSB0aW1lb3V0ID8KCj4gTm90IHN1cmUgaWYgc2FtZSBvZmZzZXQgZm9yIGFsbCBwaHlz IHRob3VnaC4KCk1vc3QgcHJvYmFibHkgbm90LCBpdCB3b3VsZCBiZSB0b28gZWFzeSA6LSkgSSds bCBpbnZlc3RpZ2F0ZSAod2hpY2ggd2lsbCAKbGlrZWx5IGluY2x1ZGUgbG90cyBvZiBndWVzc3dv cmspLiBJZiB5b3UgY2FuIGZpbmQgYW55IGluZm9ybWF0aW9uIGFib3V0IHRoYXQgCihhbmQgZXNw ZWNpYWxseSBhYm91dCB0aGUgTUhMIGFuZCBIRE1JIDIuMCBQSFlzKSB0aGF0IHdvdWxkIGJlIHZl cnkgCmFwcHJlY2lhdGVkLCBhcyBJIGRvbid0IGhhdmUgYWNjZXNzIHRvIGFueSBkb2N1bWVudGF0 aW9uIHRoYXQgbWVudGlvbnMgYSAKVFhfUkVBRFkgYml0IGZvciB0aG9zZS4KCi0tIApSZWdhcmRz LAoKTGF1cmVudCBQaW5jaGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRl c2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8v ZHJpLWRldmVsCg==