Hi Jose,
On Tuesday 20 Dec 2016 11:39:21 Jose Abreu wrote:
> On 20-12-2016 01:33, Laurent Pinchart wrote:
> > Detect the PHY type and use it to handle the PHY type-specific SVSRET
> > signal.
> >
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas at ideasonboard.com>
> > ---
> >
> > drivers/gpu/drm/bridge/dw-hdmi.c | 68 +++++++++++++++++++++++++++++++++--
> > include/drm/bridge/dw_hdmi.h | 10 ++++++
> > 2 files changed, 75 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/dw-hdmi.c index f4faa14213e5..ef4f2f96ed2c
> > 100644
> > --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
[snip]
> > struct i2c_adapter *ddc;
> > @@ -1015,7 +1023,8 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi,
> > int cscon)>
> > dw_hdmi_phy_gen2_txpwron(hdmi, 1);
> > dw_hdmi_phy_gen2_pddq(hdmi, 0);
> >
> > - if (hdmi->dev_type == RK3288_HDMI)
> > + /* The DWC MHL and HDMI 2.0 PHYs need the SVSRET signal to be set. */
> > + if (hdmi->phy->has_svsret)
> > dw_hdmi_phy_enable_svsret(hdmi, 1);
>
> I didn't review the original code but according to the docs the
> svsret signal should be set before de-asserting phy reset.
I don't have access to the documentation so I can't comment on that :-) What
does the SVSRET signal control (and what does the name stand for) ?
By de-asserting PHY reset, do you mean
/* PHY reset */
hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_DEASSERT, HDMI_MC_PHYRSTZ);
hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_ASSERT, HDMI_MC_PHYRSTZ);
? HDMI_MC_PHYRSTZ_DEASSERT is defined as 0x01 and HDMI_MC_PHYRSTZ_ASSERT as
0x00, which I believe leads to correct operation on Gen2 PHYs, but is
incorrect on Gen1 PHYs that have an active low reset signal. Could you confirm
that ? The DEASSERT and ASSERT macros should be renamed as they're obviously
incorrect.
I can move the SVSRET assertion before PHY reset and test it on RK3288 and R-
Car H3.
> > /*Wait for PHY PLL lock */
> >
> > @@ -1840,6 +1849,54 @@ static irqreturn_t dw_hdmi_irq(int irq, void
> > *dev_id)
> > return IRQ_HANDLED;
> > }
> >
> > +static const struct dw_hdmi_phy_data dw_hdmi_phys[] = {
> > + {
> > + .type = DW_HDMI_PHY_DWC_HDMI_TX_PHY,
> > + .name = "DWC HDMI TX PHY",
> > + }, {
> > + .type = DW_HDMI_PHY_DWC_MHL_PHY_HEAC,
> > + .name = "DWC MHL PHY + HEAC PHY",
> > + .has_svsret = true,
> > + }, {
> > + .type = DW_HDMI_PHY_DWC_MHL_PHY,
> > + .name = "DWC MHL PHY",
> > + .has_svsret = true,
> > + }, {
> > + .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC,
> > + .name = "DWC HDMI 3D TX PHY + HEAC PHY",
> > + }, {
> > + .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY,
> > + .name = "DWC HDMI 3D TX PHY",
> > + }, {
> > + .type = DW_HDMI_PHY_DWC_HDMI20_TX_PHY,
> > + .name = "DWC HDMI 2.0 TX PHY",
> > + .has_svsret = true,
>
> Hmm, what I have here is that only MHL phys have svsret. Is this
> case for your phy?
The SoC datasheet mentions the SVSRET bit in the HDMI TX registers but doesn't
document it any further. If I don't set SVSRET the HDMI output stays dead, so
I assume I need to set it :-)
> > + }
> > +};
[snip]
--
Regards,
Laurent Pinchart
