Re: [PATCH v13 06/14] drm/mediatek: Add HDMI support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Philipp & Jie,

Sorry I only now had a chance to dig deeper and review the HDMI driver.

Lots of comments inline below...

On Tue, Mar 8, 2016 at 9:27 PM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote:
> From: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
>
> This patch adds drivers for the HDMI bridge connected to the DPI0
> display subsystem function block, for the HDMI DDC block, and for
> the HDMI PHY to support HDMI output.
>
> Signed-off-by: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/mediatek/Kconfig               |   7 +
>  drivers/gpu/drm/mediatek/Makefile              |   9 +
>  drivers/gpu/drm/mediatek/mtk_cec.c             | 245 ++++++++++
>  drivers/gpu/drm/mediatek/mtk_cec.h             |  25 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c         |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_hdmi_drv.c    | 579 ++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_hdmi.c            | 479 ++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_hdmi.h            | 221 +++++++++
>  drivers/gpu/drm/mediatek/mtk_hdmi_ddc_drv.c    | 362 ++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_hdmi_hw.c         | 652 +++++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_hdmi_hw.h         |  73 +++
>  drivers/gpu/drm/mediatek/mtk_hdmi_regs.h       | 221 +++++++++
>  drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c | 505 +++++++++++++++++++
>  13 files changed, 3379 insertions(+)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_cec.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_cec.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_drm_hdmi_drv.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_ddc_drv.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_hw.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_hw.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_regs.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c
>
> diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
> index 0c49a94..e2ff158 100644
> --- a/drivers/gpu/drm/mediatek/Kconfig
> +++ b/drivers/gpu/drm/mediatek/Kconfig
> @@ -12,3 +12,10 @@ config DRM_MEDIATEK
>           The module will be called mediatek-drm
>           This driver provides kernel mode setting and
>           buffer management to userspace.
> +
> +config DRM_MEDIATEK_HDMI
> +       tristate "DRM HDMI Support for Mediatek SoCs"
> +       depends on DRM_MEDIATEK
> +       select GENERIC_PHY
> +       help
> +         DRM/KMS HDMI driver for Mediatek SoCs
> diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
> index 5fcf58e..6d53bee 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -12,3 +12,12 @@ mediatek-drm-y := mtk_disp_ovl.o \
>                   mtk_dpi.o
>
>  obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
> +
> +mediatek-drm-hdmi-objs := mtk_cec.o \
> +                         mtk_drm_hdmi_drv.o \
> +                         mtk_hdmi.o \
> +                         mtk_hdmi_ddc_drv.o \
> +                         mtk_hdmi_hw.o \
> +                         mtk_mt8173_hdmi_phy.o
> +
> +obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o
> diff --git a/drivers/gpu/drm/mediatek/mtk_cec.c b/drivers/gpu/drm/mediatek/mtk_cec.c
> new file mode 100644
> index 0000000..cba3647
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_cec.c
> @@ -0,0 +1,245 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +
> +#include "mtk_cec.h"
> +
> +#define TR_CONFIG              0x00
> +#define CLEAR_CEC_IRQ                  BIT(15)
> +
> +#define CEC_CKGEN              0x04
> +#define CEC_32K_PDN                    BIT(19)
> +#define PDN                            BIT(16)
> +
> +#define RX_EVENT               0x54
> +#define HDMI_PORD                      BIT(25)
> +#define HDMI_HTPLG                     BIT(24)
> +#define HDMI_PORD_INT_EN               BIT(9)
> +#define HDMI_HTPLG_INT_EN              BIT(8)
> +
> +#define RX_GEN_WD              0x58
> +#define HDMI_PORD_INT_32K_STATUS       BIT(26)
> +#define RX_RISC_INT_32K_STATUS         BIT(25)
> +#define HDMI_HTPLG_INT_32K_STATUS      BIT(24)
> +#define HDMI_PORD_INT_32K_CLR          BIT(18)
> +#define RX_INT_32K_CLR                 BIT(17)
> +#define HDMI_HTPLG_INT_32K_CLR         BIT(16)
> +#define HDMI_PORD_INT_32K_STA_MASK     BIT(10)
> +#define RX_RISC_INT_32K_STA_MASK       BIT(9)
> +#define HDMI_HTPLG_INT_32K_STA_MASK    BIT(8)
> +#define HDMI_PORD_INT_32K_EN           BIT(2)
> +#define RX_INT_32K_EN                  BIT(1)
> +#define HDMI_HTPLG_INT_32K_EN          BIT(0)
> +
> +#define NORMAL_INT_CTRL                0x5C
> +#define HDMI_HTPLG_INT_STA             BIT(0)
> +#define HDMI_PORD_INT_STA              BIT(1)
> +#define HDMI_HTPLG_INT_CLR             BIT(16)
> +#define HDMI_PORD_INT_CLR              BIT(17)
> +#define HDMI_FULL_INT_CLR              BIT(20)
> +
> +struct mtk_cec {
> +       void __iomem *regs;
> +       struct clk *clk;
> +       int irq;
> +       bool hpd;
> +       void (*hpd_event)(bool hpd, struct device *dev);
> +       struct device *hdmi_dev;
> +};
> +
> +static void mtk_cec_mask(struct mtk_cec *cec, unsigned int offset,
> +                        unsigned int val, unsigned int mask)
> +{
> +       u32 tmp = readl(cec->regs + offset) & ~mask;
> +
> +       tmp |= val & mask;
> +       writel(val, cec->regs + offset);
> +}
> +
> +void mtk_cec_set_hpd_event(struct device *dev,
> +                          void (*hpd_event)(bool hpd, struct device *dev),
> +                          struct device *hdmi_dev)
> +{
> +       struct mtk_cec *cec = dev_get_drvdata(dev);
> +
> +       cec->hdmi_dev = hdmi_dev;
> +       cec->hpd_event = hpd_event;

Lock this so to prevent race with the irq?

> +}
> +
> +bool mtk_cec_hpd_high(struct device *dev)
> +{
> +       struct mtk_cec *cec = dev_get_drvdata(dev);
> +       unsigned int status;
> +
> +       status = readl(cec->regs + RX_EVENT);
> +
> +       return (status & (HDMI_PORD | HDMI_HTPLG)) == (HDMI_PORD | HDMI_HTPLG);
> +}
> +
> +int mtk_cec_irq(struct device *dev)

AFAICT, this function is never used.

> +{
> +       struct mtk_cec *cec = dev_get_drvdata(dev);
> +
> +       return cec->irq;
> +}
> +
> +static void mtk_cec_htplg_irq_enable(struct mtk_cec *cec)
> +{
> +       mtk_cec_mask(cec, CEC_CKGEN, 0, PDN);
> +       mtk_cec_mask(cec, CEC_CKGEN, CEC_32K_PDN, CEC_32K_PDN);
> +       mtk_cec_mask(cec, RX_GEN_WD, HDMI_PORD_INT_32K_CLR,
> +                    HDMI_PORD_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, RX_INT_32K_CLR, RX_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, HDMI_HTPLG_INT_32K_CLR,
> +                    HDMI_HTPLG_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, HDMI_PORD_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, RX_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, HDMI_HTPLG_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, HDMI_PORD_INT_32K_EN);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, RX_INT_32K_EN);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, HDMI_HTPLG_INT_32K_EN);

This is a bit wasteful.  Can you just clear all of these bits in a single write?
(this applies to this entire file).

> +
> +       mtk_cec_mask(cec, RX_EVENT, HDMI_PORD_INT_EN, HDMI_PORD_INT_EN);
> +       mtk_cec_mask(cec, RX_EVENT, HDMI_HTPLG_INT_EN, HDMI_HTPLG_INT_EN);
> +}
> +
> +static void mtk_cec_htplg_irq_disable(struct mtk_cec *cec)
> +{

Why does irq_enable do so much more work than irq_disable?

> +       mtk_cec_mask(cec, RX_EVENT, 0, HDMI_PORD_INT_EN);
> +       mtk_cec_mask(cec, RX_EVENT, 0, HDMI_HTPLG_INT_EN);
> +}
> +
> +static void mtk_cec_clear_htplg_irq(struct mtk_cec *cec)
> +{
> +       mtk_cec_mask(cec, TR_CONFIG, CLEAR_CEC_IRQ, CLEAR_CEC_IRQ);
> +       mtk_cec_mask(cec, NORMAL_INT_CTRL, HDMI_HTPLG_INT_CLR,
> +                    HDMI_HTPLG_INT_CLR);
> +       mtk_cec_mask(cec, NORMAL_INT_CTRL, HDMI_PORD_INT_CLR,
> +                    HDMI_PORD_INT_CLR);
> +       mtk_cec_mask(cec, NORMAL_INT_CTRL, HDMI_FULL_INT_CLR,
> +                    HDMI_FULL_INT_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, HDMI_PORD_INT_32K_CLR,
> +                    HDMI_PORD_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, RX_INT_32K_CLR, RX_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, HDMI_HTPLG_INT_32K_CLR,
> +                    HDMI_HTPLG_INT_32K_CLR);
> +       udelay(5);

Do you really need this delay in the middle of the isr handler?

> +       mtk_cec_mask(cec, NORMAL_INT_CTRL, 0, HDMI_HTPLG_INT_CLR);
> +       mtk_cec_mask(cec, NORMAL_INT_CTRL, 0, HDMI_PORD_INT_CLR);
> +       mtk_cec_mask(cec, TR_CONFIG, 0, CLEAR_CEC_IRQ);
> +       mtk_cec_mask(cec, NORMAL_INT_CTRL, 0, HDMI_FULL_INT_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, HDMI_PORD_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, RX_INT_32K_CLR);
> +       mtk_cec_mask(cec, RX_GEN_WD, 0, HDMI_HTPLG_INT_32K_CLR);
> +}
> +
> +static irqreturn_t mtk_cec_htplg_isr_thread(int irq, void *arg)
> +{
> +       struct device *dev = arg;
> +       struct mtk_cec *cec = dev_get_drvdata(dev);
> +       bool hpd;
> +
> +       mtk_cec_clear_htplg_irq(cec);
> +       hpd = mtk_cec_hpd_high(dev);
> +
> +       if (cec->hpd != hpd) {
> +               dev_info(dev, "hotplug event!,cur hpd = %d, hpd = %d\n",
> +                        cec->hpd, hpd);

dev_dbg if anything

> +               cec->hpd = hpd;
> +               if (cec->hpd_event)
> +                       cec->hpd_event(hpd, cec->hdmi_dev);
> +       }
> +       return IRQ_HANDLED;
> +}
> +
> +static int mtk_cec_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct mtk_cec *cec;
> +       struct resource *res;
> +       int ret;
> +
> +       cec = devm_kzalloc(dev, sizeof(*cec), GFP_KERNEL);
> +       if (!cec)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, cec);
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       cec->regs = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(cec->regs)) {
> +               ret = PTR_ERR(cec->regs);
> +               dev_err(dev, "Failed to ioremap cec: %d\n", ret);
> +               return ret;
> +       }
> +
> +       cec->clk = devm_clk_get(dev, NULL);
> +       if (IS_ERR(cec->clk)) {
> +               ret = PTR_ERR(cec->clk);
> +               dev_err(dev, "Failed to get cec clock: %d\n", ret);
> +               return ret;
> +       }
> +
> +       cec->irq = platform_get_irq(pdev, 0);
> +       if (cec->irq < 0) {
> +               dev_err(dev, "Failed to get cec irq: %d\n", cec->irq);
> +               return cec->irq;
> +       }
> +
> +       ret = devm_request_threaded_irq(dev, cec->irq, NULL,
> +                                       mtk_cec_htplg_isr_thread,
> +                                       IRQF_SHARED | IRQF_TRIGGER_LOW |
> +                                       IRQF_ONESHOT, "hdmi hpd", dev);
> +       if (ret) {
> +               dev_err(dev, "Failed to register cec irq: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = clk_prepare_enable(cec->clk);
> +       if (ret) {
> +               dev_err(dev, "Failed to enable cec clock: %d\n", ret);
> +               return ret;
> +       }
> +
> +       mtk_cec_htplg_irq_enable(cec);
> +
> +       return 0;
> +}
> +
> +static int mtk_cec_remove(struct platform_device *pdev)
> +{
> +       struct mtk_cec *cec = platform_get_drvdata(pdev);
> +
> +       mtk_cec_htplg_irq_disable(cec);
> +       clk_disable_unprepare(cec->clk);
> +       return 0;
> +}
> +
> +static const struct of_device_id mtk_cec_of_ids[] = {
> +       { .compatible = "mediatek,mt8173-cec", },
> +       {}
> +};
> +
> +struct platform_driver mtk_cec_driver = {
> +       .probe = mtk_cec_probe,
> +       .remove = mtk_cec_remove,
> +       .driver = {
> +               .name = "mediatek-cec",
> +               .of_match_table = mtk_cec_of_ids,
> +       },
> +};
> diff --git a/drivers/gpu/drm/mediatek/mtk_cec.h b/drivers/gpu/drm/mediatek/mtk_cec.h
> new file mode 100644
> index 0000000..fe6e8fd
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_cec.h
> @@ -0,0 +1,25 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef _MTK_CEC_H
> +#define _MTK_CEC_H
> +
> +struct device;
> +
> +void mtk_cec_set_hpd_event(struct device *dev,
> +                          void (*hotplug_event)(bool hpd, struct device *dev),
> +                          struct device *hdmi_dev);
> +bool mtk_cec_hpd_high(struct device *dev);
> +int mtk_cec_irq(struct device *dev);
> +
> +#endif /* _MTK_CEC_H */
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index a69958c..80056d1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/pm_runtime.h>
>
> +#include "mtk_cec.h"
>  #include "mtk_drm_crtc.h"
>  #include "mtk_drm_ddp.h"
>  #include "mtk_drm_ddp_comp.h"
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_hdmi_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_hdmi_drv.c
> new file mode 100644
> index 0000000..ff661e0
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_hdmi_drv.c
> @@ -0,0 +1,579 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <linux/clk.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_platform.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_graph.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include "mtk_cec.h"
> +#include "mtk_hdmi.h"
> +#include "mtk_hdmi_hw.h"
> +
> +static const char * const mtk_hdmi_clk_names[MTK_HDMI_CLK_COUNT] = {
> +       [MTK_HDMI_CLK_HDMI_PIXEL] = "pixel",
> +       [MTK_HDMI_CLK_HDMI_PLL] = "pll",
> +       [MTK_HDMI_CLK_AUD_BCLK] = "bclk",
> +       [MTK_HDMI_CLK_AUD_SPDIF] = "spdif",
> +};
> +
> +static const enum mtk_hdmi_clk_id mtk_hdmi_enable_clocks[] = {
> +       MTK_HDMI_CLK_AUD_BCLK,
> +       MTK_HDMI_CLK_AUD_SPDIF,
> +};
> +
> +static int mtk_hdmi_get_all_clk(struct mtk_hdmi *hdmi,
> +                               struct device_node *np)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(mtk_hdmi_clk_names); i++) {
> +               hdmi->clk[i] = of_clk_get_by_name(np,
> +                                                 mtk_hdmi_clk_names[i]);
> +               if (IS_ERR(hdmi->clk[i]))
> +                       return PTR_ERR(hdmi->clk[i]);
> +       }
> +       return 0;
> +}
> +
> +static int mtk_hdmi_clk_enable_audio(struct mtk_hdmi *hdmi)
> +{
> +       int ret;
> +
> +       ret = clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_AUD_BCLK]);
> +       if (ret)
> +               return ret;
> +
> +       ret = clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_AUD_SPDIF]);
> +       if (ret)
> +               goto err;
> +
> +       return 0;
> +err:
> +       clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_AUD_BCLK]);
> +       return ret;
> +}
> +
> +static void mtk_hdmi_clk_disable_audio(struct mtk_hdmi *hdmi)
> +{
> +       clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_AUD_BCLK]);
> +       clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_AUD_SPDIF]);
> +}
> +
> +static enum drm_connector_status hdmi_conn_detect(struct drm_connector *conn,
> +                                                 bool force)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_conn(conn);
> +
> +       return mtk_hdmi_hpd_high(hdmi) ?
> +              connector_status_connected : connector_status_disconnected;
> +}
> +
> +static void hdmi_conn_destroy(struct drm_connector *conn)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_conn(conn);
> +
> +       mtk_cec_set_hpd_event(hdmi->cec_dev, NULL, NULL);
> +
> +       drm_connector_unregister(conn);
> +       drm_connector_cleanup(conn);
> +}
> +
> +static int hdmi_conn_set_property(struct drm_connector *conn,
> +                                 struct drm_property *property, uint64_t val)
> +{
> +       return 0;
> +}
> +
> +static int mtk_hdmi_conn_get_modes(struct drm_connector *conn)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_conn(conn);
> +       struct edid *edid;
> +       int ret;
> +
> +       if (!hdmi->ddc_adpt)
> +               return -ENODEV;
> +
> +       edid = drm_get_edid(conn, hdmi->ddc_adpt);
> +       if (!edid)
> +               return -ENODEV;
> +
> +       hdmi->dvi_mode = !drm_detect_monitor_audio(edid);
> +
> +       drm_mode_connector_update_edid_property(conn, edid);
> +
> +       ret = drm_add_edid_modes(conn, edid);
> +       drm_edid_to_eld(conn, edid);
> +       kfree(edid);
> +       return ret;
> +}
> +
> +static int mtk_hdmi_conn_mode_valid(struct drm_connector *conn,
> +                                   struct drm_display_mode *mode)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_conn(conn);
> +
> +       dev_dbg(hdmi->dev, "xres=%d, yres=%d, refresh=%d, intl=%d clock=%d\n",
> +               mode->hdisplay, mode->vdisplay, mode->vrefresh,
> +               !!(mode->flags & DRM_MODE_FLAG_INTERLACE), mode->clock * 1000);
> +
> +       if (hdmi->bridge.next) {
> +               struct drm_display_mode adjusted_mode;
> +
> +               drm_mode_copy(&adjusted_mode, mode);
> +               if (!drm_bridge_mode_fixup(hdmi->bridge.next, mode,
> +                                          &adjusted_mode))
> +                       return MODE_BAD;
> +       }
> +
> +       if (mode->clock < 27000)
> +               return MODE_CLOCK_LOW;
> +       if (mode->clock > 297000)
> +               return MODE_CLOCK_HIGH;
> +
> +       return drm_mode_validate_size(mode, 0x1fff, 0x1fff);
> +}
> +
> +static struct drm_encoder *mtk_hdmi_conn_best_enc(struct drm_connector *conn)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_conn(conn);
> +
> +       return hdmi->bridge.encoder;
> +}
> +
> +static const struct drm_connector_funcs mtk_hdmi_connector_funcs = {
> +       .dpms = drm_atomic_helper_connector_dpms,
> +       .detect = hdmi_conn_detect,
> +       .fill_modes = drm_helper_probe_single_connector_modes,
> +       .destroy = hdmi_conn_destroy,
> +       .set_property = hdmi_conn_set_property,
> +       .reset = drm_atomic_helper_connector_reset,
> +       .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +       .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static const struct drm_connector_helper_funcs
> +               mtk_hdmi_connector_helper_funcs = {
> +       .get_modes = mtk_hdmi_conn_get_modes,
> +       .mode_valid = mtk_hdmi_conn_mode_valid,
> +       .best_encoder = mtk_hdmi_conn_best_enc,
> +};
> +
> +static void mtk_hdmi_hpd_event(bool hpd, struct device *dev)
> +{
> +       struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +       if (hdmi && hdmi->bridge.encoder && hdmi->bridge.encoder->dev)
> +               drm_helper_hpd_irq_event(hdmi->bridge.encoder->dev);
> +}
> +
> +/*
> + * Bridge callbacks
> + */
> +
> +static int mtk_hdmi_bridge_attach(struct drm_bridge *bridge)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> +       int ret;
> +
> +       ret = drm_connector_init(bridge->encoder->dev, &hdmi->conn,
> +                                &mtk_hdmi_connector_funcs,
> +                                DRM_MODE_CONNECTOR_HDMIA);
> +       if (ret) {
> +               dev_err(hdmi->dev, "Failed to initialize connector: %d\n", ret);
> +               return ret;
> +       }
> +       drm_connector_helper_add(&hdmi->conn, &mtk_hdmi_connector_helper_funcs);
> +
> +       hdmi->conn.polled = DRM_CONNECTOR_POLL_HPD;
> +       hdmi->conn.interlace_allowed = true;
> +       hdmi->conn.doublescan_allowed = false;
> +
> +       ret = drm_connector_register(&hdmi->conn);
> +       if (ret) {
> +               dev_err(hdmi->dev, "Failed to register connector: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = drm_mode_connector_attach_encoder(&hdmi->conn,
> +                                               bridge->encoder);
> +       if (ret) {
> +               dev_err(hdmi->dev,
> +                       "Failed to attach connector to encoder: %d\n", ret);
> +               return ret;
> +       }
> +
> +       if (bridge->next) {
> +               bridge->next->encoder = bridge->encoder;
> +               ret = drm_bridge_attach(bridge->encoder->dev, bridge->next);
> +               if (ret) {
> +                       dev_err(hdmi->dev,
> +                               "Failed to attach external bridge: %d\n", ret);
> +                       return ret;
> +               }
> +       }
> +
> +       mtk_cec_set_hpd_event(hdmi->cec_dev, mtk_hdmi_hpd_event, hdmi->dev);
> +
> +       return 0;
> +}
> +
> +static bool mtk_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
> +                                      const struct drm_display_mode *mode,
> +                                      struct drm_display_mode *adjusted_mode)
> +{
> +       return true;
> +}
> +
> +static void mtk_hdmi_bridge_disable(struct drm_bridge *bridge)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> +
> +       phy_power_off(hdmi->phy);
> +       clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PIXEL]);
> +       clk_disable_unprepare(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]);

