Re: [PATCH v1 1/5] drm/msm/dpu: add support for DSC encoder v1.2 engine

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21/04/2023 02:25, Kuogee Hsieh wrote:
Add support for DSC 1.2 by providing the necessary hooks to program
the DPU DSC 1.2 encoder.

Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
---
  drivers/gpu/drm/msm/Makefile                   |   1 +
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |  38 ++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h     |  17 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 388 +++++++++++++++++++++++++
  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c         |   7 +-
  5 files changed, 444 insertions(+), 7 deletions(-)
  create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index b814fc8..b9af5e4 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
  	disp/dpu1/dpu_hw_catalog.o \
  	disp/dpu1/dpu_hw_ctl.o \
  	disp/dpu1/dpu_hw_dsc.o \
+	disp/dpu1/dpu_hw_dsc_1_2.o \
  	disp/dpu1/dpu_hw_interrupts.o \
  	disp/dpu1/dpu_hw_intf.o \
  	disp/dpu1/dpu_hw_lm.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 71584cd..22421b9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -1,6 +1,6 @@
  /* SPDX-License-Identifier: GPL-2.0-only */
  /*
- * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
   * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
   */
@@ -241,12 +241,20 @@ enum {
  };
/**
- * DSC features
- * @DPU_DSC_OUTPUT_CTRL       Configure which PINGPONG block gets
- *                            the pixel output from this DSC.
+ * DSC sub-blocks/features
+ * @DPU_DSC_OUTPUT_CTRL         Configure which PINGPONG block gets
+ *                              the pixel output from this DSC.

Any reason to change the alignment

+ * @DPU_DSC_HW_REV_1_1          DSC block supports dsc 1.1 only
+ * @DPU_DSC_HW_REV_1_2          DSC block supports dsc 1.1 and 1.2
+ * @DPU_DSC_NATIVE_422_EN       Supports native422 and native420 encoding
+ * @DPU_DSC_MAX
   */
  enum {
  	DPU_DSC_OUTPUT_CTRL = 0x1,
+	DPU_DSC_HW_REV_1_1,
+	DPU_DSC_HW_REV_1_2,
+	DPU_DSC_NATIVE_422_EN,
+	DPU_DSC_MAX
  };
/**
@@ -311,6 +319,14 @@ struct dpu_pp_blk {
  };
/**
+ * struct dpu_dsc_blk - DSC Encoder sub-blk information
+ * @info:   HW register and features supported by this sub-blk
+ */
+struct dpu_dsc_blk {
+	DPU_HW_SUBBLK_INFO;
+};
+
+/**
   * enum dpu_qos_lut_usage - define QoS LUT use cases
   */
  enum dpu_qos_lut_usage {
@@ -459,6 +475,17 @@ struct dpu_pingpong_sub_blks {
  };
/**
+ * struct dpu_dsc_sub_blks - DSC sub-blks
+ * @enc: DSC encoder sub block
+ * @ctl: DSC controller sub block
+ *
+ */
+struct dpu_dsc_sub_blks {
+	struct dpu_dsc_blk enc;
+	struct dpu_dsc_blk ctl;
+};
+
+/**
   * dpu_clk_ctrl_type - Defines top level clock control signals
   */
  enum dpu_clk_ctrl_type {
@@ -612,10 +639,13 @@ struct dpu_merge_3d_cfg  {
   * struct dpu_dsc_cfg - information of DSC blocks
   * @id                 enum identifying this block
   * @base               register offset of this block
+ * @len:               length of hardware block
   * @features           bit mask identifying sub-blocks/features
+ * @sblk               sub-blocks information
   */
  struct dpu_dsc_cfg {
  	DPU_HW_BLK_INFO;
+	const struct dpu_dsc_sub_blks *sblk;
  };
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
index ae9b5db..d0f8b8b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -1,5 +1,8 @@
  /* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2020-2022, Linaro Limited */
+/*
+ * Copyright (c) 2020-2022, Linaro Limited
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
#ifndef _DPU_HW_DSC_H
  #define _DPU_HW_DSC_H
@@ -61,7 +64,7 @@ struct dpu_hw_dsc {
  };
/**
- * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
+ * dpu_hw_dsc_init - initializes the v1.1 dsc block for the passed dsc idx.
   * @idx:  DSC index for which driver object is required
   * @addr: Mapped register io address of MDP
   * @m:    Pointer to mdss catalog data
@@ -71,6 +74,16 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
  				   const struct dpu_mdss_cfg *m);
/**
+ * dpu_hw_dsc_init_1_2 - initializes the v1.2 dsc block for the passed dsc idx.
+ * @idx:  DSC index for which driver object is required
+ * @addr: Mapped register io address of MDP
+ * @m:    Pointer to mdss catalog data
+ * Returns: Error code or allocated dpu_hw_dsc context
+ */
+struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(enum dpu_dsc idx, void __iomem *addr,
+				   const struct dpu_mdss_cfg *m);
+
+/**
   * dpu_hw_dsc_destroy - destroys dsc driver context
   * @dsc:   Pointer to dsc driver context returned by dpu_hw_dsc_init
   */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
new file mode 100644
index 00000000..455d7f2
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
@@ -0,0 +1,388 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#include <drm/display/drm_dsc_helper.h>
+
+#include "dpu_kms.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_dsc.h"
+
+
+#define DSC_CMN_MAIN_CNF           0x00
+
+/* DPU_DSC_ENC register offsets */
+#define ENC_DF_CTRL                0x00
+#define ENC_GENERAL_STATUS         0x04
+#define ENC_HSLICE_STATUS          0x08
+#define ENC_OUT_STATUS             0x0C
+#define ENC_INT_STAT               0x10
+#define ENC_INT_CLR                0x14
+#define ENC_INT_MASK               0x18
+#define DSC_MAIN_CONF              0x30
+#define DSC_PICTURE_SIZE           0x34
+#define DSC_SLICE_SIZE             0x38
+#define DSC_MISC_SIZE              0x3C
+#define DSC_HRD_DELAYS             0x40
+#define DSC_RC_SCALE               0x44
+#define DSC_RC_SCALE_INC_DEC       0x48
+#define DSC_RC_OFFSETS_1           0x4C
+#define DSC_RC_OFFSETS_2           0x50
+#define DSC_RC_OFFSETS_3           0x54
+#define DSC_RC_OFFSETS_4           0x58
+#define DSC_FLATNESS_QP            0x5C
+#define DSC_RC_MODEL_SIZE          0x60
+#define DSC_RC_CONFIG              0x64
+#define DSC_RC_BUF_THRESH_0        0x68
+#define DSC_RC_BUF_THRESH_1        0x6C
+#define DSC_RC_BUF_THRESH_2        0x70
+#define DSC_RC_BUF_THRESH_3        0x74
+#define DSC_RC_MIN_QP_0            0x78
+#define DSC_RC_MIN_QP_1            0x7C
+#define DSC_RC_MIN_QP_2            0x80
+#define DSC_RC_MAX_QP_0            0x84
+#define DSC_RC_MAX_QP_1            0x88
+#define DSC_RC_MAX_QP_2             0x8C

As you are adding new definitions, could please make them aligned from the beginning?

+#define DSC_RC_RANGE_BPG_OFFSETS_0  0x90
+#define DSC_RC_RANGE_BPG_OFFSETS_1  0x94
+#define DSC_RC_RANGE_BPG_OFFSETS_2  0x98
+
+/* DPU_DSC_CTL register offsets */
+#define DSC_CTL                    0x00
+#define DSC_CFG                    0x04
+#define DSC_DATA_IN_SWAP           0x08
+#define DSC_CLK_CTRL               0x0C
+
+/*
+ * @DPU_DSC_ENC                 DSC encoder sub block
+ * @DPU_DSC_CTL                 DSC controller sub block
+ */
+enum {
+	DPU_DSC_ENC,
+	DPU_DSC_CTL,
+};
+
+static int _dsc_calc_ob_max_addr(struct dpu_hw_dsc *hw_dsc, int num_ss)

If this is used from a single place, please inline this function.

+{
+	int max_addr = 2400 / num_ss;
+
+	if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_422_EN))
+		max_addr /= 2;

Is this really guarded by the hw block feature or by native_422 being enabled?

+
+	return max_addr - 1;
+};
+


[skipped]

+static void dpu_hw_dsc_config_thresh_1_2(struct dpu_hw_dsc *hw_dsc,
+					struct drm_dsc_config *dsc)
+{
+	struct dpu_hw_blk_reg_map *hw;
+	u32 offset, off;
+	int i, j = 0;
+	struct drm_dsc_rc_range_parameters *rc;
+	u32 data = 0, min_qp = 0, max_qp = 0, bpg_off = 0;
+
+	if (!hw_dsc || !dsc)
+		return;
+
+	_dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
+
+	hw = &hw_dsc->hw;
+
+	rc = dsc->rc_range_params;
+
+	off = 0;
+	for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) {
+		data |= dsc->rc_buf_thresh[i] << (8 * j);
+		j++;
+		if ((j == 4) || (i == DSC_NUM_BUF_RANGES - 2)) {
+			DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_0 + off,
+					data);
+			off += 4;
+			j = 0;
+			data = 0;
+		}
+	}
+
+	off = 0;
+	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
+		min_qp |= rc[i].range_min_qp << (5 * j);
+		max_qp |= rc[i].range_max_qp << (5 * j);
+		bpg_off |= rc[i].range_bpg_offset << (6 * j);
+		j++;
+		if (j == 5) {
+			DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_0 + off,
+					min_qp);
+			DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_0 + off,
+					max_qp);
+			DPU_REG_WRITE(hw,
+					offset + DSC_RC_RANGE_BPG_OFFSETS_0 + off,
+					bpg_off);
+			off += 4;
+			j = 0;
+			min_qp = 0;
+			max_qp = 0;
+			bpg_off = 0;
+		}
+	}

