[Intel-gfx] [PATCH] snd/hda: Only get/put display_power once

[Chris Wilson]

Quoting Takashi Iwai (2019-04-10 12:03:22)
> On Wed, 10 Apr 2019 12:44:49 +0200,
> Takashi Iwai wrote:
> > 
> > On Wed, 10 Apr 2019 12:24:24 +0200,
> > Chris Wilson wrote:
> > > 
> > > Quoting Takashi Iwai (2019-04-10 11:09:47)
> > > > On Wed, 10 Apr 2019 10:17:33 +0200,
> > > > Chris Wilson wrote:
> > > > > 
> > > > > While we only allow a single display power reference, the current
> > > > > acquisition/release is racy and a direct call may run concurrently with
> > > > > a runtime-pm worker. Prevent the double unreference by atomically
> > > > > tracking the display_power_active cookie.
> > > > > 
> > > > > Testcase: igt/i915_pm_rpm/module-reload #glk-dsi
> > > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > > Cc: Takashi Iwai <tiwai at suse.de>
> > > > > Cc: Imre Deak <imre.deak at intel.com>
> > > > 
> > > > I rather prefer a more straightforward conversion, e.g. something like
> > > > below.  Checking the returned cookie as the state flag is not quite
> > > > intuitive, so revive the boolean state flag, and handle it
> > > > atomically.
> > > 
> > > Access to the cookie itself is not atomic there, and theoretically
> > > there could be a get/put/get running concurrently. Are you sure don't
> > > want a refcount and lock here? :)
> > 
> > The refcount is what we want to reduce, so the suitable option would
> > be a (yet another) mutex although the cmpxchg() should be enough for
> > normal cases.
> > 
> > > Your call. For the case CI is hitting, it should do the trick (as we are
> > > only seeing the race on put/put I think). CI will answer in a hour or
> > > two.
> > 
> > OK, once when it seems working, I'll respin a patch with a mutex
> > instead.  We have already a one that is used for the link state
> > change, and this can be reused.
> 
> It's even simpler, so maybe this is a better way to go...
> 
> If this is confirmed to work, feel free to queue via drm tree.
> I can't apply this because this is on top of your recent cookie and
> sub-component changes that aren't on sound git tree (yet).

Success \o/
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Ok, we'll plonk it in dinq, but I think it should apply to sound.git?
Looks fairly separate.

Anyway that can all be resolved in a later merge if required.

Thanks,
-Chris