Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH 6.1.y-cip 1/5] [PARTIAL BACKPORT]drm/i915: Fix HPD polling, reenabling the output poll work as needed
       [not found] <20250723125427.59324-1-nicusor.huhulea@siemens.com>
@ 2025-07-23 12:54 ` Nicusor Huhulea
  2025-07-23 12:54 ` [PATCH 6.1.y-cip 2/5] [PARTIAL BACKPORT]drm: Add an HPD poll helper to reschedule the poll work Nicusor Huhulea
  1 sibling, 0 replies; 4+ messages in thread
From: Nicusor Huhulea @ 2025-07-23 12:54 UTC (permalink / raw)
  To: cip-dev
  Cc: Imre Deak, stable, Dmitry Baryshkov, dri-devel,
	Jouni Högander, Rodrigo Vivi, Nicusor Huhulea

From: Imre Deak <imre.deak@intel.com>

After the commit in the Fixes: line below, HPD polling stopped working
on i915, since after that change calling drm_kms_helper_poll_enable()
doesn't restart drm_mode_config::output_poll_work if the work was
stopped (no connectors needing polling) and enabling polling for a
connector (during runtime suspend or detecting an HPD IRQ storm).

After the above change calling drm_kms_helper_poll_enable() is a nop
after it's been called already and polling for some connectors was
disabled/re-enabled.

Fix this by calling drm_kms_helper_poll_reschedule() added in the
previous patch instead, which reschedules the work whenever expected.

Fixes: d33a54e3991d ("drm/probe_helper: sort out poll_running vs poll_enabled")
CC: stable@vger.kernel.org # 6.4+
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230822113015.41224-2-imre.deak@intel.com
(cherry picked from commit 50452f2f76852322620b63e62922b85e955abe94)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Partial-Backport-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>
---
 drivers/gpu/drm/i915/display/intel_hotplug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index f7a2f485b177c..6ba2d7b0cd1b7 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -208,7 +208,7 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv)
 
 	/* Enable polling and queue hotplug re-enabling. */
 	if (hpd_disabled) {
-		drm_kms_helper_poll_enable(dev);
+		drm_kms_helper_poll_reschedule(&dev_priv->drm);
 		mod_delayed_work(system_wq, &dev_priv->display.hotplug.reenable_work,
 				 msecs_to_jiffies(HPD_STORM_REENABLE_DELAY));
 	}
@@ -638,7 +638,7 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
 	drm_connector_list_iter_end(&conn_iter);
 
 	if (enabled)
-		drm_kms_helper_poll_enable(dev);
+		drm_kms_helper_poll_reschedule(&dev_priv->drm);
 
 	mutex_unlock(&dev->mode_config.mutex);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 6.1.y-cip 2/5] [PARTIAL BACKPORT]drm: Add an HPD poll helper to reschedule the poll work
       [not found] <20250723125427.59324-1-nicusor.huhulea@siemens.com>
  2025-07-23 12:54 ` [PATCH 6.1.y-cip 1/5] [PARTIAL BACKPORT]drm/i915: Fix HPD polling, reenabling the output poll work as needed Nicusor Huhulea
@ 2025-07-23 12:54 ` Nicusor Huhulea
  2025-07-23 13:38   ` Greg KH
  1 sibling, 1 reply; 4+ messages in thread
From: Nicusor Huhulea @ 2025-07-23 12:54 UTC (permalink / raw)
  To: cip-dev
  Cc: Imre Deak, stable, Dmitry Baryshkov, dri-devel,
	Jouni Högander, Rodrigo Vivi, Nicusor Huhulea

From: Imre Deak <imre.deak@intel.com>

Add a helper to reschedule drm_mode_config::output_poll_work after
polling has been enabled for a connector (and needing a reschedule,
since previously polling was disabled for all connectors and hence
output_poll_work was not running).

This is needed by the next patch fixing HPD polling on i915.

CC: stable@vger.kernel.org # 6.4+
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230822113015.41224-1-imre.deak@intel.com
(cherry picked from commit fe2352fd64029918174de4b460dfe6df0c6911cd)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Partial-Backport-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 74 +++++++++++++++++++-----------
 include/drm/drm_probe_helper.h     |  1 +
 2 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 0e5eadc6d44de..787f6699971f1 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -224,6 +224,26 @@ drm_connector_mode_valid(struct drm_connector *connector,
 }
 
 #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
+static void reschedule_output_poll_work(struct drm_device *dev)
+{
+	unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
+
+	if (dev->mode_config.delayed_event)
+		/*
+		 * FIXME:
+		 *
+		 * Use short (1s) delay to handle the initial delayed event.
+		 * This delay should not be needed, but Optimus/nouveau will
+		 * fail in a mysterious way if the delayed event is handled as
+		 * soon as possible like it is done in
+		 * drm_helper_probe_single_connector_modes() in case the poll
+		 * was enabled before.
+		 */
+		delay = HZ;
+
+	schedule_delayed_work(&dev->mode_config.output_poll_work, delay);
+}
+
 /**
  * drm_kms_helper_poll_enable - re-enable output polling.
  * @dev: drm_device
@@ -244,43 +264,45 @@ drm_connector_mode_valid(struct drm_connector *connector,
  */
 void drm_kms_helper_poll_enable(struct drm_device *dev)
 {
-	bool poll = false;
+
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
-	unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
 
 	if (drm_WARN_ON_ONCE(dev, !dev->mode_config.poll_enabled) ||
 	    !drm_kms_helper_poll || dev->mode_config.poll_running)
 		return;
 
-	drm_connector_list_iter_begin(dev, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
-		if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
-					 DRM_CONNECTOR_POLL_DISCONNECT))
-			poll = true;
-	}
-	drm_connector_list_iter_end(&conn_iter);
+	if (drm_kms_helper_enable_hpd(dev) ||
+	    dev->mode_config.delayed_event)
+		reschedule_output_poll_work(dev);
 
-	if (dev->mode_config.delayed_event) {
-		/*
-		 * FIXME:
-		 *
-		 * Use short (1s) delay to handle the initial delayed event.
-		 * This delay should not be needed, but Optimus/nouveau will
-		 * fail in a mysterious way if the delayed event is handled as
-		 * soon as possible like it is done in
-		 * drm_helper_probe_single_connector_modes() in case the poll
-		 * was enabled before.
-		 */
-		poll = true;
-		delay = HZ;
-	}
-
-	if (poll)
-		schedule_delayed_work(&dev->mode_config.output_poll_work, delay);
+	dev->mode_config.poll_running = true;
 }
 EXPORT_SYMBOL(drm_kms_helper_poll_enable);
 
+/**
+ * drm_kms_helper_poll_reschedule - reschedule the output polling work
+ * @dev: drm_device
+ *
+ * This function reschedules the output polling work, after polling for a
+ * connector has been enabled.
+ *
+ * Drivers must call this helper after enabling polling for a connector by
+ * setting %DRM_CONNECTOR_POLL_CONNECT / %DRM_CONNECTOR_POLL_DISCONNECT flags
+ * in drm_connector::polled. Note that after disabling polling by clearing these
+ * flags for a connector will stop the output polling work automatically if
+ * the polling is disabled for all other connectors as well.
+ *
+ * The function can be called only after polling has been enabled by calling
+ * drm_kms_helper_poll_init() / drm_kms_helper_poll_enable().
+ */
+void drm_kms_helper_poll_reschedule(struct drm_device *dev)
+{
+	if (dev->mode_config.poll_running)
+		reschedule_output_poll_work(dev);
+}
+EXPORT_SYMBOL(drm_kms_helper_poll_reschedule);
+
 static enum drm_connector_status
 drm_helper_probe_detect_ctx(struct drm_connector *connector, bool force)
 {
diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h
index 5880daa146240..429a85f38036a 100644
--- a/include/drm/drm_probe_helper.h
+++ b/include/drm/drm_probe_helper.h
@@ -25,6 +25,7 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector);
 
 void drm_kms_helper_poll_disable(struct drm_device *dev);
 void drm_kms_helper_poll_enable(struct drm_device *dev);
+void drm_kms_helper_poll_reschedule(struct drm_device *dev);
 bool drm_kms_helper_is_poll_worker(void);
 
 enum drm_mode_status drm_crtc_helper_mode_valid_fixed(struct drm_crtc *crtc,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 6.1.y-cip 2/5] [PARTIAL BACKPORT]drm: Add an HPD poll helper to reschedule the poll work
  2025-07-23 12:54 ` [PATCH 6.1.y-cip 2/5] [PARTIAL BACKPORT]drm: Add an HPD poll helper to reschedule the poll work Nicusor Huhulea