As far as I can tell, __drm_helper_disable_unused_functions() doesn't
check if crtc/encoder/bridge are disabled before calling the
->*en/disable*() callbacks.

So, these clk_disable_unprepare() may be called with the HDMI already disabled,
trigerring their WARN_ON().

So, perhaps we also need to track enabled/disabled state separately here in
the driver.

> +}
> +
> +static void mtk_hdmi_bridge_post_disable(struct drm_bridge *bridge)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> +
> +       mtk_hdmi_power_off(hdmi);
> +}
> +
> +static void mtk_hdmi_bridge_mode_set(struct drm_bridge *bridge,
> +                                    struct drm_display_mode *mode,
> +                                    struct drm_display_mode *adjusted_mode)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> +
> +       dev_dbg(hdmi->dev, "cur info: name:%s, hdisplay:%d\n",
> +               adjusted_mode->name, adjusted_mode->hdisplay);
> +       dev_dbg(hdmi->dev, "hsync_start:%d,hsync_end:%d, htotal:%d",
> +               adjusted_mode->hsync_start, adjusted_mode->hsync_end,
> +               adjusted_mode->htotal);
> +       dev_dbg(hdmi->dev, "hskew:%d, vdisplay:%d\n",
> +               adjusted_mode->hskew, adjusted_mode->vdisplay);
> +       dev_dbg(hdmi->dev, "vsync_start:%d, vsync_end:%d, vtotal:%d",
> +               adjusted_mode->vsync_start, adjusted_mode->vsync_end,
> +               adjusted_mode->vtotal);
> +       dev_dbg(hdmi->dev, "vscan:%d, flag:%d\n",
> +               adjusted_mode->vscan, adjusted_mode->flags);
> +
> +       drm_mode_copy(&hdmi->mode, adjusted_mode);
> +}
> +
> +static void mtk_hdmi_bridge_pre_enable(struct drm_bridge *bridge)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> +
> +       mtk_hdmi_power_on(hdmi);
> +}
> +
> +static void mtk_hdmi_bridge_enable(struct drm_bridge *bridge)
> +{
> +       struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
> +
> +       mtk_hdmi_output_set_display_mode(hdmi, &hdmi->mode);
> +       clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]);
> +       clk_prepare_enable(hdmi->clk[MTK_HDMI_CLK_HDMI_PIXEL]);
> +       phy_power_on(hdmi->phy);
> +}
> +
> +static const struct drm_bridge_funcs mtk_hdmi_bridge_funcs = {
> +       .attach = mtk_hdmi_bridge_attach,
> +       .mode_fixup = mtk_hdmi_bridge_mode_fixup,
> +       .disable = mtk_hdmi_bridge_disable,
> +       .post_disable = mtk_hdmi_bridge_post_disable,
> +       .mode_set = mtk_hdmi_bridge_mode_set,
> +       .pre_enable = mtk_hdmi_bridge_pre_enable,
> +       .enable = mtk_hdmi_bridge_enable,
> +};
> +
> +static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
> +                                  struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       struct device_node *cec_np, *port, *ep, *remote, *i2c_np;
> +       struct platform_device *cec_pdev;
> +       struct regmap *regmap;
> +       struct resource *mem;
> +       int ret;
> +
> +       ret = mtk_hdmi_get_all_clk(hdmi, np);
> +       if (ret) {
> +               dev_err(dev, "Failed to get clocks: %d\n", ret);
> +               return ret;
> +       }
> +
> +       /* The CEC module handles HDMI hotplug detection */
> +       cec_np = of_find_compatible_node(np->parent, NULL,
> +                                        "mediatek,mt8173-cec");
> +       if (!cec_np) {
> +               dev_err(dev, "Failed to find CEC node\n");
> +               return -EINVAL;
> +       }
> +
> +       cec_pdev = of_find_device_by_node(cec_np);
> +       if (!cec_pdev) {
> +               dev_err(hdmi->dev, "Waiting for CEC device %s\n",
> +                       cec_np->full_name);
> +               return -EPROBE_DEFER;
> +       }
> +       hdmi->cec_dev = &cec_pdev->dev;
> +
> +       /*
> +        * The mediatek,syscon-hdmi property contains a phandle link to the
> +        * MMSYS_CONFIG device and the register offset of the HDMI_SYS_CFG
> +        * registers it contains.
> +        */
> +       regmap = syscon_regmap_lookup_by_phandle(np, "mediatek,syscon-hdmi");
> +       ret = of_property_read_u32_index(np, "mediatek,syscon-hdmi", 1,
> +                                        &hdmi->sys_offset);
> +       if (IS_ERR(regmap))
> +               ret = PTR_ERR(regmap);
> +       if (ret) {
> +               ret = PTR_ERR(regmap);
> +               dev_err(dev,
> +                       "Failed to get system configuration registers: %d\n",
> +                       ret);
> +               return ret;
> +       }
> +       hdmi->sys_regmap = regmap;
> +
> +       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       hdmi->regs = devm_ioremap_resource(dev, mem);
> +       if (IS_ERR(hdmi->regs)) {
> +               dev_err(dev, "Failed to ioremap hdmi_shell: %ld\n",
> +                       PTR_ERR(hdmi->regs));

What is hdmi_shell?
In any case, I don't think you need to print anything here.

> +               return PTR_ERR(hdmi->regs);
> +       }
> +
> +       port = of_graph_get_port_by_id(np, 1);
> +       if (!port) {
> +               dev_err(dev, "Missing output port node\n");
> +               return -EINVAL;
> +       }
> +
> +       ep = of_get_child_by_name(port, "endpoint");
> +       if (!ep) {
> +               dev_err(dev, "Missing endpoint node in port %s\n",
> +                       port->full_name);
> +               of_node_put(port);
> +               return -EINVAL;
> +       }
> +       of_node_put(port);
> +
> +       remote = of_graph_get_remote_port_parent(ep);
> +       if (!remote) {
> +               dev_err(dev, "Missing connector/bridge node for endpoint %s\n",
> +                       ep->full_name);
> +               of_node_put(ep);
> +               return -EINVAL;
> +       }
> +       of_node_put(ep);
> +
> +       if (!of_device_is_compatible(remote, "hdmi-connector")) {
> +               hdmi->bridge.next = of_drm_find_bridge(remote);
> +               if (!hdmi->bridge.next) {
> +                       dev_err(dev, "Waiting for external bridge\n");
> +                       of_node_put(remote);
> +                       return -EPROBE_DEFER;
> +               }
> +       }
> +
> +       i2c_np = of_parse_phandle(remote, "ddc-i2c-bus", 0);
> +       if (!i2c_np) {
> +               dev_err(dev, "Failed to find ddc-i2c-bus node in %s\n",
> +                       remote->full_name);
> +               of_node_put(remote);
> +               return -EINVAL;
> +       }
> +       of_node_put(remote);
> +
> +       hdmi->ddc_adpt = of_find_i2c_adapter_by_node(i2c_np);
> +       if (!hdmi->ddc_adpt) {
> +               dev_err(dev, "Failed to get ddc i2c adapter by node\n");
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int mtk_drm_hdmi_probe(struct platform_device *pdev)
> +{
> +       struct mtk_hdmi *hdmi;
> +       struct device *dev = &pdev->dev;
> +       int ret;
> +
> +       hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
> +       if (!hdmi)
> +               return -ENOMEM;
> +
> +       hdmi->dev = dev;
> +
> +       ret = mtk_hdmi_dt_parse_pdata(hdmi, pdev);
> +       if (ret)
> +               return ret;
> +
> +       hdmi->phy = devm_phy_get(dev, "hdmi");
> +       if (IS_ERR(hdmi->phy)) {
> +               ret = PTR_ERR(hdmi->phy);
> +               dev_err(dev, "Failed to get HDMI PHY: %d\n", ret);
> +               return ret;
> +       }
> +
> +       platform_set_drvdata(pdev, hdmi);
> +
> +       ret = mtk_hdmi_output_init(hdmi);
> +       if (ret) {
> +               dev_err(dev, "Failed to initialize hdmi output\n");
> +               return ret;
> +       }
> +
> +       hdmi->bridge.funcs = &mtk_hdmi_bridge_funcs;
> +       hdmi->bridge.of_node = pdev->dev.of_node;
> +       ret = drm_bridge_add(&hdmi->bridge);
> +       if (ret) {
> +               dev_err(dev, "failed to add bridge, ret = %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = mtk_hdmi_clk_enable_audio(hdmi);
> +       if (ret) {
> +               dev_err(dev, "Failed to enable audio clocks: %d\n", ret);
> +               goto err_bridge_remove;
> +       }
> +
> +       dev_dbg(dev, "mediatek hdmi probe success\n");
> +       return 0;
> +
> +err_bridge_remove:
> +       drm_bridge_remove(&hdmi->bridge);
> +       return ret;
> +}
> +
> +static int mtk_drm_hdmi_remove(struct platform_device *pdev)
> +{
> +       struct mtk_hdmi *hdmi = platform_get_drvdata(pdev);
> +
> +       drm_bridge_remove(&hdmi->bridge);
> +       platform_device_unregister(hdmi->audio_pdev);

audio_pdev is not set in this patch.
Is there more audio stuff that should be removed from this patch?

> +       platform_set_drvdata(pdev, NULL);

I don't think this is necessary.

> +       mtk_hdmi_clk_disable_audio(hdmi);
> +       return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mtk_hdmi_suspend(struct device *dev)
> +{
> +       struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +       mtk_hdmi_power_off(hdmi);
> +       mtk_hdmi_clk_disable_audio(hdmi);
> +       phy_power_off(hdmi->phy);
> +       dev_dbg(dev, "hdmi suspend success!\n");
> +       return 0;
> +}
> +
> +static int mtk_hdmi_resume(struct device *dev)
> +{
> +       struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +       int ret = 0;
> +
> +       ret = mtk_hdmi_clk_enable_audio(hdmi);
> +       if (ret) {
> +               dev_err(dev, "hdmi resume failed!\n");
> +               return ret;
> +       }
> +
> +       mtk_hdmi_power_on(hdmi);
> +       mtk_hdmi_output_set_display_mode(hdmi, &hdmi->mode);
> +       phy_power_on(hdmi->phy);
> +       dev_dbg(dev, "hdmi resume success!\n");
> +       return 0;
> +}
> +#endif
> +static SIMPLE_DEV_PM_OPS(mtk_hdmi_pm_ops,
> +                        mtk_hdmi_suspend, mtk_hdmi_resume);

I do not think these suspend/resume ops are needed.
The MTK DRM driver turn off connectors at suspend, and re-enables them
at resume.

> +
> +static const struct of_device_id mtk_drm_hdmi_of_ids[] = {
> +       { .compatible = "mediatek,mt8173-hdmi", },
> +       {}
> +};
> +
> +static struct platform_driver mtk_hdmi_driver = {
> +       .probe = mtk_drm_hdmi_probe,
> +       .remove = mtk_drm_hdmi_remove,
> +       .driver = {
> +               .name = "mediatek-drm-hdmi",
> +               .of_match_table = mtk_drm_hdmi_of_ids,
> +               .pm = &mtk_hdmi_pm_ops,
> +       },
> +};
> +
> +static struct platform_driver * const mtk_hdmi_drivers[] = {
> +       &mtk_hdmi_phy_driver,
> +       &mtk_hdmi_ddc_driver,
> +       &mtk_cec_driver,
> +       &mtk_hdmi_driver,
> +};
> +
> +static int __init mtk_hdmitx_init(void)
> +{
> +       int ret;
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(mtk_hdmi_drivers); i++) {
> +               ret = platform_driver_register(mtk_hdmi_drivers[i]);
> +               if (ret < 0) {
> +                       pr_err("Failed to register %s driver: %d\n",
> +                              mtk_hdmi_drivers[i]->driver.name, ret);
> +                       goto err;
> +               }
> +       }
> +
> +       return 0;
> +
> +err:
> +       while (--i >= 0)
> +               platform_driver_unregister(mtk_hdmi_drivers[i]);
> +
> +       return ret;
> +}
> +
> +static void __exit mtk_hdmitx_exit(void)
> +{
> +       int i;
> +
> +       for (i = ARRAY_SIZE(mtk_hdmi_drivers) - 1; i >= 0; i--)
> +               platform_driver_unregister(mtk_hdmi_drivers[i]);
> +}
> +
> +module_init(mtk_hdmitx_init);
> +module_exit(mtk_hdmitx_exit);
> +
> +MODULE_AUTHOR("Jie Qiu <jie.qiu@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("MediaTek HDMI Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> new file mode 100644
> index 0000000..30ec7b5
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c

Hmm... what is the value in splitting this out into its own separate mtk_hdmi.c?

> @@ -0,0 +1,479 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <drm/drm_edid.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/phy/phy.h>
> +#include "mtk_cec.h"
> +#include "mtk_hdmi.h"
> +#include "mtk_hdmi_hw.h"
> +
> +static u8 mtk_hdmi_aud_get_chnl_count(enum hdmi_aud_channel_type channel_type)
> +{
> +       switch (channel_type) {
> +       case HDMI_AUD_CHAN_TYPE_1_0:
> +       case HDMI_AUD_CHAN_TYPE_1_1:
> +       case HDMI_AUD_CHAN_TYPE_2_0:
> +               return 2;
> +       case HDMI_AUD_CHAN_TYPE_2_1:
> +       case HDMI_AUD_CHAN_TYPE_3_0:
> +               return 3;
> +       case HDMI_AUD_CHAN_TYPE_3_1:
> +       case HDMI_AUD_CHAN_TYPE_4_0:
> +       case HDMI_AUD_CHAN_TYPE_3_0_LRS:
> +               return 4;
> +       case HDMI_AUD_CHAN_TYPE_4_1:
> +       case HDMI_AUD_CHAN_TYPE_5_0:
> +       case HDMI_AUD_CHAN_TYPE_3_1_LRS:
> +       case HDMI_AUD_CHAN_TYPE_4_0_CLRS:
> +               return 5;
> +       case HDMI_AUD_CHAN_TYPE_5_1:
> +       case HDMI_AUD_CHAN_TYPE_6_0:
> +       case HDMI_AUD_CHAN_TYPE_4_1_CLRS:
> +       case HDMI_AUD_CHAN_TYPE_6_0_CS:
> +       case HDMI_AUD_CHAN_TYPE_6_0_CH:
> +       case HDMI_AUD_CHAN_TYPE_6_0_OH:
> +       case HDMI_AUD_CHAN_TYPE_6_0_CHR:
> +               return 6;
> +       case HDMI_AUD_CHAN_TYPE_6_1:
> +       case HDMI_AUD_CHAN_TYPE_6_1_CS:
> +       case HDMI_AUD_CHAN_TYPE_6_1_CH:
> +       case HDMI_AUD_CHAN_TYPE_6_1_OH:
> +       case HDMI_AUD_CHAN_TYPE_6_1_CHR:
> +       case HDMI_AUD_CHAN_TYPE_7_0:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LH_RH:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LSR_RSR:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LC_RC:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LW_RW:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LSD_RSD:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LSS_RSS:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LHS_RHS:
> +       case HDMI_AUD_CHAN_TYPE_7_0_CS_CH:
> +       case HDMI_AUD_CHAN_TYPE_7_0_CS_OH:
> +       case HDMI_AUD_CHAN_TYPE_7_0_CS_CHR:
> +       case HDMI_AUD_CHAN_TYPE_7_0_CH_OH:
> +       case HDMI_AUD_CHAN_TYPE_7_0_CH_CHR:
> +       case HDMI_AUD_CHAN_TYPE_7_0_OH_CHR:
> +       case HDMI_AUD_CHAN_TYPE_7_0_LSS_RSS_LSR_RSR:
> +       case HDMI_AUD_CHAN_TYPE_8_0_LH_RH_CS:
> +               return 7;
> +       case HDMI_AUD_CHAN_TYPE_7_1:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LH_RH:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LSR_RSR:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LC_RC:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LW_RW:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LSD_RSD:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LSS_RSS:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LHS_RHS:
> +       case HDMI_AUD_CHAN_TYPE_7_1_CS_CH:
> +       case HDMI_AUD_CHAN_TYPE_7_1_CS_OH:
> +       case HDMI_AUD_CHAN_TYPE_7_1_CS_CHR:
> +       case HDMI_AUD_CHAN_TYPE_7_1_CH_OH:
> +       case HDMI_AUD_CHAN_TYPE_7_1_CH_CHR:
> +       case HDMI_AUD_CHAN_TYPE_7_1_OH_CHR:
> +       case HDMI_AUD_CHAN_TYPE_7_1_LSS_RSS_LSR_RSR:
> +               return 8;
> +       default:
> +               return 2;
> +       }
> +}
> +
> +static int mtk_hdmi_video_change_vpll(struct mtk_hdmi *hdmi, u32 clock)
> +{
> +       unsigned long rate;
> +       int ret;
> +
> +       /* The DPI driver already should have set TVDPLL to the correct rate */
> +       ret = clk_set_rate(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL], clock);
> +       if (ret) {
> +               dev_err(hdmi->dev, "Failed to set PLL to %u Hz: %d\n", clock,
> +                       ret);
> +               return ret;
> +       }
> +
> +       rate = clk_get_rate(hdmi->clk[MTK_HDMI_CLK_HDMI_PLL]);
> +
> +       if (DIV_ROUND_CLOSEST(rate, 1000) != DIV_ROUND_CLOSEST(clock, 1000))
> +               dev_warn(hdmi->dev, "Want PLL %u Hz, got %lu Hz\n", clock,
> +                        rate);
> +       else
> +               dev_dbg(hdmi->dev, "Want PLL %u Hz, got %lu Hz\n", clock, rate);
> +
> +       mtk_hdmi_hw_config_sys(hdmi);
> +       mtk_hdmi_hw_set_deep_color_mode(hdmi);
> +       return 0;
> +}
> +
> +static void mtk_hdmi_video_set_display_mode(struct mtk_hdmi *hdmi,
> +                                           struct drm_display_mode *mode)
> +{
> +       mtk_hdmi_hw_reset(hdmi);
> +       mtk_hdmi_hw_enable_notice(hdmi, true);
> +       mtk_hdmi_hw_write_int_mask(hdmi, 0xff);
> +       mtk_hdmi_hw_enable_dvi_mode(hdmi, hdmi->dvi_mode);
> +       mtk_hdmi_hw_ncts_auto_write_enable(hdmi, true);
> +
> +       mtk_hdmi_hw_msic_setting(hdmi, mode);
> +}
> +
> +static int mtk_hdmi_aud_enable_packet(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_hw_send_aud_packet(hdmi, enable);
> +       return 0;
> +}
> +
> +static int mtk_hdmi_aud_on_off_hw_ncts(struct mtk_hdmi *hdmi, bool on)
> +{
> +       mtk_hdmi_hw_ncts_enable(hdmi, on);
> +       return 0;
> +}
> +
> +static int mtk_hdmi_aud_set_input(struct mtk_hdmi *hdmi)
> +{
> +       u8 chan_count;
> +
> +       mtk_hdmi_hw_aud_set_channel_swap(hdmi, HDMI_AUD_SWAP_LFE_CC);
> +       mtk_hdmi_hw_aud_raw_data_enable(hdmi, true);
> +
> +       if (hdmi->aud_param.aud_input_type == HDMI_AUD_INPUT_SPDIF &&
> +           hdmi->aud_param.aud_codec == HDMI_AUDIO_CODING_TYPE_DST) {
> +               mtk_hdmi_hw_aud_set_bit_num(hdmi,
> +                                           HDMI_AUDIO_SAMPLE_SIZE_24);
> +       } else if (hdmi->aud_param.aud_i2s_fmt ==
> +                       HDMI_I2S_MODE_LJT_24BIT) {
> +               hdmi->aud_param.aud_i2s_fmt = HDMI_I2S_MODE_LJT_16BIT;
> +       }
> +
> +       mtk_hdmi_hw_aud_set_i2s_fmt(hdmi,
> +                                   hdmi->aud_param.aud_i2s_fmt);
> +       mtk_hdmi_hw_aud_set_bit_num(hdmi, HDMI_AUDIO_SAMPLE_SIZE_24);
> +
> +       mtk_hdmi_hw_aud_set_high_bitrate(hdmi, false);
> +       mtk_hdmi_phy_aud_dst_normal_double_enable(hdmi, false);
> +       mtk_hdmi_hw_aud_dst_enable(hdmi, false);

These three all change the same register.  Combine them into a single helper
function that just writes GRL_AUDIO_CFG once.

> +
> +       if (hdmi->aud_param.aud_input_type == HDMI_AUD_INPUT_SPDIF) {
> +               mtk_hdmi_hw_aud_dsd_enable(hdmi, false);
> +               if (hdmi->aud_param.aud_codec ==
> +                       HDMI_AUDIO_CODING_TYPE_DST) {
> +                       mtk_hdmi_phy_aud_dst_normal_double_enable(hdmi,
> +                                                                 true);
> +                       mtk_hdmi_hw_aud_dst_enable(hdmi, true);
> +               }
> +
> +               chan_count = mtk_hdmi_aud_get_chnl_count
> +                                                (HDMI_AUD_CHAN_TYPE_2_0);
> +               mtk_hdmi_hw_aud_set_i2s_chan_num(hdmi,
> +                                                HDMI_AUD_CHAN_TYPE_2_0,
> +                                                chan_count);
> +               mtk_hdmi_hw_aud_set_input_type(hdmi,
> +                                              HDMI_AUD_INPUT_SPDIF);
> +       } else {
> +               mtk_hdmi_hw_aud_dsd_enable(hdmi, false);
> +               chan_count =
> +                       mtk_hdmi_aud_get_chnl_count(
> +                       hdmi->aud_param.aud_input_chan_type);
> +               mtk_hdmi_hw_aud_set_i2s_chan_num(
> +                       hdmi,
> +                       hdmi->aud_param.aud_input_chan_type,
> +                       chan_count);
> +               mtk_hdmi_hw_aud_set_input_type(hdmi,
> +                                              HDMI_AUD_INPUT_I2S);
> +       }
> +       return 0;
> +}
> +
> +static int mtk_hdmi_aud_set_src(struct mtk_hdmi *hdmi,
> +                               struct drm_display_mode *display_mode)
> +{
> +       mtk_hdmi_aud_on_off_hw_ncts(hdmi, false);
> +
> +       if (hdmi->aud_param.aud_input_type == HDMI_AUD_INPUT_I2S) {
> +               switch (hdmi->aud_param.aud_hdmi_fs) {
> +               case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
> +               case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
> +               case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
> +               case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
> +               case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
> +                       mtk_hdmi_hw_aud_src_off(hdmi);
> +                       /* mtk_hdmi_hw_aud_src_enable(hdmi, false); */

why is this commented out?

> +                       mtk_hdmi_hw_aud_set_mclk(
> +                       hdmi,
> +                       hdmi->aud_param.aud_mclk);

indentation is funny here

> +                       mtk_hdmi_hw_aud_aclk_inv_enable(hdmi, false);
> +                       break;
> +               default:
> +                       break;
> +               }
> +       } else {
> +               switch (hdmi->aud_param.iec_frame_fs) {
> +               case HDMI_IEC_32K:
> +                       hdmi->aud_param.aud_hdmi_fs =
> +                           HDMI_AUDIO_SAMPLE_FREQUENCY_32000;

indentation is funny here

> +                       mtk_hdmi_hw_aud_src_off(hdmi);
> +                       mtk_hdmi_hw_aud_set_mclk(hdmi,
> +                                                HDMI_AUD_MCLK_128FS);
> +                       mtk_hdmi_hw_aud_aclk_inv_enable(hdmi, false);
> +                       break;
> +               case HDMI_IEC_48K:
> +                       hdmi->aud_param.aud_hdmi_fs =
> +                           HDMI_AUDIO_SAMPLE_FREQUENCY_48000;

indentation is funny here

> +                       mtk_hdmi_hw_aud_src_off(hdmi);
> +                       mtk_hdmi_hw_aud_set_mclk(hdmi,
> +                                                HDMI_AUD_MCLK_128FS);
> +                       mtk_hdmi_hw_aud_aclk_inv_enable(hdmi, false);
> +                       break;
> +               case HDMI_IEC_44K:
> +                       hdmi->aud_param.aud_hdmi_fs =
> +                           HDMI_AUDIO_SAMPLE_FREQUENCY_44100;

indentation is funny here

> +                       mtk_hdmi_hw_aud_src_off(hdmi);
> +                       mtk_hdmi_hw_aud_set_mclk(hdmi,
> +                                                HDMI_AUD_MCLK_128FS);
> +                       mtk_hdmi_hw_aud_aclk_inv_enable(hdmi, false);

These clauses all seem to do the same thing; only the mclk parameter is
different.  Can you refactor them into a helper function?

> +                       break;
> +               default:
> +                       break;
> +               }
> +       }
> +       mtk_hdmi_hw_aud_set_ncts(hdmi, hdmi->aud_param.aud_hdmi_fs,
> +                                display_mode->clock);
> +
> +       mtk_hdmi_hw_aud_src_reenable(hdmi);
> +       return 0;
> +}
> +
> +static int mtk_hdmi_aud_set_chnl_status(struct mtk_hdmi *hdmi)
> +{
> +       mtk_hdmi_hw_aud_set_channel_status(
> +               hdmi,
> +          hdmi->aud_param.hdmi_l_channel_state,
> +          hdmi->aud_param.hdmi_r_channel_state,
> +          hdmi->aud_param.aud_hdmi_fs);

indentation is funny here

> +       return 0;
> +}
> +
> +static int mtk_hdmi_aud_output_config(struct mtk_hdmi *hdmi,
> +                                     struct drm_display_mode *display_mode)
> +{
> +       mtk_hdmi_hw_aud_mute(hdmi, true);
> +       mtk_hdmi_aud_enable_packet(hdmi, false);
> +
> +       mtk_hdmi_aud_set_input(hdmi);
> +       mtk_hdmi_aud_set_src(hdmi, display_mode);
> +       mtk_hdmi_aud_set_chnl_status(hdmi);
> +
> +       usleep_range(50, 100);
> +
> +       mtk_hdmi_aud_on_off_hw_ncts(hdmi, true);
> +       mtk_hdmi_aud_enable_packet(hdmi, true);
> +       mtk_hdmi_hw_aud_mute(hdmi, false);
> +       return 0;
> +}
> +
> +static int mtk_hdmi_setup_av_mute_packet(struct mtk_hdmi *hdmi)
> +{
> +       mtk_hdmi_hw_send_av_mute(hdmi);
> +       return 0;
> +}
> +
> +static int mtk_hdmi_setup_av_unmute_packet(struct mtk_hdmi *hdmi)
> +{
> +       mtk_hdmi_hw_send_av_unmute(hdmi);
> +       return 0;
> +}
> +
> +static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
> +                                       struct drm_display_mode *mode)
> +{
> +       struct hdmi_avi_infoframe frame;
> +       u8 buffer[17];
> +       ssize_t err;
> +
> +       err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> +       if (err < 0) {
> +               dev_err(hdmi->dev,
> +                       "Failed to get AVI infoframe from mode: %zd\n", err);
> +               return err;
> +       }
> +
> +       err = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));
> +       if (err < 0) {
> +               dev_err(hdmi->dev, "Failed to pack AVI infoframe: %zd\n", err);
> +               return err;
> +       }
> +
> +       mtk_hdmi_hw_send_info_frame(hdmi, buffer, sizeof(buffer));
> +       return 0;
> +}
> +
> +static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
> +                                       const char *vendor,
> +                                       const char *product)
> +{
> +       struct hdmi_spd_infoframe frame;
> +       u8 buffer[29];
> +       ssize_t err;
> +
> +       err = hdmi_spd_infoframe_init(&frame, vendor, product);
> +       if (err < 0) {
> +               dev_err(hdmi->dev, "Failed to initialize SPD infoframe: %zd\n",
> +                       err);
> +               return err;
> +       }
> +
> +       err = hdmi_spd_infoframe_pack(&frame, buffer, sizeof(buffer));
> +       if (err < 0) {
> +               dev_err(hdmi->dev, "Failed to pack SDP infoframe: %zd\n", err);
> +               return err;
> +       }
> +
> +       mtk_hdmi_hw_send_info_frame(hdmi, buffer, sizeof(buffer));
> +       return 0;
> +}
> +
> +static int mtk_hdmi_setup_audio_infoframe(struct mtk_hdmi *hdmi)
> +{
> +       struct hdmi_audio_infoframe frame;
> +       u8 buffer[14];
> +       ssize_t err;
> +
> +       err = hdmi_audio_infoframe_init(&frame);
> +       if (err < 0) {
> +               dev_err(hdmi->dev, "Failed to setup audio infoframe: %zd\n",
> +                       err);
> +               return err;
> +       }
> +
> +       frame.coding_type = HDMI_AUDIO_CODING_TYPE_STREAM;
> +       frame.sample_frequency = HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM;
> +       frame.sample_size = HDMI_AUDIO_SAMPLE_SIZE_STREAM;
> +       frame.channels =
> +           mtk_hdmi_aud_get_chnl_count(
> +           hdmi->aud_param.aud_input_chan_type);

indentation is funny

> +
> +       err = hdmi_audio_infoframe_pack(&frame, buffer, sizeof(buffer));
> +       if (err < 0) {
> +               dev_err(hdmi->dev, "Failed to pack audio infoframe: %zd\n",
> +                       err);
> +               return err;
> +       }
> +
> +       mtk_hdmi_hw_send_info_frame(hdmi, buffer, sizeof(buffer));
> +       return 0;
> +}
> +
> +static int mtk_hdmi_setup_vendor_specific_infoframe(struct mtk_hdmi *hdmi,
> +                                               struct drm_display_mode *mode)
> +{
> +       struct hdmi_vendor_infoframe frame;
> +       u8 buffer[10];
> +       ssize_t err;
> +
> +       err = drm_hdmi_vendor_infoframe_from_display_mode(&frame, mode);
> +       if (err) {
> +               dev_err(hdmi->dev,
> +                       "Failed to get vendor infoframe from mode: %zd\n", err);
> +               return err;
> +       }
> +
> +       err = hdmi_vendor_infoframe_pack(&frame, buffer, sizeof(buffer));
> +       if (err) {
> +               dev_err(hdmi->dev, "Failed to pack vendor infoframe: %zd\n",
> +                       err);
> +               return err;
> +       }
> +
> +       mtk_hdmi_hw_send_info_frame(hdmi, buffer, sizeof(buffer));
> +       return 0;
> +}
> +
> +int mtk_hdmi_hpd_high(struct mtk_hdmi *hdmi)
> +{
> +       return hdmi->cec_dev ? mtk_cec_hpd_high(hdmi->cec_dev) : false;

I don't think we would get this far if cec_dev was NULL.

> +}
> +
> +int mtk_hdmi_output_init(struct mtk_hdmi *hdmi)
> +{
> +       struct hdmi_audio_param *aud_param = &hdmi->aud_param;
> +
> +       if (hdmi->init)
> +               return -EINVAL;

This check is not needed; this function is only called once, during probe.
In fact, I don't think the "->init" field is needed at all.

> +
> +       hdmi->csp = HDMI_COLORSPACE_RGB;
> +       hdmi->output = true;
> +       aud_param->aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
> +       aud_param->aud_hdmi_fs = HDMI_AUDIO_SAMPLE_FREQUENCY_48000;
> +       aud_param->aud_sampe_size = HDMI_AUDIO_SAMPLE_SIZE_16;
> +       aud_param->aud_input_type = HDMI_AUD_INPUT_I2S;
> +       aud_param->aud_i2s_fmt = HDMI_I2S_MODE_I2S_24BIT;
> +       aud_param->aud_mclk = HDMI_AUD_MCLK_128FS;
> +       aud_param->iec_frame_fs = HDMI_IEC_48K;
> +       aud_param->aud_input_chan_type = HDMI_AUD_CHAN_TYPE_2_0;
> +       aud_param->hdmi_l_channel_state[2] = 2;
> +       aud_param->hdmi_r_channel_state[2] = 2;
> +       hdmi->init = true;
> +
> +       return 0;
> +}
> +
> +void mtk_hdmi_power_on(struct mtk_hdmi *hdmi)
> +{
> +       mtk_hdmi_hw_make_reg_writable(hdmi, true);
> +       mtk_hdmi_hw_1p4_version_enable(hdmi, true);
> +}
> +
> +void mtk_hdmi_power_off(struct mtk_hdmi *hdmi)
> +{
> +       mtk_hdmi_hw_1p4_version_enable(hdmi, true);
> +       mtk_hdmi_hw_make_reg_writable(hdmi, false);
> +}
> +
> +int mtk_hdmi_output_set_display_mode(struct mtk_hdmi *hdmi,
> +                                    struct drm_display_mode *mode)
> +{
> +       int ret;
> +
> +       if (!hdmi->init) {

Is this possible?

> +               dev_err(hdmi->dev, "doesn't init hdmi control context!\n");
> +               return -EINVAL;
> +       }
> +
> +       mtk_hdmi_hw_vid_black(hdmi, true);
> +       mtk_hdmi_hw_aud_mute(hdmi, true);
> +       mtk_hdmi_setup_av_mute_packet(hdmi);
> +       phy_power_off(hdmi->phy);
> +
> +       ret = mtk_hdmi_video_change_vpll(hdmi,
> +                                        mode->clock * 1000);
> +       if (ret) {
> +               dev_err(hdmi->dev, "Failed to set vpll: %d\n", ret);

cleanup on error?

> +               return ret;
> +       }
> +       mtk_hdmi_video_set_display_mode(hdmi, mode);
> +
> +       phy_power_on(hdmi->phy);
> +       mtk_hdmi_aud_output_config(hdmi, mode);
> +
> +       mtk_hdmi_setup_audio_infoframe(hdmi);
> +       mtk_hdmi_setup_avi_infoframe(hdmi, mode);
> +       mtk_hdmi_setup_spd_infoframe(hdmi, "mediatek", "chromebook");

what?  No.  The "product" should refer to the MTK HDMI block.

> +       if (mode->flags & DRM_MODE_FLAG_3D_MASK)
> +               mtk_hdmi_setup_vendor_specific_infoframe(hdmi, mode);
> +
> +       mtk_hdmi_hw_vid_black(hdmi, false);
> +       mtk_hdmi_hw_aud_mute(hdmi, false);
> +       mtk_hdmi_setup_av_unmute_packet(hdmi);
> +
> +       return 0;
> +}
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.h b/drivers/gpu/drm/mediatek/mtk_hdmi.h
> new file mode 100644
> index 0000000..9403915
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.h
> @@ -0,0 +1,221 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef _MTK_HDMI_CTRL_H
> +#define _MTK_HDMI_CTRL_H
> +
> +#include <drm/drm_crtc.h>
> +#include <linux/hdmi.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +
> +struct clk;
> +struct device;
> +struct device_node;
> +struct i2c_adapter;
> +struct platform_device;
> +struct phy;
> +struct regmap;
> +
> +enum mtk_hdmi_clk_id {
> +       MTK_HDMI_CLK_HDMI_PIXEL,
> +       MTK_HDMI_CLK_HDMI_PLL,
> +       MTK_HDMI_CLK_AUD_BCLK,
> +       MTK_HDMI_CLK_AUD_SPDIF,
> +       MTK_HDMI_CLK_COUNT
> +};
> +
> +enum hdmi_aud_input_type {
> +       HDMI_AUD_INPUT_I2S = 0,
> +       HDMI_AUD_INPUT_SPDIF,
> +};
> +
> +enum hdmi_aud_i2s_fmt {
> +       HDMI_I2S_MODE_RJT_24BIT = 0,
> +       HDMI_I2S_MODE_RJT_16BIT,
> +       HDMI_I2S_MODE_LJT_24BIT,
> +       HDMI_I2S_MODE_LJT_16BIT,
> +       HDMI_I2S_MODE_I2S_24BIT,
> +       HDMI_I2S_MODE_I2S_16BIT
> +};
> +
> +enum hdmi_aud_mclk {
> +       HDMI_AUD_MCLK_128FS,
> +       HDMI_AUD_MCLK_192FS,
> +       HDMI_AUD_MCLK_256FS,
> +       HDMI_AUD_MCLK_384FS,
> +       HDMI_AUD_MCLK_512FS,
> +       HDMI_AUD_MCLK_768FS,
> +       HDMI_AUD_MCLK_1152FS,
> +};
> +
> +enum hdmi_aud_iec_frame_rate {
> +       HDMI_IEC_32K = 0,
> +       HDMI_IEC_96K,
> +       HDMI_IEC_192K,
> +       HDMI_IEC_768K,
> +       HDMI_IEC_44K,
> +       HDMI_IEC_88K,
> +       HDMI_IEC_176K,
> +       HDMI_IEC_705K,
> +       HDMI_IEC_16K,
> +       HDMI_IEC_22K,
> +       HDMI_IEC_24K,
> +       HDMI_IEC_48K,
> +};
> +
> +enum hdmi_aud_channel_type {
> +       HDMI_AUD_CHAN_TYPE_1_0 = 0,
> +       HDMI_AUD_CHAN_TYPE_1_1,
> +       HDMI_AUD_CHAN_TYPE_2_0,
> +       HDMI_AUD_CHAN_TYPE_2_1,
> +       HDMI_AUD_CHAN_TYPE_3_0,
> +       HDMI_AUD_CHAN_TYPE_3_1,
> +       HDMI_AUD_CHAN_TYPE_4_0,
> +       HDMI_AUD_CHAN_TYPE_4_1,
> +       HDMI_AUD_CHAN_TYPE_5_0,
> +       HDMI_AUD_CHAN_TYPE_5_1,
> +       HDMI_AUD_CHAN_TYPE_6_0,
> +       HDMI_AUD_CHAN_TYPE_6_1,
> +       HDMI_AUD_CHAN_TYPE_7_0,
> +       HDMI_AUD_CHAN_TYPE_7_1,
> +       HDMI_AUD_CHAN_TYPE_3_0_LRS,
> +       HDMI_AUD_CHAN_TYPE_3_1_LRS,
> +       HDMI_AUD_CHAN_TYPE_4_0_CLRS,
> +       HDMI_AUD_CHAN_TYPE_4_1_CLRS,
> +       HDMI_AUD_CHAN_TYPE_6_1_CS,
> +       HDMI_AUD_CHAN_TYPE_6_1_CH,
> +       HDMI_AUD_CHAN_TYPE_6_1_OH,
> +       HDMI_AUD_CHAN_TYPE_6_1_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_1_LH_RH,
> +       HDMI_AUD_CHAN_TYPE_7_1_LSR_RSR,
> +       HDMI_AUD_CHAN_TYPE_7_1_LC_RC,
> +       HDMI_AUD_CHAN_TYPE_7_1_LW_RW,
> +       HDMI_AUD_CHAN_TYPE_7_1_LSD_RSD,
> +       HDMI_AUD_CHAN_TYPE_7_1_LSS_RSS,
> +       HDMI_AUD_CHAN_TYPE_7_1_LHS_RHS,
> +       HDMI_AUD_CHAN_TYPE_7_1_CS_CH,
> +       HDMI_AUD_CHAN_TYPE_7_1_CS_OH,
> +       HDMI_AUD_CHAN_TYPE_7_1_CS_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_1_CH_OH,
> +       HDMI_AUD_CHAN_TYPE_7_1_CH_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_1_OH_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_1_LSS_RSS_LSR_RSR,
> +       HDMI_AUD_CHAN_TYPE_6_0_CS,
> +       HDMI_AUD_CHAN_TYPE_6_0_CH,
> +       HDMI_AUD_CHAN_TYPE_6_0_OH,
> +       HDMI_AUD_CHAN_TYPE_6_0_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_0_LH_RH,
> +       HDMI_AUD_CHAN_TYPE_7_0_LSR_RSR,
> +       HDMI_AUD_CHAN_TYPE_7_0_LC_RC,
> +       HDMI_AUD_CHAN_TYPE_7_0_LW_RW,
> +       HDMI_AUD_CHAN_TYPE_7_0_LSD_RSD,
> +       HDMI_AUD_CHAN_TYPE_7_0_LSS_RSS,
> +       HDMI_AUD_CHAN_TYPE_7_0_LHS_RHS,
> +       HDMI_AUD_CHAN_TYPE_7_0_CS_CH,
> +       HDMI_AUD_CHAN_TYPE_7_0_CS_OH,
> +       HDMI_AUD_CHAN_TYPE_7_0_CS_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_0_CH_OH,
> +       HDMI_AUD_CHAN_TYPE_7_0_CH_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_0_OH_CHR,
> +       HDMI_AUD_CHAN_TYPE_7_0_LSS_RSS_LSR_RSR,
> +       HDMI_AUD_CHAN_TYPE_8_0_LH_RH_CS,
> +       HDMI_AUD_CHAN_TYPE_UNKNOWN = 0xFF
> +};
> +
> +enum hdmi_aud_channel_swap_type {
> +       HDMI_AUD_SWAP_LR,
> +       HDMI_AUD_SWAP_LFE_CC,
> +       HDMI_AUD_SWAP_LSRS,
> +       HDMI_AUD_SWAP_RLS_RRS,
> +       HDMI_AUD_SWAP_LR_STATUS,
> +};
> +
> +struct hdmi_audio_param {
> +       enum hdmi_audio_coding_type aud_codec;
> +       enum hdmi_audio_sample_frequency aud_hdmi_fs;
> +       enum hdmi_audio_sample_size aud_sampe_size;
> +       enum hdmi_aud_input_type aud_input_type;
> +       enum hdmi_aud_i2s_fmt aud_i2s_fmt;
> +       enum hdmi_aud_mclk aud_mclk;
> +       enum hdmi_aud_iec_frame_rate iec_frame_fs;
> +       enum hdmi_aud_channel_type aud_input_chan_type;
> +       u8 hdmi_l_channel_state[6];
> +       u8 hdmi_r_channel_state[6];
> +};
> +
> +struct mtk_hdmi {
> +       struct drm_bridge bridge;
> +       struct drm_connector conn;
> +       struct device *dev;
> +       struct phy *phy;
> +       struct device *cec_dev;
> +       struct i2c_adapter *ddc_adpt;
> +       struct clk *clk[MTK_HDMI_CLK_COUNT];
> +#if defined(CONFIG_DEBUG_FS)
> +       struct dentry *debugfs;
> +#endif

Remove all of the debugfs stuff from this patch, since it isn't implemented.

> +       struct platform_device *audio_pdev;
> +       struct drm_display_mode mode;
> +       bool dvi_mode;
> +       u32 min_clock;
> +       u32 max_clock;
> +       u32 max_hdisplay;
> +       u32 max_vdisplay;
> +       u32 ibias;
> +       u32 ibias_up;
> +       struct regmap *sys_regmap;
> +       unsigned int sys_offset;
> +       void __iomem *regs;
> +       bool init;
> +       enum hdmi_colorspace csp;
> +       bool audio_enable;
> +       bool output;
> +       struct hdmi_audio_param aud_param;
> +};
> +
> +static inline struct mtk_hdmi *hdmi_ctx_from_bridge(struct drm_bridge *b)
> +{
> +       return container_of(b, struct mtk_hdmi, bridge);
> +}
> +
> +static inline struct mtk_hdmi *hdmi_ctx_from_conn(struct drm_connector *c)
> +{
> +       return container_of(c, struct mtk_hdmi, conn);
> +}
> +
> +int mtk_hdmi_output_init(struct mtk_hdmi *hdmi);
> +int mtk_hdmi_hpd_high(struct mtk_hdmi *hdmi);
> +int mtk_hdmi_output_set_display_mode(struct mtk_hdmi *hdmi,
> +                                    struct drm_display_mode *mode);
> +void mtk_hdmi_power_on(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_power_off(struct mtk_hdmi *hdmi);
> +#if defined(CONFIG_DEBUG_FS)
> +int mtk_drm_hdmi_debugfs_init(struct mtk_hdmi *hdmi);
> +void mtk_drm_hdmi_debugfs_exit(struct mtk_hdmi *hdmi);
> +#else
> +int mtk_drm_hdmi_debugfs_init(struct mtk_hdmi *hdmi)
> +{
> +       return 0;
> +}
> +
> +void mtk_drm_hdmi_debugfs_exit(struct mtk_hdmi *hdmi)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
> +extern struct platform_driver mtk_cec_driver;
> +extern struct platform_driver mtk_hdmi_ddc_driver;
> +extern struct platform_driver mtk_hdmi_phy_driver;
> +#endif /* _MTK_HDMI_CTRL_H */
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_drv.c b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_drv.c
> new file mode 100644
> index 0000000..22e5487
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_ddc_drv.c
> @@ -0,0 +1,362 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/time.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +
> +#define SIF1_CLOK              (288)
> +#define DDC_DDCMCTL0           (0x0)
> +#define DDCM_ODRAIN                    BIT(31)
> +#define DDCM_CLK_DIV_OFFSET            (16)
> +#define DDCM_CLK_DIV_MASK              (0xfff << 16)
> +#define DDCM_CS_STATUS                 BIT(4)
> +#define DDCM_SCL_STATE                 BIT(3)
> +#define DDCM_SDA_STATE                 BIT(2)
> +#define DDCM_SM0EN                     BIT(1)
> +#define DDCM_SCL_STRECH                        BIT(0)
> +#define DDC_DDCMCTL1           (0x4)
> +#define DDCM_ACK_OFFSET                        (16)
> +#define DDCM_ACK_MASK                  (0xff << 16)
> +#define DDCM_PGLEN_OFFSET              (8)
> +#define DDCM_PGLEN_MASK                        (0x7 << 8)
> +#define DDCM_SIF_MODE_OFFSET           (4)
> +#define DDCM_SIF_MODE_MASK             (0x7 << 4)
> +#define DDCM_START                     (0x1)
> +#define DDCM_WRITE_DATA                        (0x2)
> +#define DDCM_STOP                      (0x3)
> +#define DDCM_READ_DATA_NO_ACK          (0x4)
> +#define DDCM_READ_DATA_ACK             (0x5)
> +#define DDCM_TRI                       BIT(0)
> +#define DDC_DDCMD0             (0x8)
> +#define DDCM_DATA3                     (0xff << 24)
> +#define DDCM_DATA2                     (0xff << 16)
> +#define DDCM_DATA1                     (0xff << 8)
> +#define DDCM_DATA0                     (0xff << 0)
> +#define DDC_DDCMD1             (0xc)
> +#define DDCM_DATA7                     (0xff << 24)
> +#define DDCM_DATA6                     (0xff << 16)
> +#define DDCM_DATA5                     (0xff << 8)
> +#define DDCM_DATA4                     (0xff << 0)
> +
> +struct mtk_hdmi_i2c {

Throughout this driver, I think we should:

s/mtk_hdmi_i2c/mtk_hdmi_ddc

> +       struct i2c_adapter adap;
> +       struct clk *clk;
> +       void __iomem *regs;
> +};
> +
> +static inline void sif_set_bit(struct mtk_hdmi_i2c *i2c, unsigned int offset,
> +                              unsigned int val)
> +{
> +       writel(readl(i2c->regs + offset) | val, i2c->regs + offset);
> +}
> +
> +static inline void sif_clr_bit(struct mtk_hdmi_i2c *i2c, unsigned int offset,
> +                              unsigned int val)
> +{
> +       writel(readl(i2c->regs + offset) & ~val, i2c->regs + offset);
> +}
> +
> +static inline bool sif_bit_is_set(struct mtk_hdmi_i2c *i2c, unsigned int offset,
> +                                 unsigned int val)
> +{
> +       return (readl(i2c->regs + offset) & val) == val;
> +}
> +
> +static inline void sif_write_mask(struct mtk_hdmi_i2c *i2c, unsigned int offset,
> +                                 unsigned int mask, unsigned int shift,
> +                                 unsigned int val)
> +{
> +       unsigned int tmp;
> +
> +       tmp = readl(i2c->regs + offset);
> +       tmp &= ~mask;
> +       tmp |= (val << shift) & mask;
> +       writel(tmp, i2c->regs + offset);
> +}
> +
> +static inline unsigned int sif_read_mask(struct mtk_hdmi_i2c *i2c,
> +                                        unsigned int offset, unsigned int mask,
> +                                        unsigned int shift)
> +{
> +       return (readl(i2c->regs + offset) & mask) >> shift;
> +}
> +
> +static void ddcm_trigger_mode(struct mtk_hdmi_i2c *i2c, int mode)
> +{
> +       int timeout = 20 * 1000;
> +
> +       sif_write_mask(i2c, DDC_DDCMCTL1, DDCM_SIF_MODE_MASK,
> +                      DDCM_SIF_MODE_OFFSET, mode);
> +       sif_set_bit(i2c, DDC_DDCMCTL1, DDCM_TRI);
> +       while (sif_bit_is_set(i2c, DDC_DDCMCTL1, DDCM_TRI)) {
> +               timeout -= 2;
> +               udelay(2);
> +               if (timeout <= 0)
> +                       break;
> +       }

Use iopoll?


> +}
> +
> +static int mtk_hdmi_i2c_read_msg(struct mtk_hdmi_i2c *i2c, struct i2c_msg *msg)
> +{
> +       struct device *dev = i2c->adap.dev.parent;
> +       u32 remain_count, ack_count, ack_final, read_count, temp_count;
> +       u32 index = 0;
> +       u32 ack;
> +       int i;
> +
> +       ddcm_trigger_mode(i2c, DDCM_START);
> +       sif_write_mask(i2c, DDC_DDCMD0, 0xff, 0, (msg->addr << 1) | 0x01);
> +       sif_write_mask(i2c, DDC_DDCMCTL1, DDCM_PGLEN_MASK, DDCM_PGLEN_OFFSET,
> +                      0x00);
> +       ddcm_trigger_mode(i2c, DDCM_WRITE_DATA);
> +       ack = sif_read_mask(i2c, DDC_DDCMCTL1, DDCM_ACK_MASK, DDCM_ACK_OFFSET);
> +       dev_dbg(dev, "ack = 0x%x\n", ack);
> +       if (ack != 0x01) {
> +               dev_err(dev, "i2c ack err!\n");
> +               return -ENXIO;
> +       }
> +
> +       remain_count = msg->len;
> +       ack_count = (msg->len - 1) / 8;
> +       ack_final = 0;
> +
> +       while (remain_count > 0) {
> +               if (ack_count > 0) {
> +                       read_count = 8;
> +                       ack_final = 0;
> +                       ack_count--;
> +               } else {
> +                       read_count = remain_count;
> +                       ack_final = 1;
> +               }
> +
> +               sif_write_mask(i2c, DDC_DDCMCTL1, DDCM_PGLEN_MASK,
> +                              DDCM_PGLEN_OFFSET, read_count - 1);
> +               ddcm_trigger_mode(i2c, (ack_final == 1) ?
> +                                 DDCM_READ_DATA_NO_ACK :
> +                                 DDCM_READ_DATA_ACK);
> +
> +               ack = sif_read_mask(i2c, DDC_DDCMCTL1, DDCM_ACK_MASK,
> +                                   DDCM_ACK_OFFSET);
> +               temp_count = 0;
> +               while (((ack & (1 << temp_count)) != 0) && (temp_count < 8))
> +                       temp_count++;
> +               if (((ack_final == 1) && (temp_count != (read_count - 1))) ||
> +                   ((ack_final == 0) && (temp_count != read_count))) {
> +                       dev_err(dev, "Address NACK! ACK(0x%x)\n", ack);
> +                       break;
> +               }
> +
> +               for (i = read_count; i >= 1; i--) {
> +                       int shift;
> +                       int offset;
> +
> +                       if (i > 4) {
> +                               offset = DDC_DDCMD1;
> +                               shift = (i - 5) * 8;
> +                       } else {
> +                               offset = DDC_DDCMD0;
> +                               shift = (i - 1) * 8;
> +                       }
> +
> +                       msg->buf[index + i - 1] = sif_read_mask(i2c, offset,
> +                                                               0xff << shift,
> +                                                               shift);
> +               }
> +
> +               remain_count -= read_count;
> +               index += read_count;
> +       }
> +
> +       return 0;
> +}
> +
> +static int mtk_hdmi_i2c_write_msg(struct mtk_hdmi_i2c *i2c, struct i2c_msg *msg)
> +{
> +       struct device *dev = i2c->adap.dev.parent;
> +       u32 ack;
> +
> +       ddcm_trigger_mode(i2c, DDCM_START);
> +       sif_write_mask(i2c, DDC_DDCMD0, DDCM_DATA0, 0, msg->addr << 1);
> +       sif_write_mask(i2c, DDC_DDCMD0, DDCM_DATA1, 8, msg->buf[0]);
> +       sif_write_mask(i2c, DDC_DDCMCTL1, DDCM_PGLEN_MASK, DDCM_PGLEN_OFFSET,
> +                      0x1);
> +       ddcm_trigger_mode(i2c, DDCM_WRITE_DATA);
> +
> +       ack = sif_read_mask(i2c, DDC_DDCMCTL1, DDCM_ACK_MASK, DDCM_ACK_OFFSET);
> +       dev_dbg(dev, "ack = %d\n", ack);
> +
> +       if (ack != 0x03) {
> +               dev_err(dev, "i2c ack err!\n");
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
> +static int mtk_hdmi_i2c_xfer(struct i2c_adapter *adapter,
> +                            struct i2c_msg *msgs, int num)
> +{
> +       struct mtk_hdmi_i2c *i2c = adapter->algo_data;
> +       struct device *dev = adapter->dev.parent;
> +       int ret;
> +       int i;
> +
> +       if (!i2c) {
> +               dev_err(dev, "invalid arguments\n");
> +               return -EINVAL;
> +       }
> +
> +       sif_set_bit(i2c, DDC_DDCMCTL0, DDCM_SCL_STRECH);
> +       sif_set_bit(i2c, DDC_DDCMCTL0, DDCM_SM0EN);
> +       sif_clr_bit(i2c, DDC_DDCMCTL0, DDCM_ODRAIN);
> +
> +       if (sif_bit_is_set(i2c, DDC_DDCMCTL1, DDCM_TRI)) {
> +               dev_err(dev, "ddc line is busy!\n");
> +               return -EBUSY;
> +       }
> +
> +       sif_write_mask(i2c, DDC_DDCMCTL0, DDCM_CLK_DIV_MASK,
> +                      DDCM_CLK_DIV_OFFSET, SIF1_CLOK);
> +
> +       for (i = 0; i < num; i++) {
> +               struct i2c_msg *msg = &msgs[i];
> +
> +               dev_dbg(dev, "i2c msg, adr:0x%x, flags:%d, len :0x%x\n",
> +                       msg->addr, msg->flags, msg->len);
> +
> +               if (msg->flags & I2C_M_RD)
> +                       ret = mtk_hdmi_i2c_read_msg(i2c, msg);
> +               else
> +                       ret = mtk_hdmi_i2c_write_msg(i2c, msg);
> +               if (ret < 0)
> +                       goto xfer_end;
> +       }
> +
> +       ddcm_trigger_mode(i2c, DDCM_STOP);
> +
> +       return i;
> +
> +xfer_end:
> +       ddcm_trigger_mode(i2c, DDCM_STOP);
> +       dev_err(dev, "ddc failed!\n");
> +       return ret;
> +}
> +
> +static u32 mtk_hdmi_i2c_func(struct i2c_adapter *adapter)
> +{
> +       return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static const struct i2c_algorithm mtk_hdmi_i2c_algorithm = {
> +       .master_xfer = mtk_hdmi_i2c_xfer,
> +       .functionality = mtk_hdmi_i2c_func,
> +};
> +
> +static int mtk_hdmi_i2c_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct mtk_hdmi_i2c *i2c;
> +       struct resource *mem;
> +       int ret;
> +
> +       i2c = devm_kzalloc(dev, sizeof(struct mtk_hdmi_i2c), GFP_KERNEL);
> +       if (!i2c)
> +               return -ENOMEM;
> +
> +       i2c->clk = devm_clk_get(dev, "ddc-i2c");
> +       if (IS_ERR(i2c->clk)) {
> +               dev_err(dev, "get ddc_clk failed : %p ,\n", i2c->clk);

nit, no space before ':'

> +               return PTR_ERR(i2c->clk);
> +       }
> +
> +       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       i2c->regs = devm_ioremap_resource(&pdev->dev, mem);
> +       if (IS_ERR(i2c->regs)) {
> +               dev_err(dev, "get memory source fail!\n");

nit: don't really need to print anything here.

> +               return PTR_ERR(i2c->regs);
> +       }
> +
> +       ret = clk_prepare_enable(i2c->clk);
> +       if (ret) {
> +               dev_err(dev, "enable ddc clk failed!\n");
> +               return ret;
> +       }
> +
> +       strlcpy(i2c->adap.name, "mediatek-hdmi-i2c", sizeof(i2c->adap.name));
> +       i2c->adap.owner = THIS_MODULE;

i2c->adap.class = I2C_CLASS_DDC;

> +       i2c->adap.algo = &mtk_hdmi_i2c_algorithm;
> +       i2c->adap.retries = 3;

why set this?

> +       i2c->adap.dev.of_node = dev->of_node;
> +       i2c->adap.algo_data = i2c;
> +       i2c->adap.dev.parent = &pdev->dev;
> +
> +       ret = i2c_add_adapter(&i2c->adap);
> +       if (ret < 0) {
> +               dev_err(dev, "failed to add bus to i2c core\n");
> +               goto err_clk_disable;
> +       }
> +
> +       platform_set_drvdata(pdev, i2c);
> +
> +       dev_dbg(dev, "i2c->adap: %p\n", &i2c->adap);
> +       dev_dbg(dev, "i2c->clk: %p\n", i2c->clk);
> +       dev_dbg(dev, "physical adr: 0x%llx, end: 0x%llx\n", mem->start,
> +               mem->end);

remove these debugging lines.

> +
> +       return 0;
> +
> +err_clk_disable:
> +       clk_disable_unprepare(i2c->clk);
> +       return ret;
> +}
> +
> +static int mtk_hdmi_i2c_remove(struct platform_device *pdev)
> +{
> +       struct mtk_hdmi_i2c *i2c = platform_get_drvdata(pdev);
> +
> +       clk_disable_unprepare(i2c->clk);
> +       i2c_del_adapter(&i2c->adap);

To match probe order, call i2c_del_adapter() first.

> +
> +       return 0;
> +}
> +
> +static const struct of_device_id mtk_hdmi_i2c_match[] = {
> +       { .compatible = "mediatek,mt8173-hdmi-ddc", },
> +       {},
> +};
> +
> +struct platform_driver mtk_hdmi_ddc_driver = {
> +       .probe = mtk_hdmi_i2c_probe,
> +       .remove = mtk_hdmi_i2c_remove,
> +       .driver = {
> +               .name = "mediatek-hdmi-ddc",
> +               .of_match_table = mtk_hdmi_i2c_match,
> +       },
> +};
> +
> +MODULE_AUTHOR("Jie Qiu <jie.qiu@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("MediaTek HDMI i2c Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_hw.c b/drivers/gpu/drm/mediatek/mtk_hdmi_hw.c
> new file mode 100644
> index 0000000..99c7ffc
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_hw.c

What is the value in having this as a separate mtk_hdmi_hw.c file?
If these functions were in mtk_hdmi.c, they could all be static, and
the compiler
could inline them away.


> @@ -0,0 +1,652 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include "mtk_hdmi_hw.h"
> +#include "mtk_hdmi_regs.h"
> +#include "mtk_hdmi.h"

I think these usually go after system includes.

> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/hdmi.h>
> +#include <linux/io.h>
> +#include <linux/regmap.h>
> +
> +static u32 mtk_hdmi_read(struct mtk_hdmi *hdmi, u32 offset)
> +{
> +       return readl(hdmi->regs + offset);
> +}
> +
> +static void mtk_hdmi_write(struct mtk_hdmi *hdmi, u32 offset, u32 val)
> +{
> +       writel(val, hdmi->regs + offset);
> +}
> +
> +static void mtk_hdmi_mask(struct mtk_hdmi *hdmi, u32 offset, u32 val, u32 mask)
> +{
> +       u32 tmp = mtk_hdmi_read(hdmi, offset) & ~mask;
> +
> +       tmp |= (val & mask);
> +       mtk_hdmi_write(hdmi, offset, tmp);
> +}
> +
> +#define NCTS_BYTES          0x07

move above the functions

> +
> +void mtk_hdmi_hw_vid_black(struct mtk_hdmi *hdmi,
> +                          bool black)
> +{
> +       mtk_hdmi_mask(hdmi, VIDEO_CFG_4, black ? GEN_RGB : NORMAL_PATH,
> +                     VIDEO_SOURCE_SEL);
> +}
> +
> +void mtk_hdmi_hw_make_reg_writable(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG20,
> +                          HDMI_PCLK_FREE_RUN, enable ? HDMI_PCLK_FREE_RUN : 0);
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG1C,
> +                          HDMI_ON | ANLG_ON, enable ? (HDMI_ON | ANLG_ON) : 0);
> +}
> +
> +void mtk_hdmi_hw_1p4_version_enable(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG20,
> +                          HDMI2P0_EN, enable ? 0 : HDMI2P0_EN);
> +}
> +
> +void mtk_hdmi_hw_aud_mute(struct mtk_hdmi *hdmi, bool mute)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_AUDIO_CFG, mute ? AUDIO_ZERO : 0, AUDIO_ZERO);
> +}
> +
> +void mtk_hdmi_hw_reset(struct mtk_hdmi *hdmi)
> +{
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG1C,
> +                          HDMI_RST, HDMI_RST);
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG1C,
> +                          HDMI_RST, 0);
> +       mtk_hdmi_mask(hdmi, GRL_CFG3, 0, CFG3_CONTROL_PACKET_DELAY);
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG1C,
> +                          ANLG_ON, ANLG_ON);
> +}
> +
> +void mtk_hdmi_hw_enable_notice(struct mtk_hdmi *hdmi, bool enable_notice)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_CFG2, enable_notice ? CFG2_NOTICE_EN : 0,
> +                     CFG2_NOTICE_EN);
> +}
> +
> +void mtk_hdmi_hw_write_int_mask(struct mtk_hdmi *hdmi, u32 int_mask)
> +{
> +       mtk_hdmi_write(hdmi, GRL_INT_MASK, int_mask);
> +}
> +
> +void mtk_hdmi_hw_enable_dvi_mode(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_CFG1, enable ? CFG1_DVI : 0, CFG1_DVI);
> +}
> +
> +void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer, u8 len)
> +{
> +       u32 ctrl_reg = GRL_CTRL;
> +       int i;
> +       u8 *frame_data;
> +       u8 frame_type;
> +       u8 frame_ver;
> +       u8 frame_len;
> +       u8 checksum;
> +       int ctrl_frame_en = 0;
> +
> +       frame_type = *buffer;
> +       buffer += 1;
> +       frame_ver = *buffer;
> +       buffer += 1;
> +       frame_len = *buffer;
> +       buffer += 1;
> +       checksum = *buffer;
> +       buffer += 1;
> +       frame_data = buffer;
> +
> +       dev_dbg(hdmi->dev,
> +               "frame_type:0x%x,frame_ver:0x%x,frame_len:0x%x,checksum:0x%x\n",
> +               frame_type, frame_ver, frame_len, checksum);
> +
> +       switch (frame_type) {
> +       case HDMI_INFOFRAME_TYPE_AVI:
> +               ctrl_frame_en = CTRL_AVI_EN;
> +               ctrl_reg = GRL_CTRL;
> +               break;
> +       case HDMI_INFOFRAME_TYPE_SPD:
> +               ctrl_frame_en = CTRL_SPD_EN;
> +               ctrl_reg = GRL_CTRL;
> +               break;
> +       case HDMI_INFOFRAME_TYPE_AUDIO:
> +               ctrl_frame_en = CTRL_AUDIO_EN;
> +               ctrl_reg = GRL_CTRL;
> +               break;
> +       case HDMI_INFOFRAME_TYPE_VENDOR:
> +               ctrl_frame_en = VS_EN;
> +               ctrl_reg = GRL_ACP_ISRC_CTRL;
> +               break;
> +       default:

Just fall through if none of the above?

> +               break;
> +       }
> +       mtk_hdmi_mask(hdmi, ctrl_reg, 0, ctrl_frame_en);
> +       mtk_hdmi_write(hdmi, GRL_INFOFRM_TYPE, frame_type);
> +       mtk_hdmi_write(hdmi, GRL_INFOFRM_VER, frame_ver);
> +       mtk_hdmi_write(hdmi, GRL_INFOFRM_LNG, frame_len);
> +
> +       mtk_hdmi_write(hdmi, GRL_IFM_PORT, checksum);
> +       for (i = 0; i < frame_len; i++)
> +               mtk_hdmi_write(hdmi, GRL_IFM_PORT, frame_data[i]);
> +
> +       mtk_hdmi_mask(hdmi, ctrl_reg, ctrl_frame_en, ctrl_frame_en);
> +}
> +
> +void mtk_hdmi_hw_send_aud_packet(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_SHIFT_R2, enable ? 0 : AUDIO_PACKET_OFF,
> +                     AUDIO_PACKET_OFF);
> +}
> +
> +void mtk_hdmi_hw_config_sys(struct mtk_hdmi *hdmi)
> +{
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG20,
> +                          HDMI_OUT_FIFO_EN | MHL_MODE_ON, 0);
> +       mdelay(2);

