Re: [PATCH 01/10] drm/i915: Add support for tracking wakerefs w/o power-on guarantee

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Imre Deak (2019-05-03 00:26:39)
> @@ -4521,12 +4602,12 @@ void intel_runtime_pm_cleanup(struct drm_i915_private *i915)
>         struct i915_runtime_pm *rpm = &i915->runtime_pm;
>         int count;
>  
> -       count = atomic_fetch_inc(&rpm->wakeref_count); /* balance untrack */
> +       count = atomic_fetch_inc(&rpm->wakeref_track_count); /* balance untrack */
>         WARN(count,
> -            "i915->runtime_pm.wakeref_count=%d on cleanup\n",
> +            "i915->runtime_pm.wakeref_track_count=%d on cleanup\n",
>              count);
>  
> -       untrack_intel_runtime_pm_wakeref(i915);
> +       untrack_intel_runtime_pm_wakeref_raw(i915);

So this basically sums up the dilemma. The warning here should be if the
wakeref_count != 0 (meaning we have someone still using the device) at
cleanup. That track_count may be zero just implies loss of available
information.

Looking through this, I think it would be better if the track_count was
pushed into the runtime_pm.debug substruct, and really does only mean
the number of wakerefs being tracked by the debug backend. I believe that
helps with the separation of intent -- my only condition for the design
is that if you have a wakeref cookie, it is clear that you hold a
reference to the runtime-pm. (Where you want to add new api for
untracked wakerefs... I need to go back to the end again and see if you
really do need untracked wakerefs and if not the async power domain
merely needs to track it own wakeref independently of its clients.)

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