@ 2025-07-23 13:38   ` Greg KH
  2025-07-23 17:18     ` nicusor.huhulea
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2025-07-23 13:38 UTC (permalink / raw)
  To: Nicusor Huhulea
  Cc: cip-dev, Imre Deak, stable, Dmitry Baryshkov, dri-devel,
	Jouni Högander, Rodrigo Vivi

On Wed, Jul 23, 2025 at 03:54:24PM +0300, Nicusor Huhulea wrote:
> From: Imre Deak <imre.deak@intel.com>
> 
> Add a helper to reschedule drm_mode_config::output_poll_work after
> polling has been enabled for a connector (and needing a reschedule,
> since previously polling was disabled for all connectors and hence
> output_poll_work was not running).
> 
> This is needed by the next patch fixing HPD polling on i915.
> 
> CC: stable@vger.kernel.org # 6.4+
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20230822113015.41224-1-imre.deak@intel.com
> (cherry picked from commit fe2352fd64029918174de4b460dfe6df0c6911cd)
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Partial-Backport-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>

What does "Partial-Backport-by:" mean?  I don't see that in the
documentation files as a valid tag to put in kernel commits :(

confused,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 6.1.y-cip 2/5] [PARTIAL BACKPORT]drm: Add an HPD poll helper to reschedule the poll work
  2025-07-23 13:38   ` Greg KH
@ 2025-07-23 17:18     ` nicusor.huhulea
  0 siblings, 0 replies; 4+ messages in thread
From: nicusor.huhulea @ 2025-07-23 17:18 UTC (permalink / raw)
  To: Greg KH
  Cc: cip-dev@lists.cip-project.org, Imre Deak, stable@vger.kernel.org,
	Dmitry Baryshkov, dri-devel@lists.freedesktop.org,
	Jouni Högander, Rodrigo Vivi

thank you for your review! :)

I'll refine the series as I forgot to add all the authors/participants, therefore the confusion on the context/issue


Nicu

________________________________________
From: Greg KH <gregkh@linuxfoundation.org>
Sent: Wednesday, July 23, 2025 4:38 PM
To: Huhulea, Nicusor Liviu (FT FDS CES LX PBU 1)
Cc: cip-dev@lists.cip-project.org; Imre Deak; stable@vger.kernel.org; Dmitry Baryshkov; dri-devel@lists.freedesktop.org; Jouni Högander; Rodrigo Vivi
Subject: Re: [PATCH 6.1.y-cip 2/5] [PARTIAL BACKPORT]drm: Add an HPD poll helper to reschedule the poll work

On Wed, Jul 23, 2025 at 03:54:24PM +0300, Nicusor Huhulea wrote:
> From: Imre Deak <imre.deak@intel.com>
>
> Add a helper to reschedule drm_mode_config::output_poll_work after
> polling has been enabled for a connector (and needing a reschedule,
> since previously polling was disabled for all connectors and hence
> output_poll_work was not running).
>
> This is needed by the next patch fixing HPD polling on i915.
>
> CC: stable@vger.kernel.org # 6.4+
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20230822113015.41224-1-imre.deak@intel.com
> (cherry picked from commit fe2352fd64029918174de4b460dfe6df0c6911cd)
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Partial-Backport-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>

What does "Partial-Backport-by:" mean?  I don't see that in the
documentation files as a valid tag to put in kernel commits :(

confused,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-23 17:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250723125427.59324-1-nicusor.huhulea@siemens.com>
2025-07-23 12:54 ` [PATCH 6.1.y-cip 1/5] [PARTIAL BACKPORT]drm/i915: Fix HPD polling, reenabling the output poll work as needed Nicusor Huhulea
2025-07-23 12:54 ` [PATCH 6.1.y-cip 2/5] [PARTIAL BACKPORT]drm: Add an HPD poll helper to reschedule the poll work Nicusor Huhulea
2025-07-23 13:38   ` Greg KH
2025-07-23 17:18     ` nicusor.huhulea

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox