| Message ID | 20231212002245.23715-8-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add CDM support for MSM writeback | expand |
On Tue, 12 Dec 2023 at 02:23, 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 blocks, this > change adds the dpu_hw_cdm abstraction for the CDM block. > > changes in v3: > - fix commit text from sub-blk to blk for CDM > - fix kbot issue for missing static for dpu_hw_cdm_enable() > - fix kbot issue for incorrect documentation style > - add more documentation for enums and struct in dpu_hw_cdm.h > - drop "enable" parameter from bind_pingpong_blk() as we can > just use PINGPONG_NONE for disable cases > - drop unnecessary bit operation for zero value of cdm_cfg > > 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() > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202312101815.B3ZH7Pfy-lkp@intel.com/ > 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 | 263 ++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 130 ++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > 4 files changed, 395 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..4976f8a05ce7 > --- /dev/null > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c > @@ -0,0 +1,263 @@ > +// 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) I think it might be easier to define enum { CDM_CDWN2_METHOD_DROP_PIXEL = 0, CDM_CDWN2_METHOD_AVG = 1, CDM_CDWN2_METHOD_ = 2, CDM_CDWN2_METHOD_DROP_PIXEL = 3, }; then use FIELD_PREP() > +#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5) > +#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3) Why are they called foo_DROP_bar? > + > +/* 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; No need to init it, please drop. > + > + if (cfg->output_bit_depth != CDM_CDWN_OUTPUT_10BIT) > + 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; Please, can we get rid of clears for the zero-initialised variable? If you move the 10bit/8bit check after this switch, you can drop the = 0 from the variable definition and instead have: switch (type) { case DISABLE: opmode = 0; break; case PIXEL_DROP: opmode = CDM_CDWN2_OP_MODE_ENABLE_H | FIELD_PREP(CDM_CDWM2_OP_MODE_METHOD_H, CDM_CDWN2_METHOD_DROP_PIXEL); break; case AVG: opmode = CDM_CDWN2_OP_MODE_ENABLE_H | FIELD_PREP(CDM_CDWM2_OP_MODE_METHOD_H, CDM_CDWN2_METHOD_AVG); break; // etc. } Same for the v_type. Also could you please drop useless comments which repeat what is being done in the next line? > + 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) */ So, is Average 0x1 or 0x2? Or 0x3 as written below? > + 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: > + DPU_ERROR("%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; > +} > + > +static 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, cdm->pp_id); > + > + DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc); > + DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode); > + return 0; > +} > + > +static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, const enum dpu_pingpong pp) > +{ > + struct dpu_hw_blk_reg_map *c; > + int mux_cfg = 0xF; /* Disabled */ lowercase hex. And it is easier to move it to the if (pp) condition, like it was done for INTF or WB. > + > + c = &ctx->hw; > + > + if (pp) > + 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; > + 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..e7d57dbd6103 > --- /dev/null > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h > @@ -0,0 +1,130 @@ > +/* 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 : current configuration of CDM block > + * > + * @output_width: output ROI width of CDM block > + * @output_height: output ROI height of CDM block > + * @output_bit_depth: output bit-depth of CDM block > + * @h_cdwn_type: downsample type used for horizontal pixels > + * @v_cdwn_type: downsample type used for vertical pixels > + * @output_fmt: handle to dpu_format of CDM block > + * @csc_cfg: handle to CSC matrix programmed for CDM block > + * @output_type: interface to which CDM is paired (HDMI/WB) > + * @pp_id: ping-pong block to which CDM is bound to > + */ > +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; > +}; > + > +/* > + * These values are used indicate which type of downsample is used > + * in the horizontal/vertical direction for the CDM block. > + */ > +enum dpu_hw_cdwn_type { > + CDM_CDWN_DISABLE, > + CDM_CDWN_PIXEL_DROP, > + CDM_CDWN_AVG, > + CDM_CDWN_COSITE, > + CDM_CDWN_OFFSITE, > +}; > + > +/* > + * CDM block can be paired with WB or HDMI block. These values match > + * the input with which the CDM block is paired. > + */ > +enum dpu_hw_cdwn_output_type { > + CDM_CDWN_OUTPUT_HDMI, > + CDM_CDWN_OUTPUT_WB, > +}; > + > +/* > + * CDM block can give an 8-bit or 10-bit output. These values > + * are used to indicate the output bit depth of CDM block > + */ > +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 > + * @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); > + > + /** > + * Enable/disable the connection with pingpong > + * @cdm Pointer to chroma down context > + * @pp pingpong block id. > + */ > + void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, 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 >
On 12/12/2023 1:40 AM, Dmitry Baryshkov wrote: > On Tue, 12 Dec 2023 at 02:23, 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 blocks, this >> change adds the dpu_hw_cdm abstraction for the CDM block. >> >> changes in v3: >> - fix commit text from sub-blk to blk for CDM >> - fix kbot issue for missing static for dpu_hw_cdm_enable() >> - fix kbot issue for incorrect documentation style >> - add more documentation for enums and struct in dpu_hw_cdm.h >> - drop "enable" parameter from bind_pingpong_blk() as we can >> just use PINGPONG_NONE for disable cases >> - drop unnecessary bit operation for zero value of cdm_cfg >> >> 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() >> >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202312101815.B3ZH7Pfy-lkp@intel.com/ >> 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 | 263 ++++++++++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 130 ++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + >> 4 files changed, 395 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..4976f8a05ce7 >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c >> @@ -0,0 +1,263 @@ >> +// 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) > > I think it might be easier to define > > enum { > CDM_CDWN2_METHOD_DROP_PIXEL = 0, > CDM_CDWN2_METHOD_AVG = 1, > CDM_CDWN2_METHOD_ = 2, > CDM_CDWN2_METHOD_DROP_PIXEL = 3, > }; > > then use FIELD_PREP() > Ok, let me explore that option. We already have below enum in dpu_hw_cdm.h, we can re-use that +enum dpu_hw_cdwn_type { + CDM_CDWN_DISABLE, + CDM_CDWN_PIXEL_DROP, + CDM_CDWN_AVG, + CDM_CDWN_COSITE, + CDM_CDWN_OFFSITE, +}; >> +#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5) >> +#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3) > > Why are they called foo_DROP_bar? > Because they are always used with ~ And as per the documentation 0 for this field is pixel drop. If we are dropping the clearing altogether, then these masks will go away too. >> + >> +/* 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; > > No need to init it, please drop. > alright. >> + >> + if (cfg->output_bit_depth != CDM_CDWN_OUTPUT_10BIT) >> + 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; > > Please, can we get rid of clears for the zero-initialised variable? If > you move the 10bit/8bit check after this switch, you can drop the = 0 > from the variable definition and instead have: > Ok, will refine further. > switch (type) { > case DISABLE: > opmode = 0; > break; > case PIXEL_DROP: > opmode = CDM_CDWN2_OP_MODE_ENABLE_H | > FIELD_PREP(CDM_CDWM2_OP_MODE_METHOD_H, > CDM_CDWN2_METHOD_DROP_PIXEL); > break; > case AVG: > opmode = CDM_CDWN2_OP_MODE_ENABLE_H | > FIELD_PREP(CDM_CDWM2_OP_MODE_METHOD_H, > CDM_CDWN2_METHOD_AVG); > break; > // etc. > } > > Same for the v_type. > > Also could you please drop useless comments which repeat what is being > done in the next line? > ack :) >> + 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) */ > > So, is Average 0x1 or 0x2? Or 0x3 as written below? > The comment is wrong. It should say Co-Site is 0x2, Off-Site is 0x3. And for the record, average is 0x1 :) >> + 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: >> + DPU_ERROR("%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; >> +} >> + >> +static 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, cdm->pp_id); >> + >> + DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc); >> + DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode); >> + return 0; >> +} >> + >> +static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, const enum dpu_pingpong pp) >> +{ >> + struct dpu_hw_blk_reg_map *c; >> + int mux_cfg = 0xF; /* Disabled */ > > lowercase hex. And it is easier to move it to the if (pp) condition, > like it was done for INTF or WB. > ack for the lowercase hex. hmm, i followed the dpu_hw_dsc_1_2 model, so basically for 0 (PINGPONG_NONE) cases it will stay at 0xf. >> + >> + c = &ctx->hw; >> + >> + if (pp) >> + 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; >> + 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..e7d57dbd6103 >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h >> @@ -0,0 +1,130 @@ >> +/* 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 : current configuration of CDM block >> + * >> + * @output_width: output ROI width of CDM block >> + * @output_height: output ROI height of CDM block >> + * @output_bit_depth: output bit-depth of CDM block >> + * @h_cdwn_type: downsample type used for horizontal pixels >> + * @v_cdwn_type: downsample type used for vertical pixels >> + * @output_fmt: handle to dpu_format of CDM block >> + * @csc_cfg: handle to CSC matrix programmed for CDM block >> + * @output_type: interface to which CDM is paired (HDMI/WB) >> + * @pp_id: ping-pong block to which CDM is bound to >> + */ >> +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; >> +}; >> + >> +/* >> + * These values are used indicate which type of downsample is used >> + * in the horizontal/vertical direction for the CDM block. >> + */ >> +enum dpu_hw_cdwn_type { >> + CDM_CDWN_DISABLE, >> + CDM_CDWN_PIXEL_DROP, >> + CDM_CDWN_AVG, >> + CDM_CDWN_COSITE, >> + CDM_CDWN_OFFSITE, >> +}; >> + >> +/* >> + * CDM block can be paired with WB or HDMI block. These values match >> + * the input with which the CDM block is paired. >> + */ >> +enum dpu_hw_cdwn_output_type { >> + CDM_CDWN_OUTPUT_HDMI, >> + CDM_CDWN_OUTPUT_WB, >> +}; >> + >> +/* >> + * CDM block can give an 8-bit or 10-bit output. These values >> + * are used to indicate the output bit depth of CDM block >> + */ >> +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 >> + * @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); >> + >> + /** >> + * Enable/disable the connection with pingpong >> + * @cdm Pointer to chroma down context >> + * @pp pingpong block id. >> + */ >> + void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, 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 >> > >
On Tue, 12 Dec 2023 at 19:51, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 12/12/2023 1:40 AM, Dmitry Baryshkov wrote: > > On Tue, 12 Dec 2023 at 02:23, 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 blocks, this > >> change adds the dpu_hw_cdm abstraction for the CDM block. > >> > >> changes in v3: > >> - fix commit text from sub-blk to blk for CDM > >> - fix kbot issue for missing static for dpu_hw_cdm_enable() > >> - fix kbot issue for incorrect documentation style > >> - add more documentation for enums and struct in dpu_hw_cdm.h > >> - drop "enable" parameter from bind_pingpong_blk() as we can > >> just use PINGPONG_NONE for disable cases > >> - drop unnecessary bit operation for zero value of cdm_cfg > >> > >> 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() > >> > >> Reported-by: kernel test robot <lkp@intel.com> > >> Closes: https://lore.kernel.org/oe-kbuild-all/202312101815.B3ZH7Pfy-lkp@intel.com/ > >> 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 | 263 ++++++++++++++++++++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 130 ++++++++++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > >> 4 files changed, 395 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..4976f8a05ce7 > >> --- /dev/null > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c > >> @@ -0,0 +1,263 @@ > >> +// 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) > > > > I think it might be easier to define > > > > enum { > > CDM_CDWN2_METHOD_DROP_PIXEL = 0, > > CDM_CDWN2_METHOD_AVG = 1, > > CDM_CDWN2_METHOD_ = 2, > > CDM_CDWN2_METHOD_DROP_PIXEL = 3, > > }; > > > > then use FIELD_PREP() > > > > Ok, let me explore that option. > > We already have below enum in dpu_hw_cdm.h, we can re-use that > > +enum dpu_hw_cdwn_type { > + CDM_CDWN_DISABLE, > + CDM_CDWN_PIXEL_DROP, > + CDM_CDWN_AVG, > + CDM_CDWN_COSITE, > + CDM_CDWN_OFFSITE, > +}; > > >> +#define CDM_CDWN2_V_PIXEL_DROP_MASK GENMASK(6, 5) > >> +#define CDM_CDWN2_H_PIXEL_DROP_MASK GENMASK(4, 3) > > > > Why are they called foo_DROP_bar? > > > > Because they are always used with ~ > > And as per the documentation 0 for this field is pixel drop. > > If we are dropping the clearing altogether, then these masks will go > away too. > > >> + > >> +/* 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; > > > > No need to init it, please drop. > > > > alright. > > >> + > >> + if (cfg->output_bit_depth != CDM_CDWN_OUTPUT_10BIT) > >> + 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; > > > > Please, can we get rid of clears for the zero-initialised variable? If > > you move the 10bit/8bit check after this switch, you can drop the = 0 > > from the variable definition and instead have: > > > > Ok, will refine further. > > > switch (type) { > > case DISABLE: > > opmode = 0; > > break; > > case PIXEL_DROP: > > opmode = CDM_CDWN2_OP_MODE_ENABLE_H | > > FIELD_PREP(CDM_CDWM2_OP_MODE_METHOD_H, > > CDM_CDWN2_METHOD_DROP_PIXEL); > > break; > > case AVG: > > opmode = CDM_CDWN2_OP_MODE_ENABLE_H | > > FIELD_PREP(CDM_CDWM2_OP_MODE_METHOD_H, > > CDM_CDWN2_METHOD_AVG); > > break; > > // etc. > > } > > > > Same for the v_type. > > > > > Also could you please drop useless comments which repeat what is being > > done in the next line? > > > > ack :) > > >> + 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) */ > > > > So, is Average 0x1 or 0x2? Or 0x3 as written below? > > > > The comment is wrong. It should say Co-Site is 0x2, Off-Site is 0x3. > > And for the record, average is 0x1 :) > > > >> + 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: > >> + DPU_ERROR("%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; > >> +} > >> + > >> +static 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, cdm->pp_id); > >> + > >> + DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc); > >> + DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode); > >> + return 0; > >> +} > >> + > >> +static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, const enum dpu_pingpong pp) > >> +{ > >> + struct dpu_hw_blk_reg_map *c; > >> + int mux_cfg = 0xF; /* Disabled */ > > > > lowercase hex. And it is easier to move it to the if (pp) condition, > > like it was done for INTF or WB. > > > > ack for the lowercase hex. > > hmm, i followed the dpu_hw_dsc_1_2 model, so basically for 0 > (PINGPONG_NONE) cases it will stay at 0xf. My preference was towards more explicit code, like INTF or WB. > > >> + > >> + c = &ctx->hw; > >> + > >> + if (pp) > >> + 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; > >> + 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..e7d57dbd6103 > >> --- /dev/null > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h > >> @@ -0,0 +1,130 @@ > >> +/* 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 : current configuration of CDM block > >> + * > >> + * @output_width: output ROI width of CDM block > >> + * @output_height: output ROI height of CDM block > >> + * @output_bit_depth: output bit-depth of CDM block > >> + * @h_cdwn_type: downsample type used for horizontal pixels > >> + * @v_cdwn_type: downsample type used for vertical pixels > >> + * @output_fmt: handle to dpu_format of CDM block > >> + * @csc_cfg: handle to CSC matrix programmed for CDM block > >> + * @output_type: interface to which CDM is paired (HDMI/WB) > >> + * @pp_id: ping-pong block to which CDM is bound to > >> + */ > >> +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; > >> +}; > >> + > >> +/* > >> + * These values are used indicate which type of downsample is used > >> + * in the horizontal/vertical direction for the CDM block. > >> + */ > >> +enum dpu_hw_cdwn_type { > >> + CDM_CDWN_DISABLE, > >> + CDM_CDWN_PIXEL_DROP, > >> + CDM_CDWN_AVG, > >> + CDM_CDWN_COSITE, > >> + CDM_CDWN_OFFSITE, > >> +}; > >> + > >> +/* > >> + * CDM block can be paired with WB or HDMI block. These values match > >> + * the input with which the CDM block is paired. > >> + */ > >> +enum dpu_hw_cdwn_output_type { > >> + CDM_CDWN_OUTPUT_HDMI, > >> + CDM_CDWN_OUTPUT_WB, > >> +}; > >> + > >> +/* > >> + * CDM block can give an 8-bit or 10-bit output. These values > >> + * are used to indicate the output bit depth of CDM block > >> + */ > >> +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 > >> + * @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); > >> + > >> + /** > >> + * Enable/disable the connection with pingpong > >> + * @cdm Pointer to chroma down context > >> + * @pp pingpong block id. > >> + */ > >> + void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, 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 > >> > > > >
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..4976f8a05ce7 --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.c @@ -0,0 +1,263 @@ +// 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; + + /* 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: + DPU_ERROR("%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; +} + +static 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, cdm->pp_id); + + DPU_REG_WRITE(c, CDM_CSC_10_OPMODE, csc); + DPU_REG_WRITE(c, CDM_HDMI_PACK_OP_MODE, opmode); + return 0; +} + +static void dpu_hw_cdm_bind_pingpong_blk(struct dpu_hw_cdm *ctx, const enum dpu_pingpong pp) +{ + struct dpu_hw_blk_reg_map *c; + int mux_cfg = 0xF; /* Disabled */ + + c = &ctx->hw; + + if (pp) + 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; + 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..e7d57dbd6103 --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h @@ -0,0 +1,130 @@ +/* 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 : current configuration of CDM block + * + * @output_width: output ROI width of CDM block + * @output_height: output ROI height of CDM block + * @output_bit_depth: output bit-depth of CDM block + * @h_cdwn_type: downsample type used for horizontal pixels + * @v_cdwn_type: downsample type used for vertical pixels + * @output_fmt: handle to dpu_format of CDM block + * @csc_cfg: handle to CSC matrix programmed for CDM block + * @output_type: interface to which CDM is paired (HDMI/WB) + * @pp_id: ping-pong block to which CDM is bound to + */ +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; +}; + +/* + * These values are used indicate which type of downsample is used + * in the horizontal/vertical direction for the CDM block. + */ +enum dpu_hw_cdwn_type { + CDM_CDWN_DISABLE, + CDM_CDWN_PIXEL_DROP, + CDM_CDWN_AVG, + CDM_CDWN_COSITE, + CDM_CDWN_OFFSITE, +}; + +/* + * CDM block can be paired with WB or HDMI block. These values match + * the input with which the CDM block is paired. + */ +enum dpu_hw_cdwn_output_type { + CDM_CDWN_OUTPUT_HDMI, + CDM_CDWN_OUTPUT_WB, +}; + +/* + * CDM block can give an 8-bit or 10-bit output. These values + * are used to indicate the output bit depth of CDM block + */ +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 + * @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); + + /** + * Enable/disable the connection with pingpong + * @cdm Pointer to chroma down context + * @pp pingpong block id. + */ + void (*bind_pingpong_blk)(struct dpu_hw_cdm *cdm, 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
CDM block comes with its own set of registers and operations which can be done. In-line with other hardware blocks, this change adds the dpu_hw_cdm abstraction for the CDM block. changes in v3: - fix commit text from sub-blk to blk for CDM - fix kbot issue for missing static for dpu_hw_cdm_enable() - fix kbot issue for incorrect documentation style - add more documentation for enums and struct in dpu_hw_cdm.h - drop "enable" parameter from bind_pingpong_blk() as we can just use PINGPONG_NONE for disable cases - drop unnecessary bit operation for zero value of cdm_cfg 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() Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202312101815.B3ZH7Pfy-lkp@intel.com/ 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 | 263 ++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_cdm.h | 130 ++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + 4 files changed, 395 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