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

Neil Armstrong neil.armstrong at linaro.org
Mon Dec 11 10:08:24 CET 2023


On 11/12/2023 09:59, Jagan Teki wrote:
> From: Jagan Teki <jagan at 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 at edgeble.ai>
> ---
>   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;
> +}
> +
>   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..63ca3ac52e 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_cfg,

Pretty sure this should be 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;
> +};

Why do you need a separate phy_ops and this phy_force_vendor ?

Simply use the provided phy_ops is pointer != NULL

I think simply use a single struct passed as pdata to have:

if (!hdmi->data)
	hdmi->ops = &dw_hdmi_synopsys_phy_ops;
else
	/* hook Vendor HDMI PHYs ops */
	hdmi->ops = hdmi->data;

Neil

> +
>   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);
>   };



More information about the U-Boot mailing list