| Message ID | 20161220013400.28317-18-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > Instead of spreading version-dependent PHY power handling code around, > group it in two functions to power the PHY on and off and use them > through the driver. > > Powering off the PHY at the beginning of the setup phase is currently > split in two locations for first and second generation PHYs, group all > the operations in the dw_hdmi_phy_init() function. This changes the behaviour of the driver. > +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > +{ > + if (hdmi->phy->gen == 1) { > + dw_hdmi_phy_enable_tmds(hdmi, 0); > + dw_hdmi_phy_enable_powerdown(hdmi, true); > + } else { > + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > + dw_hdmi_phy_gen2_pddq(hdmi, 1); > + } > +} > @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi) > if (!hdmi->phy_enabled) > return; > > - dw_hdmi_phy_enable_tmds(hdmi, 0); > - dw_hdmi_phy_enable_powerdown(hdmi, true); > - > + dw_hdmi_phy_power_off(hdmi); This makes dw_hdmi_phy_disable() power down a gen2 phy. The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a gen2 phy. I've been carrying this change for a while, which I've had to revert (and finally expunge), as it causes problems on iMX6: @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi) if (!hdmi->phy_enabled) return; + /* Actually set the phy into low power mode */ + dw_hdmi_phy_gen2_txpwron(hdmi, 0); + + /* FIXME: We should wait for TX_READY to be deasserted */ + + dw_hdmi_phy_gen2_pddq(hdmi, 1); + + /* This appears to have no effect on iMX6 */ dw_hdmi_phy_enable_tmds(hdmi, 0); dw_hdmi_phy_enable_powerdown(hdmi, true); So, I think your change here will cause problems for iMX6. From what I remember, it seems that iMX6 has issues with RXSENSE/HPD bouncing when the PHY is powered down. I can't promise when I'll be able to check for that again.
Hi Russell, On 20-12-2016 11:45, Russell King - ARM Linux wrote: > On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: >> Instead of spreading version-dependent PHY power handling code around, >> group it in two functions to power the PHY on and off and use them >> through the driver. >> >> Powering off the PHY at the beginning of the setup phase is currently >> split in two locations for first and second generation PHYs, group all >> the operations in the dw_hdmi_phy_init() function. > This changes the behaviour of the driver. > >> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) >> +{ >> + if (hdmi->phy->gen == 1) { >> + dw_hdmi_phy_enable_tmds(hdmi, 0); >> + dw_hdmi_phy_enable_powerdown(hdmi, true); >> + } else { >> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); >> + dw_hdmi_phy_gen2_pddq(hdmi, 1); >> + } >> +} >> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi) >> if (!hdmi->phy_enabled) >> return; >> >> - dw_hdmi_phy_enable_tmds(hdmi, 0); >> - dw_hdmi_phy_enable_powerdown(hdmi, true); >> - >> + dw_hdmi_phy_power_off(hdmi); > This makes dw_hdmi_phy_disable() power down a gen2 phy. > > The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > gen2 phy. I've been carrying this change for a while, which I've had > to revert (and finally expunge), as it causes problems on iMX6: > > @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi) > if (!hdmi->phy_enabled) > return; > > + /* Actually set the phy into low power mode */ > + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > + > + /* FIXME: We should wait for TX_READY to be deasserted */ > + > + dw_hdmi_phy_gen2_pddq(hdmi, 1); > + > + /* This appears to have no effect on iMX6 */ > dw_hdmi_phy_enable_tmds(hdmi, 0); > dw_hdmi_phy_enable_powerdown(hdmi, true); > > So, I think your change here will cause problems for iMX6. > > From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > bouncing when the PHY is powered down. I can't promise when I'll be > able to check for that again. > Indeed TX_READY must be low before asserting pddq. HPD and RXSENSE should work in power down mode by enabling enhpdrxsense bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, enhpdrxsense and enable PDDQ and PHY _RESET. Only after doing this (and consequently entering power down mode) you can enable again enhpdrxsense. Best regards, Jose Miguel Abreu
Hi Russell, Thank you for the review. On Tuesday 20 Dec 2016 11:45:53 Russell King - ARM Linux wrote: > On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > > Instead of spreading version-dependent PHY power handling code around, > > group it in two functions to power the PHY on and off and use them > > through the driver. > > > > Powering off the PHY at the beginning of the setup phase is currently > > split in two locations for first and second generation PHYs, group all > > the operations in the dw_hdmi_phy_init() function. > > This changes the behaviour of the driver. It does, but slightly only (I'm of course very aware that slightly can easily be way too much :-)). Let's analyse the differences, starting with PHY initialization in dw_hdmi_phy_init(). Before this patch, the function calls for (i = 0; i < 2; i++) { dw_hdmi_phy_sel_data_en_pol(hdmi, 1); dw_hdmi_phy_sel_interface_control(hdmi, 0); dw_hdmi_phy_enable_tmds(hdmi, 0); dw_hdmi_phy_enable_powerdown(hdmi, true); /* Enable CSC */ ret = hdmi_phy_configure(hdmi, cscon); if (ret) return ret; } and hdmi_phy_configure() starts with (I've removed lines that don't touch the hardware) /* Enable csc path */ if (cscon) val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH; else val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS; hdmi_writeb(hdmi, val, HDMI_MC_FLOWCTRL); /* gen2 tx power off */ dw_hdmi_phy_gen2_txpwron(hdmi, 0); /* gen2 pddq */ dw_hdmi_phy_gen2_pddq(hdmi, 1); After the change, dw_hdmi_phy_init() calls for (i = 0; i < 2; i++) { dw_hdmi_phy_sel_data_en_pol(hdmi, 1); dw_hdmi_phy_sel_interface_control(hdmi, 0); /* Enable CSC */ ret = hdmi_phy_configure(hdmi, cscon); if (ret) return ret; } and hdmi_phy_configure() starts with /* Enable csc path */ if (cscon) val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH; else val = HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_BYPASS; hdmi_writeb(hdmi, val, HDMI_MC_FLOWCTRL); dw_hdmi_phy_power_off(hdmi); with dw_hdmi_phy_power_off() defined as if (hdmi->phy->gen == 1) { dw_hdmi_phy_enable_tmds(hdmi, 0); dw_hdmi_phy_enable_powerdown(hdmi, true); } else { dw_hdmi_phy_gen2_txpwron(hdmi, 0); dw_hdmi_phy_gen2_pddq(hdmi, 1); } This patch thus changes the behaviour in two ways: - Move the dw_hdmi_phy_enable_tmds() and dw_hdmi_phy_enable_powerdown() calls after CSC configuration (HDMI_MC_FLOWCTRL). - Only touch the power bits related to the PHY generation. I believe the first change to be harmless. Furthermore, given that the i.MX6 and Rockchip SoCs use a Gen2 PHY, the TMDS and POWERDOWN bits should be no- ops, so it should have no effect at all on those platforms. I also believe the second change to be harmless as the TMDS and POWERDOWN bits should be no-ops on i.MX6 and Rockchip. I have tested this patch series on RK3288 and i.MX6Q and haven't noticed any regression. The power on path should similarly be fine too, as the change - dw_hdmi_phy_enable_powerdown(hdmi, false); - - /* toggle TMDS enable */ - dw_hdmi_phy_enable_tmds(hdmi, 0); - dw_hdmi_phy_enable_tmds(hdmi, 1); - - /* gen2 tx power on */ - dw_hdmi_phy_gen2_txpwron(hdmi, 1); - dw_hdmi_phy_gen2_pddq(hdmi, 0); + dw_hdmi_phy_power_on(hdmi); results in static void dw_hdmi_phy_power_on(struct dw_hdmi *hdmi) { if (hdmi->phy->gen == 1) { dw_hdmi_phy_enable_powerdown(hdmi, false); /* Toggle TMDS enable. */ dw_hdmi_phy_enable_tmds(hdmi, 0); dw_hdmi_phy_enable_tmds(hdmi, 1); } else { dw_hdmi_phy_gen2_txpwron(hdmi, 1); dw_hdmi_phy_gen2_pddq(hdmi, 0); } } which splits the Gen1/Gen2 code but doesn't reorder anything. The last change is in dw_hdmi_phy_disable(), see below. > > +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > > +{ > > + if (hdmi->phy->gen == 1) { > > + dw_hdmi_phy_enable_tmds(hdmi, 0); > > + dw_hdmi_phy_enable_powerdown(hdmi, true); > > + } else { > > + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > > + dw_hdmi_phy_gen2_pddq(hdmi, 1); > > + } > > +} > > > > @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > > *hdmi) > > if (!hdmi->phy_enabled) > > return; > > > > - dw_hdmi_phy_enable_tmds(hdmi, 0); > > - dw_hdmi_phy_enable_powerdown(hdmi, true); > > - > > + dw_hdmi_phy_power_off(hdmi); > > This makes dw_hdmi_phy_disable() power down a gen2 phy. That's correct. > The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > gen2 phy. To the best of my knowledge, based on the documentation I've been able to gather, the information I've received from different developers and the tests I've performed so far, this is the case. > I've been carrying this change for a while, which I've had > to revert (and finally expunge), as it causes problems on iMX6: > > @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi) > if (!hdmi->phy_enabled) > return; > > + /* Actually set the phy into low power mode */ > + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > + > + /* FIXME: We should wait for TX_READY to be deasserted */ > + > + dw_hdmi_phy_gen2_pddq(hdmi, 1); > + > + /* This appears to have no effect on iMX6 */ > dw_hdmi_phy_enable_tmds(hdmi, 0); > dw_hdmi_phy_enable_powerdown(hdmi, true); > > So, I think your change here will cause problems for iMX6. > > From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > bouncing when the PHY is powered down. I can't promise when I'll be > able to check for that again. The current code is supposed to power the PHY down in dw_hdmi_phy_disable() but doesn't do so as it only handles Gen1 PHY signals there. Gen2 PHYs thus always stay enabled. If this causes an issue on i.MX6 related to RXSENSE and HPD, I think it should be handled by explicitly skipping power down in dw_hdmi_phy_disable() instead of pretending we power the PHY down. This could be achieved through a quirk flag if the issue is specific to i.MX6, or based on the PHY model if the issue is present in all DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC PHYs. Jose, would you happen to be aware of RXSENSE/HPD issues when the PHY is disabled ? If there's an easy way to reproduce the problem on i.MX6Q I can try locally. I unfortunately don't have access to i.MX6DL.
Hi Jose, On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: > On 20-12-2016 11:45, Russell King - ARM Linux wrote: > > On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > >> Instead of spreading version-dependent PHY power handling code around, > >> group it in two functions to power the PHY on and off and use them > >> through the driver. > >> > >> Powering off the PHY at the beginning of the setup phase is currently > >> split in two locations for first and second generation PHYs, group all > >> the operations in the dw_hdmi_phy_init() function. > > > > This changes the behaviour of the driver. > > > >> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > >> +{ > >> + if (hdmi->phy->gen == 1) { > >> + dw_hdmi_phy_enable_tmds(hdmi, 0); > >> + dw_hdmi_phy_enable_powerdown(hdmi, true); > >> + } else { > >> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >> + } > >> +} > >> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >> *hdmi) > >> > >> if (!hdmi->phy_enabled) > >> > >> return; > >> > >> - dw_hdmi_phy_enable_tmds(hdmi, 0); > >> - dw_hdmi_phy_enable_powerdown(hdmi, true); > >> - > >> + dw_hdmi_phy_power_off(hdmi); > > > > This makes dw_hdmi_phy_disable() power down a gen2 phy. > > > > The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > > gen2 phy. I've been carrying this change for a while, which I've had > > to revert (and finally expunge), as it causes problems on iMX6: > > > > @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > > *hdmi)> > > if (!hdmi->phy_enabled) > > > > return; > > > > + /* Actually set the phy into low power mode */ > > + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > > + > > + /* FIXME: We should wait for TX_READY to be deasserted */ > > + > > + dw_hdmi_phy_gen2_pddq(hdmi, 1); > > + > > + /* This appears to have no effect on iMX6 */ > > > > dw_hdmi_phy_enable_tmds(hdmi, 0); > > dw_hdmi_phy_enable_powerdown(hdmi, true); > > > > So, I think your change here will cause problems for iMX6. > > > > From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > > bouncing when the PHY is powered down. I can't promise when I'll be > > able to check for that again. > > Indeed TX_READY must be low before asserting pddq. The TX_READY signal is documented in the i.MX6 datasheet as being a PHY output signal, but there seems to be no HDMI TX register from which its state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register through I2C ? How long is the PHY expected to take to set TX_READY to 0 ? > HPD and RXSENSE should work in power down mode by enabling enhpdrxsense > bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, > enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and > consequently entering power down mode) you can enable again enhpdrxsense. Thanks, I'll give it a try.
Hi Laurent, On 05-01-2017 12:29, Laurent Pinchart wrote: > Hi Jose, > > On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: >> On 20-12-2016 11:45, Russell King - ARM Linux wrote: >>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: >>>> Instead of spreading version-dependent PHY power handling code around, >>>> group it in two functions to power the PHY on and off and use them >>>> through the driver. >>>> >>>> Powering off the PHY at the beginning of the setup phase is currently >>>> split in two locations for first and second generation PHYs, group all >>>> the operations in the dw_hdmi_phy_init() function. >>> This changes the behaviour of the driver. >>> >>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) >>>> +{ >>>> + if (hdmi->phy->gen == 1) { >>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); >>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); >>>> + } else { >>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); >>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); >>>> + } >>>> +} >>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi >>>> *hdmi) >>>> >>>> if (!hdmi->phy_enabled) >>>> >>>> return; >>>> >>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); >>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); >>>> - >>>> + dw_hdmi_phy_power_off(hdmi); >>> This makes dw_hdmi_phy_disable() power down a gen2 phy. >>> >>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a >>> gen2 phy. I've been carrying this change for a while, which I've had >>> to revert (and finally expunge), as it causes problems on iMX6: >>> >>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi >>> *hdmi)> >>> if (!hdmi->phy_enabled) >>> >>> return; >>> >>> + /* Actually set the phy into low power mode */ >>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); >>> + >>> + /* FIXME: We should wait for TX_READY to be deasserted */ >>> + >>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); >>> + >>> + /* This appears to have no effect on iMX6 */ >>> >>> dw_hdmi_phy_enable_tmds(hdmi, 0); >>> dw_hdmi_phy_enable_powerdown(hdmi, true); >>> >>> So, I think your change here will cause problems for iMX6. >>> >>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD >>> bouncing when the PHY is powered down. I can't promise when I'll be >>> able to check for that again. >> Indeed TX_READY must be low before asserting pddq. > The TX_READY signal is documented in the i.MX6 datasheet as being a PHY output > signal, but there seems to be no HDMI TX register from which its state can be > read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register through I2C ? How > long is the PHY expected to take to set TX_READY to 0 ? TX_READY can be read from register 0x1A of phy, BIT(2) (through I2C). Not sure if same offset for all phys though. Best regards, Jose Miguel Abreu > >> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense >> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, >> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and >> consequently entering power down mode) you can enable again enhpdrxsense. > Thanks, I'll give it a try. >
Hi Jose, On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote: > On 05-01-2017 12:29, Laurent Pinchart wrote: > > On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: > >> On 20-12-2016 11:45, Russell King - ARM Linux wrote: > >>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > >>>> Instead of spreading version-dependent PHY power handling code around, > >>>> group it in two functions to power the PHY on and off and use them > >>>> through the driver. > >>>> > >>>> Powering off the PHY at the beginning of the setup phase is currently > >>>> split in two locations for first and second generation PHYs, group all > >>>> the operations in the dw_hdmi_phy_init() function. > >>> > >>> This changes the behaviour of the driver. > >>> > >>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > >>>> +{ > >>>> + if (hdmi->phy->gen == 1) { > >>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>> + } else { > >>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>> + } > >>>> +} > >>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>> *hdmi) > >>>> > >>>> if (!hdmi->phy_enabled) > >>>> > >>>> return; > >>>> > >>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>> - > >>>> + dw_hdmi_phy_power_off(hdmi); > >>> > >>> This makes dw_hdmi_phy_disable() power down a gen2 phy. > >>> > >>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > >>> gen2 phy. I've been carrying this change for a while, which I've had > >>> to revert (and finally expunge), as it causes problems on iMX6: > >>> > >>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>> *hdmi)> > >>> > >>> if (!hdmi->phy_enabled) > >>> > >>> return; > >>> > >>> + /* Actually set the phy into low power mode */ > >>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>> + > >>> + /* FIXME: We should wait for TX_READY to be deasserted */ > >>> + > >>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>> + > >>> + /* This appears to have no effect on iMX6 */ > >>> > >>> dw_hdmi_phy_enable_tmds(hdmi, 0); > >>> dw_hdmi_phy_enable_powerdown(hdmi, true); > >>> > >>> So, I think your change here will cause problems for iMX6. > >>> > >>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > >>> bouncing when the PHY is powered down. I can't promise when I'll be > >>> able to check for that again. > >> > >> Indeed TX_READY must be low before asserting pddq. > > > > The TX_READY signal is documented in the i.MX6 datasheet as being a PHY > > output signal, but there seems to be no HDMI TX register from which its > > state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register > > through I2C ? How long is the PHY expected to take to set TX_READY to 0 ? > > TX_READY can be read from register 0x1A of phy, BIT(2) (through > I2C). That's what I thought, I'll poll that then. Do you have any idea how long it's supposed to take, to set an appropriate timeout ? > Not sure if same offset for all phys though. Most probably not, it would be too easy :-) I'll investigate (which will likely include lots of guesswork). If you can find any information about that (and especially about the MHL and HDMI 2.0 PHYs) that would be very appreciated, as I don't have access to any documentation that mentions a TX_READY bit for those.
Hi Jose, On Thursday 05 Jan 2017 17:33:58 Laurent Pinchart wrote: > On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote: > > On 05-01-2017 12:29, Laurent Pinchart wrote: > >> On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: > >>> On 20-12-2016 11:45, Russell King - ARM Linux wrote: > >>>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > >>>>> Instead of spreading version-dependent PHY power handling code > >>>>> around, group it in two functions to power the PHY on and off and use > >>>>> them through the driver. > >>>>> > >>>>> Powering off the PHY at the beginning of the setup phase is currently > >>>>> split in two locations for first and second generation PHYs, group > >>>>> all the operations in the dw_hdmi_phy_init() function. > >>>> > >>>> This changes the behaviour of the driver. > >>>> > >>>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > >>>>> +{ > >>>>> + if (hdmi->phy->gen == 1) { > >>>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>> + } else { > >>>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>>> + } > >>>>> +} > >>>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>>> *hdmi) > >>>>> if (!hdmi->phy_enabled) > >>>>> return; > >>>>> > >>>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>> - > >>>>> + dw_hdmi_phy_power_off(hdmi); > >>>> > >>>> This makes dw_hdmi_phy_disable() power down a gen2 phy. > >>>> > >>>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > >>>> gen2 phy. I've been carrying this change for a while, which I've had > >>>> to revert (and finally expunge), as it causes problems on iMX6: > >>>> > >>>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>> *hdmi) > >>>> if (!hdmi->phy_enabled) > >>>> return; > >>>> > >>>> + /* Actually set the phy into low power mode */ > >>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>> + > >>>> + /* FIXME: We should wait for TX_READY to be deasserted */ > >>>> + > >>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>> + > >>>> + /* This appears to have no effect on iMX6 */ > >>>> dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>> dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>> > >>>> So, I think your change here will cause problems for iMX6. > >>>> > >>>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > >>>> bouncing when the PHY is powered down. I can't promise when I'll be > >>>> able to check for that again. > >>> > >>> Indeed TX_READY must be low before asserting pddq. > >> > >> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY > >> output signal, but there seems to be no HDMI TX register from which its > >> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register > >> through I2C ? How long is the PHY expected to take to set TX_READY to 0 > >> ? > > > > TX_READY can be read from register 0x1A of phy, BIT(2) (through > > I2C). > > That's what I thought, I'll poll that then. Do you have any idea how long > it's supposed to take, to set an appropriate timeout ? On i.MX6 (3D TX PHY) register 0x1a reads as 0x0007 before powering down the PHY (by deasserting TXPWRON) and as 0x0000 immediately after. On RK3288 (MHL PHY) the register reads as 0x0207 and as 0x0000 immediately after deasserting TXPWRON. It seems that one I2C read is a sufficient delay for TX_READY to go low. > > Not sure if same offset for all phys though. > > Most probably not, it would be too easy :-) I'll investigate (which will > likely include lots of guesswork). If you can find any information about > that (and especially about the MHL and HDMI 2.0 PHYs) that would be very > appreciated, as I don't have access to any documentation that mentions a > TX_READY bit for those. I haven't tested the HDMI 2.0 PHY yet, but I'd be surprised if the TX_READY bit was in the same register at the same position.
Hi Laurent, Sorry for the delayed answer but I am quite busy at the moment. On 06-01-2017 01:48, Laurent Pinchart wrote: [snip] >>>> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY >>>> output signal, but there seems to be no HDMI TX register from which its >>>> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register >>>> through I2C ? How long is the PHY expected to take to set TX_READY to 0 >>>> ? >>> TX_READY can be read from register 0x1A of phy, BIT(2) (through >>> I2C). >> That's what I thought, I'll poll that then. Do you have any idea how long >> it's supposed to take, to set an appropriate timeout ? For 3d tx phy and for 25 MHz input reference clock the power-up time is ~1ms, there is no data in the docs to power-down time but it should be similar. Reference clock value is board dependent and the minimum value for HDMI shall be 13.5MHz. > On i.MX6 (3D TX PHY) register 0x1a reads as 0x0007 before powering down the > PHY (by deasserting TXPWRON) and as 0x0000 immediately after. On RK3288 (MHL > PHY) the register reads as 0x0207 and as 0x0000 immediately after deasserting > TXPWRON. It seems that one I2C read is a sufficient delay for TX_READY to go > low. >>> Not sure if same offset for all phys though. >> Most probably not, it would be too easy :-) I'll investigate (which will >> likely include lots of guesswork). If you can find any information about >> that (and especially about the MHL and HDMI 2.0 PHYs) that would be very >> appreciated, as I don't have access to any documentation that mentions a >> TX_READY bit for those. > I haven't tested the HDMI 2.0 PHY yet, but I'd be surprised if the TX_READY > bit was in the same register at the same position. > The info I got the register offset is from an HDMI 2.0 phy :) Notice that there are a lot of phy versions and some of them (used in dw-hdmi) maybe customized, I don't think I have access to that custom phys documentation. Please test the HDMI 2.0 phy and check if the register is the same, it should be. In the meantime it would really be helpful if some of the developers that used dw-hdmi supplied this info about the registers as they should know exactly what phy was used. Best regards, Jose Miguel Abreu
Hi Jose, On Friday 06 Jan 2017 10:07:03 Jose Abreu wrote: > Hi Laurent, > > Sorry for the delayed answer but I am quite busy at the moment. No worries, your help is really appreciated. > On 06-01-2017 01:48, Laurent Pinchart wrote: > > [snip] > > >>>> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY > >>>> output signal, but there seems to be no HDMI TX register from which its > >>>> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register > >>>> through I2C ? How long is the PHY expected to take to set TX_READY to 0 > >>>> ? > >>> > >>> TX_READY can be read from register 0x1A of phy, BIT(2) (through > >>> I2C). > >> > >> That's what I thought, I'll poll that then. Do you have any idea how long > >> it's supposed to take, to set an appropriate timeout ? > > For 3d tx phy and for 25 MHz input reference clock the power-up > time is ~1ms, there is no data in the docs to power-down time but > it should be similar. Reference clock value is board dependent > and the minimum value for HDMI shall be 13.5MHz. > > > On i.MX6 (3D TX PHY) register 0x1a reads as 0x0007 before powering down > > the PHY (by deasserting TXPWRON) and as 0x0000 immediately after. On > > RK3288 (MHL PHY) the register reads as 0x0207 and as 0x0000 immediately > > after deasserting TXPWRON. It seems that one I2C read is a sufficient > > delay for TX_READY to go low. > > > >>> Not sure if same offset for all phys though. > >> > >> Most probably not, it would be too easy :-) I'll investigate (which will > >> likely include lots of guesswork). If you can find any information about > >> that (and especially about the MHL and HDMI 2.0 PHYs) that would be very > >> appreciated, as I don't have access to any documentation that mentions a > >> TX_READY bit for those. > > > > I haven't tested the HDMI 2.0 PHY yet, but I'd be surprised if the > > TX_READY bit was in the same register at the same position. > > The info I got the register offset is from an HDMI 2.0 phy :) That's good news :-) > Notice that there are a lot of phy versions and some of them > (used in dw-hdmi) maybe customized, I don't think I have access > to that custom phys documentation. I think we will eventually have to implement PHY-specific power up and power down sequences, but for now a common sequence should work. > Please test the HDMI 2.0 phy and check if the register is the same, it > should be. I did, and it is \o/ I'll send patches. > In the meantime it would really be helpful if some of the developers > that used dw-hdmi supplied this info about the registers as they > should know exactly what phy was used. I will ask internally for the R-Car H3 SoC.
Hi Jose, On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote: > On 05-01-2017 12:29, Laurent Pinchart wrote: > > On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: > >> On 20-12-2016 11:45, Russell King - ARM Linux wrote: > >>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > >>>> Instead of spreading version-dependent PHY power handling code around, > >>>> group it in two functions to power the PHY on and off and use them > >>>> through the driver. > >>>> > >>>> Powering off the PHY at the beginning of the setup phase is currently > >>>> split in two locations for first and second generation PHYs, group all > >>>> the operations in the dw_hdmi_phy_init() function. > >>> > >>> This changes the behaviour of the driver. > >>> > >>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > >>>> +{ > >>>> + if (hdmi->phy->gen == 1) { > >>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>> + } else { > >>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>> + } > >>>> +} > >>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>> *hdmi) > >>>> if (!hdmi->phy_enabled) > >>>> return; > >>>> > >>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>> - > >>>> + dw_hdmi_phy_power_off(hdmi); > >>> > >>> This makes dw_hdmi_phy_disable() power down a gen2 phy. > >>> > >>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > >>> gen2 phy. I've been carrying this change for a while, which I've had > >>> to revert (and finally expunge), as it causes problems on iMX6: > >>> > >>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>> *hdmi) > >>> if (!hdmi->phy_enabled) > >>> return; > >>> > >>> + /* Actually set the phy into low power mode */ > >>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>> + > >>> + /* FIXME: We should wait for TX_READY to be deasserted */ > >>> + > >>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>> + > >>> + /* This appears to have no effect on iMX6 */ > >>> dw_hdmi_phy_enable_tmds(hdmi, 0); > >>> dw_hdmi_phy_enable_powerdown(hdmi, true); > >>> > >>> So, I think your change here will cause problems for iMX6. > >>> > >>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > >>> bouncing when the PHY is powered down. I can't promise when I'll be > >>> able to check for that again. > >> > >> Indeed TX_READY must be low before asserting pddq. > > > > The TX_READY signal is documented in the i.MX6 datasheet as being a PHY > > output signal, but there seems to be no HDMI TX register from which its > > state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register > > through I2C ? How long is the PHY expected to take to set TX_READY to 0 ? > > TX_READY can be read from register 0x1A of phy, BIT(2) (through > I2C). Not sure if same offset for all phys though. I have been told that another option is to poll the TX_PHY_LOCK bit in the phy_stat0 register. That would be a simpler solution that doesn't require I2C access. Could you confirm (or disconfirm) this ? > >> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense > >> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, > >> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and > >> consequently entering power down mode) you can enable again enhpdrxsense. > > > > Thanks, I'll give it a try.
Hi Laurent, On 01-03-2017 11:09, Laurent Pinchart wrote: > Hi Jose, > > On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote: >> On 05-01-2017 12:29, Laurent Pinchart wrote: >>> On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: >>>> On 20-12-2016 11:45, Russell King - ARM Linux wrote: >>>>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: >>>>>> Instead of spreading version-dependent PHY power handling code around, >>>>>> group it in two functions to power the PHY on and off and use them >>>>>> through the driver. >>>>>> >>>>>> Powering off the PHY at the beginning of the setup phase is currently >>>>>> split in two locations for first and second generation PHYs, group all >>>>>> the operations in the dw_hdmi_phy_init() function. >>>>> This changes the behaviour of the driver. >>>>> >>>>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) >>>>>> +{ >>>>>> + if (hdmi->phy->gen == 1) { >>>>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); >>>>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); >>>>>> + } else { >>>>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); >>>>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); >>>>>> + } >>>>>> +} >>>>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi >>>>>> *hdmi) >>>>>> if (!hdmi->phy_enabled) >>>>>> return; >>>>>> >>>>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); >>>>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); >>>>>> - >>>>>> + dw_hdmi_phy_power_off(hdmi); >>>>> This makes dw_hdmi_phy_disable() power down a gen2 phy. >>>>> >>>>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a >>>>> gen2 phy. I've been carrying this change for a while, which I've had >>>>> to revert (and finally expunge), as it causes problems on iMX6: >>>>> >>>>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi >>>>> *hdmi) >>>>> if (!hdmi->phy_enabled) >>>>> return; >>>>> >>>>> + /* Actually set the phy into low power mode */ >>>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); >>>>> + >>>>> + /* FIXME: We should wait for TX_READY to be deasserted */ >>>>> + >>>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); >>>>> + >>>>> + /* This appears to have no effect on iMX6 */ >>>>> dw_hdmi_phy_enable_tmds(hdmi, 0); >>>>> dw_hdmi_phy_enable_powerdown(hdmi, true); >>>>> >>>>> So, I think your change here will cause problems for iMX6. >>>>> >>>>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD >>>>> bouncing when the PHY is powered down. I can't promise when I'll be >>>>> able to check for that again. >>>> Indeed TX_READY must be low before asserting pddq. >>> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY >>> output signal, but there seems to be no HDMI TX register from which its >>> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register >>> through I2C ? How long is the PHY expected to take to set TX_READY to 0 ? >> TX_READY can be read from register 0x1A of phy, BIT(2) (through >> I2C). Not sure if same offset for all phys though. > I have been told that another option is to poll the TX_PHY_LOCK bit in the > phy_stat0 register. That would be a simpler solution that doesn't require I2C > access. Could you confirm (or disconfirm) this ? Yes (In our implementation we have TX_PHY_LOCK wired up to TX_READY that comes from phy. But if using a custom phy or an unusual implementation then this may not hold). Best regards, Jose Miguel Abreu >>>> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense >>>> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, >>>> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and >>>> consequently entering power down mode) you can enable again enhpdrxsense. >>> Thanks, I'll give it a try.
Hi Jose, On Wednesday 01 Mar 2017 16:25:46 Jose Abreu wrote: > On 01-03-2017 11:09, Laurent Pinchart wrote: > > On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote: > >> On 05-01-2017 12:29, Laurent Pinchart wrote: > >>> On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: > >>>> On 20-12-2016 11:45, Russell King - ARM Linux wrote: > >>>>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > >>>>>> Instead of spreading version-dependent PHY power handling code > >>>>>> around, group it in two functions to power the PHY on and off and use > >>>>>> them through the driver. > >>>>>> > >>>>>> Powering off the PHY at the beginning of the setup phase is currently > >>>>>> split in two locations for first and second generation PHYs, group > >>>>>> all the operations in the dw_hdmi_phy_init() function. > >>>>> > >>>>> This changes the behaviour of the driver. > >>>>> > >>>>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > >>>>>> +{ > >>>>>> + if (hdmi->phy->gen == 1) { > >>>>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>>> + } else { > >>>>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>>>> + } > >>>>>> +} > >>>>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>>>> *hdmi) > >>>>>> if (!hdmi->phy_enabled) > >>>>>> return; > >>>>>> > >>>>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>>> - > >>>>>> + dw_hdmi_phy_power_off(hdmi); > >>>>> > >>>>> This makes dw_hdmi_phy_disable() power down a gen2 phy. > >>>>> > >>>>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > >>>>> gen2 phy. I've been carrying this change for a while, which I've had > >>>>> to revert (and finally expunge), as it causes problems on iMX6: > >>>>> > >>>>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>>> *hdmi) > >>>>> if (!hdmi->phy_enabled) > >>>>> return; > >>>>> > >>>>> + /* Actually set the phy into low power mode */ > >>>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>>> + > >>>>> + /* FIXME: We should wait for TX_READY to be deasserted */ > >>>>> + > >>>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>>> + > >>>>> + /* This appears to have no effect on iMX6 */ > >>>>> dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>>> dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>>> > >>>>> So, I think your change here will cause problems for iMX6. > >>>>> > >>>>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > >>>>> bouncing when the PHY is powered down. I can't promise when I'll be > >>>>> able to check for that again. > >>>> > >>>> Indeed TX_READY must be low before asserting pddq. > >>> > >>> The TX_READY signal is documented in the i.MX6 datasheet as being a PHY > >>> output signal, but there seems to be no HDMI TX register from which its > >>> state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register > >>> through I2C ? How long is the PHY expected to take to set TX_READY to 0 > >>> ? > >> > >> TX_READY can be read from register 0x1A of phy, BIT(2) (through > >> I2C). Not sure if same offset for all phys though. > > > > I have been told that another option is to poll the TX_PHY_LOCK bit in the > > phy_stat0 register. That would be a simpler solution that doesn't require > > I2C access. Could you confirm (or disconfirm) this ? > > Yes (In our implementation we have TX_PHY_LOCK wired up to > TX_READY that comes from phy. But if using a custom phy or an > unusual implementation then this may not hold). Thank you for the information. I've submitted a v4 that poll the TX_PHY_LOCK bit. With the patch series applied the vendor PHY handling code already delegates PHY power control to the glue code, there's thus no issue there. For platforms using a Synopsys PHY, all the ones we have to support today have TX_READY wired to the PHY lock signal so we should be good as well there. If that doesn't remain true in the future we'll fix it. > >>>> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense > >>>> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, > >>>> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and > >>>> consequently entering power down mode) you can enable again > >>>> enhpdrxsense. > >>> > >>> Thanks, I'll give it a try.
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c index ef4f2f96ed2c..6167eb6806fe 100644 --- a/drivers/gpu/drm/bridge/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/dw-hdmi.c @@ -116,6 +116,7 @@ struct dw_hdmi_i2c { struct dw_hdmi_phy_data { enum dw_hdmi_phy_type type; const char *name; + unsigned int gen; bool has_svsret; }; @@ -940,6 +941,31 @@ static void dw_hdmi_phy_sel_interface_control(struct dw_hdmi *hdmi, u8 enable) HDMI_PHY_CONF0_SELDIPIF_MASK); } +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) +{ + if (hdmi->phy->gen == 1) { + dw_hdmi_phy_enable_tmds(hdmi, 0); + dw_hdmi_phy_enable_powerdown(hdmi, true); + } else { + dw_hdmi_phy_gen2_txpwron(hdmi, 0); + dw_hdmi_phy_gen2_pddq(hdmi, 1); + } +} + +static void dw_hdmi_phy_power_on(struct dw_hdmi *hdmi) +{ + if (hdmi->phy->gen == 1) { + dw_hdmi_phy_enable_powerdown(hdmi, false); + + /* Toggle TMDS enable. */ + dw_hdmi_phy_enable_tmds(hdmi, 0); + dw_hdmi_phy_enable_tmds(hdmi, 1); + } else { + dw_hdmi_phy_gen2_txpwron(hdmi, 1); + dw_hdmi_phy_gen2_pddq(hdmi, 0); + } +} + static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon) { u8 val, msec; @@ -980,11 +1006,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon) hdmi_writeb(hdmi, val, HDMI_MC_FLOWCTRL); - /* gen2 tx power off */ - dw_hdmi_phy_gen2_txpwron(hdmi, 0); - - /* gen2 pddq */ - dw_hdmi_phy_gen2_pddq(hdmi, 1); + dw_hdmi_phy_power_off(hdmi); /* PHY reset */ hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_DEASSERT, HDMI_MC_PHYRSTZ); @@ -1013,15 +1035,7 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi, int cscon) /* REMOVE CLK TERM */ hdmi_phy_i2c_write(hdmi, 0x8000, 0x05); /* CKCALCTRL */ - dw_hdmi_phy_enable_powerdown(hdmi, false); - - /* toggle TMDS enable */ - dw_hdmi_phy_enable_tmds(hdmi, 0); - dw_hdmi_phy_enable_tmds(hdmi, 1); - - /* gen2 tx power on */ - dw_hdmi_phy_gen2_txpwron(hdmi, 1); - dw_hdmi_phy_gen2_pddq(hdmi, 0); + dw_hdmi_phy_power_on(hdmi); /* The DWC MHL and HDMI 2.0 PHYs need the SVSRET signal to be set. */ if (hdmi->phy->has_svsret) @@ -1058,8 +1072,6 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi) for (i = 0; i < 2; i++) { dw_hdmi_phy_sel_data_en_pol(hdmi, 1); dw_hdmi_phy_sel_interface_control(hdmi, 0); - dw_hdmi_phy_enable_tmds(hdmi, 0); - dw_hdmi_phy_enable_powerdown(hdmi, true); /* Enable CSC */ ret = hdmi_phy_configure(hdmi, cscon); @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi) if (!hdmi->phy_enabled) return; - dw_hdmi_phy_enable_tmds(hdmi, 0); - dw_hdmi_phy_enable_powerdown(hdmi, true); - + dw_hdmi_phy_power_off(hdmi); hdmi->phy_enabled = false; } @@ -1853,23 +1863,29 @@ static const struct dw_hdmi_phy_data dw_hdmi_phys[] = { { .type = DW_HDMI_PHY_DWC_HDMI_TX_PHY, .name = "DWC HDMI TX PHY", + .gen = 1, }, { .type = DW_HDMI_PHY_DWC_MHL_PHY_HEAC, .name = "DWC MHL PHY + HEAC PHY", + .gen = 2, .has_svsret = true, }, { .type = DW_HDMI_PHY_DWC_MHL_PHY, .name = "DWC MHL PHY", + .gen = 2, .has_svsret = true, }, { .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY_HEAC, .name = "DWC HDMI 3D TX PHY + HEAC PHY", + .gen = 2, }, { .type = DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY, .name = "DWC HDMI 3D TX PHY", + .gen = 2, }, { .type = DW_HDMI_PHY_DWC_HDMI20_TX_PHY, .name = "DWC HDMI 2.0 TX PHY", + .gen = 2, .has_svsret = true, } };
Instead of spreading version-dependent PHY power handling code around, group it in two functions to power the PHY on and off and use them through the driver. Powering off the PHY at the beginning of the setup phase is currently split in two locations for first and second generation PHYs, group all the operations in the dw_hdmi_phy_init() function. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/gpu/drm/bridge/dw-hdmi.c | 54 ++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 19 deletions(-)