On 2014? 08? 18? 20:38, Andrzej Hajda wrote: > 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 <inki.dae at samsung.com> >> --- >> 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.
Thanks for comments and CCing Thierry. I think it'd better to handle this solution flexibly. There was a concern that my solution makes all commands to be transmitted in low power or high speed by default. So if we have other solution which passes a flag per a message, then we could transmit each command in low power or high speed, which could also be used selectively with my solution - if msg->flags has MIPI_DSI_MODE_LPM, then each flag of the new API is ignored. Thierry, which one do you prefer? Otherwise, do you have other opinions? Thanks, Inki Dae > > 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, > >