Can this be msleep instead of mdelay?
It is a bit rude to hog the CPU for 2 msec.

> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG20,
> +                          HDMI_OUT_FIFO_EN | MHL_MODE_ON, HDMI_OUT_FIFO_EN);
> +}
> +
> +void mtk_hdmi_hw_set_deep_color_mode(struct mtk_hdmi *hdmi)
> +{
> +       regmap_update_bits(hdmi->sys_regmap, hdmi->sys_offset + HDMI_SYS_CFG20,
> +                          DEEP_COLOR_MODE_MASK | DEEP_COLOR_EN, COLOR_8BIT_MODE);
> +}
> +
> +void mtk_hdmi_hw_send_av_mute(struct mtk_hdmi *hdmi)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_CFG4, 0, CTRL_AVMUTE);
> +       mdelay(2);

Can this be msleep instead of mdelay?

> +       mtk_hdmi_mask(hdmi, GRL_CFG4, CTRL_AVMUTE, CTRL_AVMUTE);
> +}
> +
> +void mtk_hdmi_hw_send_av_unmute(struct mtk_hdmi *hdmi)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_CFG4, CFG4_AV_UNMUTE_EN,
> +                     CFG4_AV_UNMUTE_EN | CFG4_AV_UNMUTE_SET);
> +       mdelay(2);

