| Message ID | 20221009185316.462522-1-marijn.suijten@somainline.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Fix math issues in MSM DSC implementation | expand |
On 09/10/2022 21:53, Marijn Suijten wrote: > drm_dsc_config's bits_per_pixel field holds a fractional value with 4 > bits, which all panel drivers should adhere to for > drm_dsc_pps_payload_pack() to generate a valid payload. All code in the > DSI driver here seems to assume that this field doesn't contain any > fractional bits, hence resulting in the wrong values being computed. > Since none of the calculations leave any room for fractional bits or > seem to indicate any possible area of support, disallow such values > altogether. calculate_rc_params() in intel_vdsc.c performs an identical > bitshift to get at this integer value. > > Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data") > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) >
On 10/9/2022 11:53 AM, Marijn Suijten wrote: > drm_dsc_config's bits_per_pixel field holds a fractional value with 4 > bits, which all panel drivers should adhere to for > drm_dsc_pps_payload_pack() to generate a valid payload. All code in the > DSI driver here seems to assume that this field doesn't contain any > fractional bits, hence resulting in the wrong values being computed. > Since none of the calculations leave any room for fractional bits or > seem to indicate any possible area of support, disallow such values > altogether. calculate_rc_params() in intel_vdsc.c performs an identical > bitshift to get at this integer value. > > Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data") > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 7e6b7e506ae8..46032c576a59 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -34,7 +34,7 @@ > > #define DSI_RESET_TOGGLE_DELAY_MS 20 > > -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc); > +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc); > > static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor) > { > @@ -909,6 +909,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > u32 va_end = va_start + mode->vdisplay; > u32 hdisplay = mode->hdisplay; > u32 wc; > + int ret; > > DBG(""); > > @@ -944,7 +945,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) > /* we do the calculations for dsc parameters here so that > * panel can use these parameters > */ > - dsi_populate_dsc_params(dsc); > + ret = dsi_populate_dsc_params(msm_host, dsc); > + if (ret) > + return; > > /* Divide the display by 3 but keep back/font porch and > * pulse width same > @@ -1770,9 +1773,15 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = { > 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12 > }; > > -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc) > +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc) > { > int i; > + u16 bpp = dsc->bits_per_pixel >> 4; > + > + if (dsc->bits_per_pixel & 0xf) { > + DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support fractional bits_per_pixel\n"); > + return -EINVAL; > + } > > if (dsc->bits_per_component != 8) { > DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n"); > @@ -1798,8 +1807,8 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc) > dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i]; > } > > - dsc->initial_offset = 6144; /* Not bpp 12 */ > - if (dsc->bits_per_pixel != 8) > + dsc->initial_offset = 6144; /* Not bpp 12 */ > + if (bpp != 8) > dsc->initial_offset = 2048; /* bpp = 12 */ > > if (dsc->bits_per_component <= 10)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 7e6b7e506ae8..46032c576a59 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -34,7 +34,7 @@ #define DSI_RESET_TOGGLE_DELAY_MS 20 -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc); +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc); static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor) { @@ -909,6 +909,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) u32 va_end = va_start + mode->vdisplay; u32 hdisplay = mode->hdisplay; u32 wc; + int ret; DBG(""); @@ -944,7 +945,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) /* we do the calculations for dsc parameters here so that * panel can use these parameters */ - dsi_populate_dsc_params(dsc); + ret = dsi_populate_dsc_params(msm_host, dsc); + if (ret) + return; /* Divide the display by 3 but keep back/font porch and * pulse width same @@ -1770,9 +1773,15 @@ static char bpg_offset[DSC_NUM_BUF_RANGES] = { 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12 }; -static int dsi_populate_dsc_params(struct drm_dsc_config *dsc) +static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc) { int i; + u16 bpp = dsc->bits_per_pixel >> 4; + + if (dsc->bits_per_pixel & 0xf) { + DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support fractional bits_per_pixel\n"); + return -EINVAL; + } if (dsc->bits_per_component != 8) { DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n"); @@ -1798,8 +1807,8 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc) dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i]; } - dsc->initial_offset = 6144; /* Not bpp 12 */ - if (dsc->bits_per_pixel != 8) + dsc->initial_offset = 6144; /* Not bpp 12 */ + if (bpp != 8) dsc->initial_offset = 2048; /* bpp = 12 */ if (dsc->bits_per_component <= 10)
drm_dsc_config's bits_per_pixel field holds a fractional value with 4 bits, which all panel drivers should adhere to for drm_dsc_pps_payload_pack() to generate a valid payload. All code in the DSI driver here seems to assume that this field doesn't contain any fractional bits, hence resulting in the wrong values being computed. Since none of the calculations leave any room for fractional bits or seem to indicate any possible area of support, disallow such values altogether. calculate_rc_params() in intel_vdsc.c performs an identical bitshift to get at this integer value. Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data") Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> --- drivers/gpu/drm/msm/dsi/dsi_host.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)