Skip to content

Commit

Permalink
drm/nouveau/kms: Only use hpd_work for reprobing in HPD paths
Browse files Browse the repository at this point in the history
Currently we perform both short IRQ handling for DP, and connector
reprobing in the HPD IRQ handler. However since we need to grab
connection_mutex in order to reprobe a connector, in theory we could
accidentally block ourselves from handling any short IRQs until after a
modeset completes if a connector hotplug happens to occur in parallel
with a modeset.

I haven't seen this actually happen yet, but since we're cleaning up
nouveau's hotplug handling code anyway and we already have a hpd worker,
we can simply fix this by only relying on the HPD worker to actually
reprobe connectors when we receive a HPD IRQ. We also add a mask to
nouveau_drm to keep track of which connectors are waiting to be reprobed
in response to an HPD IRQ.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Ben Skeggs <bskeggs@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200826182456.322681-13-lyude@redhat.com
  • Loading branch information
Lyude Paul committed Aug 31, 2020
1 parent 02bb7fe commit d297ce4
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 36 deletions.
42 changes: 16 additions & 26 deletions drivers/gpu/drm/nouveau/nouveau_connector.c
Original file line number Diff line number Diff line change
Expand Up @@ -1138,6 +1138,20 @@ nouveau_connector_funcs_lvds = {
.early_unregister = nouveau_connector_early_unregister,
};

void
nouveau_connector_hpd(struct drm_connector *connector)
{
struct nouveau_drm *drm = nouveau_drm(connector->dev);
u32 mask = drm_connector_mask(connector);

mutex_lock(&drm->hpd_lock);
if (!(drm->hpd_pending & mask)) {
drm->hpd_pending |= mask;
schedule_work(&drm->hpd_work);
}
mutex_unlock(&drm->hpd_lock);
}

static int
nouveau_connector_hotplug(struct nvif_notify *notify)
{
Expand All @@ -1147,40 +1161,16 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
struct drm_device *dev = connector->dev;
struct nouveau_drm *drm = nouveau_drm(dev);
const struct nvif_notify_conn_rep_v0 *rep = notify->data;
const char *name = connector->name;
int ret;
bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);

if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) {
nouveau_dp_irq(drm, nv_connector);
return NVIF_NOTIFY_KEEP;
}

ret = pm_runtime_get(drm->dev->dev);
if (ret == 0) {
/* We can't block here if there's a pending PM request
* running, as we'll deadlock nouveau_display_fini() when it
* calls nvif_put() on our nvif_notify struct. So, simply
* defer the hotplug event until the device finishes resuming
*/
NV_DEBUG(drm, "Deferring HPD on %s until runtime resume\n",
name);
schedule_work(&drm->hpd_work);

pm_runtime_put_noidle(drm->dev->dev);
return NVIF_NOTIFY_KEEP;
} else if (ret != 1 && ret != -EACCES) {
NV_WARN(drm, "HPD on %s dropped due to RPM failure: %d\n",
name, ret);
return NVIF_NOTIFY_DROP;
}

NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name);

drm_helper_hpd_irq_event(connector->dev);
NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", connector->name);
nouveau_connector_hpd(connector);

pm_runtime_mark_last_busy(drm->dev->dev);
pm_runtime_put_autosuspend(drm->dev->dev);
return NVIF_NOTIFY_KEEP;
}

Expand Down
1 change: 1 addition & 0 deletions drivers/gpu/drm/nouveau/nouveau_connector.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)

struct drm_connector *
nouveau_connector_create(struct drm_device *, const struct dcb_output *);
void nouveau_connector_hpd(struct drm_connector *connector);

extern int nouveau_tv_disable;
extern int nouveau_ignorelid;
Expand Down
70 changes: 63 additions & 7 deletions drivers/gpu/drm/nouveau/nouveau_display.c
Original file line number Diff line number Diff line change
Expand Up @@ -457,16 +457,70 @@ static struct nouveau_drm_prop_enum_list dither_depth[] = {
} \
} while(0)

void
nouveau_display_hpd_resume(struct drm_device *dev)
{
struct nouveau_drm *drm = nouveau_drm(dev);

mutex_lock(&drm->hpd_lock);
drm->hpd_pending = ~0;
mutex_unlock(&drm->hpd_lock);

schedule_work(&drm->hpd_work);
}

static void
nouveau_display_hpd_work(struct work_struct *work)
{
struct nouveau_drm *drm = container_of(work, typeof(*drm), hpd_work);
struct drm_device *dev = drm->dev;
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
u32 pending;
bool changed = false;

pm_runtime_get_sync(dev->dev);

pm_runtime_get_sync(drm->dev->dev);
mutex_lock(&drm->hpd_lock);
pending = drm->hpd_pending;
drm->hpd_pending = 0;
mutex_unlock(&drm->hpd_lock);

drm_helper_hpd_irq_event(drm->dev);
/* Nothing to do, exit early without updating the last busy counter */
if (!pending)
goto noop;

mutex_lock(&dev->mode_config.mutex);
drm_connector_list_iter_begin(dev, &conn_iter);

nouveau_for_each_non_mst_connector_iter(connector, &conn_iter) {
enum drm_connector_status old_status = connector->status;
u64 old_epoch_counter = connector->epoch_counter;

if (!(pending & drm_connector_mask(connector)))
continue;

connector->status = drm_helper_probe_detect(connector, NULL,
false);
if (old_epoch_counter == connector->epoch_counter)
continue;

changed = true;
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] status updated from %s to %s (epoch counter %llu->%llu)\n",
connector->base.id, connector->name,
drm_get_connector_status_name(old_status),
drm_get_connector_status_name(connector->status),
old_epoch_counter, connector->epoch_counter);
}

