The dw_hdmi_setup_rx_sense exported function should not use struct device
to recover the dw-hdmi context using drvdata, but take struct dw_hdmi
directly like other exported functions.
This caused a regression using Meson DRM on S905X since v4.17-rc1 :
Internal error: Oops: 96000007 [#1] PREEMPT SMP
[...]
CPU: 0 PID: 124 Comm: irq/32-dw_hdmi_ Not tainted 4.17.0-rc7 #2
Hardware name: Libre Technology CC (DT)
[...]
pc : osq_lock+0x54/0x188
lr : __mutex_lock.isra.0+0x74/0x530
[...]
Process irq/32-dw_hdmi_ (pid: 124, stack limit = 0x00000000adf418cb)
Call trace:
osq_lock+0x54/0x188
__mutex_lock_slowpath+0x10/0x18
mutex_lock+0x30/0x38
__dw_hdmi_setup_rx_sense+0x28/0x98
dw_hdmi_setup_rx_sense+0x10/0x18
dw_hdmi_top_thread_irq+0x2c/0x50
irq_thread_fn+0x28/0x68
irq_thread+0x10c/0x1a0
kthread+0x128/0x130
ret_from_fork+0x10/0x18
Code: 34000964 d00050a2 51000484 9135c042 (f864d844)
---[ end trace 945641e1fbbc07da ]---
note: irq/32-dw_hdmi_[124] exited with preempt_count 1
genirq: exiting task "irq/32-dw_hdmi_" (124) is an active IRQ thread (irq 32)
Fixes: eea034af90c6 ("drm/bridge/synopsys: dw-hdmi: don't clobber drvdata")
Signed-off-by: Neil Armstrong <[email protected]>
Tested-by: Koen Kooi <[email protected]>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 15 ++++-----------
drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +-
include/drm/bridge/dw_hdmi.h | 2 +-
3 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ec8d000..3c136f2b 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2077,7 +2077,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
return ret;
}
-void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
+void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
{
mutex_lock(&hdmi->mutex);
@@ -2103,13 +2103,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
}
mutex_unlock(&hdmi->mutex);
}
-
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
-{
- struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
- __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
-}
EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
@@ -2145,9 +2138,9 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
*/
if (intr_stat &
(HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
- __dw_hdmi_setup_rx_sense(hdmi,
- phy_stat & HDMI_PHY_HPD,
- phy_stat & HDMI_PHY_RX_SENSE);
+ dw_hdmi_setup_rx_sense(hdmi,
+ phy_stat & HDMI_PHY_HPD,
+ phy_stat & HDMI_PHY_RX_SENSE);
if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
cec_notifier_set_phys_addr(hdmi->cec_notifier,
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index a393095..c9ad456 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -529,7 +529,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
if (stat & HDMITX_TOP_INTR_HPD_RISE)
hpd_connected = true;
- dw_hdmi_setup_rx_sense(dw_hdmi->dev, hpd_connected,
+ dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
hpd_connected);
drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index dd2a8cf..ccb5aa8 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -151,7 +151,7 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
struct drm_encoder *encoder,
const struct dw_hdmi_plat_data *plat_data);
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
+void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
--
2.7.4
On Wed, May 30, 2018 at 11:43:58AM +0200, Neil Armstrong wrote:
> The dw_hdmi_setup_rx_sense exported function should not use struct device
> to recover the dw-hdmi context using drvdata, but take struct dw_hdmi
> directly like other exported functions.
>
> This caused a regression using Meson DRM on S905X since v4.17-rc1 :
>
> Internal error: Oops: 96000007 [#1] PREEMPT SMP
> [...]
> CPU: 0 PID: 124 Comm: irq/32-dw_hdmi_ Not tainted 4.17.0-rc7 #2
> Hardware name: Libre Technology CC (DT)
> [...]
> pc : osq_lock+0x54/0x188
> lr : __mutex_lock.isra.0+0x74/0x530
> [...]
> Process irq/32-dw_hdmi_ (pid: 124, stack limit = 0x00000000adf418cb)
> Call trace:
> osq_lock+0x54/0x188
> __mutex_lock_slowpath+0x10/0x18
> mutex_lock+0x30/0x38
> __dw_hdmi_setup_rx_sense+0x28/0x98
> dw_hdmi_setup_rx_sense+0x10/0x18
> dw_hdmi_top_thread_irq+0x2c/0x50
> irq_thread_fn+0x28/0x68
> irq_thread+0x10c/0x1a0
> kthread+0x128/0x130
> ret_from_fork+0x10/0x18
> Code: 34000964 d00050a2 51000484 9135c042 (f864d844)
> ---[ end trace 945641e1fbbc07da ]---
> note: irq/32-dw_hdmi_[124] exited with preempt_count 1
> genirq: exiting task "irq/32-dw_hdmi_" (124) is an active IRQ thread (irq 32)
>
> Fixes: eea034af90c6 ("drm/bridge/synopsys: dw-hdmi: don't clobber drvdata")
> Signed-off-by: Neil Armstrong <[email protected]>
> Tested-by: Koen Kooi <[email protected]>
Thanks for your patch, Neil. I've applied it to -misc-fixes and will send it to
Dave in hope it can sneak into 4.17.
Sean
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 15 ++++-----------
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +-
> include/drm/bridge/dw_hdmi.h | 2 +-
> 3 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ec8d000..3c136f2b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2077,7 +2077,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
> return ret;
> }
>
> -void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> +void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> {
> mutex_lock(&hdmi->mutex);
>
> @@ -2103,13 +2103,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> }
> mutex_unlock(&hdmi->mutex);
> }
> -
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
> -{
> - struct dw_hdmi *hdmi = dev_get_drvdata(dev);
> -
> - __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
> -}
> EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>
> static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> @@ -2145,9 +2138,9 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> */
> if (intr_stat &
> (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
> - __dw_hdmi_setup_rx_sense(hdmi,
> - phy_stat & HDMI_PHY_HPD,
> - phy_stat & HDMI_PHY_RX_SENSE);
> + dw_hdmi_setup_rx_sense(hdmi,
> + phy_stat & HDMI_PHY_HPD,
> + phy_stat & HDMI_PHY_RX_SENSE);
>
> if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
> cec_notifier_set_phys_addr(hdmi->cec_notifier,
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index a393095..c9ad456 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -529,7 +529,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
> if (stat & HDMITX_TOP_INTR_HPD_RISE)
> hpd_connected = true;
>
> - dw_hdmi_setup_rx_sense(dw_hdmi->dev, hpd_connected,
> + dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
> hpd_connected);
>
> drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index dd2a8cf..ccb5aa8 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -151,7 +151,7 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
> struct drm_encoder *encoder,
> const struct dw_hdmi_plat_data *plat_data);
>
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
> +void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
>
> void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
> void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Sean Paul, Software Engineer, Google / Chromium OS
On Wednesday 30 May 2018 03:13 PM, Neil Armstrong wrote:
> The dw_hdmi_setup_rx_sense exported function should not use struct device
> to recover the dw-hdmi context using drvdata, but take struct dw_hdmi
> directly like other exported functions.
>
> This caused a regression using Meson DRM on S905X since v4.17-rc1 :
Reviewed-by: Archit Taneja <[email protected]>
>
> Internal error: Oops: 96000007 [#1] PREEMPT SMP
> [...]
> CPU: 0 PID: 124 Comm: irq/32-dw_hdmi_ Not tainted 4.17.0-rc7 #2
> Hardware name: Libre Technology CC (DT)
> [...]
> pc : osq_lock+0x54/0x188
> lr : __mutex_lock.isra.0+0x74/0x530
> [...]
> Process irq/32-dw_hdmi_ (pid: 124, stack limit = 0x00000000adf418cb)
> Call trace:
> osq_lock+0x54/0x188
> __mutex_lock_slowpath+0x10/0x18
> mutex_lock+0x30/0x38
> __dw_hdmi_setup_rx_sense+0x28/0x98
> dw_hdmi_setup_rx_sense+0x10/0x18
> dw_hdmi_top_thread_irq+0x2c/0x50
> irq_thread_fn+0x28/0x68
> irq_thread+0x10c/0x1a0
> kthread+0x128/0x130
> ret_from_fork+0x10/0x18
> Code: 34000964 d00050a2 51000484 9135c042 (f864d844)
> ---[ end trace 945641e1fbbc07da ]---
> note: irq/32-dw_hdmi_[124] exited with preempt_count 1
> genirq: exiting task "irq/32-dw_hdmi_" (124) is an active IRQ thread (irq 32)
>
> Fixes: eea034af90c6 ("drm/bridge/synopsys: dw-hdmi: don't clobber drvdata")
> Signed-off-by: Neil Armstrong <[email protected]>
> Tested-by: Koen Kooi <[email protected]>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 15 ++++-----------
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 +-
> include/drm/bridge/dw_hdmi.h | 2 +-
> 3 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ec8d000..3c136f2b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2077,7 +2077,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
> return ret;
> }
>
> -void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> +void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> {
> mutex_lock(&hdmi->mutex);
>
> @@ -2103,13 +2103,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> }
> mutex_unlock(&hdmi->mutex);
> }
> -
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
> -{
> - struct dw_hdmi *hdmi = dev_get_drvdata(dev);
> -
> - __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
> -}
> EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>
> static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> @@ -2145,9 +2138,9 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> */
> if (intr_stat &
> (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
> - __dw_hdmi_setup_rx_sense(hdmi,
> - phy_stat & HDMI_PHY_HPD,
> - phy_stat & HDMI_PHY_RX_SENSE);
> + dw_hdmi_setup_rx_sense(hdmi,
> + phy_stat & HDMI_PHY_HPD,
> + phy_stat & HDMI_PHY_RX_SENSE);
>
> if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
> cec_notifier_set_phys_addr(hdmi->cec_notifier,
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index a393095..c9ad456 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -529,7 +529,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
> if (stat & HDMITX_TOP_INTR_HPD_RISE)
> hpd_connected = true;
>
> - dw_hdmi_setup_rx_sense(dw_hdmi->dev, hpd_connected,
> + dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
> hpd_connected);
>
> drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index dd2a8cf..ccb5aa8 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -151,7 +151,7 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
> struct drm_encoder *encoder,
> const struct dw_hdmi_plat_data *plat_data);
>
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
> +void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
>
> void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
> void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
>