[PATCH 12/22] drm: bridge: dw-hdmi: Abstract the platform PHY configuration

[Laurent Pinchart]

Hi Jose,

On Friday 02 Dec 2016 11:15:19 Jose Abreu wrote:
> On 01-12-2016 23:43, Laurent Pinchart wrote:
> > From: Kieran Bingham 
> > 
> > Platforms implement the dw-hdmi with a separate PHY entity. It is
> > configured over it's own I2C bus. To allow for different PHY's to be
> > configured from the dw-hdmi driver, abstract the actual programming of
> > the PHY to its own functions, as configured by the platform.
> > 
> > Signed-off-by: Kieran Bingham 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> > drivers/gpu/drm/bridge/dw-hdmi.c| 79 +++-
> > drivers/gpu/drm/imx/dw_hdmi-imx.c   |  2 +
> > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  1 +
> > include/drm/bridge/dw_hdmi.h| 12 +
> > 4 files changed, 63 insertions(+), 31 deletions(-)

[snip]

> > diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> > index e551b457c100..fa7655836c81 100644
> > --- a/include/drm/bridge/dw_hdmi.h
> > +++ b/include/drm/bridge/dw_hdmi.h
> > @@ -52,6 +52,10 @@ struct dw_hdmi_plat_data {
> > const struct dw_hdmi_mpll_config *mpll_cfg;
> > const struct dw_hdmi_curr_ctrl *cur_ctr;
> > const struct dw_hdmi_phy_config *phy_config;
> > +   int (*configure_phy)(struct dw_hdmi *hdmi,
> > +const struct dw_hdmi_plat_data *pdata,
> > +unsigned long mpixelclock,
> > +enum dw_hdmi_resolution resolution);
> 
> I think the name of this callback here is a little bit misleading
> because you are only configuring the phy pll. Maybe
> .configure_phy_pll() would be more suitable.

Isn't there more than the PLL ? For instance register TXTERM configures the 
transmission line termination.

> > enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> > 
> >struct drm_display_mode *mode);
> >  
> >  };

-- 
Regards,

Laurent Pinchart

[PATCH 12/22] drm: bridge: dw-hdmi: Abstract the platform PHY configuration

[Jose Abreu]

Hi Laurent,


On 01-12-2016 23:43, Laurent Pinchart wrote:
> From: Kieran Bingham 
>
> Platforms implement the dw-hdmi with a separate PHY entity. It is
> configured over it's own I2C bus. To allow for different PHY's to be
> configured from the dw-hdmi driver, abstract the actual programming of
> the PHY to its own functions, as configured by the platform.
>
> Signed-off-by: Kieran Bingham 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c| 79 
> ++---
>  drivers/gpu/drm/imx/dw_hdmi-imx.c   |  2 +
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  1 +
>  include/drm/bridge/dw_hdmi.h| 12 +
>  4 files changed, 63 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index 074ceb1e4897..06a44a2cdf3b 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -867,8 +867,8 @@ static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi, 
> int msec)
>   return true;
>  }
>  
> -static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> -  unsigned char addr)
> +void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> +unsigned char addr)
>  {
>   hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
>   hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR);
> @@ -880,6 +880,7 @@ static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, 
> unsigned short data,
>   HDMI_PHY_I2CM_OPERATION_ADDR);
>   hdmi_phy_wait_i2c_done(hdmi, 1000);
>  }
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write);
>  
>  static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi *hdmi, bool enable)
>  {
> @@ -930,38 +931,61 @@ static void dw_hdmi_phy_sel_interface_control(struct 
> dw_hdmi *hdmi, u8 enable)
>HDMI_PHY_CONF0_SELDIPIF_MASK);
>  }
>  
> -static int hdmi_phy_configure(struct dw_hdmi *hdmi,
> -   enum dw_hdmi_resolution res_idx, int cscon)
> +int dw_hdmi_phy_configure_synopsys(struct dw_hdmi *hdmi,
> +const struct dw_hdmi_plat_data *pdata,
> +unsigned long mpixelclock,
> +enum dw_hdmi_resolution resolution)
> +
>  {
> - u8 val, msec;
> - const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
>   const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
>   const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
>   const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;
>  
>   /* PLL/MPLL Cfg - always match on final entry */
>   for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
> - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> - mpll_config->mpixelclock)
> + if (mpixelclock <= mpll_config->mpixelclock)
>   break;
>  
>   for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
> - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> - curr_ctrl->mpixelclock)
> + if (mpixelclock <= curr_ctrl->mpixelclock)
>   break;
>  
>   for (; phy_config->mpixelclock != ~0UL; phy_config++)
> - if (hdmi->hdmi_data.video_mode.mpixelclock <=
> - phy_config->mpixelclock)
> + if (mpixelclock <= phy_config->mpixelclock)
>   break;
>  
>   if (mpll_config->mpixelclock == ~0UL ||
>   curr_ctrl->mpixelclock == ~0UL ||
> - phy_config->mpixelclock == ~0UL) {
> - dev_err(hdmi->dev, "Pixel clock %d - unsupported by HDMI\n",
> - hdmi->hdmi_data.video_mode.mpixelclock);
> + phy_config->mpixelclock == ~0UL)
>   return -EINVAL;
> - }
> +
> + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[resolution].cpce, 0x06);
> + dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[resolution].gmp, 0x15);
> +
> + /* CURRCTRL */
> + dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[resolution], 0x10);
> +
> + dw_hdmi_phy_i2c_write(hdmi, 0x, 0x13); /* PLLPHBYCTRL */
> + dw_hdmi_phy_i2c_write(hdmi, 0x0006, 0x17);
> +
> + dw_hdmi_phy_i2c_write(hdmi, phy_config->term, 0x19); /* TXTERM */
> + dw_hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr, 0x09); /* CKSYMTXCTRL 
> */
> + dw_hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr, 0x0E); /* VLEVCTRL */
> +
> + /* REMOVE CLK TERM */
> + dw_hdmi_phy_i2c_write(hdmi, 0x8000, 0x05); /* CKCALCTRL */
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_configure_synopsys);
> +
> +static int hdmi_phy_configure(struct dw_hdmi *hdmi,
> +   enum dw_hdmi_resolution resolution, int cscon)
> +{
> + const struct 

[PATCH 12/22] drm: bridge: dw-hdmi: Abstract the platform PHY configuration

[Laurent Pinchart]

From: Kieran Bingham 

Platforms implement the dw-hdmi with a separate PHY entity. It is
configured over it's own I2C bus. To allow for different PHY's to be
configured from the dw-hdmi driver, abstract the actual programming of
the PHY to its own functions, as configured by the platform.

Signed-off-by: Kieran Bingham 
Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/bridge/dw-hdmi.c| 79 ++---
 drivers/gpu/drm/imx/dw_hdmi-imx.c   |  2 +
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |  1 +
 include/drm/bridge/dw_hdmi.h| 12 +
 4 files changed, 63 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 074ceb1e4897..06a44a2cdf3b 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -867,8 +867,8 @@ static bool hdmi_phy_wait_i2c_done(struct dw_hdmi *hdmi, 
int msec)
return true;
 }

