From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Subject: Re: [PATCH 3/4] drm/dsi: Make mipi_dsi_dcs_{read, write}() symmetrical Date: Tue, 22 Jul 2014 09:32:58 +0200 Message-ID: <53CE13AA.5040408@samsung.com> References: <1406013141-18552-1-git-send-email-thierry.reding@gmail.com> <1406013141-18552-3-git-send-email-thierry.reding@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by gabe.freedesktop.org (Postfix) with ESMTP id A6A396E237 for ; Tue, 22 Jul 2014 00:33:26 -0700 (PDT) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0N93005IXRNISG20@mailout1.w1.samsung.com> for dri-devel@lists.freedesktop.org; Tue, 22 Jul 2014 08:33:18 +0100 (BST) In-reply-to: <1406013141-18552-3-git-send-email-thierry.reding@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thierry Reding Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 07/22/2014 09:12 AM, Thierry Reding wrote: > From: Thierry Reding > > Currently the mipi_dsi_dcs_write() function requires the DCS command > byte to be embedded within the write buffer whereas mipi_dsi_dcs_read() > has a separate parameter. Make them more symmetrical by adding an extra > command parameter to mipi_dsi_dcs_write(). > > Also update the S6E8AA0 panel driver (the only user of this API) to > adapt to this new API. I do not agree with that. DCS read and write are not symmetrical by design: - write just sends data, - read sends data then reads response. Forcing API to be symmetrical complicates the implementation without real gain. Regards Andrzej > > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/drm_mipi_dsi.c | 45 ++++++++++++++++++++++++----------- > drivers/gpu/drm/panel/panel-s6e8aa0.c | 4 ++-- > include/drm/drm_mipi_dsi.h | 4 ++-- > 3 files changed, 35 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 6aa6a9e95570..bbab06ef80c9 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -201,29 +201,41 @@ EXPORT_SYMBOL(mipi_dsi_detach); > /** > * mipi_dsi_dcs_write - send DCS write command > * @dsi: DSI device > - * @data: pointer to the command followed by parameters > - * @len: length of @data > + * @cmd: DCS command > + * @data: pointer to the command payload > + * @len: payload length > */ > -ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, const void *data, > - size_t len) > +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd, > + const void *data, size_t len) > { > const struct mipi_dsi_host_ops *ops = dsi->host->ops; > - struct mipi_dsi_msg msg = { > - .channel = dsi->channel, > - .tx_buf = data, > - .tx_len = len > - }; > + struct mipi_dsi_msg msg; > + ssize_t err; > + u8 *tx; > > if (!ops || !ops->transfer) > return -ENOSYS; > > + if (len > 0) { > + tx = kmalloc(1 + len, GFP_KERNEL); > + if (!tx) > + return -ENOMEM; > + > + tx[0] = cmd; > + memcpy(tx + 1, data, len); > + } else { > + tx = &cmd; > + } > + > + msg.channel = dsi->channel; > + msg.tx_len = 1 + len; > + msg.tx_buf = tx; > + > switch (len) { > case 0: > - return -EINVAL; > - case 1: > msg.type = MIPI_DSI_DCS_SHORT_WRITE; > break; > - case 2: > + case 1: > msg.type = MIPI_DSI_DCS_SHORT_WRITE_PARAM; > break; > default: > @@ -231,14 +243,19 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, const void *data, > break; > } > > - return ops->transfer(dsi->host, &msg); > + err = ops->transfer(dsi->host, &msg); > + > + if (len > 0) > + kfree(tx); > + > + return err; > } > EXPORT_SYMBOL(mipi_dsi_dcs_write); > > /** > * mipi_dsi_dcs_read - send DCS read request command > * @dsi: DSI device > - * @cmd: DCS read command > + * @cmd: DCS command > * @data: pointer to read buffer > * @len: length of @data > * > diff --git a/drivers/gpu/drm/panel/panel-s6e8aa0.c b/drivers/gpu/drm/panel/panel-s6e8aa0.c > index 5502ef6bc074..d39bee36816c 100644 > --- a/drivers/gpu/drm/panel/panel-s6e8aa0.c > +++ b/drivers/gpu/drm/panel/panel-s6e8aa0.c > @@ -130,7 +130,7 @@ static int s6e8aa0_clear_error(struct s6e8aa0 *ctx) > return ret; > } > > -static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const void *data, size_t len) > +static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const u8 *data, size_t len) > { > struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev); > ssize_t ret; > @@ -138,7 +138,7 @@ static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const void *data, size_t len) > if (ctx->error < 0) > return; > > - ret = mipi_dsi_dcs_write(dsi, data, len); > + ret = mipi_dsi_dcs_write(dsi, data[0], data + 1, len - 1); > if (ret < 0) { > dev_err(ctx->dev, "error %zd writing dcs seq: %*ph\n", ret, len, > data); > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index 7b5e1a9244e1..bea181121e8b 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -127,8 +127,8 @@ struct mipi_dsi_device { > > int mipi_dsi_attach(struct mipi_dsi_device *dsi); > int mipi_dsi_detach(struct mipi_dsi_device *dsi); > -ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, const void *data, > - size_t len); > +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd, > + const void *data, size_t len); > ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data, > size_t len); >