CEC blocks idle on omap4

CEC blocks idle on omap4

[Tony Lindgren]

Hi Hans,

Looks like CONFIG_OMAP4_DSS_HDMI_CEC=y blocks SoC core retention
idle on omap4 if selected.

Should we maybe move hdmi4_cec_init() to hdmi_display_enable()
and hdmi4_cec_uninit() to hdmi_display_disable()?

Or add some enable/disable calls in addtion to the init and
uninit calls that can be called from hdmi_display_enable()
and hdmi_display_disable()?

Cheers,

Tony

Re: CEC blocks idle on omap4

[Hans Verkuil]

Hi Tony,

On 3/25/19 4:32 PM, Tony Lindgren wrote:
> Hi Hans,
> 
> Looks like CONFIG_OMAP4_DSS_HDMI_CEC=y blocks SoC core retention
> idle on omap4 if selected.
> 
> Should we maybe move hdmi4_cec_init() to hdmi_display_enable()
> and hdmi4_cec_uninit() to hdmi_display_disable()?
> 
> Or add some enable/disable calls in addtion to the init and
> uninit calls that can be called from hdmi_display_enable()
> and hdmi_display_disable()?

For proper HDMI CEC behavior the CEC adapter has to remain active
even if the HPD of the display is low. Some displays pull down the
HPD when in standby, but CEC can still be used to wake them up.

And we see this more often as regulations for the maximum power
consumption of displays are getting more and more strict.

So disabling CEC when the display is disabled is not an option.

Disabling CEC if the source is no longer transmitting isn't a good
idea either since the display will typically still send periodic
CEC commands to the source that it expects to reply to.

The reality is that HDMI CEC and HDMI video are really independent of
one another. So I wonder if it isn't better to explain the downsides
of enabling CEC for the omap4 in the CONFIG_OMAP4_DSS_HDMI_CEC
description. And perhaps disable it by default?

Regards,

	Hans

Re: CEC blocks idle on omap4

[Laurent Pinchart]

Hi Hans,

On Mon, Mar 25, 2019 at 04:51:57PM +0100, Hans Verkuil wrote:
> On 3/25/19 4:32 PM, Tony Lindgren wrote:
> > Hi Hans,
> > 
> > Looks like CONFIG_OMAP4_DSS_HDMI_CEC=y blocks SoC core retention
> > idle on omap4 if selected.
> > 
> > Should we maybe move hdmi4_cec_init() to hdmi_display_enable()
> > and hdmi4_cec_uninit() to hdmi_display_disable()?
> > 
> > Or add some enable/disable calls in addtion to the init and
> > uninit calls that can be called from hdmi_display_enable()
> > and hdmi_display_disable()?
> 
> For proper HDMI CEC behavior the CEC adapter has to remain active
> even if the HPD of the display is low. Some displays pull down the
> HPD when in standby, but CEC can still be used to wake them up.
> 
> And we see this more often as regulations for the maximum power
> consumption of displays are getting more and more strict.
> 
> So disabling CEC when the display is disabled is not an option.
> 
> Disabling CEC if the source is no longer transmitting isn't a good
> idea either since the display will typically still send periodic
> CEC commands to the source that it expects to reply to.

What's the periodicity of those commands ? Can the system be put to
sleep and get woken up when there's CEC activity ?

> The reality is that HDMI CEC and HDMI video are really independent of
> one another. So I wonder if it isn't better to explain the downsides
> of enabling CEC for the omap4 in the CONFIG_OMAP4_DSS_HDMI_CEC
> description. And perhaps disable it by default?

This should be controllable by userspace. From a product point of view,
it should be possible to put the system in a deep sleep state where CEC
isn't available, or in a low sleep state where CEC works as expected.

-- 
Regards,

Laurent Pinchart

Re: CEC blocks idle on omap4

[Hans Verkuil]

On 3/25/19 4:55 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Mon, Mar 25, 2019 at 04:51:57PM +0100, Hans Verkuil wrote:
>> On 3/25/19 4:32 PM, Tony Lindgren wrote:
>>> Hi Hans,
>>>
>>> Looks like CONFIG_OMAP4_DSS_HDMI_CEC=y blocks SoC core retention
>>> idle on omap4 if selected.
>>>
>>> Should we maybe move hdmi4_cec_init() to hdmi_display_enable()
>>> and hdmi4_cec_uninit() to hdmi_display_disable()?
>>>
>>> Or add some enable/disable calls in addtion to the init and
>>> uninit calls that can be called from hdmi_display_enable()
>>> and hdmi_display_disable()?
>>
>> For proper HDMI CEC behavior the CEC adapter has to remain active
>> even if the HPD of the display is low. Some displays pull down the
>> HPD when in standby, but CEC can still be used to wake them up.
>>
>> And we see this more often as regulations for the maximum power
>> consumption of displays are getting more and more strict.
>>
>> So disabling CEC when the display is disabled is not an option.
>>
>> Disabling CEC if the source is no longer transmitting isn't a good
>> idea either since the display will typically still send periodic
>> CEC commands to the source that it expects to reply to.
> 
> What's the periodicity of those commands ? Can the system be put to
> sleep and get woken up when there's CEC activity ?

