From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Stuebner Subject: Re: [PATCH v5 07/36] drm/bridge: analogix_dp: Move enable video into config_video() Date: Sat, 17 Mar 2018 17:00:21 +0100 Message-ID: <1685480.Fci6377Q9J@phil> References: <20180309222327.18689-1-enric.balletbo@collabora.com> <20180309222327.18689-8-enric.balletbo@collabora.com> <79009f48-d677-ccc0-4d61-3b379d8448a8@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <79009f48-d677-ccc0-4d61-3b379d8448a8@codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Archit Taneja Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org, thierry.reding@gmail.com, Laurent.pinchart@ideasonboard.com, ykk@rock-chips.com, kernel@collabora.com, m.szyprowski@samsung.com, linux-samsung-soc@vger.kernel.org, rydberg@bitmath.org, krzk@kernel.org, linux-rockchip@lists.infradead.org, kgene@kernel.org, linux-input@vger.kernel.org, orjan.eide@arm.com, wxt@rock-chips.com, linux-kernel@vger.kernel.org, jeffy.chen@rock-chips.com, =?ISO-8859-1?Q?St=E9phane?= Marchesin , linux-arm-kernel@lists.infradead.org, mark.yao@rock-chips.com, wzz@rock-chips.com, hl@rock-chips.com, jingoohan1@gmail.com, sw0312.kim@samsung.com, dianders@chromium.org, tfiga@chromium.org, kyungmin.park@samsung.com, Enric Balletbo i Serra , kuankuan.y@gmail.com, hshi@chromium.org List-Id: linux-input@vger.kernel.org SGkgQXJjaGl0LAoKQW0gTWl0dHdvY2gsIDE0LiBNw6RyeiAyMDE4LCAwNjo1OTo1OSBDRVQgc2No cmllYiBBcmNoaXQgVGFuZWphOgo+IE9uIFNhdHVyZGF5IDEwIE1hcmNoIDIwMTggMDM6NTIgQU0s IEVucmljIEJhbGxldGJvIGkgU2VycmEgd3JvdGU6Cj4gPiBGcm9tOiBMaW4gSHVhbmcgPGhsQHJv Y2stY2hpcHMuY29tPgo+ID4gCj4gPiBXZSBuZWVkIHRvIGVuYWJsZSB2aWRlbyBiZWZvcmUgYW5h bG9naXhfZHBfaXNfdmlkZW9fc3RyZWFtX29uKCksIHNvCj4gPiB3ZSBjYW4gZ2V0IHRoZSByaWdo dCB2aWRlbyBzdHJlYW0gc3RhdHVzLgo+ID4gCj4gPiBDYzog5b6B5aKeIOeOiyA8d3p6QHJvY2st Y2hpcHMuY29tPgo+ID4gQ2M6IFN0w6lwaGFuZSBNYXJjaGVzaW4gPG1hcmNoZXVAY2hyb21pdW0u b3JnPgo+ID4gU2lnbmVkLW9mZi1ieTogTGluIEh1YW5nIDxobEByb2NrLWNoaXBzLmNvbT4KPiA+ IFNpZ25lZC1vZmYtYnk6IFNlYW4gUGF1bCA8c2VhbnBhdWxAY2hyb21pdW0ub3JnPgo+ID4gU2ln bmVkLW9mZi1ieTogVGhpZXJyeSBFc2NhbmRlIDx0aGllcnJ5LmVzY2FuZGVAY29sbGFib3JhLmNv bT4KPiA+IFJldmlld2VkLWJ5OiBBbmRyemVqIEhhamRhIDxhLmhhamRhQHNhbXN1bmcuY29tPgo+ ID4gU2lnbmVkLW9mZi1ieTogRW5yaWMgQmFsbGV0Ym8gaSBTZXJyYSA8ZW5yaWMuYmFsbGV0Ym9A Y29sbGFib3JhLmNvbT4KPiA+IFRlc3RlZC1ieTogTWFyZWsgU3p5cHJvd3NraSA8bS5zenlwcm93 c2tpQHNhbXN1bmcuY29tPgo+ID4gLS0tCj4gPiAKPiA+ICAgZHJpdmVycy9ncHUvZHJtL2JyaWRn ZS9hbmFsb2dpeC9hbmFsb2dpeF9kcF9jb3JlLmMgfCAxMSArKysrKy0tLS0tLQo+ID4gICAxIGZp bGUgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQo+ID4gCj4gPiBkaWZm IC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9hbmFsb2dpeC9hbmFsb2dpeF9kcF9jb3Jl LmMgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2FuYWxvZ2l4L2FuYWxvZ2l4X2RwX2NvcmUuYwo+ ID4gaW5kZXggNWEyZTM1ZGM0MWUzLi5mOTY2MWI0MTBjYjkgMTAwNjQ0Cj4gPiAtLS0gYS9kcml2 ZXJzL2dwdS9kcm0vYnJpZGdlL2FuYWxvZ2l4L2FuYWxvZ2l4X2RwX2NvcmUuYwo+ID4gKysrIGIv ZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9hbmFsb2dpeC9hbmFsb2dpeF9kcF9jb3JlLmMKPiA+IEBA IC04MTksMTEgKzgxOSwxMCBAQCBzdGF0aWMgaW50IGFuYWxvZ2l4X2RwX2NvbmZpZ192aWRlbyhz dHJ1Y3QgYW5hbG9naXhfZHBfZGV2aWNlICpkcCkKPiA+ICAgCQlpZiAoYW5hbG9naXhfZHBfaXNf c2xhdmVfdmlkZW9fc3RyZWFtX2Nsb2NrX29uKGRwKSA9PSAwKQo+ID4gICAJCQlicmVhazsKPiA+ ICAgCQlpZiAodGltZW91dF9sb29wID4gRFBfVElNRU9VVF9MT09QX0NPVU5UKSB7Cj4gPiAtCQkJ ZGV2X2VycihkcC0+ZGV2LCAiVGltZW91dCBvZiB2aWRlbyBzdHJlYW1jbGsgb2tcbiIpOwo+ID4g KwkJCWRldl9lcnIoZHAtPmRldiwgIlRpbWVvdXQgb2Ygc2xhdmUgdmlkZW8gc3RyZWFtY2xrIG9r XG4iKTsKPiA+ICAgCQkJcmV0dXJuIC1FVElNRURPVVQ7Cj4gPiAgIAkJfQo+ID4gLQo+ID4gLQkJ dXNsZWVwX3JhbmdlKDEsIDIpOwo+ID4gKwkJdXNsZWVwX3JhbmdlKDEwMDAsIDEwMDEpOwo+IAo+ IENvdWxkIHdlIGJyaWVmbHkgZXhwbGFpbiBpbiB0aGUgY29tbWl0IG1lc3NhZ2Ugd2h5IHdlIG5l ZWQgdG8gaW5jcmVhc2UKPiB0aGUgZGVsYXkgaW4gdGhlIHRpbWVvdXQgbG9vcD8gSXMgaXQgYSBj b25zZXF1ZW5jZSBvZiBjYWxsaW5nCj4gYW5hbG9naXhfZHBfc3RhcnRfdmlkZW8oKSBlYXJsaWVy LCBvciBpcyB0aGlzIHRoZSBwcmVmZXJyZWQgdGltZQo+IG1lbnRpb25lZCBpbiB0aGUgc3BlY3M/ CgpJIGFza2VkIExpbiwgdGhlIG9yaWdpbmFsIGF1dGhvciBvZiB0aGlzIHBhdGNoLCByZXNwb25z ZSBxdW90ZWQgYmVsb3c6CgoiVGhlcmUgaXMgcmFuZG9tICJUaW1lb3V0IG9mIHZpZGVvIHN0cmVh bWNsayBvayIgbWVzc2FnZSBoYXBwZW4gd2hlbiAKZGVidWcgZWRwIHBhbmVsLApTbyB3ZSBleHRl bmQgdGhpcyB0aW1lLCAgdGhpcyB0aW1lIGRvIG5vdCBkZWZpbmUgaW4gdGhlIHNwZWMuIgoKU28g aXQgbG9va3MgbGlrZSBpdCB3YXMgd29ya2luZyBieSBjaGFuY2UgYmVmb3JlIGFuZCB0aGlzIG1v dmUgdHJpZ2dlcmVkCnNvbWUgc29ydCBvZiB0aW1pbmcgaXNzdWUuCgoKSGVpa28KX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcg bGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko Stuebner) Date: Sat, 17 Mar 2018 17:00:21 +0100 Subject: [PATCH v5 07/36] drm/bridge: analogix_dp: Move enable video into config_video() In-Reply-To: <79009f48-d677-ccc0-4d61-3b379d8448a8@codeaurora.org> References: <20180309222327.18689-1-enric.balletbo@collabora.com> <20180309222327.18689-8-enric.balletbo@collabora.com> <79009f48-d677-ccc0-4d61-3b379d8448a8@codeaurora.org> Message-ID: <1685480.Fci6377Q9J@phil> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Archit, Am Mittwoch, 14. M?rz 2018, 06:59:59 CET schrieb Archit Taneja: > On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote: > > From: Lin Huang > > > > We need to enable video before analogix_dp_is_video_stream_on(), so > > we can get the right video stream status. > > > > Cc: ?? ? > > Cc: St?phane Marchesin > > Signed-off-by: Lin Huang > > Signed-off-by: Sean Paul > > Signed-off-by: Thierry Escande > > Reviewed-by: Andrzej Hajda > > Signed-off-by: Enric Balletbo i Serra > > Tested-by: Marek Szyprowski > > --- > > > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > index 5a2e35dc41e3..f9661b410cb9 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > @@ -819,11 +819,10 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) > > if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0) > > break; > > if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) { > > - dev_err(dp->dev, "Timeout of video streamclk ok\n"); > > + dev_err(dp->dev, "Timeout of slave video streamclk ok\n"); > > return -ETIMEDOUT; > > } > > - > > - usleep_range(1, 2); > > + usleep_range(1000, 1001); > > Could we briefly explain in the commit message why we need to increase > the delay in the timeout loop? Is it a consequence of calling > analogix_dp_start_video() earlier, or is this the preferred time > mentioned in the specs? I asked Lin, the original author of this patch, response quoted below: "There is random "Timeout of video streamclk ok" message happen when debug edp panel, So we extend this time, this time do not define in the spec." So it looks like it was working by chance before and this move triggered some sort of timing issue. Heiko From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753622AbeCQQAn convert rfc822-to-8bit (ORCPT ); Sat, 17 Mar 2018 12:00:43 -0400 Received: from gloria.sntech.de ([95.129.55.99]:60032 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753528AbeCQQAl (ORCPT ); Sat, 17 Mar 2018 12:00:41 -0400 From: Heiko Stuebner To: Archit Taneja Cc: Enric Balletbo i Serra , inki.dae@samsung.com, thierry.reding@gmail.com, hjc@rock-chips.com, seanpaul@chromium.org, airlied@linux.ie, tfiga@chromium.org, dri-devel@lists.freedesktop.org, dianders@chromium.org, a.hajda@samsung.com, ykk@rock-chips.com, kernel@collabora.com, m.szyprowski@samsung.com, linux-samsung-soc@vger.kernel.org, jy0922.shim@samsung.com, rydberg@bitmath.org, krzk@kernel.org, linux-rockchip@lists.infradead.org, kgene@kernel.org, linux-input@vger.kernel.org, orjan.eide@arm.com, wxt@rock-chips.com, jeffy.chen@rock-chips.com, linux-arm-kernel@lists.infradead.org, mark.yao@rock-chips.com, wzz@rock-chips.com, hl@rock-chips.com, jingoohan1@gmail.com, sw0312.kim@samsung.com, linux-kernel@vger.kernel.org, kyungmin.park@samsung.com, Laurent.pinchart@ideasonboard.com, kuankuan.y@gmail.com, hshi@chromium.org, =?ISO-8859-1?Q?St=E9phane?= Marchesin Subject: Re: [PATCH v5 07/36] drm/bridge: analogix_dp: Move enable video into config_video() Date: Sat, 17 Mar 2018 17:00:21 +0100 Message-ID: <1685480.Fci6377Q9J@phil> In-Reply-To: <79009f48-d677-ccc0-4d61-3b379d8448a8@codeaurora.org> References: <20180309222327.18689-1-enric.balletbo@collabora.com> <20180309222327.18689-8-enric.balletbo@collabora.com> <79009f48-d677-ccc0-4d61-3b379d8448a8@codeaurora.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Archit, Am Mittwoch, 14. März 2018, 06:59:59 CET schrieb Archit Taneja: > On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote: > > From: Lin Huang > > > > We need to enable video before analogix_dp_is_video_stream_on(), so > > we can get the right video stream status. > > > > Cc: 征增 王 > > Cc: Stéphane Marchesin > > Signed-off-by: Lin Huang > > Signed-off-by: Sean Paul > > Signed-off-by: Thierry Escande > > Reviewed-by: Andrzej Hajda > > Signed-off-by: Enric Balletbo i Serra > > Tested-by: Marek Szyprowski > > --- > > > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > index 5a2e35dc41e3..f9661b410cb9 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > @@ -819,11 +819,10 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) > > if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0) > > break; > > if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) { > > - dev_err(dp->dev, "Timeout of video streamclk ok\n"); > > + dev_err(dp->dev, "Timeout of slave video streamclk ok\n"); > > return -ETIMEDOUT; > > } > > - > > - usleep_range(1, 2); > > + usleep_range(1000, 1001); > > Could we briefly explain in the commit message why we need to increase > the delay in the timeout loop? Is it a consequence of calling > analogix_dp_start_video() earlier, or is this the preferred time > mentioned in the specs? I asked Lin, the original author of this patch, response quoted below: "There is random "Timeout of video streamclk ok" message happen when debug edp panel, So we extend this time, this time do not define in the spec." So it looks like it was working by chance before and this move triggered some sort of timing issue. Heiko