2023-12-08 05:08:02

by Abhinav Kumar

[permalink] [raw]
Subject: [PATCH v2 07/16] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block

CDM block comes with its own set of registers and operations
which can be done. In-line with other hardware sub-blocks, this
change adds the dpu_hw_cdm abstraction for the CDM block.

changes in v2:
- replace bit magic with relevant defines
- use drmm_kzalloc instead of kzalloc/free
- some formatting fixes
- inline _setup_cdm_ops()
- protect bind_pingpong_blk with core_rev check
- drop setup_csc_data() and setup_cdwn() ops as they
are merged into enable()

Signed-off-by: Abhinav Kumar <[email protected]>
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 276 ++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 114 ++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
4 files changed, 392 insertions(+)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 49671364fdcf..b1173128b5b9 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_encoder_phys_wb.o \
disp/dpu1/dpu_formats.o \
disp/dpu1/dpu_hw_catalog.o \
+ disp/dpu1/dpu_hw_cdm.o \
disp/dpu1/dpu_hw_ctl.o \
disp/dpu1/dpu_hw_dsc.o \
disp/dpu1/dpu_hw_dsc_1_2.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
new file mode 100644
index 000000000000..0dbe2df56cc8
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, The Linux Foundation. All rights reserved.
+ */
+
+#include <drm/drm_managed.h>
+
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_util.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hw_cdm.h"
+#include "dpu_kms.h"
+
+#define CDM_CSC_10_OPMODE 0x000
+#define CDM_CSC_10_BASE 0x004
+
+#define CDM_CDWN2_OP_MODE 0x100
+#define CDM_CDWN2_CLAMP_OUT 0x104
+#define CDM_CDWN2_PARAMS_3D_0 0x108
+#define CDM_CDWN2_PARAMS_3D_1 0x10C
+#define CDM_CDWN2_COEFF_COSITE_H_0 0x110
+#define CDM_CDWN2_COEFF_COSITE_H_1 0x114
+#define CDM_CDWN2_COEFF_COSITE_H_2 0x118
+#define CDM_CDWN2_COEFF_OFFSITE_H_0 0x11C
+#define CDM_CDWN2_COEFF_OFFSITE_H_1 0x120
+#define CDM_CDWN2_COEFF_OFFSITE_H_2 0x124
+#define CDM_CDWN2_COEFF_COSITE_V 0x128
+#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C
+#define CDM_CDWN2_OUT_SIZE 0x130
+
+#define CDM_HDMI_PACK_OP_MODE 0x200
+#define CDM_CSC_10_MATRIX_COEFF_0 0x004
+
+#define CDM_MUX 0x224
+
+/* CDM CDWN2 sub-block bit definitions */
+#define CDM_CDWN2_OP_MODE_EN BIT(0)
+#define CDM_CDWN2_OP_MODE_ENABLE_H BIT(1)
+#define CDM_CDWN2_OP_MODE_ENABLE_V BIT(2)
+#define CDM_CDWN2_OP_MODE_METHOD_H_AVG BIT(3)
+#define CDM_CDWN2_OP_MODE_METHOD_H_COSITE BIT(4)
+#define CDM_CDWN2_OP_MODE_METHOD_V_AVG BIT(5)
+#define CDM_CDWN2_OP_MODE_METHOD_V_COSITE BIT(6)
+#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7)
+#define CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE GENMASK(4, 3)
+#define CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE GENMASK(6, 5)
+#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5)
+#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3)
+
+/* CDM CSC10 sub-block bit definitions */
+#define CDM_CSC10_OP_MODE_EN BIT(0)
+#define CDM_CSC10_OP_MODE_SRC_FMT_YUV BIT(1)
+#define CDM_CSC10_OP_MODE_DST_FMT_YUV BIT(2)
+
+/* CDM HDMI pack sub-block bit definitions */
+#define CDM_HDMI_PACK_OP_MODE_EN BIT(0)
+
+/**
+ * Horizontal coefficients for cosite chroma downscale
+ * s13 representation of coefficients
+ */
+static u32 cosite_h_coeff[] = {0x00000016, 0x000001cc, 0x0100009e};
+
+/**
+ * Horizontal coefficients for offsite chroma downscale
+ */
+static u32 offsite_h_coeff[] = {0x000b0005, 0x01db01eb, 0x00e40046};
+
+/**
+ * Vertical coefficients for cosite chroma downscale
+ */
+static u32 cosite_v_coeff[] = {0x00080004};
+/**
+ * Vertical coefficients for offsite chroma downscale
+ */
+static u32 offsite_v_coeff[] = {0x00060002};
+
+static int dpu_hw_cdm_setup_cdwn(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cfg)
+{
+ struct dpu_hw_blk_reg_map *c = &ctx->hw;
+ u32 opmode = 0;
+ u32 out_size = 0;
+
+ if (cfg->output_bit_depth == CDM_CDWN_OUTPUT_10BIT)
+ opmode &= ~CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
+ else
+ opmode |= CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
+
+ /* ENABLE DWNS_H bit */
+ opmode |= CDM_CDWN2_OP_MODE_ENABLE_H;
+
+ switch (cfg->h_cdwn_type) {
+ case CDM_CDWN_DISABLE:
+ /* CLEAR METHOD_H field */
+ opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
+ /* CLEAR DWNS_H bit */
+ opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_H;
+ break;
+ case CDM_CDWN_PIXEL_DROP:
+ /* Clear METHOD_H field (pixel drop is 0) */
+ opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
+ break;
+ case CDM_CDWN_AVG:
+ /* Clear METHOD_H field (Average is 0x1) */
+ opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
+ opmode |= CDM_CDWN2_OP_MODE_METHOD_H_AVG;
+ break;
+ case CDM_CDWN_COSITE:
+ /* Clear METHOD_H field (Average is 0x2) */
+ opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
+ opmode |= CDM_CDWN2_OP_MODE_METHOD_H_COSITE;
+ /* Co-site horizontal coefficients */
+ DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_0,
+ cosite_h_coeff[0]);
+ DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_1,
+ cosite_h_coeff[1]);
+ DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_2,
+ cosite_h_coeff[2]);
+ break;
+ case CDM_CDWN_OFFSITE:
+ /* Clear METHOD_H field (Average is 0x3) */
+ opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
+ opmode |= CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE;
+
+ /* Off-site horizontal coefficients */
+ DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_0,
+ offsite_h_coeff[0]);
+ DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_1,
+ offsite_h_coeff[1]);
+ DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_2,
+ offsite_h_coeff[2]);
+ break;
+ default:
+ pr_err("%s invalid horz down sampling type\n", __func__);
+ return -EINVAL;
+ }
+
+ /* ENABLE DWNS_V bit */
+ opmode |= CDM_CDWN2_OP_MODE_ENABLE_V;
+
+ switch (cfg->v_cdwn_type) {
+ case CDM_CDWN_DISABLE:
+ /* CLEAR METHOD_V field */
+ opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
+ /* CLEAR DWNS_V bit */
+ opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_V;
+ break;
+ case CDM_CDWN_PIXEL_DROP:
+ /* Clear METHOD_V field (pixel drop is 0) */
+ opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
+ break;
+ case CDM_CDWN_AVG:
+ /* Clear METHOD_V field (Average is 0x1) */
+ opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
+ opmode |= CDM_CDWN2_OP_MODE_METHOD_V_AVG;
+ break;
+ case CDM_CDWN_COSITE:
+ /* Clear METHOD_V field (Average is 0x2) */
+ opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
+ opmode |= CDM_CDWN2_OP_MODE_METHOD_V_COSITE;
+ /* Co-site vertical coefficients */
+ DPU_REG_WRITE(c,
+ CDM_CDWN2_COEFF_COSITE_V,
+ cosite_v_coeff[0]);
+ break;
+ case CDM_CDWN_OFFSITE:
+ /* Clear METHOD_V field (Average is 0x3) */
+ opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
+ opmode |= CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE;
+
+ /* Off-site vertical coefficients */
+ DPU_REG_WRITE(c,
+ CDM_CDWN2_COEFF_OFFSITE_V,
+ offsite_v_coeff[0]);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (cfg->v_cdwn_type || cfg->h_cdwn_type)
+ opmode |= CDM_CDWN2_OP_MODE_EN; /* EN CDWN module */
+ else
+ opmode &= ~CDM_CDWN2_OP_MODE_EN;
+
+ out_size = (cfg->output_width & 0xFFFF) | ((cfg->output_height & 0xFFFF) << 16);
+ DPU_REG_WRITE(c, CDM_CDWN2_OUT_SIZE, out_size);
+ DPU_REG_WRITE(c, CDM_CDWN2_OP_MODE, opmode);
+ DPU_REG_WRITE(c, CDM_CDWN2_CLAMP_OUT, ((0x3FF << 16) | 0x0));
+
+ return 0;
+}
+
+int dpu_hw_cdm_enable(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cdm)
+{
+ struct dpu_hw_blk_reg_map *c = &ctx->hw;
+ const struct dpu_format *fmt;
+ u32 opmode = 0;
+ u32 csc = 0;
+
+ if (!ctx || !cdm)
+ return -EINVAL;
+
+ fmt = cdm->output_fmt;
+
+ if (!DPU_FORMAT_IS_YUV(fmt))
+ return -EINVAL;
+
+ dpu_hw_csc_setup(&ctx->hw, CDM_CSC_10_MATRIX_COEFF_0, cdm->csc_cfg, true);
+ dpu_hw_cdm_setup_cdwn(ctx, cdm);
+
+ if (cdm->output_type == CDM_CDWN_OUTPUT_HDMI) {
+ if (fmt->chroma_sample != DPU_CHROMA_H1V2)
+ return -EINVAL; /*unsupported format */
+ opmode = CDM_HDMI_PACK_OP_MODE_EN;
+ opmode |= (fmt->chroma_sample << 1);
+ }
+
+ csc |= CDM_CSC10_OP_MODE_DST_FMT_YUV;
+ csc &= ~CDM_CSC10_OP_MODE_SRC_FMT_YUV;
+ csc |= CDM_CSC10_OP_MODE_EN;
+
+ if (ctx && ctx->ops.bind_pingpong_blk)
+ ctx->ops.bind_pingpong_blk(ctx, true, cdm->pp_id);
+
+ DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc);
+ DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode);
+ return 0;
+}
+
+void dpu_hw_cdm_disable(struct dpu_hw_cdm *ctx)
+{
+ if (!ctx)
+ return;
+
+ if (ctx && ctx->ops.bind_pingpong_blk)
+ ctx->ops.bind_pingpong_blk(ctx, false, PINGPONG_NONE);
+}
+
+static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, bool enable,
+ const enum dpu_pingpong pp)
+{
+ struct dpu_hw_blk_reg_map *c;
+ int mux_cfg = 0xF;
+
+ c = &ctx->hw;
+
+ if (enable)
+ mux_cfg = (pp - PINGPONG_0) & 0x7;
+
+ DPU_REG_WRITE(c, CDM_MUX, mux_cfg);
+}
+
+struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
+ const struct dpu_cdm_cfg *cfg, void __iomem *addr,
+ const struct dpu_mdss_version *mdss_rev)
+{
+ struct dpu_hw_cdm *c;
+
+ c = drmm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return ERR_PTR(-ENOMEM);
+
+ c->hw.blk_addr = addr + cfg->base;
+ c->hw.log_mask = DPU_DBG_MASK_CDM;
+
+ /* Assign ops */
+ c->idx = cfg->id;
+ c->caps = cfg;
+
+ c->ops.enable = dpu_hw_cdm_enable;
+ c->ops.disable = dpu_hw_cdm_disable;
+ if (mdss_rev->core_major_ver >= 5)
+ c->ops.bind_pingpong_blk = dpu_hw_cdm_bind_pingpong_blk;
+
+ return c;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
new file mode 100644
index 000000000000..1ca806f9d18d
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DPU_HW_CDM_H
+#define _DPU_HW_CDM_H
+
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_top.h"
+
+struct dpu_hw_cdm;
+
+struct dpu_hw_cdm_cfg {
+ u32 output_width;
+ u32 output_height;
+ u32 output_bit_depth;
+ u32 h_cdwn_type;
+ u32 v_cdwn_type;
+ const struct dpu_format *output_fmt;
+ const struct dpu_csc_cfg *csc_cfg;
+ u32 output_type;
+ int pp_id;
+};
+
+enum dpu_hw_cdwn_type {
+ CDM_CDWN_DISABLE,
+ CDM_CDWN_PIXEL_DROP,
+ CDM_CDWN_AVG,
+ CDM_CDWN_COSITE,
+ CDM_CDWN_OFFSITE,
+};
+
+enum dpu_hw_cdwn_output_type {
+ CDM_CDWN_OUTPUT_HDMI,
+ CDM_CDWN_OUTPUT_WB,
+};
+
+enum dpu_hw_cdwn_output_bit_depth {
+ CDM_CDWN_OUTPUT_8BIT,
+ CDM_CDWN_OUTPUT_10BIT,
+};
+
+/**
+ * struct dpu_hw_cdm_ops : Interface to the chroma down Hw driver functions
+ * Assumption is these functions will be called after
+ * clocks are enabled
+ * @enable: Enables the output to interface and programs the
+ * output packer
+ * @disable: Puts the cdm in bypass mode
+ * @bind_pingpong_blk: enable/disable the connection with pingpong which
+ * will feed pixels to this cdm
+ */
+struct dpu_hw_cdm_ops {
+ /**
+ * Enable the CDM module
+ * @cdm Pointer to chroma down context
+ */
+ int (*enable)(struct dpu_hw_cdm *cdm, struct dpu_hw_cdm_cfg *cfg);
+
+ /**
+ * Disable the CDM module
+ * @cdm Pointer to chroma down context
+ */
+ void (*disable)(struct dpu_hw_cdm *cdm);
+
+ /**
+ * Enable/disable the connection with pingpong
+ * @cdm Pointer to chroma down context
+ * @enable Enable/disable control
+ * @pp pingpong block id.
+ */
+ void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, bool enable,
+ const enum dpu_pingpong pp);
+};
+
+/**
+ * struct dpu_hw_cdm - cdm description
+ * @base: Hardware block base structure
+ * @hw: Block hardware details
+ * @idx: CDM index
+ * @caps: Pointer to cdm_cfg
+ * @ops: handle to operations possible for this CDM
+ */
+struct dpu_hw_cdm {
+ struct dpu_hw_blk base;
+ struct dpu_hw_blk_reg_map hw;
+
+ /* chroma down */
+ const struct dpu_cdm_cfg *caps;
+ enum dpu_cdm idx;
+
+ /* ops */
+ struct dpu_hw_cdm_ops ops;
+};
+
+/**
+ * dpu_hw_cdm_init - initializes the cdm hw driver object.
+ * should be called once before accessing every cdm.
+ * @dev: DRM device handle
+ * @cdm: CDM catalog entry for which driver object is required
+ * @addr : mapped register io address of MDSS
+ * @mdss_rev: mdss hw core revision
+ */
+struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
+ const struct dpu_cdm_cfg *cdm, void __iomem *addr,
+ const struct dpu_mdss_version *mdss_rev);
+
+static inline struct dpu_hw_cdm *to_dpu_hw_cdm(struct dpu_hw_blk *hw)
+{
+ return container_of(hw, struct dpu_hw_cdm, base);
+}
+
+#endif /*_DPU_HW_CDM_H */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index f319c8232ea5..9db4cf61bd29 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -466,6 +466,7 @@ struct dpu_mdss_color {
#define DPU_DBG_MASK_ROT (1 << 9)
#define DPU_DBG_MASK_DSPP (1 << 10)
#define DPU_DBG_MASK_DSC (1 << 11)
+#define DPU_DBG_MASK_CDM (1 << 12)

