On Mon, Dec 11, 2023 at 2:38 PM Neil Armstrong
<neil.armstr...@linaro.org> wrote:
>
> On 11/12/2023 09:59, Jagan Teki wrote:
> > From: Jagan Teki <ja...@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 <ja...@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
Correct, I will update this.
>
> > +};
> > +
> > +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;
This logic seems not proper according to DW HDMI vendor phy support.
phy_set -> dw_hdmi_phy_cfg in dw_hdmi.c is what synopsys default phy
ops set. and reset defined in their hdmi drivers are vendor-related.
So only rk3399 has default synopsys phy_set but meson, sunxi and this
rk3328 has vendor phy ops. We need to define plat_data with phy_ops
and phy_force_vendor flag to support this setup.
Thanks,
Jagan.
