| Message ID | 1446251908-2603-1-git-send-email-bjorn@kryo.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 30, 2015 at 5:38 PM, <bjorn@kryo.se> wrote: Ping? > From: Werner Johansson <werner.johansson@sonymobile.com> > > The MIPI_DSI_TURN_ON_PERIPHERAL and MIPI_DSI_SHUTDOWN_PERIPHERAL > packets are required for some panels, one example being the > Panasonic VVX10F034N00 panel. > > Signed-off-by: Werner Johansson <werner.johansson@sonymobile.com> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > --- > drivers/gpu/drm/drm_mipi_dsi.c | 47 ++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_mipi_dsi.h | 3 +++ > 2 files changed, 50 insertions(+) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 2d5ca8ee..13b4a9c 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -862,6 +862,53 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) > } > EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); > > +/** > + * mipi_dsi_raw_short_write() - Sends a data-less short DSI packet > + * @dsi: DSI peripheral device > + * @type: Data Type of packet to send > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +static ssize_t mipi_dsi_raw_short_write(struct mipi_dsi_device *dsi, u8 type) > +{ > + u8 dummy[2] = { 0, 0 }; > + struct mipi_dsi_msg msg = { > + .channel = dsi->channel, > + .tx_buf = dummy, > + .tx_len = sizeof(dummy), > + .type = type > + }; > + > + if (mipi_dsi_packet_format_is_short(type)) > + return mipi_dsi_device_transfer(dsi, &msg); > + else > + return -1; > +} > + > +/** > + * mipi_dsi_turn_on_peripheral() - Sends Turn On Peripheral DSI command > + * @dsi: DSI peripheral device > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +ssize_t mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi) > +{ > + return mipi_dsi_raw_short_write(dsi, MIPI_DSI_TURN_ON_PERIPHERAL); > +} > +EXPORT_SYMBOL(mipi_dsi_turn_on_peripheral); > + > +/** > + * mipi_dsi_shutdown_peripheral() - Sends Shutdown Peripheral DSI command > + * @dsi: DSI peripheral device > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +ssize_t mipi_dsi_shutdown_peripheral(struct mipi_dsi_device *dsi) > +{ > + return mipi_dsi_raw_short_write(dsi, MIPI_DSI_SHUTDOWN_PERIPHERAL); > +} > +EXPORT_SYMBOL(mipi_dsi_shutdown_peripheral); > + > static int mipi_dsi_drv_probe(struct device *dev) > { > struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index f1d8d0d..2e0f057 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -215,6 +215,9 @@ int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, > enum mipi_dsi_dcs_tear_mode mode); > int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format); > > +ssize_t mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi); > +ssize_t mipi_dsi_shutdown_peripheral(struct mipi_dsi_device *dsi); > + > /** > * struct mipi_dsi_driver - DSI driver > * @driver: device driver model driver > -- > 2.3.2 (Apple Git-55) >
On Fri, Oct 30, 2015 at 05:38:26PM -0700, bjorn@kryo.se wrote: > From: Werner Johansson <werner.johansson@sonymobile.com> > > The MIPI_DSI_TURN_ON_PERIPHERAL and MIPI_DSI_SHUTDOWN_PERIPHERAL > packets are required for some panels, one example being the > Panasonic VVX10F034N00 panel. > > Signed-off-by: Werner Johansson <werner.johansson@sonymobile.com> > Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> > --- > drivers/gpu/drm/drm_mipi_dsi.c | 47 ++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_mipi_dsi.h | 3 +++ > 2 files changed, 50 insertions(+) This being a dependency on 3/3 it would've been good to send this To: me as well. I hadn't seen this in my inbox and had simply discarded it as being independent. Of course if I had properly checked the build I would've noticed... Anyway, I've applied this with slight modifications. I removed the raw short write helper. I don't think it buys us much and we already have an open-coded variant in mipi_dsi_set_maximum_return_packet_size(). I've also moved these functions closer to the Set Maximum Return Packet Size command implementation because they are part of the same command set. I also made the functions return int instead of ssize_t to avoid potential confusion about their return value (ssize_t implies "number of bytes transferred, or a negative error code on failure). One minor comment below... > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 2d5ca8ee..13b4a9c 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -862,6 +862,53 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) > } > EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); > > +/** > + * mipi_dsi_raw_short_write() - Sends a data-less short DSI packet > + * @dsi: DSI peripheral device > + * @type: Data Type of packet to send > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +static ssize_t mipi_dsi_raw_short_write(struct mipi_dsi_device *dsi, u8 type) > +{ > + u8 dummy[2] = { 0, 0 }; > + struct mipi_dsi_msg msg = { > + .channel = dsi->channel, > + .tx_buf = dummy, > + .tx_len = sizeof(dummy), > + .type = type > + }; > + > + if (mipi_dsi_packet_format_is_short(type)) > + return mipi_dsi_device_transfer(dsi, &msg); > + else > + return -1; > +} When the kerneldoc comment says "negative error code", developers will expect one of the -E* constants. -1 maps to -EPERM (operation not permitted), which isn't a good error code for this case. This is also an internal function, so these errors really should never happen. Anyway, since I removed this helper this isn't an issue anymore, just wanted to mention it. Thanks, Thierry
On Tue, Nov 24, 2015 at 1:33 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Fri, Oct 30, 2015 at 05:38:26PM -0700, bjorn@kryo.se wrote: >> From: Werner Johansson <werner.johansson@sonymobile.com> >> >> The MIPI_DSI_TURN_ON_PERIPHERAL and MIPI_DSI_SHUTDOWN_PERIPHERAL >> packets are required for some panels, one example being the >> Panasonic VVX10F034N00 panel. >> >> Signed-off-by: Werner Johansson <werner.johansson@sonymobile.com> >> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com> >> --- >> drivers/gpu/drm/drm_mipi_dsi.c | 47 ++++++++++++++++++++++++++++++++++++++++++ >> include/drm/drm_mipi_dsi.h | 3 +++ >> 2 files changed, 50 insertions(+) > > This being a dependency on 3/3 it would've been good to send this To: me > as well. I hadn't seen this in my inbox and had simply discarded it as > being independent. Of course if I had properly checked the build I > would've noticed... > Sorry for not including you as a recipient, I relied too heavily on get_maintainer. Thanks for fixing my misstake. > Anyway, I've applied this with slight modifications. I removed the raw > short write helper. I don't think it buys us much and we already have an > open-coded variant in mipi_dsi_set_maximum_return_packet_size(). I've > also moved these functions closer to the Set Maximum Return Packet Size > command implementation because they are part of the same command set. I > also made the functions return int instead of ssize_t to avoid potential > confusion about their return value (ssize_t implies "number of bytes > transferred, or a negative error code on failure). > Sounds good, thanks. > One minor comment below... > >> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c >> index 2d5ca8ee..13b4a9c 100644 >> --- a/drivers/gpu/drm/drm_mipi_dsi.c >> +++ b/drivers/gpu/drm/drm_mipi_dsi.c >> @@ -862,6 +862,53 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) >> } >> EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); >> >> +/** >> + * mipi_dsi_raw_short_write() - Sends a data-less short DSI packet >> + * @dsi: DSI peripheral device >> + * @type: Data Type of packet to send >> + * >> + * Return: 0 on success or a negative error code on failure. >> + */ >> +static ssize_t mipi_dsi_raw_short_write(struct mipi_dsi_device *dsi, u8 type) >> +{ >> + u8 dummy[2] = { 0, 0 }; >> + struct mipi_dsi_msg msg = { >> + .channel = dsi->channel, >> + .tx_buf = dummy, >> + .tx_len = sizeof(dummy), >> + .type = type >> + }; >> + >> + if (mipi_dsi_packet_format_is_short(type)) >> + return mipi_dsi_device_transfer(dsi, &msg); >> + else >> + return -1; >> +} > > When the kerneldoc comment says "negative error code", developers will > expect one of the -E* constants. -1 maps to -EPERM (operation not > permitted), which isn't a good error code for this case. This is also an > internal function, so these errors really should never happen. > > Anyway, since I removed this helper this isn't an issue anymore, just > wanted to mention it. > Right. Thanks for fixing it! Regards, Bjorn
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index 2d5ca8ee..13b4a9c 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -862,6 +862,53 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) } EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); +/** + * mipi_dsi_raw_short_write() - Sends a data-less short DSI packet + * @dsi: DSI peripheral device + * @type: Data Type of packet to send + * + * Return: 0 on success or a negative error code on failure. + */ +static ssize_t mipi_dsi_raw_short_write(struct mipi_dsi_device *dsi, u8 type) +{ + u8 dummy[2] = { 0, 0 }; + struct mipi_dsi_msg msg = { + .channel = dsi->channel, + .tx_buf = dummy, + .tx_len = sizeof(dummy), + .type = type + }; + + if (mipi_dsi_packet_format_is_short(type)) + return mipi_dsi_device_transfer(dsi, &msg); + else + return -1; +} + +/** + * mipi_dsi_turn_on_peripheral() - Sends Turn On Peripheral DSI command + * @dsi: DSI peripheral device + * + * Return: 0 on success or a negative error code on failure. + */ +ssize_t mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi) +{ + return mipi_dsi_raw_short_write(dsi, MIPI_DSI_TURN_ON_PERIPHERAL); +} +EXPORT_SYMBOL(mipi_dsi_turn_on_peripheral); + +/** + * mipi_dsi_shutdown_peripheral() - Sends Shutdown Peripheral DSI command + * @dsi: DSI peripheral device + * + * Return: 0 on success or a negative error code on failure. + */ +ssize_t mipi_dsi_shutdown_peripheral(struct mipi_dsi_device *dsi) +{ + return mipi_dsi_raw_short_write(dsi, MIPI_DSI_SHUTDOWN_PERIPHERAL); +} +EXPORT_SYMBOL(mipi_dsi_shutdown_peripheral); + static int mipi_dsi_drv_probe(struct device *dev) { struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index f1d8d0d..2e0f057 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -215,6 +215,9 @@ int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, enum mipi_dsi_dcs_tear_mode mode); int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format); +ssize_t mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi); +ssize_t mipi_dsi_shutdown_peripheral(struct mipi_dsi_device *dsi); + /** * struct mipi_dsi_driver - DSI driver * @driver: device driver model driver