[PATCH v14 0/9] Introduce MSM-specific DSC helpers

[PATCH v14 0/9] Introduce MSM-specific DSC helpers

[Jessica Zhang]

There are some overlap in calculations for MSM-specific DSC variables
between DP and DSI. In addition, the calculations for initial_scale_value
and det_thresh_flatness that are defined within the DSC 1.2 specifications,
but aren't yet included in drm_dsc_helper.c.

This series moves these calculations to a shared msm_dsc_helper.c file and
defines drm_dsc_helper methods for initial_scale_value and
det_thresh_flatness.

Note: For now, the MSM specific helper methods are only called for the DSI
path, but will called for DP once DSC 1.2 support for DP has been added.

Depends on: "drm/i915: move DSC RC tables to drm_dsc_helper.c" [1]

[1] https://patchwork.freedesktop.org/series/114472/

---
Changes in v14:
- Added kernel docs and made DRM DSC helper functions (Jani)
- Link to v13: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v13-0-d7581e7becec@quicinc.com

Changes in v13:
- Reworded comment doc for msm_dsc_get_slices_per_intf()
- Changed intf_width to u32
- msm_dsc_calculate_slices_per_intf -> msm_dsc_get_slices_per_intf
- Link to v12: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v12-0-9cdb7401f614@quicinc.com

Changes in v12:
- Reworded summary comment for msm_dsc_helper.h
- msm_dsc_get_slices_per_intf -> msm_dsc_calculate_slices_per_intf
- Corrected total_bytes_per_intf math in dsc_update_dsc_timing
- Rebased on top of latest version of "drm/i915: move DSC RC tables to drm_dsc_helper.c"
- Link to v11: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v11-0-30270e1eeac3@quicinc.com

Changes in v11:
- Fixed mismatched return types
- Moved MSM DSC helpers summary comment to under copyright
- Moved msm_dsc_get_bpp_int() to drm_dsc_helper.h
- Reworded MSM DSC helper comment docs for clarity
- Added const keyword where applicable
- Renamed msm_dsc_get_slice_per_intf to msm_dsc_get_slices_per_intf
- Removed msm_dsc_get_slice_per_intf()
- Reworded commit message for "drm/msm/dsi: update hdisplay calculation
  for dsi_timing_setup"
- Link to v10: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v10-0-4cb21168c227@quicinc.com

Changes in v10:
- Removed msm_dsc_get_bytes_per_slice helper
- Inlined msm_dsc_get_bytes_per_intf
- Refactored drm_dsc_set_initial_scale_value() to be a pure function
- Renamed DRM DSC initial_scale and flatness_det_thresh helpers
- Removed msm_dsc_helpers.o from Makefile
- Link to v9: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v9-0-87daeaec2c0a@quicinc.com

Changes in v9:
- Fixed incorrect math for msm_dsc_get_bytes_per_line()
- Link to v8: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v8-0-2c9b2bb1209c@quicinc.com

Changes in v8:
- *_bytes_per_soft_slice --> *_bytes_per_slice
- Fixed comment doc formatting for MSM DSC helpers
- Use slice_chunk_size in msm_dsc_get_bytes_per_line calculation
- Reworded "drm/msm/dpu: Use DRM DSC helper for det_thresh_flatness"
  commit title for clarity
- Picked up "Reviewed-by" tags
- Added duplicate Signed-off-by tag to "drm/display/dsc: Add flatness
  and initial scale value calculations" to reflect patch history
- Link to v7: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v7-0-df48a2c54421@quicinc.com

Changes in v7:
- Renamed msm_dsc_get_pclk_per_intf to msm_dsc_get_bytes_per_line
- Removed duplicate msm_dsc_get_dce_bytes_per_line
- Reworded commit message for "drm/msm/dpu: Use DRM DSC helper for
  det_thresh_flatness"
- Dropped slice_per_pkt change (it will be included in the later
  "Add DSC v1.2 Support for DSI" series)
- Picked up "drm/display/dsc: Add flatness and initial scale value
  calculations" and "drm/display/dsc: add helper to set semi-const
  parameters", which were dropped from "drm/i915: move DSC RC tables to
  drm_dsc_helper.c" series
- Picked up "Reviewed-by" tags
- Removed changelog in individual patches
- Link to v6: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v6-0-cb7f59f0f7fb@quicinc.com

Changes in v6:
- Documented return values for MSM DSC helpers
- Fixed dependency issue in msm_dsc_helper.c
- Link to v5: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v5-0-0108401d7886@quicinc.com

Changes in v5:
- Added extra line at end of msm_dsc_helper.h
- Simplified msm_dsc_get_bytes_per_soft_slice() math
- Simplified and inlined msm_dsc_get_pclk_per_intf() math
- "Fix calculations pkt_per_line" --> "... Fix calculation for pkt_per_line"
- Split dsc->slice_width check into a separate patch
- Picked up Dmitry's msm/dsi patch ("drm/msm/dsi: use new helpers for
  DSC setup")
- Removed unused headers in MSM DSC helper files
- Picked up Reviewed-by tags
- Link to v4: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v4-0-1b79c78b30d7@quicinc.com

Changes in v4:
- Changed msm_dsc_get_uncompressed_pclk_per_intf to msm_dsc_get_pclk_per_intf
- Moved pclk_per_intf calculation for dsi_timing_setup to `if
  (msm_host->dsc)` block
- Link to v3: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v3-0-6bec0d277a83@quicinc.com

Changes in v3:
- Dropped src_bpp parameter from all methods -- src_bpp can be
  calculated as dsc->bits_per_component * 3- Cleaned up unused parameters
- Dropped intf_width parameter from get_bytes_per_soft_slice()
- Moved dsc->bits_per_component to numerator calculation in
  get_bytes_per_soft_slice()
- Made get_bytes_per_soft_slice() a public method (this will be called
  later to help calculate DP pclk params)- Added comment documentation to
  MSM DSC helpers
- Renamed msm_dsc_get_uncompressed_pclk_per_line to
  *_get_uncompressed_pclk_per_intf()
- Removed dsc->slice_width check from msm_dsc_get_uncompressed_pclk_per_intf()
- Added documentation in comments
- Moved extra_eol_bytes math out of msm_dsc_get_eol_byte_num()
- Renamed msm_dsc_get_eol_byte_num to *_get_bytes_per_intf.
- Reworded slice_last_group_size calculation to `(dsc->slice_width + 2) % 3`
- Used MSM DSC helper to calculate total_bytes_per_intf
- Initialized hdisplay as uncompressed pclk per line at the beginning of
  dsi_timing_setup as to not break dual DSI calculations
