[Intel-gfx] [PATCH 55/55] drm/i915: Rename the somewhat reduced i915_gem_object_flush_active()

John Harrison John.C.Harrison at Intel.com
Thu Jun 18 04:03:12 PDT 2015


On 17/06/2015 15:21, Chris Wilson wrote:
> On Wed, Jun 17, 2015 at 04:06:05PM +0200, Daniel Vetter wrote:
>> On Fri, May 29, 2015 at 05:44:16PM +0100, John.C.Harrison at Intel.com wrote:
>>> From: John Harrison <John.C.Harrison at Intel.com>
>>>
>>> The i915_gem_object_flush_active() call used to do lots. Over time it has done
>>> less and less. Now all it does check the various associated requests to see if
>>> they can be retired. Hence this patch renames the function and updates the
>>> comments around it to match the current operation.
>>>
>>> For: VIZ-5115
>>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> When rebasing patches and especially like here when also renaming them a
>> bit please leave some indication of what you've changed. Took me a while
>> to figure out where one of my pending comments from the previous round
>> went too.
>>
>> And please don't just "v2: rebase", but please add some indicators against
>> what it conflicted if it's obvious.
> This function doesn't do an unconditional retire - the new name is much
> worse since it is inconsistent with how requests retire. In my make GEM
> umpteen times faster patches, I repurposed this function for reporting
> the object's current activeness and called it bool i915_gem_oject_active()
>   - though that is probably better as i915_gem_object_is_active().
> -Chris
>

Retiring is generally not an unconditional operation. 
i915_gem_retire_requests[_ring]() does not forcefully retire all 
requests, it only retires stuff that has completed. Same here. It could 
be called i915_gem_object_check_retire() or some such if you prefer. 
Either way, it is better than i915_gem_object_flush_active() as that is 
referring to flushing out the OLR which no longer exists.

If you are adding other functionality back in then feel free to rename 
it again as appropriate. But as this patch series stands, the function 
is a 'retire completed work associated with this object' operation and 
really needs to be named accordingly.



More information about the Intel-gfx mailing list