Hi Laurent, On 20-12-2016 13:11, Laurent Pinchart wrote: > 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@xxxxxxxxxxxxxxxx> >>> --- >>> >>> 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] > 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) ? SVSRET stands for SVSRET :) (no idea what it means) Its a low power mode of consumption. > > 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. Correct. Older phys require PHYRSTZ to be deasserted (i.e. low) for a PHY-dependent time. Newer phys require PHYRSTZ to be asserted (i.e. high) for, again, a PHY-dependent time. This is the kind of things that made me suggest you to extract all the phy configuration from dw-hdmi. I think that having a bunch of if's because of all the phy's that we need to support does not make much sense. The downside is, of course, having code duplicated. > > I can move the SVSRET assertion before PHY reset and test it on RK3288 and R- > Car H3. Probably wont make much difference unless you have a way to measure how much power the phy is consuming. But I think it is the right thing to do according to docs. [snip] > 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 :-) > Hmm, ok. I still haven't figured out which phy you are using so can't comment much further. Best regards, Jose Miguel Abreu
