Hi Laurent,
On 01-12-2016 23:43, Laurent Pinchart wrote:
> From: Kieran Bingham <kieran.bingham+renesas at ideasonboard.com>
>
> 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 <kieran.bingham+renesas at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
> ---
> 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, 0x0000, 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 (cscon)
> @@ -988,21 +1012,14 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi,
> HDMI_PHY_I2CM_SLAVE_ADDR);
> hdmi_phy_test_clear(hdmi, 0);
>
> - hdmi_phy_i2c_write(hdmi, mpll_config->res[res_idx].cpce, 0x06);
> - hdmi_phy_i2c_write(hdmi, mpll_config->res[res_idx].gmp, 0x15);
> -
> - /* CURRCTRL */
> - hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[res_idx], 0x10);
> -
> - hdmi_phy_i2c_write(hdmi, 0x0000, 0x13); /* PLLPHBYCTRL */
> - hdmi_phy_i2c_write(hdmi, 0x0006, 0x17);
> -
> - hdmi_phy_i2c_write(hdmi, phy_config->term, 0x19); /* TXTERM */
> - hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr, 0x09); /* CKSYMTXCTRL */
> - hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr, 0x0E); /* VLEVCTRL */
> -
> - /* REMOVE CLK TERM */
> - hdmi_phy_i2c_write(hdmi, 0x8000, 0x05); /* CKCALCTRL */
> + /* Write to the PHY as configured by the platform */
> + ret = pdata->configure_phy(hdmi, pdata, mpixelclock, resolution);
> + if (ret) {
> + dev_err(hdmi->dev,
> + "PHY configuration failed (clock %lu resolution %u)\n",
> + mpixelclock, resolution);
> + return ret;
> + }
>
> dw_hdmi_phy_enable_powerdown(hdmi, false);
>
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index f645275e6e63..f1cb25c429cf 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -177,6 +177,7 @@ static struct dw_hdmi_plat_data imx6q_hdmi_drv_data = {
> .phy_config = imx_phy_config,
> .dev_type = IMX6Q_HDMI,
> .mode_valid = imx6q_hdmi_mode_valid,
> + .configure_phy = dw_hdmi_phy_configure_synopsys,
> };
>
> static struct dw_hdmi_plat_data imx6dl_hdmi_drv_data = {
> @@ -185,6 +186,7 @@ static struct dw_hdmi_plat_data imx6dl_hdmi_drv_data = {
> .phy_config = imx_phy_config,
> .dev_type = IMX6DL_HDMI,
> .mode_valid = imx6dl_hdmi_mode_valid,
> + .configure_phy = dw_hdmi_phy_configure_synopsys,
> };
>
> static const struct of_device_id dw_hdmi_imx_dt_ids[] = {
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index a6d4a0236e8f..55b1f2f27d6e 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -237,6 +237,7 @@ static const struct dw_hdmi_plat_data
> rockchip_hdmi_drv_data = {
> .mpll_cfg = rockchip_mpll_cfg,
> .cur_ctr = rockchip_cur_ctr,
> .phy_config = rockchip_phy_config,
> + .configure_phy = dw_hdmi_phy_configure_synopsys,
> .dev_type = RK3288_HDMI,
> };
>
> 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.
> enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> struct drm_display_mode *mode);
> };
> @@ -67,4 +71,12 @@ void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi,
> unsigned int rate);
> void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
> void dw_hdmi_audio_disable(struct dw_hdmi *hdmi);
>
> +/* PHY configuration */
> +void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned short data,
> + unsigned char addr);
> +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);
> +
> #endif /* __IMX_HDMI_H__ */
Best regards,
Jose Miguel Abreu
