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

2024-01-10 Thread neil . armstrong

On 09/01/2024 21:04, Jagan Teki wrote:

Hi Neil,

On Tue, Dec 19, 2023 at 5:21 PM Jagan Teki  wrote:


On Tue, Dec 19, 2023 at 2:34 PM Neil Armstrong
 wrote:


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

From: Jagan Teki 

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 
---
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.


Let me elaborate more.

DW HDMI IP must have phy ops. It never be NULL. Either it uses
1. Internal PHY via DW called them Synopsys PHYs ops - for example, rk3399
2. Vendor PHY via vendor phy meson, sunxi, rk3328

For case 1) phy_force_vendor is false so it uses dw_hdmi_synopsys_phy_ops
For case 2) phy_force_vendor is true so it uses dw_hdmi_plat_data phy ops

dw_hdmi_detect_phy assigns internal phy ops first and then vendor phy
ops based on phy_force_vendor flag.

If we remove dw_hdmi_plat_data how can we assign or differentiate two
types of phy ops hooks? can you explain?


Let me know if you have any comments on this. I'm sending V3 would
probably hit in MW.


Sure, I understand your point, but there's a limited number of users and it can
be greatly simplified, no need to keep the current structure, there's no 
external
users of the driver.

Just set hdmi->ops with the vendor phy ops from the glue driver if needed,
or let it to NULL if the platform uses the synopsys PHY, it's as simple as that.
If hdmi->ops is NULL it would be replaced with dw_hdmi_synopsys_phy_ops in the
dw_hdmi_detect_phy() function.

Neil



Thanks,
Jagan.




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

2024-01-09 Thread Jagan Teki
Hi Neil,

On Tue, Dec 19, 2023 at 5:21 PM Jagan Teki  wrote:
>
> On Tue, Dec 19, 2023 at 2:34 PM Neil Armstrong
>  wrote:
> >
> > On 18/12/2023 20:10, Jagan Teki wrote:
> > > From: Jagan Teki 
> > >
> > > 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 
> > > ---
> > > 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.
>
> Let me elaborate more.
>
> DW HDMI IP must have phy ops. It never be NULL. Either it uses
> 1. Internal PHY via DW called them Synopsys PHYs ops - for example, rk3399
> 2. Vendor PHY via vendor phy meson, sunxi, rk3328
>
> For case 1) phy_force_vendor is false so it uses dw_hdmi_synopsys_phy_ops
> For case 2) phy_force_vendor is true so it uses dw_hdmi_plat_data phy ops
>
> dw_hdmi_detect_phy assigns internal phy ops first and then vendor phy
> ops based on phy_force_vendor flag.
>
> If we remove dw_hdmi_plat_data how can we assign or differentiate two
> types of phy ops hooks? can you explain?

Let me know if you have any comments on this. I'm sending V3 would
probably hit in MW.

Thanks,
Jagan.


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

2023-12-19 Thread Jagan Teki
On Tue, Dec 19, 2023 at 2:34 PM Neil Armstrong
 wrote:
>
> On 18/12/2023 20:10, Jagan Teki wrote:
> > From: Jagan Teki 
> >
> > 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 
> > ---
> > 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.

Let me elaborate more.

DW HDMI IP must have phy ops. It never be NULL. Either it uses
1. Internal PHY via DW called them Synopsys PHYs ops - for example, rk3399
2. Vendor PHY via vendor phy meson, sunxi, rk3328

For case 1) phy_force_vendor is false so it uses dw_hdmi_synopsys_phy_ops
For case 2) phy_force_vendor is true so it uses dw_hdmi_plat_data phy ops

dw_hdmi_detect_phy assigns internal phy ops first and then vendor phy
ops based on phy_force_vendor flag.

If we remove dw_hdmi_plat_data how can we assign or differentiate two
types of phy ops hooks? can you explain?

Thanks,
Jagan.


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

2023-12-19 Thread Neil Armstrong

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

From: Jagan Teki 

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 
---
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

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

2023-12-18 Thread Jagan Teki
From: Jagan Teki 

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