[PATCH 6/8] omapdrm: hdmi4: refcount hdmi_power_on/off_core

[Hans Verkuil]

On 04/28/17 13:30, Tomi Valkeinen wrote:
> On 14/04/17 13:25, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil at cisco.com>
>>
>> The hdmi_power_on/off_core functions can be called multiple times:
>> when the HPD changes and when the HDMI CEC support needs to power
>> the HDMI core.
>>
>> So use a counter to know when to really power on or off the HDMI core.
>>
>> Also call hdmi4_core_powerdown_disable() in hdmi_power_on_core() to
>> power up the HDMI core (needed for CEC).
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil at cisco.com>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/hdmi4.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> index 4a164dc01f15..e371b47ff6ff 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> @@ -124,14 +124,19 @@ static int hdmi_power_on_core(struct omap_dss_device *dssdev)
>>  {
>>  	int r;
>>  
>> +	if (hdmi.core.core_pwr_cnt++)
>> +		return 0;
>> +
> 
> How's the locking between the CEC side and the DRM side? Normally these
> functions are protected with the DRM modesetting locks, but CEC doesn't
> come from there. We have the hdmi.lock, did you check that it's held
> when CEC side calls shared functions?

Yes, the hdmi_power_on/off_core functions are all called from other functions
with the hdmi.lock taken. The CEC code calls those higher level functions
(hdmi4_core_enable/disable).

> 
>>  	r = regulator_enable(hdmi.vdda_reg);
>>  	if (r)
>> -		return r;
>> +		goto err_reg_enable;
>>  
>>  	r = hdmi_runtime_get();
>>  	if (r)
>>  		goto err_runtime_get;
>>  
>> +	hdmi4_core_powerdown_disable(&hdmi.core);
> 
> I'd like to have the powerdown_disable as a separate patch.

Will do.

> Also, now
> that you call it here, I believe it can be dropped from hdmi4_configure().

I was a bit scared of messing with that function. But if you say it can
be removed, then who am I to argue? :-)

> 
> Hmm, but in hdmi4_configure we call hdmi_core_swreset_assert() before
> hdmi4_core_powerdown_disable(). I wonder what exactly that does, and
> whether we end up resetting also the CEC parts when we change the videomode.

Good one. I'll attempt to check this.

Regards,

	Hans

> 
>  Tomi
>