[Intel-gfx] [PATCH 1/7] drm/i915: Lift runtime-pm acquire callbacks out of intel_wakeref.mutex

[Andi Shyti]

Hi Jani,

[...]

> >  	 * upon acquiring the wakeref.
> >  	 */
> >  	mutex_lock_nested(&wf->mutex, SINGLE_DEPTH_NESTING);
> > -	if (!atomic_read(&wf->count)) {
> > -		int err;
> >  
> > -		rpm_get(wf);
> > +	if (likely(!atomic_read(&wf->count))) {
> 
> Adding the likely should be a separate patch with rationale, not a
> random drive-by change. (And maybe it just should not be added at all.)

Agree, this can be made in a separate patch.

> > +		INTEL_WAKEREF_BUG_ON(wf->wakeref);
> > +		wf->wakeref = fetch_and_zero(&wakeref);
> 
> fetch_and_zero() should just die. All it does here is make things more
> confusing, not less. Please don't add new users.
> 
> The get and put helpers could probably stay, modified, to make this more
> readable.

it actually looks straight forward to me and even more
understandable. get/put are OK if there are multiple users, but
when used in such simple context it looks a bit of an overkill.

Especially when we don't need anymore the actions taken bu get
and put.

So that replacing the pointer with NULL is a natural process, no?

Andi