From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932930AbcKVRiW (ORCPT ); Tue, 22 Nov 2016 12:38:22 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:47812 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755328AbcKVRiV (ORCPT ); Tue, 22 Nov 2016 12:38:21 -0500 From: Laurent Pinchart To: John Stultz Cc: lkml , David Airlie , Archit Taneja , Wolfram Sang , Lars-Peter Clausen , "dri-devel@lists.freedesktop.org" Subject: Re: [RFC][PATCH 1/3] drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally Date: Tue, 22 Nov 2016 19:38:37 +0200 Message-ID: <1661379.b8y5I25JKx@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: References: <1479775052-28194-1-git-send-email-john.stultz@linaro.org> <1961666.AZMl081OYz@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 Tuesday 22 Nov 2016 09:25:22 John Stultz wrote: > On Tue, Nov 22, 2016 at 12:14 AM, Laurent Pinchart wrote: > > On Monday 21 Nov 2016 16:37:30 John Stultz wrote: > @@ -545,24 +554,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); > > > > The __adv7511_power_on() function does more than the above, in particular > > it performs an expensive regcache_sync() and calls adv7533_dsi_power_on() > > for the ADV7533. Don't those operations have side effects that are either > > not wanted or not needed here ? In any case this patch modifies the > > behaviour of the driver, which needs to be documented in the kernel > > message. > > So yes, while the adv7533 bits aren't needed in the internal function, > I'm finding the logic to pulse the HPD and the regcache_sync call seem > to be needed side effects, as without that logic, I get i2c_transfer() > errors in adv7511_get_edid_block(). Does this patch fix the problem without requiring the 200ms delay ? > I'll try to rework this patch to split the two changes of reworking > the power_on/off function to be re-used (with no logic chage), and the > patch to reuse it in get_modes() which resolves a bug. Have you identified which register write fixes your problem here ? > Thanks so much for the review! -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [RFC][PATCH 1/3] drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally Date: Tue, 22 Nov 2016 19:38:37 +0200 Message-ID: <1661379.b8y5I25JKx@avalon> References: <1479775052-28194-1-git-send-email-john.stultz@linaro.org> <1961666.AZMl081OYz@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 51ECF6E324 for ; Tue, 22 Nov 2016 17:38:20 +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: lkml , "dri-devel@lists.freedesktop.org" , Wolfram Sang List-Id: dri-devel@lists.freedesktop.org SGkgSm9obiwKCk9uIFR1ZXNkYXkgMjIgTm92IDIwMTYgMDk6MjU6MjIgSm9obiBTdHVsdHogd3Jv dGU6Cj4gT24gVHVlLCBOb3YgMjIsIDIwMTYgYXQgMTI6MTQgQU0sIExhdXJlbnQgUGluY2hhcnQg d3JvdGU6Cj4gPiBPbiBNb25kYXkgMjEgTm92IDIwMTYgMTY6Mzc6MzAgSm9obiBTdHVsdHogd3Jv dGU6Cj4gIEBAIC01NDUsMjQgKzU1NCwxMyBAQCBzdGF0aWMgaW50IGFkdjc1MTFfZ2V0X21vZGVz KHN0cnVjdCBhZHY3NTExICphZHY3NTExLAo+ID4+ICAgICAgIHVuc2lnbmVkIGludCBjb3VudDsK PiA+PiAgICAgICAKPiA+PiAgICAgICAvKiBSZWFkaW5nIHRoZSBFRElEIG9ubHkgd29ya3MgaWYg dGhlIGRldmljZSBpcyBwb3dlcmVkICovCj4gPj4gCj4gPj4gLSAgICAgaWYgKCFhZHY3NTExLT5w b3dlcmVkKSB7Cj4gPj4gLSAgICAgICAgICAgICByZWdtYXBfdXBkYXRlX2JpdHMoYWR2NzUxMS0+ cmVnbWFwLCBBRFY3NTExX1JFR19QT1dFUiwKPiA+PiAtICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICBBRFY3NTExX1BPV0VSX1BPV0VSX0RPV04sIDApOwo+ID4+IC0gICAgICAgICAgICAg aWYgKGFkdjc1MTEtPmkyY19tYWluLT5pcnEpIHsKPiA+PiAtICAgICAgICAgICAgICAgICAgICAg cmVnbWFwX3dyaXRlKGFkdjc1MTEtPnJlZ21hcCwKPiA+PiBBRFY3NTExX1JFR19JTlRfRU5BQkxF KDApLAo+ID4+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgQURWNzUxMV9JTlQw X0VESURfUkVBRFkpOwo+ID4+IC0gICAgICAgICAgICAgICAgICAgICByZWdtYXBfd3JpdGUoYWR2 NzUxMS0+cmVnbWFwLAo+ID4+IEFEVjc1MTFfUkVHX0lOVF9FTkFCTEUoMSksCj4gPj4gLSAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBBRFY3NTExX0lOVDFfRERDX0VSUk9SKTsKPiA+ PiAtICAgICAgICAgICAgIH0KPiA+PiAtICAgICAgICAgICAgIGFkdjc1MTEtPmN1cnJlbnRfZWRp ZF9zZWdtZW50ID0gLTE7Cj4gPj4gLSAgICAgfQo+ID4+ICsgICAgIGlmICghYWR2NzUxMS0+cG93 ZXJlZCkKPiA+PiArICAgICAgICAgICAgIF9fYWR2NzUxMV9wb3dlcl9vbihhZHY3NTExKTsKPiA+ IAo+ID4gVGhlIF9fYWR2NzUxMV9wb3dlcl9vbigpIGZ1bmN0aW9uIGRvZXMgbW9yZSB0aGFuIHRo ZSBhYm92ZSwgaW4gcGFydGljdWxhcgo+ID4gaXQgcGVyZm9ybXMgYW4gZXhwZW5zaXZlIHJlZ2Nh Y2hlX3N5bmMoKSBhbmQgY2FsbHMgYWR2NzUzM19kc2lfcG93ZXJfb24oKQo+ID4gZm9yIHRoZSBB RFY3NTMzLiBEb24ndCB0aG9zZSBvcGVyYXRpb25zIGhhdmUgc2lkZSBlZmZlY3RzIHRoYXQgYXJl IGVpdGhlcgo+ID4gbm90IHdhbnRlZCBvciBub3QgbmVlZGVkIGhlcmUgPyBJbiBhbnkgY2FzZSB0 aGlzIHBhdGNoIG1vZGlmaWVzIHRoZQo+ID4gYmVoYXZpb3VyIG9mIHRoZSBkcml2ZXIsIHdoaWNo IG5lZWRzIHRvIGJlIGRvY3VtZW50ZWQgaW4gdGhlIGtlcm5lbAo+ID4gbWVzc2FnZS4KPiAKPiBT byB5ZXMsIHdoaWxlIHRoZSBhZHY3NTMzIGJpdHMgYXJlbid0IG5lZWRlZCBpbiB0aGUgaW50ZXJu YWwgZnVuY3Rpb24sCj4gSSdtIGZpbmRpbmcgdGhlIGxvZ2ljIHRvIHB1bHNlIHRoZSBIUEQgYW5k IHRoZSByZWdjYWNoZV9zeW5jIGNhbGwgc2VlbQo+IHRvIGJlIG5lZWRlZCBzaWRlIGVmZmVjdHMs IGFzIHdpdGhvdXQgdGhhdCBsb2dpYywgSSBnZXQgaTJjX3RyYW5zZmVyKCkKPiBlcnJvcnMgaW4g YWR2NzUxMV9nZXRfZWRpZF9ibG9jaygpLgoKRG9lcyB0aGlzIHBhdGNoIGZpeCB0aGUgcHJvYmxl bSB3aXRob3V0IHJlcXVpcmluZyB0aGUgMjAwbXMgZGVsYXkgPwoKPiBJJ2xsIHRyeSB0byByZXdv cmsgdGhpcyBwYXRjaCB0byBzcGxpdCB0aGUgdHdvIGNoYW5nZXMgb2YgcmV3b3JraW5nCj4gdGhl IHBvd2VyX29uL29mZiBmdW5jdGlvbiB0byBiZSByZS11c2VkICh3aXRoIG5vIGxvZ2ljIGNoYWdl KSwgYW5kIHRoZQo+IHBhdGNoIHRvIHJldXNlIGl0IGluIGdldF9tb2RlcygpIHdoaWNoIHJlc29s dmVzIGEgYnVnLgoKSGF2ZSB5b3UgaWRlbnRpZmllZCB3aGljaCByZWdpc3RlciB3cml0ZSBmaXhl cyB5b3VyIHByb2JsZW0gaGVyZSA/Cgo+IFRoYW5rcyBzbyBtdWNoIGZvciB0aGUgcmV2aWV3IQoK LS0gClJlZ2FyZHMsCgpMYXVyZW50IFBpbmNoYXJ0CgpfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBs aXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1h bi9saXN0aW5mby9kcmktZGV2ZWwK