Re: [PATCH v2 11/14] video: mediatek: add new hdmi driver

Le mer. 26 août 2026 à 19:11, David Lechner <[email protected]> a écrit :
>
> On 8/26/26 8:09 AM, Julien Stephan wrote:
> > Add a UCLASS_VIDEO driver for the HDMI TX v2 IP found on the MT8188.
> >
> > On probe, the driver waits for a plugged and powered display, reads
> > its EDID through the DDC bus to select the preferred mode, configures
> > the HDMI controller and PHY accordingly, and then programs all the
> > components of the vdosys1 pipeline (MDP RDMA, padding, VPP merge,
> > ETHDR mixer, mutex, DPI1) to scan out the framebuffer. The
> > framebuffer lives in a dedicated reserved-memory region referenced by
> > the "memory-region" property of the hdmi node.
> >
> > Signed-off-by: Pavlo Yadvychuk <[email protected]>
> > Signed-off-by: Julien Stephan <[email protected]>
> > ---
...
> > +
> > +#define DEEPCOLOR_MODE_10BIT                 BIT(8)
> > +#define DEEPCOLOR_MODE_12BIT                 GENMASK(9, 8)
> > +#define DEEPCOLOR_MODE_16BIT                 GENMASK(10, 8)
> > +#define DEEPCOLOR_MODE_8BIT                  (0)
>
> These are confusing and don't quite match linux. Should be a field with
> GENMASK(9, 8) and values 0, 1, 2, 3.

Hi David,

Right. These defines are actually wrong. I'll use the header from
upstream kernel verbatim, this will fix this issue and some other name
that do not match kernel, with one exception see below.
By the way, deep color is just dead code, only 8 bit is used right
now, so I'll drop the c code.

>
> > +#define DEEPCOLOR_MODE_MASKBIT                       GENMASK(10, 8)
> > +#define DEEPCOLOR_PAT_EN                     BIT(12)
> > +#define DEEP_COLOR_ADD                               BIT(4)
> > +
> > +#define HDMI2_OFF                            (0)
> > +#define HDMI2_ON                             BIT(2)
> > +
> > +#define HDMITX_SW_HPD                                BIT(29)
> > +#define HDMITX_SW_RSTB                               BIT(31)
> > +
> > +#define HDMI_MODE_DVI                                (0)
> > +#define HDMI_MODE_HDMI                               BIT(3)
> > +
> > +#define HDMI_YUV420_MODE                     BIT(10)
> > +
> > +#define HPD_DDC_STATUS                               0xC60
> > +#define PORD_PIN_STA                         BIT(5)
> > +#define HPD_PIN_STA                          BIT(4)
> > +
> > +#define NULL_PKT_EN                          BIT(2)
> > +#define NULL_PKT_VSYNC_HIGH_EN                       BIT(3)
> > +
> > +#define OUTPUT_FORMAT_DEMUX_420_ENABLE               BIT(10)
> > +
> > +#define REG_VMUTE_EN                         BIT(16)
> > +
> > +#define SCR_OFF                                      0
> > +#define SCR_ON                                       BIT(4)
> > +
> > +#define SPD_DIS                                      0
> > +#define SPD_DIS_WR                           0
> > +#define SPD_EN                                       BIT(1)
> > +#define SPD_EN_WR                            BIT(17)
> > +#define SPD_RPT_DIS                          0
> > +#define SPD_RPT_EN                           BIT(1)
> > +
> > +#define TOP_CFG00                            0x000
> > +#define TOP_CFG01                            0x004
> > +#define TOP_INFO_EN                          0x01C
> > +#define TOP_INFO_RPT                         0x020
> > +#define TOP_AVI_HEADER                               0x024
> > +#define TOP_AVI_PKT00                                0x028
> > +#define TOP_AVI_PKT01                                0x02C
> > +#define TOP_AVI_PKT02                                0x030
> > +#define TOP_AVI_PKT03                                0x034
> > +#define TOP_AVI_PKT04                                0x038
> > +#define TOP_AVI_PKT05                                0x03C
> > +#define TOP_INT_MASK00                               0x1B0
> > +#define TOP_INT_MASK01                               0x1B4
>
> These MASK names don't match Linux.

fixed by using verbatim header

>
> > +#define TOP_MISC_CTLR                                0x1A4
> > +#define TOP_SPDIF_HEADER                     0x054
> > +#define TOP_SPDIF_PKT00                              0x058
> > +#define TOP_SPDIF_PKT01                              0x05C
> > +#define TOP_SPDIF_PKT02                              0x060
> > +#define TOP_SPDIF_PKT03                              0x064
> > +#define TOP_SPDIF_PKT04                              0x068
> > +#define TOP_SPDIF_PKT05                              0x06C
> > +#define TOP_SPDIF_PKT06                              0x070
> > +#define TOP_SPDIF_PKT07                              0x074
> > +#define TOP_VMUTE_CFG1                               0x1C8
> > +
> > +#define VID_DOWNSAMPLE_CONFIG                        0x8F0
>
> This doesn't match upstream Linux (but does match downstream). We should
> confirm which is correct.

According to the datasheet, correct value is 0x8f0. Upstream driver is
wrong. Confirmed with MediaTek.
Here is the patch sent upstream to fix it in the kernel
https://lore.kernel.org/all/20260828-mtk-hdmi-v2-fix-register-offset-v1-1-118ad5d7ebe3@baylibre.com/

>
> > +#define VID_OUT_FORMAT                               0x8FC
> > +
> > +#define ABIST_ENABLE                         BIT(31)
> > +
> > +#define HDCP_TOP_CTRL                                0xC00
> > +#define HDMI_CONFIG                          0xEA0
> > +
...