From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Subject: Re: [PATCH 1/2] drm/mipi-dsi: consider low power transmission Date: Mon, 18 Aug 2014 13:38:11 +0200 Message-ID: <53F1E5A3.6000804@samsung.com> References: <1408349495-25568-1-git-send-email-inki.dae@samsung.com> <1408349495-25568-2-git-send-email-inki.dae@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by gabe.freedesktop.org (Postfix) with ESMTP id 467356E3D5 for ; Mon, 18 Aug 2014 04:38:21 -0700 (PDT) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NAI00GI02ZAPD90@mailout2.w1.samsung.com> for dri-devel@lists.freedesktop.org; Mon, 18 Aug 2014 12:37:58 +0100 (BST) In-reply-to: <1408349495-25568-2-git-send-email-inki.dae@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Inki Dae , dri-devel@lists.freedesktop.org Cc: Thierry Reding List-Id: dri-devel@lists.freedesktop.org On 08/18/2014 10:11 AM, Inki Dae wrote: > This patch adds a new flag, MIPI_DSI-MODE_LPM, to transmit data > in low power. With this flag, msg.flags has MIPI_DSI_MSG_USE_LPM > so that host driver of each SoC can clear or set relevant register > bit for low power transmission. > > All host drivers shall support continuous clock behavior on the > Clock Lane, and optionally may support non-continuous clock behavior. > Both of them can transmit data in high speed of low power. > > With each clock behavior, non-continuous or continuous clock mode, > host controller will transmit data in high speed by default so if > peripheral wants to receive data in low power, the peripheral driver > should set MIPI_DSI_MODE_LPM flag. I think it would be better to remove last two paragraphs as irrelevant, LPM and clock behavior are orthogonal. > > Signed-off-by: Inki Dae > --- > drivers/gpu/drm/drm_mipi_dsi.c | 6 ++++++ > include/drm/drm_mipi_dsi.h | 2 ++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 6aa6a9e..eb6dfe5 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -231,6 +231,9 @@ ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, const void *data, > break; > } > > + if (dsi->mode_flags & MIPI_DSI_MODE_LPM) > + msg.flags = MIPI_DSI_MSG_USE_LPM; > + > return ops->transfer(dsi->host, &msg); > } > EXPORT_SYMBOL(mipi_dsi_dcs_write); > @@ -260,6 +263,9 @@ ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data, > if (!ops || !ops->transfer) > return -ENOSYS; > > + if (dsi->mode_flags & MIPI_DSI_MODE_LPM) > + msg.flags = MIPI_DSI_MSG_USE_LPM; > + I see three other ways of adding LPM to DCS: 1. Add flags argument to DCS command, eg: mipi_dsi_dcs_write(dsi, data, len, flags) 2. Pass struct mipi_dsi_device to transfer callback: ssize_t (*transfer)(struct mipi_dsi_device *dsi, struct mipi_dsi_msg *msg); or ssize_t (*transfer)(struct mipi_dsi_host *host, struct mipi_dsi_device *dsi, struct mipi_dsi_msg *msg); This way DSI host will have access to mipi_dsi_device and to its flags. 3. Create new API function, lets call it dsi_transfer, which should by called by DCS helpers instead of transfer op. This function shall translate device flags to message flags and call the original op. I think the 3rd solution is the best one, but I have no strong feelings against the other ones, including your. As I remember Thierry have also some ideas about changes in DSI API so I have added him to CC. Regards Andrzej > return ops->transfer(dsi->host, &msg); > } > EXPORT_SYMBOL(mipi_dsi_dcs_read); > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index 2bb55b8..8569dc5 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -96,6 +96,8 @@ void mipi_dsi_host_unregister(struct mipi_dsi_host *host); > #define MIPI_DSI_MODE_EOT_PACKET BIT(9) > /* device supports non-continuous clock behavior (DSI spec 5.6.1) */ > #define MIPI_DSI_CLOCK_NON_CONTINUOUS BIT(10) > +/* transmit data in low power */ > +#define MIPI_DSI_MODE_LPM BIT(11) > > enum mipi_dsi_pixel_format { > MIPI_DSI_FMT_RGB888,