Skip to content

Commit 19625e8

Browse files
Lyudedanvet
authored andcommitted
drm/i915: Enable polling when we don't have hpd
Unfortunately, there's two situations where we lose hpd right now: - Runtime suspend - When we've shut off all of the power wells on Valleyview/Cherryview While it would be nice if this didn't cause issues, this has the ability to get us in some awkward states where a user won't be able to get their display to turn on. For instance; if we boot a Valleyview system without any monitors connected, it won't need any of it's power wells and thus shut them off. Since this causes us to lose HPD, this means that unless the user knows how to ssh into their machine and do a manual reprobe for monitors, none of the monitors they connect after booting will actually work. Eventually we should come up with a better fix then having to enable polling for this, since this makes rpm a lot less useful, but for now the infrastructure in i915 just isn't there yet to get hpd in these situations. Changes since v1: - Add comment explaining the addition of the if (!mode_config->poll_running) in intel_hpd_init() - Remove unneeded if (!dev->mode_config.poll_enabled) in i915_hpd_poll_init_work() - Call to drm_helper_hpd_irq_event() after we disable polling - Add cancel_work_sync() call to intel_hpd_cancel_work() Changes since v2: - Apparently dev->mode_config.poll_running doesn't actually reflect whether or not a poll is currently in progress, and is actually used for dynamic module paramter enabling/disabling. So now we instead keep track of our own poll_running variable in dev_priv->hotplug - Clean i915_hpd_poll_init_work() a little bit Changes since v3: - Remove the now-redundant connector loop in intel_hpd_init(), just rely on intel_hpd_poll_enable() for setting connector->polled correctly on each connector - Get rid of poll_running - Don't assign enabled in i915_hpd_poll_init_work before we actually lock dev->mode_config.mutex - Wrap enabled assignment in i915_hpd_poll_init_work() in READ_ONCE() for doc purposes - Do the same for dev_priv->hotplug.poll_enabled with WRITE_ONCE in intel_hpd_poll_enable() - Add some comments about racing not mattering in intel_hpd_poll_enable Changes since v4: - Rename intel_hpd_poll_enable() to intel_hpd_poll_init() - Drop the bool argument from intel_hpd_poll_init() - Remove redundant calls to intel_hpd_poll_init() - Rename poll_enable_work to poll_init_work - Add some kerneldoc for intel_hpd_poll_init() - Cross-reference intel_hpd_poll_init() in intel_hpd_init() - Just copy the loop from intel_hpd_init() in intel_hpd_poll_init() Changes since v5: - Minor kerneldoc nitpicks Cc: [email protected] Cc: Ville Syrjälä <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Signed-off-by: Lyude <[email protected]> Signed-off-by: Daniel Vetter <[email protected]>
1 parent b236d7c commit 19625e8

File tree

5 files changed

+88
-12
lines changed

5 files changed

+88
-12
lines changed

drivers/gpu/drm/i915/i915_drv.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2409,6 +2409,9 @@ static int intel_runtime_suspend(struct device *device)
24092409

24102410
assert_forcewakes_inactive(dev_priv);
24112411

2412+
if (!IS_VALLEYVIEW(dev_priv) || !IS_CHERRYVIEW(dev_priv))
2413+
intel_hpd_poll_init(dev_priv);
2414+
24122415
DRM_DEBUG_KMS("Device suspended\n");
24132416
return 0;
24142417
}

drivers/gpu/drm/i915/i915_drv.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,9 @@ struct i915_hotplug {
284284
u32 short_port_mask;
285285
struct work_struct dig_port_work;
286286

287+
struct work_struct poll_init_work;
288+
bool poll_enabled;
289+
287290
/*
288291
* if we get a HPD irq from DP and a HPD irq from non-DP
289292
* the non-DP HPD could block the workqueue on a mode config

drivers/gpu/drm/i915/intel_drv.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,6 +1425,8 @@ int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector);
14251425

14261426
/* intel_dvo.c */
14271427
void intel_dvo_init(struct drm_device *dev);
1428+
/* intel_hotplug.c */
1429+
void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
14281430

14291431

14301432
/* legacy fbdev emulation in intel_fbdev.c */

drivers/gpu/drm/i915/intel_hotplug.c

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -452,20 +452,47 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
452452
*
453453
* This is a separate step from interrupt enabling to simplify the locking rules
454454
* in the driver load and resume code.
455+
*
456+
* Also see: intel_hpd_poll_init(), which enables connector polling
455457
*/
456458
void intel_hpd_init(struct drm_i915_private *dev_priv)
457459
{
458-
struct drm_device *dev = &dev_priv->drm;
459-
struct drm_mode_config *mode_config = &dev->mode_config;
460-
struct drm_connector *connector;
461460
int i;
462461

463462
for_each_hpd_pin(i) {
464463
dev_priv->hotplug.stats[i].count = 0;
465464
dev_priv->hotplug.stats[i].state = HPD_ENABLED;
466465
}
466+
467+
WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
468+
schedule_work(&dev_priv->hotplug.poll_init_work);
469+
470+
/*
471+
* Interrupt setup is already guaranteed to be single-threaded, this is
472+
* just to make the assert_spin_locked checks happy.
473+
*/
474+
spin_lock_irq(&dev_priv->irq_lock);
475+
if (dev_priv->display.hpd_irq_setup)
476+
dev_priv->display.hpd_irq_setup(dev_priv);
477+
spin_unlock_irq(&dev_priv->irq_lock);
478+
}
479+
480+
void i915_hpd_poll_init_work(struct work_struct *work) {
481+
struct drm_i915_private *dev_priv =
482+
container_of(work, struct drm_i915_private,
483+
hotplug.poll_init_work);
484+
struct drm_device *dev = &dev_priv->drm;
485+
struct drm_mode_config *mode_config = &dev->mode_config;
486+
struct drm_connector *connector;
487+
bool enabled;
488+
489+
mutex_lock(&dev->mode_config.mutex);
490+
491+
enabled = READ_ONCE(dev_priv->hotplug.poll_enabled);
492+
467493
list_for_each_entry(connector, &mode_config->connector_list, head) {
468-
struct intel_connector *intel_connector = to_intel_connector(connector);
494+
struct intel_connector *intel_connector =
495+
to_intel_connector(connector);
469496
connector->polled = intel_connector->polled;
470497

471498
/* MST has a dynamic intel_connector->encoder and it's reprobing
@@ -474,24 +501,62 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
474501
continue;
475502

476503
if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
477-
intel_connector->encoder->hpd_pin > HPD_NONE)
478-
connector->polled = DRM_CONNECTOR_POLL_HPD;
504+
intel_connector->encoder->hpd_pin > HPD_NONE) {
505+
connector->polled = enabled ?
506+
DRM_CONNECTOR_POLL_CONNECT |
507+
DRM_CONNECTOR_POLL_DISCONNECT :
508+
DRM_CONNECTOR_POLL_HPD;
509+
}
479510
}
480511

512+
if (enabled)
513+
drm_kms_helper_poll_enable_locked(dev);
514+
515+
mutex_unlock(&dev->mode_config.mutex);
516+
481517
/*
482-
* Interrupt setup is already guaranteed to be single-threaded, this is
483-
* just to make the assert_spin_locked checks happy.
518+
* We might have missed any hotplugs that happened while we were
519+
* in the middle of disabling polling
484520
*/
485-
spin_lock_irq(&dev_priv->irq_lock);
486-
if (dev_priv->display.hpd_irq_setup)
487-
dev_priv->display.hpd_irq_setup(dev_priv);
488-
spin_unlock_irq(&dev_priv->irq_lock);
521+
if (!enabled)
522+
drm_helper_hpd_irq_event(dev);
523+
}
524+
525+
/**
526+
* intel_hpd_poll_init - enables/disables polling for connectors with hpd
527+
* @dev_priv: i915 device instance
528+
* @enabled: Whether to enable or disable polling
529+
*
530+
* This function enables polling for all connectors, regardless of whether or
531+
* not they support hotplug detection. Under certain conditions HPD may not be
532+
* functional. On most Intel GPUs, this happens when we enter runtime suspend.
533+
* On Valleyview and Cherryview systems, this also happens when we shut off all
534+
* of the powerwells.
535+
*
536+
* Since this function can get called in contexts where we're already holding
537+
* dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
538+
* worker.
539+
*
540+
* Also see: intel_hpd_init(), which restores hpd handling.
541+
*/
542+
void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
543+
{
544+
WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
545+
546+
/*
547+
* We might already be holding dev->mode_config.mutex, so do this in a
548+
* seperate worker
549+
* As well, there's no issue if we race here since we always reschedule
550+
* this worker anyway
551+
*/
552+
schedule_work(&dev_priv->hotplug.poll_init_work);
489553
}
490554

491555
void intel_hpd_init_work(struct drm_i915_private *dev_priv)
492556
{
493557
INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
494558
INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func);
559+
INIT_WORK(&dev_priv->hotplug.poll_init_work, i915_hpd_poll_init_work);
495560
INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
496561
intel_hpd_irq_storm_reenable_work);
497562
}
@@ -508,6 +573,7 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
508573

509574
cancel_work_sync(&dev_priv->hotplug.dig_port_work);
510575
cancel_work_sync(&dev_priv->hotplug.hotplug_work);
576+
cancel_work_sync(&dev_priv->hotplug.poll_init_work);
511577
cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
512578
}
513579

drivers/gpu/drm/i915/intel_runtime_pm.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,8 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
11331133
synchronize_irq(dev_priv->drm.irq);
11341134

11351135
intel_power_sequencer_reset(dev_priv);
1136+
1137+
intel_hpd_poll_init(dev_priv);
11361138
}
11371139

11381140
static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,

0 commit comments

Comments
 (0)