Re: [PATCH v3 1/6] drm/msm: Add MSM-specific DSC helper methods

2023-04-04 Thread Abhinav Kumar




On 4/4/2023 4:56 PM, Jessica Zhang wrote:

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

Changes in v2:
- Moved files up to msm/ directory
- Dropped get_comp_ratio() helper
- Used drm_int2fixp() to convert to integers to fp
- Style changes to improve readability
- Dropped unused bpp variable in msm_dsc_get_dce_bytes_per_line()
- Changed msm_dsc_get_slice_per_intf() to a static inline method
- Dropped last division step of msm_dsc_get_pclk_per_line() and changed
   method name accordingly
- Changed DSC_BPP macro to drm_dsc_get_bpp_int() helper method
- Fixed some math issues caused by passing in incorrect types to
   drm_fixed methods in get_bytes_per_soft_slice()

Changes in v3:
- Dropped src_bpp parameter from all methods -- src_bpp can be
   calculated as dsc->bits_per_component * 3
- Dropped intf_width parameter from get_bytes_per_soft_slice()
- Moved dsc->bits_per_component to numerator calculation in
   get_bytes_per_soft_slice()
- 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()
- Made get_bytes_per_soft_slice() a public method (this will be called
   later to help calculate DP pclk params)
- Added documentation in comments
- Moved extra_eol_bytes math out of msm_dsc_get_eol_byte_num() and
   renamed msm_dsc_get_eol_byte_num to *_get_bytes_per_intf.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/msm/Makefile |  1 +
  drivers/gpu/drm/msm/msm_dsc_helper.c | 47 
  drivers/gpu/drm/msm/msm_dsc_helper.h | 70 
  3 files changed, 118 insertions(+)

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 7274c41228ed..b814fc80e2d5 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -94,6 +94,7 @@ msm-y += \
msm_atomic_tracepoints.o \
msm_debugfs.o \
msm_drv.o \
+   msm_dsc_helper.o \
msm_fb.o \
msm_fence.o \
msm_gem.o \
diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.c 
b/drivers/gpu/drm/msm/msm_dsc_helper.c
new file mode 100644
index ..c8c530211f50
--- /dev/null
+++ b/drivers/gpu/drm/msm/msm_dsc_helper.c
@@ -0,0 +1,47 @@
+// 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"
+
+s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc)
+{
+   int bpp = msm_dsc_get_bpp_int(dsc);
+   s64 numerator_fp, denominator_fp;
+   s64 comp_ratio_fp = drm_fixp_from_fraction(dsc->bits_per_component * 3, 
bpp);
+
+   numerator_fp = drm_int2fixp(dsc->slice_width * 3 * 
dsc->bits_per_component);
+   denominator_fp = drm_fixp_mul(comp_ratio_fp, drm_int2fixp(8));
+
+   return drm_fixp_div(numerator_fp, denominator_fp);
+}
+
+u32 msm_dsc_get_bytes_per_intf(struct drm_dsc_config *dsc, int intf_width)
+{
+   u32 bytes_per_soft_slice, bytes_per_intf;
+   s64 bytes_per_soft_slice_fp;
+   int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width);
+
+   bytes_per_soft_slice_fp = get_bytes_per_soft_slice(dsc);
+   bytes_per_soft_slice = drm_fixp2int_ceil(bytes_per_soft_slice_fp);
+
+   bytes_per_intf = bytes_per_soft_slice * slice_per_intf;
+
+   return bytes_per_intf;
+}
+
+int msm_dsc_get_uncompressed_pclk_per_intf(struct drm_dsc_config *dsc)
+{
+   s64 data_width;
+
+   data_width = drm_fixp_mul(drm_int2fixp(dsc->slice_count),
+   get_bytes_per_soft_slice(dsc));
+
+   return drm_fixp2int_ceil(data_width);
+}


This is not really uncompressed pclk_per_intf. This is still calculated 
using the compression ratio so it is still compressed.


I would suggest changing this to msm_dsc_get_pclk_per_intf().

Lets keep it simple.


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 ..5ee972eb247c
--- /dev/null
+++ b/drivers/gpu/drm/msm/msm_dsc_helper.h
@@ -0,0 +1,70 @@
+/* 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.
+ */
+
+/**
+ * msm_dsc_get_bpp_int - get bits per pixel integer value
+ * @dsc: Pointer to drm dsc config struct
+ */
+static inline int msm_dsc_get_bpp_int(struct drm_dsc_config *dsc)
+{
+   WARN_ON_ONCE(dsc->bits_per_pixel & 0xf);
+   return dsc->bits_per_pixel >> 4;
+}
+
+/**
+ * msm_dsc_get_slice_per_intf - get number of slices per interface
+ * @dsc: Pointer to drm dsc config struct
+ * @intf_width: interface width
+ */
+static inline int 