Can this be msleep instead of mdelay?

> +       mtk_hdmi_mask(hdmi, GRL_CFG4, CFG4_AV_UNMUTE_SET,
> +                     CFG4_AV_UNMUTE_EN | CFG4_AV_UNMUTE_SET);
> +}
> +
> +void mtk_hdmi_hw_ncts_enable(struct mtk_hdmi *hdmi, bool on)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_CTS_CTRL, on ? 0 : CTS_CTRL_SOFT,
> +                     CTS_CTRL_SOFT);
> +}
> +
> +void mtk_hdmi_hw_ncts_auto_write_enable(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_CTS_CTRL, enable ? NCTS_WRI_ANYTIME : 0,
> +                     NCTS_WRI_ANYTIME);
> +}
> +
> +void mtk_hdmi_hw_msic_setting(struct mtk_hdmi *hdmi,
> +                             struct drm_display_mode *mode)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_CFG4, 0, CFG_MHL_MODE);
> +
> +       if (mode->flags & DRM_MODE_FLAG_INTERLACE &&
> +           mode->clock == 74250 &&
> +           mode->vdisplay == 1080)
> +               mtk_hdmi_mask(hdmi, GRL_CFG2, 0, MHL_DE_SEL);
> +       else
> +               mtk_hdmi_mask(hdmi, GRL_CFG2, MHL_DE_SEL, MHL_DE_SEL);
> +}
> +
> +void mtk_hdmi_hw_aud_set_channel_swap(struct mtk_hdmi *hdmi,
> +                                     enum hdmi_aud_channel_swap_type swap)
> +{
> +       u8 swap_bit;
> +
> +       switch (swap) {
> +       case HDMI_AUD_SWAP_LR:
> +               swap_bit = LR_SWAP;
> +               break;
> +       case HDMI_AUD_SWAP_LFE_CC:
> +               swap_bit = LFE_CC_SWAP;
> +               break;
> +       case HDMI_AUD_SWAP_LSRS:
> +               swap_bit = LSRS_SWAP;
> +               break;
> +       case HDMI_AUD_SWAP_RLS_RRS:
> +               swap_bit = RLS_RRS_SWAP;
> +               break;
> +       case HDMI_AUD_SWAP_LR_STATUS:
> +               swap_bit = LR_STATUS_SWAP;
> +               break;
> +       default:
> +               swap_bit = LFE_CC_SWAP;
> +               break;
> +       }
> +       mtk_hdmi_mask(hdmi, GRL_CH_SWAP, swap_bit, 0xff);
> +}
> +
> +void mtk_hdmi_hw_aud_raw_data_enable(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_MIX_CTRL, enable ? MIX_CTRL_FLAT : 0,
> +                     MIX_CTRL_FLAT);
> +}
> +
> +void mtk_hdmi_hw_aud_set_bit_num(struct mtk_hdmi *hdmi,
> +                                enum hdmi_audio_sample_size bit_num)
> +{
> +       u32 val = 0;

no 0 init

> +
> +       if (bit_num == HDMI_AUDIO_SAMPLE_SIZE_16)
> +               val = AOUT_16BIT;
> +       else if (bit_num == HDMI_AUDIO_SAMPLE_SIZE_20)
> +               val = AOUT_20BIT;
> +       else if (bit_num == HDMI_AUDIO_SAMPLE_SIZE_24)
> +               val = AOUT_24BIT;
> +
> +       mtk_hdmi_mask(hdmi, GRL_AOUT_BNUM_SEL, val, 0x03);
> +}
> +
> +void mtk_hdmi_hw_aud_set_i2s_fmt(struct mtk_hdmi *hdmi,
> +                                enum hdmi_aud_i2s_fmt i2s_fmt)
> +{
> +       u32 val = 0;

no 0 init

> +
> +       val = mtk_hdmi_read(hdmi, GRL_CFG0);
> +       val &= ~0x33;

#define this mask

> +
> +       switch (i2s_fmt) {
> +       case HDMI_I2S_MODE_RJT_24BIT:
> +               val |= (CFG0_I2S_MODE_RTJ | CFG0_I2S_MODE_24BIT);
> +               break;
> +       case HDMI_I2S_MODE_RJT_16BIT:
> +               val |= (CFG0_I2S_MODE_RTJ | CFG0_I2S_MODE_16BIT);
> +               break;
> +       case HDMI_I2S_MODE_LJT_24BIT:
> +               val |= (CFG0_I2S_MODE_LTJ | CFG0_I2S_MODE_24BIT);
> +               break;
> +       case HDMI_I2S_MODE_LJT_16BIT:
> +               val |= (CFG0_I2S_MODE_LTJ | CFG0_I2S_MODE_16BIT);
> +               break;
> +       case HDMI_I2S_MODE_I2S_24BIT:
> +               val |= (CFG0_I2S_MODE_I2S | CFG0_I2S_MODE_24BIT);
> +               break;
> +       case HDMI_I2S_MODE_I2S_16BIT:
> +               val |= (CFG0_I2S_MODE_I2S | CFG0_I2S_MODE_16BIT);
> +               break;
> +       default:
> +               break;
> +       }
> +       mtk_hdmi_write(hdmi, GRL_CFG0, val);
> +}
> +
> +void mtk_hdmi_hw_aud_set_high_bitrate(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_AOUT_BNUM_SEL,
> +                     enable ? HIGH_BIT_RATE_PACKET_ALIGN : 0,
> +                     HIGH_BIT_RATE_PACKET_ALIGN);
> +       mtk_hdmi_mask(hdmi, GRL_AUDIO_CFG, enable ? HIGH_BIT_RATE : 0,
> +                     HIGH_BIT_RATE);
> +}
> +
> +void mtk_hdmi_phy_aud_dst_normal_double_enable(struct mtk_hdmi *hdmi,
> +                                              bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_AUDIO_CFG, enable ? DST_NORMAL_DOUBLE : 0,
> +                     DST_NORMAL_DOUBLE);
> +}
> +
> +void mtk_hdmi_hw_aud_dst_enable(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_AUDIO_CFG, enable ? SACD_DST : 0, SACD_DST);
> +}
> +
> +void mtk_hdmi_hw_aud_dsd_enable(struct mtk_hdmi *hdmi, bool enable)
> +{
> +       mtk_hdmi_mask(hdmi, GRL_AUDIO_CFG, enable ? SACD_SEL : 0, SACD_SEL);
> +}
> +
> +void mtk_hdmi_hw_aud_set_i2s_chan_num(struct mtk_hdmi *hdmi,
> +                                     enum hdmi_aud_channel_type channel_type,
> +                                     u8 channel_count)
> +{
> +       u8 val_1, val_2, val_3, val_4;

better:
  u8 sw[3], uv;

> +
> +       if (channel_count == 2) {
> +               val_1 = 0x04;
> +               val_2 = 0x50;

Some #defines with meaningful names would be nice here.


> +       } else if (channel_count == 3 || channel_count == 4) {
> +               if (channel_count == 4 &&
> +                   (channel_type == HDMI_AUD_CHAN_TYPE_3_0_LRS ||
> +                   channel_type == HDMI_AUD_CHAN_TYPE_4_0)) {
> +                       val_1 = 0x14;
> +               } else {
> +                       val_1 = 0x0c;
> +               }
> +               val_2 = 0x50;
> +       } else if (channel_count == 6 || channel_count == 5) {
> +               if (channel_count == 6 &&
> +                   channel_type != HDMI_AUD_CHAN_TYPE_5_1 &&
> +                   channel_type != HDMI_AUD_CHAN_TYPE_4_1_CLRS) {
> +                       val_1 = 0x3c;
> +                       val_2 = 0x50;
> +               } else {
> +                       val_1 = 0x1c;
> +                       val_2 = 0x50;
> +               }
> +       } else if (channel_count == 8 || channel_count == 7) {
> +               val_1 = 0x3c;
> +               val_2 = 0x50;
> +       } else {
> +               val_1 = 0x04;
> +               val_2 = 0x50;
> +       }
> +
> +       val_3 = 0xc6;
> +       val_4 = 0xfa;
> +
> +       mtk_hdmi_write(hdmi, GRL_CH_SW0, val_2);
> +       mtk_hdmi_write(hdmi, GRL_CH_SW1, val_3);
> +       mtk_hdmi_write(hdmi, GRL_CH_SW2, val_4);
> +       mtk_hdmi_write(hdmi, GRL_I2S_UV, val_1);
> +}
> +
> +void mtk_hdmi_hw_aud_set_input_type(struct mtk_hdmi *hdmi,
> +                                   enum hdmi_aud_input_type input_type)
> +{
> +       u32 val = 0;

no need to 0 init

> +
> +       val = mtk_hdmi_read(hdmi, GRL_CFG1);
> +       if (input_type == HDMI_AUD_INPUT_I2S &&
> +           (val & CFG1_SPDIF) == CFG1_SPDIF) {
> +               val &= ~CFG1_SPDIF;
> +       } else if (input_type == HDMI_AUD_INPUT_SPDIF &&
> +               (val & CFG1_SPDIF) == 0) {
> +               val |= CFG1_SPDIF;
> +       }
> +       mtk_hdmi_write(hdmi, GRL_CFG1, val);
> +}
> +
> +void mtk_hdmi_hw_aud_set_channel_status(struct mtk_hdmi *hdmi,
> +                                       u8 *l_chan_status, u8 *r_chan_status,
> +                                       enum hdmi_audio_sample_frequency
> +                                       aud_hdmi_fs)
> +{
> +       u8 l_status[5];
> +       u8 r_status[5];
> +       u8 val = 0;

no need to 0 init

> +
> +       l_status[0] = l_chan_status[0];
> +       l_status[1] = l_chan_status[1];
> +       l_status[2] = l_chan_status[2];
> +       r_status[0] = r_chan_status[0];
> +       r_status[1] = r_chan_status[1];
> +       r_status[2] = r_chan_status[2];
> +
> +       l_status[0] &= ~0x02;
> +       r_status[0] &= ~0x02;
> +
> +       val = l_chan_status[3] & 0xf0;
> +       switch (aud_hdmi_fs) {
> +       case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
> +               val |= 0x03;
> +               break;
> +       case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
> +               break;
> +       case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
> +               val |= 0x08;
> +               break;
> +       case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
> +               val |= 0x0a;
> +               break;
> +       case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
> +               val |= 0x02;
> +               break;
> +       default:
> +               val |= 0x02;
> +               break;
> +       }
> +       l_status[3] = val;
> +       r_status[3] = val;
> +
> +       val = l_chan_status[4];
> +       val |= ((~(l_status[3] & 0x0f)) << 4);
> +       l_status[4] = val;
> +       r_status[4] = val;
> +
> +       val = l_status[0];

nit: You don't need to bounce through val here.
You can just write the *_status[n] value directly.

> +       mtk_hdmi_write(hdmi, GRL_I2S_C_STA0, val);
> +       mtk_hdmi_write(hdmi, GRL_L_STATUS_0, val);
> +
> +       val = r_status[0];
> +       mtk_hdmi_write(hdmi, GRL_R_STATUS_0, val);
> +
> +       val = l_status[1];
> +       mtk_hdmi_write(hdmi, GRL_I2S_C_STA1, val);
> +       mtk_hdmi_write(hdmi, GRL_L_STATUS_1, val);
> +
> +       val = r_status[1];
> +       mtk_hdmi_write(hdmi, GRL_R_STATUS_1, val);
> +
> +       val = l_status[2];
> +       mtk_hdmi_write(hdmi, GRL_I2S_C_STA2, val);
> +       mtk_hdmi_write(hdmi, GRL_L_STATUS_2, val);
> +
> +       val = r_status[2];
> +       mtk_hdmi_write(hdmi, GRL_R_STATUS_2, val);
> +
> +       val = l_status[3];
> +       mtk_hdmi_write(hdmi, GRL_I2S_C_STA3, val);
> +       mtk_hdmi_write(hdmi, GRL_L_STATUS_3, val);
> +
> +       val = r_status[3];
> +       mtk_hdmi_write(hdmi, GRL_R_STATUS_3, val);
> +
> +       val = l_status[4];
> +       mtk_hdmi_write(hdmi, GRL_I2S_C_STA4, val);
> +       mtk_hdmi_write(hdmi, GRL_L_STATUS_4, val);
> +
> +       val = r_status[4];
> +       mtk_hdmi_write(hdmi, GRL_R_STATUS_4, val);
> +
> +       for (val = 0; val < 19; val++) {
> +               mtk_hdmi_write(hdmi, GRL_L_STATUS_5 + val * 4, 0);
> +               mtk_hdmi_write(hdmi, GRL_R_STATUS_5 + val * 4, 0);
> +       }
> +}
> +
> +void mtk_hdmi_hw_aud_src_reenable(struct mtk_hdmi *hdmi)
> +{
> +       u32 val;
> +
> +       val = mtk_hdmi_read(hdmi, GRL_MIX_CTRL);
> +       if (val & MIX_CTRL_SRC_EN) {
> +               val &= ~MIX_CTRL_SRC_EN;
> +               mtk_hdmi_write(hdmi, GRL_MIX_CTRL, val);
> +               usleep_range(255, 512);

Those are some very precise values for a range...

> +               val |= MIX_CTRL_SRC_EN;
> +               mtk_hdmi_write(hdmi, GRL_MIX_CTRL, val);
> +       }
> +}
> +
> +void mtk_hdmi_hw_aud_src_off(struct mtk_hdmi *hdmi)

