Hi,
This series aims at fixing a complete and silent hang when one tries to use CEC while the display output is off.
This can be tested with:
echo off > /sys/class/drm/card0-HDMI-A-1/status cec-ctl --tuner -p 1.0.0.0 cec-compliance
This series addresses it by making sure the HDMI controller is powered up as soon as the CEC device is opened by the userspace.
Let me know what you think, Maxime
Maxime Ripard (4): drm/vc4: hdmi: Mark the device as active if running drm/vc4: hdmi: Put the device on error in pre_crtc_configure drm/vc4: hdmi: Split the CEC disable / enable functions in two drm/vc4: hdmi: Make sure the device is powered with CEC
drivers/gpu/drm/vc4/vc4_hdmi.c | 85 ++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 30 deletions(-)
If we detect the device as running we make sure we get the reference count right for clocks, but we don't do as such for the runtime PM count, let's call pm_runtime_enable to make sure it's accurate.
Fixes: 5b0060004236 ("drm/vc4: hdmi: Prevent clock unbalance") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index aab1b36ceb3c..707fe43ffeea 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2182,6 +2182,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") || of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) && HDMI_READ(HDMI_VID_CTL) & VC4_HD_VID_CTL_ENABLE) { + pm_runtime_set_active(dev); clk_prepare_enable(vc4_hdmi->pixel_clock); clk_prepare_enable(vc4_hdmi->hsm_clock); clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
In the vc4_hdmi_encoder_pre_crtc_configure() function error path we never actually call pm_runtime_put() even though pm_runtime_resume_and_get() is our very first call.
Fixes: 4f6e3d66ac52 ("drm/vc4: Add runtime PM support to the HDMI encoder driver") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 707fe43ffeea..b3db38d37550 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -958,6 +958,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, if (ret) { DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret); clk_disable_unprepare(vc4_hdmi->pixel_clock); + pm_runtime_put(&vc4_hdmi->pdev->dev); return; }
@@ -965,6 +966,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, if (ret) { DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret); clk_disable_unprepare(vc4_hdmi->pixel_clock); + pm_runtime_put(&vc4_hdmi->pdev->dev); return; }
In order to ease further additions to the CEC enable and disable, let's split the function into two functions, one to enable and the other to disable.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 73 ++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index b3db38d37550..3f244027eb58 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1735,7 +1735,7 @@ static irqreturn_t vc4_cec_irq_handler(int irq, void *priv) return ret; }
-static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable) +static int vc4_hdmi_cec_enable(struct cec_adapter *adap) { struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap); /* clock period in microseconds */ @@ -1748,38 +1748,53 @@ static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable) val |= ((4700 / usecs) << VC4_HDMI_CEC_CNT_TO_4700_US_SHIFT) | ((4500 / usecs) << VC4_HDMI_CEC_CNT_TO_4500_US_SHIFT);
- if (enable) { - HDMI_WRITE(HDMI_CEC_CNTRL_5, val | - VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET); - HDMI_WRITE(HDMI_CEC_CNTRL_5, val); - HDMI_WRITE(HDMI_CEC_CNTRL_2, - ((1500 / usecs) << VC4_HDMI_CEC_CNT_TO_1500_US_SHIFT) | - ((1300 / usecs) << VC4_HDMI_CEC_CNT_TO_1300_US_SHIFT) | - ((800 / usecs) << VC4_HDMI_CEC_CNT_TO_800_US_SHIFT) | - ((600 / usecs) << VC4_HDMI_CEC_CNT_TO_600_US_SHIFT) | - ((400 / usecs) << VC4_HDMI_CEC_CNT_TO_400_US_SHIFT)); - HDMI_WRITE(HDMI_CEC_CNTRL_3, - ((2750 / usecs) << VC4_HDMI_CEC_CNT_TO_2750_US_SHIFT) | - ((2400 / usecs) << VC4_HDMI_CEC_CNT_TO_2400_US_SHIFT) | - ((2050 / usecs) << VC4_HDMI_CEC_CNT_TO_2050_US_SHIFT) | - ((1700 / usecs) << VC4_HDMI_CEC_CNT_TO_1700_US_SHIFT)); - HDMI_WRITE(HDMI_CEC_CNTRL_4, - ((4300 / usecs) << VC4_HDMI_CEC_CNT_TO_4300_US_SHIFT) | - ((3900 / usecs) << VC4_HDMI_CEC_CNT_TO_3900_US_SHIFT) | - ((3600 / usecs) << VC4_HDMI_CEC_CNT_TO_3600_US_SHIFT) | - ((3500 / usecs) << VC4_HDMI_CEC_CNT_TO_3500_US_SHIFT)); + HDMI_WRITE(HDMI_CEC_CNTRL_5, val | + VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET); + HDMI_WRITE(HDMI_CEC_CNTRL_5, val); + HDMI_WRITE(HDMI_CEC_CNTRL_2, + ((1500 / usecs) << VC4_HDMI_CEC_CNT_TO_1500_US_SHIFT) | + ((1300 / usecs) << VC4_HDMI_CEC_CNT_TO_1300_US_SHIFT) | + ((800 / usecs) << VC4_HDMI_CEC_CNT_TO_800_US_SHIFT) | + ((600 / usecs) << VC4_HDMI_CEC_CNT_TO_600_US_SHIFT) | + ((400 / usecs) << VC4_HDMI_CEC_CNT_TO_400_US_SHIFT)); + HDMI_WRITE(HDMI_CEC_CNTRL_3, + ((2750 / usecs) << VC4_HDMI_CEC_CNT_TO_2750_US_SHIFT) | + ((2400 / usecs) << VC4_HDMI_CEC_CNT_TO_2400_US_SHIFT) | + ((2050 / usecs) << VC4_HDMI_CEC_CNT_TO_2050_US_SHIFT) | + ((1700 / usecs) << VC4_HDMI_CEC_CNT_TO_1700_US_SHIFT)); + HDMI_WRITE(HDMI_CEC_CNTRL_4, + ((4300 / usecs) << VC4_HDMI_CEC_CNT_TO_4300_US_SHIFT) | + ((3900 / usecs) << VC4_HDMI_CEC_CNT_TO_3900_US_SHIFT) | + ((3600 / usecs) << VC4_HDMI_CEC_CNT_TO_3600_US_SHIFT) | + ((3500 / usecs) << VC4_HDMI_CEC_CNT_TO_3500_US_SHIFT)); + + if (!vc4_hdmi->variant->external_irq_controller) + HDMI_WRITE(HDMI_CEC_CPU_MASK_CLEAR, VC4_HDMI_CPU_CEC);
- if (!vc4_hdmi->variant->external_irq_controller) - HDMI_WRITE(HDMI_CEC_CPU_MASK_CLEAR, VC4_HDMI_CPU_CEC); - } else { - if (!vc4_hdmi->variant->external_irq_controller) - HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, VC4_HDMI_CPU_CEC); - HDMI_WRITE(HDMI_CEC_CNTRL_5, val | - VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET); - } return 0; }
+static int vc4_hdmi_cec_disable(struct cec_adapter *adap) +{ + struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap); + + if (!vc4_hdmi->variant->external_irq_controller) + HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, VC4_HDMI_CPU_CEC); + + HDMI_WRITE(HDMI_CEC_CNTRL_5, HDMI_READ(HDMI_CEC_CNTRL_5) | + VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET); + + return 0; +} + +static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable) +{ + if (enable) + return vc4_hdmi_cec_enable(adap); + else + return vc4_hdmi_cec_disable(adap); +} + static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr) { struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
Similarly to what we encountered with the detect hook with DRM, nothing actually prevents any of the CEC callback from being run while the HDMI output is disabled.
However, this is an issue since any register access to the controller when it's powered down will result in a silent hang.
Let's make sure we run the runtime_pm hooks when the CEC adapter is opened and closed by the userspace to avoid that issue.
Fixes: 15b4511a4af6 ("drm/vc4: add HDMI CEC support") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 3f244027eb58..20cb6e62f7a4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1741,6 +1741,11 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap) /* clock period in microseconds */ const u32 usecs = 1000000 / CEC_CLOCK_FREQ; u32 val = HDMI_READ(HDMI_CEC_CNTRL_5); + int ret; + + ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev); + if (ret) + return ret;
val &= ~(VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET | VC4_HDMI_CEC_CNT_TO_4700_US_MASK | @@ -1784,6 +1789,8 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap) HDMI_WRITE(HDMI_CEC_CNTRL_5, HDMI_READ(HDMI_CEC_CNTRL_5) | VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET);
+ pm_runtime_put(&vc4_hdmi->pdev->dev); + return 0; }
dri-devel@lists.freedesktop.org
-
Maxime Ripard