- Added slice_width check to dsi_timing_setup
- Dropped 78c8b81d57d8 ("drm/display/dsc: Add flatness and initial scale
  value calculations") patch as it was absorbed in Dmitry's DSC series [1]
- Split dsi_timing_setup() hdisplay calculation to a separate patch
- Link to v2: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v2-0-3c13ced536b2@quicinc.com

Changes in v2:
- Changed det_thresh_flatness to flatness_det_thresh
- Set initial_scale_value directly in helper
- Moved msm_dsc_helper files to msm/ directory
- Dropped get_comp_ratio() helper
- Used drm_int2fixp() to convert to integers to fp
- Fixed type mismatch issues in MSM DSC helpers
- Changed DSC_BPP macro to drm_dsc_get_bpp_int() helper method
- Style changes to improve readability
- Dropped last division step of msm_dsc_get_pclk_per_line() and changed
  method name accordingly
- Dropped unused bpp variable in msm_dsc_get_dce_bytes_per_line()
- Changed msm_dsc_get_slice_per_intf() to a static inline method
- Split eol_byte_num and pkt_per_line calculation into a separate patch
- Moved pclk_per_line calculation into `if (dsc)` block in
  dsi_timing_setup()
- *_calculate_initial_scale_value --> *_set_initial_scale_value
- Picked up Fixes tags for patches 3/5 and 4/5
- Picked up Reviewed-by for patch 4/5
- Link to v1: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v1-0-f3e479f59b6d@quicinc.com

---
Dmitry Baryshkov (2):
      drm/display/dsc: add helper to set semi-const parameters
      drm/msm/dsi: use DRM DSC helpers for DSC setup

Jessica Zhang (7):
      drm/display/dsc: Add flatness and initial scale value calculations
      drm/display/dsc: Add drm_dsc_get_bpp_int helper
      drm/msm: Add MSM-specific DSC helper methods
      drm/msm/dpu: Use fixed DRM DSC helper for det_thresh_flatness
      drm/msm/dpu: Fix slice_last_group_size calculation
      drm/msm/dsi: Use MSM and DRM DSC helper methods
      drm/msm/dsi: update hdisplay calculation for dsi_timing_setup

 drivers/gpu/drm/display/drm_dsc_helper.c   | 59 ++++++++++++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c |  9 ++--
 drivers/gpu/drm/msm/dsi/dsi_host.c         | 68 ++++++------------------------
 drivers/gpu/drm/msm/msm_dsc_helper.h       | 38 +++++++++++++++++
 include/drm/display/drm_dsc_helper.h       |  4 ++
 5 files changed, 119 insertions(+), 59 deletions(-)
---
base-commit: c0c7f04818136b3204589da42b4532b5aa3d4971
change-id: 20230329-rfc-msm-dsc-helper-981a95edfbd0

Best regards,
-- 
Jessica Zhang <quic_jesszhan@quicinc.com>

[PATCH v14 1/9] drm/display/dsc: Add flatness and initial scale value calculations

[Jessica Zhang]

Add helpers to calculate det_thresh_flatness and initial_scale_value as
these calculations are defined within the DSC spec.

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/display/drm_dsc_helper.c | 24 ++++++++++++++++++++++++
 include/drm/display/drm_dsc_helper.h     |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c
index fc187a8d8873..4efb6236d22c 100644
--- a/drivers/gpu/drm/display/drm_dsc_helper.c
+++ b/drivers/gpu/drm/display/drm_dsc_helper.c
@@ -1413,3 +1413,27 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
 	return 0;
 }
 EXPORT_SYMBOL(drm_dsc_compute_rc_parameters);
+
+/**
+ * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config
+ * @dsc: Pointer to DRM DSC config struct
+ *
+ * Return: Calculated initial scale value
+ */
+u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc)
+{
+	return 8 * dsc->rc_model_size / (dsc->rc_model_size - dsc->initial_offset);
+}
+EXPORT_SYMBOL(drm_dsc_initial_scale_value);
+
+/**
+ * drm_dsc_flatness_det_thresh() - Calculate the flatness_det_thresh for the given DSC config
+ * @dsc: Pointer to DRM DSC config struct
+ *
+ * Return: Calculated flatness det thresh value
+ */
+u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc)
+{
+	return 2 << (dsc->bits_per_component - 8);
+}
+EXPORT_SYMBOL(drm_dsc_flatness_det_thresh);
diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h
index fc2104415dcb..71789fb34e17 100644
--- a/include/drm/display/drm_dsc_helper.h
+++ b/include/drm/display/drm_dsc_helper.h
@@ -24,6 +24,8 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
 void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg);
 int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params_type type);
 int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
+u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc);
+u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc);
 
 #endif /* _DRM_DSC_HELPER_H_ */
 

-- 
2.40.1

[PATCH v14 2/9] drm/display/dsc: add helper to set semi-const parameters

[Jessica Zhang]

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Add a helper setting config values which are typically constant across
operating modes (table E-4 of the standard) and mux_word_size (which is
a const according to 3.5.2).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/display/drm_dsc_helper.c | 22 ++++++++++++++++++++++
 include/drm/display/drm_dsc_helper.h     |  1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c
index 4efb6236d22c..b31fe9849784 100644
--- a/drivers/gpu/drm/display/drm_dsc_helper.c
+++ b/drivers/gpu/drm/display/drm_dsc_helper.c
@@ -270,6 +270,28 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_payload,
 }
 EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
 
+/**
+ * drm_dsc_set_const_params() - Set DSC parameters considered typically
+ * constant across operation modes
+ *
+ * @vdsc_cfg:
+ * DSC Configuration data partially filled by driver
+ */
+void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg)
+{
+	if (!vdsc_cfg->rc_model_size)
+		vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST;
+	vdsc_cfg->rc_edge_factor = DSC_RC_EDGE_FACTOR_CONST;
+	vdsc_cfg->rc_tgt_offset_high = DSC_RC_TGT_OFFSET_HI_CONST;
+	vdsc_cfg->rc_tgt_offset_low = DSC_RC_TGT_OFFSET_LO_CONST;
+
+	if (vdsc_cfg->bits_per_component <= 10)
+		vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
+	else
+		vdsc_cfg->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC;
+}
+EXPORT_SYMBOL(drm_dsc_set_const_params);
+
 /* From DSC_v1.11 spec, rc_parameter_Set syntax element typically constant */
 static const u16 drm_dsc_rc_buf_thresh[] = {
 	896, 1792, 2688, 3584, 4480, 5376, 6272, 6720, 7168, 7616,
diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h
index 71789fb34e17..f4e18e5d077a 100644
--- a/include/drm/display/drm_dsc_helper.h
+++ b/include/drm/display/drm_dsc_helper.h
@@ -21,6 +21,7 @@ void drm_dsc_dp_pps_header_init(struct dp_sdp_header *pps_header);
 int drm_dsc_dp_rc_buffer_size(u8 rc_buffer_block_size, u8 rc_buffer_size);
 void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
 			      const struct drm_dsc_config *dsc_cfg);
