Re: [PATCH v2 08/14] video: mediatek: add dpi1 component

On 8/26/26 8:09 AM, Julien Stephan wrote:
> DPI is one of the components of the video pipeline on some MediaTek
> SoCs, such as the MT8188: it is the timing generator interface
> feeding the HDMI TX. Sync polarities, porches and the interlace mode
> are taken from the struct display_timing selected from the EDID.
> 

...

> +void mtk_dpi_config(struct udevice *dev, const struct display_timing *timing,
> +		    bool rgb)
> +{
> +	struct mtk_dpintf_yc_limit limit;
> +	struct mtk_dpintf_polarities dpi_pol;
> +	struct mtk_dpintf_sync_param hsync;
> +	struct mtk_dpintf_sync_param vsync_lodd = { 0 };
> +	struct mtk_dpintf_sync_param vsync_leven = { 0 };
> +	struct mtk_dpintf_sync_param vsync_rodd = { 0 };
> +	struct mtk_dpintf_sync_param vsync_reven = { 0 };

nit: these can just be `{ }`.

> +	bool interlaced = timing->flags & DISPLAY_FLAGS_INTERLACED;
> +
> +	limit.c_bottom = 0x0010;
> +	limit.c_top = 0x0FE0;
> +	limit.y_bottom = 0x0010;
> +	limit.y_top = 0x0FE0;
> +
> +	dpi_pol.ck_pol = MTK_DPINTF_POLARITY_FALLING;
> +	dpi_pol.de_pol = MTK_DPINTF_POLARITY_RISING;
> +	dpi_pol.hsync_pol = timing->flags & DISPLAY_FLAGS_HSYNC_HIGH ?
> +			    MTK_DPINTF_POLARITY_RISING :
> +			    MTK_DPINTF_POLARITY_FALLING;
> +	dpi_pol.vsync_pol = timing->flags & DISPLAY_FLAGS_VSYNC_HIGH ?
> +			    MTK_DPINTF_POLARITY_RISING :
> +			    MTK_DPINTF_POLARITY_FALLING;
> +
> +	hsync.sync_width = timing->hsync_len.typ;
> +	hsync.back_porch = timing->hback_porch.typ;
> +	hsync.front_porch = timing->hfront_porch.typ;
> +	hsync.shift_half_line = false;
> +
> +	vsync_lodd.sync_width = timing->vsync_len.typ;
> +	vsync_lodd.back_porch = timing->vback_porch.typ;
> +	vsync_lodd.front_porch = timing->vfront_porch.typ;
> +	vsync_lodd.shift_half_line = false;
> +
> +	if (interlaced) {
> +		vsync_leven = vsync_lodd;
> +		vsync_leven.shift_half_line = true;
> +	}
> +
> +	mtk_dpi_sw_reset(dev, true);
> +	mtk_dpi_config_pol(dev, &dpi_pol);
> +
> +	mtk_dpi_config_hsync(dev, &hsync);
> +	mtk_dpi_config_vsync_lodd(dev, &vsync_lodd);
> +	mtk_dpi_config_vsync_rodd(dev, &vsync_rodd);
> +	mtk_dpi_config_vsync_leven(dev, &vsync_leven);
> +	mtk_dpi_config_vsync_reven(dev, &vsync_reven);
> +
> +	mtk_dpi_config_interface(dev, interlaced);
> +	if (interlaced)
> +		mtk_dpi_config_fb_size(dev, timing->hactive.typ,
> +				       timing->vactive.typ / 2);
> +	else
> +		mtk_dpi_config_fb_size(dev, timing->hactive.typ,
> +				       timing->vactive.typ);
> +
> +	mtk_dpi_config_input_2p(dev, true);
> +	mtk_dpi_config_output_1t1p(dev, true);
> +	mtk_dpi_config_channel_limit(dev, &limit);
> +	mtk_dpi_config_bit_num(dev, MTK_DPINTF_OUT_BIT_NUM_8BITS);
> +	mtk_dpi_config_channel_swap(dev, MTK_DPINTF_OUT_CHANNEL_SWAP_RGB);
> +	mtk_dpi_internal_matrix_sel(dev, !rgb);
> +	mtk_dpi_config_yc_map(dev, MTK_DPINTF_OUT_YC_MAP_RGB);
> +	mtk_dpi_config_swap_input(dev, false);
> +	mtk_dpi_sw_reset(dev, false);
> +}
> +
> +static const struct udevice_id mtk_dpi_ids[] = {
> +	{ .compatible = "mediatek,mt8188-dpi" },

Another one where mt8195 fallback should work.

> +	{}
> +};
> +
> +U_BOOT_DRIVER(mtk_dpi) = {
> +	.name	   = "mtk_dpi",
> +	.id	   = UCLASS_MISC,
> +	.of_match  = mtk_dpi_ids,
> +	.probe	   = mtk_disp_comp_probe,
> +	.priv_auto = sizeof(struct mtk_disp_comp_priv),
> +};
> diff --git a/drivers/video/mediatek/mtk_dpi.h b/drivers/video/mediatek/mtk_dpi.h
> new file mode 100644
> index 00000000000..ccce83ec8d3
> --- /dev/null
> +++ b/drivers/video/mediatek/mtk_dpi.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Mediatek MT8188 DPI Support
> + *
> + * Copyright (c) 2026 BayLibre, SAS.
> + * Author: Julien Stephan <[email protected]>
> + */
> +
> +#ifndef _MTK_DPI_H
> +#define _MTK_DPI_H
> +
> +struct display_timing;
> +struct udevice;
> +
> +void mtk_dpi_hw_enable(struct udevice *dev);
> +void mtk_dpi_hw_disable(struct udevice *dev);
> +void mtk_dpi_config(struct udevice *dev, const struct display_timing *timing,
> +		    bool rgb);
> +
> +#endif
> diff --git a/drivers/video/mediatek/mtk_dpi_regs.h b/drivers/video/mediatek/mtk_dpi_regs.h
> new file mode 100644
> index 00000000000..54e370460f0
> --- /dev/null
> +++ b/drivers/video/mediatek/mtk_dpi_regs.h