You don't control that. The sink can transmit a CEC message at any
time and the omap4 CEC adapter has to be active to correctly react.

> 
>> The reality is that HDMI CEC and HDMI video are really independent of
>> one another. So I wonder if it isn't better to explain the downsides
>> of enabling CEC for the omap4 in the CONFIG_OMAP4_DSS_HDMI_CEC
>> description. And perhaps disable it by default?
> 
> This should be controllable by userspace. From a product point of view,
> it should be possible to put the system in a deep sleep state where CEC
> isn't available, or in a low sleep state where CEC works as expected.
> 

Userspace can always disable CEC. The hdmi_cec_adap_enable() callback in
hdmi4_cec.c is called whenever the CEC adapter is enabled or disabled.

I'm not actually sure why hdmi4_cec_init() would block anything since it
just registers the CEC device. It does not enable it until userspace
explicitly enables it with e.g. 'cec-ctl --playback'.

hdmi4_cec_init() does configure a CEC clock, but that can be moved to
hdmi_cec_adap_enable() if necessary.

Note that I am not sure what is meant with 'SoC core retention idle',
so perhaps I just misunderstand the problem.

Regards,

	Hans

Re: CEC blocks idle on omap4

[Tony Lindgren]

* Hans Verkuil <hverkuil@xs4all.nl> [190325 16:12]:
> On 3/25/19 4:55 PM, Laurent Pinchart wrote:
> >> The reality is that HDMI CEC and HDMI video are really independent of
> >> one another. So I wonder if it isn't better to explain the downsides
> >> of enabling CEC for the omap4 in the CONFIG_OMAP4_DSS_HDMI_CEC
> >> description. And perhaps disable it by default?
> > 
> > This should be controllable by userspace. From a product point of view,
> > it should be possible to put the system in a deep sleep state where CEC
> > isn't available, or in a low sleep state where CEC works as expected.
> > 
> 
> Userspace can always disable CEC. The hdmi_cec_adap_enable() callback in
> hdmi4_cec.c is called whenever the CEC adapter is enabled or disabled.

OK

> I'm not actually sure why hdmi4_cec_init() would block anything since it
> just registers the CEC device. It does not enable it until userspace
> explicitly enables it with e.g. 'cec-ctl --playback'.
> 
> hdmi4_cec_init() does configure a CEC clock, but that can be moved to
> hdmi_cec_adap_enable() if necessary.

Hey I'm pretty sure that's the right fix then :)

> Note that I am not sure what is meant with 'SoC core retention idle',
> so perhaps I just misunderstand the problem.

If certain SoC clocks are busy, the SoC will not enter deeper
idle states. The hardware has autoidle type features on omaps.

Regards,

Tony

Re: CEC blocks idle on omap4

[Hans Verkuil]

On 3/25/19 5:21 PM, Tony Lindgren wrote:
> * Hans Verkuil <hverkuil@xs4all.nl> [190325 16:12]:
>> On 3/25/19 4:55 PM, Laurent Pinchart wrote:
>>>> The reality is that HDMI CEC and HDMI video are really independent of
>>>> one another. So I wonder if it isn't better to explain the downsides
>>>> of enabling CEC for the omap4 in the CONFIG_OMAP4_DSS_HDMI_CEC
>>>> description. And perhaps disable it by default?
>>>
>>> This should be controllable by userspace. From a product point of view,
>>> it should be possible to put the system in a deep sleep state where CEC
>>> isn't available, or in a low sleep state where CEC works as expected.
>>>
>>
>> Userspace can always disable CEC. The hdmi_cec_adap_enable() callback in
>> hdmi4_cec.c is called whenever the CEC adapter is enabled or disabled.
> 
> OK
> 
>> I'm not actually sure why hdmi4_cec_init() would block anything since it
>> just registers the CEC device. It does not enable it until userspace
>> explicitly enables it with e.g. 'cec-ctl --playback'.
>>
>> hdmi4_cec_init() does configure a CEC clock, but that can be moved to
>> hdmi_cec_adap_enable() if necessary.
> 
> Hey I'm pretty sure that's the right fix then :)
> 
>> Note that I am not sure what is meant with 'SoC core retention idle',
>> so perhaps I just misunderstand the problem.
> 
> If certain SoC clocks are busy, the SoC will not enter deeper
> idle states. The hardware has autoidle type features on omaps.

