by Guillaume Ranquet

[permalink] [raw]

Subject: Re: [PATCH v3 01/12] dt-bindings: phy: mediatek: hdmi-phy: Add mt8195 compatible

On Mon, 07 Nov 2022 12:20, AngeloGioacchino Del Regno
<[email protected]> wrote:
>Il 04/11/22 15:09, Guillaume Ranquet ha scritto:
>> Add a compatible for the HDMI PHY on MT8195
>>
>> Acked-by: Krzysztof Kozlowski <[email protected]>
>> Signed-off-by: Guillaume Ranquet <[email protected]>
>
>Ack and R-b tags go after your S-o-b.
>
>Apart from that:
>Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
>
>
>
>

Hi Angelo,

Thx for your review.
I'm not exactly sure what to do here as I'm using b4 trailers to
collect Acked-by and Reviewed-by

And there's an ongoing discussion about that topic on the tools
mailing list [1], it seems there's
no agreement on what to do with trailers ordering.
I'll keep using b4 trailers for now as for me this is the sane thing to do.
I'll update the ordering if I see concensus on that discussion.

Thx,
Guillaume.

---

[1] https://lore.kernel.org/tools/[email protected]

2022-12-26 03:14:38

by CK Hu (胡俊光)

[permalink] [raw]

Subject: Re: [PATCH v3 07/12] drm/mediatek: hdmi: add v2 support

Hi, Guillaume:

On Fri, 2022-11-04 at 15:09 +0100, Guillaume Ranquet wrote:
> Adds hdmi and hdmi-ddc support for v2 IP.
>
> Signed-off-by: Guillaume Ranquet <[email protected]>
> ---
> drivers/gpu/drm/mediatek/Makefile | 2 +
> drivers/gpu/drm/mediatek/mtk_hdmi_common.c | 14 +
> drivers/gpu/drm/mediatek/mtk_hdmi_common.h | 1 +
> drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c | 367 +++++++
> drivers/gpu/drm/mediatek/mtk_hdmi_regs_v2.h | 309 ++++++
> drivers/gpu/drm/mediatek/mtk_hdmi_v2.c | 1379
> +++++++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_hdmi_v2.h | 29 +
> 7 files changed, 2101 insertions(+)
>
>

[snip]

> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_regs_v2.h
> b/drivers/gpu/drm/mediatek/mtk_hdmi_regs_v2.h
> new file mode 100644
> index 000000000000..f4c3652f4eab
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_regs_v2.h
> @@ -0,0 +1,309 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2021 MediaTek Inc.
> + * Copyright (c) 2021 BayLibre, SAS
> + */
> +
> +#ifndef _MTK_HDMI_REGS_H
> +#define _MTK_HDMI_REGS_H
> +
> +#define AIF_HEADER GENMASK(19, 0)
> +#define AIF_PKT00 GENMASK(31, 0)
> +#define AIF_PKT01 (23, 0)
> +#define AIF_PKT02 (31, 0)
> +#define AIF_PKT03 (23, 0)

Useless, drop it.

> +
> +#define AIP_CTRL BIT(10)

I think this is

#define AIP_CTRL 0x400

> +#define AIP_CTS_SVAL 0x408
> +#define AIP_DOWNSAMPLE_CTRL 0x41C
> +#define AIP_I2S_CHST0 0x414
> +#define AIP_I2S_CHST1 0x418
> +#define AIP_I2S_CTRL 0x410
> +#define AIP_N_VAL 0x404
> +#define AIP_SPDIF_CTRL 0x40C
> +#define AIP_TPI_CTRL 0x428
> +#define AIP_TXCTRL 0x424
> +
> +#define AUD_DIS (0x0)
> +#define AUD_DIS_WR (0x0)
> +#define AUD_EN BIT(2)
> +#define AUD_EN_WR BIT(18)
> +#define AUD_ERR_THRESH GENMASK(29, 24)
> +#define AUD_IN_EN BIT(8)
> +#define AUD_MUTE_DIS (0x0)
> +#define AUD_MUTE_FIFO_EN BIT(5)
> +#define AUD_PACKET_DROP BIT(6)
> +#define AUD_RPT_DIS (0x0)
> +#define AUD_RPT_EN BIT(2)
> +#define AUD_SEL_OWRT BIT(9)

I would like to put the bitwise definition with double word definition
together. You could reference [1].

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/mediatek/mtk_disp_rdma.c?h=v6.2-rc1

Regards,
CK

> +
> +#define AVI_DIS (0)
> +#define AVI_DIS_WR (0)
> +#define AVI_EN (0x1)
> +#define AVI_EN_WR BIT(16)
> +#define AVI_HEADER GENMASK(23, 0)
> +#define AVI_PKT00 GENMASK(31, 0)
> +#define AVI_PKT01 GENMASK(23, 0)
> +#define AVI_PKT02 GENMASK(31, 0)
> +#define AVI_PKT03 GENMASK(23, 0)
> +#define AVI_PKT04 GENMASK(31, 0)
> +#define AVI_PKT05 GENMASK(23, 0)
> +#define AVI_RPT_DIS (0x0)
> +#define AVI_RPT_EN (0x1)
> +
> +#define C422_C420_CONFIG_BYPASS BIT(5)
> +#define C422_C420_CONFIG_ENABLE BIT(4)
> +#define C422_C420_CONFIG_OUT_CB_OR_CR BIT(6)
> +#define C444_C422_CONFIG_ENABLE (0x1)
> +
> +#define CBIT_ORDER_SAME BIT(13)
> +
> +#define CEA_AUD_EN BIT(9)
> +#define CEA_AVI_EN BIT(11)
> +#define CEA_CP_EN BIT(6)
> +#define CEA_SPD_EN BIT(10)
> +
> +#define CLEAR_FIFO 0x9
> +
> +#define CLOCK_SCL 0xA
> +
> +#define CP_CLR_MUTE_EN BIT(1)
> +#define CP_EN BIT(5)
> +#define CP_EN_WR BIT(21)
> +#define CP_RPT_EN BIT(5)
> +#define CP_SET_MUTE_DIS (0)
> +#define CP_SET_MUTE_EN (1)
> +
> +#define CTS_CAL_N4 BIT(23)
> +#define CTS_REQ_EN BIT(1)
> +#define CTS_SW_SEL (1)
> +
>