Perhaps *_disable() would be more consistent.

> +{
> +       u32 val;
> +
> +       val = mtk_hdmi_read(hdmi, GRL_MIX_CTRL);
> +       val &= ~MIX_CTRL_SRC_EN;
> +       mtk_hdmi_write(hdmi, GRL_MIX_CTRL, val);
> +       mtk_hdmi_write(hdmi, GRL_SHIFT_L1, 0x00);
> +}
> +
> +void mtk_hdmi_hw_aud_set_mclk(struct mtk_hdmi *hdmi, enum hdmi_aud_mclk mclk)
> +{
> +       u32 val;
> +
> +       val = mtk_hdmi_read(hdmi, GRL_CFG5);
> +       val &= CFG5_CD_RATIO_MASK;
> +
> +       switch (mclk) {
> +       case HDMI_AUD_MCLK_128FS:
> +               val |= CFG5_FS128;
> +               break;
> +       case HDMI_AUD_MCLK_256FS:
> +               val |= CFG5_FS256;
> +               break;
> +       case HDMI_AUD_MCLK_384FS:
> +               val |= CFG5_FS384;
> +               break;
> +       case HDMI_AUD_MCLK_512FS:
> +               val |= CFG5_FS512;
> +               break;
> +       case HDMI_AUD_MCLK_768FS:
> +               val |= CFG5_FS768;
> +               break;
> +       default:
> +               val |= CFG5_FS256;
> +               break;
> +       }
> +       mtk_hdmi_write(hdmi, GRL_CFG5, val);
> +}
> +
> +void mtk_hdmi_hw_aud_aclk_inv_enable(struct mtk_hdmi *hdmi, bool enable)

