[RFC 1/2] drm/xe: Introduce PM guard classes

[Michal Wajdeczko]

On 17.06.2024 16:37, Lucas De Marchi wrote:
> On Mon, Jun 17, 2024 at 04:01:19PM GMT, Michal Wajdeczko wrote:
>> There is support for 'classes' with constructor and destructor
>> semantics that can be used for scope-based resource management,
>> like our device power management.
>>
>> Use provided macros from linux/cleanup.h to generate required
>> code definitions.
>>
>> This should allow us to use:
>>
>>     scoped_guard(xe_pm, xe)
>>         foo();
>> or
>>     CLASS(xe_pm, var)(xe);
>>
>> without any concern of leaking the pm reference.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_pm.h | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
>> index 104a21ae6dfd..26293a3b18af 100644
>> --- a/drivers/gpu/drm/xe/xe_pm.h
>> +++ b/drivers/gpu/drm/xe/xe_pm.h
>> @@ -6,6 +6,7 @@
>> #ifndef _XE_PM_H_
>> #define _XE_PM_H_
>>
>> +#include <linux/cleanup.h>
> 
> Cool, I didn't know we had  helpers in the kernel for attribute cleanup
> and friends.
> 
>> #include <linux/pm_runtime.h>
>>
>> #define DEFAULT_VRAM_THRESHOLD 300 /* in MB */
>> @@ -33,4 +34,8 @@ int xe_pm_set_vram_threshold(struct xe_device *xe,
>> u32 threshold);
>> void xe_pm_d3cold_allowed_toggle(struct xe_device *xe);
>> struct task_struct *xe_pm_read_callback_task(struct xe_device *xe);
>>
>> +DEFINE_GUARD(xe_pm, struct xe_device *, xe_pm_runtime_get(_T),
>> xe_pm_runtime_put(_T))
> 
> per linux/cleanup.h doc it seems DEFINE_GUARD() was intended
> specifically for locks though:
> 
>  * DEFINE_GUARD(name, type, lock, unlock):
>  *      trivial wrapper around DEFINE_CLASS() above specifically
>  *      for locks.
> 
> we probably shouldn't abuse it or get the doc changed

probably the latter as this is exactly what we want

besides, maybe we can argue that what we do is actually 'locking' since
this is a required step, like obtaining mutex, before we start to use
the device

> 
> Lucas De Marchi
> 
>> +DEFINE_GUARD_COND(xe_pm, _if_active, xe_pm_runtime_get_if_active(_T))
>> +DEFINE_GUARD_COND(xe_pm, _if_in_use, xe_pm_runtime_get_if_in_use(_T))
>> +
>> #endif
>> -- 
>> 2.43.0
>>