On 29/5/24 07:52, Vasily Khoruzhick wrote:
If the card doesn't have display hardware, hpd_work and hpd_lock are
left uninitialized which causes BUG when attempting to schedule hpd_work
on runtime PM resume.
Hi,
Good catch, thank you for looking at this. A couple of initial comments
below:
Ben.
Fix it by adding headless flag to DRM and skip any hpd if it's set.
Fixes: ae1aadb1eb8d ("nouveau: don't fail driver load if no display hw
present.")
Link: https://gitlab.freedesktop.org/drm/nouveau/-/issues/337
Signed-off-by: Vasily Khoruzhick <anars...@gmail.com>
---
drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +++
drivers/gpu/drm/nouveau/nouveau_display.c | 11 ++++++++++-
drivers/gpu/drm/nouveau/nouveau_drv.h | 1 +
5 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 13705c5f1497..4b7497a8755c 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -68,7 +68,7 @@ nv04_display_fini(struct drm_device *dev, bool runtime, bool
suspend)
if (nv_two_heads(dev))
NVWriteCRTC(dev, 1, NV_PCRTC_INTR_EN_0, 0);
- if (!runtime)
+ if (!runtime && !drm->headless)
cancel_work_sync(&drm->hpd_work);
if (!suspend)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 88728a0b2c25..674dc567e179 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2680,7 +2680,7 @@ nv50_display_fini(struct drm_device *dev, bool runtime,
bool suspend)
nv50_mstm_fini(nouveau_encoder(encoder));
}
- if (!runtime)
+ if (!runtime && !drm->headless)
cancel_work_sync(&drm->hpd_work);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 856b3ef5edb8..b315a2ef5b28 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1190,6 +1190,9 @@ nouveau_connector_hpd(struct nouveau_connector
*nv_connector, u64 bits)
u32 mask = drm_connector_mask(&nv_connector->base);
unsigned long flags;
+ if (drm->headless)
+ return;
+
You shouldn't need this change, the function can't be called if there's
no display.
spin_lock_irqsave(&drm->hpd_lock, flags);
if (!(drm->hpd_pending & mask)) {
nv_connector->hpd_pending |= bits;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index aed5d5b51b43..1961ef665e97 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -450,6 +450,9 @@ nouveau_display_hpd_resume(struct drm_device *dev)
{
struct nouveau_drm *drm = nouveau_drm(dev);
+ if (drm->headless)
+ return;
+
spin_lock_irq(&drm->hpd_lock);
drm->hpd_pending = ~0;
spin_unlock_irq(&drm->hpd_lock);
@@ -468,6 +471,11 @@ nouveau_display_hpd_work(struct work_struct *work)
int changed = 0;
struct drm_connector *first_changed_connector = NULL;
+ WARN_ON_ONCE(drm->headless);
+
+ if (drm->headless)
+ return;
+
Same here.
pm_runtime_get_sync(dev->dev);
spin_lock_irq(&drm->hpd_lock);
@@ -635,7 +643,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend,
bool runtime)
}
drm_connector_list_iter_end(&conn_iter);
- if (!runtime)
+ if (!runtime && !drm->headless)
cancel_work_sync(&drm->hpd_work);
drm_kms_helper_poll_disable(dev);
@@ -729,6 +737,7 @@ nouveau_display_create(struct drm_device *dev)
/* no display hw */
if (ret == -ENODEV) {
ret = 0;
+ drm->headless = true;
goto disp_create_err;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index e239c6bf4afa..25fca98a20bc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -276,6 +276,7 @@ struct nouveau_drm {
/* modesetting */
struct nvbios vbios;
struct nouveau_display *display;
+ bool headless;
struct work_struct hpd_work;
spinlock_t hpd_lock;
u32 hpd_pending;