nit: I prefer explicit _enable() and _disable() functions w/out the
'enable' parameter.

> +{
> +       u32 val;
> +
> +       val = mtk_hdmi_read(hdmi, GRL_CFG2);
> +       if (enable)
> +               val |= 0x80;
> +       else
> +               val &= ~0x80;
> +       mtk_hdmi_write(hdmi, GRL_CFG2, val);
> +}
> +
> +struct hdmi_acr_n {
> +       unsigned int clock;
> +       unsigned int n[3];
> +};
> +
> +/* Recommended N values from HDMI specification, tables 7-1 to 7-3 */
> +static const struct hdmi_acr_n hdmi_rec_n_table[] = {
> +       /* Clock, N: 32kHz 44.1kHz 48kHz */
> +       {  25175, {  4576,  7007,  6864 } },
> +       {  74176, { 11648, 17836, 11648 } },
> +       { 148352, { 11648,  8918,  5824 } },
> +       { 296703, {  5824,  4459,  5824 } },
> +       { 297000, {  3072,  4704,  5120 } },
> +       {      0, {  4096,  6272,  6144 } }, /* all other TMDS clocks */
> +};
> +
> +/**
> + * hdmi_recommended_n() - Return N value recommended by HDMI specification
> + * @freq: audio sample rate in Hz
> + * @clock: rounded TMDS clock in kHz
> + */
> +static unsigned int hdmi_recommended_n(unsigned int freq, unsigned int clock)
> +{
> +       const struct hdmi_acr_n *recommended;
> +       unsigned int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(hdmi_rec_n_table) - 1; i++) {
> +               if (clock == hdmi_rec_n_table[i].clock)
> +                       break;
> +       }
> +       recommended = hdmi_rec_n_table + i;
> +
> +       switch (freq) {
> +       case 32000:
> +               return recommended->n[0];
> +       case 44100:
> +               return recommended->n[1];
> +       case 48000:
> +               return recommended->n[2];
> +       case 88200:
> +               return recommended->n[1] * 2;
> +       case 96000:
> +               return recommended->n[2] * 2;
> +       case 176400:
> +               return recommended->n[1] * 4;
> +       case 192000:
> +               return recommended->n[2] * 4;
> +       default:
> +               return (128 * freq) / 1000;
> +       }
> +}
> +
> +static unsigned int hdmi_mode_clock_to_hz(unsigned int clock)
> +{
> +       switch (clock) {
> +       case 25175:
> +               return 25174825;        /* 25.2/1.001 MHz */
> +       case 74176:
> +               return 74175824;        /* 74.25/1.001 MHz */
> +       case 148352:
> +               return 148351648;       /* 148.5/1.001 MHz */
> +       case 296703:
> +               return 296703297;       /* 297/1.001 MHz */
> +       default:
> +               return clock * 1000;
> +       }
> +}
> +
> +static unsigned int hdmi_expected_cts(unsigned int audio_sample_rate,
> +                                     unsigned int tmds_clock, unsigned int n)
> +{
> +       return DIV_ROUND_CLOSEST_ULL((u64)hdmi_mode_clock_to_hz(tmds_clock) * n,
> +                                    128 * audio_sample_rate);
> +}

Doug Anderson may have some opinions about how N & CTS are computed.