+void drm_dsc_set_const_params(struct drm_dsc_config *vdsc_cfg);
 void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg);
 int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params_type type);
 int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);

-- 
2.40.1

[PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper

[Jessica Zhang]

Add helper to get the integer value of drm_dsc_config.bits_per_pixel

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/display/drm_dsc_helper.c | 13 +++++++++++++
 include/drm/display/drm_dsc_helper.h     |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c
index b31fe9849784..4424380c6cb6 100644
--- a/drivers/gpu/drm/display/drm_dsc_helper.c
+++ b/drivers/gpu/drm/display/drm_dsc_helper.c
@@ -1436,6 +1436,19 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
 }
 EXPORT_SYMBOL(drm_dsc_compute_rc_parameters);
 
+/**
+ * drm_dsc_get_bpp_int() - Get integer bits per pixel value for the given DRM DSC config
+ * @vdsc_cfg: Pointer to DRM DSC config struct
+ *
+ * Return: Integer BPP value
+ */
+u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg)
+{
+	WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf);
+	return vdsc_cfg->bits_per_pixel >> 4;
+}
+EXPORT_SYMBOL(drm_dsc_get_bpp_int);
+
 /**
  * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config
  * @dsc: Pointer to DRM DSC config struct
diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h
index f4e18e5d077a..913aa2071232 100644
--- a/include/drm/display/drm_dsc_helper.h
+++ b/include/drm/display/drm_dsc_helper.h
@@ -27,6 +27,7 @@ int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params
 int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
 u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc);
 u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc);
+u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg);
 
 #endif /* _DRM_DSC_HELPER_H_ */
 

-- 
2.40.1

[PATCH v14 4/9] drm/msm/dsi: use DRM DSC helpers for DSC setup

[Jessica Zhang]

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Use new DRM DSC helpers to setup DSI DSC configuration. The
initial_scale_value needs to be adjusted according to the standard, but
this is a separate change.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 61 +++++---------------------------------
 1 file changed, 8 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 961689a255c4..74d38f90398a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1731,28 +1731,9 @@ static int dsi_host_parse_lane_data(struct msm_dsi_host *msm_host,
 	return -EINVAL;
 }
 
-static u32 dsi_dsc_rc_buf_thresh[DSC_NUM_BUF_RANGES - 1] = {
-	0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62,
-	0x69, 0x70, 0x77, 0x79, 0x7b, 0x7d, 0x7e
-};
-
-/* only 8bpc, 8bpp added */
-static char min_qp[DSC_NUM_BUF_RANGES] = {
-	0, 0, 1, 1, 3, 3, 3, 3, 3, 3, 5, 5, 5, 7, 13
-};
-
-static char max_qp[DSC_NUM_BUF_RANGES] = {
-	4, 4, 5, 6, 7, 7, 7, 8, 9, 10, 11, 12, 13, 13, 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 msm_dsi_host *msm_host, struct drm_dsc_config *dsc)
 {
-	int i;
-	u16 bpp = dsc->bits_per_pixel >> 4;
+	int ret;
 
 	if (dsc->bits_per_pixel & 0xf) {
 		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support fractional bits_per_pixel\n");
@@ -1764,49 +1745,23 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
 		return -EOPNOTSUPP;
 	}
 
-	dsc->rc_model_size = 8192;
-	dsc->first_line_bpg_offset = 12;
-	dsc->rc_edge_factor = 6;
-	dsc->rc_tgt_offset_high = 3;
-	dsc->rc_tgt_offset_low = 3;
 	dsc->simple_422 = 0;
 	dsc->convert_rgb = 1;
 	dsc->vbr_enable = 0;
 
-	/* handle only bpp = bpc = 8 */
-	for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
-		dsc->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
+	drm_dsc_set_const_params(dsc);
+	drm_dsc_set_rc_buf_thresh(dsc);
 
-	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
-		dsc->rc_range_params[i].range_min_qp = min_qp[i];
-		dsc->rc_range_params[i].range_max_qp = max_qp[i];
-		/*
-		 * Range BPG Offset contains two's-complement signed values that fill
-		 * 8 bits, yet the registers and DCS PPS field are only 6 bits wide.
-		 */
-		dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i] & DSC_RANGE_BPG_OFFSET_MASK;
+	/* handle only bpp = bpc = 8, pre-SCR panels */
+	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
+	if (ret) {
+		DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
+		return ret;
 	}
 
-	dsc->initial_offset = 6144;		/* Not bpp 12 */
-	if (bpp != 8)
-		dsc->initial_offset = 2048;	/* bpp = 12 */
-
-	if (dsc->bits_per_component <= 10)
-		dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
-	else
-		dsc->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC;
-
-	dsc->initial_xmit_delay = 512;
 	dsc->initial_scale_value = 32;
-	dsc->first_line_bpg_offset = 12;
 	dsc->line_buf_depth = dsc->bits_per_component + 1;
 
-	/* bpc 8 */
-	dsc->flatness_min_qp = 3;
-	dsc->flatness_max_qp = 12;
-	dsc->rc_quant_incr_limit0 = 11;
-	dsc->rc_quant_incr_limit1 = 11;
-
 	return drm_dsc_compute_rc_parameters(dsc);
 }
 

-- 
2.40.1

[PATCH v14 5/9] drm/msm: Add MSM-specific DSC helper methods

[Jessica Zhang]

