Re: [Freedreno] [PATCH RFC 2/5] drm/msm: Add MSM-specific DSC helper methods
On 3/29/2023 5:40 PM, Dmitry Baryshkov wrote:
On 30/03/2023 02:18, Jessica Zhang wrote:
Introduce MSM-specific DSC helper methods, as some calculations are
common between DP and DSC.
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74
+++
drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28
3 files changed, 103 insertions(+)
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 7274c41228ed..897a5b1c88f6 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -90,6 +90,7 @@ msm-y += \
disp/mdp_kms.o \
disp/msm_disp_snapshot.o \
disp/msm_disp_snapshot_util.o \
+ disp/msm_dsc_helper.o \
msm_atomic.o \
msm_atomic_tracepoints.o \
msm_debugfs.o \
diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
new file mode 100644
index ..ec15c0d829e8
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights
reserved
+ */
+
+#include
+#include
+#include
+
+#include "msm_drv.h"
+#include "msm_dsc_helper.h"
+
+static int get_comp_ratio(struct drm_dsc_config *dsc, u32 src_bpp)
+{
+ return mult_frac(100, src_bpp, DSC_BPP(*dsc));
+}
+
+static s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc, int
intf_width, int comp_ratio)
+{
+ s64 comp_ratio_fp, num_bits_fp;
+ s64 numerator_fp, denominator_fp;
+
+ comp_ratio_fp = drm_fixp_from_fraction(comp_ratio, 100);
Please inline comp_ration calculation here. Don't use mult_frac().
+ num_bits_fp = drm_fixp_from_fraction(8, 1);
drm_int2fixp
+
+ numerator_fp = drm_fixp_from_fraction(dsc->slice_width *
dsc->bits_per_component * 3, 1);
And here too.
+ denominator_fp = drm_fixp_mul(comp_ratio_fp, num_bits_fp);
And num_bits_fp can be inlined too.
denominator_fp = drm_fixp_from_fraction(src_bpp * 8, DSC_BPP)
+
+ return drm_fixp_div(numerator_fp, denominator_fp);
dsc->slice_width * bpc * 3 / (8 * src_bpp / DSC_BPP), thus:
drm_fixp_from_fraction(dsc->slice_width * bpc * 3 * DSC_BPP, 8 * src_bpp)
but I will not insist on this one.
+}
+
+u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int
intf_width, u32 src_bpp)
+{
+ u32 bytes_per_ss, extra_eol_bytes, bytes_per_intf;
+ s64 bytes_per_ss_fp;
+ int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width);
+ int comp_ratio = get_comp_ratio(dsc, src_bpp);
+
+ bytes_per_ss_fp = get_bytes_per_soft_slice(dsc, intf_width,
comp_ratio);
+ bytes_per_ss = drm_fixp2int_ceil(bytes_per_ss_fp);
s/_ss/_soft_slice/g
+
+ bytes_per_intf = bytes_per_ss * slice_per_intf;
+ extra_eol_bytes = bytes_per_intf % 3;
+ if (extra_eol_bytes != 0)
+ extra_eol_bytes = 3 - extra_eol_bytes;
+
+ return extra_eol_bytes;
+}
+
+u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int
intf_width)
+{
+ u32 bpp;
+ u32 dce_bytes_per_line;
+
+ bpp = DSC_BPP(*dsc);
Didn't this cause a warning on the unused-but-set variable?
+ dce_bytes_per_line = DIV_ROUND_UP(dsc->bits_per_pixel *
intf_width, 8);
+
+ return dce_bytes_per_line;
+}
If you have msm_dsc_get_slice_per_intf() as a static inline, this
function can be a static inline too. Nothing more than a single
DIV_ROUND_UP.
+
+int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int
intf_width, u32 src_bpp)
+{
+ s64 data_width;
+ int comp_ratio = get_comp_ratio(dsc, src_bpp);
+
+ if (!dsc->slice_width || (intf_width < dsc->slice_width))
+ return -EINVAL;
+
+ data_width = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio);
+ data_width = drm_fixp_mul(dsc->slice_count, data_width);
+ data_width = drm_fixp_from_fraction(data_width, 3);
Reusing a variable is a nice trick, but it can be confusing. Not to
mention that the last call should probably be drm_fixp_div()
Hi Dmitry,
Acked (for all the comments here).
Planning to move the last divide by 3 out of this method (as the value
that uncompressed pclk is divided by depends on DSI/DP and if widebus is
enabled), so I'll merge the get_bytes_per_soft_slice call with the 2nd line.
Thanks,
Jessica Zhang
+
+ return drm_fixp2int_ceil(data_width);
+}
diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
new file mode 100644
index ..308069b2b5a4
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights
reserved
+ */
+
+#ifndef MSM_DSC_HELPER_H_
+#define MSM_DSC_HELPER_H_
+
+#include
+#include
+
+/*
+ * Helper methods for MSM specific DSC calculations that are common
between timing engine,
+ * DSI,
Re: [PATCH RFC 2/5] drm/msm: Add MSM-specific DSC helper methods
On 30/03/2023 02:18, Jessica Zhang wrote: Introduce MSM-specific DSC helper methods, as some calculations are common between DP and DSC. Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74 +++ drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28 Also: outside of disp, please. 3 files changed, 103 insertions(+) -- With best wishes Dmitry
Re: [PATCH RFC 2/5] drm/msm: Add MSM-specific DSC helper methods
On 30/03/2023 02:18, Jessica Zhang wrote:
Introduce MSM-specific DSC helper methods, as some calculations are
common between DP and DSC.
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74 +++
drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28
3 files changed, 103 insertions(+)
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 7274c41228ed..897a5b1c88f6 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -90,6 +90,7 @@ msm-y += \
disp/mdp_kms.o \
disp/msm_disp_snapshot.o \
disp/msm_disp_snapshot_util.o \
+ disp/msm_dsc_helper.o \
msm_atomic.o \
msm_atomic_tracepoints.o \
msm_debugfs.o \
diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
new file mode 100644
index ..ec15c0d829e8
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#include
+#include
+#include
+
+#include "msm_drv.h"
+#include "msm_dsc_helper.h"
+
+static int get_comp_ratio(struct drm_dsc_config *dsc, u32 src_bpp)
+{
+ return mult_frac(100, src_bpp, DSC_BPP(*dsc));
+}
+
+static s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc, int
intf_width, int comp_ratio)
+{
+ s64 comp_ratio_fp, num_bits_fp;
+ s64 numerator_fp, denominator_fp;
+
+ comp_ratio_fp = drm_fixp_from_fraction(comp_ratio, 100);
Please inline comp_ration calculation here. Don't use mult_frac().
+ num_bits_fp = drm_fixp_from_fraction(8, 1);
drm_int2fixp
+
+ numerator_fp = drm_fixp_from_fraction(dsc->slice_width *
dsc->bits_per_component * 3, 1);
And here too.
+ denominator_fp = drm_fixp_mul(comp_ratio_fp, num_bits_fp);
And num_bits_fp can be inlined too.
denominator_fp = drm_fixp_from_fraction(src_bpp * 8, DSC_BPP)
+
+ return drm_fixp_div(numerator_fp, denominator_fp);
dsc->slice_width * bpc * 3 / (8 * src_bpp / DSC_BPP), thus:
drm_fixp_from_fraction(dsc->slice_width * bpc * 3 * DSC_BPP, 8 * src_bpp)
but I will not insist on this one.
+}
+
+u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int intf_width, u32
src_bpp)
+{
+ u32 bytes_per_ss, extra_eol_bytes, bytes_per_intf;
+ s64 bytes_per_ss_fp;
+ int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width);
+ int comp_ratio = get_comp_ratio(dsc, src_bpp);
+
+ bytes_per_ss_fp = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio);
+ bytes_per_ss = drm_fixp2int_ceil(bytes_per_ss_fp);
s/_ss/_soft_slice/g
+
+ bytes_per_intf = bytes_per_ss * slice_per_intf;
+ extra_eol_bytes = bytes_per_intf % 3;
+ if (extra_eol_bytes != 0)
+ extra_eol_bytes = 3 - extra_eol_bytes;
+
+ return extra_eol_bytes;
+}
+
+u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int intf_width)
+{
+ u32 bpp;
+ u32 dce_bytes_per_line;
+
+ bpp = DSC_BPP(*dsc);
Didn't this cause a warning on the unused-but-set variable?
+ dce_bytes_per_line = DIV_ROUND_UP(dsc->bits_per_pixel * intf_width, 8);
+
+ return dce_bytes_per_line;
+}
If you have msm_dsc_get_slice_per_intf() as a static inline, this
function can be a static inline too. Nothing more than a single
DIV_ROUND_UP.
+
+int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int intf_width, u32
src_bpp)
+{
+ s64 data_width;
+ int comp_ratio = get_comp_ratio(dsc, src_bpp);
+
+ if (!dsc->slice_width || (intf_width < dsc->slice_width))
+ return -EINVAL;
+
+ data_width = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio);
+ data_width = drm_fixp_mul(dsc->slice_count, data_width);
+ data_width = drm_fixp_from_fraction(data_width, 3);
Reusing a variable is a nice trick, but it can be confusing. Not to
mention that the last call should probably be drm_fixp_div()
+
+ return drm_fixp2int_ceil(data_width);
+}
diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
new file mode 100644
index ..308069b2b5a4
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#ifndef MSM_DSC_HELPER_H_
+#define MSM_DSC_HELPER_H_
+
+#include
+#include
+
+/*
+ * Helper methods for MSM specific DSC calculations that are common between
timing engine,
+ * DSI, and DP.
+ */
+
+#define MSM_DSC_SLICE_PER_PKT 1
+#define DSC_BPP(config) ((config).bits_per_pixel >> 4)
Oh. Please. If you have used (config)->bits_per_pixel here, you wouldn't
have to use clumsy DSC_BPP(*dsc). It might make sense to add:
[PATCH RFC 2/5] drm/msm: Add MSM-specific DSC helper methods
Introduce MSM-specific DSC helper methods, as some calculations are
common between DP and DSC.
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74 +++
drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28
3 files changed, 103 insertions(+)
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 7274c41228ed..897a5b1c88f6 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -90,6 +90,7 @@ msm-y += \
disp/mdp_kms.o \
disp/msm_disp_snapshot.o \
disp/msm_disp_snapshot_util.o \
+ disp/msm_dsc_helper.o \
msm_atomic.o \
msm_atomic_tracepoints.o \
msm_debugfs.o \
diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
new file mode 100644
index ..ec15c0d829e8
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#include
+#include
+#include
+
+#include "msm_drv.h"
+#include "msm_dsc_helper.h"
+
+static int get_comp_ratio(struct drm_dsc_config *dsc, u32 src_bpp)
+{
+ return mult_frac(100, src_bpp, DSC_BPP(*dsc));
+}
+
+static s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc, int
intf_width, int comp_ratio)
+{
+ s64 comp_ratio_fp, num_bits_fp;
+ s64 numerator_fp, denominator_fp;
+
+ comp_ratio_fp = drm_fixp_from_fraction(comp_ratio, 100);
+ num_bits_fp = drm_fixp_from_fraction(8, 1);
+
+ numerator_fp = drm_fixp_from_fraction(dsc->slice_width *
dsc->bits_per_component * 3, 1);
+ denominator_fp = drm_fixp_mul(comp_ratio_fp, num_bits_fp);
+
+ return drm_fixp_div(numerator_fp, denominator_fp);
+}
+
+u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int intf_width, u32
src_bpp)
+{
+ u32 bytes_per_ss, extra_eol_bytes, bytes_per_intf;
+ s64 bytes_per_ss_fp;
+ int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width);
+ int comp_ratio = get_comp_ratio(dsc, src_bpp);
+
+ bytes_per_ss_fp = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio);
+ bytes_per_ss = drm_fixp2int_ceil(bytes_per_ss_fp);
+
+ bytes_per_intf = bytes_per_ss * slice_per_intf;
+ extra_eol_bytes = bytes_per_intf % 3;
+ if (extra_eol_bytes != 0)
+ extra_eol_bytes = 3 - extra_eol_bytes;
+
+ return extra_eol_bytes;
+}
+
+u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int intf_width)
+{
+ u32 bpp;
+ u32 dce_bytes_per_line;
+
+ bpp = DSC_BPP(*dsc);
+ dce_bytes_per_line = DIV_ROUND_UP(dsc->bits_per_pixel * intf_width, 8);
+
+ return dce_bytes_per_line;
+}
+
+int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int intf_width, u32
src_bpp)
+{
+ s64 data_width;
+ int comp_ratio = get_comp_ratio(dsc, src_bpp);
+
+ if (!dsc->slice_width || (intf_width < dsc->slice_width))
+ return -EINVAL;
+
+ data_width = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio);
+ data_width = drm_fixp_mul(dsc->slice_count, data_width);
+ data_width = drm_fixp_from_fraction(data_width, 3);
+
+ return drm_fixp2int_ceil(data_width);
+}
diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
new file mode 100644
index ..308069b2b5a4
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#ifndef MSM_DSC_HELPER_H_
+#define MSM_DSC_HELPER_H_
+
+#include
+#include
+
+/*
+ * Helper methods for MSM specific DSC calculations that are common between
timing engine,
+ * DSI, and DP.
+ */
+
+#define MSM_DSC_SLICE_PER_PKT 1
+#define DSC_BPP(config) ((config).bits_per_pixel >> 4)
+
+static inline int msm_dsc_get_slice_per_intf(struct drm_dsc_config *dsc, int
intf_width)
+{
+ return DIV_ROUND_UP(intf_width, dsc->slice_width);
+}
+
+u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int intf_width, u32
src_bpp);
+u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int intf_width);
+int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int intf_width, u32
src_bpp);
+#endif /* MSM_DSC_HELPER_H_ */
--
2.39.2