> +
> +static void do_hdmi_hw_aud_set_ncts(struct mtk_hdmi *hdmi, unsigned int n,
> +                                   unsigned int cts)
> +{
> +       unsigned char val[NCTS_BYTES];
> +       int i;
> +
> +       mtk_hdmi_write(hdmi, GRL_NCTS, 0);
> +       mtk_hdmi_write(hdmi, GRL_NCTS, 0);
> +       mtk_hdmi_write(hdmi, GRL_NCTS, 0);
> +       memset(val, 0, sizeof(val));

not necessary, since you fill in all 7 bytes anyway.

> +
> +       val[0] = (cts >> 24) & 0xff;
> +       val[1] = (cts >> 16) & 0xff;
> +       val[2] = (cts >> 8) & 0xff;
> +       val[3] = cts & 0xff;
> +
> +       val[4] = (n >> 16) & 0xff;
> +       val[5] = (n >> 8) & 0xff;
> +       val[6] = n & 0xff;

all of these "& 0xff" are not needed, since val is an unsigned char array.

> +
> +       for (i = 0; i < NCTS_BYTES; i++)
> +               mtk_hdmi_write(hdmi, GRL_NCTS, val[i]);

What an interesting design.  We write all 10 bytes to the same register address?
In this case, why bother with val at all?
Just directly call mtk_hdmi_write() for each of the bytes above.


> +}
> +
> +void mtk_hdmi_hw_aud_set_ncts(struct mtk_hdmi *hdmi, unsigned int sample_rate,
> +                             unsigned int clock)
> +{
> +       unsigned int n, cts;
> +
> +       n = hdmi_recommended_n(sample_rate, clock);
> +       cts = hdmi_expected_cts(sample_rate, clock, n);
> +
> +       dev_dbg(hdmi->dev, "%s: sample_rate=%u, clock=%d, cts=%u, n=%u\n",
> +               __func__, sample_rate, clock, n, cts);
> +
> +       mtk_hdmi_mask(hdmi, DUMMY_304, AUDIO_I2S_NCTS_SEL_64,
> +                     AUDIO_I2S_NCTS_SEL);
> +       do_hdmi_hw_aud_set_ncts(hdmi, n, cts);
> +}
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_hw.h b/drivers/gpu/drm/mediatek/mtk_hdmi_hw.h
> new file mode 100644
> index 0000000..9013219
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_hw.h
> @@ -0,0 +1,73 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef _MTK_HDMI_HW_H
> +#define _MTK_HDMI_HW_H
> +
> +#include <linux/types.h>
> +#include <linux/hdmi.h>
> +#include "mtk_hdmi.h"
> +
> +void mtk_hdmi_hw_vid_black(struct mtk_hdmi *hdmi, bool black);
> +void mtk_hdmi_hw_aud_mute(struct mtk_hdmi *hdmi, bool mute);
> +void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer, u8 len);
> +void mtk_hdmi_hw_send_aud_packet(struct mtk_hdmi *hdmi, bool enable);
> +int mtk_hdmi_hw_set_clock(struct mtk_hdmi *hctx, u32 clock);
> +void mtk_hdmi_hw_config_sys(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_set_deep_color_mode(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_send_av_mute(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_send_av_unmute(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_ncts_enable(struct mtk_hdmi *hdmi, bool on);
> +void mtk_hdmi_hw_aud_set_channel_swap(struct mtk_hdmi *hdmi,
> +                                     enum hdmi_aud_channel_swap_type swap);
> +void mtk_hdmi_hw_aud_raw_data_enable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_aud_set_bit_num(struct mtk_hdmi *hdmi,
> +                                enum hdmi_audio_sample_size bit_num);
> +void mtk_hdmi_hw_aud_set_high_bitrate(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_phy_aud_dst_normal_double_enable(struct mtk_hdmi *hdmi,
> +                                              bool enable);
> +void mtk_hdmi_hw_aud_dst_enable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_aud_dsd_enable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_aud_set_i2s_fmt(struct mtk_hdmi *hdmi,
> +                                enum hdmi_aud_i2s_fmt i2s_fmt);
> +void mtk_hdmi_hw_aud_set_i2s_chan_num(struct mtk_hdmi *hdmi,
> +                                     enum hdmi_aud_channel_type channel_type,
> +                                     u8 channel_count);
> +void mtk_hdmi_hw_aud_set_input_type(struct mtk_hdmi *hdmi,
> +                                   enum hdmi_aud_input_type input_type);
> +void mtk_hdmi_hw_aud_set_channel_status(struct mtk_hdmi *hdmi,
> +                                       u8 *l_chan_status, u8 *r_chan_staus,
> +                                       enum hdmi_audio_sample_frequency
> +                                       aud_hdmi_fs);
> +void mtk_hdmi_hw_aud_src_enable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_aud_set_mclk(struct mtk_hdmi *hdmi, enum hdmi_aud_mclk mclk);
> +void mtk_hdmi_hw_aud_src_off(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_aud_src_reenable(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_aud_aclk_inv_enable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_aud_set_ncts(struct mtk_hdmi *hdmi, unsigned int sample_rate,
> +                             unsigned int clock);
> +bool mtk_hdmi_hw_is_hpd_high(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_make_reg_writable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_reset(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_enable_notice(struct mtk_hdmi *hdmi, bool enable_notice);
> +void mtk_hdmi_hw_write_int_mask(struct mtk_hdmi *hdmi, u32 int_mask);
> +void mtk_hdmi_hw_enable_dvi_mode(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_ncts_auto_write_enable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_msic_setting(struct mtk_hdmi *hdmi,
> +                             struct drm_display_mode *mode);
> +void mtk_hdmi_hw_1p4_version_enable(struct mtk_hdmi *hdmi, bool enable);
> +void mtk_hdmi_hw_htplg_irq_enable(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_htplg_irq_disable(struct mtk_hdmi *hdmi);
> +void mtk_hdmi_hw_clear_htplg_irq(struct mtk_hdmi *hdmi);
> +
> +#endif /* _MTK_HDMI_HW_H */
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_regs.h b/drivers/gpu/drm/mediatek/mtk_hdmi_regs.h
> new file mode 100644
> index 0000000..8c1d318
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_regs.h
> @@ -0,0 +1,221 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef _MTK_HDMI_REGS_H
> +#define _MTK_HDMI_REGS_H
> +
> +#define GRL_INT_MASK           0x18
> +#define GRL_IFM_PORT           0x188
> +#define GRL_CH_SWAP            0x198
> +#define LR_SWAP                                BIT(0)
> +#define LFE_CC_SWAP                    BIT(1)
> +#define LSRS_SWAP                      BIT(2)
> +#define RLS_RRS_SWAP                   BIT(3)
> +#define LR_STATUS_SWAP                 BIT(4)
> +#define GRL_I2S_C_STA0         0x140
> +#define GRL_I2S_C_STA1         0x144
> +#define GRL_I2S_C_STA2         0x148
> +#define GRL_I2S_C_STA3         0x14C
> +#define GRL_I2S_C_STA4         0x150
> +#define GRL_I2S_UV             0x154
> +#define GRL_ACP_ISRC_CTRL      0x158
> +#define VS_EN                          BIT(0)
> +#define ACP_EN                         BIT(1)
> +#define ISRC1_EN                       BIT(2)
> +#define ISRC2_EN                       BIT(3)
> +#define GAMUT_EN                       BIT(4)
> +#define GRL_CTS_CTRL           0x160
> +#define CTS_CTRL_SOFT                  BIT(0)
> +#define GRL_INT                        0x14
> +#define INT_MDI                                BIT(0)
> +#define INT_HDCP                       BIT(1)
> +#define INT_FIFO_O                     BIT(2)
> +#define INT_FIFO_U                     BIT(3)
> +#define INT_IFM_ERR                    BIT(4)
> +#define INT_INF_DONE                   BIT(5)
> +#define INT_NCTS_DONE                  BIT(6)
> +#define INT_CTRL_PKT_DONE              BIT(7)
> +#define GRL_INT_MASK           0x18
> +#define GRL_CTRL               0x1C
> +#define CTRL_GEN_EN                    BIT(2)
> +#define CTRL_SPD_EN                    BIT(3)
> +#define CTRL_MPEG_EN                   BIT(4)
> +#define CTRL_AUDIO_EN                  BIT(5)
> +#define CTRL_AVI_EN                    BIT(6)
> +#define CTRL_AVMUTE                    BIT(7)
> +#define        GRL_STATUS              0x20
> +#define STATUS_HTPLG                   BIT(0)
> +#define STATUS_PORD                    BIT(1)
> +#define GRL_DIVN               0x170
> +#define NCTS_WRI_ANYTIME               BIT(6)
> +#define GRL_AUDIO_CFG          0x17C
> +#define AUDIO_ZERO                     BIT(0)
> +#define HIGH_BIT_RATE                  BIT(1)
> +#define SACD_DST                       BIT(2)
> +#define DST_NORMAL_DOUBLE              BIT(3)
> +#define DSD_INV                                BIT(4)
> +#define LR_INV                         BIT(5)
> +#define LR_MIX                         BIT(6)
> +#define SACD_SEL                       BIT(7)
> +#define GRL_NCTS               0x184
> +#define GRL_CH_SW0             0x18C
> +#define GRL_CH_SW1             0x190
> +#define GRL_CH_SW2             0x194
> +#define GRL_INFOFRM_VER                0x19C
> +#define GRL_INFOFRM_TYPE       0x1A0
> +#define GRL_INFOFRM_LNG                0x1A4
> +#define GRL_MIX_CTRL           0x1B4
> +#define MIX_CTRL_SRC_EN                        BIT(0)
> +#define BYPASS_VOLUME                  BIT(1)
> +#define MIX_CTRL_FLAT                  BIT(7)
> +#define GRL_AOUT_BNUM_SEL      0x1C4
> +#define AOUT_24BIT                     0x00
> +#define AOUT_20BIT                     0x02
> +#define AOUT_16BIT                     0x03
> +#define HIGH_BIT_RATE_PACKET_ALIGN     (0x3 << 6)
> +#define GRL_SHIFT_L1           0x1C0
> +#define GRL_SHIFT_R2           0x1B0
> +#define AUDIO_PACKET_OFF               BIT(6)
> +#define GRL_CFG0               0x24
> +#define CFG0_I2S_MODE_RTJ              0x1
> +#define CFG0_I2S_MODE_LTJ              0x0
> +#define CFG0_I2S_MODE_I2S              0x2
> +#define CFG0_I2S_MODE_24BIT            0x00
> +#define CFG0_I2S_MODE_16BIT            0x10
> +#define GRL_CFG1               0x28
> +#define CFG1_EDG_SEL                   BIT(0)
> +#define CFG1_SPDIF                     BIT(1)
> +#define CFG1_DVI                       BIT(2)
> +#define CFG1_HDCP_DEBUG                        BIT(3)
> +#define GRL_CFG2               0x2c
> +#define CFG2_NOTICE_EN                 BIT(6)
> +#define MHL_DE_SEL                     BIT(3)
> +#define GRL_CFG3               0x30
> +#define CFG3_AES_KEY_INDEX_MASK                0x3f
> +#define CFG3_CONTROL_PACKET_DELAY      BIT(6)
> +#define CFG3_KSV_LOAD_START            BIT(7)
> +#define GRL_CFG4               0x34
> +#define CFG4_AES_KEY_LOAD              BIT(4)
> +#define CFG4_AV_UNMUTE_EN              BIT(5)
> +#define CFG4_AV_UNMUTE_SET             BIT(6)
> +#define CFG_MHL_MODE                   BIT(7)
> +#define GRL_CFG5               0x38
> +#define CFG5_CD_RATIO_MASK     0x8F
> +#define CFG5_FS128                     (0x1 << 4)
> +#define CFG5_FS256                     (0x2 << 4)
> +#define CFG5_FS384                     (0x3 << 4)
> +#define CFG5_FS512                     (0x4 << 4)
> +#define CFG5_FS768                     (0x6 << 4)
> +#define DUMMY_304              0x304
> +#define CHMO_SEL                       (0x3 << 2)
> +#define CHM1_SEL                       (0x3 << 4)
> +#define CHM2_SEL                       (0x3 << 6)
> +#define AUDIO_I2S_NCTS_SEL             BIT(1)
> +#define AUDIO_I2S_NCTS_SEL_64          (1 << 1)
> +#define AUDIO_I2S_NCTS_SEL_128         (0 << 1)
> +#define NEW_GCP_CTRL                   BIT(0)
> +#define NEW_GCP_CTRL_MERGE             BIT(0)
> +#define GRL_L_STATUS_0         0x200
> +#define GRL_L_STATUS_1         0x204
> +#define GRL_L_STATUS_2         0x208
> +#define GRL_L_STATUS_3         0x20c
> +#define GRL_L_STATUS_4         0x210
> +#define GRL_L_STATUS_5         0x214
> +#define GRL_L_STATUS_6         0x218
> +#define GRL_L_STATUS_7         0x21c
> +#define GRL_L_STATUS_8         0x220
> +#define GRL_L_STATUS_9         0x224
> +#define GRL_L_STATUS_10                0x228
> +#define GRL_L_STATUS_11                0x22c
> +#define GRL_L_STATUS_12                0x230
> +#define GRL_L_STATUS_13                0x234
> +#define GRL_L_STATUS_14                0x238
> +#define GRL_L_STATUS_15                0x23c
> +#define GRL_L_STATUS_16                0x240
> +#define GRL_L_STATUS_17                0x244
> +#define GRL_L_STATUS_18                0x248
> +#define GRL_L_STATUS_19                0x24c
> +#define GRL_L_STATUS_20                0x250
> +#define GRL_L_STATUS_21                0x254
> +#define GRL_L_STATUS_22                0x258
> +#define GRL_L_STATUS_23                0x25c
> +#define GRL_R_STATUS_0         0x260
> +#define GRL_R_STATUS_1         0x264
> +#define GRL_R_STATUS_2         0x268
> +#define GRL_R_STATUS_3         0x26c
> +#define GRL_R_STATUS_4         0x270
> +#define GRL_R_STATUS_5         0x274
> +#define GRL_R_STATUS_6         0x278
> +#define GRL_R_STATUS_7         0x27c
> +#define GRL_R_STATUS_8         0x280
> +#define GRL_R_STATUS_9         0x284
> +#define GRL_R_STATUS_10                0x288
> +#define GRL_R_STATUS_11                0x28c
> +#define GRL_R_STATUS_12                0x290
> +#define GRL_R_STATUS_13                0x294
> +#define GRL_R_STATUS_14                0x298
> +#define GRL_R_STATUS_15                0x29c
> +#define GRL_R_STATUS_16                0x2a0
> +#define GRL_R_STATUS_17                0x2a4
> +#define GRL_R_STATUS_18                0x2a8
> +#define GRL_R_STATUS_19                0x2ac
> +#define GRL_R_STATUS_20                0x2b0
> +#define GRL_R_STATUS_21                0x2b4
> +#define GRL_R_STATUS_22                0x2b8
> +#define GRL_R_STATUS_23                0x2bc
> +#define GRL_ABIST_CTRL0                0x2D4
> +#define GRL_ABIST_CTRL1                0x2D8
> +#define ABIST_EN                       BIT(7)
> +#define ABIST_DATA_FMT                 (0x7 << 0)
> +#define VIDEO_CFG_0            0x380
> +#define VIDEO_CFG_1            0x384
> +#define VIDEO_CFG_2            0x388
> +#define VIDEO_CFG_3            0x38c
> +#define VIDEO_CFG_4            0x390
> +#define VIDEO_SOURCE_SEL               BIT(7)
> +#define NORMAL_PATH                    (1 << 7)
> +#define GEN_RGB                                (0 << 7)
> +
> +#define HDMI_SYS_CFG1C         0x000
> +#define HDMI_ON                                BIT(0)
> +#define HDMI_RST                       BIT(1)
> +#define ANLG_ON                                BIT(2)
> +#define CFG10_DVI                      BIT(3)
> +#define HDMI_TST                       BIT(3)
> +#define SYS_KEYMASK1                   (0xff << 8)
> +#define SYS_KEYMASK2                   (0xff << 16)
> +#define AUD_OUTSYNC_EN                 BIT(24)
> +#define AUD_OUTSYNC_PRE_EN             BIT(25)
> +#define I2CM_ON                                BIT(26)
> +#define E2PROM_TYPE_8BIT               BIT(27)
> +#define MCM_E2PROM_ON                  BIT(28)
> +#define EXT_E2PROM_ON                  BIT(29)
> +#define HTPLG_PIN_SEL_OFF              BIT(30)
> +#define AES_EFUSE_ENABLE               BIT(31)
> +#define HDMI_SYS_CFG20         0x004
> +#define DEEP_COLOR_MODE_MASK           (3 << 1)
> +#define COLOR_8BIT_MODE                        (0 << 1)
> +#define COLOR_10BIT_MODE               (1 << 1)
> +#define COLOR_12BIT_MODE               (2 << 1)
> +#define COLOR_16BIT_MODE               (3 << 1)
> +#define DEEP_COLOR_EN                  BIT(0)
> +#define HDMI_AUDIO_TEST_SEL            BIT(8)
> +#define HDMI2P0_EN                     BIT(11)
> +#define HDMI_OUT_FIFO_EN               BIT(16)
> +#define HDMI_OUT_FIFO_CLK_INV          BIT(17)
> +#define MHL_MODE_ON                    BIT(28)
> +#define MHL_PP_MODE                    BIT(29)
> +#define MHL_SYNC_AUTO_EN               BIT(30)
> +#define HDMI_PCLK_FREE_RUN             BIT(31)
> +
> +#endif
> diff --git a/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c b/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c
> new file mode 100644
> index 0000000..5d9f07f
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_mt8173_hdmi_phy.c
> @@ -0,0 +1,505 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Jie Qiu <jie.qiu@xxxxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#define HDMI_CON0              0x00
> +#define RG_HDMITX_PLL_EN               BIT(31)
> +#define RG_HDMITX_PLL_FBKDIV           (0x7f << 24)
> +#define PLL_FBKDIV_SHIFT               24
> +#define RG_HDMITX_PLL_FBKSEL           (0x3 << 22)
> +#define PLL_FBKSEL_SHIFT               22
> +#define RG_HDMITX_PLL_PREDIV           (0x3 << 20)
> +#define PREDIV_SHIFT                   20
> +#define RG_HDMITX_PLL_POSDIV           (0x3 << 18)
> +#define POSDIV_SHIFT                   18
> +#define RG_HDMITX_PLL_RST_DLY          (0x3 << 16)
> +#define RG_HDMITX_PLL_IR               (0xf << 12)
> +#define PLL_IR_SHIFT                   12
> +#define RG_HDMITX_PLL_IC               (0xf << 8)
> +#define PLL_IC_SHIFT                   8
> +#define RG_HDMITX_PLL_BP               (0xf << 4)
> +#define PLL_BP_SHIFT                   4
> +#define RG_HDMITX_PLL_BR               (0x3 << 2)
> +#define PLL_BR_SHIFT                   2
> +#define RG_HDMITX_PLL_BC               (0x3 << 0)
> +#define PLL_BC_SHIFT                   0
> +#define HDMI_CON1              0x04
> +#define RG_HDMITX_PLL_DIVEN            (0x7 << 29)
> +#define PLL_DIVEN_SHIFT                        29
> +#define RG_HDMITX_PLL_AUTOK_EN         BIT(28)
> +#define RG_HDMITX_PLL_AUTOK_KF         (0x3 << 26)
> +#define RG_HDMITX_PLL_AUTOK_KS         (0x3 << 24)
> +#define RG_HDMITX_PLL_AUTOK_LOAD       BIT(23)
> +#define RG_HDMITX_PLL_BAND             (0x3f << 16)
> +#define RG_HDMITX_PLL_REF_SEL          BIT(15)
> +#define RG_HDMITX_PLL_BIAS_EN          BIT(14)
> +#define RG_HDMITX_PLL_BIAS_LPF_EN      BIT(13)
> +#define RG_HDMITX_PLL_TXDIV_EN         BIT(12)
> +#define RG_HDMITX_PLL_TXDIV            (0x3 << 10)
> +#define PLL_TXDIV_SHIFT                        10
> +#define RG_HDMITX_PLL_LVROD_EN         BIT(9)
> +#define RG_HDMITX_PLL_MONVC_EN         BIT(8)
> +#define RG_HDMITX_PLL_MONCK_EN         BIT(7)
> +#define RG_HDMITX_PLL_MONREF_EN                BIT(6)
> +#define RG_HDMITX_PLL_TST_EN           BIT(5)
> +#define RG_HDMITX_PLL_TST_CK_EN                BIT(4)
> +#define RG_HDMITX_PLL_TST_SEL          (0xf << 0)
> +#define HDMI_CON2              0x08
> +#define RGS_HDMITX_PLL_AUTOK_BAND      (0x7f << 8)
> +#define RGS_HDMITX_PLL_AUTOK_FAIL      BIT(1)
> +#define RG_HDMITX_EN_TX_CKLDO          BIT(0)
> +#define HDMI_CON3              0x0c
> +#define RG_HDMITX_SER_EN               (0xf << 28)
> +#define RG_HDMITX_PRD_EN               (0xf << 24)
> +#define RG_HDMITX_PRD_IMP_EN           (0xf << 20)
> +#define RG_HDMITX_DRV_EN               (0xf << 16)
> +#define RG_HDMITX_DRV_IMP_EN           (0xf << 12)
> +#define DRV_IMP_EN_SHIFT               12
> +#define RG_HDMITX_MHLCK_FORCE          BIT(10)
> +#define RG_HDMITX_MHLCK_PPIX_EN                BIT(9)
> +#define RG_HDMITX_MHLCK_EN             BIT(8)
> +#define RG_HDMITX_SER_DIN_SEL          (0xf << 4)
> +#define RG_HDMITX_SER_5T1_BIST_EN      BIT(3)
> +#define RG_HDMITX_SER_BIST_TOG         BIT(2)
> +#define RG_HDMITX_SER_DIN_TOG          BIT(1)
> +#define RG_HDMITX_SER_CLKDIG_INV       BIT(0)
> +#define HDMI_CON4              0x10
> +#define RG_HDMITX_PRD_IBIAS_CLK                (0xf << 24)
> +#define RG_HDMITX_PRD_IBIAS_D2         (0xf << 16)
> +#define RG_HDMITX_PRD_IBIAS_D1         (0xf << 8)
> +#define RG_HDMITX_PRD_IBIAS_D0         (0xf << 0)
> +#define PRD_IBIAS_CLK_SHIFT            24
> +#define PRD_IBIAS_D2_SHIFT             16
> +#define PRD_IBIAS_D1_SHIFT             8
> +#define PRD_IBIAS_D0_SHIFT             0
> +#define HDMI_CON5              0x14
> +#define RG_HDMITX_DRV_IBIAS_CLK                (0x3f << 24)
> +#define RG_HDMITX_DRV_IBIAS_D2         (0x3f << 16)
> +#define RG_HDMITX_DRV_IBIAS_D1         (0x3f << 8)
> +#define RG_HDMITX_DRV_IBIAS_D0         (0x3f << 0)
> +#define DRV_IBIAS_CLK_SHIFT            24
> +#define DRV_IBIAS_D2_SHIFT             16
> +#define DRV_IBIAS_D1_SHIFT             8
> +#define DRV_IBIAS_D0_SHIFT             0
> +#define HDMI_CON6              0x18
> +#define RG_HDMITX_DRV_IMP_CLK          (0x3f << 24)
> +#define RG_HDMITX_DRV_IMP_D2           (0x3f << 16)
> +#define RG_HDMITX_DRV_IMP_D1           (0x3f << 8)
> +#define RG_HDMITX_DRV_IMP_D0           (0x3f << 0)
> +#define DRV_IMP_CLK_SHIFT              24
> +#define DRV_IMP_D2_SHIFT               16
> +#define DRV_IMP_D1_SHIFT               8
> +#define DRV_IMP_D0_SHIFT               0
> +#define HDMI_CON7              0x1c
> +#define RG_HDMITX_MHLCK_DRV_IBIAS      (0x1f << 27)
> +#define RG_HDMITX_SER_DIN              (0x3ff << 16)
> +#define RG_HDMITX_CHLDC_TST            (0xf << 12)
> +#define RG_HDMITX_CHLCK_TST            (0xf << 8)
> +#define RG_HDMITX_RESERVE              (0xff << 0)
> +#define HDMI_CON8              0x20
> +#define RGS_HDMITX_2T1_LEV             (0xf << 16)
> +#define RGS_HDMITX_2T1_EDG             (0xf << 12)
> +#define RGS_HDMITX_5T1_LEV             (0xf << 8)
> +#define RGS_HDMITX_5T1_EDG             (0xf << 4)
> +#define RGS_HDMITX_PLUG_TST            BIT(0)
> +
> +struct mtk_hdmi_phy {
> +       void __iomem *regs;
> +       struct device *dev;
> +       struct clk *pll;
> +       struct clk_hw pll_hw;
> +       unsigned long pll_rate;
> +       u8 drv_imp_clk;
> +       u8 drv_imp_d2;
> +       u8 drv_imp_d1;
> +       u8 drv_imp_d0;
> +       u32 ibias;
> +       u32 ibias_up;
> +};
> +
> +static const u8 PREDIV[3][4] = {
> +       {0x0, 0x0, 0x0, 0x0},   /* 27Mhz */
> +       {0x1, 0x1, 0x1, 0x1},   /* 74Mhz */
> +       {0x1, 0x1, 0x1, 0x1}    /* 148Mhz */
> +};
> +
> +static const u8 TXDIV[3][4] = {
> +       {0x3, 0x3, 0x3, 0x2},   /* 27Mhz */
> +       {0x2, 0x1, 0x1, 0x1},   /* 74Mhz */
> +       {0x1, 0x0, 0x0, 0x0}    /* 148Mhz */
> +};
> +
> +static const u8 FBKSEL[3][4] = {
> +       {0x1, 0x1, 0x1, 0x1},   /* 27Mhz */
> +       {0x1, 0x0, 0x1, 0x1},   /* 74Mhz */
> +       {0x1, 0x0, 0x1, 0x1}    /* 148Mhz */
> +};
> +
> +static const u8 FBKDIV[3][4] = {
> +       {19, 24, 29, 19},       /* 27Mhz */
> +       {19, 24, 14, 19},       /* 74Mhz */
> +       {19, 24, 14, 19}        /* 148Mhz */
> +};
> +
> +static const u8 DIVEN[3][4] = {
> +       {0x2, 0x1, 0x1, 0x2},   /* 27Mhz */
> +       {0x2, 0x2, 0x2, 0x2},   /* 74Mhz */
> +       {0x2, 0x2, 0x2, 0x2}    /* 148Mhz */
> +};
> +
> +static const u8 HTPLLBP[3][4] = {
> +       {0xc, 0xc, 0x8, 0xc},   /* 27Mhz */
> +       {0xc, 0xf, 0xf, 0xc},   /* 74Mhz */
> +       {0xc, 0xf, 0xf, 0xc}    /* 148Mhz */
> +};
> +
> +static const u8 HTPLLBC[3][4] = {
> +       {0x2, 0x3, 0x3, 0x2},   /* 27Mhz */
> +       {0x2, 0x3, 0x3, 0x2},   /* 74Mhz */
> +       {0x2, 0x3, 0x3, 0x2}    /* 148Mhz */
> +};
> +
> +static const u8 HTPLLBR[3][4] = {
> +       {0x1, 0x1, 0x0, 0x1},   /* 27Mhz */
> +       {0x1, 0x2, 0x2, 0x1},   /* 74Mhz */
> +       {0x1, 0x2, 0x2, 0x1}    /* 148Mhz */
> +};
> +
> +static void mtk_hdmi_phy_mask(struct mtk_hdmi_phy *hdmi_phy, u32 offset,
> +                             u32 val, u32 mask)
> +{
> +       u32 tmp = readl(hdmi_phy->regs  + offset) & ~mask;
> +
> +       tmp |= (val & mask);
> +       writel(tmp, hdmi_phy->regs + offset);
> +}
> +
> +static inline struct mtk_hdmi_phy *to_mtk_hdmi_phy(struct clk_hw *hw)
> +{
> +       return container_of(hw, struct mtk_hdmi_phy, pll_hw);
> +}
> +
> +static int mtk_hdmi_pll_prepare(struct clk_hw *hw)
> +{
> +       struct mtk_hdmi_phy *hdmi_phy = to_mtk_hdmi_phy(hw);
> +
> +       dev_dbg(hdmi_phy->dev, "prepare\n");
> +
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, RG_HDMITX_PLL_AUTOK_EN,
> +                         RG_HDMITX_PLL_AUTOK_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0, RG_HDMITX_PLL_POSDIV,
> +                         RG_HDMITX_PLL_POSDIV);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, 0, RG_HDMITX_MHLCK_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, RG_HDMITX_PLL_BIAS_EN,
> +                         RG_HDMITX_PLL_BIAS_EN);
> +       usleep_range(100, 150);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0, RG_HDMITX_PLL_EN,
> +                         RG_HDMITX_PLL_EN);
> +       usleep_range(100, 150);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, RG_HDMITX_PLL_BIAS_LPF_EN,
> +                         RG_HDMITX_PLL_BIAS_LPF_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, RG_HDMITX_PLL_TXDIV_EN,
> +                         RG_HDMITX_PLL_TXDIV_EN);
> +
> +       return 0;
> +}
> +
> +static void mtk_hdmi_pll_unprepare(struct clk_hw *hw)
> +{
> +       struct mtk_hdmi_phy *hdmi_phy = to_mtk_hdmi_phy(hw);
> +
> +       dev_dbg(hdmi_phy->dev, "prepare\n");

nit: "unprepare" (or just '"%s\n", __func__)', here and everywhere.

> +
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, 0, RG_HDMITX_PLL_TXDIV_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, 0, RG_HDMITX_PLL_BIAS_LPF_EN);
> +       usleep_range(100, 150);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0, 0, RG_HDMITX_PLL_EN);
> +       usleep_range(100, 150);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, 0, RG_HDMITX_PLL_BIAS_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0, 0, RG_HDMITX_PLL_POSDIV);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1, 0, RG_HDMITX_PLL_AUTOK_EN);
> +       usleep_range(100, 150);
> +}
> +
> +static int mtk_hdmi_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> +                                unsigned long parent_rate)
> +{
> +       struct mtk_hdmi_phy *hdmi_phy = to_mtk_hdmi_phy(hw);
> +       unsigned int pre_div;
> +       unsigned int div;
> +
> +       dev_dbg(hdmi_phy->dev, "set rate : %lu, parent: %lu\n", rate,

nit, no space before the first ':'.

> +               parent_rate);
> +
> +       if (rate <= 27000000) {
> +               pre_div = 0;
> +               div = 3;
> +       } else if (rate <= 74250000) {
> +               pre_div = 1;
> +               div = 2;
> +       } else {
> +               pre_div = 1;
> +               div = 1;
> +       }
> +
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0,
> +                         (pre_div << PREDIV_SHIFT),
> +                         RG_HDMITX_PLL_PREDIV);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0, RG_HDMITX_PLL_POSDIV,
> +                         RG_HDMITX_PLL_POSDIV);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0,
> +                         (0x1 << PLL_IC_SHIFT) | (0x1 << PLL_IR_SHIFT),
> +                         RG_HDMITX_PLL_IC | RG_HDMITX_PLL_IR);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1,
> +                         (div << PLL_TXDIV_SHIFT),
> +                         RG_HDMITX_PLL_TXDIV);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0,
> +                         (0x1 << PLL_FBKSEL_SHIFT) | (19 << PLL_FBKDIV_SHIFT),
> +                         RG_HDMITX_PLL_FBKSEL | RG_HDMITX_PLL_FBKDIV);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON1,
> +                         (0x2 << PLL_DIVEN_SHIFT),
> +                         RG_HDMITX_PLL_DIVEN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON0,
> +                         (0xc << PLL_BP_SHIFT) | (0x2 << PLL_BC_SHIFT) |
> +                         (0x1 << PLL_BR_SHIFT),
> +                         RG_HDMITX_PLL_BP | RG_HDMITX_PLL_BC |
> +                         RG_HDMITX_PLL_BR);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, 0, RG_HDMITX_PRD_IMP_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON4,
> +                         (0x3 << PRD_IBIAS_CLK_SHIFT) |
> +                         (0x3 << PRD_IBIAS_D2_SHIFT) |
> +                         (0x3 << PRD_IBIAS_D1_SHIFT) |
> +                         (0x3 << PRD_IBIAS_D0_SHIFT),
> +                         RG_HDMITX_PRD_IBIAS_CLK |
> +                         RG_HDMITX_PRD_IBIAS_D2 |
> +                         RG_HDMITX_PRD_IBIAS_D1 |
> +                         RG_HDMITX_PRD_IBIAS_D0);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3,
> +                         (0x0 << DRV_IMP_EN_SHIFT),
> +                         RG_HDMITX_DRV_IMP_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON6,
> +                         (hdmi_phy->drv_imp_clk << DRV_IMP_CLK_SHIFT) |
> +                         (hdmi_phy->drv_imp_d2 << DRV_IMP_D2_SHIFT) |
> +                         (hdmi_phy->drv_imp_d1 << DRV_IMP_D1_SHIFT) |
> +                         (hdmi_phy->drv_imp_d0 << DRV_IMP_D0_SHIFT),
> +                         RG_HDMITX_DRV_IMP_CLK | RG_HDMITX_DRV_IMP_D2 |
> +                         RG_HDMITX_DRV_IMP_D1 | RG_HDMITX_DRV_IMP_D0);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON5,
> +                         (hdmi_phy->ibias << DRV_IBIAS_CLK_SHIFT) |
> +                         (hdmi_phy->ibias << DRV_IBIAS_D2_SHIFT) |
> +                         (hdmi_phy->ibias << DRV_IBIAS_D1_SHIFT) |
> +                         (hdmi_phy->ibias << DRV_IBIAS_D0_SHIFT),
> +                         RG_HDMITX_DRV_IBIAS_CLK | RG_HDMITX_DRV_IBIAS_D2 |
> +                         RG_HDMITX_DRV_IBIAS_D1 | RG_HDMITX_DRV_IBIAS_D0);
> +       return 0;
> +}
> +
> +static long mtk_hdmi_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                   unsigned long *parent_rate)
> +{
> +       struct mtk_hdmi_phy *hdmi_phy = to_mtk_hdmi_phy(hw);
> +
> +       hdmi_phy->pll_rate = rate;
> +       if (rate <= 74250000)
> +               *parent_rate = rate;
> +       else
> +               *parent_rate = rate / 2;
> +
> +       return rate;
> +}
> +
> +static unsigned long mtk_hdmi_pll_recalc_rate(struct clk_hw *hw,
> +                                             unsigned long parent_rate)
> +{
> +       struct mtk_hdmi_phy *hdmi_phy = to_mtk_hdmi_phy(hw);
> +
> +       return hdmi_phy->pll_rate;
> +}
> +
> +static const struct clk_ops mtk_hdmi_pll_ops = {
> +       .prepare = mtk_hdmi_pll_prepare,
> +       .unprepare = mtk_hdmi_pll_unprepare,
> +       .set_rate = mtk_hdmi_pll_set_rate,
> +       .round_rate = mtk_hdmi_pll_round_rate,
> +       .recalc_rate = mtk_hdmi_pll_recalc_rate,
> +};
> +
> +static void mtk_hdmi_phy_enable_tmds(struct mtk_hdmi_phy *hdmi_phy)
> +{
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, RG_HDMITX_SER_EN,
> +                         RG_HDMITX_SER_EN);

