[PATCH v5 07/10] drm: bridge: dw-hdmi: Add support for custom PHY configuration

郑阳 yang.zheng at rock-chips.com
Thu Apr 27 04:16:20 UTC 2017


Hi, Jose Abreu:

Thanks for your patch, it works fine on RK3399 / RK3368。

在 2017年04月26日 19:04, Jose Abreu 写道:
> Hi,
>
>
> On 24-04-2017 08:46, 郑阳 wrote:
>> Hi, Laurent Pinchart:
>>
>> 在 2017年03月04日 01:20, Laurent Pinchart 写道:
>>> From: Kieran Bingham <kieran.bingham+renesas at ideasonboard.com>
>>>
>>> The DWC HDMI TX controller interfaces with a companion PHY. While
>>> Synopsys provides multiple standard PHYs, SoC vendors can also
>>> integrate
>>> a custom PHY.
>>>
>>> Modularize PHY configuration to support vendor PHYs through
>>> platform
>>> data. The existing PHY configuration code was originally
>>> written to
>>> support the DWC HDMI 3D TX PHY, and seems to be compatible
>>> with the DWC
>>> MLP PHY. The HDMI 2.0 PHY will require a separate configuration
>>> function.
>>>
>>> Signed-off-by: Kieran Bingham
>>> <kieran.bingham+renesas at ideasonboard.com>
>>> Signed-off-by: Laurent Pinchart
>>> <laurent.pinchart+renesas at ideasonboard.com>
>>> Tested-by: Neil Armstrong <narmstrong at baylibre.com>
>>> Reviewed-by: Jose Abreu <Jose.Abreu at synopsys.com>
>>> ---
>>> Changes since v1:
>>>
>>> - Check pdata->phy_configure in hdmi_phy_configure() to avoid
>>>     dereferencing NULL pointer.
>>> ---
>>>    drivers/gpu/drm/bridge/dw-hdmi.c | 109
>>> ++++++++++++++++++++++++++-------------
>>>    include/drm/bridge/dw_hdmi.h     |   7 +++
>>>    2 files changed, 81 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
>>> b/drivers/gpu/drm/bridge/dw-hdmi.c
>>> index cb2703862be2..b835d81bb471 100644
>>> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
>>> @@ -118,6 +118,9 @@ struct dw_hdmi_phy_data {
>>>        const char *name;
>>>        unsigned int gen;
>>>        bool has_svsret;
>>> +    int (*configure)(struct dw_hdmi *hdmi,
>>> +             const struct dw_hdmi_plat_data *pdata,
>>> +             unsigned long mpixelclock);
>>>    };
>>>      struct dw_hdmi {
>>> @@ -860,8 +863,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);
>>> @@ -873,6 +876,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)
>>>    {
>>> @@ -993,37 +997,67 @@ static int dw_hdmi_phy_power_on(struct
>>> dw_hdmi *hdmi)
>>>        return 0;
>>>    }
>>>    -static int hdmi_phy_configure(struct dw_hdmi *hdmi)
>>> +/*
>>> + * PHY configuration function for the DWC HDMI 3D TX PHY.
>>> Based on the available
>>> + * information the DWC MHL PHY has the same register layout
>>> and is thus also
>>> + * supported by this function.
>>> + */
>>> +static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct dw_hdmi
>>> *hdmi,
>>> +        const struct dw_hdmi_plat_data *pdata,
>>> +        unsigned long mpixelclock)
>>>    {
>>> -    const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
>>> -    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[0].cpce,
>>> +                  HDMI_3D_TX_PHY_CPCE_CTRL);
>>> +    dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
>>> +                  HDMI_3D_TX_PHY_GMPCTRL);
>>> +    dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
>>> +                  HDMI_3D_TX_PHY_CURRCTRL);
>>> +
>>> +    dw_hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL);
>>> +    dw_hdmi_phy_i2c_write(hdmi,
>>> HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK,
>>> +                  HDMI_3D_TX_PHY_MSM_CTRL);
>>> +
>>> +    dw_hdmi_phy_i2c_write(hdmi, phy_config->term,
>>> HDMI_3D_TX_PHY_TXTERM);
>>> +    dw_hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr,
>>> +                  HDMI_3D_TX_PHY_CKSYMTXCTRL);
>>> +    dw_hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr,
>>> +                  HDMI_3D_TX_PHY_VLEVCTRL);
>>> +
>>> +    /* Override and disable clock termination. */
>>> +    dw_hdmi_phy_i2c_write(hdmi,
>>> HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE,
>>> +                  HDMI_3D_TX_PHY_CKCALCTRL);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int hdmi_phy_configure(struct dw_hdmi *hdmi)
>>> +{
>>> +    const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
>>> +    const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
>>> +    unsigned long mpixelclock =
>>> hdmi->hdmi_data.video_mode.mpixelclock;
>>> +    int ret;
>>>          dw_hdmi_phy_power_off(hdmi);
>>>    @@ -1042,26 +1076,16 @@ 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[0].cpce,
>>> -               HDMI_3D_TX_PHY_CPCE_CTRL);
>>> -    hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
>>> -               HDMI_3D_TX_PHY_GMPCTRL);
>>> -    hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
>>> -               HDMI_3D_TX_PHY_CURRCTRL);
>>> -
>>> -    hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL);
>>> -    hdmi_phy_i2c_write(hdmi,
>>> HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK,
>>> -               HDMI_3D_TX_PHY_MSM_CTRL);
>>> -
>>> -    hdmi_phy_i2c_write(hdmi, phy_config->term,
>>> HDMI_3D_TX_PHY_TXTERM);
>>> -    hdmi_phy_i2c_write(hdmi, phy_config->sym_ctr,
>>> -               HDMI_3D_TX_PHY_CKSYMTXCTRL);
>>> -    hdmi_phy_i2c_write(hdmi, phy_config->vlev_ctr,
>>> -               HDMI_3D_TX_PHY_VLEVCTRL);
>>> -
>>> -    /* Override and disable clock termination. */
>>> -    hdmi_phy_i2c_write(hdmi, HDMI_3D_TX_PHY_CKCALCTRL_OVERRIDE,
>>> -               HDMI_3D_TX_PHY_CKCALCTRL);
>>> +    /* Write to the PHY as configured by the platform */
>>> +    if (pdata->configure_phy)
>>> +        ret = pdata->configure_phy(hdmi, pdata, mpixelclock);
>>> +    else
>>> +        ret = phy->configure(hdmi, pdata, mpixelclock);
>>> +    if (ret) {
>>> +        dev_err(hdmi->dev, "PHY configuration failed (clock
>>> %lu)\n",
>>> +            mpixelclock);
>>> +        return ret;
>>> +    }
>>>          return dw_hdmi_phy_power_on(hdmi);
>>>    }
>>> @@ -1895,24 +1919,31 @@ static const struct dw_hdmi_phy_data
>>> dw_hdmi_phys[] = {
>>>            .name = "DWC MHL PHY + HEAC PHY",
>>>            .gen = 2,
>>>            .has_svsret = true,
>>> +        .configure = hdmi_phy_configure_dwc_hdmi_3d_tx,
>>>        }, {
>>>            .type = DW_HDMI_PHY_DWC_MHL_PHY,
>>>            .name = "DWC MHL PHY",
>>>            .gen = 2,
>>>            .has_svsret = true,
>>> +        .configure = hdmi_phy_configure_dwc_hdmi_3d_tx,
>>>        }, {
>>>            .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC,
>>>            .name = "DWC HDMI 3D TX PHY + HEAC PHY",
>>>            .gen = 2,
>>> +        .configure = hdmi_phy_configure_dwc_hdmi_3d_tx,
>>>        }, {
>>>            .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY,
>>>            .name = "DWC HDMI 3D TX PHY",
>>>            .gen = 2,
>>> +        .configure = hdmi_phy_configure_dwc_hdmi_3d_tx,
>>>        }, {
>>>            .type = DW_HDMI_PHY_DWC_HDMI20_TX_PHY,
>>>            .name = "DWC HDMI 2.0 TX PHY",
>>>            .gen = 2,
>>>            .has_svsret = true,
>> After this commit, the dw-hdmi on RK3368/RK3399 run into the
>> "DWC HDMI 2.0 TX PHY requires platform support error".
>> the phy-type of RK3368/RK3399 is 0xf3, and phy register layout is
>> compatible with DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC.
>> Is here missing a default phy configure function?
> If this has the same register layout then this patch should solve
> the problem:
>
> ----------------------------------------------------------------
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 4e1f54a..ff96864 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2159,6 +2159,7 @@ static irqreturn_t dw_hdmi_irq(int irq,
> void *dev_id)
>                  .name = "DWC HDMI 2.0 TX PHY",
>                  .gen = 2,
>                  .has_svsret = true,
> +               .configure = hdmi_phy_configure_dwc_hdmi_3d_tx,
>          }, {
>                  .type = DW_HDMI_PHY_VENDOR_PHY,
>                  .name = "Vendor PHY",
> ----------------------------------------------------------------
>
> Please test and let me know if its ok. This shouldn't impact
> other platforms which supply custom configuration function by
> pdata as the check for pdata configure() is done before checking
> the internal configure().
>
> Best regards,
> Jose Miguel Abreu
>
>>> +    }, {
>>> +        .type = DW_HDMI_PHY_VENDOR_PHY,
>>> +        .name = "Vendor PHY",
>>>        }
>>>    };
>>>    @@ -1943,6 +1974,14 @@ static int dw_hdmi_detect_phy(struct
>>> dw_hdmi *hdmi)
>>>                hdmi->phy.ops = &dw_hdmi_synopsys_phy_ops;
>>>                hdmi->phy.name = dw_hdmi_phys[i].name;
>>>                hdmi->phy.data = (void *)&dw_hdmi_phys[i];
>>> +
>>> +            if (!dw_hdmi_phys[i].configure &&
>>> +                !hdmi->plat_data->configure_phy) {
>>> +                dev_err(hdmi->dev, "%s requires platform
>>> support\n",
>>> +                    hdmi->phy.name);
>>> +                return -ENODEV;
>>> +            }
>>> +
>>>                return 0;
>>>            }
>>>        }
>>> diff --git a/include/drm/bridge/dw_hdmi.h
>>> b/include/drm/bridge/dw_hdmi.h
>>> index 0f583ca7e66e..dd330259a3dc 100644
>>> --- a/include/drm/bridge/dw_hdmi.h
>>> +++ b/include/drm/bridge/dw_hdmi.h
>>> @@ -78,6 +78,9 @@ 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);
>>>    };
>>>      int dw_hdmi_probe(struct platform_device *pdev,
>>> @@ -91,4 +94,8 @@ 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);
>>> +
>>>    #endif /* __IMX_HDMI_H__ */
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=cjy3Og8_l5PxS4qa2_RekJB_Bi_J4sVXR1zH24rY7ng&s=bH0i9klWoHnYe5d9wM4Ei2u_2bkGUeZJZV5J7BsBCqE&e=
>
>
>
>




More information about the dri-devel mailing list