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.suij...@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
Signed-off-by: Jessica Zhang <quic_jessz...@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

Reply via email to