Does this really need to be a separate header? Seems like we can just put 
this stuff in the .c file.

> @@ -0,0 +1,113 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026 MediaTek Inc.
> + * Copyright (c) 2026 BayLibre, SAS
> + */
> +
> +#ifndef __MTK_DPI_REGS_H
> +#define __MTK_DPI_REGS_H
> +
> +#define DPI_EN				0x00
> +#define EN				(0x1 << 0)
> +
> +#define DPI_RST				0x04
> +#define RST				(0x1 << 0)
> +
> +#define DPI_INTEN			0x08
> +#define INT_VSYNC_EN			(0x1 << 0)
> +
> +#define DPI_CON				0x10
> +#define IN_RB_SWAP			(0x1 << 1)
> +#define INTL_EN				(0x1 << 2)
> +#define TDFP_EN				(0x1 << 3)
> +#define YUV422_EN			(0x1 << 5)
> +#define CSC_ENABLE			(0x1 << 6)
> +#define OUTPUT_1T1P_EN			(0x1 << 24)
> +#define INPUT_2P_EN			(0x1 << 25)
> +
> +#define DPI_OUTPUT_SETTING		0x14
> +#define CH_SWAP				0
> +#define CH_SWAP_MASK			(0x7 << 0)
> +#define SWAP_RGB			0x00
> +#define SWAP_GBR			0x01
> +#define SWAP_BRG			0x02
> +#define SWAP_RBG			0x03
> +#define SWAP_GRB			0x04
> +#define SWAP_BGR			0x05
> +#define DE_POL				(0x1 << 12)
> +#define HSYNC_POL			(0x1 << 13)
> +#define VSYNC_POL			(0x1 << 14)
> +#define CK_POL				(0x1 << 15)
> +#define OUT_BIT				18
> +#define OUT_BIT_MASK			(0x3 << 18)
> +#define OUT_BIT_8			0x00
> +#define OUT_BIT_10			0x01
> +#define OUT_BIT_12			0x02
> +#define OUT_BIT_16			0x03
> +#define YC_MAP				20
> +#define YC_MAP_MASK			(0x7 << 20)
> +#define YC_MAP_RGB			0x00
> +#define YC_MAP_CYCY			0x04
> +#define YC_MAP_YCYC			0x05
> +#define YC_MAP_CY			0x06
> +#define YC_MAP_YC			0x07
> +
> +#define DPI_SIZE			0x18
> +#define HSIZE				0
> +#define HSIZE_MASK			(0x1FFF << 0)
> +#define VSIZE				16
> +#define VSIZE_MASK			(0x1FFF << 16)
> +
> +#define DPI_DDR_SETTING			0x1C
> +#define DDR_EN				(0x1 << 0)
> +#define DDR_4PHASE			(0x1 << 2)
> +
> +#define DPI_TGEN_HWIDTH			0x20
> +#define HPW				0
> +#define HPW_MASK			(0xFFF << 0)
> +
> +#define DPI_TGEN_HPORCH			0x24
> +#define HBP				0
> +#define HBP_MASK			(0xFFF << 0)
> +#define HFP				16
> +#define HFP_MASK			(0xFFF << 16)
> +
> +#define DPI_TGEN_VWIDTH			0x28
> +#define DPI_TGEN_VPORCH			0x2C
> +
> +#define VSYNC_WIDTH_SHIFT		0
> +#define VSYNC_WIDTH_MASK		(0xFFF << 0)
> +#define VSYNC_HALF_LINE_SHIFT		16
> +#define VSYNC_HALF_LINE_MASK		(0xFFF << 16)
> +#define VSYNC_BACK_PORCH_SHIFT		0
> +#define VSYNC_BACK_PORCH_MASK		(0xFFF << 0)
> +#define VSYNC_FRONT_PORCH_SHIFT		16
> +#define VSYNC_FRONT_PORCH_MASK		(0xFFF << 16)
> +
> +#define DPI_TGEN_VWIDTH_LEVEN		0x68
> +#define DPI_TGEN_VPORCH_LEVEN		0x6C
> +#define DPI_TGEN_VWIDTH_RODD		0x70
> +#define DPI_TGEN_VPORCH_RODD		0x74
> +#define DPI_TGEN_VWIDTH_REVEN		0x78
> +#define DPI_TGEN_VPORCH_REVEN		0x7C
> +
> +#define DPI_Y_LIMIT			0x98
> +#define Y_LIMINT_BOT			0
> +#define Y_LIMINT_BOT_MASK		(0xFFF << 0)
> +#define Y_LIMINT_TOP			16
> +#define Y_LIMINT_TOP_MASK		(0xFFF << 16)
> +
> +#define DPI_C_LIMIT			0x9C
> +#define C_LIMIT_BOT			0
> +#define C_LIMIT_BOT_MASK		(0xFFF << 0)
> +#define C_LIMIT_TOP			16
> +#define C_LIMIT_TOP_MASK		(0xFFF << 16)
> +
> +#define DPI_MATRIX_SET			0xB4
> +#define INT_MATRIX_SEL_MASK		(0x1F << 0)
> +#define RGB_TO_BT709			0x03
> +
> +#define DPI_PATTERN0			0xF00
> +#define DPI_PATTERN_EN			BIT(0)
> +#define DPI_PATTERN_COLOR_BAR		(0x4 << 4)
> +#endif /* __MTK_DPI_REGS_H */
> 


Also would be nice to use BIT()/GENMASK().