Re: [Intel-xe] [PATCH 2/3] drm/xe: Use spinlock in forcewake instead of mutex

[Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]


On 31/08/23 03:49, Dixit, Ashutosh wrote:

> On Wed, 30 Aug 2023 13:56:57 -0700, Rodrigo Vivi wrote:
>> On Tue, Aug 29, 2023 at 10:33:02PM -0700, Dixit, Ashutosh wrote:
>>> On Tue, 29 Aug 2023 22:15:43 -0700, Aravind Iddamsetty wrote:
>>> Hi Aravind,
>>>
>>>> @@ -162,7 +162,7 @@ int xe_force_wake_get(struct xe_force_wake *fw,
>>>> 				   domain->id, ret);
>>>> 	}
>>>> 	fw->awake_domains |= woken;
>>>> -	mutex_unlock(&fw->lock);
>>>> +	spin_unlock(&fw->lock);
>>> No need to change anything yet, but let's get some more opinion on this: is
>>> it ok to (a) just replace the mutex with a spinlock in these force_wake
>>> functions, or, (b) should we have a second set of functions to be called in
>>> atomic context, say: xe_force_wake_get/put_atomic? So we should use (b) in
>>> atomic contexts and everywhere else we just continue to use the previous
>>> set of non-atomic functions? Or just converting the default set of
>>> functions to use spin lock (as is done in this patch) is ok?
>> It looks okay to me,
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Still thinking about this, maybe some time (not part of this series) we
> should do a power measurement comparison between mutex and spinlock and see
> if there's an appreciable difference (unless we already know?). But till we
> do that, this is fine, so this is also:
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

Thanks Rodrigo and Ashutosh for your r-b.

Regards,

Aravind.

[-- Attachment #2: Type: text/html, Size: 2748 bytes --]