Messages in this thread | | | Subject | Re: [PATCH 4/4] drm/msm/dsi: fix DSC for the bonded DSI case | From | Jonathan Marek <> | Date | Tue, 14 Nov 2023 14:00:19 -0500 |
| |
On 11/14/23 1:28 PM, Marijn Suijten wrote: > On what hardware have you been testing this? Dmitry and I have a stack of > patches to resolve support for Active CTL programming on newer hardware (DPU > 5.0+ IIRC), where a single CTL is responsible for programming multiple INTF and > DSC blocks as used in bonded DSI. >
I am also using DPU 6+ but I won't be posting patches for DPU to support this as I am not using the upstream DPU codebase.
> On 2023-11-14 12:42:16, Jonathan Marek wrote: >> 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. >> >> Signed-off-by: Jonathan Marek <jonathan@marek.ca> >> --- >> drivers/gpu/drm/msm/dsi/dsi.h | 3 ++- >> drivers/gpu/drm/msm/dsi/dsi_host.c | 20 +++++++++++--------- >> drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +- >> 3 files changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h >> index 28379b1af63f..3a641e69447c 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi.h >> +++ b/drivers/gpu/drm/msm/dsi/dsi.h >> @@ -93,7 +93,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 7284346ab787..a6286eb9d006 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> @@ -938,8 +938,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) >> mode->hdisplay, mode->vdisplay); >> return; >> } >> - >> - dsc->pic_width = mode->hdisplay; >> + dsc->pic_width = hdisplay; > > In my testing and debugging on CMDmode panels downstream this value/register > was always programmed to the _full_ width of the bonded panel. Is that maybe > different for video mode? >
downstream dual DSI panel timings are specified for a single panel ("qcom,mdss-dsi-panel-width" is for a single panel, not both panels)
>> dsc->pic_height = mode->vdisplay; >> DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height); >> >> @@ -950,6 +949,11 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) >> if (ret) >> return; >> >> + if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) >> + dsi_update_dsc_timing(msm_host, false, hdisplay); >> + else >> + dsi_update_dsc_timing(msm_host, true, hdisplay); >> + > > Such cleanups (which appear unrelated) should probably be posted as separate > patches. > > - Marijn >
Its not unrelated, dsi_update_dsc_timing call is moved up so it can use the single-panel "hdisplay" value before it gets adjusted for DSC.
>> /* Divide the display by 3 but keep back/font porch and >> * pulse width same >> */ >> @@ -966,9 +970,6 @@ 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_write(msm_host, REG_DSI_ACTIVE_H, >> DSI_ACTIVE_H_START(ha_start) | >> DSI_ACTIVE_H_END(ha_end)); >> @@ -987,9 +988,6 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) >> DSI_ACTIVE_VSYNC_VPOS_START(vs_start) | >> DSI_ACTIVE_VSYNC_VPOS_END(vs_end)); >> } else { /* command mode */ >> - if (msm_host->dsc) >> - dsi_update_dsc_timing(msm_host, true, mode->hdisplay); >> - >> /* image data and 1 byte write_memory_start cmd */ >> if (!msm_host->dsc) >> wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1; >> @@ -2487,7 +2485,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; >> @@ -2497,6 +2496,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 896f369fdd53..2ca1a7ca3659 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c >> @@ -455,7 +455,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 const struct drm_bridge_funcs dsi_mgr_bridge_funcs = { >> -- >> 2.26.1 >>
| |