Introduce MSM-specific DSC helper methods, as some calculations are
common between DP and DSC.

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/msm_dsc_helper.h | 38 ++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.h b/drivers/gpu/drm/msm/msm_dsc_helper.h
new file mode 100644
index 000000000000..b9049fe1e279
--- /dev/null
+++ b/drivers/gpu/drm/msm/msm_dsc_helper.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ *
+ * Helper methods for MSM-specific DSC calculations that are common between timing engine,
+ * DSI, and DP.
+ */
+
+#ifndef MSM_DSC_HELPER_H_
+#define MSM_DSC_HELPER_H_
+
+#include <linux/math.h>
+#include <drm/display/drm_dsc_helper.h>
+
+/**
+ * msm_dsc_get_slices_per_intf() - calculate number of slices per interface
+ * @dsc: Pointer to drm dsc config struct
+ * @intf_width: interface width in pixels
+ * Returns: Integer representing the number of slices for the given interface
+ */
+static inline u32 msm_dsc_get_slices_per_intf(const struct drm_dsc_config *dsc, u32 intf_width)
+{
+	return DIV_ROUND_UP(intf_width, dsc->slice_width);
+}
+
+/**
+ * msm_dsc_get_bytes_per_line() - calculate bytes per line
+ * @dsc: Pointer to drm dsc config struct
+ * Returns: Integer value representing bytes per line. DSI and DP need
+ *          to perform further calculations to turn this into pclk_per_intf,
+ *          such as dividing by different values depending on if widebus is enabled.
+ */
+static inline u32 msm_dsc_get_bytes_per_line(const struct drm_dsc_config *dsc)
+{
+	return dsc->slice_count * dsc->slice_chunk_size;
+}
+
+#endif /* MSM_DSC_HELPER_H_ */

-- 
2.40.1

[PATCH v14 6/9] drm/msm/dpu: Use fixed DRM DSC helper for det_thresh_flatness

[Jessica Zhang]

The current dpu_hw_dsc calculation for det_thresh_flatness does not
match the downstream calculation or the DSC spec.

Use the DRM DSC helper for det_thresh_flatness to match downstream
implementation and the DSC spec.

Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index 4e1396575e6a..3cad6a80af97 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -3,6 +3,8 @@
  * Copyright (c) 2020-2022, Linaro Limited
  */
 
+#include <drm/display/drm_dsc_helper.h>
+
 #include "dpu_kms.h"
 #include "dpu_hw_catalog.h"
 #include "dpu_hwio.h"
@@ -102,7 +104,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
 	data |= dsc->final_offset;
 	DPU_REG_WRITE(c, DSC_DSC_OFFSET, data);
 
-	det_thresh_flatness = 7 + 2 * (dsc->bits_per_component - 8);
+	det_thresh_flatness = drm_dsc_flatness_det_thresh(dsc);
 	data = det_thresh_flatness << 10;
 	data |= dsc->flatness_max_qp << 5;
 	data |= dsc->flatness_min_qp;

-- 
2.40.1

[PATCH v14 7/9] drm/msm/dpu: Fix slice_last_group_size calculation

[Jessica Zhang]

Correct the math for slice_last_group_size so that it matches the
calculations downstream.

Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index 3cad6a80af97..ea7d828b8812 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -56,9 +56,10 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
 	if (is_cmd_mode)
 		initial_lines += 1;
 
-	slice_last_group_size = 3 - (dsc->slice_width % 3);
+	slice_last_group_size = (dsc->slice_width + 2) % 3;
+
 	data = (initial_lines << 20);
-	data |= ((slice_last_group_size - 1) << 18);
+	data |= (slice_last_group_size << 18);
 	/* bpp is 6.4 format, 4 LSBs bits are for fractional part */
 	data |= (dsc->bits_per_pixel << 8);
 	data |= (dsc->block_pred_enable << 7);

-- 
2.40.1

[PATCH v14 8/9] drm/msm/dsi: Use MSM and DRM DSC helper methods

[Jessica Zhang]

Use MSM and DRM DSC helper methods to configure DSC for DSI.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 74d38f90398a..5526a51b3d97 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -28,6 +28,7 @@
 #include "dsi.xml.h"
 #include "sfpb.xml.h"
 #include "dsi_cfg.h"
+#include "msm_dsc_helper.h"
 #include "msm_kms.h"
 #include "msm_gem.h"
 #include "phy/dsi_phy.h"
@@ -848,7 +849,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
 	/* first calculate dsc parameters and then program
 	 * compress mode registers
 	 */
-	slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
+	slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
 
 	/*
 	 * If slice_count is greater than slice_per_intf
@@ -1759,7 +1760,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
 		return ret;
 	}
 
-	dsc->initial_scale_value = 32;
+	dsc->initial_scale_value = drm_dsc_initial_scale_value(dsc);
 	dsc->line_buf_depth = dsc->bits_per_component + 1;
 
 	return drm_dsc_compute_rc_parameters(dsc);

-- 
2.40.1

[PATCH v14 9/9] drm/msm/dsi: update hdisplay calculation for dsi_timing_setup

[Jessica Zhang]

Currently, hdisplay is being divided by 3 for DSC. However, this
calculation only works for cases where BPP = 8.

Update hdisplay calculation to be bytes_per_line / 3, so that it
accounts for cases where BPP != 8.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
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 5526a51b3d97..9223d7ec5a73 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 /= 3;
+		hdisplay = msm_dsc_get_bytes_per_line(msm_host->dsc) / 3;
 		h_total += hdisplay;
 		ha_end = ha_start + hdisplay;
 	}

-- 
2.40.1

Re: [PATCH v14 1/9] drm/display/dsc: Add flatness and initial scale value calculations

[Marijn Suijten]

On 2023-05-24 10:45:14, Jessica Zhang wrote:
> Add helpers to calculate det_thresh_flatness and initial_scale_value as
> these calculations are defined within the DSC spec.
> 
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/display/drm_dsc_helper.c | 24 ++++++++++++++++++++++++
>  include/drm/display/drm_dsc_helper.h     |  2 ++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c
> index fc187a8d8873..4efb6236d22c 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -1413,3 +1413,27 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_dsc_compute_rc_parameters);
> +
> +/**
> + * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config
> + * @dsc: Pointer to DRM DSC config struct
> + *
> + * Return: Calculated initial scale value

Perhaps just drop Calculated from Return:?

> + */
> +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc)
> +{
> +	return 8 * dsc->rc_model_size / (dsc->rc_model_size - dsc->initial_offset);
> +}
> +EXPORT_SYMBOL(drm_dsc_initial_scale_value);
> +
> +/**
> + * drm_dsc_flatness_det_thresh() - Calculate the flatness_det_thresh for the given DSC config

You've written out the word ("flatness det thresh" and "initial scale
value") entirely elsewhere, why the underscores in the doc comment here?

Instead we should have the full meaning here (and in the Return: below),
please correct me if I'm wrong but in VESA DSC v1.2a spec 6.8.5.1
Encoder Flatness Decision I think this variable means "flatness
determination threshold"?  If so, use that in the doc comment :)

(and drop the leading "the", so just "Calculate flatness determination
threshold for the given DSC config")

> + * @dsc: Pointer to DRM DSC config struct
> + *
> + * Return: Calculated flatness det thresh value

Nit: perhaps we can just drop "calculated" here?

- Marijn

> + */
> +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc)
> +{
> +	return 2 << (dsc->bits_per_component - 8);
> +}
> +EXPORT_SYMBOL(drm_dsc_flatness_det_thresh);
> diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h
> index fc2104415dcb..71789fb34e17 100644
> --- a/include/drm/display/drm_dsc_helper.h
> +++ b/include/drm/display/drm_dsc_helper.h
> @@ -24,6 +24,8 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
>  void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg);
>  int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params_type type);
>  int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
> +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc);
> +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc);
>  
>  #endif /* _DRM_DSC_HELPER_H_ */
>  
> 
> -- 
> 2.40.1
> 