/**
* struct dpu_hw_tear_check - Struct contains parameters to configure
--
2.40.1


2023-12-08 12:07:28

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2 07/16] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block

On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> wrote:
>
> CDM block comes with its own set of registers and operations
> which can be done. In-line with other hardware sub-blocks, this

I always thought that sub-blocks refer to the dpu_foo_sub_blks data,
which CDM doesn't have.


> change adds the dpu_hw_cdm abstraction for the CDM block.
>
> changes in v2:
> - replace bit magic with relevant defines
> - use drmm_kzalloc instead of kzalloc/free
> - some formatting fixes
> - inline _setup_cdm_ops()
> - protect bind_pingpong_blk with core_rev check
> - drop setup_csc_data() and setup_cdwn() ops as they
> are merged into enable()
>
> Signed-off-by: Abhinav Kumar <[email protected]>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 276 ++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 114 ++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
> 4 files changed, 392 insertions(+)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 49671364fdcf..b1173128b5b9 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
> disp/dpu1/dpu_encoder_phys_wb.o \
> disp/dpu1/dpu_formats.o \
> disp/dpu1/dpu_hw_catalog.o \
> + disp/dpu1/dpu_hw_cdm.o \
> disp/dpu1/dpu_hw_ctl.o \
> disp/dpu1/dpu_hw_dsc.o \
> disp/dpu1/dpu_hw_dsc_1_2.o \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> new file mode 100644
> index 000000000000..0dbe2df56cc8
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> @@ -0,0 +1,276 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <drm/drm_managed.h>
> +
> +#include "dpu_hw_mdss.h"
> +#include "dpu_hw_util.h"
> +#include "dpu_hw_catalog.h"
> +#include "dpu_hw_cdm.h"
> +#include "dpu_kms.h"
> +
> +#define CDM_CSC_10_OPMODE 0x000
> +#define CDM_CSC_10_BASE 0x004
> +
> +#define CDM_CDWN2_OP_MODE 0x100
> +#define CDM_CDWN2_CLAMP_OUT 0x104
> +#define CDM_CDWN2_PARAMS_3D_0 0x108
> +#define CDM_CDWN2_PARAMS_3D_1 0x10C
> +#define CDM_CDWN2_COEFF_COSITE_H_0 0x110
> +#define CDM_CDWN2_COEFF_COSITE_H_1 0x114
> +#define CDM_CDWN2_COEFF_COSITE_H_2 0x118
> +#define CDM_CDWN2_COEFF_OFFSITE_H_0 0x11C
> +#define CDM_CDWN2_COEFF_OFFSITE_H_1 0x120
> +#define CDM_CDWN2_COEFF_OFFSITE_H_2 0x124
> +#define CDM_CDWN2_COEFF_COSITE_V 0x128
> +#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C
> +#define CDM_CDWN2_OUT_SIZE 0x130
> +
> +#define CDM_HDMI_PACK_OP_MODE 0x200
> +#define CDM_CSC_10_MATRIX_COEFF_0 0x004
> +
> +#define CDM_MUX 0x224
> +
> +/* CDM CDWN2 sub-block bit definitions */
> +#define CDM_CDWN2_OP_MODE_EN BIT(0)
> +#define CDM_CDWN2_OP_MODE_ENABLE_H BIT(1)
> +#define CDM_CDWN2_OP_MODE_ENABLE_V BIT(2)
> +#define CDM_CDWN2_OP_MODE_METHOD_H_AVG BIT(3)
> +#define CDM_CDWN2_OP_MODE_METHOD_H_COSITE BIT(4)
> +#define CDM_CDWN2_OP_MODE_METHOD_V_AVG BIT(5)
> +#define CDM_CDWN2_OP_MODE_METHOD_V_COSITE BIT(6)
> +#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7)
> +#define CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE GENMASK(4, 3)
> +#define CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE GENMASK(6, 5)
> +#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5)
> +#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3)
> +
> +/* CDM CSC10 sub-block bit definitions */
> +#define CDM_CSC10_OP_MODE_EN BIT(0)
> +#define CDM_CSC10_OP_MODE_SRC_FMT_YUV BIT(1)
> +#define CDM_CSC10_OP_MODE_DST_FMT_YUV BIT(2)
> +
> +/* CDM HDMI pack sub-block bit definitions */
> +#define CDM_HDMI_PACK_OP_MODE_EN BIT(0)
> +
> +/**
> + * Horizontal coefficients for cosite chroma downscale
> + * s13 representation of coefficients
> + */
> +static u32 cosite_h_coeff[] = {0x00000016, 0x000001cc, 0x0100009e};
> +
> +/**
> + * Horizontal coefficients for offsite chroma downscale
> + */
> +static u32 offsite_h_coeff[] = {0x000b0005, 0x01db01eb, 0x00e40046};
> +
> +/**
> + * Vertical coefficients for cosite chroma downscale
> + */
> +static u32 cosite_v_coeff[] = {0x00080004};
> +/**
> + * Vertical coefficients for offsite chroma downscale
> + */
> +static u32 offsite_v_coeff[] = {0x00060002};
> +
> +static int dpu_hw_cdm_setup_cdwn(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cfg)
> +{
> + struct dpu_hw_blk_reg_map *c = &ctx->hw;
> + u32 opmode = 0;
> + u32 out_size = 0;
> +
> + if (cfg->output_bit_depth == CDM_CDWN_OUTPUT_10BIT)
> + opmode &= ~CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;

We start from opmode = 0. Does it really make sense to mask bits from
the zero opmode?

> + else
> + opmode |= CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
> +
> + /* ENABLE DWNS_H bit */
> + opmode |= CDM_CDWN2_OP_MODE_ENABLE_H;
> +
> + switch (cfg->h_cdwn_type) {
> + case CDM_CDWN_DISABLE:
> + /* CLEAR METHOD_H field */
> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> + /* CLEAR DWNS_H bit */
> + opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_H;
> + break;
> + case CDM_CDWN_PIXEL_DROP:
> + /* Clear METHOD_H field (pixel drop is 0) */
> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> + break;
> + case CDM_CDWN_AVG:
> + /* Clear METHOD_H field (Average is 0x1) */
> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_AVG;
> + break;
> + case CDM_CDWN_COSITE:
> + /* Clear METHOD_H field (Average is 0x2) */
> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_COSITE;
> + /* Co-site horizontal coefficients */
> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_0,
> + cosite_h_coeff[0]);
> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_1,
> + cosite_h_coeff[1]);
> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_2,
> + cosite_h_coeff[2]);
> + break;
> + case CDM_CDWN_OFFSITE:
> + /* Clear METHOD_H field (Average is 0x3) */
> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE;
> +
> + /* Off-site horizontal coefficients */
> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_0,
> + offsite_h_coeff[0]);
> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_1,
> + offsite_h_coeff[1]);
> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_2,
> + offsite_h_coeff[2]);
> + break;
> + default:
> + pr_err("%s invalid horz down sampling type\n", __func__);

DPU_ERROR or drm_err

> + return -EINVAL;
> + }
> +
> + /* ENABLE DWNS_V bit */
> + opmode |= CDM_CDWN2_OP_MODE_ENABLE_V;
> +
> + switch (cfg->v_cdwn_type) {
> + case CDM_CDWN_DISABLE:
> + /* CLEAR METHOD_V field */
> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> + /* CLEAR DWNS_V bit */
> + opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_V;
> + break;
> + case CDM_CDWN_PIXEL_DROP:
> + /* Clear METHOD_V field (pixel drop is 0) */
> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> + break;
> + case CDM_CDWN_AVG:
> + /* Clear METHOD_V field (Average is 0x1) */
> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_AVG;
> + break;
> + case CDM_CDWN_COSITE:
> + /* Clear METHOD_V field (Average is 0x2) */
> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_COSITE;
> + /* Co-site vertical coefficients */
> + DPU_REG_WRITE(c,
> + CDM_CDWN2_COEFF_COSITE_V,
> + cosite_v_coeff[0]);
> + break;
> + case CDM_CDWN_OFFSITE:
> + /* Clear METHOD_V field (Average is 0x3) */
> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE;
> +
> + /* Off-site vertical coefficients */
> + DPU_REG_WRITE(c,
> + CDM_CDWN2_COEFF_OFFSITE_V,
> + offsite_v_coeff[0]);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (cfg->v_cdwn_type || cfg->h_cdwn_type)
> + opmode |= CDM_CDWN2_OP_MODE_EN; /* EN CDWN module */
> + else
> + opmode &= ~CDM_CDWN2_OP_MODE_EN;
> +
> + out_size = (cfg->output_width & 0xFFFF) | ((cfg->output_height & 0xFFFF) << 16);
> + DPU_REG_WRITE(c, CDM_CDWN2_OUT_SIZE, out_size);
> + DPU_REG_WRITE(c, CDM_CDWN2_OP_MODE, opmode);
> + DPU_REG_WRITE(c, CDM_CDWN2_CLAMP_OUT, ((0x3FF << 16) | 0x0));
> +
> + return 0;
> +}
> +
> +int dpu_hw_cdm_enable(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cdm)
> +{
> + struct dpu_hw_blk_reg_map *c = &ctx->hw;
> + const struct dpu_format *fmt;
> + u32 opmode = 0;
> + u32 csc = 0;
> +
> + if (!ctx || !cdm)
> + return -EINVAL;
> +
> + fmt = cdm->output_fmt;
> +
> + if (!DPU_FORMAT_IS_YUV(fmt))
> + return -EINVAL;
> +
> + dpu_hw_csc_setup(&ctx->hw, CDM_CSC_10_MATRIX_COEFF_0, cdm->csc_cfg, true);
> + dpu_hw_cdm_setup_cdwn(ctx, cdm);
> +
> + if (cdm->output_type == CDM_CDWN_OUTPUT_HDMI) {
> + if (fmt->chroma_sample != DPU_CHROMA_H1V2)
> + return -EINVAL; /*unsupported format */
> + opmode = CDM_HDMI_PACK_OP_MODE_EN;
> + opmode |= (fmt->chroma_sample << 1);
> + }
> +
> + csc |= CDM_CSC10_OP_MODE_DST_FMT_YUV;
> + csc &= ~CDM_CSC10_OP_MODE_SRC_FMT_YUV;
> + csc |= CDM_CSC10_OP_MODE_EN;
> +
> + if (ctx && ctx->ops.bind_pingpong_blk)
> + ctx->ops.bind_pingpong_blk(ctx, true, cdm->pp_id);
> +
> + DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc);
> + DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode);
> + return 0;
> +}
> +
> +void dpu_hw_cdm_disable(struct dpu_hw_cdm *ctx)
> +{
> + if (!ctx)
> + return;
> +
> + if (ctx && ctx->ops.bind_pingpong_blk)
> + ctx->ops.bind_pingpong_blk(ctx, false, PINGPONG_NONE);

So the bind/un_pingpong_block gets hidden here. Why do we need to
unbind it manually in the dpu_encoder then?

> +}
> +
> +static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, bool enable,
> + const enum dpu_pingpong pp)

I think we settled on the PINGPONG_NONE for removing the binding

> +{
> + struct dpu_hw_blk_reg_map *c;
> + int mux_cfg = 0xF;
> +
> + c = &ctx->hw;
> +
> + if (enable)
> + mux_cfg = (pp - PINGPONG_0) & 0x7;
> +
> + DPU_REG_WRITE(c, CDM_MUX, mux_cfg);
> +}
> +
> +struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
> + const struct dpu_cdm_cfg *cfg, void __iomem *addr,
> + const struct dpu_mdss_version *mdss_rev)
> +{
> + struct dpu_hw_cdm *c;
> +
> + c = drmm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return ERR_PTR(-ENOMEM);
> +
> + c->hw.blk_addr = addr + cfg->base;
> + c->hw.log_mask = DPU_DBG_MASK_CDM;
> +
> + /* Assign ops */
> + c->idx = cfg->id;
> + c->caps = cfg;
> +
> + c->ops.enable = dpu_hw_cdm_enable;
> + c->ops.disable = dpu_hw_cdm_disable;
> + if (mdss_rev->core_major_ver >= 5)
> + c->ops.bind_pingpong_blk = dpu_hw_cdm_bind_pingpong_blk;
> +
> + return c;
> +}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
> new file mode 100644
> index 000000000000..1ca806f9d18d
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
> @@ -0,0 +1,114 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef _DPU_HW_CDM_H
> +#define _DPU_HW_CDM_H
> +
> +#include "dpu_hw_mdss.h"
> +#include "dpu_hw_top.h"
> +
> +struct dpu_hw_cdm;
> +
> +struct dpu_hw_cdm_cfg {
> + u32 output_width;
> + u32 output_height;
> + u32 output_bit_depth;
> + u32 h_cdwn_type;
> + u32 v_cdwn_type;
> + const struct dpu_format *output_fmt;
> + const struct dpu_csc_cfg *csc_cfg;
> + u32 output_type;
> + int pp_id;
> +};
> +
> +enum dpu_hw_cdwn_type {
> + CDM_CDWN_DISABLE,
> + CDM_CDWN_PIXEL_DROP,
> + CDM_CDWN_AVG,
> + CDM_CDWN_COSITE,
> + CDM_CDWN_OFFSITE,
> +};
> +
> +enum dpu_hw_cdwn_output_type {
> + CDM_CDWN_OUTPUT_HDMI,
> + CDM_CDWN_OUTPUT_WB,
> +};
> +
> +enum dpu_hw_cdwn_output_bit_depth {
> + CDM_CDWN_OUTPUT_8BIT,
> + CDM_CDWN_OUTPUT_10BIT,
> +};

Can we please get some documentation for these enums?

