From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757539AbcILKQJ (ORCPT ); Mon, 12 Sep 2016 06:16:09 -0400 Received: from mailgw02.mediatek.com ([210.61.82.184]:40803 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751955AbcILKQI (ORCPT ); Mon, 12 Sep 2016 06:16:08 -0400 Message-ID: <1473675360.12398.25.camel@mtksdaap41> Subject: Re: [PATCH v7 6/9] drm/mediatek: add dsi interrupt control From: YT Shen To: CK Hu CC: , Philipp Zabel , David Airlie , Matthias Brugger , Daniel Kurtz , Mao Huang , Bibby Hsieh , "Daniel Vetter" , Thierry Reding , Jie Qiu , Maxime Ripard , Chris Wilson , shaoming chen , Jitao Shi , Boris Brezillon , Dan Carpenter , , , , , Sascha Hauer , , Date: Mon, 12 Sep 2016 18:16:00 +0800 In-Reply-To: <1473212348.11736.16.camel@mtksdaap41> References: <1472815484-43821-1-git-send-email-yt.shen@mediatek.com> <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> <1473212348.11736.16.camel@mtksdaap41> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi CK, On Wed, 2016-09-07 at 09:39 +0800, CK Hu wrote: > Hi, YT: > > On Fri, 2016-09-02 at 19:24 +0800, YT Shen wrote: > > From: shaoming chen > > > > add dsi interrupt control > > > > Signed-off-by: shaoming chen > > --- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 76 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 76 insertions(+) > > > > [snip...] > > > > > +static wait_queue_head_t _dsi_irq_wait_queue; > > I think it's better to move this global variable into platform driver > data. Maybe one day you have two dsi device and one global variable is > not enough. OK, we will move _dsi_irq_wait_queue into platform driver data. > > > + > > [snip...] > > > + > > +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id) > > +{ > > + struct mtk_dsi *dsi = dev_id; > > + u32 status, tmp; > > + u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG; > > + > > + status = readl(dsi->regs + DSI_INTSTA); > > If you define as > > status = readl(dsi->regs + DSI_INTSTA) & flag; > > You can remove 'flag' in below statements and reduce code size. Will do. > > > + > > + if (status & flag) { > > + do { > > + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK); > > + tmp = readl(dsi->regs + DSI_INTSTA); > > + } while (tmp & DSI_BUSY); > > + > > + mtk_dsi_mask(dsi, DSI_INTSTA, status & flag, 0); > > + mtk_dsi_irq_data_set(dsi, status & flag); > > + wake_up_interruptible(&_dsi_irq_wait_queue); > > + } > > + > > + return IRQ_HANDLED; > > +} > > + > > [snip...] > > > > > @@ -869,8 +926,27 @@ static int mtk_dsi_probe(struct platform_device *pdev) > > return ret; > > } > > > > + irq_num = platform_get_irq(pdev, 0); > > + if (irq_num < 0) { > > + dev_err(&pdev->dev, "failed to request dsi irq resource\n"); > > + return -EPROBE_DEFER; > > + } > > + > > + irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW); > > + ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq, > > + IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to request mediatek dsi irq\n"); > > + return -EPROBE_DEFER; > > + } > > + > > + dsi->irq_data = 0; > > You use devm_kzalloc() to allocate 'dsi', so this statement is > redundant. Will remove. Regards, yt.shen > > > + dev_info(dev, "dsi irq num is 0x%x\n", irq_num); > > + > > platform_set_drvdata(pdev, dsi); > > > > + init_waitqueue_head(&_dsi_irq_wait_queue); > > + > > return component_add(&pdev->dev, &mtk_dsi_component_ops); > > } > > > Regards, > CK > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: YT Shen Subject: Re: [PATCH v7 6/9] drm/mediatek: add dsi interrupt control Date: Mon, 12 Sep 2016 18:16:00 +0800 Message-ID: <1473675360.12398.25.camel@mtksdaap41> References: <1472815484-43821-1-git-send-email-yt.shen@mediatek.com> <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> <1473212348.11736.16.camel@mtksdaap41> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1473212348.11736.16.camel@mtksdaap41> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: CK Hu Cc: Daniel Vetter , Jie Qiu , Mao Huang , yingjoe.chen@mediatek.com, Dan Carpenter , Jitao Shi , Sascha Hauer , linux-mediatek@lists.infradead.org, dri-devel@lists.freedesktop.org, Matthias Brugger , shaoming chen , linux-arm-kernel@lists.infradead.org, srv_heupstream@mediatek.com, emil.l.velikov@gmail.com, linux-kernel@vger.kernel.org, Maxime Ripard List-Id: linux-mediatek@lists.infradead.org SGkgQ0ssCgpPbiBXZWQsIDIwMTYtMDktMDcgYXQgMDk6MzkgKzA4MDAsIENLIEh1IHdyb3RlOgo+ IEhpLCBZVDoKPiAKPiBPbiBGcmksIDIwMTYtMDktMDIgYXQgMTk6MjQgKzA4MDAsIFlUIFNoZW4g d3JvdGU6Cj4gPiBGcm9tOiBzaGFvbWluZyBjaGVuIDxzaGFvbWluZy5jaGVuQG1lZGlhdGVrLmNv bT4KPiA+IAo+ID4gYWRkIGRzaSBpbnRlcnJ1cHQgY29udHJvbAo+ID4gCj4gPiBTaWduZWQtb2Zm LWJ5OiBzaGFvbWluZyBjaGVuIDxzaGFvbWluZy5jaGVuQG1lZGlhdGVrLmNvbT4KPiA+IC0tLQo+ ID4gIGRyaXZlcnMvZ3B1L2RybS9tZWRpYXRlay9tdGtfZHNpLmMgfCA3NiArKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKysrKwo+ID4gIDEgZmlsZSBjaGFuZ2VkLCA3NiBpbnNlcnRp b25zKCspCj4gPiAKPiAKPiBbc25pcC4uLl0KPiAKPiA+ICAKPiA+ICtzdGF0aWMgd2FpdF9xdWV1 ZV9oZWFkX3QgX2RzaV9pcnFfd2FpdF9xdWV1ZTsKPiAKPiBJIHRoaW5rIGl0J3MgYmV0dGVyIHRv IG1vdmUgdGhpcyBnbG9iYWwgdmFyaWFibGUgaW50byBwbGF0Zm9ybSBkcml2ZXIKPiBkYXRhLiBN YXliZSBvbmUgZGF5IHlvdSBoYXZlIHR3byBkc2kgZGV2aWNlIGFuZCBvbmUgZ2xvYmFsIHZhcmlh YmxlIGlzCj4gbm90IGVub3VnaC4KT0ssIHdlIHdpbGwgbW92ZSBfZHNpX2lycV93YWl0X3F1ZXVl IGludG8gcGxhdGZvcm0gZHJpdmVyIGRhdGEuCgo+IAo+ID4gKwo+IAo+IFtzbmlwLi4uXQo+IAo+ ID4gKwo+ID4gK3N0YXRpYyBpcnFyZXR1cm5fdCBtdGtfZHNpX2lycShpbnQgaXJxLCB2b2lkICpk ZXZfaWQpCj4gPiArewo+ID4gKwlzdHJ1Y3QgbXRrX2RzaSAqZHNpID0gZGV2X2lkOwo+ID4gKwl1 MzIgc3RhdHVzLCB0bXA7Cj4gPiArCXUzMiBmbGFnID0gTFBSWF9SRF9SRFlfSU5UX0ZMQUcgfCBD TURfRE9ORV9JTlRfRkxBRyB8IFZNX0RPTkVfSU5UX0ZMQUc7Cj4gPiArCj4gPiArCXN0YXR1cyA9 IHJlYWRsKGRzaS0+cmVncyArIERTSV9JTlRTVEEpOwo+IAo+IElmIHlvdSBkZWZpbmUgYXMKPiAK PiBzdGF0dXMgPSByZWFkbChkc2ktPnJlZ3MgKyBEU0lfSU5UU1RBKSAmIGZsYWc7Cj4gCj4gWW91 IGNhbiByZW1vdmUgJ2ZsYWcnIGluIGJlbG93IHN0YXRlbWVudHMgYW5kIHJlZHVjZSBjb2RlIHNp emUuCldpbGwgZG8uCgo+IAo+ID4gKwo+ID4gKwlpZiAoc3RhdHVzICYgZmxhZykgewo+ID4gKwkJ ZG8gewo+ID4gKwkJCW10a19kc2lfbWFzayhkc2ksIERTSV9SQUNLLCBSQUNLLCBSQUNLKTsKPiA+ ICsJCQl0bXAgPSByZWFkbChkc2ktPnJlZ3MgKyBEU0lfSU5UU1RBKTsKPiA+ICsJCX0gd2hpbGUg KHRtcCAmIERTSV9CVVNZKTsKPiA+ICsKPiA+ICsJCW10a19kc2lfbWFzayhkc2ksIERTSV9JTlRT VEEsIHN0YXR1cyAmIGZsYWcsIDApOwo+ID4gKwkJbXRrX2RzaV9pcnFfZGF0YV9zZXQoZHNpLCBz dGF0dXMgJiBmbGFnKTsKPiA+ICsJCXdha2VfdXBfaW50ZXJydXB0aWJsZSgmX2RzaV9pcnFfd2Fp dF9xdWV1ZSk7Cj4gPiArCX0KPiA+ICsKPiA+ICsJcmV0dXJuIElSUV9IQU5ETEVEOwo+ID4gK30K PiA+ICsKPiAKPiBbc25pcC4uLl0KPiAKPiA+ICAKPiA+IEBAIC04NjksOCArOTI2LDI3IEBAIHN0 YXRpYyBpbnQgbXRrX2RzaV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQo+ID4g IAkJcmV0dXJuIHJldDsKPiA+ICAJfQo+ID4gIAo+ID4gKwlpcnFfbnVtID0gcGxhdGZvcm1fZ2V0 X2lycShwZGV2LCAwKTsKPiA+ICsJaWYgKGlycV9udW0gPCAwKSB7Cj4gPiArCQlkZXZfZXJyKCZw ZGV2LT5kZXYsICJmYWlsZWQgdG8gcmVxdWVzdCBkc2kgaXJxIHJlc291cmNlXG4iKTsKPiA+ICsJ CXJldHVybiAtRVBST0JFX0RFRkVSOwo+ID4gKwl9Cj4gPiArCj4gPiArCWlycV9zZXRfc3RhdHVz X2ZsYWdzKGlycV9udW0sIElSUV9UWVBFX0xFVkVMX0xPVyk7Cj4gPiArCXJldCA9IGRldm1fcmVx dWVzdF9pcnEoJnBkZXYtPmRldiwgaXJxX251bSwgbXRrX2RzaV9pcnEsCj4gPiArCQkJICAgICAg IElSUUZfVFJJR0dFUl9MT1csIGRldl9uYW1lKCZwZGV2LT5kZXYpLCBkc2kpOwo+ID4gKwlpZiAo cmV0KSB7Cj4gPiArCQlkZXZfZXJyKCZwZGV2LT5kZXYsICJmYWlsZWQgdG8gcmVxdWVzdCBtZWRp YXRlayBkc2kgaXJxXG4iKTsKPiA+ICsJCXJldHVybiAtRVBST0JFX0RFRkVSOwo+ID4gKwl9Cj4g PiArCj4gPiArCWRzaS0+aXJxX2RhdGEgPSAwOwo+IAo+IFlvdSB1c2UgZGV2bV9remFsbG9jKCkg dG8gYWxsb2NhdGUgJ2RzaScsIHNvIHRoaXMgc3RhdGVtZW50IGlzCj4gcmVkdW5kYW50LgpXaWxs IHJlbW92ZS4KClJlZ2FyZHMsCnl0LnNoZW4KCj4gCj4gPiArCWRldl9pbmZvKGRldiwgImRzaSBp cnEgbnVtIGlzIDB4JXhcbiIsIGlycV9udW0pOwo+ID4gKwo+ID4gIAlwbGF0Zm9ybV9zZXRfZHJ2 ZGF0YShwZGV2LCBkc2kpOwo+ID4gIAo+ID4gKwlpbml0X3dhaXRxdWV1ZV9oZWFkKCZfZHNpX2ly cV93YWl0X3F1ZXVlKTsKPiA+ICsKPiA+ICAJcmV0dXJuIGNvbXBvbmVudF9hZGQoJnBkZXYtPmRl diwgJm10a19kc2lfY29tcG9uZW50X29wcyk7Cj4gPiAgfQo+IAo+IAo+IFJlZ2FyZHMsCj4gQ0sg Cj4gCj4gCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18K ZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0 dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: yt.shen@mediatek.com (YT Shen) Date: Mon, 12 Sep 2016 18:16:00 +0800 Subject: [PATCH v7 6/9] drm/mediatek: add dsi interrupt control In-Reply-To: <1473212348.11736.16.camel@mtksdaap41> References: <1472815484-43821-1-git-send-email-yt.shen@mediatek.com> <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> <1473212348.11736.16.camel@mtksdaap41> Message-ID: <1473675360.12398.25.camel@mtksdaap41> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi CK, On Wed, 2016-09-07 at 09:39 +0800, CK Hu wrote: > Hi, YT: > > On Fri, 2016-09-02 at 19:24 +0800, YT Shen wrote: > > From: shaoming chen > > > > add dsi interrupt control > > > > Signed-off-by: shaoming chen > > --- > > drivers/gpu/drm/mediatek/mtk_dsi.c | 76 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 76 insertions(+) > > > > [snip...] > > > > > +static wait_queue_head_t _dsi_irq_wait_queue; > > I think it's better to move this global variable into platform driver > data. Maybe one day you have two dsi device and one global variable is > not enough. OK, we will move _dsi_irq_wait_queue into platform driver data. > > > + > > [snip...] > > > + > > +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id) > > +{ > > + struct mtk_dsi *dsi = dev_id; > > + u32 status, tmp; > > + u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG; > > + > > + status = readl(dsi->regs + DSI_INTSTA); > > If you define as > > status = readl(dsi->regs + DSI_INTSTA) & flag; > > You can remove 'flag' in below statements and reduce code size. Will do. > > > + > > + if (status & flag) { > > + do { > > + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK); > > + tmp = readl(dsi->regs + DSI_INTSTA); > > + } while (tmp & DSI_BUSY); > > + > > + mtk_dsi_mask(dsi, DSI_INTSTA, status & flag, 0); > > + mtk_dsi_irq_data_set(dsi, status & flag); > > + wake_up_interruptible(&_dsi_irq_wait_queue); > > + } > > + > > + return IRQ_HANDLED; > > +} > > + > > [snip...] > > > > > @@ -869,8 +926,27 @@ static int mtk_dsi_probe(struct platform_device *pdev) > > return ret; > > } > > > > + irq_num = platform_get_irq(pdev, 0); > > + if (irq_num < 0) { > > + dev_err(&pdev->dev, "failed to request dsi irq resource\n"); > > + return -EPROBE_DEFER; > > + } > > + > > + irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW); > > + ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq, > > + IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to request mediatek dsi irq\n"); > > + return -EPROBE_DEFER; > > + } > > + > > + dsi->irq_data = 0; > > You use devm_kzalloc() to allocate 'dsi', so this statement is > redundant. Will remove. Regards, yt.shen > > > + dev_info(dev, "dsi irq num is 0x%x\n", irq_num); > > + > > platform_set_drvdata(pdev, dsi); > > > > + init_waitqueue_head(&_dsi_irq_wait_queue); > > + > > return component_add(&pdev->dev, &mtk_dsi_component_ops); > > } > > > Regards, > CK > >