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

[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.

And when it comes to power and the analogix-dp driver, I've been
carrying this[1] for a while. Fully exploitable from userspace. I know
it is about to be replaced by this series, but at least 4.15 and 4.16
are affected.

	M.

[1] https://www.spinics.net/lists/arm-kernel/msg623892.html
-- 
Jazz is not dead. It just smells funny...