Re: [PATCH v3 05/43] drm/bridge: analogix_dp: Don't power bridge in analogix_dp_bind

[Heiko Stübner]

Am Mittwoch, 28. Februar 2018, 15:54:30 CET schrieb Marc Zyngier:
> On 28/02/18 14:37, Heiko Stübner wrote:
> > Am Dienstag, 30. Januar 2018, 21:28:35 CET schrieb Thierry Escande:
> >> From: zain wang <wzz@xxxxxxxxxxxxxx>
> >> 
> >> The bridge does not need to be powered in analogix_dp_bind(), so
> >> remove the calls to pm_runtime_get()/phy_power_on()/analogix_dp_init_dp()
> >> as well as their power-off counterparts.
> >> 
> >> Cc: Stéphane Marchesin <marcheu@xxxxxxxxxxxx>
> >> Signed-off-by: zain wang <wzz@xxxxxxxxxxxxxx>
> >> Signed-off-by: Caesar Wang <wxt@xxxxxxxxxxxxxx>
> >> [the patch originally just removed the power_on portion, seanpaul removed
> >> the power off code as well as improved the commit message]
> >> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> >> Signed-off-by: Thierry Escande <thierry.escande@xxxxxxxxxxxxx>
> >> ---
> >> 
> >>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 10 ----------
> >>  1 file changed, 10 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index
> >> cb5e18d6ba04..1477ea9ba85d 100644
> >> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> @@ -1382,11 +1382,6 @@ analogix_dp_bind(struct device *dev, struct
> >> drm_device *drm_dev,
> >> 
> >>  	pm_runtime_enable(dev);
> >> 
> >> -	pm_runtime_get_sync(dev);
> >> -	phy_power_on(dp->phy);
> >> -
> >> -	analogix_dp_init_dp(dp);
> >> -
> >> 
> >>  	ret = devm_request_threaded_irq(&pdev->dev, dp->irq,
> >>  	
> >>  					analogix_dp_hardirq,
> >>  					analogix_dp_irq_thread,
> > 
> > Not 100% sure here, as the driver has the request-irq + disable-irq hack
> > here. So a pending interrupt could possibly fire between request and
> > disable.
> > 
> > Right now the block should be on, but can it still handle such an irq
> > when the power is removed?
> 
> Probably not (see below).
> 
> > So before removing the power here, we might want something
> > similar to what Marc posted for the vop [0] for the analogix-dp?
> 
> You can do that trick only if the interrupt is not shared. In the VOP
> case, it is shared with the IOMMU, which makes it more... interesting.

Yep, which is why I mentioned it, as the dp-irq should not be shared
I'd think :-)


Heiko