Re: [PATCH v14 0/9] Introduce MSM-specific DSC helpers

[Marijn Suijten]

On 2023-05-24 10:45:13, Jessica Zhang wrote:
> There are some overlap in calculations for MSM-specific DSC variables
> between DP and DSI. In addition, the calculations for initial_scale_value
> and det_thresh_flatness that are defined within the DSC 1.2 specifications,
> but aren't yet included in drm_dsc_helper.c.
> 
> This series moves these calculations to a shared msm_dsc_helper.c file and
> defines drm_dsc_helper methods for initial_scale_value and
> det_thresh_flatness.
> 
> Note: For now, the MSM specific helper methods are only called for the DSI
> path, but will called for DP once DSC 1.2 support for DP has been added.
> 
> Depends on: "drm/i915: move DSC RC tables to drm_dsc_helper.c" [1]
> 
> [1] https://patchwork.freedesktop.org/series/114472/
> 
> ---
> Changes in v14:
> - Added kernel docs and made DRM DSC helper functions (Jani)

They were already helper functions: I think you meant to write that you
have moved from from inlined header functions to exported symbols in the
.c file?

- Marijn

> - Link to v13: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v13-0-d7581e7becec@quicinc.com
> 
> Changes in v13:
> - Reworded comment doc for msm_dsc_get_slices_per_intf()
> - Changed intf_width to u32
> - msm_dsc_calculate_slices_per_intf -> msm_dsc_get_slices_per_intf
> - Link to v12: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v12-0-9cdb7401f614@quicinc.com
> 
> Changes in v12:
> - Reworded summary comment for msm_dsc_helper.h
> - msm_dsc_get_slices_per_intf -> msm_dsc_calculate_slices_per_intf
> - Corrected total_bytes_per_intf math in dsc_update_dsc_timing
> - Rebased on top of latest version of "drm/i915: move DSC RC tables to drm_dsc_helper.c"
> - Link to v11: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v11-0-30270e1eeac3@quicinc.com
> 
> Changes in v11:
> - Fixed mismatched return types
> - Moved MSM DSC helpers summary comment to under copyright
> - Moved msm_dsc_get_bpp_int() to drm_dsc_helper.h
> - Reworded MSM DSC helper comment docs for clarity
> - Added const keyword where applicable
> - Renamed msm_dsc_get_slice_per_intf to msm_dsc_get_slices_per_intf
> - Removed msm_dsc_get_slice_per_intf()
> - Reworded commit message for "drm/msm/dsi: update hdisplay calculation
>   for dsi_timing_setup"
> - Link to v10: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v10-0-4cb21168c227@quicinc.com
> 
> Changes in v10:
> - Removed msm_dsc_get_bytes_per_slice helper
> - Inlined msm_dsc_get_bytes_per_intf
> - Refactored drm_dsc_set_initial_scale_value() to be a pure function
> - Renamed DRM DSC initial_scale and flatness_det_thresh helpers
> - Removed msm_dsc_helpers.o from Makefile
> - Link to v9: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v9-0-87daeaec2c0a@quicinc.com
> 
> Changes in v9:
> - Fixed incorrect math for msm_dsc_get_bytes_per_line()
> - Link to v8: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v8-0-2c9b2bb1209c@quicinc.com
> 
> Changes in v8:
> - *_bytes_per_soft_slice --> *_bytes_per_slice
> - Fixed comment doc formatting for MSM DSC helpers
> - Use slice_chunk_size in msm_dsc_get_bytes_per_line calculation
> - Reworded "drm/msm/dpu: Use DRM DSC helper for det_thresh_flatness"
>   commit title for clarity
> - Picked up "Reviewed-by" tags
> - Added duplicate Signed-off-by tag to "drm/display/dsc: Add flatness
>   and initial scale value calculations" to reflect patch history
> - Link to v7: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v7-0-df48a2c54421@quicinc.com
> 
> Changes in v7:
> - Renamed msm_dsc_get_pclk_per_intf to msm_dsc_get_bytes_per_line
> - Removed duplicate msm_dsc_get_dce_bytes_per_line
> - Reworded commit message for "drm/msm/dpu: Use DRM DSC helper for
>   det_thresh_flatness"
> - Dropped slice_per_pkt change (it will be included in the later
>   "Add DSC v1.2 Support for DSI" series)
> - Picked up "drm/display/dsc: Add flatness and initial scale value
>   calculations" and "drm/display/dsc: add helper to set semi-const
>   parameters", which were dropped from "drm/i915: move DSC RC tables to
>   drm_dsc_helper.c" series
> - Picked up "Reviewed-by" tags
> - Removed changelog in individual patches
> - Link to v6: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v6-0-cb7f59f0f7fb@quicinc.com
> 
> Changes in v6:
> - Documented return values for MSM DSC helpers
> - Fixed dependency issue in msm_dsc_helper.c
> - Link to v5: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v5-0-0108401d7886@quicinc.com
> 
> Changes in v5:
> - Added extra line at end of msm_dsc_helper.h
> - Simplified msm_dsc_get_bytes_per_soft_slice() math
> - Simplified and inlined msm_dsc_get_pclk_per_intf() math
> - "Fix calculations pkt_per_line" --> "... Fix calculation for pkt_per_line"
> - Split dsc->slice_width check into a separate patch
> - Picked up Dmitry's msm/dsi patch ("drm/msm/dsi: use new helpers for
>   DSC setup")
> - Removed unused headers in MSM DSC helper files
> - Picked up Reviewed-by tags
> - Link to v4: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v4-0-1b79c78b30d7@quicinc.com
> 
> Changes in v4:
> - Changed msm_dsc_get_uncompressed_pclk_per_intf to msm_dsc_get_pclk_per_intf
> - Moved pclk_per_intf calculation for dsi_timing_setup to `if
>   (msm_host->dsc)` block
> - Link to v3: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v3-0-6bec0d277a83@quicinc.com
> 
> Changes in v3:
> - Dropped src_bpp parameter from all methods -- src_bpp can be
>   calculated as dsc->bits_per_component * 3- Cleaned up unused parameters
> - Dropped intf_width parameter from get_bytes_per_soft_slice()
> - Moved dsc->bits_per_component to numerator calculation in
>   get_bytes_per_soft_slice()
> - Made get_bytes_per_soft_slice() a public method (this will be called
>   later to help calculate DP pclk params)- Added comment documentation to
>   MSM DSC helpers
> - Renamed msm_dsc_get_uncompressed_pclk_per_line to
>   *_get_uncompressed_pclk_per_intf()
> - Removed dsc->slice_width check from msm_dsc_get_uncompressed_pclk_per_intf()
> - Added documentation in comments
> - Moved extra_eol_bytes math out of msm_dsc_get_eol_byte_num()
> - Renamed msm_dsc_get_eol_byte_num to *_get_bytes_per_intf.
> - Reworded slice_last_group_size calculation to `(dsc->slice_width + 2) % 3`
> - Used MSM DSC helper to calculate total_bytes_per_intf
> - Initialized hdisplay as uncompressed pclk per line at the beginning of
>   dsi_timing_setup as to not break dual DSI calculations
> - Added slice_width check to dsi_timing_setup
> - Dropped 78c8b81d57d8 ("drm/display/dsc: Add flatness and initial scale
>   value calculations") patch as it was absorbed in Dmitry's DSC series [1]
> - Split dsi_timing_setup() hdisplay calculation to a separate patch
> - Link to v2: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v2-0-3c13ced536b2@quicinc.com
> 
> Changes in v2:
> - Changed det_thresh_flatness to flatness_det_thresh
> - Set initial_scale_value directly in helper
> - Moved msm_dsc_helper files to msm/ directory
> - Dropped get_comp_ratio() helper
> - Used drm_int2fixp() to convert to integers to fp
> - Fixed type mismatch issues in MSM DSC helpers
> - Changed DSC_BPP macro to drm_dsc_get_bpp_int() helper method
> - Style changes to improve readability
> - Dropped last division step of msm_dsc_get_pclk_per_line() and changed
>   method name accordingly
> - Dropped unused bpp variable in msm_dsc_get_dce_bytes_per_line()
> - Changed msm_dsc_get_slice_per_intf() to a static inline method
> - Split eol_byte_num and pkt_per_line calculation into a separate patch
> - Moved pclk_per_line calculation into `if (dsc)` block in
>   dsi_timing_setup()
> - *_calculate_initial_scale_value --> *_set_initial_scale_value
> - Picked up Fixes tags for patches 3/5 and 4/5
> - Picked up Reviewed-by for patch 4/5
> - Link to v1: https://lore.kernel.org/r/20230329-rfc-msm-dsc-helper-v1-0-f3e479f59b6d@quicinc.com
> 
> ---
> Dmitry Baryshkov (2):
>       drm/display/dsc: add helper to set semi-const parameters
>       drm/msm/dsi: use DRM DSC helpers for DSC setup
> 
> Jessica Zhang (7):
>       drm/display/dsc: Add flatness and initial scale value calculations
>       drm/display/dsc: Add drm_dsc_get_bpp_int helper
>       drm/msm: Add MSM-specific DSC helper methods
>       drm/msm/dpu: Use fixed DRM DSC helper for det_thresh_flatness
>       drm/msm/dpu: Fix slice_last_group_size calculation
>       drm/msm/dsi: Use MSM and DRM DSC helper methods
>       drm/msm/dsi: update hdisplay calculation for dsi_timing_setup
> 
>  drivers/gpu/drm/display/drm_dsc_helper.c   | 59 ++++++++++++++++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c |  9 ++--
>  drivers/gpu/drm/msm/dsi/dsi_host.c         | 68 ++++++------------------------
>  drivers/gpu/drm/msm/msm_dsc_helper.h       | 38 +++++++++++++++++
>  include/drm/display/drm_dsc_helper.h       |  4 ++
>  5 files changed, 119 insertions(+), 59 deletions(-)
> ---
> base-commit: c0c7f04818136b3204589da42b4532b5aa3d4971
> change-id: 20230329-rfc-msm-dsc-helper-981a95edfbd0
> 
> Best regards,
> -- 
> Jessica Zhang <quic_jesszhan@quicinc.com>
> 

Re: [PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper

[Marijn Suijten]

On 2023-05-24 10:45:16, Jessica Zhang wrote:
> Add helper to get the integer value of drm_dsc_config.bits_per_pixel
> 
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>

Nit: if there comes a v15, perhaps this can be squashed with patch 1/9?
I always thought they were separate because one extends the header while
this extends the C file... but now both extend the C file with helpers.

> ---
>  drivers/gpu/drm/display/drm_dsc_helper.c | 13 +++++++++++++
>  include/drm/display/drm_dsc_helper.h     |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c
> index b31fe9849784..4424380c6cb6 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -1436,6 +1436,19 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
>  }
>  EXPORT_SYMBOL(drm_dsc_compute_rc_parameters);
>  
> +/**
> + * drm_dsc_get_bpp_int() - Get integer bits per pixel value for the given DRM DSC config
> + * @vdsc_cfg: Pointer to DRM DSC config struct
> + *
> + * Return: Integer BPP value
> + */
> +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg)
> +{
> +	WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf);

You did not add linux/bug.h back, presumably because Dmitry added
another use of WARN_ON_ONCE to this file in a previous series and it
compiles fine as the definition trickles in via another header?

- Marijn

> +	return vdsc_cfg->bits_per_pixel >> 4;
> +}
> +EXPORT_SYMBOL(drm_dsc_get_bpp_int);
> +
>  /**
>   * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config
>   * @dsc: Pointer to DRM DSC config struct
> diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h
> index f4e18e5d077a..913aa2071232 100644
> --- a/include/drm/display/drm_dsc_helper.h
> +++ b/include/drm/display/drm_dsc_helper.h
> @@ -27,6 +27,7 @@ int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params
>  int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
>  u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc);
>  u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc);
> +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg);
>  
>  #endif /* _DRM_DSC_HELPER_H_ */
>  
> 
> -- 
> 2.40.1
> 

Re: [PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper

[Jessica Zhang]

On 5/24/2023 12:14 PM, Marijn Suijten wrote:
> On 2023-05-24 10:45:16, Jessica Zhang wrote:
>> Add helper to get the integer value of drm_dsc_config.bits_per_pixel
>>
>> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> 
> Nit: if there comes a v15, perhaps this can be squashed with patch 1/9?
> I always thought they were separate because one extends the header while
> this extends the C file... but now both extend the C file with helpers.

Hi Marijn,

Sure, will squash this if there is a v15.

> 
>> ---
>>   drivers/gpu/drm/display/drm_dsc_helper.c | 13 +++++++++++++
>>   include/drm/display/drm_dsc_helper.h     |  1 +
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c
>> index b31fe9849784..4424380c6cb6 100644
>> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
>> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
>> @@ -1436,6 +1436,19 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
>>   }
>>   EXPORT_SYMBOL(drm_dsc_compute_rc_parameters);
>>   
>> +/**
>> + * drm_dsc_get_bpp_int() - Get integer bits per pixel value for the given DRM DSC config
>> + * @vdsc_cfg: Pointer to DRM DSC config struct
>> + *
>> + * Return: Integer BPP value
>> + */
>> +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg)
>> +{
>> +	WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf);
> 
> You did not add linux/bug.h back, presumably because Dmitry added
> another use of WARN_ON_ONCE to this file in a previous series and it
> compiles fine as the definition trickles in via another header?

