From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 3/3] drm/omap: displays: encoder-tpd12s015: Support for hot plug detection Date: Tue, 23 May 2017 12:48:30 +0300 Message-ID: <8383085.j5ORBifRWg@avalon> References: <20170515090312.32051-1-peter.ujfalusi@ti.com> <20170515090312.32051-4-peter.ujfalusi@ti.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 B4FC66E105 for ; Tue, 23 May 2017 09:48:13 +0000 (UTC) In-Reply-To: <20170515090312.32051-4-peter.ujfalusi@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Peter Ujfalusi Cc: tomi.valkeinen@ti.com, dri-devel@lists.freedesktop.org, jsarha@ti.com, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGkgUGV0ZXIsCgpUaGFuayB5b3UgZm9yIHRoZSBwYXRjaC4KCk9uIE1vbmRheSAxNSBNYXkgMjAx NyAxMjowMzoxMiBQZXRlciBVamZhbHVzaSB3cm90ZToKPiBVc2UgaW50ZXJydXB0IGhhbmRsZXIg Zm9yIGhwZCBHUElPIHRvIHJlYWN0IHRvIEhQRCBjaGFuZ2VzLgo+IAo+IFNpZ25lZC1vZmYtYnk6 IFBldGVyIFVqZmFsdXNpIDxwZXRlci51amZhbHVzaUB0aS5jb20+Cj4gLS0tCj4gIC4uLi9ncHUv ZHJtL29tYXBkcm0vZGlzcGxheXMvZW5jb2Rlci10cGQxMnMwMTUuYyAgIHwgODEgKysrKysrKysr KysrKysrKysrKwo+ICAxIGZpbGUgY2hhbmdlZCwgODEgaW5zZXJ0aW9ucygrKQo+IAo+IGRpZmYg LS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vb21hcGRybS9kaXNwbGF5cy9lbmNvZGVyLXRwZDEyczAx NS5jCj4gYi9kcml2ZXJzL2dwdS9kcm0vb21hcGRybS9kaXNwbGF5cy9lbmNvZGVyLXRwZDEyczAx NS5jIGluZGV4Cj4gNTgyNzZhNDgxMTJlLi41MjliODUwOWRkOTkgMTAwNjQ0Cj4gLS0tIGEvZHJp dmVycy9ncHUvZHJtL29tYXBkcm0vZGlzcGxheXMvZW5jb2Rlci10cGQxMnMwMTUuYwo+ICsrKyBi L2RyaXZlcnMvZ3B1L2RybS9vbWFwZHJtL2Rpc3BsYXlzL2VuY29kZXItdHBkMTJzMDE1LmMKPiBA QCAtMTUsMTIgKzE1LDE3IEBACj4gICNpbmNsdWRlIDxsaW51eC9zbGFiLmg+Cj4gICNpbmNsdWRl IDxsaW51eC9wbGF0Zm9ybV9kZXZpY2UuaD4KPiAgI2luY2x1ZGUgPGxpbnV4L2dwaW8vY29uc3Vt ZXIuaD4KPiArI2luY2x1ZGUgPGxpbnV4L3NwaW5sb2NrLmg+CgpEaWQgeW91IG1lYW4gbGludXgv bXV0ZXguaCA/Cgo+IAo+ICAjaW5jbHVkZSAiLi4vZHNzL29tYXBkc3MuaCIKPiAKPiAgc3RydWN0 IHBhbmVsX2Rydl9kYXRhIHsKPiAgCXN0cnVjdCBvbWFwX2Rzc19kZXZpY2UgZHNzZGV2Owo+ICAJ c3RydWN0IG9tYXBfZHNzX2RldmljZSAqaW47Cj4gKwl2b2lkICgqaHBkX2NiKSh2b2lkICpjYl9k YXRhLCBlbnVtIGRybV9jb25uZWN0b3Jfc3RhdHVzIHN0YXR1cyk7Cj4gKwl2b2lkICpocGRfY2Jf ZGF0YTsKPiArCWJvb2wgaHBkX2VuYWJsZWQ7Cj4gKwlzdHJ1Y3QgbXV0ZXggaHBkX2xvY2s7Cj4g Cj4gIAlzdHJ1Y3QgZ3Bpb19kZXNjICpjdF9jcF9ocGRfZ3BpbzsKPiAgCXN0cnVjdCBncGlvX2Rl c2MgKmxzX29lX2dwaW87Cj4gQEAgLTE2Miw2ICsxNjcsNDkgQEAgc3RhdGljIGJvb2wgdHBkX2Rl dGVjdChzdHJ1Y3Qgb21hcF9kc3NfZGV2aWNlICpkc3NkZXYpCj4gIAlyZXR1cm4gZ3Bpb2RfZ2V0 X3ZhbHVlX2NhbnNsZWVwKGRkYXRhLT5ocGRfZ3Bpbyk7Cj4gIH0KPiAKPiArc3RhdGljIGludCB0 cGRfcmVnaXN0ZXJfaHBkX2NiKHN0cnVjdCBvbWFwX2Rzc19kZXZpY2UgKmRzc2RldiwKPiArCQkJ ICAgICAgIHZvaWQgKCpjYikodm9pZCAqY2JfZGF0YSwKPiArCQkJCQkgIGVudW0gZHJtX2Nvbm5l Y3Rvcl9zdGF0dXMgc3RhdHVzKSwKPiArCQkJICAgICAgIHZvaWQgKmNiX2RhdGEpCj4gK3sKPiAr CXN0cnVjdCBwYW5lbF9kcnZfZGF0YSAqZGRhdGEgPSB0b19wYW5lbF9kYXRhKGRzc2Rldik7Cj4g Kwo+ICsJbXV0ZXhfbG9jaygmZGRhdGEtPmhwZF9sb2NrKTsKPiArCWRkYXRhLT5ocGRfY2IgPSBj YjsKPiArCWRkYXRhLT5ocGRfY2JfZGF0YSA9IGNiX2RhdGE7Cj4gKwltdXRleF91bmxvY2soJmRk YXRhLT5ocGRfbG9jayk7Cj4gKwo+ICsJcmV0dXJuIDA7Cj4gK30KPiArCj4gK3N0YXRpYyB2b2lk IHRwZF91bnJlZ2lzdGVyX2hwZF9jYihzdHJ1Y3Qgb21hcF9kc3NfZGV2aWNlICpkc3NkZXYpCj4g K3sKPiArCXN0cnVjdCBwYW5lbF9kcnZfZGF0YSAqZGRhdGEgPSB0b19wYW5lbF9kYXRhKGRzc2Rl dik7Cj4gKwo+ICsJbXV0ZXhfbG9jaygmZGRhdGEtPmhwZF9sb2NrKTsKPiArCWRkYXRhLT5ocGRf Y2IgPSBOVUxMOwo+ICsJZGRhdGEtPmhwZF9jYl9kYXRhID0gTlVMTDsKPiArCW11dGV4X3VubG9j aygmZGRhdGEtPmhwZF9sb2NrKTsKPiArfQo+ICsKPiArc3RhdGljIHZvaWQgdHBkX2VuYWJsZV9o cGQoc3RydWN0IG9tYXBfZHNzX2RldmljZSAqZHNzZGV2KQo+ICt7Cj4gKwlzdHJ1Y3QgcGFuZWxf ZHJ2X2RhdGEgKmRkYXRhID0gdG9fcGFuZWxfZGF0YShkc3NkZXYpOwo+ICsKPiArCW11dGV4X2xv Y2soJmRkYXRhLT5ocGRfbG9jayk7Cj4gKwlkZGF0YS0+aHBkX2VuYWJsZWQgPSB0cnVlOwo+ICsJ bXV0ZXhfdW5sb2NrKCZkZGF0YS0+aHBkX2xvY2spOwo+ICt9Cj4gKwo+ICtzdGF0aWMgdm9pZCB0 cGRfZGlzYWJsZV9ocGQoc3RydWN0IG9tYXBfZHNzX2RldmljZSAqZHNzZGV2KQo+ICt7Cj4gKwlz dHJ1Y3QgcGFuZWxfZHJ2X2RhdGEgKmRkYXRhID0gdG9fcGFuZWxfZGF0YShkc3NkZXYpOwo+ICsK PiArCW11dGV4X2xvY2soJmRkYXRhLT5ocGRfbG9jayk7Cj4gKwlkZGF0YS0+aHBkX2VuYWJsZWQg PSBmYWxzZTsKPiArCW11dGV4X3VubG9jaygmZGRhdGEtPmhwZF9sb2NrKTsKPiArfQo+ICsKPiAg c3RhdGljIGludCB0cGRfc2V0X2luZm9mcmFtZShzdHJ1Y3Qgb21hcF9kc3NfZGV2aWNlICpkc3Nk ZXYsCj4gIAkJY29uc3Qgc3RydWN0IGhkbWlfYXZpX2luZm9mcmFtZSAqYXZpKQo+ICB7Cj4gQEAg LTE5MywxMCArMjQxLDM0IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3Qgb21hcGRzc19oZG1pX29wcyB0 cGRfaGRtaV9vcHMgPSB7Cj4gCj4gIAkucmVhZF9lZGlkCQk9IHRwZF9yZWFkX2VkaWQsCj4gIAku ZGV0ZWN0CQkJPSB0cGRfZGV0ZWN0LAo+ICsJLnJlZ2lzdGVyX2hwZF9jYgk9IHRwZF9yZWdpc3Rl cl9ocGRfY2IsCj4gKwkudW5yZWdpc3Rlcl9ocGRfY2IJPSB0cGRfdW5yZWdpc3Rlcl9ocGRfY2Is Cj4gKwkuZW5hYmxlX2hwZAkJPSB0cGRfZW5hYmxlX2hwZCwKPiArCS5kaXNhYmxlX2hwZAkJPSB0 cGRfZGlzYWJsZV9ocGQsCj4gIAkuc2V0X2luZm9mcmFtZQkJPSB0cGRfc2V0X2luZm9mcmFtZSwK PiAgCS5zZXRfaGRtaV9tb2RlCQk9IHRwZF9zZXRfaGRtaV9tb2RlLAo+ICB9Owo+IAo+ICtzdGF0 aWMgaXJxcmV0dXJuX3QgdHBkX2hwZF9pc3IoaW50IGlycSwgdm9pZCAqZGF0YSkKPiArewo+ICsJ c3RydWN0IHBhbmVsX2Rydl9kYXRhICpkZGF0YSA9IGRhdGE7Cj4gKwo+ICsJbXV0ZXhfbG9jaygm ZGRhdGEtPmhwZF9sb2NrKTsKPiArCWlmIChkZGF0YS0+aHBkX2VuYWJsZWQgJiYgZGRhdGEtPmhw ZF9jYikgewo+ICsJCWVudW0gZHJtX2Nvbm5lY3Rvcl9zdGF0dXMgc3RhdHVzOwo+ICsKPiArCQlp ZiAodHBkX2RldGVjdCgmZGRhdGEtPmRzc2RldikpCj4gKwkJCXN0YXR1cyA9IGNvbm5lY3Rvcl9z dGF0dXNfY29ubmVjdGVkOwo+ICsJCWVsc2UKPiArCQkJc3RhdHVzID0gY29ubmVjdG9yX3N0YXR1 c19kaXNjb25uZWN0ZWQ7Cj4gKwo+ICsJCWRkYXRhLT5ocGRfY2IoZGRhdGEtPmhwZF9jYl9kYXRh LCBzdGF0dXMpOwo+ICsJfQo+ICsJbXV0ZXhfdW5sb2NrKCZkZGF0YS0+aHBkX2xvY2spOwo+ICsK PiArCXJldHVybiBJUlFfSEFORExFRDsKPiArfQo+ICsKPiAgc3RhdGljIGludCB0cGRfcHJvYmVf b2Yoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldikKPiAgewo+ICAJc3RydWN0IHBhbmVsX2Ry dl9kYXRhICpkZGF0YSA9IHBsYXRmb3JtX2dldF9kcnZkYXRhKHBkZXYpOwo+IEBAIC0yNjEsNiAr MzMzLDE1IEBAIHN0YXRpYyBpbnQgdHBkX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBk ZXYpCj4gCj4gIAlkZGF0YS0+aHBkX2dwaW8gPSBncGlvOwo+IAo+ICsJbXV0ZXhfaW5pdCgmZGRh dGEtPmhwZF9sb2NrKTsKPiArCj4gKwlyID0gZGV2bV9yZXF1ZXN0X3RocmVhZGVkX2lycSgmcGRl di0+ZGV2LCBncGlvZF90b19pcnEoZGRhdGEtCj5ocGRfZ3BpbyksCgpBcyB0aGUgY29ubmVjdG9y IGNvZGUgY2FuIGhhbmRsZSBHUElPIEhQRCB0aGFua3MgdG8gcGF0Y2ggMi8zLCB3aHkgZG9lcyBp dCAKaGF2ZSB0byBiZSBkdXBsaWNhdGVkIGhlcmUgPyBJIGFncmVlIHRoYXQgZW5jb2RlcnMgc2hv dWxkIHN1cHBvcnQgcmVwb3J0aW5nIG9mIApob3RwbHVnIGV2ZW50cyB3aGVuIHRoZSBIUEQgc2ln bmFsIGlzIGNvbm5lY3RlZCB0byBhbiBlbmNvZGVyLCBidXQgaWYgaXQncyAKY29ubmVjdGVkIHRv IGEgR1BJTywgaXQgc2VlbXMgdG8gbWUgdGhhdCBpdCBzaG91bGQgYmUgdGhlIHNvbGUgcmVzcG9u c2liaWxpdHkgCm9mIHRoZSBjb25uZWN0b3IgY29kZSB0byBoYW5kbGUgaXQuCgo+ICsJCU5VTEws IHRwZF9ocGRfaXNyLAo+ICsJCUlSUUZfVFJJR0dFUl9SSVNJTkcgfCBJUlFGX1RSSUdHRVJfRkFM TElORyB8IElSUUZfT05FU0hPVCwKPiArCQkidHBkMTJzMDE1IGhwZCIsIGRkYXRhKTsKPiArCWlm IChyKQo+ICsJCWdvdG8gZXJyX2dwaW87Cj4gKwo+ICAJZHNzZGV2ID0gJmRkYXRhLT5kc3NkZXY7 Cj4gIAlkc3NkZXYtPm9wcy5oZG1pID0gJnRwZF9oZG1pX29wczsKPiAgCWRzc2Rldi0+ZGV2ID0g JnBkZXYtPmRldjsKCi0tIApSZWdhcmRzLAoKTGF1cmVudCBQaW5jaGFydAoKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlz dApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030379AbdEWJsP (ORCPT ); Tue, 23 May 2017 05:48:15 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:46263 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934610AbdEWJsO (ORCPT ); Tue, 23 May 2017 05:48:14 -0400 From: Laurent Pinchart To: Peter Ujfalusi Cc: tomi.valkeinen@ti.com, airlied@linux.ie, jsarha@ti.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] drm/omap: displays: encoder-tpd12s015: Support for hot plug detection Date: Tue, 23 May 2017 12:48:30 +0300 Message-ID: <8383085.j5ORBifRWg@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.16-gentoo; KDE/4.14.32; x86_64; ; ) In-Reply-To: <20170515090312.32051-4-peter.ujfalusi@ti.com> References: <20170515090312.32051-1-peter.ujfalusi@ti.com> <20170515090312.32051-4-peter.ujfalusi@ti.com> 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 Peter, Thank you for the patch. On Monday 15 May 2017 12:03:12 Peter Ujfalusi wrote: > Use interrupt handler for hpd GPIO to react to HPD changes. > > Signed-off-by: Peter Ujfalusi > --- > .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 81 +++++++++++++++++++ > 1 file changed, 81 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c index > 58276a48112e..529b8509dd99 100644 > --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > @@ -15,12 +15,17 @@ > #include > #include > #include > +#include Did you mean linux/mutex.h ? > > #include "../dss/omapdss.h" > > struct panel_drv_data { > struct omap_dss_device dssdev; > struct omap_dss_device *in; > + void (*hpd_cb)(void *cb_data, enum drm_connector_status status); > + void *hpd_cb_data; > + bool hpd_enabled; > + struct mutex hpd_lock; > > struct gpio_desc *ct_cp_hpd_gpio; > struct gpio_desc *ls_oe_gpio; > @@ -162,6 +167,49 @@ static bool tpd_detect(struct omap_dss_device *dssdev) > return gpiod_get_value_cansleep(ddata->hpd_gpio); > } > > +static int tpd_register_hpd_cb(struct omap_dss_device *dssdev, > + void (*cb)(void *cb_data, > + enum drm_connector_status status), > + void *cb_data) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_cb = cb; > + ddata->hpd_cb_data = cb_data; > + mutex_unlock(&ddata->hpd_lock); > + > + return 0; > +} > + > +static void tpd_unregister_hpd_cb(struct omap_dss_device *dssdev) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_cb = NULL; > + ddata->hpd_cb_data = NULL; > + mutex_unlock(&ddata->hpd_lock); > +} > + > +static void tpd_enable_hpd(struct omap_dss_device *dssdev) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_enabled = true; > + mutex_unlock(&ddata->hpd_lock); > +} > + > +static void tpd_disable_hpd(struct omap_dss_device *dssdev) > +{ > + struct panel_drv_data *ddata = to_panel_data(dssdev); > + > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_enabled = false; > + mutex_unlock(&ddata->hpd_lock); > +} > + > static int tpd_set_infoframe(struct omap_dss_device *dssdev, > const struct hdmi_avi_infoframe *avi) > { > @@ -193,10 +241,34 @@ static const struct omapdss_hdmi_ops tpd_hdmi_ops = { > > .read_edid = tpd_read_edid, > .detect = tpd_detect, > + .register_hpd_cb = tpd_register_hpd_cb, > + .unregister_hpd_cb = tpd_unregister_hpd_cb, > + .enable_hpd = tpd_enable_hpd, > + .disable_hpd = tpd_disable_hpd, > .set_infoframe = tpd_set_infoframe, > .set_hdmi_mode = tpd_set_hdmi_mode, > }; > > +static irqreturn_t tpd_hpd_isr(int irq, void *data) > +{ > + struct panel_drv_data *ddata = data; > + > + mutex_lock(&ddata->hpd_lock); > + if (ddata->hpd_enabled && ddata->hpd_cb) { > + enum drm_connector_status status; > + > + if (tpd_detect(&ddata->dssdev)) > + status = connector_status_connected; > + else > + status = connector_status_disconnected; > + > + ddata->hpd_cb(ddata->hpd_cb_data, status); > + } > + mutex_unlock(&ddata->hpd_lock); > + > + return IRQ_HANDLED; > +} > + > static int tpd_probe_of(struct platform_device *pdev) > { > struct panel_drv_data *ddata = platform_get_drvdata(pdev); > @@ -261,6 +333,15 @@ static int tpd_probe(struct platform_device *pdev) > > ddata->hpd_gpio = gpio; > > + mutex_init(&ddata->hpd_lock); > + > + r = devm_request_threaded_irq(&pdev->dev, gpiod_to_irq(ddata- >hpd_gpio), As the connector code can handle GPIO HPD thanks to patch 2/3, why does it have to be duplicated here ? I agree that encoders should support reporting of hotplug events when the HPD signal is connected to an encoder, but if it's connected to a GPIO, it seems to me that it should be the sole responsibility of the connector code to handle it. > + NULL, tpd_hpd_isr, > + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > + "tpd12s015 hpd", ddata); > + if (r) > + goto err_gpio; > + > dssdev = &ddata->dssdev; > dssdev->ops.hdmi = &tpd_hdmi_ops; > dssdev->dev = &pdev->dev; -- Regards, Laurent Pinchart