On Wed, Sep 24, 2025 at 11:08:10PM +0800, Jun Nie wrote:
> Some panels support multiple slice to be sent in a single DSC packet. And

s/support/require/

If the panel supports something, then we can omit that and send 1 slice
as we currently do. If the panel requires multiple slices, it's
mandatory.

> this feature is a must for specific panels, such as JDI LPM026M648C. Add a

A panel driver which executes this API is appreciated. Otherwise in a
few months / years somebody will submit a patch 'field foo is not used
by the kernel drivers, drop it'.

> dsc_slice_per_pkt member into struct mipi_dsi_device and support the
> feature in msm mdss driver.
> 
> Co-developed-by: Jonathan Marek <[email protected]>
> Signed-off-by: Jonathan Marek <[email protected]>
> Signed-off-by: Jun Nie <[email protected]>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 25 ++++++++++---------------
>  include/drm/drm_mipi_dsi.h         |  2 ++
>  2 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 
> f297e3de8c3dc4e1326e70c78c046b5a19568cef..de51cb02f267205320c5a94fc4188413e05907e6
>  100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -157,6 +157,7 @@ struct msm_dsi_host {
>  
>       struct drm_display_mode *mode;
>       struct drm_dsc_config *dsc;
> +     unsigned int dsc_slice_per_pkt;
>  
>       /* connected device info */
>       unsigned int channel;
> @@ -849,17 +850,10 @@ static void dsi_update_dsc_timing(struct msm_dsi_host 
> *msm_host, bool is_cmd_mod
>       slice_per_intf = dsc->slice_count;
>  
>       total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
> -     bytes_per_pkt = dsc->slice_chunk_size; /* * slice_per_pkt; */
> +     bytes_per_pkt = dsc->slice_chunk_size * msm_host->dsc_slice_per_pkt;
>  
>       eol_byte_num = total_bytes_per_intf % 3;
> -
> -     /*
> -      * Typically, pkt_per_line = slice_per_intf * slice_per_pkt.
> -      *
> -      * Since the current driver only supports slice_per_pkt = 1,
> -      * pkt_per_line will be equal to slice per intf for now.
> -      */
> -     pkt_per_line = slice_per_intf;
> +     pkt_per_line = slice_per_intf / msm_host->dsc_slice_per_pkt;
>  
>       if (is_cmd_mode) /* packet data type */
>               reg = 
> DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
> @@ -1008,12 +1002,8 @@ static void dsi_timing_setup(struct msm_dsi_host 
> *msm_host, bool is_bonded_dsi)
>               else
>                       /*
>                        * When DSC is enabled, WC = slice_chunk_size * 
> slice_per_pkt + 1.
> -                      * Currently, the driver only supports default value of 
> slice_per_pkt = 1
> -                      *
> -                      * TODO: Expand mipi_dsi_device struct to hold 
> slice_per_pkt info
> -                      *       and adjust DSC math to account for 
> slice_per_pkt.
>                        */
> -                     wc = msm_host->dsc->slice_chunk_size + 1;
> +                     wc = msm_host->dsc->slice_chunk_size * 
> msm_host->dsc_slice_per_pkt + 1;
>  
>               dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
>                       DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
> @@ -1623,8 +1613,13 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
>       msm_host->lanes = dsi->lanes;
>       msm_host->format = dsi->format;
>       msm_host->mode_flags = dsi->mode_flags;
> -     if (dsi->dsc)
> +     if (dsi->dsc) {
>               msm_host->dsc = dsi->dsc;
> +             msm_host->dsc_slice_per_pkt = dsi->dsc_slice_per_pkt;
> +             /* for backwards compatibility, assume 1 if not set */
> +             if (!msm_host->dsc_slice_per_pkt)
> +                     msm_host->dsc_slice_per_pkt = 1;

msm_host->dsc_slice_per_pkt = dsi->dsc_slice_per_pkt ?: 1;

Yes, I think it's more ideomatic.

> +     }
>  
>       ret = dsi_dev_attach(msm_host->pdev);
>       if (ret)
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 
> 6d2c08e8110151a97620389197f1ef79c058329d..5a85ba01f402a3866b70828391bb417bb8dd5956
>  100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -182,6 +182,7 @@ struct mipi_dsi_device_info {
>   * be set to the real limits of the hardware, zero is only accepted for
>   * legacy drivers
>   * @dsc: panel/bridge DSC pps payload to be sent
> + * @dsc_slice_per_pkt: number of DSC slices to be sent as in a single packet

s/as //

>   */
>  struct mipi_dsi_device {
>       struct mipi_dsi_host *host;
> @@ -196,6 +197,7 @@ struct mipi_dsi_device {
>       unsigned long hs_rate;
>       unsigned long lp_rate;
>       struct drm_dsc_config *dsc;
> +     unsigned int dsc_slice_per_pkt;

Why is it a part of the DSI device config? What if a device specifies
dsc_slice_per_pkt, but not DSC config? What are the legit boundaries for
this field?

>  };
>  
>  /**
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Reply via email to