Re: [PATCH v4 1/4] drm/i915: Fix false-positive assert_rpm_wakelock_held in i915_pmic_bus_access_notifier

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 20-08-17 15:05, Hans de Goede wrote:
Hi,

Note this v4 is send from my gmail in an attempt to keep the
X-Mailer: git-send-email header which the test-infra wants, but
that seems to have failed.

I've also just send another copy through my isp-s mail server,
but I've not received a copy of that copy myself ? If anyone
else has a second copy of this series can you please check if
the git-send-email header is there ?

If not can someone resend this series so that it can go through
the test infra ?

Ok, it seems that this first attempt (sending through gmail)
actually worked, but I could not see the header because it
gets stripped by the RH mail infra on receiving it too...

Anyways this series now has gone through the Fi.CI.BAT
without problems.

If someone can pick these up and push them to drm-intel-next-queued
that would be great.

Regards,

Hans




On 20-08-17 14:51, Hans de Goede wrote:
assert_rpm_wakelock_held is triggered from i915_pmic_bus_access_notifier
even though it gets unregistered on (runtime) suspend, this is caused
by a race happening under the following circumstances:

intel_runtime_pm_put does:

    atomic_dec(&dev_priv->pm.wakeref_count);

    pm_runtime_mark_last_busy(kdev);
    pm_runtime_put_autosuspend(kdev);

And pm_runtime_put_autosuspend calls intel_runtime_suspend from
a workqueue, so there is ample of time between the atomic_dec() and
intel_runtime_suspend() unregistering the notifier. If the notifier
gets called in this windowd assert_rpm_wakelock_held falsely triggers
(at this point we're not runtime-suspended yet).

This commit adds disable_rpm_wakeref_asserts and
enable_rpm_wakeref_asserts calls around the
intel_uncore_forcewake_get(FORCEWAKE_ALL) call in
i915_pmic_bus_access_notifier fixing the false-positive WARN_ON.

Reported-by: FKr <bugs-freedesktop@xxxxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
---
Changes in v2:
-Rebase on current (July 6th 2017) drm-next

Changes in v3:
-Reword comment explaining why disabling the wakeref asserts is
  ok and necessary
-Add Imre's Reviewed-by
---
  drivers/gpu/drm/i915/intel_uncore.c | 7 +++++++
  1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 1d7b879cc68c..2d3aad319229 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1171,8 +1171,15 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb,
           * bus, which will be busy after this notification, leading to:
           * "render: timed out waiting for forcewake ack request."
           * errors.
+         *
+         * The notifier is unregistered during intel_runtime_suspend(),
+         * so it's ok to access the HW here without holding a RPM
+         * wake reference -> disable wakeref asserts for the time of
+         * the access.
           */
+        disable_rpm_wakeref_asserts(dev_priv);
          intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+        enable_rpm_wakeref_asserts(dev_priv);
          break;
      case MBI_PMIC_BUS_ACCESS_END:
          intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux