Re: [PATCH v2 02/17] video: dw_hdmi: Add Vendor PHY handling

On 18/12/2023 20:10, Jagan Teki wrote:

From: Jagan Teki jagan@edgeble.ai

DW HDMI support Vendor PHY like Rockchip RK3328 Inno HDMI PHY.

Extend the vendor phy handling by adding platform phy hooks.

Signed-off-by: Jagan Teki jagan@edgeble.ai

Changes for v2:

• fix meson cfg

  drivers/video/dw_hdmi.c | 29 +++++++++++++++++++++++++++- drivers/video/meson/meson_dw_hdmi.c | 11 ++++++++++- drivers/video/rockchip/rk3399_hdmi.c | 8 +++++++- drivers/video/rockchip/rk_hdmi.c | 2 +- drivers/video/sunxi/sunxi_dw_hdmi.c | 11 ++++++++++- include/dw_hdmi.h | 14 +++++++++++++- 6 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/video/dw_hdmi.c b/drivers/video/dw_hdmi.c index c4fbb18294..ea12a09407 100644 --- a/drivers/video/dw_hdmi.c +++ b/drivers/video/dw_hdmi.c @@ -988,7 +988,7 @@ int dw_hdmi_enable(struct dw_hdmi *hdmi, const struct display_timing *edid)

hdmi_av_composer(hdmi, edid);

• ret = hdmi->phy_set(hdmi, edid->pixelclock.typ);

• ret = hdmi->ops->phy_set(hdmi, edid->pixelclock.typ); if (ret) return ret;

@@ -1009,10 +1009,37 @@ int dw_hdmi_enable(struct dw_hdmi *hdmi, const struct display_timing *edid) return 0; }

+static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {

• .phy_set = dw_hdmi_phy_cfg,

+};

+static void dw_hdmi_detect_phy(struct dw_hdmi *hdmi) +{

• if (!hdmi->data)

• return;
  

• /* hook Synopsys PHYs ops */
• if (!hdmi->data->phy_force_vendor) {

• hdmi->ops = &dw_hdmi_synopsys_phy_ops;
  

• return;
  

• }
• /* Vendor HDMI PHYs must assign phy_ops in plat_data */
• if (!hdmi->data->phy_ops) {

• printf("Unsupported Vendor HDMI phy_ops\n");
  

• return;
  

• }
• /* hook Vendor HDMI PHYs ops */
• hdmi->ops = hdmi->data->phy_ops;

Sorry but I still don't understand why you need phy_force_vendor & phy_ops, this code clearly fails if you have phy_force_vendor=true && phy_ops=NULL, so drop phy_force_vendor and simply use phy_ops if != NULL, and since it's the only element of dw_hdmi_plat_data, drop dw_hdmi_plat_data and pass dw_hdmi_phy_ops directly in the dw_hdmi struct.

So in dw_hdmi_detect_phy(), if hdmi->ops is NULL, set it to dw_hdmi_synopsys_phy_ops.

Neil

+}

