Re: [PATCH v2 11/15] drm/mediatek: add BLENDER support for MT8196

["Paul-pl Chen (陳柏霖)" <Paul-pl.Chen@mediatek.com>]

On Mon, 2025-03-24 at 08:33 +0000, CK Hu (胡俊光) wrote:
> On Fri, 2025-03-21 at 17:33 +0800, paul-pl.chen wrote:
> > From: Nancy Lin <nancy.lin@mediatek.com>
> > 
> > BLENDER executes the alpha blending function for overlapping
> > layers from different sources, which is the primary function
> > of the overlapping system.
> > 
> > Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> > Signed-off-by: Paul-pl Chen <paul-pl.chen@mediatek.com>
> > 
> > ---
> >  drivers/gpu/drm/mediatek/Makefile           |   1 +
> >  drivers/gpu/drm/mediatek/mtk_ddp_comp.c     |   1 +
> >  drivers/gpu/drm/mediatek/mtk_ddp_comp.h     |   1 +
> >  drivers/gpu/drm/mediatek/mtk_disp_blender.c | 276
> > ++++++++++++++++++++
> >  drivers/gpu/drm/mediatek/mtk_disp_blender.h |  18 ++
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  12 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   1 +
> >  8 files changed, 311 insertions(+)
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_blender.c
> >  create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_blender.h
> > 
> > diff --git a/drivers/gpu/drm/mediatek/Makefile
> > b/drivers/gpu/drm/mediatek/Makefile
> > index db92f4fb353d..a7b9ebe27f68 100644
> > --- a/drivers/gpu/drm/mediatek/Makefile
> > +++ b/drivers/gpu/drm/mediatek/Makefile
> > @@ -3,6 +3,7 @@
> >  mediatek-drm-y := mtk_crtc.o \
> >  		  mtk_ddp_comp.o \
> >  		  mtk_disp_aal.o \
> > +		  mtk_disp_blender.o \
> >  		  mtk_disp_ccorr.o \
> >  		  mtk_disp_color.o \
> >  		  mtk_disp_exdma.o \
> > diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > index 3e0739d8e6f1..e65c6df987f2 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > @@ -445,6 +445,7 @@ static const char * const
> > mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX] = {
> >  	[MTK_DP_INTF] = "dp-intf",
> >  	[MTK_DPI] = "dpi",
> >  	[MTK_DSI] = "dsi",
> > +	[MTK_OVL_BLENDER] = "blender",
> >  	[MTK_OVL_EXDMA] = "exdma",
> >  };
> >  
> > diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> > b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> > index 86dc0ee3924c..075ba5683f93 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> > @@ -43,6 +43,7 @@ enum mtk_ddp_comp_type {
> >  	MTK_DPI,
> >  	MTK_DP_INTF,
> >  	MTK_DSI,
> > +	MTK_OVL_BLENDER,
> >  	MTK_OVL_EXDMA,
> >  	MTK_DDP_COMP_TYPE_MAX,
> >  };
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_blender.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_blender.c
> > new file mode 100644
> > index 000000000000..32c9e3d463a4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_blender.c
> > @@ -0,0 +1,276 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2025 MediaTek Inc.
> > + */
> > +
> > +#include <drm/drm_fourcc.h>
> > +#include <drm/drm_blend.h>
> > +#include <drm/drm_framebuffer.h>
> > +#include <linux/clk.h>
> > +#include <linux/component.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reset.h>
> > +#include <linux/soc/mediatek/mtk-cmdq.h>
> > +#include <linux/soc/mediatek/mtk-mmsys.h>
> > +
> > +#include "mtk_crtc.h"
> > +#include "mtk_ddp_comp.h"
> > +#include "mtk_disp_drv.h"
> > +#include "mtk_drm_drv.h"
> > +#include "mtk_disp_blender.h"
> > +#include "mtk_disp_ovl.h"
> > +
> > +#define OVL_BLD_ALPHA					0xff
> > +#define DISP_REG_OVL_BLD_DATAPATH_CON			0x010
> > +#define
> > OVL_BLD_BGCLR_IN_SEL					BIT(0)
> > +#define
> > OVL_BLD_BGCLR_OUT_TO_PROC				BIT(4)
> > +#define
> > OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER				BIT(5)
> > +
> > +#define DISP_REG_OVL_BLD_EN				0x020
> > +#define
> > OVL_BLD_EN						BIT(0)
> > +#define
> > OVL_BLD_FORCE_RELAY_MODE				BIT(4)
> > +#define
> > OVL_BLD_RELAY_MODE					BIT(5)
> > +#define DISP_REG_OVL_BLD_RST				0x024
> > +#define
> > OVL_BLD_RST						BIT(0)
> > +#define DISP_REG_OVL_BLD_SHADOW_CTRL			0x028
> > +#define
> > OVL_BLD_BYPASS_SHADOW					BIT(2)
> > +#define DISP_REG_OVL_BLD_BGCLR_BALCK			0xff000000
> > +#define DISP_REG_OVL_BLD_ROI_SIZE			0x030
> > +#define DISP_REG_OVL_BLD_L_EN				0x040
> > +#define
> > OVL_BLD_L_EN						BIT(0)
> > +#define
> > DISP_REG_OVL_BLD_OFFSET				0x044
> > +#define DISP_REG_OVL_BLD_SRC_SIZE			0x048
> > +#define DISP_REG_OVL_BLD_L0_CLRFMT			0x050
> > +#define
> > OVL_BLD_CON_FLD_CLRFMT					GENMASK(3, 0)
> > +#define
> > OVL_BLD_CON_CLRFMT_MAN					BIT(4)
> > +#define
> > OVL_BLD_CON_FLD_CLRFMT_NB				GENMASK(9, 8)
> > +#define
> > OVL_BLD_CON_CLRFMT_NB_10_BIT				BIT(8)
> > +#define
> > OVL_BLD_CON_BYTE_SWAP					BIT(16)
> > +#define
> > OVL_BLD_CON_RGB_SWAP					BIT(17)
> > +#define DISP_REG_OVL_BLD_BGCLR_CLR			0x104
> > +#define
> > DISP_REG_OVL_BLD_L_CON2				0x200
> > +#define
> > OVL_BLD_L_ALPHA						GENMASK(7, 0)
> > +#define
> > OVL_BLD_L_ALPHA_EN					BIT(12)
> > +#define DISP_REG_OVL_BLD_L0_PITCH			0x208
> 
> DISP_REG_OVL_BLD_L0_PITCH is useless, so drop it.
> 
> 
> This will be used in the mtk_disp_blender_config.
Sorry for the misleading, the naming would be
#define DISP_REG_OVL_BLD_L0_ALPHA_SEL	0x208
> 
> 
> > +#define
> > OVL_BLD_L0_CONST					BIT(24)
> > +#define
> > DISP_REG_OVL_BLD_L0_CLR				0x20c
> > +#define
> > OVL_BLD_CON_CLRFMT_MAN					BIT(4)
> > +#define
> > OVL_BLD_L0_SRC_PITCH					GENMASK(15, 0)
> > +#define DISP_REG_OVL_BLD_PITCH				0x2f4
> > +
> > +struct mtk_disp_blender {
> > +	void __iomem		*regs;
> > +	struct clk		*clk;
> > +	struct cmdq_client_reg	cmdq_reg;
> > +};
> > +
> > +void mtk_disp_blender_layer_config(struct device *dev, struct
> > mtk_plane_state *state,
> > +				   struct cmdq_pkt *cmdq_pkt)
> > +{
> > +	struct mtk_disp_blender *priv = dev_get_drvdata(dev);
> > +	struct mtk_plane_pending_state *pending = &state->pending;
> > +	unsigned int align_width = ALIGN_DOWN(pending->width, 2);
> 
> If width is 101, changing width to 100 would get correct display
> output?
> 
> 
Yes, according to our internal experiments, the ALIGN_DOWN code can
be removed simultaneously from the drives for "EXDMA, BLENDER, 
OVLSYS_ADAPTOR". I will fix it.

> 
> > +	unsigned int alpha;
> > +	unsigned int clrfmt;
> > +	unsigned int blend_mode = mtk_ovl_get_blend_mode(state,
> > +							
> > BIT(DRM_MODE_BLEND_PREMULTI) |
> > +							
> > BIT(DRM_MODE_BLEND_COVERAGE) |
> > +							
> > BIT(DRM_MODE_BLEND_PIXEL_NONE));
> > +	unsigned int ignore_pixel_alpha = 0;
> > +
> > +	if (!pending->enable) {
> > +		mtk_ddp_write(cmdq_pkt, 0, &priv->cmdq_reg, priv-
> > >regs, DISP_REG_OVL_BLD_L_EN);
> > +		return;
> > +	}
> > +
> > +	mtk_ddp_write(cmdq_pkt, pending->y << 16 | pending->x,
> > &priv->cmdq_reg, priv->regs,
> > +		      DISP_REG_OVL_BLD_OFFSET);
> > +
> > +	mtk_ddp_write(cmdq_pkt, pending->height << 16 |
> > align_width, &priv->cmdq_reg,
> > +		      priv->regs, DISP_REG_OVL_BLD_SRC_SIZE);
> > +
> > +	clrfmt = mtk_ovl_fmt_convert(pending->format, blend_mode,
> > true, false, 0,
> > +				     OVL_BLD_CON_CLRFMT_MAN,
> > OVL_BLD_CON_BYTE_SWAP,
> > +				     OVL_BLD_CON_RGB_SWAP);
> > +	clrfmt |= mtk_ovl_is_10bit_rgb(pending->format) ?
> > OVL_BLD_CON_CLRFMT_NB_10_BIT : 0;
> > +	mtk_ddp_write_mask(cmdq_pkt, clrfmt, &priv->cmdq_reg,
> > priv->regs,
> > +			   DISP_REG_OVL_BLD_L0_CLRFMT,
> > OVL_BLD_CON_CLRFMT_MAN |
> > +			   OVL_BLD_CON_RGB_SWAP | 
> > OVL_BLD_CON_BYTE_SWAP |
> > +			   OVL_BLD_CON_FLD_CLRFMT |
> > OVL_BLD_CON_FLD_CLRFMT_NB);
> > +
> > +	alpha = (OVL_BLD_ALPHA & (state->base.alpha >> 8)) |
> > +		OVL_BLD_L_ALPHA_EN;
> > +
> > +	if (mtk_ovl_is_ignore_pixel_alpha(state, blend_mode))
> > +		ignore_pixel_alpha = OVL_BLD_L0_CONST;
> > +
> > +	mtk_ddp_write_mask(cmdq_pkt, pending->pitch |
> > ignore_pixel_alpha,
> > +			   &priv->cmdq_reg, priv->regs,
> > +			   DISP_REG_OVL_BLD_PITCH,
> > OVL_BLD_L0_CONST | OVL_BLD_L0_SRC_PITCH);
> 
> EXDMA already has pitch function, why BLENDER also has pitch
> function?
> What does BLENDER pitch do?

The input source of blender layer can be EXDMA layer output or 
the blender constanat itself.
The color setting is configured for the blender constant layer. 
> 
> > +
> > +	mtk_ddp_write_mask(cmdq_pkt, alpha, &priv->cmdq_reg, priv-
> > >regs,
> > +			   DISP_REG_OVL_BLD_L_CON2,
> > OVL_BLD_L_ALPHA_EN | OVL_BLD_L_ALPHA);
> > +
> > +	mtk_ddp_write(cmdq_pkt, OVL_BLD_L_EN, &priv->cmdq_reg,
> > priv->regs, DISP_REG_OVL_BLD_L_EN);
> > +}
> > +
> > +void mtk_disp_blender_config(struct device *dev, unsigned int w,
> > +			     unsigned int h, unsigned int
> > vrefresh,
> > +			     unsigned int bpc, bool most_top,
> > +			     bool most_bottom, struct cmdq_pkt
> > *cmdq_pkt)
> > +{
> > +	struct mtk_disp_blender *priv = dev_get_drvdata(dev);
> > +	u32 datapath;
> > +
> > +	dev_dbg(dev, "%s-w:%d, h:%d\n", __func__, w, h);
> > +	mtk_ddp_write(cmdq_pkt, h << 16 | w, &priv->cmdq_reg,
> > priv->regs,
> > +		      DISP_REG_OVL_BLD_ROI_SIZE);
> > +	mtk_ddp_write(cmdq_pkt, DISP_REG_OVL_BLD_BGCLR_BALCK,
> > &priv->cmdq_reg, priv->regs,
> > +		      DISP_REG_OVL_BLD_BGCLR_CLR);
> > +	mtk_ddp_write(cmdq_pkt, DISP_REG_OVL_BLD_BGCLR_BALCK,
> > &priv->cmdq_reg, priv->regs,
> > +		      DISP_REG_OVL_BLD_L0_CLR);
> 
> I think I know what is BGLCR_CLR, but what is L0_CLR?
> 
> 
DISP_REG_OVL_BLD_L0_CLR controls te constant color for each
individual layer,
while OVL_BLD_BGCLR_CLR controls the background color for
the entire display.
> 
> 
> 
> > +
> > +	if (most_top)
> > +		datapath = OVL_BLD_BGCLR_OUT_TO_PROC;
> > +	else
> > +		datapath = OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER;
> > +	/*
> > +	 * The primary input is from EXDMA and the second input
> > +	 * is optionally from another blender
> > +	 */
> > +	if (!most_bottom)
> > +		datapath |= OVL_BLD_BGCLR_IN_SEL;
> > +
> > +	mtk_ddp_write_mask(cmdq_pkt, datapath,
> > +			   &priv->cmdq_reg, priv->regs,
> > DISP_REG_OVL_BLD_DATAPATH_CON,
> > +			   OVL_BLD_BGCLR_OUT_TO_PROC |
> > OVL_BLD_BGCLR_OUT_TO_NEXT_LAYER |
> > +			   OVL_BLD_BGCLR_IN_SEL);
> > +}
> > +
> > +void mtk_disp_blender_start(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt)
> > +{
> > +	struct mtk_disp_blender *priv = dev_get_drvdata(dev);
> > +	unsigned int tmp;
> > +
> > +	tmp = readl(priv->regs + DISP_REG_OVL_BLD_SHADOW_CTRL);
> > +	tmp = tmp | OVL_BLD_BYPASS_SHADOW;
> > +	writel(tmp, priv->regs + DISP_REG_OVL_BLD_SHADOW_CTRL);
> > +	mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_EN, &priv->cmdq_reg,
> > priv->regs,
> > +			   DISP_REG_OVL_BLD_EN, OVL_BLD_EN);
> > +}
> > +
> > +void mtk_disp_blender_stop(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt)
> > +{
> > +	struct mtk_disp_blender *priv = dev_get_drvdata(dev);
> > +
> > +	mtk_ddp_write(cmdq_pkt, 0, &priv->cmdq_reg, priv->regs,
> > DISP_REG_OVL_BLD_L_EN);
> > +	mtk_ddp_write_mask(cmdq_pkt, 0, &priv->cmdq_reg, priv-
> > >regs,
> > +			   DISP_REG_OVL_BLD_EN, OVL_BLD_EN);
> 
> You already control OVL_BLD_EN in layer config, so it's not necessary
> to control here.
> 
> Regards,
> CK
> 
> OK I will fix it.
> 
> Best, Paul
> 
> > +	mtk_ddp_write_mask(cmdq_pkt, OVL_BLD_RST, &priv->cmdq_reg,
> > priv->regs,
> > +			   DISP_REG_OVL_BLD_RST, OVL_BLD_RST);
> > +	mtk_ddp_write_mask(cmdq_pkt, 0, &priv->cmdq_reg, priv-
> > >regs,
> > +			   DISP_REG_OVL_BLD_RST, OVL_BLD_RST);
> > +}
> > +
> > 
>