Yep, this compiles fine without any error or warning.

Thanks,

Jessica Zhang

> 
> - Marijn
> 
>> +	return vdsc_cfg->bits_per_pixel >> 4;
>> +}
>> +EXPORT_SYMBOL(drm_dsc_get_bpp_int);
>> +
>>   /**
>>    * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config
>>    * @dsc: Pointer to DRM DSC config struct
>> diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h
>> index f4e18e5d077a..913aa2071232 100644
>> --- a/include/drm/display/drm_dsc_helper.h
>> +++ b/include/drm/display/drm_dsc_helper.h
>> @@ -27,6 +27,7 @@ int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params
>>   int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
>>   u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc);
>>   u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc);
>> +u32 drm_dsc_get_bpp_int(const struct drm_dsc_config *vdsc_cfg);
>>   
>>   #endif /* _DRM_DSC_HELPER_H_ */
>>   
>>
>> -- 
>> 2.40.1
>>

Re: [PATCH v14 1/9] drm/display/dsc: Add flatness and initial scale value calculations

[Jessica Zhang]

On 5/24/2023 12:05 PM, Marijn Suijten wrote:
> On 2023-05-24 10:45:14, Jessica Zhang wrote:
>> Add helpers to calculate det_thresh_flatness and initial_scale_value as
>> these calculations are defined within the DSC spec.
>>
>> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   drivers/gpu/drm/display/drm_dsc_helper.c | 24 ++++++++++++++++++++++++
>>   include/drm/display/drm_dsc_helper.h     |  2 ++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c b/drivers/gpu/drm/display/drm_dsc_helper.c
>> index fc187a8d8873..4efb6236d22c 100644
>> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
>> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
>> @@ -1413,3 +1413,27 @@ int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg)
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL(drm_dsc_compute_rc_parameters);
>> +
>> +/**
>> + * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config
>> + * @dsc: Pointer to DRM DSC config struct
>> + *
>> + * Return: Calculated initial scale value
> 
> Perhaps just drop Calculated from Return:?
> 
>> + */
>> +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc)
>> +{
>> +	return 8 * dsc->rc_model_size / (dsc->rc_model_size - dsc->initial_offset);
>> +}
>> +EXPORT_SYMBOL(drm_dsc_initial_scale_value);
>> +
>> +/**
>> + * drm_dsc_flatness_det_thresh() - Calculate the flatness_det_thresh for the given DSC config
> 
> You've written out the word ("flatness det thresh" and "initial scale
> value") entirely elsewhere, why the underscores in the doc comment here?
> 
> Instead we should have the full meaning here (and in the Return: below),
> please correct me if I'm wrong but in VESA DSC v1.2a spec 6.8.5.1
> Encoder Flatness Decision I think this variable means "flatness
> determination threshold"?  If so, use that in the doc comment :)
> 
> (and drop the leading "the", so just "Calculate flatness determination
> threshold for the given DSC config")
> 
>> + * @dsc: Pointer to DRM DSC config struct
>> + *
>> + * Return: Calculated flatness det thresh value
> 
> Nit: perhaps we can just drop "calculated" here?


Hi Marijn,

Sure, I will make these changes if a v15 is necessary.

In the future, can we try to group comments on wording/grammar/patch 
formatting with comments on the code itself?

I really appreciate your feedback and help in improving the 
documentation around this feature, however I don't find it very 
productive to have revisions where the only changes are on (in my 
opinion) small wording details.

Thanks,

Jessica Zhang

> 
> - Marijn
> 
>> + */
>> +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc)
>> +{
>> +	return 2 << (dsc->bits_per_component - 8);
>> +}
>> +EXPORT_SYMBOL(drm_dsc_flatness_det_thresh);
>> diff --git a/include/drm/display/drm_dsc_helper.h b/include/drm/display/drm_dsc_helper.h
>> index fc2104415dcb..71789fb34e17 100644
>> --- a/include/drm/display/drm_dsc_helper.h
>> +++ b/include/drm/display/drm_dsc_helper.h
>> @@ -24,6 +24,8 @@ void drm_dsc_pps_payload_pack(struct drm_dsc_picture_parameter_set *pps_sdp,
>>   void drm_dsc_set_rc_buf_thresh(struct drm_dsc_config *vdsc_cfg);
>>   int drm_dsc_setup_rc_params(struct drm_dsc_config *vdsc_cfg, enum drm_dsc_params_type type);
>>   int drm_dsc_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg);
>> +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc);
>> +u32 drm_dsc_flatness_det_thresh(const struct drm_dsc_config *dsc);
>>   
>>   #endif /* _DRM_DSC_HELPER_H_ */
>>   
>>
>> -- 
>> 2.40.1
>>

Re: [PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper

[Marijn Suijten]

On 2023-05-24 15:38:23, Jessica Zhang wrote:
<snip>
> >> +	WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf);
> > 
> > You did not add linux/bug.h back, presumably because Dmitry added
> > another use of WARN_ON_ONCE to this file in a previous series and it
> > compiles fine as the definition trickles in via another header?
> 
> Yep, this compiles fine without any error or warning.

