| Message ID | 1408349495-25568-2-git-send-email-inki.dae@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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. 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,
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@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, > >
On Mon, Aug 18, 2014 at 10:11 AM Inki Dae <inki.dae@samsung.com> 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. > > Signed-off-by: Inki Dae <inki.dae@samsung.com> Hi Inki Dae, I recently used this flag in a driver to the effect that all writes become LPM, including updates to a command mode-only panel, so the pixels will draw very slowly on the screen. In this case I added the feature for debugging (my display works fine in LP but not yet in HS mode). This is not a problem with a video mode panel since these are by the specification required to operate in HS mode, so we know they are always HS. But in command mode, all commands are equal, even if they are screen updates. I program my hardware to update the display with a stream of commands, and with this flag set, that stream will go slow. Is this how it is intended? Using the flag like this is kind of interesting because it is good for debugging but I don't know if this is how it was intended. If the flag is only supposed to be for DCS write commands, so that both write and read happen in LP mode, then we should probably rename it accordingly I think? Yours, Linus Walleij
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; + 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,
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. Signed-off-by: Inki Dae <inki.dae@samsung.com> --- drivers/gpu/drm/drm_mipi_dsi.c | 6 ++++++ include/drm/drm_mipi_dsi.h | 2 ++ 2 files changed, 8 insertions(+)