All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] drm/vc4: hdmi: Fix CEC access while disabled
@ 2021-06-29 12:57 ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: linux-kernel, bcm-kernel-feedback-list, Boris Brezillon,
	Emma Anholt, Maxime Ripard, Dave Stevenson, Hans Verkuil,
	Nicolas Saenz Julienne, Phil Elwell, Tim Gover, Dom Cobley,
	linux-rpi-kernel

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(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 0/4] drm/vc4: hdmi: Fix CEC access while disabled
@ 2021-06-29 12:57 ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: Nicolas Saenz Julienne, Tim Gover, Emma Anholt, Dave Stevenson,
	Boris Brezillon, linux-kernel, bcm-kernel-feedback-list,
	linux-rpi-kernel, Hans Verkuil, Phil Elwell, Dom Cobley

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(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] drm/vc4: hdmi: Mark the device as active if running
  2021-06-29 12:57 ` Maxime Ripard
@ 2021-06-29 12:57   ` Maxime Ripard
  -1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: linux-kernel, bcm-kernel-feedback-list, Boris Brezillon,
	Emma Anholt, Maxime Ripard, Dave Stevenson, Hans Verkuil,
	Nicolas Saenz Julienne, Phil Elwell, Tim Gover, Dom Cobley,
	linux-rpi-kernel

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);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/4] drm/vc4: hdmi: Mark the device as active if running
@ 2021-06-29 12:57   ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: Nicolas Saenz Julienne, Tim Gover, Emma Anholt, Dave Stevenson,
	Boris Brezillon, linux-kernel, bcm-kernel-feedback-list,
	linux-rpi-kernel, Hans Verkuil, Phil Elwell, Dom Cobley

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);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] drm/vc4: hdmi: Put the device on error in pre_crtc_configure
  2021-06-29 12:57 ` Maxime Ripard
@ 2021-06-29 12:57   ` Maxime Ripard
  -1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: linux-kernel, bcm-kernel-feedback-list, Boris Brezillon,
	Emma Anholt, Maxime Ripard, Dave Stevenson, Hans Verkuil,
	Nicolas Saenz Julienne, Phil Elwell, Tim Gover, Dom Cobley,
	linux-rpi-kernel

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;
 	}
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] drm/vc4: hdmi: Put the device on error in pre_crtc_configure
@ 2021-06-29 12:57   ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: Nicolas Saenz Julienne, Tim Gover, Emma Anholt, Dave Stevenson,
	Boris Brezillon, linux-kernel, bcm-kernel-feedback-list,
	linux-rpi-kernel, Hans Verkuil, Phil Elwell, Dom Cobley

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;
 	}
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] drm/vc4: hdmi: Split the CEC disable / enable functions in two
  2021-06-29 12:57 ` Maxime Ripard
@ 2021-06-29 12:57   ` Maxime Ripard
  -1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: linux-kernel, bcm-kernel-feedback-list, Boris Brezillon,
	Emma Anholt, Maxime Ripard, Dave Stevenson, Hans Verkuil,
	Nicolas Saenz Julienne, Phil Elwell, Tim Gover, Dom Cobley,
	linux-rpi-kernel

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);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] drm/vc4: hdmi: Split the CEC disable / enable functions in two
@ 2021-06-29 12:57   ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: Nicolas Saenz Julienne, Tim Gover, Emma Anholt, Dave Stevenson,
	Boris Brezillon, linux-kernel, bcm-kernel-feedback-list,
	linux-rpi-kernel, Hans Verkuil, Phil Elwell, Dom Cobley

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);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] drm/vc4: hdmi: Make sure the device is powered with CEC
  2021-06-29 12:57 ` Maxime Ripard
@ 2021-06-29 12:57   ` Maxime Ripard
  -1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: linux-kernel, bcm-kernel-feedback-list, Boris Brezillon,
	Emma Anholt, Maxime Ripard, Dave Stevenson, Hans Verkuil,
	Nicolas Saenz Julienne, Phil Elwell, Tim Gover, Dom Cobley,
	linux-rpi-kernel

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;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] drm/vc4: hdmi: Make sure the device is powered with CEC
@ 2021-06-29 12:57   ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-29 12:57 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
	Thomas Zimmermann, Maxime Ripard
  Cc: Nicolas Saenz Julienne, Tim Gover, Emma Anholt, Dave Stevenson,
	Boris Brezillon, linux-kernel, bcm-kernel-feedback-list,
	linux-rpi-kernel, Hans Verkuil, Phil Elwell, Dom Cobley

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;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-06-29 12:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-29 12:57 [PATCH 0/4] drm/vc4: hdmi: Fix CEC access while disabled Maxime Ripard
2021-06-29 12:57 ` Maxime Ripard
2021-06-29 12:57 ` [PATCH 1/4] drm/vc4: hdmi: Mark the device as active if running Maxime Ripard
2021-06-29 12:57   ` Maxime Ripard
2021-06-29 12:57 ` [PATCH 2/4] drm/vc4: hdmi: Put the device on error in pre_crtc_configure Maxime Ripard
2021-06-29 12:57   ` Maxime Ripard
2021-06-29 12:57 ` [PATCH 3/4] drm/vc4: hdmi: Split the CEC disable / enable functions in two Maxime Ripard
2021-06-29 12:57   ` Maxime Ripard
2021-06-29 12:57 ` [PATCH 4/4] drm/vc4: hdmi: Make sure the device is powered with CEC Maxime Ripard
2021-06-29 12:57   ` Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.