diff mbox series

[v2,07/16] drm/msm/dpu: add dpu_hw_cdm abstraction for CDM block

Message ID 20231208050641.32582-8-quic_abhinavk@quicinc.com (mailing list archive)
State Not Applicable
Headers show
Series [v2,01/16] drm/msm/dpu: add formats check for writeback encoder | expand

Commit Message

Abhinav Kumar Dec. 8, 2023, 5:06 a.m. UTC 
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 <quic_abhinavk@quicinc.com>
---
 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

Comments

Dmitry Baryshkov Dec. 8, 2023, 12:06 p.m. UTC | #1 
On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> 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 <quic_abhinavk@quicinc.com>
> ---
>  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
Abhinav Kumar Dec. 8, 2023, 5:08 p.m. UTC | #2 
On 12/8/2023 4:06 AM, Dmitry Baryshkov wrote:
> On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> 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 <quic_abhinavk@quicinc.com>
>> ---
>>   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
Dmitry Baryshkov Dec. 8, 2023, 6:17 p.m. UTC | #3 
On Fri, 8 Dec 2023 at 19:09, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 12/8/2023 4:06 AM, Dmitry Baryshkov wrote:
> > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> 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 <quic_abhinavk@quicinc.com>
> >> ---
> >>   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
kernel test robot Dec. 10, 2023, 10:53 a.m. UTC | #4 
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/202312101815.B3ZH7Pfy-lkp@intel.com/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/202312101815.B3ZH7Pfy-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312101815.B3ZH7Pfy-lkp@intel.com/

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
diff mbox series

Patch

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