lkml.org 
[lkml]   [2014]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 09/19] drm/radeon: handle lockup in delayed work, v2
    Am 31.07.2014 um 17:33 schrieb Maarten Lankhorst:
    > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
    > ---
    > V1 had a nasty bug breaking gpu lockup recovery. The fix is not
    > allowing radeon_fence_driver_check_lockup to take exclusive_lock,
    > and kill it during lockup recovery instead.

    That looks like the delayed work starts running as soon as we submit a
    fence, and not when it's needed for waiting.

    Since it's a backup for failing IRQs I would rather put it into
    radeon_irq_kms.c and start/stop it when the IRQs are started/stoped.

    Christian.

    > ---
    > drivers/gpu/drm/radeon/radeon.h | 3 +
    > drivers/gpu/drm/radeon/radeon_device.c | 5 +
    > drivers/gpu/drm/radeon/radeon_fence.c | 124 ++++++++++++++++++--------------
    > drivers/gpu/drm/radeon/radeon_ring.c | 1
    > 4 files changed, 77 insertions(+), 56 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
    > index 60c47f829122..b01d88fc10cb 100644
    > --- a/drivers/gpu/drm/radeon/radeon.h
    > +++ b/drivers/gpu/drm/radeon/radeon.h
    > @@ -347,6 +347,8 @@ struct radeon_fence_driver {
    > uint64_t sync_seq[RADEON_NUM_RINGS];
    > atomic64_t last_seq;
    > bool initialized;
    > + struct delayed_work fence_check_work;
    > + struct radeon_device *rdev;
    > };
    >
    > struct radeon_fence {
    > @@ -360,6 +362,7 @@ struct radeon_fence {
    >
    > int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring);
    > int radeon_fence_driver_init(struct radeon_device *rdev);
    > +void radeon_fence_cancel_delayed_check(struct radeon_device *rdev, int ring);
    > void radeon_fence_driver_fini(struct radeon_device *rdev);
    > void radeon_fence_driver_force_completion(struct radeon_device *rdev);
    > int radeon_fence_emit(struct radeon_device *rdev, struct radeon_fence **fence, int ring);
    > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
    > index 697add2cd4e3..21efd32d07ee 100644
    > --- a/drivers/gpu/drm/radeon/radeon_device.c
    > +++ b/drivers/gpu/drm/radeon/radeon_device.c
    > @@ -1637,6 +1637,11 @@ int radeon_gpu_reset(struct radeon_device *rdev)
    > radeon_save_bios_scratch_regs(rdev);
    > /* block TTM */
    > resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
    > +
    > + /* kill all the hangcheck tests too */
    > + for (i = 0; i < RADEON_NUM_RINGS; ++i)
    > + radeon_fence_cancel_delayed_check(rdev, i);
    > +
    > radeon_pm_suspend(rdev);
    > radeon_suspend(rdev);
    >
    > diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
    > index 913787085dfa..c055acc3a271 100644
    > --- a/drivers/gpu/drm/radeon/radeon_fence.c
    > +++ b/drivers/gpu/drm/radeon/radeon_fence.c
    > @@ -125,16 +125,7 @@ int radeon_fence_emit(struct radeon_device *rdev,
    > return 0;
    > }
    >
    > -/**
    > - * radeon_fence_process - process a fence
    > - *
    > - * @rdev: radeon_device pointer
    > - * @ring: ring index the fence is associated with
    > - *
    > - * Checks the current fence value and wakes the fence queue
    > - * if the sequence number has increased (all asics).
    > - */
    > -void radeon_fence_process(struct radeon_device *rdev, int ring)
    > +static bool __radeon_fence_process(struct radeon_device *rdev, int ring)
    > {
    > uint64_t seq, last_seq, last_emitted;
    > unsigned count_loop = 0;
    > @@ -190,7 +181,51 @@ void radeon_fence_process(struct radeon_device *rdev, int ring)
    > }
    > } while (atomic64_xchg(&rdev->fence_drv[ring].last_seq, seq) > seq);
    >
    > - if (wake)
    > + if (seq < last_emitted)
    > + mod_delayed_work(system_power_efficient_wq,
    > + &rdev->fence_drv[ring].fence_check_work,
    > + RADEON_FENCE_JIFFIES_TIMEOUT);
    > +
    > + return wake;
    > +}
    > +
    > +static void radeon_fence_driver_check_lockup(struct work_struct *work)
    > +{
    > + struct radeon_fence_driver *fence_drv;
    > + struct radeon_device *rdev;
    > + unsigned long iring;
    > +
    > + fence_drv = container_of(work, struct radeon_fence_driver, fence_check_work.work);
    > + rdev = fence_drv->rdev;
    > + iring = fence_drv - &rdev->fence_drv[0];
    > +
    > + if (__radeon_fence_process(rdev, iring))
    > + wake_up_all(&rdev->fence_queue);
    > + else if (radeon_ring_is_lockup(rdev, iring, &rdev->ring[iring])) {
    > + /* good news we believe it's a lockup */
    > + dev_warn(rdev->dev, "GPU lockup (current fence id "
    > + "0x%016llx last fence id 0x%016llx on ring %ld)\n",
    > + (uint64_t)atomic64_read(&fence_drv->last_seq),
    > + fence_drv->sync_seq[iring], iring);
    > +
    > + /* remember that we need an reset */
    > + rdev->needs_reset = true;
    > + wake_up_all(&rdev->fence_queue);
    > + }
    > +}
    > +
    > +/**
    > + * radeon_fence_process - process a fence
    > + *
    > + * @rdev: radeon_device pointer
    > + * @ring: ring index the fence is associated with
    > + *
    > + * Checks the current fence value and wakes the fence queue
    > + * if the sequence number has increased (all asics).
    > + */
    > +void radeon_fence_process(struct radeon_device *rdev, int ring)
    > +{
    > + if (__radeon_fence_process(rdev, ring))
    > wake_up_all(&rdev->fence_queue);
    > }
    >
    > @@ -302,9 +337,10 @@ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq,
    > {
    > uint64_t last_seq[RADEON_NUM_RINGS];
    > bool signaled;
    > - int i, r;
    > + int i;
    >
    > while (!radeon_fence_any_seq_signaled(rdev, target_seq)) {
    > + long r;
    >
    > /* Save current sequence values, used to check for GPU lockups */
    > for (i = 0; i < RADEON_NUM_RINGS; ++i) {
    > @@ -319,11 +355,11 @@ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq,
    > if (intr) {
    > r = wait_event_interruptible_timeout(rdev->fence_queue, (
    > (signaled = radeon_fence_any_seq_signaled(rdev, target_seq))
    > - || rdev->needs_reset), RADEON_FENCE_JIFFIES_TIMEOUT);
    > + || rdev->needs_reset), MAX_SCHEDULE_TIMEOUT);
    > } else {
    > r = wait_event_timeout(rdev->fence_queue, (
    > (signaled = radeon_fence_any_seq_signaled(rdev, target_seq))
    > - || rdev->needs_reset), RADEON_FENCE_JIFFIES_TIMEOUT);
    > + || rdev->needs_reset), MAX_SCHEDULE_TIMEOUT);
    > }
    >
    > for (i = 0; i < RADEON_NUM_RINGS; ++i) {
    > @@ -334,50 +370,11 @@ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq,
    > trace_radeon_fence_wait_end(rdev->ddev, i, target_seq[i]);
    > }
    >
    > - if (unlikely(r < 0))
    > + if (r < 0)
    > return r;
    >
    > - if (unlikely(!signaled)) {
    > - if (rdev->needs_reset)
    > - return -EDEADLK;
    > -
    > - /* we were interrupted for some reason and fence
    > - * isn't signaled yet, resume waiting */
    > - if (r)
    > - continue;
    > -
    > - for (i = 0; i < RADEON_NUM_RINGS; ++i) {
    > - if (!target_seq[i])
    > - continue;
    > -
    > - if (last_seq[i] != atomic64_read(&rdev->fence_drv[i].last_seq))
    > - break;
    > - }
    > -
    > - if (i != RADEON_NUM_RINGS)
    > - continue;
    > -
    > - for (i = 0; i < RADEON_NUM_RINGS; ++i) {
    > - if (!target_seq[i])
    > - continue;
    > -
    > - if (radeon_ring_is_lockup(rdev, i, &rdev->ring[i]))
    > - break;
    > - }
    > -
    > - if (i < RADEON_NUM_RINGS) {
    > - /* good news we believe it's a lockup */
    > - dev_warn(rdev->dev, "GPU lockup (waiting for "
    > - "0x%016llx last fence id 0x%016llx on"
    > - " ring %d)\n",
    > - target_seq[i], last_seq[i], i);
    > -
    > - /* remember that we need an reset */
    > - rdev->needs_reset = true;
    > - wake_up_all(&rdev->fence_queue);
    > - return -EDEADLK;
    > - }
    > - }
    > + if (rdev->needs_reset)
    > + return -EDEADLK;
    > }
    > return 0;
    > }
    > @@ -711,6 +708,8 @@ static void radeon_fence_driver_init_ring(struct radeon_device *rdev, int ring)
    > rdev->fence_drv[ring].sync_seq[i] = 0;
    > atomic64_set(&rdev->fence_drv[ring].last_seq, 0);
    > rdev->fence_drv[ring].initialized = false;
    > + INIT_DELAYED_WORK(&rdev->fence_drv[ring].fence_check_work, radeon_fence_driver_check_lockup);
    > + rdev->fence_drv[ring].rdev = rdev;
    > }
    >
    > /**
    > @@ -740,6 +739,17 @@ int radeon_fence_driver_init(struct radeon_device *rdev)
    > }
    >
    > /**
    > + * radeon_fence_cancel_delayed_check - cancel all delayed checks on a ring during lockup
    > + *
    > + * This prevents the lockup check from being done while suspend is running
    > + * during a recovery from a lockup.
    > + */
    > +void radeon_fence_cancel_delayed_check(struct radeon_device *rdev, int ring)
    > +{
    > + cancel_delayed_work_sync(&rdev->fence_drv[ring].fence_check_work);
    > +}
    > +
    > +/**
    > * radeon_fence_driver_fini - tear down the fence driver
    > * for all possible rings.
    > *
    > @@ -755,6 +765,8 @@ void radeon_fence_driver_fini(struct radeon_device *rdev)
    > for (ring = 0; ring < RADEON_NUM_RINGS; ring++) {
    > if (!rdev->fence_drv[ring].initialized)
    > continue;
    > +
    > + cancel_delayed_work_sync(&rdev->fence_drv[ring].fence_check_work);
    > r = radeon_fence_wait_empty(rdev, ring);
    > if (r) {
    > /* no need to trigger GPU reset as we are unloading */
    > diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
    > index f8050f5429e2..594d871a6fce 100644
    > --- a/drivers/gpu/drm/radeon/radeon_ring.c
    > +++ b/drivers/gpu/drm/radeon/radeon_ring.c
    > @@ -261,6 +261,7 @@ int radeon_ib_ring_tests(struct radeon_device *rdev)
    >
    > r = radeon_ib_test(rdev, i, ring);
    > if (r) {
    > + radeon_fence_cancel_delayed_check(rdev, i);
    > ring->ready = false;
    > rdev->needs_reset = false;
    >
    >



    \
     
     \ /
      Last update: 2014-08-01 19:21    [W:4.313 / U:0.880 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site