> +
> +/**
> + * struct dpu_hw_cdm_ops : Interface to the chroma down Hw driver functions
> + * Assumption is these functions will be called after
> + * clocks are enabled
> + * @enable: Enables the output to interface and programs the
> + * output packer
> + * @disable: Puts the cdm in bypass mode
> + * @bind_pingpong_blk: enable/disable the connection with pingpong which
> + * will feed pixels to this cdm
> + */
> +struct dpu_hw_cdm_ops {
> + /**
> + * Enable the CDM module
> + * @cdm Pointer to chroma down context
> + */
> + int (*enable)(struct dpu_hw_cdm *cdm, struct dpu_hw_cdm_cfg *cfg);
> +
> + /**
> + * Disable the CDM module
> + * @cdm Pointer to chroma down context
> + */
> + void (*disable)(struct dpu_hw_cdm *cdm);
> +
> + /**
> + * Enable/disable the connection with pingpong
> + * @cdm Pointer to chroma down context
> + * @enable Enable/disable control
> + * @pp pingpong block id.
> + */
> + void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, bool enable,
> + const enum dpu_pingpong pp);
> +};
> +
> +/**
> + * struct dpu_hw_cdm - cdm description
> + * @base: Hardware block base structure
> + * @hw: Block hardware details
> + * @idx: CDM index
> + * @caps: Pointer to cdm_cfg
> + * @ops: handle to operations possible for this CDM
> + */
> +struct dpu_hw_cdm {
> + struct dpu_hw_blk base;
> + struct dpu_hw_blk_reg_map hw;
> +
> + /* chroma down */
> + const struct dpu_cdm_cfg *caps;
> + enum dpu_cdm idx;
> +
> + /* ops */
> + struct dpu_hw_cdm_ops ops;
> +};
> +
> +/**
> + * dpu_hw_cdm_init - initializes the cdm hw driver object.
> + * should be called once before accessing every cdm.
> + * @dev: DRM device handle
> + * @cdm: CDM catalog entry for which driver object is required
> + * @addr : mapped register io address of MDSS
> + * @mdss_rev: mdss hw core revision
> + */
> +struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
> + const struct dpu_cdm_cfg *cdm, void __iomem *addr,
> + const struct dpu_mdss_version *mdss_rev);
> +
> +static inline struct dpu_hw_cdm *to_dpu_hw_cdm(struct dpu_hw_blk *hw)
> +{
> + return container_of(hw, struct dpu_hw_cdm, base);
> +}
> +
> +#endif /*_DPU_HW_CDM_H */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index f319c8232ea5..9db4cf61bd29 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -466,6 +466,7 @@ struct dpu_mdss_color {
> #define DPU_DBG_MASK_ROT (1 << 9)
> #define DPU_DBG_MASK_DSPP (1 << 10)
> #define DPU_DBG_MASK_DSC (1 << 11)
> +#define DPU_DBG_MASK_CDM (1 << 12)
>
> /**
> * struct dpu_hw_tear_check - Struct contains parameters to configure
> --
> 2.40.1
>


--
With best wishes

Dmitry

2023-12-08 17:10:08

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH v2 07/16] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block



On 12/8/2023 4:06 AM, Dmitry Baryshkov wrote:
> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> wrote:
>>
>> CDM block comes with its own set of registers and operations
>> which can be done. In-line with other hardware sub-blocks, this
>
> I always thought that sub-blocks refer to the dpu_foo_sub_blks data,
> which CDM doesn't have.
>

All of these are DPU-sub blks in some sense. If this is confusing to
you, I will just say "in-line with other DPU hardware blocks".

>
>> change adds the dpu_hw_cdm abstraction for the CDM block.
>>
>> changes in v2:
>> - replace bit magic with relevant defines
>> - use drmm_kzalloc instead of kzalloc/free
>> - some formatting fixes
>> - inline _setup_cdm_ops()
>> - protect bind_pingpong_blk with core_rev check
>> - drop setup_csc_data() and setup_cdwn() ops as they
>> are merged into enable()
>>
>> Signed-off-by: Abhinav Kumar <[email protected]>
>> ---
>> drivers/gpu/drm/msm/Makefile | 1 +
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 276 ++++++++++++++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 114 ++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
>> 4 files changed, 392 insertions(+)
>> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
>> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
>>
>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>> index 49671364fdcf..b1173128b5b9 100644
>> --- a/drivers/gpu/drm/msm/Makefile
>> +++ b/drivers/gpu/drm/msm/Makefile
>> @@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
>> disp/dpu1/dpu_encoder_phys_wb.o \
>> disp/dpu1/dpu_formats.o \
>> disp/dpu1/dpu_hw_catalog.o \
>> + disp/dpu1/dpu_hw_cdm.o \
>> disp/dpu1/dpu_hw_ctl.o \
>> disp/dpu1/dpu_hw_dsc.o \
>> disp/dpu1/dpu_hw_dsc_1_2.o \
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
>> new file mode 100644
>> index 000000000000..0dbe2df56cc8
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
>> @@ -0,0 +1,276 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <drm/drm_managed.h>
>> +
>> +#include "dpu_hw_mdss.h"
>> +#include "dpu_hw_util.h"
>> +#include "dpu_hw_catalog.h"
>> +#include "dpu_hw_cdm.h"
>> +#include "dpu_kms.h"
>> +
>> +#define CDM_CSC_10_OPMODE 0x000
>> +#define CDM_CSC_10_BASE 0x004
>> +
>> +#define CDM_CDWN2_OP_MODE 0x100
>> +#define CDM_CDWN2_CLAMP_OUT 0x104
>> +#define CDM_CDWN2_PARAMS_3D_0 0x108
>> +#define CDM_CDWN2_PARAMS_3D_1 0x10C
>> +#define CDM_CDWN2_COEFF_COSITE_H_0 0x110
>> +#define CDM_CDWN2_COEFF_COSITE_H_1 0x114
>> +#define CDM_CDWN2_COEFF_COSITE_H_2 0x118
>> +#define CDM_CDWN2_COEFF_OFFSITE_H_0 0x11C
>> +#define CDM_CDWN2_COEFF_OFFSITE_H_1 0x120
>> +#define CDM_CDWN2_COEFF_OFFSITE_H_2 0x124
>> +#define CDM_CDWN2_COEFF_COSITE_V 0x128
>> +#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C
>> +#define CDM_CDWN2_OUT_SIZE 0x130
>> +
>> +#define CDM_HDMI_PACK_OP_MODE 0x200
>> +#define CDM_CSC_10_MATRIX_COEFF_0 0x004
>> +
>> +#define CDM_MUX 0x224
>> +
>> +/* CDM CDWN2 sub-block bit definitions */
>> +#define CDM_CDWN2_OP_MODE_EN BIT(0)
>> +#define CDM_CDWN2_OP_MODE_ENABLE_H BIT(1)
>> +#define CDM_CDWN2_OP_MODE_ENABLE_V BIT(2)
>> +#define CDM_CDWN2_OP_MODE_METHOD_H_AVG BIT(3)
>> +#define CDM_CDWN2_OP_MODE_METHOD_H_COSITE BIT(4)
>> +#define CDM_CDWN2_OP_MODE_METHOD_V_AVG BIT(5)
>> +#define CDM_CDWN2_OP_MODE_METHOD_V_COSITE BIT(6)
>> +#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7)
>> +#define CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE GENMASK(4, 3)
>> +#define CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE GENMASK(6, 5)
>> +#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5)
>> +#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3)
>> +
>> +/* CDM CSC10 sub-block bit definitions */
>> +#define CDM_CSC10_OP_MODE_EN BIT(0)
>> +#define CDM_CSC10_OP_MODE_SRC_FMT_YUV BIT(1)
>> +#define CDM_CSC10_OP_MODE_DST_FMT_YUV BIT(2)
>> +
>> +/* CDM HDMI pack sub-block bit definitions */
>> +#define CDM_HDMI_PACK_OP_MODE_EN BIT(0)
>> +
>> +/**
>> + * Horizontal coefficients for cosite chroma downscale
>> + * s13 representation of coefficients
>> + */
>> +static u32 cosite_h_coeff[] = {0x00000016, 0x000001cc, 0x0100009e};
>> +
>> +/**
>> + * Horizontal coefficients for offsite chroma downscale
>> + */
>> +static u32 offsite_h_coeff[] = {0x000b0005, 0x01db01eb, 0x00e40046};
>> +
>> +/**
>> + * Vertical coefficients for cosite chroma downscale
>> + */
>> +static u32 cosite_v_coeff[] = {0x00080004};
>> +/**
>> + * Vertical coefficients for offsite chroma downscale
>> + */
>> +static u32 offsite_v_coeff[] = {0x00060002};
>> +
>> +static int dpu_hw_cdm_setup_cdwn(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cfg)
>> +{
>> + struct dpu_hw_blk_reg_map *c = &ctx->hw;
>> + u32 opmode = 0;
>> + u32 out_size = 0;
>> +
>> + if (cfg->output_bit_depth == CDM_CDWN_OUTPUT_10BIT)
>> + opmode &= ~CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
>
> We start from opmode = 0. Does it really make sense to mask bits from
> the zero opmode?
>

Ack, We can drop the ~ and just keep the | below.

>> + else
>> + opmode |= CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
>> +
>> + /* ENABLE DWNS_H bit */
>> + opmode |= CDM_CDWN2_OP_MODE_ENABLE_H;
>> +
>> + switch (cfg->h_cdwn_type) {
>> + case CDM_CDWN_DISABLE:
>> + /* CLEAR METHOD_H field */
>> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
>> + /* CLEAR DWNS_H bit */
>> + opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_H;
>> + break;
>> + case CDM_CDWN_PIXEL_DROP:
>> + /* Clear METHOD_H field (pixel drop is 0) */
>> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
>> + break;
>> + case CDM_CDWN_AVG:
>> + /* Clear METHOD_H field (Average is 0x1) */
>> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
>> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_AVG;
>> + break;
>> + case CDM_CDWN_COSITE:
>> + /* Clear METHOD_H field (Average is 0x2) */
>> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
>> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_COSITE;
>> + /* Co-site horizontal coefficients */
>> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_0,
>> + cosite_h_coeff[0]);
>> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_1,
>> + cosite_h_coeff[1]);
>> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_2,
>> + cosite_h_coeff[2]);
>> + break;
>> + case CDM_CDWN_OFFSITE:
>> + /* Clear METHOD_H field (Average is 0x3) */
>> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
>> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE;
>> +
>> + /* Off-site horizontal coefficients */
>> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_0,
>> + offsite_h_coeff[0]);
>> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_1,
>> + offsite_h_coeff[1]);
>> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_2,
>> + offsite_h_coeff[2]);
>> + break;
>> + default:
>> + pr_err("%s invalid horz down sampling type\n", __func__);
>
> DPU_ERROR or drm_err
>

