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
>
