From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context Date: Tue, 29 Nov 2016 08:46:44 +0200 Message-ID: <3570409.tKEQBRfa3m@avalon> References: <1480395884-5471-1-git-send-email-john.stultz@linaro.org> <1480395884-5471-2-git-send-email-john.stultz@linaro.org> 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 8BEEF6E269 for ; Tue, 29 Nov 2016 06:46:30 +0000 (UTC) In-Reply-To: <1480395884-5471-2-git-send-email-john.stultz@linaro.org> 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 SGkgSm9obiwKClRoYW5rIHlvdSBmb3IgdGhlIHBhdGNoLgoKT24gTW9uZGF5IDI4IE5vdiAyMDE2 IDIxOjA0OjQwIEpvaG4gU3R1bHR6IHdyb3RlOgo+IEkgd2FzIHJlY2VudGx5IHNlZWluZyBpc3N1 ZXMgd2l0aCBFRElEIHByb2JpbmcsIHdoZXJlCj4gdGhlIGxvZ2ljIHRvIHdhaXQgZm9yIHRoZSBF RElEIHJlYWQgYml0IHRvIGJlIHNldCBieSB0aGUKPiBJUlEgd2Fzbid0IGhhcHBlbmluZyBhbmQg dGhlIGNvZGUgd291bGQgdGltZSBvdXQgYW5kIGZhaWwuCj4gCj4gRGlnZ2luZyBkZWVwZXIsIEkg Zm91bmQgdGhpcyB3YXMgZHVlIHRvIHRoZSBmYWN0IHRoYXQKPiBJUlFzIHdlcmUgZGlzYWJsZWQg YXMgd2Ugd2VyZSBydW5uaW5nIGluIElSUSBjb250ZXh0IGZyb20KPiB0aGUgSFBEIHNpZ25hbC4K PiAKPiBUaHVzIHRoaXMgcGF0Y2ggY2hhbmdlcyB0aGUgbG9naWMgdG8gaGFuZGxlIHRoZSBIUEQg c2lnbmFsCj4gdmlhIGEgd29ya19zdHJ1Y3Qgc28gd2UgY2FuIGJlIG91dCBvZiBpcnEgY29udGV4 dC4KPiAKPiBXaXRoIHRoaXMgcGF0Y2gsIHRoZSBFRElEIHByb2Jpbmcgb24gaG90cGx1ZyBkb2Vz IG5vdCB0aW1lCj4gb3V0Lgo+IAo+IENjOiBEYXZpZCBBaXJsaWUgPGFpcmxpZWRAbGludXguaWU+ Cj4gQ2M6IEFyY2hpdCBUYW5lamEgPGFyY2hpdHRAY29kZWF1cm9yYS5vcmc+Cj4gQ2M6IFdvbGZy YW0gU2FuZyA8d3NhK3JlbmVzYXNAc2FuZy1lbmdpbmVlcmluZy5jb20+Cj4gQ2M6IExhcnMtUGV0 ZXIgQ2xhdXNlbiA8bGFyc0BtZXRhZm9vLmRlPgo+IENjOiBMYXVyZW50IFBpbmNoYXJ0IDxsYXVy ZW50LnBpbmNoYXJ0QGlkZWFzb25ib2FyZC5jb20+Cj4gQ2M6IGRyaS1kZXZlbEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKPiBTaWduZWQtb2ZmLWJ5OiBKb2huIFN0dWx0eiA8am9obi5zdHVsdHpAbGlu YXJvLm9yZz4KPiAtLS0KPiB2MjogUmV3b3JrZWQgdG8gcHJvcGVybHkgZml4IHRoZSBpc3N1ZSBy YXRoZXIgdGhlbgo+ICAgICBqdXN0IGRlbGF5aW5nIGZvciAyMDBtcwo+IAo+ICBkcml2ZXJzL2dw dS9kcm0vYnJpZGdlL2Fkdjc1MTEvYWR2NzUxMS5oICAgICB8ICAyICsrCj4gIGRyaXZlcnMvZ3B1 L2RybS9icmlkZ2UvYWR2NzUxMS9hZHY3NTExX2Rydi5jIHwgMTIgKysrKysrKysrKystCj4gIDIg ZmlsZXMgY2hhbmdlZCwgMTMgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQo+IAo+IGRpZmYg LS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2Fkdjc1MTEvYWR2NzUxMS5oCj4gYi9kcml2 ZXJzL2dwdS9kcm0vYnJpZGdlL2Fkdjc1MTEvYWR2NzUxMS5oIGluZGV4IDk5MmQ3NmMuLjJhMWU3 MjIgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9hZHY3NTExL2Fkdjc1MTEu aAo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvYWR2NzUxMS9hZHY3NTExLmgKPiBAQCAt MzE3LDYgKzMxNyw4IEBAIHN0cnVjdCBhZHY3NTExIHsKPiAgCWJvb2wgZWRpZF9yZWFkOwo+IAo+ ICAJd2FpdF9xdWV1ZV9oZWFkX3Qgd3E7Cj4gKwlzdHJ1Y3Qgd29ya19zdHJ1Y3QgaXJxX3dvcms7 Cj4gKwoKSSdkIGNhbGwgdGhpcyBocGRfd29yay4KCj4gIAlzdHJ1Y3QgZHJtX2JyaWRnZSBicmlk Z2U7Cj4gIAlzdHJ1Y3QgZHJtX2Nvbm5lY3RvciBjb25uZWN0b3I7Cj4gCj4gZGlmZiAtLWdpdCBh L2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvYWR2NzUxMS9hZHY3NTExX2Rydi5jCj4gYi9kcml2ZXJz L2dwdS9kcm0vYnJpZGdlL2Fkdjc1MTEvYWR2NzUxMV9kcnYuYyBpbmRleCA4ZGJhNzI5Li5iMzhl NzQzCj4gMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9hZHY3NTExL2Fkdjc1 MTFfZHJ2LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2Fkdjc1MTEvYWR2NzUxMV9k cnYuYwo+IEBAIC00MDIsNiArNDAyLDE0IEBAIHN0YXRpYyBib29sIGFkdjc1MTFfaHBkKHN0cnVj dCBhZHY3NTExICphZHY3NTExKQo+ICAJcmV0dXJuIGZhbHNlOwo+ICB9Cj4gCj4gK3N0YXRpYyB2 b2lkIGFkdjc1MTFfaXJxX3dvcmsoc3RydWN0IHdvcmtfc3RydWN0ICp3b3JrKQo+ICt7Cj4gKwlz dHJ1Y3QgYWR2NzUxMSAqYWR2NzUxMSA9IGNvbnRhaW5lcl9vZih3b3JrLCBzdHJ1Y3QgYWR2NzUx MSwgCmlycV93b3JrKTsKPiArCj4gKwlkcm1faGVscGVyX2hwZF9pcnFfZXZlbnQoYWR2NzUxMS0+ Y29ubmVjdG9yLmRldik7Cj4gK30KPiArCj4gKwoKT25lIGJsYW5rIGxpbmUgaXMgZW5vdWdoLgoK QXBhcnQgZnJvbSB0aGF0LAoKUmV2aWV3ZWQtYnk6IExhdXJlbnQgUGluY2hhcnQgPGxhdXJlbnQu cGluY2hhcnRAaWRlYXNvbmJvYXJkLmNvbT4KCj4gIHN0YXRpYyBpbnQgYWR2NzUxMV9pcnFfcHJv Y2VzcyhzdHJ1Y3QgYWR2NzUxMSAqYWR2NzUxMSwgYm9vbCBwcm9jZXNzX2hwZCkKPiAgewo+ICAJ dW5zaWduZWQgaW50IGlycTAsIGlycTE7Cj4gQEAgLTQxOSw3ICs0MjcsNyBAQCBzdGF0aWMgaW50 IGFkdjc1MTFfaXJxX3Byb2Nlc3Moc3RydWN0IGFkdjc1MTEgKmFkdjc1MTEsCj4gYm9vbCBwcm9j ZXNzX2hwZCkgcmVnbWFwX3dyaXRlKGFkdjc1MTEtPnJlZ21hcCwgQURWNzUxMV9SRUdfSU5UKDEp LCBpcnExKTsKPiAKPiAgCWlmIChwcm9jZXNzX2hwZCAmJiBpcnEwICYgQURWNzUxMV9JTlQwX0hQ RCAmJiBhZHY3NTExLT5icmlkZ2UuZW5jb2RlcikKPiAtCQlkcm1faGVscGVyX2hwZF9pcnFfZXZl bnQoYWR2NzUxMS0+Y29ubmVjdG9yLmRldik7Cj4gKwkJc2NoZWR1bGVfd29yaygmYWR2NzUxMS0+ aXJxX3dvcmspOwo+IAo+ICAJaWYgKGlycTAgJiBBRFY3NTExX0lOVDBfRURJRF9SRUFEWSB8fCBp cnExICYgQURWNzUxMV9JTlQxX0REQ19FUlJPUikgewo+ICAJCWFkdjc1MTEtPmVkaWRfcmVhZCA9 IHRydWU7Cj4gQEAgLTEwMDYsNiArMTAxNCw4IEBAIHN0YXRpYyBpbnQgYWR2NzUxMV9wcm9iZShz dHJ1Y3QgaTJjX2NsaWVudCAqaTJjLCBjb25zdAo+IHN0cnVjdCBpMmNfZGV2aWNlX2lkICppZCkg Z290byBlcnJfaTJjX3VucmVnaXN0ZXJfZWRpZDsKPiAgCX0KPiAKPiArCUlOSVRfV09SSygmYWR2 NzUxMS0+aXJxX3dvcmssIGFkdjc1MTFfaXJxX3dvcmspOwo+ICsKPiAgCWlmIChpMmMtPmlycSkg ewo+ICAJCWluaXRfd2FpdHF1ZXVlX2hlYWQoJmFkdjc1MTEtPndxKTsKCi0tIApSZWdhcmRzLAoK TGF1cmVudCBQaW5jaGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJp LWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755005AbcK2Gqi (ORCPT ); Tue, 29 Nov 2016 01:46:38 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:59744 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752429AbcK2Gqb (ORCPT ); Tue, 29 Nov 2016 01:46:31 -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/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context Date: Tue, 29 Nov 2016 08:46:44 +0200 Message-ID: <3570409.tKEQBRfa3m@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: <1480395884-5471-2-git-send-email-john.stultz@linaro.org> References: <1480395884-5471-1-git-send-email-john.stultz@linaro.org> <1480395884-5471-2-git-send-email-john.stultz@linaro.org> 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, Thank you for the patch. On Monday 28 Nov 2016 21:04:40 John Stultz wrote: > I was recently seeing issues with EDID probing, where > the logic to wait for the EDID read bit to be set by the > IRQ wasn't happening and the code would time out and fail. > > Digging deeper, I found this was due to the fact that > IRQs were disabled as we were running in IRQ context from > the HPD signal. > > Thus this patch changes the logic to handle the HPD signal > via a work_struct so we can be out of irq context. > > With this patch, the EDID probing on hotplug does not time > out. > > Cc: David Airlie > Cc: Archit Taneja > Cc: Wolfram Sang > Cc: Lars-Peter Clausen > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz > --- > v2: Reworked to properly fix the issue rather then > just delaying for 200ms > > drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 ++ > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++++++++++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h > b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 992d76c..2a1e722 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h > @@ -317,6 +317,8 @@ struct adv7511 { > bool edid_read; > > wait_queue_head_t wq; > + struct work_struct irq_work; > + I'd call this hpd_work. > struct drm_bridge bridge; > struct drm_connector connector; > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b38e743 > 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -402,6 +402,14 @@ static bool adv7511_hpd(struct adv7511 *adv7511) > return false; > } > > +static void adv7511_irq_work(struct work_struct *work) > +{ > + struct adv7511 *adv7511 = container_of(work, struct adv7511, irq_work); > + > + drm_helper_hpd_irq_event(adv7511->connector.dev); > +} > + > + One blank line is enough. Apart from that, Reviewed-by: Laurent Pinchart > static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) > { > unsigned int irq0, irq1; > @@ -419,7 +427,7 @@ static int adv7511_irq_process(struct adv7511 *adv7511, > bool process_hpd) regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); > > if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder) > - drm_helper_hpd_irq_event(adv7511->connector.dev); > + schedule_work(&adv7511->irq_work); > > if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) { > adv7511->edid_read = true; > @@ -1006,6 +1014,8 @@ static int adv7511_probe(struct i2c_client *i2c, const > struct i2c_device_id *id) goto err_i2c_unregister_edid; > } > > + INIT_WORK(&adv7511->irq_work, adv7511_irq_work); > + > if (i2c->irq) { > init_waitqueue_head(&adv7511->wq); -- Regards, Laurent Pinchart