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

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

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?


> +	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"
>