Ack.

>> + return -EINVAL;
>> + }
>> +
>> + /* ENABLE DWNS_V bit */
>> + opmode |= CDM_CDWN2_OP_MODE_ENABLE_V;
>> +
>> + switch (cfg->v_cdwn_type) {
>> + case CDM_CDWN_DISABLE:
>> + /* CLEAR METHOD_V field */
>> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
>> + /* CLEAR DWNS_V bit */
>> + opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_V;
>> + break;
>> + case CDM_CDWN_PIXEL_DROP:
>> + /* Clear METHOD_V field (pixel drop is 0) */
>> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
>> + break;
>> + case CDM_CDWN_AVG:
>> + /* Clear METHOD_V field (Average is 0x1) */
>> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
>> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_AVG;
>> + break;
>> + case CDM_CDWN_COSITE:
>> + /* Clear METHOD_V field (Average is 0x2) */
>> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
>> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_COSITE;
>> + /* Co-site vertical coefficients */
>> + DPU_REG_WRITE(c,
>> + CDM_CDWN2_COEFF_COSITE_V,
>> + cosite_v_coeff[0]);
>> + break;
>> + case CDM_CDWN_OFFSITE:
>> + /* Clear METHOD_V field (Average is 0x3) */
>> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
>> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE;
>> +
>> + /* Off-site vertical coefficients */
>> + DPU_REG_WRITE(c,
>> + CDM_CDWN2_COEFF_OFFSITE_V,
>> + offsite_v_coeff[0]);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + if (cfg->v_cdwn_type || cfg->h_cdwn_type)
>> + opmode |= CDM_CDWN2_OP_MODE_EN; /* EN CDWN module */
>> + else
>> + opmode &= ~CDM_CDWN2_OP_MODE_EN;
>> +
>> + out_size = (cfg->output_width & 0xFFFF) | ((cfg->output_height & 0xFFFF) << 16);
>> + DPU_REG_WRITE(c, CDM_CDWN2_OUT_SIZE, out_size);
>> + DPU_REG_WRITE(c, CDM_CDWN2_OP_MODE, opmode);
>> + DPU_REG_WRITE(c, CDM_CDWN2_CLAMP_OUT, ((0x3FF << 16) | 0x0));
>> +
>> + return 0;
>> +}
>> +
>> +int dpu_hw_cdm_enable(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cdm)
>> +{
>> + struct dpu_hw_blk_reg_map *c = &ctx->hw;
>> + const struct dpu_format *fmt;
>> + u32 opmode = 0;
>> + u32 csc = 0;
>> +
>> + if (!ctx || !cdm)
>> + return -EINVAL;
>> +
>> + fmt = cdm->output_fmt;
>> +
>> + if (!DPU_FORMAT_IS_YUV(fmt))
>> + return -EINVAL;
>> +
>> + dpu_hw_csc_setup(&ctx->hw, CDM_CSC_10_MATRIX_COEFF_0, cdm->csc_cfg, true);
>> + dpu_hw_cdm_setup_cdwn(ctx, cdm);
>> +
>> + if (cdm->output_type == CDM_CDWN_OUTPUT_HDMI) {
>> + if (fmt->chroma_sample != DPU_CHROMA_H1V2)
>> + return -EINVAL; /*unsupported format */
>> + opmode = CDM_HDMI_PACK_OP_MODE_EN;
>> + opmode |= (fmt->chroma_sample << 1);
>> + }
>> +
>> + csc |= CDM_CSC10_OP_MODE_DST_FMT_YUV;
>> + csc &= ~CDM_CSC10_OP_MODE_SRC_FMT_YUV;
>> + csc |= CDM_CSC10_OP_MODE_EN;
>> +
>> + if (ctx && ctx->ops.bind_pingpong_blk)
>> + ctx->ops.bind_pingpong_blk(ctx, true, cdm->pp_id);
>> +
>> + DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc);
>> + DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode);
>> + return 0;
>> +}
>> +
>> +void dpu_hw_cdm_disable(struct dpu_hw_cdm *ctx)
>> +{
>> + if (!ctx)
>> + return;
>> +
>> + if (ctx && ctx->ops.bind_pingpong_blk)
>> + ctx->ops.bind_pingpong_blk(ctx, false, PINGPONG_NONE);
>
> So the bind/un_pingpong_block gets hidden here. Why do we need to
> unbind it manually in the dpu_encoder then?
>

hmm .... I think we can drop the disable op and just call
bind_pingpong_blk directly.

>> +}
>> +
>> +static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, bool enable,
>> + const enum dpu_pingpong pp)
>
> I think we settled on the PINGPONG_NONE for removing the binding
>

Ah okay. i probably missed this. so just drop "enable" and use pp val.

>> +{
>> + struct dpu_hw_blk_reg_map *c;
>> + int mux_cfg = 0xF;
>> +
>> + c = &ctx->hw;
>> +
>> + if (enable)
>> + mux_cfg = (pp - PINGPONG_0) & 0x7;
>> +
>> + DPU_REG_WRITE(c, CDM_MUX, mux_cfg);
>> +}
>> +
>> +struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
>> + const struct dpu_cdm_cfg *cfg, void __iomem *addr,
>> + const struct dpu_mdss_version *mdss_rev)
>> +{
>> + struct dpu_hw_cdm *c;
>> +
>> + c = drmm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
>> + if (!c)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + c->hw.blk_addr = addr + cfg->base;
>> + c->hw.log_mask = DPU_DBG_MASK_CDM;
>> +
>> + /* Assign ops */
>> + c->idx = cfg->id;
>> + c->caps = cfg;
>> +
>> + c->ops.enable = dpu_hw_cdm_enable;
>> + c->ops.disable = dpu_hw_cdm_disable;
>> + if (mdss_rev->core_major_ver >= 5)
>> + c->ops.bind_pingpong_blk = dpu_hw_cdm_bind_pingpong_blk;
>> +
>> + return c;
>> +}
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
>> new file mode 100644
>> index 000000000000..1ca806f9d18d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
>> @@ -0,0 +1,114 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#ifndef _DPU_HW_CDM_H
>> +#define _DPU_HW_CDM_H
>> +
>> +#include "dpu_hw_mdss.h"
>> +#include "dpu_hw_top.h"
>> +
>> +struct dpu_hw_cdm;
>> +
>> +struct dpu_hw_cdm_cfg {
>> + u32 output_width;
>> + u32 output_height;
>> + u32 output_bit_depth;
>> + u32 h_cdwn_type;
>> + u32 v_cdwn_type;
>> + const struct dpu_format *output_fmt;
>> + const struct dpu_csc_cfg *csc_cfg;
>> + u32 output_type;
>> + int pp_id;
>> +};
>> +
>> +enum dpu_hw_cdwn_type {
>> + CDM_CDWN_DISABLE,
>> + CDM_CDWN_PIXEL_DROP,
>> + CDM_CDWN_AVG,
>> + CDM_CDWN_COSITE,
>> + CDM_CDWN_OFFSITE,
>> +};
>> +
>> +enum dpu_hw_cdwn_output_type {
>> + CDM_CDWN_OUTPUT_HDMI,
>> + CDM_CDWN_OUTPUT_WB,
>> +};
>> +
>> +enum dpu_hw_cdwn_output_bit_depth {
>> + CDM_CDWN_OUTPUT_8BIT,
>> + CDM_CDWN_OUTPUT_10BIT,
>> +};
>
> Can we please get some documentation for these enums?
>

Ack.