Unrolling these loops would make it easier to understand them.

+}
+
+static void dpu_hw_dsc_bind_pingpong_blk_1_2(
+		struct dpu_hw_dsc *hw_dsc,
+		bool enable,
+		const enum dpu_pingpong pp)
+{
+	struct dpu_hw_blk_reg_map *hw;
+	int offset;
+	int mux_cfg = 0xf; /* Disabled */
+
+	_dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
+
+	hw = &hw_dsc->hw;
+	if (enable)
+		mux_cfg = (pp - PINGPONG_0) & 0x7;
+
+	DPU_REG_WRITE(hw, offset + DSC_CTL, mux_cfg);
+}
+
+static const struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
+				       const struct dpu_mdss_cfg *m,
+				       void __iomem *addr,
+				       struct dpu_hw_blk_reg_map *b)
+{
+	int i;
+
+	for (i = 0; i < m->dsc_count; i++) {
+		if (dsc == m->dsc[i].id) {
+			b->blk_addr = addr + m->dsc[i].base;
+			b->log_mask = DPU_DBG_MASK_DSC;
+			return &m->dsc[i];
+		}
+	}
+
+	return NULL;
+}
+
+static void _setup_dcs_ops_1_2(struct dpu_hw_dsc_ops *ops,
+		const unsigned long features)
+{
+	ops->dsc_disable = dpu_hw_dsc_disable_1_2;
+	ops->dsc_config = dpu_hw_dsc_config_1_2;
+	ops->dsc_config_thresh = dpu_hw_dsc_config_thresh_1_2;
+	ops->dsc_bind_pingpong_blk = dpu_hw_dsc_bind_pingpong_blk_1_2;
+}
+
+struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(enum dpu_dsc idx, void __iomem *addr,
+				   const struct dpu_mdss_cfg *m)
+{

Please rebase on top of https://patchwork.freedesktop.org/series/116615/

+	struct dpu_hw_dsc *c;
+	const struct dpu_dsc_cfg *cfg;
+
+	c = kzalloc(sizeof(*c), GFP_KERNEL);
+	if (!c)
+		return ERR_PTR(-ENOMEM);
+
+	cfg = _dsc_offset(idx, m, addr, &c->hw);
+	if (IS_ERR_OR_NULL(cfg)) {
+		kfree(c);
+		return ERR_PTR(-EINVAL);
+	}
+
+	c->idx = idx;
+	c->caps = cfg;
+
+	_setup_dcs_ops_1_2(&c->ops, c->caps->features);
+
+	return c;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f4dda88..f7014f5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -1,6 +1,7 @@
  // SPDX-License-Identifier: GPL-2.0-only
  /*
   * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
   */
#define pr_fmt(fmt) "[drm:%s] " fmt, __func__
@@ -250,7 +251,11 @@ int dpu_rm_init(struct dpu_rm *rm,
  		struct dpu_hw_dsc *hw;
  		const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
- hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
+		if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
+			hw = dpu_hw_dsc_init_1_2(dsc->id, mmio, cat);
+		else
+			hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
+
  		if (IS_ERR_OR_NULL(hw)) {
  			rc = PTR_ERR(hw);
  			DPU_ERROR("failed dsc object creation: err %d\n", rc);

--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux