From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from galahad.ideasonboard.com ([185.26.127.97]:39367 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965156AbeALWP7 (ORCPT ); Fri, 12 Jan 2018 17:15:59 -0500 From: Laurent Pinchart To: Sergei Shtylyov Cc: David Airlie , dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH 1/2] drm: rcar-du: lvds: fix LVDS startup on R-Car gen3 Date: Sat, 13 Jan 2018 00:15:59 +0200 Message-ID: <3090757.msatNG3mHi@avalon> In-Reply-To: <2286420.LkWkKn4WW8@avalon> References: <20180112201550.368547609@cogentembedded.com> <2286420.LkWkKn4WW8@avalon> 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 Friday, 12 January 2018 23:51:35 EET Laurent Pinchart wrote: > On Friday, 12 January 2018 22:12:04 EET Sergei Shtylyov wrote: > > According to the latest revisions of the R-Car gen3 manual, the LVDS mode > > must be set before the LVDS I/O pins are enabled, not after -- fix the > > gen3 LVDS startup sequence accordingly... > > > > While at it, also fix the comment preceding the first LVDCR0 write in > > the R-Car gen2 startup code that still talks about hardcoding the LVDS > > mode 0... > > How about fixing that in patch 2/2 that touches the Gen2 initialization > sequence ? I think I'd even go as far as squashing both patches, I don't > think there's a need to split them. > > > Fixes: e947eccbeba4 ("drm: rcar-du: Add support for LVDS mode selection") > > Signed-off-by: Sergei Shtylyov > > Is this really needed ? Does it fix a problem you've experienced, or is it > theoretical only ? The mode shouldn't matter before the LVDS internal logic > is turned on. Unless there's a real issue I'm not sure we should make the > code more complex. Furthermore the datasheet states "3. This refers to settings other than those that are concerned with LVDS-IF startup. These items may be set while waiting for the conditions of step 6 to be met." Doesn't this mean that the mode and input selector don't have to be set as the very first step, but can be programmed at any point before starting the LVDS encoder through the PWD bit (on Gen3) or the PLLON bit (on Gen2) ? > > --- > > > > drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c > > =================================================================== > > --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c > > +++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c > > @@ -60,8 +60,8 @@ static void rcar_du_lvdsenc_start_gen2(s > > rcar_lvds_write(lvds, LVDPLLCR, pllcr); > > > > /* > > - * Select the input, hardcode mode 0, enable LVDS operation and turn > > - * bias circuitry on. > > + * Set the LVDS mode, select the input, enable LVDS operation, > > + * and turn bias circuitry on. > > */ > > lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN; > > if (rcrtc->index == 2) > > @@ -106,6 +106,9 @@ static void rcar_du_lvdsenc_start_gen3(s > > > > rcar_lvds_write(lvds, LVDPLLCR, pllcr); > > > > + lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; > > + rcar_lvds_write(lvds, LVDCR0, lvdcr0); > > + > > /* Turn all the channels on. */ > > rcar_lvds_write(lvds, LVDCR1, > > LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) | > > @@ -115,7 +118,8 @@ static void rcar_du_lvdsenc_start_gen3(s > > * Turn the PLL on, set it to LVDS normal mode, wait for the startup > > * delay and turn the output on. > > */ > > - lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON; > > + > > + lvdcr0 = | LVDCR0_PLLON; > > rcar_lvds_write(lvds, LVDCR0, lvdcr0); > > > > lvdcr0 |= LVDCR0_PWD; -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 1/2] drm: rcar-du: lvds: fix LVDS startup on R-Car gen3 Date: Sat, 13 Jan 2018 00:15:59 +0200 Message-ID: <3090757.msatNG3mHi@avalon> References: <20180112201550.368547609@cogentembedded.com> <2286420.LkWkKn4WW8@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 A9B636E55E for ; Fri, 12 Jan 2018 22:15:59 +0000 (UTC) In-Reply-To: <2286420.LkWkKn4WW8@avalon> 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: David Airlie , linux-renesas-soc@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org SGkgU2VyZ2VpLAoKT24gRnJpZGF5LCAxMiBKYW51YXJ5IDIwMTggMjM6NTE6MzUgRUVUIExhdXJl bnQgUGluY2hhcnQgd3JvdGU6Cj4gT24gRnJpZGF5LCAxMiBKYW51YXJ5IDIwMTggMjI6MTI6MDQg RUVUIFNlcmdlaSBTaHR5bHlvdiB3cm90ZToKPiA+IEFjY29yZGluZyB0byB0aGUgbGF0ZXN0IHJl dmlzaW9ucyBvZiB0aGUgUi1DYXIgZ2VuMyBtYW51YWwsIHRoZSBMVkRTIG1vZGUKPiA+IG11c3Qg YmUgc2V0IGJlZm9yZSB0aGUgTFZEUyBJL08gcGlucyBhcmUgZW5hYmxlZCwgbm90IGFmdGVyIC0t ICBmaXggIHRoZQo+ID4gZ2VuMyBMVkRTIHN0YXJ0dXAgc2VxdWVuY2UgYWNjb3JkaW5nbHkuLi4K PiA+IAo+ID4gV2hpbGUgIGF0IGl0LCAgYWxzbyBmaXggdGhlIGNvbW1lbnQgIHByZWNlZGluZyB0 aGUgZmlyc3QgTFZEQ1IwIHdyaXRlIGluCj4gPiB0aGUgUi1DYXIgZ2VuMiBzdGFydHVwIGNvZGUg dGhhdCBzdGlsbCB0YWxrcyBhYm91dCBoYXJkY29kaW5nIHRoZSBMVkRTCj4gPiBtb2RlIDAuLi4K PiAKPiBIb3cgYWJvdXQgZml4aW5nIHRoYXQgaW4gcGF0Y2ggMi8yIHRoYXQgdG91Y2hlcyB0aGUg R2VuMiBpbml0aWFsaXphdGlvbgo+IHNlcXVlbmNlID8gSSB0aGluayBJJ2QgZXZlbiBnbyBhcyBm YXIgYXMgc3F1YXNoaW5nIGJvdGggcGF0Y2hlcywgSSBkb24ndAo+IHRoaW5rIHRoZXJlJ3MgYSBu ZWVkIHRvIHNwbGl0IHRoZW0uCj4gCj4gPiBGaXhlczogZTk0N2VjY2JlYmE0ICgiZHJtOiByY2Fy LWR1OiBBZGQgc3VwcG9ydCBmb3IgTFZEUyBtb2RlIHNlbGVjdGlvbiIpCj4gPiBTaWduZWQtb2Zm LWJ5OiBTZXJnZWkgU2h0eWx5b3YgPHNlcmdlaS5zaHR5bHlvdkBjb2dlbnRlbWJlZGRlZC5jb20+ Cj4gCj4gSXMgdGhpcyByZWFsbHkgbmVlZGVkID8gRG9lcyBpdCBmaXggYSBwcm9ibGVtIHlvdSd2 ZSBleHBlcmllbmNlZCwgb3IgaXMgaXQKPiB0aGVvcmV0aWNhbCBvbmx5ID8gVGhlIG1vZGUgc2hv dWxkbid0IG1hdHRlciBiZWZvcmUgdGhlIExWRFMgaW50ZXJuYWwgbG9naWMKPiBpcyB0dXJuZWQg b24uIFVubGVzcyB0aGVyZSdzIGEgcmVhbCBpc3N1ZSBJJ20gbm90IHN1cmUgd2Ugc2hvdWxkIG1h a2UgdGhlCj4gY29kZSBtb3JlIGNvbXBsZXguCgpGdXJ0aGVybW9yZSB0aGUgZGF0YXNoZWV0IHN0 YXRlcwoKIjMuIFRoaXMgcmVmZXJzIHRvIHNldHRpbmdzIG90aGVyIHRoYW4gdGhvc2UgdGhhdCBh cmUgY29uY2VybmVkIHdpdGggTFZEUy1JRiAKc3RhcnR1cC4gVGhlc2UgaXRlbXMgbWF5IGJlIHNl dCB3aGlsZSB3YWl0aW5nIGZvciB0aGUgY29uZGl0aW9ucyBvZiBzdGVwIDYgdG8gCmJlIG1ldC4i CgpEb2Vzbid0IHRoaXMgbWVhbiB0aGF0IHRoZSBtb2RlIGFuZCBpbnB1dCBzZWxlY3RvciBkb24n dCBoYXZlIHRvIGJlIHNldCBhcyB0aGUgCnZlcnkgZmlyc3Qgc3RlcCwgYnV0IGNhbiBiZSBwcm9n cmFtbWVkIGF0IGFueSBwb2ludCBiZWZvcmUgc3RhcnRpbmcgdGhlIExWRFMgCmVuY29kZXIgdGhy b3VnaCB0aGUgUFdEIGJpdCAob24gR2VuMykgb3IgdGhlIFBMTE9OIGJpdCAob24gR2VuMikgPwoK PiA+IC0tLQo+ID4gCj4gPiAgZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9sdmRzZW5j LmMgfCAgIDEwICsrKysrKystLS0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgNyBpbnNlcnRpb25zKCsp LCAzIGRlbGV0aW9ucygtKQo+ID4gCj4gPiBJbmRleDogbGludXgvZHJpdmVycy9ncHUvZHJtL3Jj YXItZHUvcmNhcl9kdV9sdmRzZW5jLmMKPiA+ID09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KPiA+IC0tLSBsaW51eC5vcmln L2RyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfbHZkc2VuYy5jCj4gPiArKysgbGludXgv ZHJpdmVycy9ncHUvZHJtL3JjYXItZHUvcmNhcl9kdV9sdmRzZW5jLmMKPiA+IEBAIC02MCw4ICs2 MCw4IEBAIHN0YXRpYyB2b2lkIHJjYXJfZHVfbHZkc2VuY19zdGFydF9nZW4yKHMKPiA+ICAJcmNh cl9sdmRzX3dyaXRlKGx2ZHMsIExWRFBMTENSLCBwbGxjcik7Cj4gPiAgCQo+ID4gIAkvKgo+ID4g LQkgKiBTZWxlY3QgdGhlIGlucHV0LCBoYXJkY29kZSBtb2RlIDAsIGVuYWJsZSBMVkRTIG9wZXJh dGlvbiBhbmQgdHVybgo+ID4gLQkgKiBiaWFzIGNpcmN1aXRyeSBvbi4KPiA+ICsJICogU2V0IHRo ZSAgTFZEUyBtb2RlLCBzZWxlY3QgdGhlIGlucHV0LCBlbmFibGUgTFZEUyBvcGVyYXRpb24sCj4g PiArCSAqIGFuZCB0dXJuIGJpYXMgY2lyY3VpdHJ5IG9uLgo+ID4gIAkgKi8KPiA+ICAJbHZkY3Iw ID0gKGx2ZHMtPm1vZGUgPDwgTFZEQ1IwX0xWTURfU0hJRlQpIHwgTFZEQ1IwX0JFTiB8IExWRENS MF9MVkVOOwo+ID4gIAlpZiAocmNydGMtPmluZGV4ID09IDIpCj4gPiBAQCAtMTA2LDYgKzEwNiw5 IEBAIHN0YXRpYyB2b2lkIHJjYXJfZHVfbHZkc2VuY19zdGFydF9nZW4zKHMKPiA+IAo+ID4gIAly Y2FyX2x2ZHNfd3JpdGUobHZkcywgTFZEUExMQ1IsIHBsbGNyKTsKPiA+IAo+ID4gKwlsdmRjcjAg PSBsdmRzLT5tb2RlIDw8IExWRENSMF9MVk1EX1NISUZUOwo+ID4gKwlyY2FyX2x2ZHNfd3JpdGUo bHZkcywgTFZEQ1IwLCBsdmRjcjApOwo+ID4gKwo+ID4gIAkvKiBUdXJuIGFsbCB0aGUgY2hhbm5l bHMgb24uICovCj4gPiAgCXJjYXJfbHZkc193cml0ZShsdmRzLCBMVkRDUjEsCj4gPiAgCQkJTFZE Q1IxX0NIU1RCWSgzKSB8IExWRENSMV9DSFNUQlkoMikgfAo+ID4gQEAgLTExNSw3ICsxMTgsOCBA QCBzdGF0aWMgdm9pZCByY2FyX2R1X2x2ZHNlbmNfc3RhcnRfZ2VuMyhzCj4gPiAgCSAqIFR1cm4g dGhlIFBMTCBvbiwgc2V0IGl0IHRvIExWRFMgbm9ybWFsIG1vZGUsIHdhaXQgZm9yIHRoZSBzdGFy dHVwCj4gPiAgCSAqIGRlbGF5IGFuZCB0dXJuIHRoZSBvdXRwdXQgb24uCj4gPiAgCSAqLwo+ID4g LQlsdmRjcjAgPSAobHZkcy0+bW9kZSA8PCBMVkRDUjBfTFZNRF9TSElGVCkgfCBMVkRDUjBfUExM T047Cj4gPiArCj4gPiArCWx2ZGNyMCA9IHwgTFZEQ1IwX1BMTE9OOwo+ID4gIAlyY2FyX2x2ZHNf d3JpdGUobHZkcywgTFZEQ1IwLCBsdmRjcjApOwo+ID4gIAkKPiA+ICAJbHZkY3IwIHw9IExWRENS MF9QV0Q7CgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJp LWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=