Re: [PATCH 4/6] drm/i915/display: convert to display runtime PM interfaces

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 11 Mar 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> 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@intel.com

> But well, not a blocker:
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks,
Jani.


-- 
Jani Nikula, Intel