* [PATCH 0/3] drm/vc4: hdmi: Interrupt fixes
@ 2021-07-07 9:51 ` Maxime Ripard
0 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-07 9:51 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,
linux-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
Hans Verkuil, Phil Elwell, Dom Cobley
Hi,
Those are three fixes for race conditions we currently have in the vc4 HDMI
driver with regard to the interrupts handling.
The first two are fixing an issue where the handler will be removed by devm
after the resources it uses have been free'd already.
The last one is there to deal with an interrupt coming in the window between
the end of the driver's bind and the DRM device registration.
Let me know what you think,
Maxime
Maxime Ripard (3):
drm/vc4: hdmi: Drop devm interrupt handler for CEC interrupts
drm/vc4: hdmi: Drop devm interrupt handler for hotplug interrupts
drm/vc4: hdmi: Only call into DRM framework if registered
drivers/gpu/drm/vc4/vc4_hdmi.c | 92 +++++++++++++++++++++++-----------
1 file changed, 62 insertions(+), 30 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] drm/vc4: hdmi: Drop devm interrupt handler for CEC interrupts
2021-07-07 9:51 ` Maxime Ripard
@ 2021-07-07 9:51 ` Maxime Ripard
-1 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-07 9:51 UTC (permalink / raw)
To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard
Cc: Emma Anholt, Hans Verkuil, Daniel Vetter, linux-kernel,
linux-rpi-kernel, bcm-kernel-feedback-list, Dave Stevenson,
Phil Elwell, Tim Gover, Dom Cobley, Maxime Ripard,
Nicolas Saenz Julienne
The CEC interrupt handlers are registered through the
devm_request_threaded_irq function. However, while free_irq is indeed
called properly when the device is unbound or bind fails, it's called
after unbind or bind is done.
In our particular case, it means that on failure it creates a window
where our interrupt handler can be called, but we're freeing every
resource (CEC adapter, DRM objects, etc.) it might need.
In order to address this, let's switch to the non-devm variant to
control better when the handler will be unregistered and allow us to
make it safe.
Fixes: 15b4511a4af6 ("drm/vc4: add HDMI CEC support")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 49 +++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index cbeec6a5cb90..d966f61966c6 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1858,38 +1858,46 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
vc4_hdmi_cec_update_clk_div(vc4_hdmi);
if (vc4_hdmi->variant->external_irq_controller) {
- ret = devm_request_threaded_irq(&pdev->dev,
- platform_get_irq_byname(pdev, "cec-rx"),
- vc4_cec_irq_handler_rx_bare,
- vc4_cec_irq_handler_rx_thread, 0,
- "vc4 hdmi cec rx", vc4_hdmi);
+ ret = request_threaded_irq(platform_get_irq_byname(pdev, "cec-rx"),
+ vc4_cec_irq_handler_rx_bare,
+ vc4_cec_irq_handler_rx_thread, 0,
+ "vc4 hdmi cec rx", vc4_hdmi);
if (ret)
goto err_delete_cec_adap;
- ret = devm_request_threaded_irq(&pdev->dev,
- platform_get_irq_byname(pdev, "cec-tx"),
- vc4_cec_irq_handler_tx_bare,
- vc4_cec_irq_handler_tx_thread, 0,
- "vc4 hdmi cec tx", vc4_hdmi);
+ ret = request_threaded_irq(platform_get_irq_byname(pdev, "cec-tx"),
+ vc4_cec_irq_handler_tx_bare,
+ vc4_cec_irq_handler_tx_thread, 0,
+ "vc4 hdmi cec tx", vc4_hdmi);
if (ret)
- goto err_delete_cec_adap;
+ goto err_remove_cec_rx_handler;
} else {
HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff);
- ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
- vc4_cec_irq_handler,
- vc4_cec_irq_handler_thread, 0,
- "vc4 hdmi cec", vc4_hdmi);
+ ret = request_threaded_irq(platform_get_irq(pdev, 0),
+ vc4_cec_irq_handler,
+ vc4_cec_irq_handler_thread, 0,
+ "vc4 hdmi cec", vc4_hdmi);
if (ret)
goto err_delete_cec_adap;
}
ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev);
if (ret < 0)
- goto err_delete_cec_adap;
+ goto err_remove_handlers;
return 0;
+err_remove_handlers:
+ if (vc4_hdmi->variant->external_irq_controller)
+ free_irq(platform_get_irq_byname(pdev, "cec-tx"), vc4_hdmi);
+ else
+ free_irq(platform_get_irq(pdev, 0), vc4_hdmi);
+
+err_remove_cec_rx_handler:
+ if (vc4_hdmi->variant->external_irq_controller)
+ free_irq(platform_get_irq_byname(pdev, "cec-rx"), vc4_hdmi);
+
err_delete_cec_adap:
cec_delete_adapter(vc4_hdmi->cec_adap);
@@ -1898,6 +1906,15 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
static void vc4_hdmi_cec_exit(struct vc4_hdmi *vc4_hdmi)
{
+ struct platform_device *pdev = vc4_hdmi->pdev;
+
+ if (vc4_hdmi->variant->external_irq_controller) {
+ free_irq(platform_get_irq_byname(pdev, "cec-rx"), vc4_hdmi);
+ free_irq(platform_get_irq_byname(pdev, "cec-tx"), vc4_hdmi);
+ } else {
+ free_irq(platform_get_irq(pdev, 0), vc4_hdmi);
+ }
+
cec_unregister_adapter(vc4_hdmi->cec_adap);
}
#else
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 1/3] drm/vc4: hdmi: Drop devm interrupt handler for CEC interrupts
@ 2021-07-07 9:51 ` Maxime Ripard
0 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-07 9:51 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,
linux-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
Hans Verkuil, Phil Elwell, Dom Cobley
The CEC interrupt handlers are registered through the
devm_request_threaded_irq function. However, while free_irq is indeed
called properly when the device is unbound or bind fails, it's called
after unbind or bind is done.
In our particular case, it means that on failure it creates a window
where our interrupt handler can be called, but we're freeing every
resource (CEC adapter, DRM objects, etc.) it might need.
In order to address this, let's switch to the non-devm variant to
control better when the handler will be unregistered and allow us to
make it safe.
Fixes: 15b4511a4af6 ("drm/vc4: add HDMI CEC support")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 49 +++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index cbeec6a5cb90..d966f61966c6 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1858,38 +1858,46 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
vc4_hdmi_cec_update_clk_div(vc4_hdmi);
if (vc4_hdmi->variant->external_irq_controller) {
- ret = devm_request_threaded_irq(&pdev->dev,
- platform_get_irq_byname(pdev, "cec-rx"),
- vc4_cec_irq_handler_rx_bare,
- vc4_cec_irq_handler_rx_thread, 0,
- "vc4 hdmi cec rx", vc4_hdmi);
+ ret = request_threaded_irq(platform_get_irq_byname(pdev, "cec-rx"),
+ vc4_cec_irq_handler_rx_bare,
+ vc4_cec_irq_handler_rx_thread, 0,
+ "vc4 hdmi cec rx", vc4_hdmi);
if (ret)
goto err_delete_cec_adap;
- ret = devm_request_threaded_irq(&pdev->dev,
- platform_get_irq_byname(pdev, "cec-tx"),
- vc4_cec_irq_handler_tx_bare,
- vc4_cec_irq_handler_tx_thread, 0,
- "vc4 hdmi cec tx", vc4_hdmi);
+ ret = request_threaded_irq(platform_get_irq_byname(pdev, "cec-tx"),
+ vc4_cec_irq_handler_tx_bare,
+ vc4_cec_irq_handler_tx_thread, 0,
+ "vc4 hdmi cec tx", vc4_hdmi);
if (ret)
- goto err_delete_cec_adap;
+ goto err_remove_cec_rx_handler;
} else {
HDMI_WRITE(HDMI_CEC_CPU_MASK_SET, 0xffffffff);
- ret = devm_request_threaded_irq(&pdev->dev, platform_get_irq(pdev, 0),
- vc4_cec_irq_handler,
- vc4_cec_irq_handler_thread, 0,
- "vc4 hdmi cec", vc4_hdmi);
+ ret = request_threaded_irq(platform_get_irq(pdev, 0),
+ vc4_cec_irq_handler,
+ vc4_cec_irq_handler_thread, 0,
+ "vc4 hdmi cec", vc4_hdmi);
if (ret)
goto err_delete_cec_adap;
}
ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev);
if (ret < 0)
- goto err_delete_cec_adap;
+ goto err_remove_handlers;
return 0;
+err_remove_handlers:
+ if (vc4_hdmi->variant->external_irq_controller)
+ free_irq(platform_get_irq_byname(pdev, "cec-tx"), vc4_hdmi);
+ else
+ free_irq(platform_get_irq(pdev, 0), vc4_hdmi);
+
+err_remove_cec_rx_handler:
+ if (vc4_hdmi->variant->external_irq_controller)
+ free_irq(platform_get_irq_byname(pdev, "cec-rx"), vc4_hdmi);
+
err_delete_cec_adap:
cec_delete_adapter(vc4_hdmi->cec_adap);
@@ -1898,6 +1906,15 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
static void vc4_hdmi_cec_exit(struct vc4_hdmi *vc4_hdmi)
{
+ struct platform_device *pdev = vc4_hdmi->pdev;
+
+ if (vc4_hdmi->variant->external_irq_controller) {
+ free_irq(platform_get_irq_byname(pdev, "cec-rx"), vc4_hdmi);
+ free_irq(platform_get_irq_byname(pdev, "cec-tx"), vc4_hdmi);
+ } else {
+ free_irq(platform_get_irq(pdev, 0), vc4_hdmi);
+ }
+
cec_unregister_adapter(vc4_hdmi->cec_adap);
}
#else
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] drm/vc4: hdmi: Drop devm interrupt handler for hotplug interrupts
2021-07-07 9:51 ` Maxime Ripard
@ 2021-07-07 9:51 ` Maxime Ripard
-1 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-07 9:51 UTC (permalink / raw)
To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard
Cc: Emma Anholt, Hans Verkuil, Daniel Vetter, linux-kernel,
linux-rpi-kernel, bcm-kernel-feedback-list, Dave Stevenson,
Phil Elwell, Tim Gover, Dom Cobley, Maxime Ripard,
Nicolas Saenz Julienne
The hotplugs interrupt handlers are registered through the
devm_request_threaded_irq function. However, while free_irq is indeed
called properly when the device is unbound or bind fails, it's called
after unbind or bind is done.
In our particular case, it means that on failure it creates a window
where our interrupt handler can be called, but we're freeing every
resource (CEC adapter, DRM objects, etc.) it might need.
In order to address this, let's switch to the non-devm variant to
control better when the handler will be unregistered and allow us to
make it safe.
Fixes: f4790083c7c2 ("drm/vc4: hdmi: Rely on interrupts to handle hotplug")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 41 +++++++++++++++++++++++-----------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d966f61966c6..50393a8a42b3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1590,25 +1590,27 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi *vc4_hdmi)
{
struct drm_connector *connector = &vc4_hdmi->connector;
struct platform_device *pdev = vc4_hdmi->pdev;
- struct device *dev = &pdev->dev;
int ret;
if (vc4_hdmi->variant->external_irq_controller) {
- ret = devm_request_threaded_irq(dev,
- platform_get_irq_byname(pdev, "hpd-connected"),
- NULL,
- vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
- "vc4 hdmi hpd connected", vc4_hdmi);
+ unsigned int hpd_con = platform_get_irq_byname(pdev, "hpd-connected");
+ unsigned int hpd_rm = platform_get_irq_byname(pdev, "hpd-removed");
+
+ ret = request_threaded_irq(hpd_con,
+ NULL,
+ vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
+ "vc4 hdmi hpd connected", vc4_hdmi);
if (ret)
return ret;
- ret = devm_request_threaded_irq(dev,
- platform_get_irq_byname(pdev, "hpd-removed"),
- NULL,
- vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
- "vc4 hdmi hpd disconnected", vc4_hdmi);
- if (ret)
+ ret = request_threaded_irq(hpd_rm,
+ NULL,
+ vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
+ "vc4 hdmi hpd disconnected", vc4_hdmi);
+ if (ret) {
+ free_irq(hpd_con, vc4_hdmi);
return ret;
+ }
connector->polled = DRM_CONNECTOR_POLL_HPD;
}
@@ -1616,6 +1618,16 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi *vc4_hdmi)
return 0;
}
+static void vc4_hdmi_hotplug_exit(struct vc4_hdmi *vc4_hdmi)
+{
+ struct platform_device *pdev = vc4_hdmi->pdev;
+
+ if (vc4_hdmi->variant->external_irq_controller) {
+ free_irq(platform_get_irq_byname(pdev, "hpd-connected"), vc4_hdmi);
+ free_irq(platform_get_irq_byname(pdev, "hpd-removed"), vc4_hdmi);
+ }
+}
+
#ifdef CONFIG_DRM_VC4_HDMI_CEC
static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv)
{
@@ -2197,7 +2209,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
ret = vc4_hdmi_cec_init(vc4_hdmi);
if (ret)
- goto err_destroy_conn;
+ goto err_free_hotplug;
ret = vc4_hdmi_audio_init(vc4_hdmi);
if (ret)
@@ -2211,6 +2223,8 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
err_free_cec:
vc4_hdmi_cec_exit(vc4_hdmi);
+err_free_hotplug:
+ vc4_hdmi_hotplug_exit(vc4_hdmi);
err_destroy_conn:
vc4_hdmi_connector_destroy(&vc4_hdmi->connector);
err_destroy_encoder:
@@ -2252,6 +2266,7 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master,
kfree(vc4_hdmi->hd_regset.regs);
vc4_hdmi_cec_exit(vc4_hdmi);
+ vc4_hdmi_hotplug_exit(vc4_hdmi);
vc4_hdmi_connector_destroy(&vc4_hdmi->connector);
drm_encoder_cleanup(&vc4_hdmi->encoder.base.base);
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/3] drm/vc4: hdmi: Drop devm interrupt handler for hotplug interrupts
@ 2021-07-07 9:51 ` Maxime Ripard
0 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-07 9:51 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,
linux-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
Hans Verkuil, Phil Elwell, Dom Cobley
The hotplugs interrupt handlers are registered through the
devm_request_threaded_irq function. However, while free_irq is indeed
called properly when the device is unbound or bind fails, it's called
after unbind or bind is done.
In our particular case, it means that on failure it creates a window
where our interrupt handler can be called, but we're freeing every
resource (CEC adapter, DRM objects, etc.) it might need.
In order to address this, let's switch to the non-devm variant to
control better when the handler will be unregistered and allow us to
make it safe.
Fixes: f4790083c7c2 ("drm/vc4: hdmi: Rely on interrupts to handle hotplug")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 41 +++++++++++++++++++++++-----------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d966f61966c6..50393a8a42b3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1590,25 +1590,27 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi *vc4_hdmi)
{
struct drm_connector *connector = &vc4_hdmi->connector;
struct platform_device *pdev = vc4_hdmi->pdev;
- struct device *dev = &pdev->dev;
int ret;
if (vc4_hdmi->variant->external_irq_controller) {
- ret = devm_request_threaded_irq(dev,
- platform_get_irq_byname(pdev, "hpd-connected"),
- NULL,
- vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
- "vc4 hdmi hpd connected", vc4_hdmi);
+ unsigned int hpd_con = platform_get_irq_byname(pdev, "hpd-connected");
+ unsigned int hpd_rm = platform_get_irq_byname(pdev, "hpd-removed");
+
+ ret = request_threaded_irq(hpd_con,
+ NULL,
+ vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
+ "vc4 hdmi hpd connected", vc4_hdmi);
if (ret)
return ret;
- ret = devm_request_threaded_irq(dev,
- platform_get_irq_byname(pdev, "hpd-removed"),
- NULL,
- vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
- "vc4 hdmi hpd disconnected", vc4_hdmi);
- if (ret)
+ ret = request_threaded_irq(hpd_rm,
+ NULL,
+ vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT,
+ "vc4 hdmi hpd disconnected", vc4_hdmi);
+ if (ret) {
+ free_irq(hpd_con, vc4_hdmi);
return ret;
+ }
connector->polled = DRM_CONNECTOR_POLL_HPD;
}
@@ -1616,6 +1618,16 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi *vc4_hdmi)
return 0;
}
+static void vc4_hdmi_hotplug_exit(struct vc4_hdmi *vc4_hdmi)
+{
+ struct platform_device *pdev = vc4_hdmi->pdev;
+
+ if (vc4_hdmi->variant->external_irq_controller) {
+ free_irq(platform_get_irq_byname(pdev, "hpd-connected"), vc4_hdmi);
+ free_irq(platform_get_irq_byname(pdev, "hpd-removed"), vc4_hdmi);
+ }
+}
+
#ifdef CONFIG_DRM_VC4_HDMI_CEC
static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv)
{
@@ -2197,7 +2209,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
ret = vc4_hdmi_cec_init(vc4_hdmi);
if (ret)
- goto err_destroy_conn;
+ goto err_free_hotplug;
ret = vc4_hdmi_audio_init(vc4_hdmi);
if (ret)
@@ -2211,6 +2223,8 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
err_free_cec:
vc4_hdmi_cec_exit(vc4_hdmi);
+err_free_hotplug:
+ vc4_hdmi_hotplug_exit(vc4_hdmi);
err_destroy_conn:
vc4_hdmi_connector_destroy(&vc4_hdmi->connector);
err_destroy_encoder:
@@ -2252,6 +2266,7 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master,
kfree(vc4_hdmi->hd_regset.regs);
vc4_hdmi_cec_exit(vc4_hdmi);
+ vc4_hdmi_hotplug_exit(vc4_hdmi);
vc4_hdmi_connector_destroy(&vc4_hdmi->connector);
drm_encoder_cleanup(&vc4_hdmi->encoder.base.base);
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm/vc4: hdmi: Only call into DRM framework if registered
2021-07-07 9:51 ` Maxime Ripard
@ 2021-07-07 9:51 ` Maxime Ripard
-1 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-07 9:51 UTC (permalink / raw)
To: dri-devel, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Maxime Ripard
Cc: Emma Anholt, Hans Verkuil, Daniel Vetter, linux-kernel,
linux-rpi-kernel, bcm-kernel-feedback-list, Dave Stevenson,
Phil Elwell, Tim Gover, Dom Cobley, Maxime Ripard,
Nicolas Saenz Julienne
Our hotplug handler will currently call the drm_kms_helper_hotplug_event
every time a hotplug interrupt is called.
However, since the device is registered after all the drivers have
finished their bind callback, we have a window between when we install
our interrupt handler and when drm_dev_register() is eventually called
where our handler can run and call drm_kms_helper_hotplug_event but the
device hasn't been registered yet, causing a null pointer dereference.
Fix this by making sure we only call drm_kms_helper_hotplug_event if our
device has been properly registered.
Fixes: f4790083c7c2 ("drm/vc4: hdmi: Rely on interrupts to handle hotplug")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 50393a8a42b3..31c28252c5f5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1580,7 +1580,7 @@ static irqreturn_t vc4_hdmi_hpd_irq_thread(int irq, void *priv)
struct vc4_hdmi *vc4_hdmi = priv;
struct drm_device *dev = vc4_hdmi->connector.dev;
- if (dev)
+ if (dev && dev->registered)
drm_kms_helper_hotplug_event(dev);
return IRQ_HANDLED;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/3] drm/vc4: hdmi: Only call into DRM framework if registered
@ 2021-07-07 9:51 ` Maxime Ripard
0 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-07 9:51 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,
linux-kernel, bcm-kernel-feedback-list, linux-rpi-kernel,
Hans Verkuil, Phil Elwell, Dom Cobley
Our hotplug handler will currently call the drm_kms_helper_hotplug_event
every time a hotplug interrupt is called.
However, since the device is registered after all the drivers have
finished their bind callback, we have a window between when we install
our interrupt handler and when drm_dev_register() is eventually called
where our handler can run and call drm_kms_helper_hotplug_event but the
device hasn't been registered yet, causing a null pointer dereference.
Fix this by making sure we only call drm_kms_helper_hotplug_event if our
device has been properly registered.
Fixes: f4790083c7c2 ("drm/vc4: hdmi: Rely on interrupts to handle hotplug")
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 50393a8a42b3..31c28252c5f5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1580,7 +1580,7 @@ static irqreturn_t vc4_hdmi_hpd_irq_thread(int irq, void *priv)
struct vc4_hdmi *vc4_hdmi = priv;
struct drm_device *dev = vc4_hdmi->connector.dev;
- if (dev)
+ if (dev && dev->registered)
drm_kms_helper_hotplug_event(dev);
return IRQ_HANDLED;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] drm/vc4: hdmi: Interrupt fixes
2021-07-07 9:51 ` Maxime Ripard
@ 2021-07-07 10:05 ` Dave Stevenson
-1 siblings, 0 replies; 12+ messages in thread
From: Dave Stevenson @ 2021-07-07 10:05 UTC (permalink / raw)
To: Maxime Ripard
Cc: DRI Development, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Emma Anholt, Hans Verkuil, Daniel Vetter,
LKML, linux-rpi-kernel, bcm-kernel-feedback-list, Phil Elwell,
Tim Gover, Dom Cobley, Maxime Ripard, Nicolas Saenz Julienne
Hi Maxime
On Wed, 7 Jul 2021 at 10:51, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> Those are three fixes for race conditions we currently have in the vc4 HDMI
> driver with regard to the interrupts handling.
>
> The first two are fixing an issue where the handler will be removed by devm
> after the resources it uses have been free'd already.
>
> The last one is there to deal with an interrupt coming in the window between
> the end of the driver's bind and the DRM device registration.
>
> Let me know what you think,
> Maxime
For the series
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Maxime Ripard (3):
> drm/vc4: hdmi: Drop devm interrupt handler for CEC interrupts
> drm/vc4: hdmi: Drop devm interrupt handler for hotplug interrupts
> drm/vc4: hdmi: Only call into DRM framework if registered
>
> drivers/gpu/drm/vc4/vc4_hdmi.c | 92 +++++++++++++++++++++++-----------
> 1 file changed, 62 insertions(+), 30 deletions(-)
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] drm/vc4: hdmi: Interrupt fixes
@ 2021-07-07 10:05 ` Dave Stevenson
0 siblings, 0 replies; 12+ messages in thread
From: Dave Stevenson @ 2021-07-07 10:05 UTC (permalink / raw)
To: Maxime Ripard
Cc: Nicolas Saenz Julienne, Emma Anholt, David Airlie, Tim Gover,
LKML, DRI Development, Hans Verkuil, linux-rpi-kernel,
Thomas Zimmermann, bcm-kernel-feedback-list, Daniel Vetter,
Phil Elwell, Dom Cobley
Hi Maxime
On Wed, 7 Jul 2021 at 10:51, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> Those are three fixes for race conditions we currently have in the vc4 HDMI
> driver with regard to the interrupts handling.
>
> The first two are fixing an issue where the handler will be removed by devm
> after the resources it uses have been free'd already.
>
> The last one is there to deal with an interrupt coming in the window between
> the end of the driver's bind and the DRM device registration.
>
> Let me know what you think,
> Maxime
For the series
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Maxime Ripard (3):
> drm/vc4: hdmi: Drop devm interrupt handler for CEC interrupts
> drm/vc4: hdmi: Drop devm interrupt handler for hotplug interrupts
> drm/vc4: hdmi: Only call into DRM framework if registered
>
> drivers/gpu/drm/vc4/vc4_hdmi.c | 92 +++++++++++++++++++++++-----------
> 1 file changed, 62 insertions(+), 30 deletions(-)
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] drm/vc4: hdmi: Interrupt fixes
2021-07-07 10:05 ` Dave Stevenson
@ 2021-07-15 8:50 ` Maxime Ripard
-1 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-15 8:50 UTC (permalink / raw)
To: Dave Stevenson
Cc: DRI Development, Daniel Vetter, David Airlie, Maarten Lankhorst,
Thomas Zimmermann, Emma Anholt, Hans Verkuil, Daniel Vetter,
LKML, linux-rpi-kernel, bcm-kernel-feedback-list, Phil Elwell,
Tim Gover, Dom Cobley, Nicolas Saenz Julienne
[-- Attachment #1: Type: text/plain, Size: 789 bytes --]
On Wed, Jul 07, 2021 at 11:05:12AM +0100, Dave Stevenson wrote:
> Hi Maxime
>
> On Wed, 7 Jul 2021 at 10:51, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > Those are three fixes for race conditions we currently have in the vc4 HDMI
> > driver with regard to the interrupts handling.
> >
> > The first two are fixing an issue where the handler will be removed by devm
> > after the resources it uses have been free'd already.
> >
> > The last one is there to deal with an interrupt coming in the window between
> > the end of the driver's bind and the DRM device registration.
> >
> > Let me know what you think,
> > Maxime
>
> For the series
> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Applied all three patches, thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] drm/vc4: hdmi: Interrupt fixes
@ 2021-07-15 8:50 ` Maxime Ripard
0 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2021-07-15 8:50 UTC (permalink / raw)
To: Dave Stevenson
Cc: Nicolas Saenz Julienne, Emma Anholt, David Airlie, Tim Gover,
LKML, DRI Development, Hans Verkuil, linux-rpi-kernel,
Thomas Zimmermann, bcm-kernel-feedback-list, Daniel Vetter,
Phil Elwell, Dom Cobley
[-- Attachment #1: Type: text/plain, Size: 789 bytes --]
On Wed, Jul 07, 2021 at 11:05:12AM +0100, Dave Stevenson wrote:
> Hi Maxime
>
> On Wed, 7 Jul 2021 at 10:51, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > Those are three fixes for race conditions we currently have in the vc4 HDMI
> > driver with regard to the interrupts handling.
> >
> > The first two are fixing an issue where the handler will be removed by devm
> > after the resources it uses have been free'd already.
> >
> > The last one is there to deal with an interrupt coming in the window between
> > the end of the driver's bind and the DRM device registration.
> >
> > Let me know what you think,
> > Maxime
>
> For the series
> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Applied all three patches, thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread