[PATCH 2/2] drm/dsc: Use 32-bit integers for some DSC parameter calculations

[Cornij, Nikola]

Looks good to me

-----Original Message-----
From: mikita.lipski at amd.com <mikita.lipski at amd.com> 
Sent: November 13, 2019 2:07 PM
To: amd-gfx at lists.freedesktop.org
Cc: Wentland, Harry <Harry.Wentland at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; Cornij, Nikola <Nikola.Cornij at amd.com>; manasi.d.navare at intel.com; Lipski, Mikita <Mikita.Lipski at amd.com>
Subject: [PATCH 2/2] drm/dsc: Use 32-bit integers for some DSC parameter calculations

From: Mikita Lipski <mikita.lipski at amd.com>

[why]
There are a few DSC PPS parameters, such as scale_increment_interval, that could overflow 16-bit integer if non-DSC-spec-compliant configuration was used. There is a check in the code against that, however 16-bit integer is used to store those values, which invalidates the check, letting invalid configurations through.

[how]
Use 32-bit integers for the affected DSC PPS parameters calculations

Signed-off-by: Nikola Cornij <nikola.cornij at amd.com>
Signed-off-by: Mikita Lipski <mikita.lipski at amd.com>
---
 drivers/gpu/drm/drm_dsc.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c index 79c71e3fc973..74f3527f567d 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -297,6 +297,9 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
 	unsigned long final_scale = 0;
 	unsigned long rbs_min = 0;
 	unsigned long max_offset = 0;
+	u32 nfl_bpg_offset;
+	u32 nsl_bpg_offset;
+	u32 scale_increment_interval;
 
 	if (vdsc_cfg->native_420 || vdsc_cfg->native_422) {
 		/* Number of groups used to code each line of a slice */ @@ -364,28 +367,32 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
 		 * hence we multiply by 2^11 for preserving the
 		 * fractional part
 		 */
-		vdsc_cfg->nfl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->first_line_bpg_offset << 11),
+		nfl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->first_line_bpg_offset << 
+11),
 							(vdsc_cfg->slice_height - 1));
 	else
-		vdsc_cfg->nfl_bpg_offset = 0;
+		nfl_bpg_offset = 0;
 
 	/* 2^16 - 1 */
-	if (vdsc_cfg->nfl_bpg_offset > 65535) {
+	if (nfl_bpg_offset > 65535) {
 		DRM_DEBUG_KMS("NflBpgOffset is too large for this slice height\n");
 		return -ERANGE;
 	}
 
+	vdsc_cfg->nfl_bpg_offset = (u16)nfl_bpg_offset;
+
 	if (vdsc_cfg->slice_height > 2)
-		vdsc_cfg->nsl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->second_line_bpg_offset << 11),
+		nsl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->second_line_bpg_offset << 
+11),
 							(vdsc_cfg->slice_height - 1));
 	else
-		vdsc_cfg->nsl_bpg_offset = 0;
+		nsl_bpg_offset = 0;
 
-	if (vdsc_cfg->nsl_bpg_offset > 65535) {
+	if (nsl_bpg_offset > 65535) {
 		DRM_DEBUG_KMS("NslBpgOffset is too large for this slice height\n");
 		return -ERANGE;
 	}
 
+	vdsc_cfg->nsl_bpg_offset = (u16)nsl_bpg_offset;
+
 	/* Number of groups used to code the entire slice */
 	groups_total = groups_per_line * vdsc_cfg->slice_height;
 
@@ -402,10 +409,10 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
 		 * as (NflBpgOffset + SliceBpgOffset) has 11 bit fractional value,
 		 * we need divide by 2^11 from pstDscCfg values
 		 */
-		vdsc_cfg->scale_increment_interval =
+		scale_increment_interval =
 				(vdsc_cfg->final_offset * (1 << 11)) /
-				((vdsc_cfg->nfl_bpg_offset +
-				vdsc_cfg->nsl_bpg_offset +
+				((nfl_bpg_offset +
+				nsl_bpg_offset +
 				vdsc_cfg->slice_bpg_offset) *
 				(final_scale - 9));
 	} else {
@@ -413,14 +420,16 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
 		 * If finalScaleValue is less than or equal to 9, a value of 0 should
 		 * be used to disable the scale increment at the end of the slice
 		 */
-		vdsc_cfg->scale_increment_interval = 0;
+		scale_increment_interval = 0;
 	}
 
-	if (vdsc_cfg->scale_increment_interval > 65535) {
+	if (scale_increment_interval > 65535) {
 		DRM_DEBUG_KMS("ScaleIncrementInterval is large for slice height\n");
 		return -ERANGE;
 	}
 
+	vdsc_cfg->scale_increment_interval = (u16)scale_increment_interval;
+
 	/*
 	 * DSC spec mentions that bits_per_pixel specifies the target
 	 * bits/pixel (bpp) rate that is used by the encoder,
--
2.17.1