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

Jagan Teki jagan at amarulasolutions.com
Thu Dec 14 05:44:28 CET 2023


Hi Simon,

On Thu, Dec 14, 2023 at 1:21 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Jagan,
>
> On Mon, 11 Dec 2023 at 02:00, Jagan Teki <jagan at amarulasolutions.com> 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(-)
> >
>
> Isn't there a PHY framework we should use in U-Boot?

Yes, the driver is using generic PHY.

>
> Regards,
> Sim on
>
> > 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,
> > +};
> > +
> > +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;
>
> ??
>
> We should use phy.h I believe. Having said that, it sorely needs a
> proper conversion to driver model.

As I said, the driver uses generic phy with phy_ops. The above ops are
dw hdmi platform phy ops that driver uses internally for rk3328. DW
HDMI is also available for rk3228, so internal platform phy ops can
vary. The generic phy ops in the driver will call platform phy ops via
driver data.

Jagan.


More information about the U-Boot mailing list