From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from galahad.ideasonboard.com ([185.26.127.97]:41612 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750796AbeAPWUl (ORCPT ); Tue, 16 Jan 2018 17:20:41 -0500 From: Laurent Pinchart To: Sergei Shtylyov Cc: Laurent Pinchart , dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH v2 2/2] drm: rcar-du: lvds: Fix LVDS startup on R-Car gen2 Date: Wed, 17 Jan 2018 00:20:45 +0200 Message-ID: <2846660.RJnc8Crq0n@avalon> In-Reply-To: <1078e267-c900-6caf-5787-de078f5b6557@cogentembedded.com> References: <11845752.a3eY2rs97p@avalon> <1635865.FStOqVGXHX@avalon> <1078e267-c900-6caf-5787-de078f5b6557@cogentembedded.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Sergei, On Tuesday, 16 January 2018 22:17:31 EET Sergei Shtylyov wrote: > On 01/16/2018 06:46 PM, Laurent Pinchart wrote: > >>> From: Sergei Shtylyov > >>> > >>> According to the latest revision 2.00 of the R-Car gen2 manual, the LVDS > >>> must be enabled and the bias crcuit enabled after the LVDS I/O pins are > > Oops, this needs fixing (note the typo!). Could you please change this > passage to: > > and the bias circuit must be enabled after the LVDS I/O pins are > > ? Sure I'll fix that. > >>> enabled, not before. Fix the gen2 LVDS startup sequence accordingly. > >>> > >>> While at it, also fix the comment preceding the first LVDCR0 write that > >>> still talks about hardcoding the LVDS mode 0. > >> > >> Please do this in a separate commit then... > > > > The reason I added it here is that I think we don't need patch 1/2 from > > this series, and I found a bit overkill to split a comment fix to a > > separate patch when we have a patch that touches the code around the > > comment. > > OK, you're the maintainer of this driver, you know better. :-) > > >>> Fixes: 90374b5c25c9 ("drm/rcar-du: Add internal LVDS encoder support") > >> > >> You forgot to specify the other commit this one fixes -- I mean the > >> comment fix. > > > > Do we need to for a comment update ? It doesn't affect fix the behaviour > > of the driver or device, and I'd thus prefer to avoid giving the wrong > > impression that this patch fixes an bug introduced in a previous commit, > > otherwise it might end up being backported unnecessarily. > > OK, no dire need to backport indeed... > > >>> Signed-off-by: Sergei Shtylyov > >>> Reviewed-by: Laurent Pinchart > >>> > >>> [Set the mode and input at the same time as the BEN and LVEN bits] > >>> Tested-by: Laurent Pinchart > >>> Signed-off-by: Laurent Pinchart > >>> > >>> --- > >>> > >>> drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 14 +++++++------- > >>> 1 file changed, 7 insertions(+), 7 deletions(-) > >>> > >>> Hi Sergei, > >>> > >>> For your convenience (and if you agree with bundling mode setup with the > >>> first write as explained in my review of patch 1/2), here's the updated > >>> version of patch 2/2 that I have taken in my development branch. If > >>> you're fine with it I'll keep it, otherwise we can continue the review > >>> discussion. > >> > >> As I said, I don't know how to interpret the note 3 in either manual. > > Moreover, it seems to me that the notes don't match the start-up procedure > anymore... How so ? > > As explained in my latest reply to patch 1/2, my understanding is that the > > parameters can be programmed at any time before step 6. The fact that the > > current code works seems to confirm that interpretation. We could ask > > Renesas for a confirmation if you want. > > Would be good to ask, indeed. I'll ask. -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 2/2] drm: rcar-du: lvds: Fix LVDS startup on R-Car gen2 Date: Wed, 17 Jan 2018 00:20:45 +0200 Message-ID: <2846660.RJnc8Crq0n@avalon> References: <11845752.a3eY2rs97p@avalon> <1635865.FStOqVGXHX@avalon> <1078e267-c900-6caf-5787-de078f5b6557@cogentembedded.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 CD4A76E2AA for ; Tue, 16 Jan 2018 22:20:42 +0000 (UTC) In-Reply-To: <1078e267-c900-6caf-5787-de078f5b6557@cogentembedded.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sergei Shtylyov Cc: linux-renesas-soc@vger.kernel.org, Laurent Pinchart , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org SGkgU2VyZ2VpLAoKT24gVHVlc2RheSwgMTYgSmFudWFyeSAyMDE4IDIyOjE3OjMxIEVFVCBTZXJn ZWkgU2h0eWx5b3Ygd3JvdGU6Cj4gT24gMDEvMTYvMjAxOCAwNjo0NiBQTSwgTGF1cmVudCBQaW5j aGFydCB3cm90ZToKPiA+Pj4gRnJvbTogU2VyZ2VpIFNodHlseW92IDxzZXJnZWkuc2h0eWx5b3ZA Y29nZW50ZW1iZWRkZWQuY29tPgo+ID4+PiAKPiA+Pj4gQWNjb3JkaW5nIHRvIHRoZSBsYXRlc3Qg cmV2aXNpb24gMi4wMCBvZiB0aGUgUi1DYXIgZ2VuMiBtYW51YWwsIHRoZSBMVkRTCj4gPj4+IG11 c3QgYmUgZW5hYmxlZCBhbmQgdGhlIGJpYXMgY3JjdWl0IGVuYWJsZWQgYWZ0ZXIgdGhlIExWRFMg SS9PIHBpbnMgYXJlCj4gCj4gT29wcywgdGhpcyBuZWVkcyBmaXhpbmcgKG5vdGUgdGhlIHR5cG8h KS4gQ291bGQgeW91IHBsZWFzZSBjaGFuZ2UgdGhpcwo+IHBhc3NhZ2UgdG86Cj4gCj4gYW5kIHRo ZSBiaWFzIGNpcmN1aXQgbXVzdCBiZSBlbmFibGVkIGFmdGVyIHRoZSBMVkRTIEkvTyBwaW5zIGFy ZQo+IAo+ID8KClN1cmUgSSdsbCBmaXggdGhhdC4KCj4gPj4+IGVuYWJsZWQsIG5vdCBiZWZvcmUu IEZpeCB0aGUgZ2VuMiBMVkRTIHN0YXJ0dXAgc2VxdWVuY2UgYWNjb3JkaW5nbHkuCj4gPj4+IAo+ ID4+PiBXaGlsZSBhdCBpdCwgYWxzbyBmaXggdGhlIGNvbW1lbnQgcHJlY2VkaW5nIHRoZSBmaXJz dCBMVkRDUjAgd3JpdGUgdGhhdAo+ID4+PiBzdGlsbCB0YWxrcyBhYm91dCBoYXJkY29kaW5nIHRo ZSBMVkRTIG1vZGUgMC4KPiA+PiAKPiA+PiBQbGVhc2UgZG8gdGhpcyBpbiBhIHNlcGFyYXRlIGNv bW1pdCB0aGVuLi4uCj4gPiAKPiA+IFRoZSByZWFzb24gSSBhZGRlZCBpdCBoZXJlIGlzIHRoYXQg SSB0aGluayB3ZSBkb24ndCBuZWVkIHBhdGNoIDEvMiBmcm9tCj4gPiB0aGlzIHNlcmllcywgYW5k IEkgZm91bmQgYSBiaXQgb3ZlcmtpbGwgdG8gc3BsaXQgYSBjb21tZW50IGZpeCB0byBhCj4gPiBz ZXBhcmF0ZSBwYXRjaCB3aGVuIHdlIGhhdmUgYSBwYXRjaCB0aGF0IHRvdWNoZXMgdGhlIGNvZGUg YXJvdW5kIHRoZQo+ID4gY29tbWVudC4KPiAKPiBPSywgeW91J3JlIHRoZSBtYWludGFpbmVyIG9m IHRoaXMgZHJpdmVyLCB5b3Uga25vdyBiZXR0ZXIuIDotKQo+IAo+ID4+PiBGaXhlczogOTAzNzRi NWMyNWM5ICgiZHJtL3JjYXItZHU6IEFkZCBpbnRlcm5hbCBMVkRTIGVuY29kZXIgc3VwcG9ydCIp Cj4gPj4gCj4gPj4gWW91IGZvcmdvdCB0byBzcGVjaWZ5IHRoZSBvdGhlciBjb21taXQgdGhpcyBv bmUgZml4ZXMgLS0gSSBtZWFuIHRoZQo+ID4+IGNvbW1lbnQgZml4Lgo+ID4gCj4gPiBEbyB3ZSBu ZWVkIHRvIGZvciBhIGNvbW1lbnQgdXBkYXRlID8gSXQgZG9lc24ndCBhZmZlY3QgZml4IHRoZSBi ZWhhdmlvdXIKPiA+IG9mIHRoZSBkcml2ZXIgb3IgZGV2aWNlLCBhbmQgSSdkIHRodXMgcHJlZmVy IHRvIGF2b2lkIGdpdmluZyB0aGUgd3JvbmcKPiA+IGltcHJlc3Npb24gdGhhdCB0aGlzIHBhdGNo IGZpeGVzIGFuIGJ1ZyBpbnRyb2R1Y2VkIGluIGEgcHJldmlvdXMgY29tbWl0LAo+ID4gb3RoZXJ3 aXNlIGl0IG1pZ2h0IGVuZCB1cCBiZWluZyBiYWNrcG9ydGVkIHVubmVjZXNzYXJpbHkuCj4gCj4g ICAgIE9LLCBubyBkaXJlIG5lZWQgdG8gYmFja3BvcnQgaW5kZWVkLi4uCj4gCj4gPj4+IFNpZ25l ZC1vZmYtYnk6IFNlcmdlaSBTaHR5bHlvdiA8c2VyZ2VpLnNodHlseW92QGNvZ2VudGVtYmVkZGVk LmNvbT4KPiA+Pj4gUmV2aWV3ZWQtYnk6IExhdXJlbnQgUGluY2hhcnQKPiA+Pj4gPGxhdXJlbnQu cGluY2hhcnQrcmVuZXNhc0BpZGVhc29uYm9hcmQuY29tPgo+ID4+PiBbU2V0IHRoZSBtb2RlIGFu ZCBpbnB1dCBhdCB0aGUgc2FtZSB0aW1lIGFzIHRoZSBCRU4gYW5kIExWRU4gYml0c10KPiA+Pj4g VGVzdGVkLWJ5OiBMYXVyZW50IFBpbmNoYXJ0IDxsYXVyZW50LnBpbmNoYXJ0K3JlbmVzYXNAaWRl YXNvbmJvYXJkLmNvbT4KPiA+Pj4gU2lnbmVkLW9mZi1ieTogTGF1cmVudCBQaW5jaGFydAo+ID4+ PiA8bGF1cmVudC5waW5jaGFydCtyZW5lc2FzQGlkZWFzb25ib2FyZC5jb20+Cj4gPj4+IC0tLQo+ ID4+PiAKPiA+Pj4gICAgZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9sdmRzZW5jLmMg fCAxNCArKysrKysrLS0tLS0tLQo+ID4+PiAgICAxIGZpbGUgY2hhbmdlZCwgNyBpbnNlcnRpb25z KCspLCA3IGRlbGV0aW9ucygtKQo+ID4+PiAKPiA+Pj4gSGkgU2VyZ2VpLAo+ID4+PiAKPiA+Pj4g Rm9yIHlvdXIgY29udmVuaWVuY2UgKGFuZCBpZiB5b3UgYWdyZWUgd2l0aCBidW5kbGluZyBtb2Rl IHNldHVwIHdpdGggdGhlCj4gPj4+IGZpcnN0IHdyaXRlIGFzIGV4cGxhaW5lZCBpbiBteSByZXZp ZXcgb2YgcGF0Y2ggMS8yKSwgaGVyZSdzIHRoZSB1cGRhdGVkCj4gPj4+IHZlcnNpb24gb2YgcGF0 Y2ggMi8yIHRoYXQgSSBoYXZlIHRha2VuIGluIG15IGRldmVsb3BtZW50IGJyYW5jaC4gSWYKPiA+ Pj4geW91J3JlIGZpbmUgd2l0aCBpdCBJJ2xsIGtlZXAgaXQsIG90aGVyd2lzZSB3ZSBjYW4gY29u dGludWUgdGhlIHJldmlldwo+ID4+PiBkaXNjdXNzaW9uLgo+ID4+IAo+ID4+IEFzIEkgc2FpZCwg SSBkb24ndCBrbm93IGhvdyB0byBpbnRlcnByZXQgdGhlIG5vdGUgMyBpbiBlaXRoZXIgbWFudWFs Lgo+IAo+IE1vcmVvdmVyLCBpdCBzZWVtcyB0byBtZSB0aGF0IHRoZSBub3RlcyBkb24ndCBtYXRj aCB0aGUgc3RhcnQtdXAgcHJvY2VkdXJlCj4gYW55bW9yZS4uLgoKSG93IHNvID8KCj4gPiBBcyBl eHBsYWluZWQgaW4gbXkgbGF0ZXN0IHJlcGx5IHRvIHBhdGNoIDEvMiwgbXkgdW5kZXJzdGFuZGlu ZyBpcyB0aGF0IHRoZQo+ID4gcGFyYW1ldGVycyBjYW4gYmUgcHJvZ3JhbW1lZCBhdCBhbnkgdGlt ZSBiZWZvcmUgc3RlcCA2LiBUaGUgZmFjdCB0aGF0IHRoZQo+ID4gY3VycmVudCBjb2RlIHdvcmtz IHNlZW1zIHRvIGNvbmZpcm0gdGhhdCBpbnRlcnByZXRhdGlvbi4gV2UgY291bGQgYXNrCj4gPiBS ZW5lc2FzIGZvciBhIGNvbmZpcm1hdGlvbiBpZiB5b3Ugd2FudC4KPiAKPiBXb3VsZCBiZSBnb29k IHRvIGFzaywgaW5kZWVkLgoKSSdsbCBhc2suCgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hh cnQKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1k ZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczov L2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=