Hi Marek,
On Friday, 23 February 2018 09:35:32 EET Marek Szyprowski wrote:
> On 2018-02-21 13:49, Laurent Pinchart wrote:
> > On Wednesday, 21 February 2018 12:04:43 EET Marek Szyprowski wrote:
> >> Patch f0a8b49c03d2 ("drm/bridge: analogix dp: Fix runtime PM state on
> >> driver bind") fixed unbalanced call to phy_power_on() in
> >> analogix_dp_bind() function by calling phy_power_off() at the end of bind
> >> operation.
> >>
> >> However it turned out that having PHY powered is required for proper DRM
> >> display pipeline initialization on Peach-Pit Chromebook2 board, as this
> >> board freezes otherwise when PHY power off is called in bind. Fix this by
> >> keeping PHY powered for the whole bind/unbind driver life cycle. The
> >> freeze is probably related to the fact that the display pipeline (Exynos
> >> FIMD CTRC -> Exynos/Analogix DP bridge -> PS8625 dp2lvds bridge ->
> >> B116XW03 panel) is already configured and enabled by the bootloader and
> >> require reset of all components for proper shutdown.
> >
> > "Probably" always makes me feel uncomfortable in such a context :-)
> > Leaving the PHY powered at all times isn't the best idea as it will
> > increase power consumption even when the DP output isn't in use. I can
> > live with that if there's no other way, but it feels to me like we should
> > first get to the bottom of the issue.
>
> Frankly I spent some time on this and I have no other idea. Board
> freezes when phy_power_off is called and the initial code worked properly on
> that board by leaking phy turned on all the time.
I don't care too much personally as I don't maintain any platform that uses
this chip, but if I did, I would mind the extra power consumption on my
platform to work around an issue on yours :-) If this gets merged I think you
should at least add a big FIXME comment that explains why the PHY isn't
powered off at bind time.
> >> Having PHY powered is also needed for proper EDID handling, which
> >> initially fixed by commit 510353a63796 ("drm/bridge: analogix dp: Fix
> >> runtime PM state in get_modes() callback").
> >>
> >> Fixes: f0a8b49c03d2 ("drm/bridge: analogix dp: Fix runtime PM state on
> >> driver bind")
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> >> ---
> >>
> >> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index
> >> b2756bc4225a..42595863cb03 100644
> >> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> @@ -1415,7 +1415,6 @@ int analogix_dp_bind(struct device *dev, struct
> >> drm_device *drm_dev,
> >> goto err_disable_pm_runtime;
> >> }
> >>
> >> - phy_power_off(dp->phy);
> >> pm_runtime_put(dev);
> >>
> >> return 0;
> >> @@ -1448,6 +1447,7 @@ void analogix_dp_unbind(struct device *dev, struct
> >> device *master,
> >>
> >> drm_dp_aux_unregister(&dp->aux);
> >> pm_runtime_disable(dev);
> >> + phy_power_off(dp->phy);
> >> clk_disable_unprepare(dp->clock);
> >> }
> >> EXPORT_SYMBOL_GPL(analogix_dp_unbind);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
