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