drm_connector_list_iter_end(&conn_iter);
mutex_unlock(&dev->mode_config.mutex);

if (changed)
drm_kms_helper_hotplug_event(dev);

pm_runtime_mark_last_busy(drm->dev->dev);
noop:
pm_runtime_put_sync(drm->dev->dev);
}

Expand All @@ -490,12 +544,11 @@ nouveau_display_acpi_ntfy(struct notifier_block *nb, unsigned long val,
*/
pm_runtime_put_autosuspend(drm->dev->dev);
} else if (ret == 0) {
/* This may be the only indication we receive
* of a connector hotplug on a runtime
* suspended GPU, schedule hpd_work to check.
/* We've started resuming the GPU already, so
* it will handle scheduling a full reprobe
* itself
*/
NV_DEBUG(drm, "ACPI requested connector reprobe\n");
schedule_work(&drm->hpd_work);
pm_runtime_put_noidle(drm->dev->dev);
} else {
NV_WARN(drm, "Dropped ACPI reprobe event due to RPM error: %d\n",
Expand Down Expand Up @@ -686,6 +739,7 @@ nouveau_display_create(struct drm_device *dev)
}

INIT_WORK(&drm->hpd_work, nouveau_display_hpd_work);
mutex_init(&drm->hpd_lock);
#ifdef CONFIG_ACPI
drm->acpi_nb.notifier_call = nouveau_display_acpi_ntfy;
register_acpi_notifier(&drm->acpi_nb);
Expand All @@ -705,9 +759,10 @@ void
nouveau_display_destroy(struct drm_device *dev)
{
struct nouveau_display *disp = nouveau_display(dev);
struct nouveau_drm *drm = nouveau_drm(dev);

#ifdef CONFIG_ACPI
unregister_acpi_notifier(&nouveau_drm(dev)->acpi_nb);
unregister_acpi_notifier(&drm->acpi_nb);
#endif

drm_kms_helper_poll_fini(dev);
Expand All @@ -719,6 +774,7 @@ nouveau_display_destroy(struct drm_device *dev)
nvif_disp_dtor(&disp->disp);

nouveau_drm(dev)->display = NULL;
mutex_destroy(&drm->hpd_lock);
kfree(disp);
}

Expand Down
1 change: 1 addition & 0 deletions drivers/gpu/drm/nouveau/nouveau_display.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ nouveau_display(struct drm_device *dev)
int nouveau_display_create(struct drm_device *dev);
void nouveau_display_destroy(struct drm_device *dev);
int nouveau_display_init(struct drm_device *dev, bool resume, bool runtime);
void nouveau_display_hpd_resume(struct drm_device *dev);
void nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime);
int nouveau_display_suspend(struct drm_device *dev, bool runtime);
void nouveau_display_resume(struct drm_device *dev, bool runtime);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/nouveau/nouveau_dp.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ void nouveau_dp_irq(struct nouveau_drm *drm,

if (mstm && mstm->is_mst) {
if (!nv50_mstm_service(drm, nv_connector, mstm))
schedule_work(&drm->hpd_work);
nouveau_connector_hpd(connector);
} else {
drm_dp_cec_irq(&nv_connector->aux);
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/nouveau/nouveau_drm.c
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ nouveau_pmops_resume(struct device *dev)
ret = nouveau_do_resume(drm_dev, false);

/* Monitors may have been connected / disconnected during suspend */
schedule_work(&nouveau_drm(drm_dev)->hpd_work);
nouveau_display_hpd_resume(drm_dev);

return ret;
}
Expand Down Expand Up @@ -1036,7 +1036,7 @@ nouveau_pmops_runtime_resume(struct device *dev)
drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;

/* Monitors may have been connected / disconnected during suspend */
schedule_work(&nouveau_drm(drm_dev)->hpd_work);
nouveau_display_hpd_resume(drm_dev);

return ret;
}
Expand Down
2 changes: 2 additions & 0 deletions drivers/gpu/drm/nouveau/nouveau_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ struct nouveau_drm {
struct nvbios vbios;
struct nouveau_display *display;
struct work_struct hpd_work;
struct mutex hpd_lock;
u32 hpd_pending;
struct work_struct fbcon_work;
int fbcon_new_state;
#ifdef CONFIG_ACPI
Expand Down

0 comments on commit d297ce4

Please sign in to comment.