Yes it does, just curious (CC Dmitry) if that is expected/intended: I am
not familiar enough with the current header includes to say for sure.

Dmitry seemed to rely on it already being available in
https://git.kernel.org/torvalds/c/2b470e5531f57c1b9bfa129cca0ee17a2ecd2183
but that could have been an oversight?

- Marijn

Re: [PATCH v14 1/9] drm/display/dsc: Add flatness and initial scale value calculations

[Marijn Suijten]

On 2023-05-24 18:05:51, Jessica Zhang wrote:
<snip>
> >> +/**
> >> + * drm_dsc_initial_scale_value() - Calculate the initial scale value for the given DSC config
> >> + * @dsc: Pointer to DRM DSC config struct
> >> + *
> >> + * Return: Calculated initial scale value
> > 
> > Perhaps just drop Calculated from Return:?
> > 
> >> + */
> >> +u8 drm_dsc_initial_scale_value(const struct drm_dsc_config *dsc)
> >> +{
> >> +	return 8 * dsc->rc_model_size / (dsc->rc_model_size - dsc->initial_offset);
> >> +}
> >> +EXPORT_SYMBOL(drm_dsc_initial_scale_value);
> >> +
> >> +/**
> >> + * drm_dsc_flatness_det_thresh() - Calculate the flatness_det_thresh for the given DSC config
> > 
> > You've written out the word ("flatness det thresh" and "initial scale
> > value") entirely elsewhere, why the underscores in the doc comment here?
> > 
> > Instead we should have the full meaning here (and in the Return: below),
> > please correct me if I'm wrong but in VESA DSC v1.2a spec 6.8.5.1
> > Encoder Flatness Decision I think this variable means "flatness
> > determination threshold"?  If so, use that in the doc comment :)
> > 
> > (and drop the leading "the", so just "Calculate flatness determination
> > threshold for the given DSC config")
> > 
> >> + * @dsc: Pointer to DRM DSC config struct
> >> + *
> >> + * Return: Calculated flatness det thresh value
> > 
> > Nit: perhaps we can just drop "calculated" here?
> 
> 
> Hi Marijn,
> 
> Sure, I will make these changes if a v15 is necessary.
> 
> In the future, can we try to group comments on wording/grammar/patch 
> formatting with comments on the code itself?

Can you clarify what you mean?  v14 here is the first series including
this doc comment so there was no way for me to have reviewed this
earlier.  Code contents were already successfully reviewed many
revisions ago.

> I really appreciate your feedback and help in improving the 
> documentation around this feature, however I don't find it very 
> productive to have revisions where the only changes are on (in my 
> opinion) small wording details.

It is also down to you to have some patience and collect more review
from other maintainers and batch up changes, instead of spinning another
revision quickly after a review comment.

But this request can also be turned around: review and scan your own
series for simple inconsistencies before sending it to the lists, that
will surely make the time spent by reviewers much more "productive" as
well.
(Note that this goes hand in hand with the request to slow down
 consecutive revisions!)

And finally, as already said before: you can always decide to ignore my
review nits.  I am not a maintainer and don't have final say on whatever
is blocking for a patch to get merged.
But, when another revision is needed, the things I pointed out can at
least be incorporated, which is why they were shared in the first place.

Thanks for understanding.

- Marijn

Re: [PATCH v14 3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper

[Dmitry Baryshkov]

On Thu, 25 May 2023 at 23:18, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2023-05-24 15:38:23, Jessica Zhang wrote:
> <snip>
> > >> +  WARN_ON_ONCE(vdsc_cfg->bits_per_pixel & 0xf);
> > >
> > > You did not add linux/bug.h back, presumably because Dmitry added
> > > another use of WARN_ON_ONCE to this file in a previous series and it
> > > compiles fine as the definition trickles in via another header?
> >
> > Yep, this compiles fine without any error or warning.
>
> Yes it does, just curious (CC Dmitry) if that is expected/intended: I am
> not familiar enough with the current header includes to say for sure.
>
> Dmitry seemed to rely on it already being available in
> https://git.kernel.org/torvalds/c/2b470e5531f57c1b9bfa129cca0ee17a2ecd2183

I think I did not care about including <linux/bug.h> I checked that
current set of headers provides WARN_ON_ONCE, that's all.

> but that could have been an oversight?
>
> - Marijn



-- 
With best wishes
Dmitry

Re: [PATCH v14 0/9] Introduce MSM-specific DSC helpers

[Dmitry Baryshkov]

On Wed, 24 May 2023 10:45:13 -0700, Jessica Zhang wrote:
> There are some overlap in calculations for MSM-specific DSC variables
> between DP and DSI. In addition, the calculations for initial_scale_value
> and det_thresh_flatness that are defined within the DSC 1.2 specifications,
> but aren't yet included in drm_dsc_helper.c.
> 
> This series moves these calculations to a shared msm_dsc_helper.c file and
> defines drm_dsc_helper methods for initial_scale_value and
> det_thresh_flatness.
> 
> [...]

Applied, thanks!

[1/9] drm/display/dsc: Add flatness and initial scale value calculations
      https://gitlab.freedesktop.org/lumag/msm/-/commit/7df1ed6ddf3d
[2/9] drm/display/dsc: add helper to set semi-const parameters
      https://gitlab.freedesktop.org/lumag/msm/-/commit/e871a70d8ccd
[3/9] drm/display/dsc: Add drm_dsc_get_bpp_int helper
      https://gitlab.freedesktop.org/lumag/msm/-/commit/688583281241
[4/9] drm/msm/dsi: use DRM DSC helpers for DSC setup
      https://gitlab.freedesktop.org/lumag/msm/-/commit/49fd30a7153b
[5/9] drm/msm: Add MSM-specific DSC helper methods
      https://gitlab.freedesktop.org/lumag/msm/-/commit/b50f06f83e0e
[6/9] drm/msm/dpu: Use fixed DRM DSC helper for det_thresh_flatness
      https://gitlab.freedesktop.org/lumag/msm/-/commit/44346191a210
[7/9] drm/msm/dpu: Fix slice_last_group_size calculation
      https://gitlab.freedesktop.org/lumag/msm/-/commit/c223059e6f83
[8/9] drm/msm/dsi: Use MSM and DRM DSC helper methods
      https://gitlab.freedesktop.org/lumag/msm/-/commit/ed1498f77419
[9/9] drm/msm/dsi: update hdisplay calculation for dsi_timing_setup
      https://gitlab.freedesktop.org/lumag/msm/-/commit/149419396a92

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>