>> +
>> +/**
>> + * struct dpu_hw_cdm_ops : Interface to the chroma down Hw driver functions
>> + * Assumption is these functions will be called after
>> + * clocks are enabled
>> + * @enable: Enables the output to interface and programs the
>> + * output packer
>> + * @disable: Puts the cdm in bypass mode
>> + * @bind_pingpong_blk: enable/disable the connection with pingpong which
>> + * will feed pixels to this cdm
>> + */
>> +struct dpu_hw_cdm_ops {
>> + /**
>> + * Enable the CDM module
>> + * @cdm Pointer to chroma down context
>> + */
>> + int (*enable)(struct dpu_hw_cdm *cdm, struct dpu_hw_cdm_cfg *cfg);
>> +
>> + /**
>> + * Disable the CDM module
>> + * @cdm Pointer to chroma down context
>> + */
>> + void (*disable)(struct dpu_hw_cdm *cdm);
>> +
>> + /**
>> + * Enable/disable the connection with pingpong
>> + * @cdm Pointer to chroma down context
>> + * @enable Enable/disable control
>> + * @pp pingpong block id.
>> + */
>> + void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, bool enable,
>> + const enum dpu_pingpong pp);
>> +};
>> +
>> +/**
>> + * struct dpu_hw_cdm - cdm description
>> + * @base: Hardware block base structure
>> + * @hw: Block hardware details
>> + * @idx: CDM index
>> + * @caps: Pointer to cdm_cfg
>> + * @ops: handle to operations possible for this CDM
>> + */
>> +struct dpu_hw_cdm {
>> + struct dpu_hw_blk base;
>> + struct dpu_hw_blk_reg_map hw;
>> +
>> + /* chroma down */
>> + const struct dpu_cdm_cfg *caps;
>> + enum dpu_cdm idx;
>> +
>> + /* ops */
>> + struct dpu_hw_cdm_ops ops;
>> +};
>> +
>> +/**
>> + * dpu_hw_cdm_init - initializes the cdm hw driver object.
>> + * should be called once before accessing every cdm.
>> + * @dev: DRM device handle
>> + * @cdm: CDM catalog entry for which driver object is required
>> + * @addr : mapped register io address of MDSS
>> + * @mdss_rev: mdss hw core revision
>> + */
>> +struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
>> + const struct dpu_cdm_cfg *cdm, void __iomem *addr,
>> + const struct dpu_mdss_version *mdss_rev);
>> +
>> +static inline struct dpu_hw_cdm *to_dpu_hw_cdm(struct dpu_hw_blk *hw)
>> +{
>> + return container_of(hw, struct dpu_hw_cdm, base);
>> +}
>> +
>> +#endif /*_DPU_HW_CDM_H */
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>> index f319c8232ea5..9db4cf61bd29 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>> @@ -466,6 +466,7 @@ struct dpu_mdss_color {
>> #define DPU_DBG_MASK_ROT (1 << 9)
>> #define DPU_DBG_MASK_DSPP (1 << 10)
>> #define DPU_DBG_MASK_DSC (1 << 11)
>> +#define DPU_DBG_MASK_CDM (1 << 12)
>>
>> /**
>> * struct dpu_hw_tear_check - Struct contains parameters to configure
>> --
>> 2.40.1
>>
>
>
> --
> With best wishes
>
> Dmitry

2023-12-08 18:17:51

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2 07/16] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block

On Fri, 8 Dec 2023 at 19:09, Abhinav Kumar <[email protected]> wrote:
>
>
>
> On 12/8/2023 4:06 AM, Dmitry Baryshkov wrote:
> > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <[email protected]> wrote:
> >>
> >> CDM block comes with its own set of registers and operations
> >> which can be done. In-line with other hardware sub-blocks, this
> >
> > I always thought that sub-blocks refer to the dpu_foo_sub_blks data,
> > which CDM doesn't have.
> >
>
> All of these are DPU-sub blks in some sense. If this is confusing to
> you, I will just say "in-line with other DPU hardware blocks".

Yes, please.

>
> >
> >> change adds the dpu_hw_cdm abstraction for the CDM block.
> >>
> >> changes in v2:
> >> - replace bit magic with relevant defines
> >> - use drmm_kzalloc instead of kzalloc/free
> >> - some formatting fixes
> >> - inline _setup_cdm_ops()
> >> - protect bind_pingpong_blk with core_rev check
> >> - drop setup_csc_data() and setup_cdwn() ops as they
> >> are merged into enable()
> >>
> >> Signed-off-by: Abhinav Kumar <[email protected]>
> >> ---
> >> drivers/gpu/drm/msm/Makefile | 1 +
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c | 276 ++++++++++++++++++++
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 114 ++++++++
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
> >> 4 files changed, 392 insertions(+)
> >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> >> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
> >>
> >> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> >> index 49671364fdcf..b1173128b5b9 100644
> >> --- a/drivers/gpu/drm/msm/Makefile
> >> +++ b/drivers/gpu/drm/msm/Makefile
> >> @@ -63,6 +63,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
> >> disp/dpu1/dpu_encoder_phys_wb.o \
> >> disp/dpu1/dpu_formats.o \
> >> disp/dpu1/dpu_hw_catalog.o \
> >> + disp/dpu1/dpu_hw_cdm.o \
> >> disp/dpu1/dpu_hw_ctl.o \
> >> disp/dpu1/dpu_hw_dsc.o \
> >> disp/dpu1/dpu_hw_dsc_1_2.o \
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> >> new file mode 100644
> >> index 000000000000..0dbe2df56cc8
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c
> >> @@ -0,0 +1,276 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
> >> + */
> >> +
> >> +#include <drm/drm_managed.h>
> >> +
> >> +#include "dpu_hw_mdss.h"
> >> +#include "dpu_hw_util.h"
> >> +#include "dpu_hw_catalog.h"
> >> +#include "dpu_hw_cdm.h"
> >> +#include "dpu_kms.h"
> >> +
> >> +#define CDM_CSC_10_OPMODE 0x000
> >> +#define CDM_CSC_10_BASE 0x004
> >> +
> >> +#define CDM_CDWN2_OP_MODE 0x100
> >> +#define CDM_CDWN2_CLAMP_OUT 0x104
> >> +#define CDM_CDWN2_PARAMS_3D_0 0x108
> >> +#define CDM_CDWN2_PARAMS_3D_1 0x10C
> >> +#define CDM_CDWN2_COEFF_COSITE_H_0 0x110
> >> +#define CDM_CDWN2_COEFF_COSITE_H_1 0x114
> >> +#define CDM_CDWN2_COEFF_COSITE_H_2 0x118
> >> +#define CDM_CDWN2_COEFF_OFFSITE_H_0 0x11C
> >> +#define CDM_CDWN2_COEFF_OFFSITE_H_1 0x120
> >> +#define CDM_CDWN2_COEFF_OFFSITE_H_2 0x124
> >> +#define CDM_CDWN2_COEFF_COSITE_V 0x128
> >> +#define CDM_CDWN2_COEFF_OFFSITE_V 0x12C
> >> +#define CDM_CDWN2_OUT_SIZE 0x130
> >> +
> >> +#define CDM_HDMI_PACK_OP_MODE 0x200
> >> +#define CDM_CSC_10_MATRIX_COEFF_0 0x004
> >> +
> >> +#define CDM_MUX 0x224
> >> +
> >> +/* CDM CDWN2 sub-block bit definitions */
> >> +#define CDM_CDWN2_OP_MODE_EN BIT(0)
> >> +#define CDM_CDWN2_OP_MODE_ENABLE_H BIT(1)
> >> +#define CDM_CDWN2_OP_MODE_ENABLE_V BIT(2)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_H_AVG BIT(3)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_H_COSITE BIT(4)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_V_AVG BIT(5)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_V_COSITE BIT(6)
> >> +#define CDM_CDWN2_OP_MODE_BITS_OUT_8BIT BIT(7)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE GENMASK(4, 3)
> >> +#define CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE GENMASK(6, 5)
> >> +#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5)
> >> +#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3)
> >> +
> >> +/* CDM CSC10 sub-block bit definitions */
> >> +#define CDM_CSC10_OP_MODE_EN BIT(0)
> >> +#define CDM_CSC10_OP_MODE_SRC_FMT_YUV BIT(1)
> >> +#define CDM_CSC10_OP_MODE_DST_FMT_YUV BIT(2)
> >> +
> >> +/* CDM HDMI pack sub-block bit definitions */
> >> +#define CDM_HDMI_PACK_OP_MODE_EN BIT(0)
> >> +
> >> +/**
> >> + * Horizontal coefficients for cosite chroma downscale
> >> + * s13 representation of coefficients
> >> + */
> >> +static u32 cosite_h_coeff[] = {0x00000016, 0x000001cc, 0x0100009e};
> >> +
> >> +/**
> >> + * Horizontal coefficients for offsite chroma downscale
> >> + */
> >> +static u32 offsite_h_coeff[] = {0x000b0005, 0x01db01eb, 0x00e40046};
> >> +
> >> +/**
> >> + * Vertical coefficients for cosite chroma downscale
> >> + */
> >> +static u32 cosite_v_coeff[] = {0x00080004};
> >> +/**
> >> + * Vertical coefficients for offsite chroma downscale
> >> + */
> >> +static u32 offsite_v_coeff[] = {0x00060002};
> >> +
> >> +static int dpu_hw_cdm_setup_cdwn(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cfg)
> >> +{
> >> + struct dpu_hw_blk_reg_map *c = &ctx->hw;
> >> + u32 opmode = 0;
> >> + u32 out_size = 0;
> >> +
> >> + if (cfg->output_bit_depth == CDM_CDWN_OUTPUT_10BIT)
> >> + opmode &= ~CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
> >
> > We start from opmode = 0. Does it really make sense to mask bits from
> > the zero opmode?
> >
>
> Ack, We can drop the ~ and just keep the | below.

Yep. I think it will make things more clear. We don't have 'update'
operation, we always setup CDM from the ground up.

>
> >> + else
> >> + opmode |= CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
> >> +
> >> + /* ENABLE DWNS_H bit */
> >> + opmode |= CDM_CDWN2_OP_MODE_ENABLE_H;
> >> +
> >> + switch (cfg->h_cdwn_type) {
> >> + case CDM_CDWN_DISABLE:
> >> + /* CLEAR METHOD_H field */
> >> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> >> + /* CLEAR DWNS_H bit */
> >> + opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_H;
> >> + break;
> >> + case CDM_CDWN_PIXEL_DROP:
> >> + /* Clear METHOD_H field (pixel drop is 0) */
> >> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> >> + break;
> >> + case CDM_CDWN_AVG:
> >> + /* Clear METHOD_H field (Average is 0x1) */
> >> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> >> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_AVG;
> >> + break;
> >> + case CDM_CDWN_COSITE:
> >> + /* Clear METHOD_H field (Average is 0x2) */
> >> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> >> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_COSITE;
> >> + /* Co-site horizontal coefficients */
> >> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_0,
> >> + cosite_h_coeff[0]);
> >> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_1,
> >> + cosite_h_coeff[1]);
> >> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_2,
> >> + cosite_h_coeff[2]);
> >> + break;
> >> + case CDM_CDWN_OFFSITE:
> >> + /* Clear METHOD_H field (Average is 0x3) */
> >> + opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
> >> + opmode |= CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE;
> >> +
> >> + /* Off-site horizontal coefficients */
> >> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_0,
> >> + offsite_h_coeff[0]);
> >> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_1,
> >> + offsite_h_coeff[1]);
> >> + DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_2,
> >> + offsite_h_coeff[2]);
> >> + break;
> >> + default:
> >> + pr_err("%s invalid horz down sampling type\n", __func__);
> >
> > DPU_ERROR or drm_err
> >
>
> Ack.
>
> >> + return -EINVAL;
> >> + }
> >> +
> >> + /* ENABLE DWNS_V bit */
> >> + opmode |= CDM_CDWN2_OP_MODE_ENABLE_V;
> >> +
> >> + switch (cfg->v_cdwn_type) {
> >> + case CDM_CDWN_DISABLE:
> >> + /* CLEAR METHOD_V field */
> >> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> >> + /* CLEAR DWNS_V bit */
> >> + opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_V;
> >> + break;
> >> + case CDM_CDWN_PIXEL_DROP:
> >> + /* Clear METHOD_V field (pixel drop is 0) */
> >> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> >> + break;
> >> + case CDM_CDWN_AVG:
> >> + /* Clear METHOD_V field (Average is 0x1) */
> >> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> >> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_AVG;
> >> + break;
> >> + case CDM_CDWN_COSITE:
> >> + /* Clear METHOD_V field (Average is 0x2) */
> >> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> >> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_COSITE;
> >> + /* Co-site vertical coefficients */
> >> + DPU_REG_WRITE(c,
> >> + CDM_CDWN2_COEFF_COSITE_V,
> >> + cosite_v_coeff[0]);
> >> + break;
> >> + case CDM_CDWN_OFFSITE:
> >> + /* Clear METHOD_V field (Average is 0x3) */
> >> + opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
> >> + opmode |= CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE;
> >> +
> >> + /* Off-site vertical coefficients */
> >> + DPU_REG_WRITE(c,
> >> + CDM_CDWN2_COEFF_OFFSITE_V,
> >> + offsite_v_coeff[0]);
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (cfg->v_cdwn_type || cfg->h_cdwn_type)
> >> + opmode |= CDM_CDWN2_OP_MODE_EN; /* EN CDWN module */
> >> + else
> >> + opmode &= ~CDM_CDWN2_OP_MODE_EN;
> >> +
> >> + out_size = (cfg->output_width & 0xFFFF) | ((cfg->output_height & 0xFFFF) << 16);
> >> + DPU_REG_WRITE(c, CDM_CDWN2_OUT_SIZE, out_size);
> >> + DPU_REG_WRITE(c, CDM_CDWN2_OP_MODE, opmode);
> >> + DPU_REG_WRITE(c, CDM_CDWN2_CLAMP_OUT, ((0x3FF << 16) | 0x0));
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +int dpu_hw_cdm_enable(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cdm)
> >> +{
> >> + struct dpu_hw_blk_reg_map *c = &ctx->hw;
> >> + const struct dpu_format *fmt;
> >> + u32 opmode = 0;
> >> + u32 csc = 0;
> >> +
> >> + if (!ctx || !cdm)
> >> + return -EINVAL;
> >> +
> >> + fmt = cdm->output_fmt;
> >> +
> >> + if (!DPU_FORMAT_IS_YUV(fmt))
> >> + return -EINVAL;
> >> +
> >> + dpu_hw_csc_setup(&ctx->hw, CDM_CSC_10_MATRIX_COEFF_0, cdm->csc_cfg, true);
> >> + dpu_hw_cdm_setup_cdwn(ctx, cdm);
> >> +
> >> + if (cdm->output_type == CDM_CDWN_OUTPUT_HDMI) {
> >> + if (fmt->chroma_sample != DPU_CHROMA_H1V2)
> >> + return -EINVAL; /*unsupported format */
> >> + opmode = CDM_HDMI_PACK_OP_MODE_EN;
> >> + opmode |= (fmt->chroma_sample << 1);
> >> + }
> >> +
> >> + csc |= CDM_CSC10_OP_MODE_DST_FMT_YUV;
> >> + csc &= ~CDM_CSC10_OP_MODE_SRC_FMT_YUV;
> >> + csc |= CDM_CSC10_OP_MODE_EN;
> >> +
> >> + if (ctx && ctx->ops.bind_pingpong_blk)
> >> + ctx->ops.bind_pingpong_blk(ctx, true, cdm->pp_id);
> >> +
> >> + DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc);
> >> + DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode);
> >> + return 0;
> >> +}
> >> +
> >> +void dpu_hw_cdm_disable(struct dpu_hw_cdm *ctx)
> >> +{
> >> + if (!ctx)
> >> + return;
> >> +
> >> + if (ctx && ctx->ops.bind_pingpong_blk)
> >> + ctx->ops.bind_pingpong_blk(ctx, false, PINGPONG_NONE);
> >
> > So the bind/un_pingpong_block gets hidden here. Why do we need to
> > unbind it manually in the dpu_encoder then?
> >
>
> hmm .... I think we can drop the disable op and just call
> bind_pingpong_blk directly.
>
> >> +}
> >> +
> >> +static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, bool enable,
> >> + const enum dpu_pingpong pp)
> >
> > I think we settled on the PINGPONG_NONE for removing the binding
> >
>
> Ah okay. i probably missed this. so just drop "enable" and use pp val.
>
> >> +{
> >> + struct dpu_hw_blk_reg_map *c;
> >> + int mux_cfg = 0xF;
> >> +
> >> + c = &ctx->hw;
> >> +
> >> + if (enable)
> >> + mux_cfg = (pp - PINGPONG_0) & 0x7;
> >> +
> >> + DPU_REG_WRITE(c, CDM_MUX, mux_cfg);
> >> +}
> >> +
> >> +struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
> >> + const struct dpu_cdm_cfg *cfg, void __iomem *addr,
> >> + const struct dpu_mdss_version *mdss_rev)
> >> +{
> >> + struct dpu_hw_cdm *c;
> >> +
> >> + c = drmm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> >> + if (!c)
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + c->hw.blk_addr = addr + cfg->base;
> >> + c->hw.log_mask = DPU_DBG_MASK_CDM;
> >> +
> >> + /* Assign ops */
> >> + c->idx = cfg->id;
> >> + c->caps = cfg;
> >> +
> >> + c->ops.enable = dpu_hw_cdm_enable;
> >> + c->ops.disable = dpu_hw_cdm_disable;
> >> + if (mdss_rev->core_major_ver >= 5)
> >> + c->ops.bind_pingpong_blk = dpu_hw_cdm_bind_pingpong_blk;
> >> +
> >> + return c;
> >> +}
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
> >> new file mode 100644
> >> index 000000000000..1ca806f9d18d
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h
> >> @@ -0,0 +1,114 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-only */
> >> +/*
> >> + * Copyright (c) 2023, The Linux Foundation. All rights reserved.
> >> + */
> >> +
> >> +#ifndef _DPU_HW_CDM_H
> >> +#define _DPU_HW_CDM_H
> >> +
> >> +#include "dpu_hw_mdss.h"
> >> +#include "dpu_hw_top.h"
> >> +
> >> +struct dpu_hw_cdm;
> >> +
> >> +struct dpu_hw_cdm_cfg {
> >> + u32 output_width;
> >> + u32 output_height;
> >> + u32 output_bit_depth;
> >> + u32 h_cdwn_type;
> >> + u32 v_cdwn_type;
> >> + const struct dpu_format *output_fmt;
> >> + const struct dpu_csc_cfg *csc_cfg;
> >> + u32 output_type;
> >> + int pp_id;
> >> +};
> >> +
> >> +enum dpu_hw_cdwn_type {
> >> + CDM_CDWN_DISABLE,
> >> + CDM_CDWN_PIXEL_DROP,
> >> + CDM_CDWN_AVG,
> >> + CDM_CDWN_COSITE,
> >> + CDM_CDWN_OFFSITE,
> >> +};
> >> +
> >> +enum dpu_hw_cdwn_output_type {
> >> + CDM_CDWN_OUTPUT_HDMI,
> >> + CDM_CDWN_OUTPUT_WB,
> >> +};
> >> +
> >> +enum dpu_hw_cdwn_output_bit_depth {
> >> + CDM_CDWN_OUTPUT_8BIT,
> >> + CDM_CDWN_OUTPUT_10BIT,
> >> +};
> >
> > Can we please get some documentation for these enums?
> >
>
> Ack.
>
> >> +
> >> +/**
> >> + * struct dpu_hw_cdm_ops : Interface to the chroma down Hw driver functions
> >> + * Assumption is these functions will be called after
> >> + * clocks are enabled
> >> + * @enable: Enables the output to interface and programs the
> >> + * output packer
> >> + * @disable: Puts the cdm in bypass mode
> >> + * @bind_pingpong_blk: enable/disable the connection with pingpong which
> >> + * will feed pixels to this cdm
> >> + */
> >> +struct dpu_hw_cdm_ops {
> >> + /**
> >> + * Enable the CDM module
> >> + * @cdm Pointer to chroma down context
> >> + */
> >> + int (*enable)(struct dpu_hw_cdm *cdm, struct dpu_hw_cdm_cfg *cfg);
> >> +
> >> + /**
> >> + * Disable the CDM module
> >> + * @cdm Pointer to chroma down context
> >> + */
> >> + void (*disable)(struct dpu_hw_cdm *cdm);
> >> +
> >> + /**
> >> + * Enable/disable the connection with pingpong
> >> + * @cdm Pointer to chroma down context
> >> + * @enable Enable/disable control
> >> + * @pp pingpong block id.
> >> + */
> >> + void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, bool enable,
> >> + const enum dpu_pingpong pp);
> >> +};
> >> +
> >> +/**
> >> + * struct dpu_hw_cdm - cdm description
> >> + * @base: Hardware block base structure
> >> + * @hw: Block hardware details
> >> + * @idx: CDM index
> >> + * @caps: Pointer to cdm_cfg
> >> + * @ops: handle to operations possible for this CDM
> >> + */
> >> +struct dpu_hw_cdm {
> >> + struct dpu_hw_blk base;
> >> + struct dpu_hw_blk_reg_map hw;
> >> +
> >> + /* chroma down */
> >> + const struct dpu_cdm_cfg *caps;
> >> + enum dpu_cdm idx;
> >> +
> >> + /* ops */
> >> + struct dpu_hw_cdm_ops ops;
> >> +};
> >> +
> >> +/**
> >> + * dpu_hw_cdm_init - initializes the cdm hw driver object.
> >> + * should be called once before accessing every cdm.
> >> + * @dev: DRM device handle
> >> + * @cdm: CDM catalog entry for which driver object is required
> >> + * @addr : mapped register io address of MDSS
> >> + * @mdss_rev: mdss hw core revision
> >> + */
> >> +struct dpu_hw_cdm *dpu_hw_cdm_init(struct drm_device *dev,
> >> + const struct dpu_cdm_cfg *cdm, void __iomem *addr,
> >> + const struct dpu_mdss_version *mdss_rev);
> >> +
> >> +static inline struct dpu_hw_cdm *to_dpu_hw_cdm(struct dpu_hw_blk *hw)
> >> +{
> >> + return container_of(hw, struct dpu_hw_cdm, base);
> >> +}
> >> +
> >> +#endif /*_DPU_HW_CDM_H */
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> >> index f319c8232ea5..9db4cf61bd29 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> >> @@ -466,6 +466,7 @@ struct dpu_mdss_color {
> >> #define DPU_DBG_MASK_ROT (1 << 9)
> >> #define DPU_DBG_MASK_DSPP (1 << 10)
> >> #define DPU_DBG_MASK_DSC (1 << 11)
> >> +#define DPU_DBG_MASK_CDM (1 << 12)
> >>
> >> /**
> >> * struct dpu_hw_tear_check - Struct contains parameters to configure
> >> --
> >> 2.40.1
> >>
> >
> >
> > --
> > With best wishes
> >
> > Dmitry



--
With best wishes
Dmitry

2023-12-10 10:54:39

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 07/16] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block

Hi Abhinav,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20231207]
[also build test WARNING on v6.7-rc4]
[cannot apply to drm-misc/drm-misc-next linus/master v6.7-rc4 v6.7-rc3 v6.7-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Abhinav-Kumar/drm-msm-dpu-add-formats-check-for-writeback-encoder/20231208-130820
base: next-20231207
patch link: https://lore.kernel.org/r/20231208050641.32582-8-quic_abhinavk%40quicinc.com
patch subject: [PATCH v2 07/16] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231210/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231210/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:193:5: warning: no previous prototype for function 'dpu_hw_cdm_enable' [-Wmissing-prototypes]
int dpu_hw_cdm_enable(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cdm)
^
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:193:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int dpu_hw_cdm_enable(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cdm)
^
static
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:230:6: warning: no previous prototype for function 'dpu_hw_cdm_disable' [-Wmissing-prototypes]
void dpu_hw_cdm_disable(struct dpu_hw_cdm *ctx)
^
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:230:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void dpu_hw_cdm_disable(struct dpu_hw_cdm *ctx)
^
static
2 warnings generated.
--
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:59: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Horizontal coefficients for cosite chroma downscale
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:65: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Horizontal coefficients for offsite chroma downscale
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:70: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Vertical coefficients for cosite chroma downscale
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c:74: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Vertical coefficients for offsite chroma downscale


vim +/dpu_hw_cdm_enable +193 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c

57
58 /**
> 59 * Horizontal coefficients for cosite chroma downscale
60 * s13 representation of coefficients
61 */
62 static u32 cosite_h_coeff[] = {0x00000016, 0x000001cc, 0x0100009e};
63
64 /**
65 * Horizontal coefficients for offsite chroma downscale
66 */
67 static u32 offsite_h_coeff[] = {0x000b0005, 0x01db01eb, 0x00e40046};
68
69 /**
70 * Vertical coefficients for cosite chroma downscale
71 */
72 static u32 cosite_v_coeff[] = {0x00080004};
73 /**
74 * Vertical coefficients for offsite chroma downscale
75 */
76 static u32 offsite_v_coeff[] = {0x00060002};
77
78 static int dpu_hw_cdm_setup_cdwn(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cfg)
79 {
80 struct dpu_hw_blk_reg_map *c = &ctx->hw;
81 u32 opmode = 0;
82 u32 out_size = 0;
83
84 if (cfg->output_bit_depth == CDM_CDWN_OUTPUT_10BIT)
85 opmode &= ~CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
86 else
87 opmode |= CDM_CDWN2_OP_MODE_BITS_OUT_8BIT;
88
89 /* ENABLE DWNS_H bit */
90 opmode |= CDM_CDWN2_OP_MODE_ENABLE_H;
91
92 switch (cfg->h_cdwn_type) {
93 case CDM_CDWN_DISABLE:
94 /* CLEAR METHOD_H field */
95 opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
96 /* CLEAR DWNS_H bit */
97 opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_H;
98 break;
99 case CDM_CDWN_PIXEL_DROP:
100 /* Clear METHOD_H field (pixel drop is 0) */
101 opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
102 break;
103 case CDM_CDWN_AVG:
104 /* Clear METHOD_H field (Average is 0x1) */
105 opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
106 opmode |= CDM_CDWN2_OP_MODE_METHOD_H_AVG;
107 break;
108 case CDM_CDWN_COSITE:
109 /* Clear METHOD_H field (Average is 0x2) */
110 opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
111 opmode |= CDM_CDWN2_OP_MODE_METHOD_H_COSITE;
112 /* Co-site horizontal coefficients */
113 DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_0,
114 cosite_h_coeff[0]);
115 DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_1,
116 cosite_h_coeff[1]);
117 DPU_REG_WRITE(c, CDM_CDWN2_COEFF_COSITE_H_2,
118 cosite_h_coeff[2]);
119 break;
120 case CDM_CDWN_OFFSITE:
121 /* Clear METHOD_H field (Average is 0x3) */
122 opmode &= ~CDM_CDWN2_H_PIXEL_DROP_MASK;
123 opmode |= CDM_CDWN2_OP_MODE_METHOD_H_OFFSITE;
124
125 /* Off-site horizontal coefficients */
126 DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_0,
127 offsite_h_coeff[0]);
128 DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_1,
129 offsite_h_coeff[1]);
130 DPU_REG_WRITE(c, CDM_CDWN2_COEFF_OFFSITE_H_2,
131 offsite_h_coeff[2]);
132 break;
133 default:
134 pr_err("%s invalid horz down sampling type\n", __func__);
135 return -EINVAL;
136 }
137
138 /* ENABLE DWNS_V bit */
139 opmode |= CDM_CDWN2_OP_MODE_ENABLE_V;
140
141 switch (cfg->v_cdwn_type) {
142 case CDM_CDWN_DISABLE:
143 /* CLEAR METHOD_V field */
144 opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
145 /* CLEAR DWNS_V bit */
146 opmode &= ~CDM_CDWN2_OP_MODE_ENABLE_V;
147 break;
148 case CDM_CDWN_PIXEL_DROP:
149 /* Clear METHOD_V field (pixel drop is 0) */
150 opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
151 break;
152 case CDM_CDWN_AVG:
153 /* Clear METHOD_V field (Average is 0x1) */
154 opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
155 opmode |= CDM_CDWN2_OP_MODE_METHOD_V_AVG;
156 break;
157 case CDM_CDWN_COSITE:
158 /* Clear METHOD_V field (Average is 0x2) */
159 opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
160 opmode |= CDM_CDWN2_OP_MODE_METHOD_V_COSITE;
161 /* Co-site vertical coefficients */
162 DPU_REG_WRITE(c,
163 CDM_CDWN2_COEFF_COSITE_V,
164 cosite_v_coeff[0]);
165 break;
166 case CDM_CDWN_OFFSITE:
167 /* Clear METHOD_V field (Average is 0x3) */
168 opmode &= ~CDM_CDWN2_V_PIXEL_DROP_MASK;
169 opmode |= CDM_CDWN2_OP_MODE_METHOD_V_OFFSITE;
170
171 /* Off-site vertical coefficients */
172 DPU_REG_WRITE(c,
173 CDM_CDWN2_COEFF_OFFSITE_V,
174 offsite_v_coeff[0]);
175 break;
176 default:
177 return -EINVAL;
178 }
179
180 if (cfg->v_cdwn_type || cfg->h_cdwn_type)
181 opmode |= CDM_CDWN2_OP_MODE_EN; /* EN CDWN module */
182 else
183 opmode &= ~CDM_CDWN2_OP_MODE_EN;
184
185 out_size = (cfg->output_width & 0xFFFF) | ((cfg->output_height & 0xFFFF) << 16);
186 DPU_REG_WRITE(c, CDM_CDWN2_OUT_SIZE, out_size);
187 DPU_REG_WRITE(c, CDM_CDWN2_OP_MODE, opmode);
188 DPU_REG_WRITE(c, CDM_CDWN2_CLAMP_OUT, ((0x3FF << 16) | 0x0));
189
190 return 0;
191 }
192
> 193 int dpu_hw_cdm_enable(struct dpu_hw_cdm *ctx, struct dpu_hw_cdm_cfg *cdm)
194 {
195 struct dpu_hw_blk_reg_map *c = &ctx->hw;
196 const struct dpu_format *fmt;
197 u32 opmode = 0;
198 u32 csc = 0;
199
200 if (!ctx || !cdm)
201 return -EINVAL;
202
203 fmt = cdm->output_fmt;
204
205 if (!DPU_FORMAT_IS_YUV(fmt))
206 return -EINVAL;
207
208 dpu_hw_csc_setup(&ctx->hw, CDM_CSC_10_MATRIX_COEFF_0, cdm->csc_cfg, true);
209 dpu_hw_cdm_setup_cdwn(ctx, cdm);
210
211 if (cdm->output_type == CDM_CDWN_OUTPUT_HDMI) {
212 if (fmt->chroma_sample != DPU_CHROMA_H1V2)
213 return -EINVAL; /*unsupported format */
214 opmode = CDM_HDMI_PACK_OP_MODE_EN;
215 opmode |= (fmt->chroma_sample << 1);
216 }
217
218 csc |= CDM_CSC10_OP_MODE_DST_FMT_YUV;
219 csc &= ~CDM_CSC10_OP_MODE_SRC_FMT_YUV;
220 csc |= CDM_CSC10_OP_MODE_EN;
221
222 if (ctx && ctx->ops.bind_pingpong_blk)
223 ctx->ops.bind_pingpong_blk(ctx, true, cdm->pp_id);
224
225 DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc);
226 DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode);
227 return 0;
228 }
229
> 230 void dpu_hw_cdm_disable(struct dpu_hw_cdm *ctx)
231 {
232 if (!ctx)
233 return;
234
235 if (ctx && ctx->ops.bind_pingpong_blk)
236 ctx->ops.bind_pingpong_blk(ctx, false, PINGPONG_NONE);
237 }
238

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki