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: Mon, 16 Jan 2017 18:03:11 +0200 Message-ID: <1965576.h4kdXt2VDW@avalon> References: <1483472502-16403-1-git-send-email-john.stultz@linaro.org> <1483472502-16403-6-git-send-email-john.stultz@linaro.org> 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 901046E3FD for ; Mon, 16 Jan 2017 16:02:56 +0000 (UTC) In-Reply-To: <1483472502-16403-6-git-send-email-john.stultz@linaro.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: dri-devel@lists.freedesktop.org Cc: Wolfram Sang , lkml List-Id: dri-devel@lists.freedesktop.org SGkgSm9obiwKClRoYW5rIHlvdSBmb3IgdGhlIHBhdGNoLgoKT24gVHVlc2RheSAwMyBKYW4gMjAx NyAxMTo0MTo0MiBKb2huIFN0dWx0eiB3cm90ZToKPiBJJ3ZlIGZvdW5kIHRoYXQgYnkganVzdCB0 dXJuaW5nIHRoZSBjaGlwIG9uIGFuZCBvZmYgdmlhIHRoZQo+IFBPV0VSX0RPV04gcmVnaXN0ZXIs IEkgZW5kIHVwIGdldHRpbmcgaTJjX3RyYW5zZmVyIGVycm9ycwo+IG9uIEhpS2V5Lgo+IAo+IElu dmVzdGlnYXRpbmcgZnVydGhlciwgaXQgc2VlbXMgc29tZSBvZiB0aGUgcmVnaXN0ZXIgc3RhdGUK PiBpbiB0aGUgcmVnbWFwIGNhY2hlIGlzIHNvbWVob3cgZ2V0dGluZyBsb3N0LiBVc2luZyB0aGUg bG9naWMKPiBpbiBfX2Fkdjc1MTFfcG93ZXJfb24vb2ZmKCkgd2hpY2ggc3luY3MgYW5kIGRpcnR5 cyB0aGUgY2FjaGUKPiBhdm9pZHMgdGhpcyBpc3N1ZS4KPiAKPiBUaHVzIHRoaXMgcGF0Y2ggY2hh bmdlcyB0aGUgRURJRCBwcm9iaW5nIGxvZ2ljIHNvIHRoYXQgd2UKPiByZS11c2UgdGhlIF9fYWR2 NzUxMV9wb3dlcl9vbi9vZmYoKSBjYWxscy4KCnJlZ2NhY2hlX3N5bmMoKSBpcyBxdWl0ZSBjb3N0 bHkgYXMgaXQgd2lsbCB3cml0ZSBhIGJ1bmNoIG9mIHJlZ2lzdGVycy4gCldvdWxkbid0IGl0IGJl IG1vcmUgZWZmaWNpZW50IHRvIG9ubHkgd3JpdGUgdGhlIHJlZ2lzdGVycyB0aGF0IGFyZSBuZWVk ZWQgZm9yIApFRElEIGFjY2VzcyA/Cgo+IENjOiBEYXZpZCBBaXJsaWUgPGFpcmxpZWRAbGludXgu aWU+Cj4gQ2M6IEFyY2hpdCBUYW5lamEgPGFyY2hpdHRAY29kZWF1cm9yYS5vcmc+Cj4gQ2M6IFdv bGZyYW0gU2FuZyA8d3NhK3JlbmVzYXNAc2FuZy1lbmdpbmVlcmluZy5jb20+Cj4gQ2M6IExhcnMt UGV0ZXIgQ2xhdXNlbiA8bGFyc0BtZXRhZm9vLmRlPgo+IENjOiBMYXVyZW50IFBpbmNoYXJ0IDxs YXVyZW50LnBpbmNoYXJ0QGlkZWFzb25ib2FyZC5jb20+Cj4gQ2M6IGRyaS1kZXZlbEBsaXN0cy5m cmVlZGVza3RvcC5vcmcKPiBTaWduZWQtb2ZmLWJ5OiBKb2huIFN0dWx0eiA8am9obi5zdHVsdHpA bGluYXJvLm9yZz4KPiAtLS0KPiAgZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9hZHY3NTExL2Fkdjc1 MTFfZHJ2LmMgfCAxNyArKystLS0tLS0tLS0tLS0tLQo+ICAxIGZpbGUgY2hhbmdlZCwgMyBpbnNl cnRpb25zKCspLCAxNCBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUv ZHJtL2JyaWRnZS9hZHY3NTExL2Fkdjc1MTFfZHJ2LmMKPiBiL2RyaXZlcnMvZ3B1L2RybS9icmlk Z2UvYWR2NzUxMS9hZHY3NTExX2Rydi5jIGluZGV4IGRiZGI3MWMuLjI0NTczZTAKPiAxMDA2NDQK PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2Fkdjc1MTEvYWR2NzUxMV9kcnYuYwo+ICsr KyBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvYWR2NzUxMS9hZHY3NTExX2Rydi5jCj4gQEAgLTU3 MiwyNCArNTcyLDEzIEBAIHN0YXRpYyBpbnQgYWR2NzUxMV9nZXRfbW9kZXMoc3RydWN0IGFkdjc1 MTEgKmFkdjc1MTEsCj4gIAl1bnNpZ25lZCBpbnQgY291bnQ7Cj4gCj4gIAkvKiBSZWFkaW5nIHRo ZSBFRElEIG9ubHkgd29ya3MgaWYgdGhlIGRldmljZSBpcyBwb3dlcmVkICovCj4gLQlpZiAoIWFk djc1MTEtPnBvd2VyZWQpIHsKPiAtCQlyZWdtYXBfdXBkYXRlX2JpdHMoYWR2NzUxMS0+cmVnbWFw LCBBRFY3NTExX1JFR19QT1dFUiwKPiAtCQkJCSAgIEFEVjc1MTFfUE9XRVJfUE9XRVJfRE9XTiwg MCk7Cj4gLQkJaWYgKGFkdjc1MTEtPmkyY19tYWluLT5pcnEpIHsKPiAtCQkJcmVnbWFwX3dyaXRl KGFkdjc1MTEtPnJlZ21hcCwgCkFEVjc1MTFfUkVHX0lOVF9FTkFCTEUoMCksCj4gLQkJCQkgICAg IEFEVjc1MTFfSU5UMF9FRElEX1JFQURZKTsKPiAtCQkJcmVnbWFwX3dyaXRlKGFkdjc1MTEtPnJl Z21hcCwgCkFEVjc1MTFfUkVHX0lOVF9FTkFCTEUoMSksCj4gLQkJCQkgICAgIEFEVjc1MTFfSU5U MV9ERENfRVJST1IpOwo+IC0JCX0KPiAtCQlhZHY3NTExLT5jdXJyZW50X2VkaWRfc2VnbWVudCA9 IC0xOwo+IC0JfQo+ICsJaWYgKCFhZHY3NTExLT5wb3dlcmVkKQo+ICsJCV9fYWR2NzUxMV9wb3dl cl9vbihhZHY3NTExKTsKPiAKPiAgCWVkaWQgPSBkcm1fZG9fZ2V0X2VkaWQoY29ubmVjdG9yLCBh ZHY3NTExX2dldF9lZGlkX2Jsb2NrLCBhZHY3NTExKTsKPiAKPiAgCWlmICghYWR2NzUxMS0+cG93 ZXJlZCkKPiAtCQlyZWdtYXBfdXBkYXRlX2JpdHMoYWR2NzUxMS0+cmVnbWFwLCBBRFY3NTExX1JF R19QT1dFUiwKPiAtCQkJCSAgIEFEVjc1MTFfUE9XRVJfUE9XRVJfRE9XTiwKPiAtCQkJCSAgIEFE Vjc1MTFfUE9XRVJfUE9XRVJfRE9XTik7Cj4gKwkJX19hZHY3NTExX3Bvd2VyX29mZihhZHY3NTEx KTsKPiAKPiAgCWtmcmVlKGFkdjc1MTEtPmVkaWQpOwo+ICAJYWR2NzUxMS0+ZWRpZCA9IGVkaWQ7 CgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVs QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWls bWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751218AbdAPQC6 (ORCPT ); Mon, 16 Jan 2017 11:02:58 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:51167 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750945AbdAPQC5 (ORCPT ); Mon, 16 Jan 2017 11:02:57 -0500 From: Laurent Pinchart To: dri-devel@lists.freedesktop.org Cc: John Stultz , lkml , Wolfram Sang Subject: Re: [PATCH 5/5 v3] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID Date: Mon, 16 Jan 2017 18:03:11 +0200 Message-ID: <1965576.h4kdXt2VDW@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: <1483472502-16403-6-git-send-email-john.stultz@linaro.org> References: <1483472502-16403-1-git-send-email-john.stultz@linaro.org> <1483472502-16403-6-git-send-email-john.stultz@linaro.org> 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, Thank you for the patch. 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 ? > Cc: David Airlie > Cc: Archit Taneja > Cc: Wolfram Sang > Cc: Lars-Peter Clausen > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 17 +++-------------- > 1 file changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index dbdb71c..24573e0 > 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -572,24 +572,13 @@ static int adv7511_get_modes(struct adv7511 *adv7511, > unsigned int count; > > /* Reading the EDID only works if the device is powered */ > - if (!adv7511->powered) { > - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > - ADV7511_POWER_POWER_DOWN, 0); > - if (adv7511->i2c_main->irq) { > - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), > - ADV7511_INT0_EDID_READY); > - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), > - ADV7511_INT1_DDC_ERROR); > - } > - adv7511->current_edid_segment = -1; > - } > + if (!adv7511->powered) > + __adv7511_power_on(adv7511); > > edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); > > if (!adv7511->powered) > - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > - ADV7511_POWER_POWER_DOWN, > - ADV7511_POWER_POWER_DOWN); > + __adv7511_power_off(adv7511); > > kfree(adv7511->edid); > adv7511->edid = edid; -- Regards, Laurent Pinchart