On 04/28/17 13:30, Tomi Valkeinen wrote: > On 14/04/17 13:25, Hans Verkuil wrote: >> From: Hans Verkuil <hans.verkuil@xxxxxxxxx> >> >> 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@xxxxxxxxx> >> --- >> 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 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel