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

[Chris Wilson]

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

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.

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

-- 
Chris Wilson, Intel Open Source Technology Centre