Re: [PATCH v2 1/3] drm/msm/dsi: Use existing per-interface slice count in DSC timing

[Jessica Zhang <quic_jesszhan@quicinc.com>]

On 2/9/2025 1:42 PM, Marijn Suijten wrote:
> When configuring the timing of DSI hosts (interfaces) in
> dsi_timing_setup() all values written to registers are taking
> bonded-mode into account by dividing the original mode width by 2
> (half the data is sent over each of the two DSI hosts), but the full
> width instead of the interface width is passed as hdisplay parameter to
> dsi_update_dsc_timing().
> 
> Currently only msm_dsc_get_slices_per_intf() is called within
> dsi_update_dsc_timing() with the `hdisplay` argument which clearly
> documents that it wants the width of a single interface (which, again,
> in bonded DSI mode is half the total width of the mode) resulting in all
> subsequent values to be completely off.
> 
> However, as soon as we start to pass the halved hdisplay
> into dsi_update_dsc_timing() we might as well discard
> msm_dsc_get_slices_per_intf() since the value it calculates is already
> available in dsc->slice_count which is per-interface by the current
> design of MSM DPU/DSI implementations and their use of the DRM DSC
> helpers.
> 
> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>

Hi Marijn,

Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

Thanks,

Jessica Zhang

> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c   |  8 ++++----
>   drivers/gpu/drm/msm/msm_dsc_helper.h | 11 -----------
>   2 files changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 007311c21fdaa0462b05d53cd8a2aad0269b1727..42e100a8adca09d7b55afce0e2553e76d898744f 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -846,7 +846,7 @@ static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,
>   		dsi_write(msm_host, REG_DSI_CPHY_MODE_CTRL, BIT(0));
>   }
>   
> -static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mode, u32 hdisplay)
> +static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mode)
>   {
>   	struct drm_dsc_config *dsc = msm_host->dsc;
>   	u32 reg, reg_ctrl, reg_ctrl2;
> @@ -858,7 +858,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
>   	/* first calculate dsc parameters and then program
>   	 * compress mode registers
>   	 */
> -	slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
> +	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; */
> @@ -991,7 +991,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>   
>   	if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) {
>   		if (msm_host->dsc)
> -			dsi_update_dsc_timing(msm_host, false, mode->hdisplay);
> +			dsi_update_dsc_timing(msm_host, false);
>   
>   		dsi_write(msm_host, REG_DSI_ACTIVE_H,
>   			DSI_ACTIVE_H_START(ha_start) |
> @@ -1012,7 +1012,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>   			DSI_ACTIVE_VSYNC_VPOS_END(vs_end));
>   	} else {		/* command mode */
>   		if (msm_host->dsc)
> -			dsi_update_dsc_timing(msm_host, true, mode->hdisplay);
> +			dsi_update_dsc_timing(msm_host, true);
>   
>   		/* image data and 1 byte write_memory_start cmd */
>   		if (!msm_host->dsc)
> diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.h b/drivers/gpu/drm/msm/msm_dsc_helper.h
> index b9049fe1e2790703a6f42dd7e6cd3fa5eea29389..63f95523b2cbb48f822210ac47cdd3526f231a89 100644
> --- a/drivers/gpu/drm/msm/msm_dsc_helper.h
> +++ b/drivers/gpu/drm/msm/msm_dsc_helper.h
> @@ -12,17 +12,6 @@
>   #include <linux/math.h>
>   #include <drm/display/drm_dsc_helper.h>
>   
> -/**
> - * msm_dsc_get_slices_per_intf() - calculate number of slices per interface
> - * @dsc: Pointer to drm dsc config struct
> - * @intf_width: interface width in pixels
> - * Returns: Integer representing the number of slices for the given interface
> - */
> -static inline u32 msm_dsc_get_slices_per_intf(const struct drm_dsc_config *dsc, u32 intf_width)
> -{
> -	return DIV_ROUND_UP(intf_width, dsc->slice_width);
> -}
> -
>   /**
>    * msm_dsc_get_bytes_per_line() - calculate bytes per line
>    * @dsc: Pointer to drm dsc config struct
> 
> -- 
> 2.48.1
> 
>