Re: [PATCH v2 02/14] video: mediatek: add ethdr component

On 8/26/26 8:09 AM, Julien Stephan wrote:
> The ETHDR is one of the components of the video pipeline on some
> MediaTek SoCs, such as the MT8188. Only its mixer sub-block is used,
> which is the block's first register window.
> 
> Signed-off-by: Julien Stephan <[email protected]>
> ---
>  drivers/video/mediatek/mtk_ethdr.c | 76 ++++++++++++++++++++++++++++++++++++++
>  drivers/video/mediatek/mtk_ethdr.h | 16 ++++++++
>  2 files changed, 92 insertions(+)
> 
> diff --git a/drivers/video/mediatek/mtk_ethdr.c b/drivers/video/mediatek/mtk_ethdr.c
> new file mode 100644
> index 00000000000..531c95f53a1
> --- /dev/null
> +++ b/drivers/video/mediatek/mtk_ethdr.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mediatek Video ETHDR support
> + *
> + * Copyright (c) 2026 BayLibre, SAS.
> + * Author: Julien Stephan <[email protected]>
> + */
> +
> +#include <dm.h>
> +
> +#include "mtk_disp_comp.h"
> +#include "mtk_ethdr.h"
> +
> +#define MIX_EN			0xc
> +#define MIX_ROI_SIZE		0x18
> +#define MIX_DATAPATH_CON	0x1c
> +#define OUTPUT_NO_RND		BIT(3)
> +#define SOURCE_RGB_SEL		BIT(7)
> +#define BACKGROUND_RELAY	(4 << 9)
> +#define MIX_ROI_BGCLR		0x20
> +#define BGCLR_BLACK		0xff000000
> +#define MIX_SRC_CON		0x24
> +
> +#define MIX_SRC_L2_EN		BIT(2)
> +#define L2_SRC_SEL		(2 << 20)
> +#define L2_OUT_SEL		(2 << 22)
> +
> +#define MIX_FUNC_DCM0		0x120
> +#define MIX_UPD_REG_CK_EN	BIT(0)
> +#define MIX_FUNC_DCM1		0x124
> +#define BG_CLR_CK_EN		BIT(7)
> +#define MIX_CK_EN		BIT(9)
> +#define MIX_FUNC_DCM_ENABLE	0xffffffff
> +
> +#define MIX_L2_CON		0x58
> +#define NON_PREMULTI_SOURCE	(2 << 12)
> +#define MIX_L2_SRC_SIZE		0x60

A few of these macros are unused compare to the Linux implementation. If
that was on purpose we, should probably just remove those macros.

> +
> +void mtk_ethdr_config(struct udevice *dev, u16 width, u16 height)
> +{
> +	/* enable internal clocks */
> +	mtk_disp_comp_write(dev, MIX_FUNC_DCM0, MIX_FUNC_DCM_ENABLE);
> +	mtk_disp_comp_write(dev, MIX_FUNC_DCM1, MIX_FUNC_DCM_ENABLE);
> +
> +	/* enable mixer */
> +	mtk_disp_comp_write(dev, MIX_EN, 0x1);
> +
> +	mtk_disp_comp_write(dev, MIX_ROI_SIZE, (height << 16) | width);
> +	mtk_disp_comp_write(dev, MIX_DATAPATH_CON,
> +			    OUTPUT_NO_RND | SOURCE_RGB_SEL);
> +	mtk_disp_comp_write(dev, MIX_L2_SRC_SIZE, (height << 16) | width);
> +	mtk_disp_comp_write(dev, MIX_ROI_BGCLR, BGCLR_BLACK);
> +
> +	/* only configure L2 since only rdma4 and rdma5 are used */
> +	mtk_disp_comp_write(dev, MIX_L2_CON, NON_PREMULTI_SOURCE);
> +	mtk_disp_comp_write(dev, MIX_SRC_CON,
> +			    MIX_SRC_L2_EN | L2_SRC_SEL | L2_OUT_SEL);
> +}
> +
> +static const struct udevice_id mtk_ethdr_ids[] = {
> +	{ .compatible = "mediatek,mt8188-disp-ethdr" },

We should be able to make this "mediatek,mt8195-disp-ethdr" (the fallback)
so that we don't have to keep adding more compatibles.

> +	{}

nit: patches are inconsistent with style for this. I would prefer `{ }`
as that seems to be the most common.

> +};
> +
> +/*
> + * The mixer is the first register window ("mixer" in reg-names) of the
> + * ethdr block, so binding the ethdr node with default register index 0
> + * gives the mixer registers.
> + */
> +U_BOOT_DRIVER(mtk_ethdr) = {
> +	.name	   = "mtk_ethdr",
> +	.id	   = UCLASS_MISC,
> +	.of_match  = mtk_ethdr_ids,
> +	.probe	   = mtk_disp_comp_probe,
> +	.priv_auto = sizeof(struct mtk_disp_comp_priv),
> +};
> diff --git a/drivers/video/mediatek/mtk_ethdr.h b/drivers/video/mediatek/mtk_ethdr.h
> new file mode 100644
> index 00000000000..038c658a704
> --- /dev/null
> +++ b/drivers/video/mediatek/mtk_ethdr.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Mediatek Video ETHDR support
> + *
> + * Copyright (c) 2026 BayLibre, SAS.
> + * Author: Julien Stephan <[email protected]>
> + */
> +
> +#ifndef _MTK_ETHDR_H
> +#define _MTK_ETHDR_H
> +
> +struct udevice;
> +
> +void mtk_ethdr_config(struct udevice *dev, u16 width, u16 height);
> +
> +#endif
>