Re: [PATCH 3/3] drm/i915: Use intel_plane_obj_offset from more places

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/3] drm/i915: Use intel_plane_obj_offset from more places
Date: Tue, 16 Jun 2015 12:18:49 +0100	[thread overview]
Message-ID: <55800619.2040300@linux.intel.com> (raw)
In-Reply-To: <20150616110200.GP28462@nuc-i3427.alporthouse.com>

On 06/16/2015 12:02 PM, Chris Wilson wrote:
> On Tue, Jun 16, 2015 at 11:17:11AM +0100, Tvrtko Ursulin wrote:
>>
>> 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@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.
>
> fbdev is a little special. All that we actually require is a plain
> ggtt_pin to ensure that the fb is still around for panics. We can leave
> the plane state in modesetting. We have have full control over the
> struct we associated with the ifbdev, so can easily stash the vma in
> there as well.

What do you mean by "We can leave the plane state in modesetting." ?

Where is the connection between ifbdev and plane state ie. our modeset?

>> 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?
>
> They all disappear. Prepare gets the vma, then commit can just use
> vma->node.start + offset (and when the plane_state is released, so is
> the ggtt pinning for the fb).

Yeah they disappear once one figures out how to handle fbdev paths. I am 
not there yet. :)

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx