On 03/27/2015 11:47 AM, Chris Wilson wrote:
On Fri, Mar 27, 2015 at 11:40:12AM +0000, Tvrtko Ursulin wrote:
+/* Like above but take and hold the uncore lock for the duration.
+ * Must be used with I915_READ_FW and friends.
+ */
+void intel_uncore_forcewake_irqlock(struct drm_i915_private *dev_priv,
+ enum forcewake_domains domains);
+void intel_uncore_forcewake_irqunlock(struct drm_i915_private *dev_priv,
+ enum forcewake_domains domains);
Oh well I don't like your colour. :)
I would make the comment clearer in saying the function itself will
take the lock and not release it since "take and hold the uncore
lock for the duration" to me reads ambiguous.
"duration of the critical section".
I would like it to be explicit function takes the lock and not that the
caller has to, for the duration of the critical section. Maybe it is my
non-native English but from "take and hold the uncore lock for the
duration.." I am not sure which one of the two it is.
Also, not sure about the _irqlock suffix. It is well established in
spinlocks and the functions even does the opposite from that!
Maybe _get_and_lock / _put_and_unlock, or other way round?
How about _irq_get for the reasons that I don't this to be widely used
elsewhere. We are trading off debugging for performance, that's only
really justifiable inside irqs or busy-waits (and for busy-waits we
already have the notrace variant).
Actually _get_irq/_put_irq.
OK, but comment needs to say that in my opinion.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
