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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Mar 12, 2025 at 12:43:42PM +0200, Jani Nikula wrote:
> 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.

Indeed.
I mean, I'm still not in love with the idea of the scope_guard thing, but
at least it is a true kernel thing and getting more traction lately.

Well, in some sense we could also compare them with the for_each, since
they are "imported" concepts of python, rust and other languages right?!

For some reason I like the for_each :)

> 
> 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.

yeap, one possibility is to try to slowly get rid of the ref_tracker? ;)

> 
> 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