From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 5/5 v3] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID Date: Tue, 17 Jan 2017 00:25:31 +0200 Message-ID: <1668382.5d9jBvLc1l@avalon> References: <1483472502-16403-1-git-send-email-john.stultz@linaro.org> <1965576.h4kdXt2VDW@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 [185.26.127.97]) by gabe.freedesktop.org (Postfix) with ESMTPS id F18C26E565 for ; Mon, 16 Jan 2017 22:25:18 +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: John Stultz Cc: Wolfram Sang , lkml , "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org SGkgSm9obiwKCk9uIE1vbmRheSAxNiBKYW4gMjAxNyAxMjoxNDo0OCBKb2huIFN0dWx0eiB3cm90 ZToKPiBPbiBNb24sIEphbiAxNiwgMjAxNyBhdCA4OjAzIEFNLCBMYXVyZW50IFBpbmNoYXJ0IHdy b3RlOgo+ID4gT24gVHVlc2RheSAwMyBKYW4gMjAxNyAxMTo0MTo0MiBKb2huIFN0dWx0eiB3cm90 ZToKPiA+PiBJJ3ZlIGZvdW5kIHRoYXQgYnkganVzdCB0dXJuaW5nIHRoZSBjaGlwIG9uIGFuZCBv ZmYgdmlhIHRoZQo+ID4+IFBPV0VSX0RPV04gcmVnaXN0ZXIsIEkgZW5kIHVwIGdldHRpbmcgaTJj X3RyYW5zZmVyIGVycm9ycwo+ID4+IG9uIEhpS2V5Lgo+ID4+IAo+ID4+IEludmVzdGlnYXRpbmcg ZnVydGhlciwgaXQgc2VlbXMgc29tZSBvZiB0aGUgcmVnaXN0ZXIgc3RhdGUKPiA+PiBpbiB0aGUg cmVnbWFwIGNhY2hlIGlzIHNvbWVob3cgZ2V0dGluZyBsb3N0LiBVc2luZyB0aGUgbG9naWMKPiA+ PiBpbiBfX2Fkdjc1MTFfcG93ZXJfb24vb2ZmKCkgd2hpY2ggc3luY3MgYW5kIGRpcnR5cyB0aGUg Y2FjaGUKPiA+PiBhdm9pZHMgdGhpcyBpc3N1ZS4KPiA+PiAKPiA+PiBUaHVzIHRoaXMgcGF0Y2gg Y2hhbmdlcyB0aGUgRURJRCBwcm9iaW5nIGxvZ2ljIHNvIHRoYXQgd2UKPiA+PiByZS11c2UgdGhl IF9fYWR2NzUxMV9wb3dlcl9vbi9vZmYoKSBjYWxscy4KPiA+IAo+ID4gcmVnY2FjaGVfc3luYygp IGlzIHF1aXRlIGNvc3RseSBhcyBpdCB3aWxsIHdyaXRlIGEgYnVuY2ggb2YgcmVnaXN0ZXJzLgo+ ID4gV291bGRuJ3QgaXQgYmUgbW9yZSBlZmZpY2llbnQgdG8gb25seSB3cml0ZSB0aGUgcmVnaXN0 ZXJzIHRoYXQgYXJlIG5lZWRlZAo+ID4gZm9yIEVESUQgYWNjZXNzID8KPiAKPiBTbyB5ZXMsIHlv dSd2ZSBtZW50aW9uZWQgdGhpcyBjb25jZXJuIGJlZm9yZSwgYW5kIEkgZGlkIHNwZW5kIHNvbWUK PiB0aW1lIHRvIG5hcnJvdyB3aGljaCBsb3N0LXJlZ2lzdGVyIHN0YXRlICgweDQzCj4gIC0gQURW NzUxMV9SRUdfRURJRF9JMkNfQUREUikgd2FzIGNhdXNpbmcgdGhlIHRyb3VibGUgd2l0aCBpMmMK PiB0cmFzbmZlciBlcnJvcnMgSSB3YXMgc2VlaW5nOgo+ICAgaHR0cHM6Ly9sa21sLm9yZy9sa21s LzIwMTYvMTEvMjIvNjc3Cj4gCj4gSG93ZXZlciwgSSBkaWRuJ3QgZ2V0IG11Y2ggZmVlZGJhY2sg b24gdGhhdCwgYW5kIGl0IHNlZW1zICh0byBtZSBhdAo+IGxlYXN0KSBjb25jZXJuaW5nIHRoYXQg d2UgYXJlIGxvc2luZyB0aGUgdW5kZXJseWluZyBzdGF0ZSBvZiBhCj4gcmVnaXN0ZXIgaW4gdGhl IGNhY2hlLCBzbyBqdXN0IHN5bmNpbmcgdGhhdCBvbmUgcmVnaXN0ZXIgYmFjayB0byB0aGUKPiBo YXJkd2FyZSBtaWdodCBzb2x2ZSB0aGUgaXNzdWUgSSB3YXMgc2VlaW5nLCBidXQgSSB3b3JyeSB3 aGF0IG90aGVyCj4gcmVnaXN0ZXJzIG1pZ2h0IGFsc28gYmUgb3V0IG9mIHN5bmMuCj4KPiBUaGUg Y29tbWVudCBhYm92ZSB0aGUgcmVnbWFwX3N5bmMgaW4gYWR2NzUxMV9wb3dlcl9vbiBhZnRlciBh bGwgc3RhdGVzOgo+ICAgICJNb3N0IG9mIHRoZSByZWdpc3RlcnMgYXJlIHJlc2V0IGR1cmluZyBw b3dlciBkb3duIG9yIHdoZW4gSFBEIGlzIGxvdy4iCgpZb3UncmUgcmlnaHQgdGhhdCBtb3N0IHJl Z2lzdGVycyB3aWxsIGJlIG91dCBvZiBzeW5jLgoKPiBTbyBpdCBzZWVtcyBsaWtlIGlmIHdlJ3Jl IHNldHRpbmcgdGhlIHBvd2VyIGRvd24gKGFuZCBzZXR0aW5nIEhQRCBpbgo+IGNhc2VzIHdoZXJl IEFyY2hpdCBoYWQgYSBwYXRjaCB0byBhZGQgSFBEIHB1bHNpbmcgdG8gdGhlCj4gYWR2NzUxMV9n ZXRfbW9kZXMgcGF0aCksIGl0IHNlZW1zIHJlYXNvbmFibGUgdG8gZG8gdGhlIHNhbWUKPiByZWdt YXBfc3luYygpPwoKSXQgd291bGQgYmUgaWYgd2UgaGFkIHRvIGtlZXAgdGhlIGRldmljZSBwb3dl cmVkIHVwLCBidXQgd2UncmUgcG93ZXJpbmcgaXQgCmRvd24gcmlnaHQgYWZ0ZXIgcmVhZGluZyB0 aGUgRURJRC4gSSBkb24ndCB0aGluayB0aGVyZSdzIGEgbmVlZCB0byByZWNvbmZpZ3VyZSAKaXQg Y29tcGxldGVseSwgb25seSBzZXR0aW5nIHRoZSByZWdpc3RlcnMgbmVlZGVkIHRvIHJlYWQgdGhl IEVESUQgc2hvdWxkIGJlIAplbm91Z2guCgo+IEJ1dCwgSSdtIG5vdCByZWFsbHkgcGlja3kgaGVy ZSwgYW5kIEknbSB2ZXJ5IG9wZW4gdG8gb3RoZXIgYXBwcm9hY2hlcwo+IChpbmNsdWRpbmcgc29t ZXRoaW5nIGxpa2UgdGhlIHBhdGNoIGluIHRoZSBsaW5rIGFib3ZlKSBpZiB5b3UgaGF2ZQo+IHN1 Z2dlc3Rpb25zL3ByZWZlcmVuY2VzLiBJIGp1c3Qgd2FudCBpdCB0byB3b3JrIHJlbGlhYmx5IG9u IG15Cj4gaGFyZHdhcmUuIDopCj4KPiBBbmQganVzdCBzbyBJIGNhbiBiZXR0ZXIgdW5kZXJzdGFu ZCBpdCwgY2FuIHlvdSBleHBsYWluIHNvbWUgYWJvdXQgdGhlCj4gaW1wYWN0IG9mIHlvdXIgZWZm aWNpZW5jeSBjb25jZXJucz8KCkknbSBub3QgdG9vIHBpY2t5IGVpdGhlciA6LSkgSWYgd2UgY2Fu J3QgZmluZCBhIHJlbGlhYmxlIHdheSB0byByZWFkIHRoZSBFRElEIApieSBqdXN0IGNvbmZpZ3Vy aW5nIHRoZSByZWdpc3RlcnMgd2UgbmVlZCwgd2UgY291bGQgZ28gZm9yIGEgZnVsbCAKcmVjb25m aWd1cmF0aW9uLiBIb3dldmVyLCByZXN0b3JpbmcgdGhlIHZhbHVlIG9mIGFsbCBjYWNoZWQgcmVn aXN0ZXJzIHdpbGwgCnJlc3VsdCBpbiBsb3RzIG9mIEkyQyB3cml0ZXMsIHdoaWNoIGFyZSB0aW1l LWNvbnN1bWluZyBvcGVyYXRpb25zLiBFRElEIHJlYWQgCndvdWxkIGJlIHNwZWQgdXAgaWYgd2Ug Y291bGQgYXZvaWQgdGhhdC4KCi0tIApSZWdhcmRzLAoKTGF1cmVudCBQaW5jaGFydAoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxp bmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJl ZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751475AbdAPWZX (ORCPT ); Mon, 16 Jan 2017 17:25:23 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:52677 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750865AbdAPWZU (ORCPT ); Mon, 16 Jan 2017 17:25:20 -0500 From: Laurent Pinchart To: John Stultz Cc: "dri-devel@lists.freedesktop.org" , lkml , Wolfram Sang Subject: Re: [PATCH 5/5 v3] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID Date: Tue, 17 Jan 2017 00:25:31 +0200 Message-ID: <1668382.5d9jBvLc1l@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: References: <1483472502-16403-1-git-send-email-john.stultz@linaro.org> <1965576.h4kdXt2VDW@avalon> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi John, On Monday 16 Jan 2017 12:14:48 John Stultz wrote: > On Mon, Jan 16, 2017 at 8:03 AM, Laurent Pinchart wrote: > > On Tuesday 03 Jan 2017 11:41:42 John Stultz wrote: > >> I've found that by just turning the chip on and off via the > >> POWER_DOWN register, I end up getting i2c_transfer errors > >> on HiKey. > >> > >> Investigating further, it seems some of the register state > >> in the regmap cache is somehow getting lost. Using the logic > >> in __adv7511_power_on/off() which syncs and dirtys the cache > >> avoids this issue. > >> > >> Thus this patch changes the EDID probing logic so that we > >> re-use the __adv7511_power_on/off() calls. > > > > regcache_sync() is quite costly as it will write a bunch of registers. > > Wouldn't it be more efficient to only write the registers that are needed > > for EDID access ? > > So yes, you've mentioned this concern before, and I did spend some > time to narrow which lost-register state (0x43 > - ADV7511_REG_EDID_I2C_ADDR) was causing the trouble with i2c > trasnfer errors I was seeing: > https://lkml.org/lkml/2016/11/22/677 > > However, I didn't get much feedback on that, and it seems (to me at > least) concerning that we are losing the underlying state of a > register in the cache, so just syncing that one register back to the > hardware might solve the issue I was seeing, but I worry what other > registers might also be out of sync. > > The comment above the regmap_sync in adv7511_power_on after all states: > "Most of the registers are reset during power down or when HPD is low." You're right that most registers will be out of sync. > So it seems like if we're setting the power down (and setting HPD in > cases where Archit had a patch to add HPD pulsing to the > adv7511_get_modes path), it seems reasonable to do the same > regmap_sync()? It would be if we had to keep the device powered up, but we're powering it down right after reading the EDID. I don't think there's a need to reconfigure it completely, only setting the registers needed to read the EDID should be enough. > But, I'm not really picky here, and I'm very open to other approaches > (including something like the patch in the link above) if you have > suggestions/preferences. I just want it to work reliably on my > hardware. :) > > And just so I can better understand it, can you explain some about the > impact of your efficiency concerns? I'm not too picky either :-) If we can't find a reliable way to read the EDID by just configuring the registers we need, we could go for a full reconfiguration. However, restoring the value of all cached registers will result in lots of I2C writes, which are time-consuming operations. EDID read would be sped up if we could avoid that. -- Regards, Laurent Pinchart