nit: lots of these calls might be more readable (and easier to maintain &
review) if we used two helper functions:

mtk_hdmi_phy_set_bits(struct mtk_hdmi_phy *hdmi_phy, u32 offset, u32 mask);
mtk_hdmi_phy_clr_bits(struct mtk_hdmi_phy *hdmi_phy, u32 offset, u32 mask);

and

mtk_hdmi_set_bits()
mtk_hdmi_clr_bits()


> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, RG_HDMITX_PRD_EN,
> +                         RG_HDMITX_PRD_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, RG_HDMITX_DRV_EN,
> +                         RG_HDMITX_DRV_EN);
> +       usleep_range(100, 150);
> +}
> +
> +static void mtk_hdmi_phy_disable_tmds(struct mtk_hdmi_phy *hdmi_phy)
> +{
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, 0, RG_HDMITX_DRV_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, 0, RG_HDMITX_PRD_EN);
> +       mtk_hdmi_phy_mask(hdmi_phy, HDMI_CON3, 0, RG_HDMITX_SER_EN);
> +}
> +
> +static int mtk_hdmi_phy_power_on(struct phy *phy)
> +{
> +       struct mtk_hdmi_phy *hdmi_phy = phy_get_drvdata(phy);
> +       int ret;
> +
> +       ret = clk_prepare_enable(hdmi_phy->pll);
> +       if (ret < 0)
> +               return ret;
> +
> +       mtk_hdmi_phy_enable_tmds(hdmi_phy);
> +
> +       return 0;
> +}
> +
> +static int mtk_hdmi_phy_power_off(struct phy *phy)
> +{
> +       struct mtk_hdmi_phy *hdmi_phy = phy_get_drvdata(phy);
> +
> +       mtk_hdmi_phy_disable_tmds(hdmi_phy);
> +       clk_disable_unprepare(hdmi_phy->pll);
> +
> +       return 0;
> +}
> +
> +static struct phy_ops mtk_hdmi_phy_ops = {

static const

> +       .power_on = mtk_hdmi_phy_power_on,
> +       .power_off = mtk_hdmi_phy_power_off,
> +       .owner = THIS_MODULE,
> +};
> +
> +static int mtk_hdmi_phy_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct mtk_hdmi_phy *hdmi_phy;
> +       struct resource *mem;
> +       struct clk *ref_clk;
> +       const char *ref_clk_name;
> +       struct clk_init_data clk_init = {
> +               .ops = &mtk_hdmi_pll_ops,
> +               .num_parents = 1,
> +               .parent_names = (const char * const *)&ref_clk_name,
> +               .flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
> +       };
> +       struct phy *phy;
> +       struct phy_provider *phy_provider;
> +       int ret;
> +
> +       hdmi_phy = devm_kzalloc(dev, sizeof(*hdmi_phy), GFP_KERNEL);
> +       if (!hdmi_phy)
> +               return -ENOMEM;
> +
> +       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       hdmi_phy->regs = devm_ioremap_resource(dev, mem);
> +       if (IS_ERR(hdmi_phy->regs)) {
> +               ret = PTR_ERR(hdmi_phy->regs);
> +               dev_err(dev, "Failed to get memory resource: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ref_clk = devm_clk_get(dev, "pll_ref");
> +       if (IS_ERR(ref_clk)) {
> +               ret = PTR_ERR(ref_clk);
> +               dev_err(&pdev->dev, "Failed to get PLL reference clock: %d\n",
> +                       ret);
> +               return ret;
> +       }
> +       ref_clk_name = __clk_get_name(ref_clk);
> +
> +       ret = of_property_read_string(dev->of_node, "clock-output-names",
> +                                     &clk_init.name);
> +       if (ret < 0) {
> +               dev_err(dev, "Failed to read clock-output-names: %d\n", ret);
> +               return ret;
> +       }
> +
> +       hdmi_phy->pll_hw.init = &clk_init;
> +       hdmi_phy->pll = devm_clk_register(dev, &hdmi_phy->pll_hw);
> +       if (IS_ERR(hdmi_phy->pll)) {
> +               ret = PTR_ERR(hdmi_phy->pll);
> +               dev_err(dev, "Failed to register PLL: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = of_property_read_u32(dev->of_node, "mediatek,ibias",
> +                                  &hdmi_phy->ibias);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "Failed to get ibias: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = of_property_read_u32(dev->of_node, "mediatek,ibias_up",
> +                                  &hdmi_phy->ibias_up);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "Failed to get ibias up: %d\n", ret);
> +               return ret;
> +       }
> +
> +       dev_info(dev, "Using default TX DRV impedance: 4.2k/36\n");
> +       hdmi_phy->drv_imp_clk = 0x30;
> +       hdmi_phy->drv_imp_d2 = 0x30;
> +       hdmi_phy->drv_imp_d1 = 0x30;
> +       hdmi_phy->drv_imp_d0 = 0x30;
> +
> +       phy = devm_phy_create(dev, NULL, &mtk_hdmi_phy_ops);
> +       if (IS_ERR(phy)) {
> +               dev_err(dev, "Failed to create HDMI PHY\n");
> +               return PTR_ERR(phy);
> +       }
> +       phy_set_drvdata(phy, hdmi_phy);
> +
> +       phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +       if (IS_ERR(phy_provider))
> +               return PTR_ERR(phy_provider);
> +
> +       hdmi_phy->dev = dev;
> +       return of_clk_add_provider(dev->of_node, of_clk_src_simple_get,
> +                                  hdmi_phy->pll);
> +}
> +
> +static int mtk_hdmi_phy_remove(struct platform_device *pdev)
> +{
> +       return 0;
> +}
> +
> +static const struct of_device_id mtk_hdmi_phy_match[] = {
> +       { .compatible = "mediatek,mt8173-hdmi-phy", },
> +       {},
> +};
> +
> +struct platform_driver mtk_hdmi_phy_driver = {
> +       .probe = mtk_hdmi_phy_probe,
> +       .remove = mtk_hdmi_phy_remove,
> +       .driver = {
> +               .name = "mediatek-hdmi-phy",
> +               .of_match_table = mtk_hdmi_phy_match,
> +       },
> +};
> +
> +MODULE_AUTHOR("Jie Qiu <jie.qiu@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("MediaTek MT8173 HDMI PHY Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux