public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Lyude Paul <lyude@redhat.com>
To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	nouveau@lists.freedesktop.org
Cc: Ben Skeggs <bskeggs@redhat.com>, David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel@ffwll.ch>,
	linux-kernel@vger.kernel.org (open list)
Subject: [RFC v4 12/20] drm/nouveau/kms: Only use hpd_work for reprobing in HPD paths
Date: Tue, 25 Aug 2020 15:50:19 -0400	[thread overview]
Message-ID: <20200825195027.74681-13-lyude@redhat.com> (raw)
In-Reply-To: <20200825195027.74681-1-lyude@redhat.com>

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>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 42 +++++--------
 drivers/gpu/drm/nouveau/nouveau_connector.h |  1 +
 drivers/gpu/drm/nouveau/nouveau_display.c   | 70 ++++++++++++++++++---
 drivers/gpu/drm/nouveau/nouveau_display.h   |  1 +
 drivers/gpu/drm/nouveau/nouveau_dp.c        |  2 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c       |  4 +-
 drivers/gpu/drm/nouveau/nouveau_drv.h       |  2 +
 7 files changed, 86 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 4a29f691c08e4..637e91594fbe8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -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)
 {
@@ -1147,8 +1161,6 @@ 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) {
@@ -1156,31 +1168,9 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
 		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;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h
index d6de5cb8e2238..d0b859c4a80ea 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -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;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 13016769a194b..bceb48a2dfca6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -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);
 }
 
@@ -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",
@@ -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);
@@ -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);
@@ -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);
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index 76ba93e01aff4..616c434270594 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -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);
diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c b/drivers/gpu/drm/nouveau/nouveau_dp.c
index 201c0b4335563..71d095409c90d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dp.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
@@ -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);
 	}
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 22d246acc5e52..42fc5c813a9bb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -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;
 }
@@ -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;
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index f63ac72aa556f..73ebf5fba2fcd 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -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
-- 
2.26.2


  parent reply	other threads:[~2020-08-25 19:52 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200825195027.74681-1-lyude@redhat.com>
2020-08-25 19:50 ` [RFC v4 01/20] drm/nouveau/kms: Fix some indenting in nouveau_dp_detect() Lyude Paul
2020-08-25 19:50 ` [RFC v4 02/20] drm/nouveau/kms/nv50-: Remove open-coded drm_dp_read_desc() Lyude Paul
2020-08-25 19:50 ` [RFC v4 03/20] drm/nouveau/kms/nv50-: Just use drm_dp_dpcd_read() in nouveau_dp.c Lyude Paul
2020-08-25 19:50 ` [RFC v4 04/20] drm/nouveau/kms/nv50-: Use macros for DP registers " Lyude Paul
2020-08-25 19:50 ` [RFC v4 05/20] drm/nouveau/kms: Don't clear DP_MST_CTRL DPCD in nv50_mstm_new() Lyude Paul
2020-08-25 19:50 ` [RFC v4 06/20] drm/nouveau/kms: Search for encoders' connectors properly Lyude Paul
2020-08-25 19:50 ` [RFC v4 07/20] drm/nouveau/kms/nv50-: Use drm_dp_dpcd_(readb|writeb)() in nv50_sor_disable() Lyude Paul
2020-08-25 19:50 ` [RFC v4 08/20] drm/nouveau/kms/nv50-: Refactor and cleanup DP HPD handling Lyude Paul
2020-08-25 19:50 ` [RFC v4 09/20] drm/i915/dp: Extract drm_dp_has_mst() Lyude Paul
2020-08-26  6:51   ` Jani Nikula
2020-08-25 19:50 ` [RFC v4 10/20] drm/nouveau/kms: Use new drm_dp_has_mst() helper for checking MST caps Lyude Paul
2020-08-25 19:50 ` [RFC v4 11/20] drm/nouveau/kms: Move drm_dp_cec_unset_edid() into nouveau_connector_detect() Lyude Paul
2020-08-25 19:50 ` Lyude Paul [this message]
2020-08-25 19:50 ` [RFC v4 13/20] drm/i915/dp: Extract drm_dp_downstream_read_info() Lyude Paul
2020-08-25 19:50 ` [RFC v4 14/20] drm/nouveau/kms/nv50-: Use downstream DP clock limits for mode validation Lyude Paul
2020-08-25 19:50 ` [RFC v4 15/20] drm/i915/dp: Extract drm_dp_has_sink_count() Lyude Paul
2020-08-25 19:50 ` [RFC v4 16/20] drm/i915/dp: Extract drm_dp_get_sink_count() Lyude Paul
2020-08-26  7:05   ` Jani Nikula
2020-08-26 17:10     ` Lyude Paul
2020-08-26 17:12     ` Lyude Paul
2020-08-25 19:50 ` [RFC v4 17/20] drm/nouveau/kms/nv50-: Add support for DP_SINK_COUNT Lyude Paul
2020-08-25 19:50 ` [RFC v4 18/20] drm/nouveau/kms: Don't change EDID when it hasn't actually changed Lyude Paul
2020-08-25 19:50 ` [RFC v4 19/20] drm/i915/dp: Extract drm_dp_read_dpcd_caps() Lyude Paul
2020-08-25 19:50 ` [RFC v4 20/20] drm/nouveau/kms: Start using drm_dp_read_dpcd_caps() Lyude Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200825195027.74681-13-lyude@redhat.com \
    --to=lyude@redhat.com \
    --cc=airlied@linux.ie \
    --cc=bskeggs@redhat.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox