From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH] drm/ttm: pass buffer object for bind/unbind callback Date: Sun, 20 Nov 2011 10:30:01 +0100 Message-ID: <4EC8C899.7040104@vmware.com> References: <1321568442-8215-1-git-send-email-j.glisse@gmail.com> <4EC60FEC.3010703@vmware.com> <1321622128.13669.4.camel@nisroch> <4EC66BEB.1090900@vmware.com> <1321637208.18323.7.camel@nisroch> <4EC6E0DA.4060907@vmware.com> <1321662373.18323.15.camel@nisroch> <4EC77FDF.5030206@vmware.com> <1321714385.21551.6.camel@nisroch> <4EC7E0CA.1030806@vmware.com> <4EC80780.8030305@vmware.com> <4EC81911.3030608@vmware.com> <4EC82F8E.8010602@vmware.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1884256298==" Return-path: Received: from smtp-outbound-2.vmware.com (smtp-outbound-2.vmware.com [65.115.85.73]) by gabe.freedesktop.org (Postfix) with ESMTP id E89C39E7B1 for ; Sun, 20 Nov 2011 01:32:47 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Jerome Glisse Cc: Jerome Glisse , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org This is a multi-part message in MIME format. --===============1884256298== Content-Type: multipart/alternative; boundary="------------020100030006070609020709" This is a multi-part message in MIME format. --------------020100030006070609020709 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 11/19/2011 11:54 PM, Jerome Glisse wrote: > >> As mentioned previously, and in the discussion with Ben, the page tables >> would not need to be rebuilt on each CS. They would be rebuilt only on the >> first CS following a move_notify that caused a page table invalidation. >> >> move_notify: >> if (is_incompatible(new_mem_type)) { >> bo->page_tables_invalid = true; >> invalidate_page_tables(bo); >> } >> >> command_submission: >> if (bo->page_tables_invalid) { >> set_up_page_tables(bo); >> bo->page_tables_invalid = false; >> } >> > Why is it different from updating page table in move notify ? I don't > see any bonus here, all the information we need are already available > in move_notify. > > I've iterated the pros of this approach at least two times before, but for completeness let's do it again: 8<---------------------------------------------------------------------------------------------------- 1) TTM doesn't need to care about the driver re-populating its GPU page tables. Since swapin is handled from the tt layer not the bo layer, this makes it a bit easier on us. 2) Transition to page-faulted GPU virtual maps is straightforward and consistent. A non-page-faulting driver sets up the maps at CS time, A pagefaulting driver can set them up directly from an irq handler without reserving, since the bo is properly fenced or pinned when the pagefault happens. 3) A non-page-faulting driver knows at CS time exactly which page-table-entries really do need populating, and can do this more efficiently. 8<----------------------------------------------------------------------------------------------------- And some extra items like partially populated TTMs that were mentioned elsewhere. >> Memory types in TTM are completely orthogonal to allowed GPU usage. The GPU >> may access a bo if it's reserved, fenced or pinned, regardless of its >> placement. >> >> A TT memory type is a *single* GPU aperture that may be mapped from the >> aperture side by the CPU (AGP). It may also be used by a single unmappable >> aperture that wants to use TTM's range management and eviction (vmwgfx GMR). >> The driver can define more than one such memory type (psb), but a bo can >> only be placed in one of those at a time, so this approach is unsuitable for >> multiple apertures pointing to the same pages. >> > radeon virtual memory have a special address space, the system address > space, it's managed by ttm through a TTM_TT (exact same code as > current one). All the other address space are not managed by ttm but > we require a bo to be bound to ttm_tt to be use, thought we can relax > that. That's the reason why i consider system placement as different. > > Yes for Radeon system memory may be different, and that's fine. But as also previously mentioned we're trying to design a generic interface here, in which we need to consider GPU- mappable system memory. I think the pros of this interface design compared to populating in bo_move are pretty well established, so can you please explain why you keep arguing against it? What is it that I have missed? /Thomas --------------020100030006070609020709 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 11/19/2011 11:54 PM, Jerome Glisse wrote:

As mentioned previously, and in the discussion with Ben, the page tables
would not need to be rebuilt on each CS. They would be rebuilt only on the
first CS following a move_notify that caused a page table invalidation.

move_notify:
if (is_incompatible(new_mem_type)) {
 bo->page_tables_invalid = true;
 invalidate_page_tables(bo);
}

command_submission:
if (bo->page_tables_invalid) {
  set_up_page_tables(bo);
  bo->page_tables_invalid = false;
}
    
Why is it different from updating page table in move notify ? I don't
see any bonus here, all the information we need are already available
in move_notify.

  

I've iterated the pros of this approach at least two times before, but for completeness let's do it again:

8<----------------------------------------------------------------------------------------------------
1) TTM doesn't need to care about the driver re-populating its GPU page
tables.
Since swapin is handled from the tt layer not the bo layer, this makes it a
bit easier on us.
2) Transition to page-faulted GPU virtual maps is straightforward and
consistent. A non-page-faulting driver sets up the maps at CS time, A
pagefaulting driver can set them up directly from an irq handler without
reserving, since the bo is properly fenced or pinned when the pagefault
happens.
3) A non-page-faulting driver knows at CS time exactly which
page-table-entries really do need populating, and can do this more
efficiently.
8<-----------------------------------------------------------------------------------------------------

And some extra items like partially populated TTMs that were mentioned elsewhere.


Memory types in TTM are completely orthogonal to allowed GPU usage. The GPU
may access a bo if it's reserved, fenced or pinned, regardless of its
placement.

A TT memory type is a *single* GPU aperture that may be mapped from the
aperture side by the CPU (AGP). It may also be used by a single unmappable
aperture that wants to use TTM's range management and eviction (vmwgfx GMR).
The driver can define more than one such memory type (psb), but a bo can
only be placed in one of those at a time, so this approach is unsuitable for
multiple apertures pointing to the same pages.
    
radeon virtual memory have a special address space, the system address
space, it's managed by ttm through a TTM_TT (exact same code as
current one). All the other address space are not managed by ttm but
we require a bo to be bound to ttm_tt to be use, thought we can relax
that. That's the reason why i consider system placement as different.

  

Yes for Radeon system memory may be different, and that's fine. But as also previously mentioned we're trying to design a generic interface here, in which we need to consider GPU- mappable system memory.

I think the pros of this interface design compared to populating in bo_move are pretty well established, so can you please explain why you keep arguing against it? What is it that I have missed?

/Thomas

--------------020100030006070609020709-- --===============1884256298== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============1884256298==--