On 8/12/2021 1:41 PM, Michel Dänzer wrote:
On 2021-08-12 7:55 a.m., Koenig, Christian wrote:
Hi James,
Evan seems to have understood how this all works together.
See while any begin/end use critical section is active the work should not be
active.
When you handle only one ring you can just call cancel in begin use and
schedule in end use. But when you have more than one ring you need a lock or
counter to prevent concurrent work items to be started.
Michelle's idea to use mod_delayed_work is a bad one because it assumes that
the delayed work is still running.
It merely assumes that the work may already have been scheduled before.
Admittedly, I missed the cancel_delayed_work_sync calls for patch 2. While I
think it can still have some effect when there's a single work item for
multiple rings, as described by James, it's probably negligible, since
presumably the time intervals between ring_begin_use and ring_end_use are
normally much shorter than a second.
So, while patch 2 is at worst a no-op (since mod_delayed_work is the same as
schedule_delayed_work if the work hasn't been scheduled yet), I'm fine with
dropping it.
Something similar applies to the first patch I think,
There are no cancel work calls in that case, so the commit log is accurate
TTBOMK.
Curious -
For patch 1, does it make a difference if any delayed work scheduled is
cancelled in the else part before proceeding?
} else if (!enable && adev->gfx.gfx_off_state) {
cancel_delayed_work();
Thanks,
Lijo
I noticed this because current mutter Git main wasn't able to sustain 60 fps on
Navi 14 with a simple glxgears -fullscreen. mutter was dropping frames because
its CPU work for a frame update occasionally took up to 3 ms, instead of the
normal 2-300 microseconds. sysprof showed a lot of cycles spent in the
functions which enable/disable GFXOFF in the HW.
so when this makes a difference it is actually a bug.
There was certainly a bug though, which patch 1 fixes. :)