Can you make a patch? It is very easy to test:

To configure the CEC adapter: cec-ctl --playback
To unconfigure the CEC adapter: cec-ctl --clear

As long as the CEC adapter is unconfigured you should be able to enter
the deeper idle states. But not if it is configured.

And if you are moving code anyway, can you fix the typo in the comment?
devider -> divider

That hurts my eyes...

Regards,

	Hans

Re: CEC blocks idle on omap4

[Tony Lindgren]

* Hans Verkuil <hverkuil@xs4all.nl> [190325 16:28]:
> On 3/25/19 5:21 PM, Tony Lindgren wrote:
> > * Hans Verkuil <hverkuil@xs4all.nl> [190325 16:12]:
> >> On 3/25/19 4:55 PM, Laurent Pinchart wrote:
> >>>> The reality is that HDMI CEC and HDMI video are really independent of
> >>>> one another. So I wonder if it isn't better to explain the downsides
> >>>> of enabling CEC for the omap4 in the CONFIG_OMAP4_DSS_HDMI_CEC
> >>>> description. And perhaps disable it by default?
> >>>
> >>> This should be controllable by userspace. From a product point of view,
> >>> it should be possible to put the system in a deep sleep state where CEC
> >>> isn't available, or in a low sleep state where CEC works as expected.
> >>>
> >>
> >> Userspace can always disable CEC. The hdmi_cec_adap_enable() callback in
> >> hdmi4_cec.c is called whenever the CEC adapter is enabled or disabled.
> > 
> > OK
> > 
> >> I'm not actually sure why hdmi4_cec_init() would block anything since it
> >> just registers the CEC device. It does not enable it until userspace
> >> explicitly enables it with e.g. 'cec-ctl --playback'.
> >>
> >> hdmi4_cec_init() does configure a CEC clock, but that can be moved to
> >> hdmi_cec_adap_enable() if necessary.
> > 
> > Hey I'm pretty sure that's the right fix then :)
> > 
> >> Note that I am not sure what is meant with 'SoC core retention idle',
> >> so perhaps I just misunderstand the problem.
> > 
> > If certain SoC clocks are busy, the SoC will not enter deeper
> > idle states. The hardware has autoidle type features on omaps.
> 
> Can you make a patch? It is very easy to test:

Sure. Hmm then we just clear HDMI_WP_CLK values then for disable
too I guess.

> To configure the CEC adapter: cec-ctl --playback
> To unconfigure the CEC adapter: cec-ctl --clear
> 
> As long as the CEC adapter is unconfigured you should be able to enter
> the deeper idle states. But not if it is configured.

OK will test tonight.

> And if you are moving code anyway, can you fix the typo in the comment?
> devider -> divider
> 
> That hurts my eyes...

Sure.

Thanks,

Tony

Re: CEC blocks idle on omap4

[Tony Lindgren]

* Hans Verkuil <hverkuil@xs4all.nl> [190325 15:52]:
> Hi Tony,
> 
> On 3/25/19 4:32 PM, Tony Lindgren wrote:
> > Hi Hans,
> > 
> > Looks like CONFIG_OMAP4_DSS_HDMI_CEC=y blocks SoC core retention
> > idle on omap4 if selected.
> > 
> > Should we maybe move hdmi4_cec_init() to hdmi_display_enable()
> > and hdmi4_cec_uninit() to hdmi_display_disable()?
> > 
> > Or add some enable/disable calls in addtion to the init and
> > uninit calls that can be called from hdmi_display_enable()
> > and hdmi_display_disable()?
> 
> For proper HDMI CEC behavior the CEC adapter has to remain active
> even if the HPD of the display is low. Some displays pull down the
> HPD when in standby, but CEC can still be used to wake them up.

OK

> And we see this more often as regulations for the maximum power
> consumption of displays are getting more and more strict.
> 
> So disabling CEC when the display is disabled is not an option.

OK

> Disabling CEC if the source is no longer transmitting isn't a good
> idea either since the display will typically still send periodic
> CEC commands to the source that it expects to reply to.

Hmm I wonder if we could test for HPD floating and then disable
cec? Then we would know nothing is connected, right?

> The reality is that HDMI CEC and HDMI video are really independent of
> one another. So I wonder if it isn't better to explain the downsides
> of enabling CEC for the omap4 in the CONFIG_OMAP4_DSS_HDMI_CEC
> description. And perhaps disable it by default?

From distro kernel point of view we should find some way to
disable cec dynamically. Can we maybe do something via /sys or
some command line option for cec.ko?

Then if we find a way to check for floating HPD pin or something
we could support that too.

Regards,

Tony