| Message ID | 20230405-add-dsc-support-v3-1-6e1d35a206b3@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DSC v1.2 Support for DSI | expand |
On 2023-05-19 14:17:26, Jessica Zhang wrote: > Currently, when compression is enabled, hdisplay is reduced via integer > division. This causes issues for modes where the original hdisplay is > not a multiple of 3. The "issue" probably being some kind of underflow, because the stream size is too small compared to how much data we actually send? > To fix this, use DIV_ROUND_UP to divide hdisplay. > > Reported-by: Marijn Suijten <marijn.suijten@somainline.org> > Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup") > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> Yes, downstream has as very clear: dsc->pclk_per_line = DIV_ROUND_UP(total_bytes_per_intf, 3); which is used for width_final in SDE, and this is one of the mandatory fixes on a 1096-pixels-wide panel, which is not a multiple of 3. Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 9223d7ec5a73..18d38b90eb28 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > * pulse width same > */ > h_total -= hdisplay; > - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3; > + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3); > h_total += hdisplay; > ha_end = ha_start + hdisplay; > } > > -- > 2.40.1 >
On 2023-05-19 14:17:26, Jessica Zhang wrote: > Currently, when compression is enabled, hdisplay is reduced via integer > division. This causes issues for modes where the original hdisplay is > not a multiple of 3. > > To fix this, use DIV_ROUND_UP to divide hdisplay. > > Reported-by: Marijn Suijten <marijn.suijten@somainline.org> This should have been: Suggested-by: Marijn Suijten <marijn.suijten@somainline.org> > Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup") This hash is not valid (and checkpatch points it out...), as it is your local commit from the MSM DSC helper methods series. The original issue was introduced in: Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration") - Marijn > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 9223d7ec5a73..18d38b90eb28 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > * pulse width same > */ > h_total -= hdisplay; > - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3; > + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3); > h_total += hdisplay; > ha_end = ha_start + hdisplay; > } > > -- > 2.40.1 >
On 5/20/2023 1:07 AM, Marijn Suijten wrote: > On 2023-05-19 14:17:26, Jessica Zhang wrote: >> Currently, when compression is enabled, hdisplay is reduced via integer >> division. This causes issues for modes where the original hdisplay is >> not a multiple of 3. >> >> To fix this, use DIV_ROUND_UP to divide hdisplay. >> >> Reported-by: Marijn Suijten <marijn.suijten@somainline.org> > > This should have been: > > Suggested-by: Marijn Suijten <marijn.suijten@somainline.org> > >> Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup") > > This hash is not valid (and checkpatch points it out...), as it is your > local commit from the MSM DSC helper methods series. The original issue > was introduced in: > > Fixes: 08802f515c3cf ("drm/msm/dsi: Add support for DSC configuration") Hi Marijn, Acked. Thanks, Jessica Zhang > > - Marijn > >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c >> index 9223d7ec5a73..18d38b90eb28 100644 >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c >> @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) >> * pulse width same >> */ >> h_total -= hdisplay; >> - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3; >> + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3); >> h_total += hdisplay; >> ha_end = ha_start + hdisplay; >> } >> >> -- >> 2.40.1 >>
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 9223d7ec5a73..18d38b90eb28 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -952,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) * pulse width same */ h_total -= hdisplay; - hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3; + hdisplay = DIV_ROUND_UP(msm_dsc_get_bytes_per_line(msm_host->dsc), 3); h_total += hdisplay; ha_end = ha_start + hdisplay; }
Currently, when compression is enabled, hdisplay is reduced via integer division. This causes issues for modes where the original hdisplay is not a multiple of 3. To fix this, use DIV_ROUND_UP to divide hdisplay. Reported-by: Marijn Suijten <marijn.suijten@somainline.org> Fixes: f3a99460406b ("drm/msm/dsi: update hdisplay calculation for dsi_timing_setup") Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)