[PATCH v3 1/6] drm/msm: Add MSM-specific DSC helper methods

2023-04-04 Thread Jessica Zhang
Introduce MSM-specific DSC helper methods, as some calculations are
common between DP and DSC.

Changes in v2:
- Moved files up to msm/ directory
- Dropped get_comp_ratio() helper
- Used drm_int2fixp() to convert to integers to fp
- Style changes to improve readability
- Dropped unused bpp variable in msm_dsc_get_dce_bytes_per_line()
- Changed msm_dsc_get_slice_per_intf() to a static inline method
- Dropped last division step of msm_dsc_get_pclk_per_line() and changed
  method name accordingly
- Changed DSC_BPP macro to drm_dsc_get_bpp_int() helper method
- Fixed some math issues caused by passing in incorrect types to
  drm_fixed methods in get_bytes_per_soft_slice()

Changes in v3:
- Dropped src_bpp parameter from all methods -- src_bpp can be
  calculated as dsc->bits_per_component * 3
- Dropped intf_width parameter from get_bytes_per_soft_slice()
- Moved dsc->bits_per_component to numerator calculation in
  get_bytes_per_soft_slice()
- 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()
- Made get_bytes_per_soft_slice() a public method (this will be called
  later to help calculate DP pclk params)
- Added documentation in comments
- Moved extra_eol_bytes math out of msm_dsc_get_eol_byte_num() and
  renamed msm_dsc_get_eol_byte_num to *_get_bytes_per_intf.

Signed-off-by: Jessica Zhang 
---
 drivers/gpu/drm/msm/Makefile |  1 +
 drivers/gpu/drm/msm/msm_dsc_helper.c | 47 
 drivers/gpu/drm/msm/msm_dsc_helper.h | 70 
 3 files changed, 118 insertions(+)

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 7274c41228ed..b814fc80e2d5 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -94,6 +94,7 @@ msm-y += \
msm_atomic_tracepoints.o \
msm_debugfs.o \
msm_drv.o \
+   msm_dsc_helper.o \
msm_fb.o \
msm_fence.o \
msm_gem.o \
diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.c 
b/drivers/gpu/drm/msm/msm_dsc_helper.c
new file mode 100644
index ..c8c530211f50
--- /dev/null
+++ b/drivers/gpu/drm/msm/msm_dsc_helper.c
@@ -0,0 +1,47 @@
+// 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"
+
+s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc)
+{
+   int bpp = msm_dsc_get_bpp_int(dsc);
+   s64 numerator_fp, denominator_fp;
+   s64 comp_ratio_fp = drm_fixp_from_fraction(dsc->bits_per_component * 3, 
bpp);
+
+   numerator_fp = drm_int2fixp(dsc->slice_width * 3 * 
dsc->bits_per_component);
+   denominator_fp = drm_fixp_mul(comp_ratio_fp, drm_int2fixp(8));
+
+   return drm_fixp_div(numerator_fp, denominator_fp);
+}
+
+u32 msm_dsc_get_bytes_per_intf(struct drm_dsc_config *dsc, int intf_width)
+{
+   u32 bytes_per_soft_slice, bytes_per_intf;
+   s64 bytes_per_soft_slice_fp;
+   int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width);
+
+   bytes_per_soft_slice_fp = get_bytes_per_soft_slice(dsc);
+   bytes_per_soft_slice = drm_fixp2int_ceil(bytes_per_soft_slice_fp);
+
+   bytes_per_intf = bytes_per_soft_slice * slice_per_intf;
+
+   return bytes_per_intf;
+}
+
+int msm_dsc_get_uncompressed_pclk_per_intf(struct drm_dsc_config *dsc)
+{
+   s64 data_width;
+
+   data_width = drm_fixp_mul(drm_int2fixp(dsc->slice_count),
+   get_bytes_per_soft_slice(dsc));
+
+   return drm_fixp2int_ceil(data_width);
+}
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 ..5ee972eb247c
--- /dev/null
+++ b/drivers/gpu/drm/msm/msm_dsc_helper.h
@@ -0,0 +1,70 @@
+/* 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.
+ */
+
+/**
+ * msm_dsc_get_bpp_int - get bits per pixel integer value
+ * @dsc: Pointer to drm dsc config struct
+ */
+static inline int msm_dsc_get_bpp_int(struct drm_dsc_config *dsc)
+{
+   WARN_ON_ONCE(dsc->bits_per_pixel & 0xf);
+   return dsc->bits_per_pixel >> 4;
+}
+
+/**
+ * msm_dsc_get_slice_per_intf - get number of slices per interface
+ * @dsc: Pointer to drm dsc config struct
+ * @intf_width: interface width
+ */
+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);
+}
+
+/**
+ * msm_dsc_get_dce_bytes_per_line - get bytes per line to help calculate data 
width
+ * when configuring the timing engine
+ * @dsc: Pointer to drm dsc