[02/21] drm/msm/dsi: fix DSC width for the bonded DSI case

From: Jonathan Marek <jonathan@marek.ca>

For the bonded DSI case, DSC pic_width and timing calculations should use
the width of a single panel instead of the total combined width.

Bonded DSI can be used to drive a single panel having two input
channels, or to drive two panels with a input channel on every panel that
behave like single panel for display controller.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi.h         | 3 ++-
 drivers/gpu/drm/msm/dsi/dsi_host.c    | 6 +++++-
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

On 2024-08-29 18:17:31, Jun Nie wrote:
> From: Jonathan Marek <jonathan@marek.ca>
> 
> For the bonded DSI case, DSC pic_width and timing calculations should use
> the width of a single panel instead of the total combined width.

When this patch was originally proposed we already discussed [1] that this is
**not** universally true.  On my hardware a single bonded panel always receives
the full width, at least on downstream kernels, and it works [2].

[1]: https://lore.kernel.org/linux-arm-msm/eanx45rnasj7lu3r2tfhtg4qkqkcidd6zctsz6ci6jlklu4fgi@3nf73w2ka4li/T/#u
[2]: https://gitlab.freedesktop.org/drm/msm/-/issues/41

Can we please figure this out before landing this patch?

- Marijn

> Bonded DSI can be used to drive a single panel having two input
> channels, or to drive two panels with a input channel on every panel that
> behave like single panel for display controller.
> 
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  drivers/gpu/drm/msm/dsi/dsi.h         | 3 ++-
>  drivers/gpu/drm/msm/dsi/dsi_host.c    | 6 +++++-
>  drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> index 87496db203d6c..35b90c462f637 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -79,7 +79,8 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host);
>  int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
>  				  const struct drm_display_mode *mode);
>  enum drm_mode_status msm_dsi_host_check_dsc(struct mipi_dsi_host *host,
> -					    const struct drm_display_mode *mode);
> +					    const struct drm_display_mode *mode,
> +					    bool is_bonded_dsi);
>  unsigned long msm_dsi_host_get_mode_flags(struct mipi_dsi_host *host);
>  int msm_dsi_host_register(struct mipi_dsi_host *host);
>  void msm_dsi_host_unregister(struct mipi_dsi_host *host);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 6388bb12696ff..7a4d9c071be5a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -2489,7 +2489,8 @@ int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
>  }
>  
>  enum drm_mode_status msm_dsi_host_check_dsc(struct mipi_dsi_host *host,
> -					    const struct drm_display_mode *mode)
> +					    const struct drm_display_mode *mode,
> +					    bool is_bonded_dsi)
>  {
>  	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
>  	struct drm_dsc_config *dsc = msm_host->dsc;
> @@ -2499,6 +2500,9 @@ enum drm_mode_status msm_dsi_host_check_dsc(struct mipi_dsi_host *host,
>  	if (!msm_host->dsc)
>  		return MODE_OK;
>  
> +	if (is_bonded_dsi)
> +		pic_width = mode->hdisplay / 2;
> +
>  	if (pic_width % dsc->slice_width) {
>  		pr_err("DSI: pic_width %d has to be multiple of slice %d\n",
>  		       pic_width, dsc->slice_width);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index a210b7c9e5ca2..6e915b57e14bb 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -420,7 +420,7 @@ static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
>  			return MODE_ERROR;
>  	}
>  
> -	return msm_dsi_host_check_dsc(host, mode);
> +	return msm_dsi_host_check_dsc(host, mode, IS_BONDED_DSI());
>  }
>  
>  static int dsi_mgr_bridge_attach(struct drm_bridge *bridge,
> 
> -- 
> 2.34.1
>

On 2024-09-03 11:50:36, Marijn Suijten wrote:
> On 2024-08-29 18:17:31, Jun Nie wrote:
> > From: Jonathan Marek <jonathan@marek.ca>
> > 
> > For the bonded DSI case, DSC pic_width and timing calculations should use
> > the width of a single panel instead of the total combined width.
> 
> When this patch was originally proposed we already discussed [1] that this is
> **not** universally true.  On my hardware a single bonded panel always receives
> the full width, at least on downstream kernels, and it works [2].
> 
> [1]: https://lore.kernel.org/linux-arm-msm/eanx45rnasj7lu3r2tfhtg4qkqkcidd6zctsz6ci6jlklu4fgi@3nf73w2ka4li/T/#u
> [2]: https://gitlab.freedesktop.org/drm/msm/-/issues/41
> 
> Can we please figure this out before landing this patch?

For completeness I've picked this patch, together with the following
mis-squashed change from patch 03/21:

	diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
	index 7a4d9c071be5a..5abade8f26b88 100644
	--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
	+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
	@@ -953,7 +953,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
	 			return;
	 		}
 
	-		dsc->pic_width = mode->hdisplay;
	+		dsc->pic_width = hdisplay;
	 		dsc->pic_height = mode->vdisplay;
	 		DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);

And this is what it looks like on a bonded DSI CMD-mode display:
https://gitlab.freedesktop.org/drm/msm/-/issues/41#note_2553207
https://gitlab.freedesktop.org/-/project/2206/uploads/dc5c53d09ecb635fdc9f190fbc9b37ac/1000027079.jpg

That's a clear regression :)

- Marijn

Marijn Suijten <marijn.suijten@somainline.org> 于2024年9月3日周二 19:51写道：
>
> On 2024-09-03 11:50:36, Marijn Suijten wrote:
> > On 2024-08-29 18:17:31, Jun Nie wrote:
> > > From: Jonathan Marek <jonathan@marek.ca>
> > >
> > > For the bonded DSI case, DSC pic_width and timing calculations should use
> > > the width of a single panel instead of the total combined width.
> >
> > When this patch was originally proposed we already discussed [1] that this is
> > **not** universally true.  On my hardware a single bonded panel always receives
> > the full width, at least on downstream kernels, and it works [2].
> >
> > [1]: https://lore.kernel.org/linux-arm-msm/eanx45rnasj7lu3r2tfhtg4qkqkcidd6zctsz6ci6jlklu4fgi@3nf73w2ka4li/T/#u
> > [2]: https://gitlab.freedesktop.org/drm/msm/-/issues/41
> >
> > Can we please figure this out before landing this patch?
>
> For completeness I've picked this patch, together with the following
> mis-squashed change from patch 03/21:
>
>         diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
>         index 7a4d9c071be5a..5abade8f26b88 100644
>         --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>         +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>         @@ -953,7 +953,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
>                                 return;
>                         }
>
>         -               dsc->pic_width = mode->hdisplay;
>         +               dsc->pic_width = hdisplay;
>                         dsc->pic_height = mode->vdisplay;
>                         DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);
>
> And this is what it looks like on a bonded DSI CMD-mode display:
> https://gitlab.freedesktop.org/drm/msm/-/issues/41#note_2553207
> https://gitlab.freedesktop.org/-/project/2206/uploads/dc5c53d09ecb635fdc9f190fbc9b37ac/1000027079.jpg
>
> That's a clear regression :)
>
> - Marijn

Surely we should figure out what's going on here before we land this
patch. Please help me to understand
your setup first. Your panel works well on mainline kernel with
2:2:2(?) topology, and you see issue with applying
the change here, right?
There are several parameter that impact the issue, video/command DSI
mode, single panel with 2 DSI vs
2 panels with single DSI on each panel, 4:4:2 topology on my device vs
your topology. Let's list all of them
before further discussion.

-Jun