On 2024-03-30 05:59:29, Dmitry Baryshkov wrote: > The LG SW43408 panel requires sending non-standard data as a part of the > MIPI_DSI_COMPRESSION_MODE packet. Rather than hacking existing > mipi_dsi_compression_mode() add mipi_dsi_compression_mode_raw(), which > accepts raw data buffer and length. Even though I doubt the usefulness of this _raw() command before further understanding the panel and driver (according the the review-followup sent a few minutes ago), let me review this a little bit. > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > --- > drivers/gpu/drm/drm_mipi_dsi.c | 34 ++++++++++++++++++++++++++-------- > include/drm/drm_mipi_dsi.h | 1 + > 2 files changed, 27 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index ef6e416522f8..f340d1e0a9a5 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -645,29 +645,47 @@ int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi, > EXPORT_SYMBOL(mipi_dsi_set_maximum_return_packet_size); > > /** > - * mipi_dsi_compression_mode() - enable/disable DSC on the peripheral > + * mipi_dsi_compression_mode_raw() - control DSC on the peripheral > * @dsi: DSI peripheral device > - * @enable: Whether to enable or disable the DSC > + * @data: data to be sent to the device > + * @len: size of the data buffer > * > - * Enable or disable Display Stream Compression on the peripheral using the > + * Control the Display Stream Compression on the peripheral using the + mode? > * default Picture Parameter Set and VESA DSC 1.1 algorithm. This is no longer true. Both the algoritm identifier and "default Picture Parameter Set" (which I assume means table *index*!) are described by the custom/raw bytes that one is allowed to pass. In fact, in the SW43408 driver that you reference in the commit message the custom data passed to the _raw() function is used to select the second PPS table (unless the panel interprets the input data in a non-standard way...), and further sets the PPS for the first table only :) > * > * Return: 0 on success or a negative error code on failure. > */ > -ssize_t mipi_dsi_compression_mode(struct mipi_dsi_device *dsi, bool enable) > +ssize_t mipi_dsi_compression_mode_raw(struct mipi_dsi_device *dsi, void *data, size_t len) > { > - /* Note: Needs updating for non-default PPS or algorithm */ > - u8 tx[2] = { enable << 0, 0 }; > struct mipi_dsi_msg msg = { > .channel = dsi->channel, > .type = MIPI_DSI_COMPRESSION_MODE, > - .tx_len = sizeof(tx), > - .tx_buf = tx, > + .tx_len = len, > + .tx_buf = data, > }; > int ret = mipi_dsi_device_transfer(dsi, &msg); > > return (ret < 0) ? ret : 0; > } > +EXPORT_SYMBOL(mipi_dsi_compression_mode_raw); > + > +/** > + * mipi_dsi_compression_mode() - enable/disable DSC on the peripheral > + * @dsi: DSI peripheral device > + * @enable: Whether to enable or disable the DSC > + * > + * Enable or disable Display Stream Compression on the peripheral using the > + * default Picture Parameter Set and VESA DSC 1.1 algorithm. And while fixing this up, let's make it clear that this doesn't change the PPS, just the *index* of which PPS to use (the PPS is updated with a different command). - Marijn > + * > + * Return: 0 on success or a negative error code on failure. > + */ > +ssize_t mipi_dsi_compression_mode(struct mipi_dsi_device *dsi, bool enable) > +{ > + /* Note: Needs updating for non-default PPS or algorithm */ > + u8 tx[2] = { enable << 0, 0 }; > + > + return mipi_dsi_compression_mode_raw(dsi, tx, sizeof(tx)); > +} > EXPORT_SYMBOL(mipi_dsi_compression_mode); > > /** > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h > index c0aec0d4d664..321d2b019687 100644 > --- a/include/drm/drm_mipi_dsi.h > +++ b/include/drm/drm_mipi_dsi.h > @@ -242,6 +242,7 @@ int mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi); > int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi, > u16 value); > ssize_t mipi_dsi_compression_mode(struct mipi_dsi_device *dsi, bool enable); > +ssize_t mipi_dsi_compression_mode_raw(struct mipi_dsi_device *dsi, void *data, size_t len); > ssize_t mipi_dsi_picture_parameter_set(struct mipi_dsi_device *dsi, > const struct drm_dsc_picture_parameter_set *pps); > > > -- > 2.39.2 >