Currently, the mtk_hdmi_ddc_v2 driver sends a i2c message by calling
the mtk_ddc_wr_one function for each byte of the payload to setup
SI2C_CTRL and DDC_CTRL registers, and perform a sequential write
transfer of one byte at a time to the target device. This leads to
incorrect transfers as the target address (at least) is also sent each
time.
So, rename mtk_ddc_wr_one function to mtk_ddcm_write_hdmi to match the
read function name (mtk_ddcm_read_hdmi) and modify its behaviour to
send all payload data in a single sequential write transfer by filling
the transfer fifo first then starting the transfer with a size equal to
the payload size and not one anymore.
Fixes: 8d0f79886273 ("drm/mediatek: Introduce HDMI/DDC v2 for MT8195/MT8188")
Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c | 48 ++++++++++++++----------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c
index 6ae7cbba8cb6dacf46c2f7ab74a2d7446d698b69..d937219fdb7ee0ed6a4ac25e950f69f90ff431a3 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c
@@ -66,11 +66,19 @@ static int mtk_ddc_check_and_rise_low_bus(struct mtk_hdmi_ddc *ddc)
return 0;
}
-static int mtk_ddc_wr_one(struct mtk_hdmi_ddc *ddc, u16 addr_id,
- u16 offset_id, u8 *wr_data)
+static int mtk_ddcm_write_hdmi(struct mtk_hdmi_ddc *ddc, u16 addr_id,
+ u16 offset_id, u16 data_cnt, u8 *wr_data)
{
u32 val;
- int ret;
+ int ret, i;
+
+ /* Don't allow transfer with a size over than the transfer fifo size
+ * (16 byte)
+ */
+ if (data_cnt > 16) {
+ dev_err(ddc->dev, "Invalid DDCM write request\n");
+ return -EINVAL;
+ }
/* If down, rise bus for write operation */
mtk_ddc_check_and_rise_low_bus(ddc);
@@ -78,16 +86,21 @@ static int mtk_ddc_wr_one(struct mtk_hdmi_ddc *ddc, u16 addr_id,
regmap_update_bits(ddc->regs, HPD_DDC_CTRL, HPD_DDC_DELAY_CNT,
FIELD_PREP(HPD_DDC_DELAY_CNT, DDC2_DLY_CNT));
+ /* In case there is no payload data, just do a single write for the
+ * address only
+ */
if (wr_data) {
- regmap_write(ddc->regs, SI2C_CTRL,
- FIELD_PREP(SI2C_ADDR, SI2C_ADDR_READ) |
- FIELD_PREP(SI2C_WDATA, *wr_data) |
- SI2C_WR);
+ /* Fill transfer fifo with payload data */
+ for (i = 0; i < data_cnt; i++) {
+ regmap_write(ddc->regs, SI2C_CTRL,
+ FIELD_PREP(SI2C_ADDR, SI2C_ADDR_READ) |
+ FIELD_PREP(SI2C_WDATA, wr_data[i]) |
+ SI2C_WR);
+ }
}
-
regmap_write(ddc->regs, DDC_CTRL,
FIELD_PREP(DDC_CTRL_CMD, DDC_CMD_SEQ_WRITE) |
- FIELD_PREP(DDC_CTRL_DIN_CNT, wr_data == NULL ? 0 : 1) |
+ FIELD_PREP(DDC_CTRL_DIN_CNT, wr_data == NULL ? 0 : data_cnt) |
FIELD_PREP(DDC_CTRL_OFFSET, offset_id) |
FIELD_PREP(DDC_CTRL_ADDR, addr_id));
usleep_range(1000, 1250);
@@ -260,24 +273,9 @@ static int mtk_hdmi_fg_ddc_data_read(struct mtk_hdmi_ddc *ddc, u16 b_dev,
static int mtk_hdmi_ddc_fg_data_write(struct mtk_hdmi_ddc *ddc, u16 b_dev,
u8 data_addr, u16 data_cnt, u8 *pr_data)
{
- int i, ret;
-
regmap_set_bits(ddc->regs, HDCP2X_POL_CTRL, HDCP2X_DIS_POLL_EN);
- /*
- * In case there is no payload data, just do a single write for the
- * address only
- */
- if (data_cnt == 0)
- return mtk_ddc_wr_one(ddc, b_dev, data_addr, NULL);
-
- i = 0;
- do {
- ret = mtk_ddc_wr_one(ddc, b_dev, data_addr + i, pr_data + i);
- if (ret)
- return ret;
- } while (++i < data_cnt);
- return 0;
+ return mtk_ddcm_write_hdmi(ddc, b_dev, data_addr, data_cnt, pr_data);
}
static int mtk_hdmi_ddc_v2_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
--
2.52.0Hi, Louis:
Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com> 於 2025年12月5日週五 下午2:22寫道：
>
> Currently, the mtk_hdmi_ddc_v2 driver sends a i2c message by calling
> the mtk_ddc_wr_one function for each byte of the payload to setup
> SI2C_CTRL and DDC_CTRL registers, and perform a sequential write
> transfer of one byte at a time to the target device. This leads to
> incorrect transfers as the target address (at least) is also sent each
> time.
>
> So, rename mtk_ddc_wr_one function to mtk_ddcm_write_hdmi to match the
> read function name (mtk_ddcm_read_hdmi) and modify its behaviour to
> send all payload data in a single sequential write transfer by filling
> the transfer fifo first then starting the transfer with a size equal to
> the payload size and not one anymore.
Applied to mediatek-drm-fixes [1], thanks.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-fixes
Regards,
Chun-Kuang.
>
> Fixes: 8d0f79886273 ("drm/mediatek: Introduce HDMI/DDC v2 for MT8195/MT8188")
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
> drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c | 48 ++++++++++++++----------------
> 1 file changed, 23 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c
> index 6ae7cbba8cb6dacf46c2f7ab74a2d7446d698b69..d937219fdb7ee0ed6a4ac25e950f69f90ff431a3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c
> @@ -66,11 +66,19 @@ static int mtk_ddc_check_and_rise_low_bus(struct mtk_hdmi_ddc *ddc)
> return 0;
> }
>
> -static int mtk_ddc_wr_one(struct mtk_hdmi_ddc *ddc, u16 addr_id,
> - u16 offset_id, u8 *wr_data)
> +static int mtk_ddcm_write_hdmi(struct mtk_hdmi_ddc *ddc, u16 addr_id,
> + u16 offset_id, u16 data_cnt, u8 *wr_data)
> {
> u32 val;
> - int ret;
> + int ret, i;
> +
> + /* Don't allow transfer with a size over than the transfer fifo size
> + * (16 byte)
> + */
> + if (data_cnt > 16) {
> + dev_err(ddc->dev, "Invalid DDCM write request\n");
> + return -EINVAL;
> + }
>
> /* If down, rise bus for write operation */
> mtk_ddc_check_and_rise_low_bus(ddc);
> @@ -78,16 +86,21 @@ static int mtk_ddc_wr_one(struct mtk_hdmi_ddc *ddc, u16 addr_id,
> regmap_update_bits(ddc->regs, HPD_DDC_CTRL, HPD_DDC_DELAY_CNT,
> FIELD_PREP(HPD_DDC_DELAY_CNT, DDC2_DLY_CNT));
>
> + /* In case there is no payload data, just do a single write for the
> + * address only
> + */
> if (wr_data) {
> - regmap_write(ddc->regs, SI2C_CTRL,
> - FIELD_PREP(SI2C_ADDR, SI2C_ADDR_READ) |
> - FIELD_PREP(SI2C_WDATA, *wr_data) |
> - SI2C_WR);
> + /* Fill transfer fifo with payload data */
> + for (i = 0; i < data_cnt; i++) {
> + regmap_write(ddc->regs, SI2C_CTRL,
> + FIELD_PREP(SI2C_ADDR, SI2C_ADDR_READ) |
> + FIELD_PREP(SI2C_WDATA, wr_data[i]) |
> + SI2C_WR);
> + }
> }
> -
> regmap_write(ddc->regs, DDC_CTRL,
> FIELD_PREP(DDC_CTRL_CMD, DDC_CMD_SEQ_WRITE) |
> - FIELD_PREP(DDC_CTRL_DIN_CNT, wr_data == NULL ? 0 : 1) |
> + FIELD_PREP(DDC_CTRL_DIN_CNT, wr_data == NULL ? 0 : data_cnt) |
> FIELD_PREP(DDC_CTRL_OFFSET, offset_id) |
> FIELD_PREP(DDC_CTRL_ADDR, addr_id));
> usleep_range(1000, 1250);
> @@ -260,24 +273,9 @@ static int mtk_hdmi_fg_ddc_data_read(struct mtk_hdmi_ddc *ddc, u16 b_dev,
> static int mtk_hdmi_ddc_fg_data_write(struct mtk_hdmi_ddc *ddc, u16 b_dev,
> u8 data_addr, u16 data_cnt, u8 *pr_data)
> {
> - int i, ret;
> -
> regmap_set_bits(ddc->regs, HDCP2X_POL_CTRL, HDCP2X_DIS_POLL_EN);
> - /*
> - * In case there is no payload data, just do a single write for the
> - * address only
> - */
> - if (data_cnt == 0)
> - return mtk_ddc_wr_one(ddc, b_dev, data_addr, NULL);
> -
> - i = 0;
> - do {
> - ret = mtk_ddc_wr_one(ddc, b_dev, data_addr + i, pr_data + i);
> - if (ret)
> - return ret;
> - } while (++i < data_cnt);
>
> - return 0;
> + return mtk_ddcm_write_hdmi(ddc, b_dev, data_addr, data_cnt, pr_data);
> }
>
> static int mtk_hdmi_ddc_v2_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
>
> --
> 2.52.0
>
© 2016 - 2026 Red Hat, Inc.