[Intel-gfx] [PATCH 3/3] drm/i915: Use intel_plane_obj_offset from more places

[Tvrtko Ursulin]

On 05/28/2015 03:09 PM, Daniel Vetter wrote:
> On Thu, May 28, 2015 at 01:36:54PM +0100, Chris Wilson wrote:
>> On Thu, May 28, 2015 at 02:24:40PM +0200, Daniel Vetter wrote:
>>> On Thu, May 28, 2015 at 09:58:30AM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> On 05/27/2015 10:15 PM, Chris Wilson wrote:
>>>>> On Wed, May 27, 2015 at 10:52:34AM +0100, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>>>
>>>>>> These are the display call sites so should use the proper helper.
>>>>>>
>>>>>> Also requires intel_plane_obj_offset to assume normal view when
>>>>>> plane pointer is not available.
>>>>>
>>>>> Eugh. If only the plane stored the offset, bonus marks for storing the
>>>>> vma cookie, then we would not have to keep recomputing the view and
>>>>> searching every single time...
>>>>
>>>> Well, the patch even decreases the number of searches! :)
>>>>
>>>> And we don't recompute when querying the offset - it just figures out what
>>>> type of vma it should look for. So I think it doesn't prevent any future
>>>> caching improvements. It actually makes it easier since it consolidates the
>>>> query.
>>>>
>>>> I'll need this, or something like it, for some future work. So at the very
>>>> moment I am not too bothered if this goes in or not.
>>>
>>> Yeah unfortunately we can't eliminate the view searches completely since
>>> the view depends upon fb + plane_state. So not perfectly aligned with our
>>> hw ... Otherwise I'd agree, caching the view in the fb would be neat.
>>
>> Not in the fb, in the plane_state. We acquire the vma for the plane
>> during prepare and then we should be using that vma reference for the
>> lifetime of that atomic plane state.
>
> Hm right that should work. And the pin will make sure it won't go poof
> prematurely.

Maybe I could do this, but at the moment I have no idea how that would 
work from intelfb_alloc who calls pin_and_fence with no state.

I could keep intel_plan_obj_offsets at all call sites and either return 
cached (say plane_state->disp_addr) address or "compute" and cache it, 
but, how would I know cached address is valid or not on startup?

I assume plane state is all initialized to zeros? And zero as a display 
address is valid AFAIK?

Do we have a intel_plane_state "constructor" somewhere which could set 
"intel_plane_state->disp_addr = -1", and so later intel_plane_obj_offset 
would know what to do?

And whatever can be done can also be done on top of this patch, which 
actually fixes things from the current state.

Regards,

Tvrtko