From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jingoo Han" Subject: Re: [PATCH v12 16/18] drm: bridge: analogix/dp: expand the wait time for looking AUX CH reply flag Date: Thu, 24 Dec 2015 00:10:22 +0900 Message-ID: <000001d13d94$120e0d60$362a2820$@com> References: <1450873538-18304-1-git-send-email-ykk@rock-chips.com> <1450875064-19627-1-git-send-email-ykk@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1450875064-19627-1-git-send-email-ykk@rock-chips.com> Content-Language: ko List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: 'Yakir Yang' , 'Inki Dae' , 'Mark Yao' , 'Heiko Stuebner' Cc: devicetree@vger.kernel.org, 'Krzysztof Kozlowski' , linux-samsung-soc@vger.kernel.org, 'Russell King' , linux-rockchip@lists.infradead.org, 'Jingoo Han' , emil.l.velikov@gmail.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, 'Kishon Vijay Abraham I' , javier@osg.samsung.com, 'Rob Herring' , 'Andy Yan' , 'Thierry Reding' , 'Gustavo Padovan' , linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org T24gV2VkbmVzZGF5LCBEZWNlbWJlciAyMywgMjAxNSA5OjUxIFBNLCBZYWtpciBZYW5nIHdyb3Rl Ogo+IAo+IE9uIFJvY2tjaGlwIHBsYXRmb3JtLCBzb21ldGltZXMgZHJpdmVyIHdvdWxkIGZhaWxl ZCBhdCByZWFkaW5nIEVESUQKPiBtZXNzYWdlLCBhbmQgaXQncyBjYXVzZWQgYnkgdGhlIEFVWCBy ZXBseSBmbGFnIHdvdWxkbid0IHJlY2VpdmVkIHVuZGVyCj4gdGhlIDEwMCoxMHVzIHdhaXQgdGlt ZS4KClRoZSBwcm9ibGVtIGlzIHNwZWNpZmljIGZvciBSb2NrY2hpcCBwbGF0Zm9ybS4KQWxzbywg MSBtcyBpcyBsb25nIHRpbWUuClRoZSBiZXN0IHdheSBpcyB0aGF0IHlvdXIgaGFyZHdhcmUgZW5n aW5lZXJzIGZpbmQgdGhlIHJvb3QgY2F1c2UuCkkgY2Fubm90IHVuZGVyc3RhbmQgd2h5IHlvdXIg ZW5naW5lZXJzIGNhbm5vdCBmaW5kIHRoZSByb290IGNhdXNlLiA6LSgKCj4gCj4gQnV0IGFmdGVy IGV4cGFuZCB0aGUgd2FpdCB0aW1lIGEgbGl0dGxlLCB0aGUgQVVYIHJlcGx5IGZsYWcgd291bGQg YmUKPiBzZXQsIHNvIG1heWJlIHRoZSB3YWl0IHRpbWUgaXMgYSBsaXR0bGUgY3JpdGljYWwuIEJl c2lkZXMgdGhlIGFuYWxvZ2l4Cj4gZHAgYm9vayBoYXZlbid0IHJlbWluZGVkIHRoZSBzdGFuZGFy ZCB3YWl0IGZvciBsb29raW5nIEFVWCByZXBseSBmbGFnLAo+IHNvIEkgdGhvdWdodCBpdCdzIG9r YXkgdG8gZXhwYW5kIHRoZSB3YWl0IHRpbWUuCj4gCj4gQW5kIHRoZSBleHRlcm5hbCB3YWl0IHRp bWUgd29uJ3QgaHVydCBFeHlub3MgRFAgdG9vIG11Y2gsIGNhdXNlIHRoZXkKPiB3b3VsZG4ndCBt ZWV0IHRoaXMgcHJvYmxlbSwgdGhlbiBkcml2ZXIgd291bGQgcmVjZWl2ZWQgdGhlIHJlcGx5IGNv bW1hbmQKPiB2ZXJ5IHNvb24sIHNvIG5vIG1vcmUgYWRkaXRpb25hbCB3YWl0IHRpbWUgd291bGQg YnJpbmcgdG8gRXh5bm9zIHBsYXRmb3JtLgoKVGhlbiwgd2hlbiBsb29wIHRpbWUgaGFwcGVucyBv biBFeHlub3MgcGxhdGZvcm0sIGl0IHdpbGwgdGFrZSBsb25nIHRpbWUKdG8gcmV0dXJuIGVycm9y LgoKPiAKPiBTaWduZWQtb2ZmLWJ5OiBZYWtpciBZYW5nIDx5a2tAcm9jay1jaGlwcy5jb20+Cj4g LS0tCj4gQ2hhbmdlcyBpbiB2MTI6Cj4gLSBVc2luZyBhbm90aGVyIHdheSB0byBleHBhbmQgdGhl IEFVWCByZXBseSB3YWl0IHRpbWUgKEppbmdvbykKPiAKPiBDaGFuZ2VzIGluIHYxMTogTm9uZQo+ IENoYW5nZXMgaW4gdjEwOiBOb25lCj4gQ2hhbmdlcyBpbiB2OTogTm9uZQo+IENoYW5nZXMgaW4g djg6IE5vbmUKPiBDaGFuZ2VzIGluIHY3OiBOb25lCj4gQ2hhbmdlcyBpbiB2NjogTm9uZQo+IENo YW5nZXMgaW4gdjU6IE5vbmUKPiBDaGFuZ2VzIGluIHY0OiBOb25lCj4gQ2hhbmdlcyBpbiB2Mzog Tm9uZQo+IENoYW5nZXMgaW4gdjI6IE5vbmUKPiAKPiAgZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9h bmFsb2dpeC9hbmFsb2dpeF9kcF9yZWcuYyB8IDEwICsrKystLS0tLS0KPiAgMSBmaWxlIGNoYW5n ZWQsIDQgaW5zZXJ0aW9ucygrKSwgNiBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL2JyaWRnZS9hbmFsb2dpeC9hbmFsb2dpeF9kcF9yZWcuYwo+IGIvZHJpdmVy cy9ncHUvZHJtL2JyaWRnZS9hbmFsb2dpeC9hbmFsb2dpeF9kcF9yZWcuYwo+IGluZGV4IGNiYTNm ZmQuLjg2ODdlZWEgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9hbmFsb2dp eC9hbmFsb2dpeF9kcF9yZWcuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvYW5hbG9n aXgvYW5hbG9naXhfZHBfcmVnLmMKPiBAQCAtNDcxLDcgKzQ3MSw3IEBAIGludCBhbmFsb2dpeF9k cF9zdGFydF9hdXhfdHJhbnNhY3Rpb24oc3RydWN0IGFuYWxvZ2l4X2RwX2RldmljZSAqZHApCj4g IHsKPiAgCWludCByZWc7Cj4gIAlpbnQgcmV0dmFsID0gMDsKPiAtCWludCB0aW1lb3V0X2xvb3Ag PSAwOwo+ICsJdW5zaWduZWQgbG9uZyB0aW1lb3V0Owo+IAo+ICAJLyogRW5hYmxlIEFVWCBDSCBv cGVyYXRpb24gKi8KPiAgCXJlZyA9IHJlYWRsKGRwLT5yZWdfYmFzZSArIEFOQUxPR0lYX0RQX0FV WF9DSF9DVExfMik7Cj4gQEAgLTQ3OSwxNCArNDc5LDEyIEBAIGludCBhbmFsb2dpeF9kcF9zdGFy dF9hdXhfdHJhbnNhY3Rpb24oc3RydWN0IGFuYWxvZ2l4X2RwX2RldmljZSAqZHApCj4gIAl3cml0 ZWwocmVnLCBkcC0+cmVnX2Jhc2UgKyBBTkFMT0dJWF9EUF9BVVhfQ0hfQ1RMXzIpOwo+IAo+ICAJ LyogSXMgQVVYIENIIGNvbW1hbmQgcmVwbHkgcmVjZWl2ZWQ/ICovCj4gLQlyZWcgPSByZWFkbChk cC0+cmVnX2Jhc2UgKyBBTkFMT0dJWF9EUF9JTlRfU1RBKTsKPiAtCXdoaWxlICghKHJlZyAmIFJQ TFlfUkVDRUlWKSkgewo+IC0JCXRpbWVvdXRfbG9vcCsrOwo+IC0JCWlmIChEUF9USU1FT1VUX0xP T1BfQ09VTlQgPCB0aW1lb3V0X2xvb3ApIHsKPiArCXRpbWVvdXQgPSBqaWZmaWVzICsgbXNlY3Nf dG9famlmZmllcyg1KTsKPiArCXdoaWxlICgocmVhZGwoZHAtPnJlZ19iYXNlICsgQU5BTE9HSVhf RFBfSU5UX1NUQSkgJiBSUExZX1JFQ0VJVikgPT0gMCkgewo+ICsJCWlmICh0aW1lX2FmdGVyKGpp ZmZpZXMsIHRpbWVvdXQpKSB7Cj4gIAkJCWRldl9lcnIoZHAtPmRldiwgIkFVWCBDSCBjb21tYW5k IHJlcGx5IGZhaWxlZCFcbiIpOwo+ICAJCQlyZXR1cm4gLUVUSU1FRE9VVDsKPiAgCQl9Cj4gLQkJ cmVnID0gcmVhZGwoZHAtPnJlZ19iYXNlICsgQU5BTE9HSVhfRFBfSU5UX1NUQSk7CgpTb3JyeSwg SSBkb24ndCBsaWtlIHlvdXIgcGF0Y2guCgpUaGUgcHJvYmxlbSBoYXBwZW5zIGJlY2F1c2Ugb2Yg Um9ja2NoaXAgcGxhdGZvcm0uClNvLCB5b3UgbmVlZCB0byBhZGQgd29ya2Fyb3VuZCBmb3Igb25s eSBSb2NrY2hpcCBwbGF0Zm9ybS4KCkp1c3QgYWRkIG5ldyBEVCBwcm9wZXJ0eSBhbmQgbmV3IHZh cmlhYmxlIGZvciB0aGUgdmFsdWUgZm9yIHdhaXQgdGltZS4KV2hlbiwgdGhlIHByb2JlIGlzIGNh bGxlZCwgbmV3IHdhaXQgdGltZSB2YWx1ZSBpcyByZWFkIGZyb20gUm9ja2NoaXAgRFQgZmlsZS4K VGhlbiwgdGhlIG5ldyB3YWl0IHRpbWUgdmFsdWUgY2FuIGJlIHdyaXR0ZW4gdG8gdGhlIG5ldyB2 YXJpYWJsZS4KCiAgICBuZXcgRFQgcHJvcGVydHk6IHdhaXQtdGltZS1hdXgKICAgIG5ldyB2YXJp YWJsZTogd2FpdF90aW1lX2F1eAoKCklmICggKSAvLyBOZXcgRFQgCiAgICB3YWl0X3RpbWVfYXV4 ID0gTmV3IERUOwplbHNlIAogICAgd2FpdF90aW1lX2F1eCA9IDEwOwoKCj4gIAkJdXNsZWVwX3Jh bmdlKDEwLCAxMSk7CgpJZiB0aGVyZSBpcyBOTyAgbmV3IHdhaXQgdGltZSB2YWx1ZSBmcm9tIERU IGZpbGUsIHRoZSBkZWZhdWx0IHZhbHVlICcxMCcgaXMKdXNlZCBmb3Igc2xlZXAuCgpCdXQsIGlm IHRoZXJlIGlzIG5ldyB3YWl0IHRpbWUgdmFsdWUgZnJvbSBEVCBmaWxlLCBuZXcgd2FpdCB0aW1l IHZhbHVlCmNhbiBiZSB1c2VkIGZvciBzbGVlcC4KCiAgICAgICAgICAgICAgICAgdXNsZWVwX3Jh bmdlKGRwLT53YWl0X3RpbWVfYXV4LCBkcC0+d2FpdF90aW1lX2F1eCArIDEpOwoKV2hhdCBJIHdh bnQgdG8gc2F5IGlzIHRoYXQgdGhlcmUgc2hvdWxkIGJlIE5PIGVmZmVjdCBvbiBFeHlub3MgcGxh dGZvcm0sCmJlY2F1c2Ugb2YgdGhlIGhhcmR3YXJlIGJ1ZyBvZiBSb2NrY2hpcCBwbGF0Zm9ybS4K CkJlc3QgcmVnYXJkcywKSmluZ29vIEhhbgoKPiAgCX0KPiAKPiAtLQo+IDEuOS4xCgoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxp bmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: jingoohan1@gmail.com (Jingoo Han) Date: Thu, 24 Dec 2015 00:10:22 +0900 Subject: [PATCH v12 16/18] drm: bridge: analogix/dp: expand the wait time for looking AUX CH reply flag In-Reply-To: <1450875064-19627-1-git-send-email-ykk@rock-chips.com> References: <1450873538-18304-1-git-send-email-ykk@rock-chips.com> <1450875064-19627-1-git-send-email-ykk@rock-chips.com> Message-ID: <000001d13d94$120e0d60$362a2820$@com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wednesday, December 23, 2015 9:51 PM, Yakir Yang wrote: > > On Rockchip platform, sometimes driver would failed at reading EDID > message, and it's caused by the AUX reply flag wouldn't received under > the 100*10us wait time. The problem is specific for Rockchip platform. Also, 1 ms is long time. The best way is that your hardware engineers find the root cause. I cannot understand why your engineers cannot find the root cause. :-( > > But after expand the wait time a little, the AUX reply flag would be > set, so maybe the wait time is a little critical. Besides the analogix > dp book haven't reminded the standard wait for looking AUX reply flag, > so I thought it's okay to expand the wait time. > > And the external wait time won't hurt Exynos DP too much, cause they > wouldn't meet this problem, then driver would received the reply command > very soon, so no more additional wait time would bring to Exynos platform. Then, when loop time happens on Exynos platform, it will take long time to return error. > > Signed-off-by: Yakir Yang > --- > Changes in v12: > - Using another way to expand the AUX reply wait time (Jingoo) > > Changes in v11: None > Changes in v10: None > Changes in v9: None > Changes in v8: None > Changes in v7: None > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > index cba3ffd..8687eea 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > @@ -471,7 +471,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp) > { > int reg; > int retval = 0; > - int timeout_loop = 0; > + unsigned long timeout; > > /* Enable AUX CH operation */ > reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2); > @@ -479,14 +479,12 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp) > writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2); > > /* Is AUX CH command reply received? */ > - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); > - while (!(reg & RPLY_RECEIV)) { > - timeout_loop++; > - if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) { > + timeout = jiffies + msecs_to_jiffies(5); > + while ((readl(dp->reg_base + ANALOGIX_DP_INT_STA) & RPLY_RECEIV) == 0) { > + if (time_after(jiffies, timeout)) { > dev_err(dp->dev, "AUX CH command reply failed!\n"); > return -ETIMEDOUT; > } > - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); Sorry, I don't like your patch. The problem happens because of Rockchip platform. So, you need to add workaround for only Rockchip platform. Just add new DT property and new variable for the value for wait time. When, the probe is called, new wait time value is read from Rockchip DT file. Then, the new wait time value can be written to the new variable. new DT property: wait-time-aux new variable: wait_time_aux If ( ) // New DT wait_time_aux = New DT; else wait_time_aux = 10; > usleep_range(10, 11); If there is NO new wait time value from DT file, the default value '10' is used for sleep. But, if there is new wait time value from DT file, new wait time value can be used for sleep. usleep_range(dp->wait_time_aux, dp->wait_time_aux + 1); What I want to say is that there should be NO effect on Exynos platform, because of the hardware bug of Rockchip platform. Best regards, Jingoo Han > } > > -- > 1.9.1 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752210AbbLWPKf (ORCPT ); Wed, 23 Dec 2015 10:10:35 -0500 Received: from mail-pf0-f174.google.com ([209.85.192.174]:33165 "EHLO mail-pf0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750963AbbLWPKb (ORCPT ); Wed, 23 Dec 2015 10:10:31 -0500 From: "Jingoo Han" To: "'Yakir Yang'" , "'Inki Dae'" , "'Mark Yao'" , "'Heiko Stuebner'" Cc: "'Thierry Reding'" , "'Krzysztof Kozlowski'" , "'Rob Herring'" , "'Russell King'" , , "'Gustavo Padovan'" , "'Kishon Vijay Abraham I'" , , "'Andy Yan'" , , , , , , , "'Jingoo Han'" References: <1450873538-18304-1-git-send-email-ykk@rock-chips.com> <1450875064-19627-1-git-send-email-ykk@rock-chips.com> In-Reply-To: <1450875064-19627-1-git-send-email-ykk@rock-chips.com> Subject: Re: [PATCH v12 16/18] drm: bridge: analogix/dp: expand the wait time for looking AUX CH reply flag Date: Thu, 24 Dec 2015 00:10:22 +0900 Message-ID: <000001d13d94$120e0d60$362a2820$@com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Thread-Index: AdE9gPER6Rky0TFqQd6CDo+AbyC+LQAD5D6g Content-Language: ko Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, December 23, 2015 9:51 PM, Yakir Yang wrote: > > On Rockchip platform, sometimes driver would failed at reading EDID > message, and it's caused by the AUX reply flag wouldn't received under > the 100*10us wait time. The problem is specific for Rockchip platform. Also, 1 ms is long time. The best way is that your hardware engineers find the root cause. I cannot understand why your engineers cannot find the root cause. :-( > > But after expand the wait time a little, the AUX reply flag would be > set, so maybe the wait time is a little critical. Besides the analogix > dp book haven't reminded the standard wait for looking AUX reply flag, > so I thought it's okay to expand the wait time. > > And the external wait time won't hurt Exynos DP too much, cause they > wouldn't meet this problem, then driver would received the reply command > very soon, so no more additional wait time would bring to Exynos platform. Then, when loop time happens on Exynos platform, it will take long time to return error. > > Signed-off-by: Yakir Yang > --- > Changes in v12: > - Using another way to expand the AUX reply wait time (Jingoo) > > Changes in v11: None > Changes in v10: None > Changes in v9: None > Changes in v8: None > Changes in v7: None > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > index cba3ffd..8687eea 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c > @@ -471,7 +471,7 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp) > { > int reg; > int retval = 0; > - int timeout_loop = 0; > + unsigned long timeout; > > /* Enable AUX CH operation */ > reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2); > @@ -479,14 +479,12 @@ int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp) > writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2); > > /* Is AUX CH command reply received? */ > - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); > - while (!(reg & RPLY_RECEIV)) { > - timeout_loop++; > - if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) { > + timeout = jiffies + msecs_to_jiffies(5); > + while ((readl(dp->reg_base + ANALOGIX_DP_INT_STA) & RPLY_RECEIV) == 0) { > + if (time_after(jiffies, timeout)) { > dev_err(dp->dev, "AUX CH command reply failed!\n"); > return -ETIMEDOUT; > } > - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); Sorry, I don't like your patch. The problem happens because of Rockchip platform. So, you need to add workaround for only Rockchip platform. Just add new DT property and new variable for the value for wait time. When, the probe is called, new wait time value is read from Rockchip DT file. Then, the new wait time value can be written to the new variable. new DT property: wait-time-aux new variable: wait_time_aux If ( ) // New DT wait_time_aux = New DT; else wait_time_aux = 10; > usleep_range(10, 11); If there is NO new wait time value from DT file, the default value '10' is used for sleep. But, if there is new wait time value from DT file, new wait time value can be used for sleep. usleep_range(dp->wait_time_aux, dp->wait_time_aux + 1); What I want to say is that there should be NO effect on Exynos platform, because of the hardware bug of Rockchip platform. Best regards, Jingoo Han > } > > -- > 1.9.1