Re: [PATCH v4 15/19] drm/mediatek: add BLENDER support for MT8196

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

On Fri, 2025-09-12 at 02:27 +0000, CK Hu (胡俊光) wrote:
> On Thu, 2025-08-28 at 16:07 +0800, Paul 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>
> > ---
> 
> [snip]
> 
> > 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..9212d2e19ca7
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_blender.c
> > @@ -0,0 +1,274 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2025 MediaTek Inc.
> > + */
> > +
> > +#include <drm/drm_fourcc.h>
> > +#include <drm/drm_blend.h>
> 
> Alphabetic order.
> 
> > +#include <drm/drm_framebuffer.h>
> > +#include <linux/clk.h>
> > +#include <linux/component.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> 
> Alphabetic order.
> 
> > +#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_blender.h"
> > +#include "mtk_disp_drv.h"
> > +#include "mtk_disp_ovl.h"
> > +#include "mtk_drm_drv.h"
> > +
> > +#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
> 
> Move this after DISP_REG_OVL_BLD_BGCLR_CLR, and this name has typo.
> It should be DISP_REG_OVL_BLD_BGCLR_BLACK
> 
> > +#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_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)
> 
> You have already define OVL_BLD_CON_CLRFMT_MAN. Drop this one.
> 
> > +
> > +struct mtk_disp_blender_data {
> > +	unsigned int max_size;
> > +};
> > +
> > +struct mtk_disp_blender {
> > +	void __iomem *regs;
> > +	struct clk *clk;
> > +	struct cmdq_client_reg cmdq_reg;
> > +	const struct mtk_disp_blender_data *data;
> > +};
> > +
> > +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;
> > +	u32 alpha, clrfmt, ignore_pixel_alpha = 0;
> > +	u32 blend_mode = mtk_ovl_get_blend_mode(state,
> > MTK_OVL_SUPPORT_BLEND_MODES);
> > +
> > +	if (!pending->enable || pending->height == 0 || pending-
> > >width == 0 ||
> > +	    pending->x > priv->data->max_size || pending->y >
> > priv->data->max_size) {
> > +		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 | pending-
> > >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);
> 
> The blender data input from exdma.
> exdma would convert YUV format to RGB, so blender would always
> receive RGB format, right?
> If pending->format is YUV, and set YUV format to blender, what does
> this mean?
> 
> 
This is the mechanism of hardware design, we will use the
ignore_pixel_alpha to close it
> > +	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);
> 
> The mask should include OVL_BLD_CON_CLRFMT_NB_10_BIT
> 
> > +
> > +	if (mtk_ovl_is_ignore_pixel_alpha(state, blend_mode))
> > +		ignore_pixel_alpha = OVL_BLD_L0_CONST;
> > +	mtk_ddp_write_mask(cmdq_pkt, ignore_pixel_alpha, &priv-
> > >cmdq_reg, priv->regs,
> > +			   DISP_REG_OVL_BLD_L0_ALPHA_SEL,
> > OVL_BLD_L0_CONST);
> > +
> > +	alpha = (OVL_BLD_L_ALPHA & (state->base.alpha >> 8)) |
> > OVL_BLD_L_ALPHA_EN;
> 
> Why do you always enable alpha?
> In ovl driver, it depend on state->base.fb->format->has_alpha to
> enable alpha.
> 
> > +	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);
> > +}
> > +
> 
> [snip]
> 
> > +static const struct mtk_disp_blender_data
> > mt8196_disp_blender_driver_data = {
> > +	.max_size = 8191,
> 
> Remove this SoC data. and
> 
> #define BLENDER_MAX_SIZE 8181
> 
> > +};
> > +
> > +static const struct of_device_id
> > mtk_disp_blender_driver_dt_match[] = {
> > +	{ .compatible = "mediatek,mt8196-blender",
> > +	  .data = &mt8196_disp_blender_driver_data},
> > +	{},
> > +};
> > +
> 
> [snip]
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_blender.h
> > b/drivers/gpu/drm/mediatek/mtk_disp_blender.h
> > new file mode 100644
> > index 000000000000..a47ab128649d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_blender.h
> > @@ -0,0 +1,10 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2025 MediaTek Inc.
> > + */
> > +
> > +#ifndef __MTK_DISP_BLENDER_H__
> > +#define __MTK_DISP_BLENDER_H__
> > +
> > +u32 mtk_disp_blender_get_blend_modes(struct device *dev);
> 
> Move this prototype to mtk_disp_drv.h
> 
> > +#endif // __MTK_DISP_BLENDER_H__
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > index b9d8f17c4f31..e0aeb87f0a5b 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -9,6 +9,7 @@
> >  #include <linux/soc/mediatek/mtk-cmdq.h>
> >  #include <linux/soc/mediatek/mtk-mmsys.h>
> >  #include <linux/soc/mediatek/mtk-mutex.h>
> > +#include "mtk_disp_blender.h"
> >  #include "mtk_mdp_rdma.h"
> >  #include "mtk_plane.h"
> >  
>