• void dw_hdmi_init(struct dw_hdmi *hdmi) { uint ih_mute;

• dw_hdmi_detect_phy(hdmi);

• /*

  • boot up defaults are:
  • hdmi_ih_mute = 0x03 (disabled)

diff --git a/drivers/video/meson/meson_dw_hdmi.c b/drivers/video/meson/meson_dw_hdmi.c index 5db01904b5..d0d878b6af 100644 --- a/drivers/video/meson/meson_dw_hdmi.c +++ b/drivers/video/meson/meson_dw_hdmi.c @@ -375,6 +375,15 @@ static int meson_dw_hdmi_wait_hpd(struct dw_hdmi *hdmi) return -ETIMEDOUT; }

+static const struct dw_hdmi_phy_ops dw_hdmi_meson_phy_ops = {

• .phy_set = meson_dw_hdmi_phy_init,

+};

+static const struct dw_hdmi_plat_data dw_hdmi_meson_plat_data = {

• .phy_force_vendor = true,
• .phy_ops = &dw_hdmi_meson_phy_ops,

+};

• static int meson_dw_hdmi_probe(struct udevice *dev) { struct meson_dw_hdmi *priv = dev_get_priv(dev);

@@ -397,7 +406,7 @@ static int meson_dw_hdmi_probe(struct udevice *dev)

priv->hdmi.hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24; priv->hdmi.hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_YUV8_1X24;

• priv->hdmi.phy_set = meson_dw_hdmi_phy_init;

• priv->hdmi.data = &dw_hdmi_meson_plat_data; if (meson_hdmi_is_compatible(priv, HDMI_COMPATIBLE_G12A)) priv->hdmi.reg_io_width = 1; else {

diff --git a/drivers/video/rockchip/rk3399_hdmi.c b/drivers/video/rockchip/rk3399_hdmi.c index 3041360c6e..b32139a8a6 100644 --- a/drivers/video/rockchip/rk3399_hdmi.c +++ b/drivers/video/rockchip/rk3399_hdmi.c @@ -64,8 +64,14 @@ static const struct dm_display_ops rk3399_hdmi_ops = { .enable = rk3399_hdmi_enable, };

+static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = { +};

• static const struct udevice_id rk3399_hdmi_ids[] = {

• { .compatible = "rockchip,rk3399-dw-hdmi" },

• {

• .compatible = "rockchip,rk3399-dw-hdmi",
  

• .data = (ulong)&rk3399_hdmi_drv_data
  

• }, { } };

diff --git a/drivers/video/rockchip/rk_hdmi.c b/drivers/video/rockchip/rk_hdmi.c index b75a174489..e34f532cd6 100644 --- a/drivers/video/rockchip/rk_hdmi.c +++ b/drivers/video/rockchip/rk_hdmi.c @@ -83,6 +83,7 @@ int rk_hdmi_of_to_plat(struct udevice *dev) struct rk_hdmi_priv *priv = dev_get_priv(dev); struct dw_hdmi *hdmi = &priv->hdmi;

• hdmi->data = (const struct dw_hdmi_plat_data *)dev_get_driver_data(dev); hdmi->ioaddr = (ulong)dev_read_addr(dev); hdmi->mpll_cfg = rockchip_mpll_cfg; hdmi->phy_cfg = rockchip_phy_config;

@@ -90,7 +91,6 @@ int rk_hdmi_of_to_plat(struct udevice *dev) /* hdmi->i2c_clk_{high,low} are set up by the SoC driver */

hdmi->reg_io_width = 4;

• hdmi->phy_set = dw_hdmi_phy_cfg;

  priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);

diff --git a/drivers/video/sunxi/sunxi_dw_hdmi.c b/drivers/video/sunxi/sunxi_dw_hdmi.c index 0324a050d0..4b67a1614e 100644 --- a/drivers/video/sunxi/sunxi_dw_hdmi.c +++ b/drivers/video/sunxi/sunxi_dw_hdmi.c @@ -369,6 +369,15 @@ static int sunxi_dw_hdmi_probe(struct udevice *dev) return 0; }

+static const struct dw_hdmi_phy_ops dw_hdmi_sunxi_phy_ops = {

• .phy_set = sunxi_dw_hdmi_phy_cfg,

+};

+static const struct dw_hdmi_plat_data dw_hdmi_sunxi_plat_data = {

• .phy_force_vendor = true,
• .phy_ops = &dw_hdmi_sunxi_phy_ops,

+};

• static int sunxi_dw_hdmi_of_to_plat(struct udevice *dev) { struct sunxi_dw_hdmi_priv *priv = dev_get_priv(dev);

@@ -379,7 +388,7 @@ static int sunxi_dw_hdmi_of_to_plat(struct udevice *dev) hdmi->i2c_clk_high = 0xd8; hdmi->i2c_clk_low = 0xfe; hdmi->reg_io_width = 1;

• hdmi->phy_set = sunxi_dw_hdmi_phy_cfg;

• hdmi->data = &dw_hdmi_sunxi_plat_data;

  ret = reset_get_bulk(dev, &priv->resets); if (ret)

diff --git a/include/dw_hdmi.h b/include/dw_hdmi.h index 8acae3839f..4ad8b39f84 100644 --- a/include/dw_hdmi.h +++ b/include/dw_hdmi.h @@ -534,6 +534,17 @@ struct hdmi_data_info { struct hdmi_vmode video_mode; };

+struct dw_hdmi;

+struct dw_hdmi_phy_ops {

• int (*phy_set)(struct dw_hdmi *hdmi, uint mpixelclock);

+};

+struct dw_hdmi_plat_data {

• bool phy_force_vendor;
• const struct dw_hdmi_phy_ops *phy_ops;

+};

• struct dw_hdmi { ulong ioaddr; const struct hdmi_mpll_config *mpll_cfg;

@@ -543,8 +554,9 @@ struct dw_hdmi { u8 reg_io_width; struct hdmi_data_info hdmi_data; struct udevice *ddc_bus;

• const struct dw_hdmi_phy_ops *ops;
• const struct dw_hdmi_plat_data *data;

• int (*phy_set)(struct dw_hdmi *hdmi, uint mpixelclock); void (*write_reg)(struct dw_hdmi *hdmi, u8 val, int offset); u8 (*read_reg)(struct dw_hdmi *hdmi, int offset); };