-static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
-unsigned char addr)
+void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
+  unsigned char addr)
 {
hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR);
@@ -880,6 +880,7 @@ static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi, 
unsigned short data,
HDMI_PHY_I2CM_OPERATION_ADDR);
hdmi_phy_wait_i2c_done(hdmi, 1000);
 }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write);

 static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi *hdmi, bool enable)
 {
@@ -930,38 +931,61 @@ static void dw_hdmi_phy_sel_interface_control(struct 
dw_hdmi *hdmi, u8 enable)
 HDMI_PHY_CONF0_SELDIPIF_MASK);
 }

-static int hdmi_phy_configure(struct dw_hdmi *hdmi,
- enum dw_hdmi_resolution res_idx, int cscon)
+int dw_hdmi_phy_configure_synopsys(struct dw_hdmi *hdmi,
+  const struct dw_hdmi_plat_data *pdata,
+  unsigned long mpixelclock,
+  enum dw_hdmi_resolution resolution)
+
 {
-   u8 val, msec;
-   const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
const struct dw_hdmi_mpll_config *mpll_config = pdata->mpll_cfg;
const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
const struct dw_hdmi_phy_config *phy_config = pdata->phy_config;

/* PLL/MPLL Cfg - always match on final entry */
for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
-   if (hdmi->hdmi_data.video_mode.mpixelclock <=
-   mpll_config->mpixelclock)
+   if (mpixelclock <= mpll_config->mpixelclock)
break;

for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
-   if (hdmi->hdmi_data.video_mode.mpixelclock <=
-   curr_ctrl->mpixelclock)
+   if (mpixelclock <= curr_ctrl->mpixelclock)
break;

for (; phy_config->mpixelclock != ~0UL; phy_config++)
-   if (hdmi->hdmi_data.video_mode.mpixelclock <=
-   phy_config->mpixelclock)
+   if (mpixelclock <= phy_config->mpixelclock)
break;

if (mpll_config->mpixelclock == ~0UL ||
curr_ctrl->mpixelclock == ~0UL ||
-   phy_config->mpixelclock == ~0UL) {
-   dev_err(hdmi->dev, "Pixel clock %d - unsupported by HDMI\n",
-   hdmi->hdmi_data.video_mode.mpixelclock);
+   phy_config->mpixelclock == ~0UL)
return -EINVAL;
-   }
+
+   dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[resolution].cpce, 0x06);
+   dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[resolution].gmp, 0x15);
+
+   /* CURRCTRL */
+   dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[resolution], 0x10);
+
+   dw_hdmi_phy_i2c_write(hdmi, 0x, 0x13); /* PLLPHBYCTRL */
+   dw_hdmi_phy_i2c_write(hdmi, 0x0006, 0x17);
+
+   dw_hdmi_phy_i2c_write(hdmi, phy_config->term, 0x19); /* TXTERM */
+   dw_hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr, 0x09); /* CKSYMTXCTRL 
*/
+   dw_hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr, 0x0E); /* VLEVCTRL */
+
+   /* REMOVE CLK TERM */
+   dw_hdmi_phy_i2c_write(hdmi, 0x8000, 0x05); /* CKCALCTRL */
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_configure_synopsys);
+
+static int hdmi_phy_configure(struct dw_hdmi *hdmi,
+ enum dw_hdmi_resolution resolution, int cscon)
+{
+   const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
+   unsigned long mpixelclock = hdmi->hdmi_data.video_mode.mpixelclock;
+   u8 val, msec;
+   int ret;

/* Enable csc path */
if