On Tue, Jun 16, 2015 at 04:10:19PM +0100, Tvrtko Ursulin wrote: > > > On 06/16/2015 02:53 PM, Chris Wilson wrote: > >On Tue, Jun 16, 2015 at 02:32:40PM +0100, Tvrtko Ursulin wrote: > >> > >>On 06/16/2015 12:48 PM, Chris Wilson wrote: > >>>On Tue, Jun 16, 2015 at 12:31:23PM +0100, Tvrtko Ursulin wrote: > >>>>That is partially correct, I do see it as problematic since I > >>>>assumed someone will modeset with this fb/object at some point, and > >>>>there will be state available then, which won't have the cached > >>>>display address at all since the state is not present during fbdev > >>>>setup. > >>>> > >>>>Does that never happens? I mean, the modeset with state using the > >>>>fb/object prepared in intefb_alloc? > >>> > >>>No. The setup in intelfb_alloc is only concerned with generating a GGTT > >>>mmapping that is consistent with later use by modesetting. The important > >>>detail is to make sure the alignment is correct (or else the modeset > >>>will fail as it cannot move the object as it is already pinned). > >>> > >>>As Ville has extracted the linear alignment, we can export that and all > >>>pin_to_display directly so that we can set up the fbdev without the > >>>confusion of calling intel_pin_and_fence_fb. Or we can just live with > >>>the confustion and comment appropriately. > >> > >>Ok, think I get it now. Will send three RFC patches shortly. > >> > >>1/3 looks innocent but it actually a bugfix once display address > >>caching come along. > >> > >>2/3 is the caching itself. > >> > >>3/3 is what is not yet needed today, but analogous to 1/3 it fixes a > >>bug which will become apparent in the future. > >> > >>If this looks more along the lines of what you had in mind I can > >>polish the comments or whatnot. 80 char line breaks were especially > >>ugly in some of them to very long variable names. :) > > > >Not far enough. It's not actually about caching the display address, > >it's about tracking the actual VMA reference allocated for the plane. > > > >What I had in mind and suggested yonks ago is: > > > >http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=nightly&id=2112f72b7adb94b0e038bbafe74a3c1ab6c851cf > > > >Ignore the atomic interface changes, they are from a bygone era. The > >important part is just in tracking vma and the > >simplification/robustification that provides. > > Not bad, looks good to me on the high level. Especially the unpin > path is much nicer with this approach. My only uncertainty is > whether stuffing object pointers into the state is acceptable with > the architect of those parts. Make it a unsigned long cookie then! :) I thought Maarten was thinking of doing callbacks for duplicating state which we could use to keep the pin_count accurate etc. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
