From mboxrd@z Thu Jan 1 00:00:00 1970 From: CK Hu Subject: Re: [PATCH v7 6/9] drm/mediatek: add dsi interrupt control Date: Wed, 7 Sep 2016 09:39:08 +0800 Message-ID: <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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: YT Shen 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 SGksIFlUOgoKT24gRnJpLCAyMDE2LTA5LTAyIGF0IDE5OjI0ICswODAwLCBZVCBTaGVuIHdyb3Rl Ogo+IEZyb206IHNoYW9taW5nIGNoZW4gPHNoYW9taW5nLmNoZW5AbWVkaWF0ZWsuY29tPgo+IAo+ IGFkZCBkc2kgaW50ZXJydXB0IGNvbnRyb2wKPiAKPiBTaWduZWQtb2ZmLWJ5OiBzaGFvbWluZyBj aGVuIDxzaGFvbWluZy5jaGVuQG1lZGlhdGVrLmNvbT4KPiAtLS0KPiAgZHJpdmVycy9ncHUvZHJt L21lZGlhdGVrL210a19kc2kuYyB8IDc2ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrCj4gIDEgZmlsZSBjaGFuZ2VkLCA3NiBpbnNlcnRpb25zKCspCj4gCgpbc25pcC4uLl0K Cj4gIAo+ICtzdGF0aWMgd2FpdF9xdWV1ZV9oZWFkX3QgX2RzaV9pcnFfd2FpdF9xdWV1ZTsKCkkg dGhpbmsgaXQncyBiZXR0ZXIgdG8gbW92ZSB0aGlzIGdsb2JhbCB2YXJpYWJsZSBpbnRvIHBsYXRm b3JtIGRyaXZlcgpkYXRhLiBNYXliZSBvbmUgZGF5IHlvdSBoYXZlIHR3byBkc2kgZGV2aWNlIGFu ZCBvbmUgZ2xvYmFsIHZhcmlhYmxlIGlzCm5vdCBlbm91Z2guCgo+ICsKCltzbmlwLi4uXQoKPiAr Cj4gK3N0YXRpYyBpcnFyZXR1cm5fdCBtdGtfZHNpX2lycShpbnQgaXJxLCB2b2lkICpkZXZfaWQp Cj4gK3sKPiArCXN0cnVjdCBtdGtfZHNpICpkc2kgPSBkZXZfaWQ7Cj4gKwl1MzIgc3RhdHVzLCB0 bXA7Cj4gKwl1MzIgZmxhZyA9IExQUlhfUkRfUkRZX0lOVF9GTEFHIHwgQ01EX0RPTkVfSU5UX0ZM QUcgfCBWTV9ET05FX0lOVF9GTEFHOwo+ICsKPiArCXN0YXR1cyA9IHJlYWRsKGRzaS0+cmVncyAr IERTSV9JTlRTVEEpOwoKSWYgeW91IGRlZmluZSBhcwoKc3RhdHVzID0gcmVhZGwoZHNpLT5yZWdz ICsgRFNJX0lOVFNUQSkgJiBmbGFnOwoKWW91IGNhbiByZW1vdmUgJ2ZsYWcnIGluIGJlbG93IHN0 YXRlbWVudHMgYW5kIHJlZHVjZSBjb2RlIHNpemUuCgo+ICsKPiArCWlmIChzdGF0dXMgJiBmbGFn KSB7Cj4gKwkJZG8gewo+ICsJCQltdGtfZHNpX21hc2soZHNpLCBEU0lfUkFDSywgUkFDSywgUkFD Syk7Cj4gKwkJCXRtcCA9IHJlYWRsKGRzaS0+cmVncyArIERTSV9JTlRTVEEpOwo+ICsJCX0gd2hp bGUgKHRtcCAmIERTSV9CVVNZKTsKPiArCj4gKwkJbXRrX2RzaV9tYXNrKGRzaSwgRFNJX0lOVFNU QSwgc3RhdHVzICYgZmxhZywgMCk7Cj4gKwkJbXRrX2RzaV9pcnFfZGF0YV9zZXQoZHNpLCBzdGF0 dXMgJiBmbGFnKTsKPiArCQl3YWtlX3VwX2ludGVycnVwdGlibGUoJl9kc2lfaXJxX3dhaXRfcXVl dWUpOwo+ICsJfQo+ICsKPiArCXJldHVybiBJUlFfSEFORExFRDsKPiArfQo+ICsKCltzbmlwLi4u XQoKPiAgCj4gQEAgLTg2OSw4ICs5MjYsMjcgQEAgc3RhdGljIGludCBtdGtfZHNpX3Byb2JlKHN0 cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4gIAkJcmV0dXJuIHJldDsKPiAgCX0KPiAgCj4g KwlpcnFfbnVtID0gcGxhdGZvcm1fZ2V0X2lycShwZGV2LCAwKTsKPiArCWlmIChpcnFfbnVtIDwg MCkgewo+ICsJCWRldl9lcnIoJnBkZXYtPmRldiwgImZhaWxlZCB0byByZXF1ZXN0IGRzaSBpcnEg cmVzb3VyY2VcbiIpOwo+ICsJCXJldHVybiAtRVBST0JFX0RFRkVSOwo+ICsJfQo+ICsKPiArCWly cV9zZXRfc3RhdHVzX2ZsYWdzKGlycV9udW0sIElSUV9UWVBFX0xFVkVMX0xPVyk7Cj4gKwlyZXQg PSBkZXZtX3JlcXVlc3RfaXJxKCZwZGV2LT5kZXYsIGlycV9udW0sIG10a19kc2lfaXJxLAo+ICsJ CQkgICAgICAgSVJRRl9UUklHR0VSX0xPVywgZGV2X25hbWUoJnBkZXYtPmRldiksIGRzaSk7Cj4g KwlpZiAocmV0KSB7Cj4gKwkJZGV2X2VycigmcGRldi0+ZGV2LCAiZmFpbGVkIHRvIHJlcXVlc3Qg bWVkaWF0ZWsgZHNpIGlycVxuIik7Cj4gKwkJcmV0dXJuIC1FUFJPQkVfREVGRVI7Cj4gKwl9Cj4g Kwo+ICsJZHNpLT5pcnFfZGF0YSA9IDA7CgpZb3UgdXNlIGRldm1fa3phbGxvYygpIHRvIGFsbG9j YXRlICdkc2knLCBzbyB0aGlzIHN0YXRlbWVudCBpcwpyZWR1bmRhbnQuCgo+ICsJZGV2X2luZm8o ZGV2LCAiZHNpIGlycSBudW0gaXMgMHgleFxuIiwgaXJxX251bSk7Cj4gKwo+ICAJcGxhdGZvcm1f c2V0X2RydmRhdGEocGRldiwgZHNpKTsKPiAgCj4gKwlpbml0X3dhaXRxdWV1ZV9oZWFkKCZfZHNp X2lycV93YWl0X3F1ZXVlKTsKPiArCj4gIAlyZXR1cm4gY29tcG9uZW50X2FkZCgmcGRldi0+ZGV2 LCAmbXRrX2RzaV9jb21wb25lbnRfb3BzKTsKPiAgfQoKClJlZ2FyZHMsCkNLIAoKCl9fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5n IGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVk ZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: ck.hu@mediatek.com (CK Hu) Date: Wed, 7 Sep 2016 09:39:08 +0800 Subject: [PATCH v7 6/9] drm/mediatek: add dsi interrupt control In-Reply-To: <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> References: <1472815484-43821-1-git-send-email-yt.shen@mediatek.com> <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> Message-ID: <1473212348.11736.16.camel@mtksdaap41> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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. > + [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. > + > + 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. > + 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 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755904AbcIGBjS (ORCPT ); Tue, 6 Sep 2016 21:39:18 -0400 Received: from mailgw02.mediatek.com ([210.61.82.184]:12454 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1754944AbcIGBjP (ORCPT ); Tue, 6 Sep 2016 21:39:15 -0400 Message-ID: <1473212348.11736.16.camel@mtksdaap41> Subject: Re: [PATCH v7 6/9] drm/mediatek: add dsi interrupt control From: CK Hu To: YT Shen 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: Wed, 7 Sep 2016 09:39:08 +0800 In-Reply-To: <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> References: <1472815484-43821-1-git-send-email-yt.shen@mediatek.com> <1472815484-43821-7-git-send-email-yt.shen@mediatek.com> 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, 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. > + [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. > + > + 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. > + 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