Re: [PATCH] drm/bridge: analogix_dp: Keep PHY powered from between driver bind/unbind

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hi Laurent,

On 2018-02-21 13:49, Laurent Pinchart wrote:
> Thank you for the patch.
>
> 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.

>> 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@samsung.com>
>> ---
>>   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);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel