On Tue, 11 Mar 2025, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
> On Tue, Mar 11, 2025 at 02:05:38PM +0200, Jani Nikula wrote:
>> - wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>> - is_enabled = intel_display_power_well_is_enabled(display,
>> - power_well_id);
>> - intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>> + with_intel_display_rpm(display)
>> + is_enabled = intel_display_power_well_is_enabled(display,
>> + power_well_id);
>>
>
> looking this here... I really dislike the 'with_' macro...
> I really prefer the explicit get and put, even with the ref_tracker
> declaration.
We might consider defining our own guard classes for runtime PM and
other things, and use scoped_guard() and guard() with them.
Something like:
DEFINE_GUARD(display_rpm, struct intel_display *, intel_display_rpm_get(_T), intel_display_rpm_put(_T))
And the above code would become:
scoped_guard(display_rpm, display) {
// ...
}
which is already gaining a lot of traction in kernel:
$ git grep scoped_guard | wc -l
527
It's still magic, but at least it's kernel common magic, not our own.
Additionally, you could use:
guard(display_rpm)(display);
which automatically releases the reference when going out of scope.
I'm not quite sure how to plug that into the ref_tracker, though, so
need to give it some more thought.
I sent an RFC about using guard() and scoped_guard() for HDCP mutexes
[1] to demonstrate this with the pre-defined mutex guard class.
[1] https://lore.kernel.org/r/20250224101428.204519-1-jani.nikula@xxxxxxxxx
> But well, not a blocker:
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Thanks,
Jani.
--
Jani Nikula, Intel
