| Message ID | 20240829-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-8-v1-3-bdb05b4b5a2e@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm: Support quad pipe with dual-DSI | expand |
On Thu, 29 Aug 2024 at 13:19, Jun Nie <jun.nie@linaro.org> wrote: > > Data width for dsc engine is aligned with pipe, not with whole screen > width. Because the width may be halved in DSI bonded case. Can't really parse this. > > The dsc width is not related to the timing with back front porch in > later stage, so update dsc timing earlier. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > 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); Two separate commits > > @@ -964,6 +964,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); > + > /* > * DPU sends 3 bytes per pclk cycle to DSI. If widebus is > * enabled, bus width is extended to 6 bytes. > @@ -990,9 +995,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)); > @@ -1011,9 +1013,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 * mipi_dsi_pixel_format_to_bpp(msm_host->format) / 8 + 1; > > -- > 2.34.1 >
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年8月29日周四 18:57写道： > > On Thu, 29 Aug 2024 at 13:19, Jun Nie <jun.nie@linaro.org> wrote: > > > > Data width for dsc engine is aligned with pipe, not with whole screen > > width. Because the width may be halved in DSI bonded case. > > Can't really parse this. Please forgive me for my bad English. Is below words better? Data width for DSC timing is aligned with the width that goes to a DSI interface, not with whole screen width. For bonded-DSI case, the width for DSC timing is half width of whole screen. > > > > > The dsc width is not related to the timing with back front porch in > > later stage, so update dsc timing earlier. > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > --- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > 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); > > Two separate commits OK. Will split it. > > > > > @@ -964,6 +964,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); > > + > > /* > > * DPU sends 3 bytes per pclk cycle to DSI. If widebus is > > * enabled, bus width is extended to 6 bytes. > > @@ -990,9 +995,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)); > > @@ -1011,9 +1013,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 * mipi_dsi_pixel_format_to_bpp(msm_host->format) / 8 + 1; > > > > -- > > 2.34.1 > > > > > -- > With best wishes > Dmitry
On Tue, 3 Sept 2024 at 10:34, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年8月29日周四 18:57写道： > > > > On Thu, 29 Aug 2024 at 13:19, Jun Nie <jun.nie@linaro.org> wrote: > > > > > > Data width for dsc engine is aligned with pipe, not with whole screen > > > width. Because the width may be halved in DSI bonded case. > > > > Can't really parse this. > > Please forgive me for my bad English. Is below words better? > > Data width for DSC timing is aligned with the width that goes to a DSI to a single DSI interface > interface, not with whole screen width. For bonded-DSI case, the > width for DSC timing is half width of whole screen. sounds much better. > > > > > > > > The dsc width is not related to the timing with back front porch in > > > later stage, so update dsc timing earlier. > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > --- > > > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 ++++++------- > > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > > > 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); > > > > Two separate commits > > OK. Will split it. > > > > > > > > > @@ -964,6 +964,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); > > > + > > > /* > > > * DPU sends 3 bytes per pclk cycle to DSI. If widebus is > > > * enabled, bus width is extended to 6 bytes. > > > @@ -990,9 +995,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)); > > > @@ -1011,9 +1013,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 * mipi_dsi_pixel_format_to_bpp(msm_host->format) / 8 + 1; > > > > > > -- > > > 2.34.1 > > > > > > > > > -- > > With best wishes > > Dmitry
On 2024-08-29 18:17:32, Jun Nie wrote: > Data width for dsc engine is aligned with pipe, not with whole screen > width. Because the width may be halved in DSI bonded case. > > The dsc width is not related to the timing with back front porch in > later stage, so update dsc timing earlier. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> I already sent a patch for this: https://lore.kernel.org/linux-arm-msm/20240417-drm-msm-initial-dualpipe-dsc-fixes-v1-2-78ae3ee9a697@somainline.org/ And then came up with a better solution, outlined in: https://lore.kernel.org/linux-arm-msm/7fqwkryeumkt7zxsec6va7ys22nfs3tr4rrcz323extdz3f6zv@w4uu2lk4uh7v/ Would you mind dropping this patch so that I can send a better solution? > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > 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; The other part of this already happened in patch 02/21? - Marijn > dsc->pic_height = mode->vdisplay; > DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height); > > @@ -964,6 +964,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); > + > /* > * DPU sends 3 bytes per pclk cycle to DSI. If widebus is > * enabled, bus width is extended to 6 bytes. > @@ -990,9 +995,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)); > @@ -1011,9 +1013,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 * mipi_dsi_pixel_format_to_bpp(msm_host->format) / 8 + 1; > > -- > 2.34.1 >
Marijn Suijten <marijn.suijten@somainline.org> 于2024年9月3日周二 18:12写道： > > On 2024-08-29 18:17:32, Jun Nie wrote: > > Data width for dsc engine is aligned with pipe, not with whole screen > > width. Because the width may be halved in DSI bonded case. > > > > The dsc width is not related to the timing with back front porch in > > later stage, so update dsc timing earlier. > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > I already sent a patch for this: > https://lore.kernel.org/linux-arm-msm/20240417-drm-msm-initial-dualpipe-dsc-fixes-v1-2-78ae3ee9a697@somainline.org/ > > And then came up with a better solution, outlined in: > https://lore.kernel.org/linux-arm-msm/7fqwkryeumkt7zxsec6va7ys22nfs3tr4rrcz323extdz3f6zv@w4uu2lk4uh7v/ > > Would you mind dropping this patch so that I can send a better solution? Sure. I am happy with a better solution from you. > > > --- > > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > 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; > > The other part of this already happened in patch 02/21? > > - Marijn > Patch 02/21 is just for parameter validation, not directly related to this patch. -Jun
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); @@ -964,6 +964,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); + /* * DPU sends 3 bytes per pclk cycle to DSI. If widebus is * enabled, bus width is extended to 6 bytes. @@ -990,9 +995,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)); @@ -1011,9 +1013,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 * mipi_dsi_pixel_format_to_bpp(msm_host->format) / 8 + 1;
Data width for dsc engine is aligned with pipe, not with whole screen width. Because the width may be halved in DSI bonded case. The dsc width is not related to the timing with back front porch in later stage, so update dsc timing earlier. Signed-off-by: Jun Nie <jun.nie@linaro.org> --- drivers/gpu/drm/msm/dsi/dsi_host.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)