From mboxrd@z Thu Jan 1 00:00:00 1970 From: CK Hu Subject: Re: [PATCH v6 07/10] drm/mediatek: add dsi transfer function Date: Thu, 11 Aug 2016 15:10:12 +0800 Message-ID: <1470899412.16554.72.camel@mtksdaap41> References: <1470308844-20895-1-git-send-email-yt.shen@mediatek.com> <1470308844-20895-8-git-send-email-yt.shen@mediatek.com> <1470391697.16554.60.camel@mtksdaap41> <1470813866.27526.14.camel@mtksdaap41> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1470813866.27526.14.camel@mtksdaap41> 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: Mark Rutland , dri-devel@lists.freedesktop.org, Russell King , Mao Huang , yingjoe.chen@mediatek.com, devicetree@vger.kernel.org, Sascha Hauer , Pawel Moll , Ian Campbell , Rob Herring , linux-mediatek@lists.infradead.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, Kumar Gala List-Id: linux-mediatek@lists.infradead.org SGksIFlUOgoKT24gV2VkLCAyMDE2LTA4LTEwIGF0IDE1OjI0ICswODAwLCBZVCBTaGVuIHdyb3Rl Ogo+IEhpIENLLAo+IAo+IE9uIEZyaSwgMjAxNi0wOC0wNSBhdCAxODowOCArMDgwMCwgQ0sgSHUg d3JvdGU6Cj4gPiBIaSwgWVQ6Cj4gPiAKPiA+IE9uIFRodSwgMjAxNi0wOC0wNCBhdCAxOTowNyAr MDgwMCwgWVQgU2hlbiB3cm90ZToKPiA+ID4gRnJvbTogc2hhb21pbmcgY2hlbiA8c2hhb21pbmcu Y2hlbkBtZWRpYXRlay5jb20+Cj4gPiA+IAo+ID4gPiBhZGQgZHNpIHJlYWQvd3JpdGUgY29tbWFu ZHMgZm9yIHRyYW5zZmVyIGZ1bmN0aW9uCj4gPiA+IAo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBzaGFv bWluZyBjaGVuIDxzaGFvbWluZy5jaGVuQG1lZGlhdGVrLmNvbT4KPiA+ID4gLS0tCj4gPiA+ICBk cml2ZXJzL2dwdS9kcm0vbWVkaWF0ZWsvbXRrX2RzaS5jIHwgIDI2MSArKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKysKPiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCAyNjEgaW5zZXJ0aW9u cygrKQo+ID4gPiAKCltzbmlwLi4uXQoKPiA+ID4gKwo+ID4gPiArc3RhdGljIHZvaWQgbXRrX2Rz aV9jbWRxKHN0cnVjdCBtdGtfZHNpICpkc2ksIGNvbnN0IHN0cnVjdCBtaXBpX2RzaV9tc2cgKm1z ZykKPiA+ID4gK3sKPiA+ID4gKwljb25zdCBjaGFyICp0eF9idWYgPSBtc2ctPnR4X2J1ZjsKPiA+ ID4gKwl1MzIgcmVnX3ZhbCwgaTsKPiA+ID4gKwl1MTYgd2MxNjsKPiA+ID4gKwl1OCBjb25maWcs IGRhdGEwLCBkYXRhMSwgdHlwZTsKPiA+ID4gKwo+ID4gPiArCWlmIChNVEtfRFNJX0hPU1RfSVNf UkVBRCh0eXBlKSkgewo+ID4gCj4gPiAndHlwZScgaXMgdXNlZCBiZWZvcmUgYXNzaWduZWQuCj4g V2lsbCBmaXguCj4gCj4gPiAKPiA+ID4gKwkJY29uZmlnID0gNDsKPiA+ID4gKwkJZGF0YTAgPSB0 eF9idWZbMF07Cj4gPiA+ICsKPiA+ID4gKwkJaWYgKG1zZy0+cnhfbGVuIDwgMykKPiA+ID4gKwkJ CXR5cGUgPSBNSVBJX0RTSV9EQ1NfUkVBRDsKPiA+ID4gKwkJZWxzZQo+ID4gPiArCQkJdHlwZSA9 IE1JUElfRFNJX0dFTkVSSUNfUkVBRF9SRVFVRVNUXzFfUEFSQU07Cj4gPiA+ICsKPiA+ID4gKwkJ ZGF0YTEgPSAwOwo+ID4gPiArCQlyZWdfdmFsID0gKGRhdGExIDw8IDI0KSB8IChkYXRhMCA8PCAx NikgfCAodHlwZSA8PCA4KSB8IGNvbmZpZzsKPiA+ID4gKwo+ID4gPiArCQl3cml0ZWwocmVnX3Zh bCwgZHNpLT5yZWdzICsgRFNJX0NNRFEwKTsKPiA+ID4gKwkJbXRrX2RzaV9tYXNrKGRzaSwgRFNJ X0NNRFFfU0laRSwgQ01EUV9TSVpFLCAxKTsKPiA+IAo+ID4gSSB0aGluayB0aGlzIHBhcnQgbG9v a3MgbGlrZSAnZWxzZScgcGFydC4gVGhlIGRpZmZlcmVuY2UgaXMgdHlwZSBhbmQKPiA+IGNvbmZp Zy4gSSB0aGluayB0eXBlIHNob3VsZCBiZSBtc2ctPnR5cGUgYW5kIHlvdSBjYW4gaW5kZXBlbmRl bnRseSBzZXQKPiA+IEJJVCgyKSBvZiBjb25maWcuCj4gbXNnLT50eXBlIG9ubHkgdGVsbHMgYWJv dXQgcmVhZCBvciB3cml0ZSwgZm9yIHRoZSBkZXRhaWxzIHdlIG5lZWQgdG8KPiBjaGVjayBvdGhl ciBwYXJhbWV0ZXJzLiAgVGhpcyBwYXJ0IGlzIGZvciByZWFkLCB0aGUgZWxzZSBwYXJ0IGlzIGZv cgo+IHdyaXRlIHNob3J0IHBhY2tldC4gIE5vdCBvbmx5IEJJVCgyKSBvZiBjb25maWcgaXMgZGlm ZmVyZW50LCBidXQgYWxzbwo+IHR5cGUgYW5kIGRhdGExIGlzIGNoYW5nZWQuICBTdWNoIGEgY2hh bmdlIHdvdWxkIGxlYWQgdG8gZGlmZmljdWx0IHRvCj4gdW5kZXJzdGFuZC4KPiAKPiA+IAo+ID4g PiArCX0gZWxzZSBpZiAobXNnLT50eF9sZW4gPiAyKSB7CS8qIHNlbmQgbG9uZyBwYWNrZXQgKi8K PiA+ID4gKwkJY29uZmlnID0gMjsKPiA+ID4gKwkJdHlwZSA9IG1zZy0+dHlwZTsKPiA+ID4gKwkJ d2MxNiA9IG1zZy0+dHhfbGVuOwo+ID4gPiArCj4gPiA+ICsJCXJlZ192YWwgPSAod2MxNiA8PCAx NikgfCAodHlwZSA8PCA4KSB8IGNvbmZpZzsKPiA+ID4gKwo+ID4gPiArCQl3cml0ZWwocmVnX3Zh bCwgZHNpLT5yZWdzICsgRFNJX0NNRFEwKTsKPiA+ID4gKwo+ID4gPiArCQlmb3IgKGkgPSAwOyBp IDwgbXNnLT50eF9sZW47IGkrKykKPiA+ID4gKwkJCXdyaXRlYih0eF9idWZbaV0sIGRzaS0+cmVn cyArIERTSV9DTURRMCArIDQgKyBpKTsKPiA+ID4gKwo+ID4gPiArCQltdGtfZHNpX21hc2soZHNp LCBEU0lfQ01EUV9TSVpFLCBDTURRX1NJWkUsCj4gPiA+ICsJCQkgICAgIDEgKyAobXNnLT50eF9s ZW4gKyAzKSAvIDQpOwo+ID4gPiArCX0gZWxzZSB7CS8qIHNlbmQgc2hvcnQgcGFja2V0ICovCj4g PiA+ICsJCWNvbmZpZyA9IDA7Cj4gPiA+ICsJCWRhdGEwID0gdHhfYnVmWzBdOwo+ID4gPiArCj4g PiA+ICsJCWlmIChtc2ctPnR4X2xlbiA9PSAyKSB7Cj4gPiA+ICsJCQl0eXBlID0gTUlQSV9EU0lf RENTX1NIT1JUX1dSSVRFX1BBUkFNOwo+ID4gCj4gPiBXaHkgZG8geW91IG5vdCBzZXQgdHlwZSBh cyBtc2ctPnR5cGU/IFRoaXMgYmVoYXZpb3IgbG9va3MgbGlrZSB5b3UKPiA+IG1vZGlmeSB0cmFu c2ZlciB0eXBlLCBidXQgaXMgdGhpcyBhY2NlcHRhYmxlPwo+IG1zZy0+dHlwZSBvbmx5IHRlbGxz IGFib3V0IHJlYWQgb3Igd3JpdGUsIGZvciB0aGUgZGV0YWlscyB3ZSBuZWVkIHRvCj4gY2hlY2sg b3RoZXIgcGFyYW1ldGVycy4KCkkgdGhpbmsgeW91IGNvdWxkIHJld3JpdGUgbXRrX2RzaV9jbWRx KCkgYXMgZm9sbG93OgoKc3RhdGljIHZvaWQgbXRrX2RzaV9jbWRxKHN0cnVjdCBtdGtfZHNpICpk c2ksIGNvbnN0IHN0cnVjdCBtaXBpX2RzaV9tc2cKKm1zZykKewoJdTMyIGk7Cgl1MzIgc3JjX29m ZiA9IChtc2ctPnR4X2xlbiA+IDIpID8gNCA6IDI7Cgl1MzIgY21kcV9zaXplID0gKG1zZy0+dHhf bGVuID4gMikgPyAxICsgKG1zZy0+dHhfbGVuICsgMykgLyA0IDogMTsKCXUzMiBjb25maWcgPSAo bXNnLT50eF9sZW4gPiAyKSA/IEJJVCgxKSA6IDA7Cgl1MzIgdHlwZSA9IG1zZy0+dHlwZTsKCglp ZiAoTVRLX0RTSV9IT1NUX0lTX1JFQUQodHlwZSkpCgkJY29uZmlnIHw9IEJJVCgyKTsKCgl3cml0 ZWwoKHR5cGUgPDwgOCkgfHwgY29uZmlnLCBkc2ktPnJlZ3MgKyBEU0lfQ01EUTApOwoJZm9yIChp ID0gMDsgaSA8IG1zZy0+dHhfbGVuOyBpKyspCgkJd3JpdGViKG1zZy0+dHhfYnVmW2ldLCBkc2kt PnJlZ3MgKyBEU0lfQ01EUTAgKyBzcmNfb2ZmICsgaSk7CgltdGtfZHNpX21hc2soZHNpLCBEU0lf Q01EUV9TSVpFLCBDTURRX1NJWkUsIGNtZHFfc2l6ZSk7Cn0KCklmIERTSSBIVyBkb2VzIG5vdCBz dXBwb3J0IHNvbWUgdHlwZSBhbmQgaXQgY2FuIGJlIHRyYW5zZmVycmVkIHRvIG90aGVyCnR5cGUs IHlvdSBjb3VsZCBhZGQgYSB0cmFuc2ZlciBmdW5jdGlvbiBsaWtlIHRoaXMKCnN0YXRpYyB1OCBt dGtfZHNpX21hcF90b19zdXBwb3J0ZWRfdHlwZSh1OCB0eXBlKQp7Cglzd2l0Y2godHlwZSkgewoJ Y2FzZSBNSVBJX0RTSV9EQ1NfU0hPUlRfV1JJVEU6CgkJcmV0dXJuIE1JUElfRFNJX0RDU19TSE9S VF9XUklURV9QQVJBTTsKCX0KCglyZXR1cm4gdHlwZTsKfQoKYW5kIHRoZW4KCnUzMiB0eXBlID0g bXRrX2RzaV9tYXBfdG9fc3VwcG9ydGVkX3R5cGUobXNnLT50eXBlKTsKCj4gCj4gPiAKPiA+ID4g KwkJCWRhdGExID0gdHhfYnVmWzFdOwo+ID4gPiArCQl9IGVsc2Ugewo+ID4gPiArCQkJdHlwZSA9 IE1JUElfRFNJX0RDU19TSE9SVF9XUklURTsKPiA+ID4gKwkJCWRhdGExID0gMDsKPiA+ID4gKwkJ fQo+ID4gPiArCj4gPiA+ICsJCXJlZ192YWwgPSAoZGF0YTEgPDwgMjQpIHwgKGRhdGEwIDw8IDE2 KSB8ICh0eXBlIDw8IDgpIHwgY29uZmlnOwo+ID4gPiArCj4gPiA+ICsJCXdyaXRlbChyZWdfdmFs LCBkc2ktPnJlZ3MgKyBEU0lfQ01EUTApOwo+ID4gPiArCQltdGtfZHNpX21hc2soZHNpLCBEU0lf Q01EUV9TSVpFLCBDTURRX1NJWkUsIDEpOwo+ID4gPiArCX0KPiA+ID4gK30KPiA+ID4gKwoKW3Nu aXAuLi5dCgo+ID4gCj4gPiBSZWdhcmRzLAo+ID4gQ0sKPiA+IAo+IAo+IAoKUmVnYXJkcywKQ0sK Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZl bCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xp c3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: ck.hu@mediatek.com (CK Hu) Date: Thu, 11 Aug 2016 15:10:12 +0800 Subject: [PATCH v6 07/10] drm/mediatek: add dsi transfer function In-Reply-To: <1470813866.27526.14.camel@mtksdaap41> References: <1470308844-20895-1-git-send-email-yt.shen@mediatek.com> <1470308844-20895-8-git-send-email-yt.shen@mediatek.com> <1470391697.16554.60.camel@mtksdaap41> <1470813866.27526.14.camel@mtksdaap41> Message-ID: <1470899412.16554.72.camel@mtksdaap41> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, YT: On Wed, 2016-08-10 at 15:24 +0800, YT Shen wrote: > Hi CK, > > On Fri, 2016-08-05 at 18:08 +0800, CK Hu wrote: > > Hi, YT: > > > > On Thu, 2016-08-04 at 19:07 +0800, YT Shen wrote: > > > From: shaoming chen > > > > > > add dsi read/write commands for transfer function > > > > > > Signed-off-by: shaoming chen > > > --- > > > drivers/gpu/drm/mediatek/mtk_dsi.c | 261 ++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 261 insertions(+) > > > [snip...] > > > + > > > +static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg) > > > +{ > > > + const char *tx_buf = msg->tx_buf; > > > + u32 reg_val, i; > > > + u16 wc16; > > > + u8 config, data0, data1, type; > > > + > > > + if (MTK_DSI_HOST_IS_READ(type)) { > > > > 'type' is used before assigned. > Will fix. > > > > > > + config = 4; > > > + data0 = tx_buf[0]; > > > + > > > + if (msg->rx_len < 3) > > > + type = MIPI_DSI_DCS_READ; > > > + else > > > + type = MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM; > > > + > > > + data1 = 0; > > > + reg_val = (data1 << 24) | (data0 << 16) | (type << 8) | config; > > > + > > > + writel(reg_val, dsi->regs + DSI_CMDQ0); > > > + mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, 1); > > > > I think this part looks like 'else' part. The difference is type and > > config. I think type should be msg->type and you can independently set > > BIT(2) of config. > msg->type only tells about read or write, for the details we need to > check other parameters. This part is for read, the else part is for > write short packet. Not only BIT(2) of config is different, but also > type and data1 is changed. Such a change would lead to difficult to > understand. > > > > > > + } else if (msg->tx_len > 2) { /* send long packet */ > > > + config = 2; > > > + type = msg->type; > > > + wc16 = msg->tx_len; > > > + > > > + reg_val = (wc16 << 16) | (type << 8) | config; > > > + > > > + writel(reg_val, dsi->regs + DSI_CMDQ0); > > > + > > > + for (i = 0; i < msg->tx_len; i++) > > > + writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + 4 + i); > > > + > > > + mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, > > > + 1 + (msg->tx_len + 3) / 4); > > > + } else { /* send short packet */ > > > + config = 0; > > > + data0 = tx_buf[0]; > > > + > > > + if (msg->tx_len == 2) { > > > + type = MIPI_DSI_DCS_SHORT_WRITE_PARAM; > > > > Why do you not set type as msg->type? This behavior looks like you > > modify transfer type, but is this acceptable? > msg->type only tells about read or write, for the details we need to > check other parameters. I think you could rewrite mtk_dsi_cmdq() as follow: static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg) { u32 i; u32 src_off = (msg->tx_len > 2) ? 4 : 2; u32 cmdq_size = (msg->tx_len > 2) ? 1 + (msg->tx_len + 3) / 4 : 1; u32 config = (msg->tx_len > 2) ? BIT(1) : 0; u32 type = msg->type; if (MTK_DSI_HOST_IS_READ(type)) config |= BIT(2); writel((type << 8) || config, dsi->regs + DSI_CMDQ0); for (i = 0; i < msg->tx_len; i++) writeb(msg->tx_buf[i], dsi->regs + DSI_CMDQ0 + src_off + i); mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size); } If DSI HW does not support some type and it can be transferred to other type, you could add a transfer function like this static u8 mtk_dsi_map_to_supported_type(u8 type) { switch(type) { case MIPI_DSI_DCS_SHORT_WRITE: return MIPI_DSI_DCS_SHORT_WRITE_PARAM; } return type; } and then u32 type = mtk_dsi_map_to_supported_type(msg->type); > > > > > > + data1 = tx_buf[1]; > > > + } else { > > > + type = MIPI_DSI_DCS_SHORT_WRITE; > > > + data1 = 0; > > > + } > > > + > > > + reg_val = (data1 << 24) | (data0 << 16) | (type << 8) | config; > > > + > > > + writel(reg_val, dsi->regs + DSI_CMDQ0); > > > + mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, 1); > > > + } > > > +} > > > + [snip...] > > > > Regards, > > CK > > > > 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 S932497AbcHKHKV (ORCPT ); Thu, 11 Aug 2016 03:10:21 -0400 Received: from mailgw01.mediatek.com ([210.61.82.183]:40646 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S932263AbcHKHKT (ORCPT ); Thu, 11 Aug 2016 03:10:19 -0400 Message-ID: <1470899412.16554.72.camel@mtksdaap41> Subject: Re: [PATCH v6 07/10] drm/mediatek: add dsi transfer function From: CK Hu To: YT Shen CC: , Philipp Zabel , Rob Herring , Pawel Moll , "Mark Rutland" , Ian Campbell , Kumar Gala , "Russell King" , David Airlie , "Matthias Brugger" , Mao Huang , "Bibby Hsieh" , , , , , , "Sascha Hauer" , , , , shaoming chen Date: Thu, 11 Aug 2016 15:10:12 +0800 In-Reply-To: <1470813866.27526.14.camel@mtksdaap41> References: <1470308844-20895-1-git-send-email-yt.shen@mediatek.com> <1470308844-20895-8-git-send-email-yt.shen@mediatek.com> <1470391697.16554.60.camel@mtksdaap41> <1470813866.27526.14.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, YT: On Wed, 2016-08-10 at 15:24 +0800, YT Shen wrote: > Hi CK, > > On Fri, 2016-08-05 at 18:08 +0800, CK Hu wrote: > > Hi, YT: > > > > On Thu, 2016-08-04 at 19:07 +0800, YT Shen wrote: > > > From: shaoming chen > > > > > > add dsi read/write commands for transfer function > > > > > > Signed-off-by: shaoming chen > > > --- > > > drivers/gpu/drm/mediatek/mtk_dsi.c | 261 ++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 261 insertions(+) > > > [snip...] > > > + > > > +static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg) > > > +{ > > > + const char *tx_buf = msg->tx_buf; > > > + u32 reg_val, i; > > > + u16 wc16; > > > + u8 config, data0, data1, type; > > > + > > > + if (MTK_DSI_HOST_IS_READ(type)) { > > > > 'type' is used before assigned. > Will fix. > > > > > > + config = 4; > > > + data0 = tx_buf[0]; > > > + > > > + if (msg->rx_len < 3) > > > + type = MIPI_DSI_DCS_READ; > > > + else > > > + type = MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM; > > > + > > > + data1 = 0; > > > + reg_val = (data1 << 24) | (data0 << 16) | (type << 8) | config; > > > + > > > + writel(reg_val, dsi->regs + DSI_CMDQ0); > > > + mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, 1); > > > > I think this part looks like 'else' part. The difference is type and > > config. I think type should be msg->type and you can independently set > > BIT(2) of config. > msg->type only tells about read or write, for the details we need to > check other parameters. This part is for read, the else part is for > write short packet. Not only BIT(2) of config is different, but also > type and data1 is changed. Such a change would lead to difficult to > understand. > > > > > > + } else if (msg->tx_len > 2) { /* send long packet */ > > > + config = 2; > > > + type = msg->type; > > > + wc16 = msg->tx_len; > > > + > > > + reg_val = (wc16 << 16) | (type << 8) | config; > > > + > > > + writel(reg_val, dsi->regs + DSI_CMDQ0); > > > + > > > + for (i = 0; i < msg->tx_len; i++) > > > + writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + 4 + i); > > > + > > > + mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, > > > + 1 + (msg->tx_len + 3) / 4); > > > + } else { /* send short packet */ > > > + config = 0; > > > + data0 = tx_buf[0]; > > > + > > > + if (msg->tx_len == 2) { > > > + type = MIPI_DSI_DCS_SHORT_WRITE_PARAM; > > > > Why do you not set type as msg->type? This behavior looks like you > > modify transfer type, but is this acceptable? > msg->type only tells about read or write, for the details we need to > check other parameters. I think you could rewrite mtk_dsi_cmdq() as follow: static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg) { u32 i; u32 src_off = (msg->tx_len > 2) ? 4 : 2; u32 cmdq_size = (msg->tx_len > 2) ? 1 + (msg->tx_len + 3) / 4 : 1; u32 config = (msg->tx_len > 2) ? BIT(1) : 0; u32 type = msg->type; if (MTK_DSI_HOST_IS_READ(type)) config |= BIT(2); writel((type << 8) || config, dsi->regs + DSI_CMDQ0); for (i = 0; i < msg->tx_len; i++) writeb(msg->tx_buf[i], dsi->regs + DSI_CMDQ0 + src_off + i); mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size); } If DSI HW does not support some type and it can be transferred to other type, you could add a transfer function like this static u8 mtk_dsi_map_to_supported_type(u8 type) { switch(type) { case MIPI_DSI_DCS_SHORT_WRITE: return MIPI_DSI_DCS_SHORT_WRITE_PARAM; } return type; } and then u32 type = mtk_dsi_map_to_supported_type(msg->type); > > > > > > + data1 = tx_buf[1]; > > > + } else { > > > + type = MIPI_DSI_DCS_SHORT_WRITE; > > > + data1 = 0; > > > + } > > > + > > > + reg_val = (data1 << 24) | (data0 << 16) | (type << 8) | config; > > > + > > > + writel(reg_val, dsi->regs + DSI_CMDQ0); > > > + mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, 1); > > > + } > > > +} > > > + [snip...] > > > > Regards, > > CK > > > > Regards, CK