Messages in this thread Patch in this message | | | From | CK Hu (胡俊光) <> | Subject | Re: [PATCH 11/15] drm/mediatek: Support CRC in VDOSYS0 | Date | Thu, 31 Aug 2023 01:45:22 +0000 |
| |
Hi, Hsio-chien:
On Wed, 2023-08-23 at 23:13 +0800, Hsiao Chien Sung wrote: > We choose OVL as CRC generator from other hardware > components that are also capable of calculating CRCs, > since its frame done event triggers vblanks, it can be > used as a signal to know when is safe to retrieve CRC of > the frame. > > Please note that position of the hardware component > that is chosen as CRC generator in the display path is > significant. For example, while OVL is the first module > in VDOSYS0, its CRC won't be affected by the modules > after it, which means effects applied by PQ, Gamma, > Dither or any other components after OVL won't be > calculated in CRC generation. > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 + > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 121 > +++++++++++++++++++- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 1 + > 3 files changed, 119 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > index 2254038519e1..d2753360ae1e 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > @@ -100,6 +100,7 @@ void mtk_ovl_enable_vblank(struct device *dev); > void mtk_ovl_disable_vblank(struct device *dev); > const u32 *mtk_ovl_get_formats(struct device *dev); > size_t mtk_ovl_get_num_formats(struct device *dev); > +u32 mtk_ovl_crc_cnt(struct device *dev); > > void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex > *mutex); > void mtk_ovl_adaptor_remove_comp(struct device *dev, struct > mtk_mutex *mutex); > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > index 824f81291293..453db2de3e83 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > @@ -25,6 +25,13 @@ > #define OVL_FME_CPL_INT BIT(1) > #define DISP_REG_OVL_INTSTA 0x0008 > #define DISP_REG_OVL_EN 0x000c > +#define OVL_EN BIT(0) > +#define OVL_OP_8BIT_MODE BIT(4) > +#define OVL_HG_FOVL_CK_ON BIT(8) > +#define OVL_HF_FOVL_CK_ON BIT(10) > +#define DISP_REG_OVL_TRIG 0x0010 > +#define OVL_CRC_EN BIT(8) > +#define OVL_CRC_CLR BIT(9) > #define DISP_REG_OVL_RST 0x0014 > #define DISP_REG_OVL_ROI_SIZE 0x0020 > #define DISP_REG_OVL_DATAPATH_CON 0x0024 > @@ -44,6 +51,8 @@ > #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n)) > #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n)) > #define DISP_REG_OVL_ADDR_MT2701 0x0040 > +#define DISP_REG_OVL_CRC 0x0270 > +#define OVL_CRC_OUT_MASK GENMASK(30, 0) > #define DISP_REG_OVL_CLRFMT_EXT 0x02D0 > #define DISP_REG_OVL_CLRFMT_EXT1 0x02D8 > #define OVL_CLRFMT_EXT1_CSC_EN(n) (1 << ((n) * 4 > + 1)) > @@ -151,6 +160,10 @@ static const u32 mt8195_formats[] = { > DRM_FORMAT_YUYV, > }; > > +static const u32 mt8195_ovl_crcs[] = { > + DISP_REG_OVL_CRC, > +}; > + > struct mtk_disp_ovl_data { > unsigned int addr; > unsigned int gmc_bits; > @@ -161,6 +174,8 @@ struct mtk_disp_ovl_data { > const u32 *formats; > size_t num_formats; > bool supports_clrfmt_ext; > + const u32 *crcs; > + size_t crc_cnt; > > }; > > /* > @@ -176,8 +191,82 @@ struct mtk_disp_ovl { > const struct mtk_disp_ovl_data *data; > void (*vblank_cb)(void *data); > void *vblank_cb_data; > + struct cmdq_client *cmdq_client;
struct cmdq_client cmdq_client;
Refer to mtk_drm_crtc.c
> + struct cmdq_pkt *cmdq_pkt;
struct cmdq_pkt cmdq_pkt;
Refer to mtk_drm_crtc.c
> + u32 cmdq_event; > }; > > +u32 mtk_ovl_crc_cnt(struct device *dev) > +{ > + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > + > + return (u32)ovl->data->crc_cnt;
If CONFIG_MTK_CMDQ is not reachable, return 0.
> +} > + > +static void mtk_ovl_crc_loop_start(struct device *dev) > +{ > + int i; > + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > + struct mtk_drm_crtc *mtk_crtc = container_of(ovl->crtc, > + struct mtk_drm_crtc, > base); > + > + if (!ovl->cmdq_event || ovl->cmdq_client) > + return; > + > + ovl->cmdq_client = cmdq_mbox_create(dev, 0); > + if (IS_ERR(ovl->cmdq_client)) { > + pr_err("failed to create mailbox client\n"); > + return; > + } > + > + ovl->cmdq_pkt = cmdq_pkt_create(ovl->cmdq_client, PAGE_SIZE); > + if (!ovl->cmdq_pkt) { > + pr_err("failed to create cmdq packet\n"); > + return; > + } > + > + cmdq_pkt_wfe(ovl->cmdq_pkt, ovl->cmdq_event, true); > + > + for (i = 0; i < ovl->data->crc_cnt; i++) { > + /* put crc to spr1 register */ > + cmdq_pkt_read_s(ovl->cmdq_pkt, ovl->cmdq_reg.subsys, > + ovl->data->crcs[i], CMDQ_THR_SPR_IDX1); > + cmdq_pkt_assign(ovl->cmdq_pkt, CMDQ_THR_SPR_IDX0, > + CMDQ_ADDR_HIGH(mtk_crtc->crc.pa + i * > sizeof(u32))); > + > + /* copy spr1 register to crc.pa */ > + cmdq_pkt_write_s(ovl->cmdq_pkt, CMDQ_THR_SPR_IDX0, > + CMDQ_ADDR_LOW(mtk_crtc->crc.pa + i * > sizeof(u32)), > + CMDQ_THR_SPR_IDX1);
I would like copy crc to ovl variable instread of crtc variable, and crtc could query crc value by a new interface.
> + } > + > + /* reset crc */ > + mtk_ddp_write_mask(ovl->cmdq_pkt, ~0, &ovl->cmdq_reg, ovl- > >regs, > + DISP_REG_OVL_TRIG, OVL_CRC_CLR); > + /* clear reset bit */ > + mtk_ddp_write_mask(ovl->cmdq_pkt, 0, &ovl->cmdq_reg, ovl->regs, > + DISP_REG_OVL_TRIG, OVL_CRC_CLR); > + > + cmdq_pkt_finalize_loop(ovl->cmdq_pkt);
Create packet in probe function and only flush packet when OVL start.
> + cmdq_pkt_flush_async(ovl->cmdq_pkt); > +} > + > +static void mtk_ovl_crc_loop_stop(struct device *dev) > +{ > + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > + > + if (ovl->cmdq_pkt) { > + cmdq_pkt_destroy(ovl->cmdq_pkt); > + ovl->cmdq_pkt = NULL; > + } > + > + if (ovl->cmdq_client) { > + mbox_flush(ovl->cmdq_client->chan, 2000); > + cmdq_mbox_destroy(ovl->cmdq_client); > + ovl->cmdq_client = NULL; > + } > +} > + > static irqreturn_t mtk_disp_ovl_irq_handler(int irq, void *dev_id) > { > struct mtk_disp_ovl *priv = dev_id; > @@ -201,6 +290,7 @@ void mtk_ovl_register_vblank_cb(struct device > *dev, > > ovl->vblank_cb = vblank_cb; > ovl->vblank_cb_data = vblank_cb_data; > + ovl->crtc = (struct drm_crtc *)vblank_cb_data; > } > > void mtk_ovl_unregister_vblank_cb(struct device *dev) > @@ -216,14 +306,14 @@ void mtk_ovl_enable_vblank(struct device *dev) > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > writel(0x0, ovl->regs + DISP_REG_OVL_INTSTA); > - writel_relaxed(OVL_FME_CPL_INT, ovl->regs + > DISP_REG_OVL_INTEN); > + writel(OVL_FME_CPL_INT, ovl->regs + DISP_REG_OVL_INTEN);
This is not related to CRC, so move to another patch.
> } > > void mtk_ovl_disable_vblank(struct device *dev) > { > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > - writel_relaxed(0x0, ovl->regs + DISP_REG_OVL_INTEN); > + writel(0x0, ovl->regs + DISP_REG_OVL_INTEN);
Ditto.
> } > > const u32 *mtk_ovl_get_formats(struct device *dev) > @@ -258,6 +348,7 @@ void mtk_ovl_start(struct device *dev) > { > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > unsigned int reg = 0; > + unsigned int val = OVL_EN; > > if (ovl->data->smi_id_en) { > reg = readl(ovl->regs + DISP_REG_OVL_DATAPATH_CON); > @@ -265,13 +356,22 @@ void mtk_ovl_start(struct device *dev) > } > reg |= OVL_OUTPUT_CLAMP; > writel_relaxed(reg, ovl->regs + DISP_REG_OVL_DATAPATH_CON); > - writel_relaxed(0x1, ovl->regs + DISP_REG_OVL_EN); > + > + if (ovl->data->crcs) > + val |= OVL_OP_8BIT_MODE | OVL_HG_FOVL_CK_ON | > OVL_HF_FOVL_CK_ON; > + > + writel_relaxed(val, ovl->regs + DISP_REG_OVL_EN); > + writel_relaxed(OVL_CRC_EN, ovl->regs + DISP_REG_OVL_TRIG);
Set this only when this SoC support crc.
> + > + mtk_ovl_crc_loop_start(dev); > } > > void mtk_ovl_stop(struct device *dev) > { > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > + mtk_ovl_crc_loop_stop(dev); > + > writel_relaxed(0x0, ovl->regs + DISP_REG_OVL_EN); > if (ovl->data->smi_id_en) { > unsigned int reg; > @@ -321,7 +421,8 @@ void mtk_ovl_config(struct device *dev, unsigned > int w, > if (w != 0 && h != 0) > mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, &ovl- > >cmdq_reg, ovl->regs, > DISP_REG_OVL_ROI_SIZE); > - mtk_ddp_write_relaxed(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, > DISP_REG_OVL_ROI_BGCLR); > + mtk_ddp_write_relaxed(cmdq_pkt, 0xff000000, &ovl->cmdq_reg, > ovl->regs, > + DISP_REG_OVL_ROI_BGCLR);
If change background color is not related to CRC, move to another patch. If it's related, add comment for why do this.
> > mtk_ddp_write(cmdq_pkt, 0x1, &ovl->cmdq_reg, ovl->regs, > DISP_REG_OVL_RST); > mtk_ddp_write(cmdq_pkt, 0x0, &ovl->cmdq_reg, ovl->regs, > DISP_REG_OVL_RST); > @@ -677,6 +778,16 @@ static int mtk_disp_ovl_probe(struct > platform_device *pdev) > #endif > > priv->data = of_device_get_match_data(dev); > + > + if (priv->data->crcs) { > + if (of_property_read_u32_index(dev->of_node, > + "mediatek,gce-events", > 0, > + &priv->cmdq_event)) { > + dev_err(dev, "failed to get gce-events\n"); > + return -ENOPARAM;
If this SoC support crc but does not support gce, I think we could just ignore crc function and keep drm driver work.
Regards, CK
> + } > + } > + > platform_set_drvdata(pdev, priv); > > ret = devm_request_irq(dev, irq, mtk_disp_ovl_irq_handler, > @@ -771,6 +882,8 @@ static const struct mtk_disp_ovl_data > mt8195_ovl_driver_data = { > .formats = mt8195_formats, > .num_formats = ARRAY_SIZE(mt8195_formats), > .supports_clrfmt_ext = true, > + .crcs = mt8195_ovl_crcs, > + .crc_cnt = ARRAY_SIZE(mt8195_ovl_crcs), > }; > > static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = { > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > index f114da4d36a9..1b747a34a06b 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > @@ -347,6 +347,7 @@ static const struct mtk_ddp_comp_funcs ddp_ovl = > { > .clk_enable = mtk_ovl_clk_enable, > .clk_disable = mtk_ovl_clk_disable, > .config = mtk_ovl_config, > + .crc_cnt = mtk_ovl_crc_cnt, > .start = mtk_ovl_start, > .stop = mtk_ovl_stop, > .register_vblank_cb = mtk_ovl_register_vblank_cb,
| |