On 11/4/2014 12:54 PM, Daniel Vetter wrote:
On Tue, Oct 07, 2014 at 06:10:56PM +0100, Michel Thierry wrote:This is based on the first 55 patches of Ben's 48b addressing work, taking into consideration the latest changes in (mainly aliasing) ppgtt rules. Because of these changes in the tree, the first 17 patches of the original series are no longer needed, and some patches required more rework than others. For GEN8, it has also been extended to work in logical ring submission (lrc) mode, as it looks like it will be the preferred mode of operation. I also tried to update the lrc code at the same time the ppgtt refactoring occurred, leaving only one patch that is exclusively for lrc. I'm asking for comments, as this is the foundation for 48b virtual addressing in Broadwell. This list can be seen in 3 parts: [01-24] Include code rework for PPGTT (all GENs). [25-28] Adds page table allocation for GEN6/GEN7 [29-38] Enables dynamic allocation in GEN8. It is enabled for both legacy and execlist submission modes. Ben Widawsky (37): drm/i915: Add some extra guards in evict_vm drm/i915/trace: Fix offsets for 64b drm/i915: Wrap VMA binding drm/i915: Make pin global flags explicit drm/i915: Split out aliasing binds drm/i915: fix gtt_total_entries() drm/i915: Rename to GEN8_LEGACY_PDPES drm/i915: Split out verbose PPGTT dumping drm/i915: s/pd/pdpe, s/pt/pde drm/i915: rename map/unmap to dma_map/unmap drm/i915: Setup less PPGTT on failed pagedir drm/i915: Un-hardcode number of page directories drm/i915: Make gen6_write_pdes gen6_map_page_tables drm/i915: Range clearing is PPGTT agnostic drm/i915: Page table helpers, and define renames drm/i915: construct page table abstractions drm/i915: Complete page table structures drm/i915: Create page table allocators drm/i915: Generalize GEN6 mapping drm/i915: Clean up pagetable DMA map & unmap drm/i915: Always dma map page table allocations drm/i915: Consolidate dma mappings drm/i915: Always dma map page directory allocations drm/i915: Track GEN6 page table usage drm/i915: Extract context switch skip logic drm/i915: Track page table reload need drm/i915: Initialize all contexts drm/i915: Finish gen6/7 dynamic page table allocation drm/i915/bdw: Use dynamic allocation idioms on free drm/i915/bdw: pagedirs rework allocation drm/i915/bdw: pagetable allocation rework drm/i915/bdw: Make the pdp switch a bit less hacky drm/i915: num_pd_pages/num_pd_entries isn't useful drm/i915: Extract PPGTT param from pagedir alloc drm/i915/bdw: Split out mappings drm/i915/bdw: begin bitmap tracking drm/i915/bdw: Dynamic page table allocations Michel Thierry (1): drm/i915/bdw: Dynamic page table allocations in lrc modeOk, high level review: - The first part of this series seems to shuffle the code around in the vma binding code. If we actually want to fix this I think we need a REBIND flag and push the logic for rewriting ptes into the vma_bind hooks. There's no other way really to fix this, and the breakage for aliasing ppgtt this current code produces is what's blocking the cmd parser atm. - Imo mergin the vma_bind/insert_entries hooks isn't useful, they imo provide good abstraction. Ofc we should move the vma_bind/unbind funcstion into the vm functions, since the vfunc dictionary varies by vm and not by vma. And they only really provide good abstraction if we first fix up the binding mess. - The code massively shuffles around the pte and page table handling code, promising a lot better future. But I simply don't get it - to me this all looks like massive amounts of churn for no clear gain. - Imo the really critical part of dynamic pagetable alloc is where exactly we add this new memory allocation point and how we handle failures. But since there's so much churn that I somehow can't see through I can't have a solid opinion on the proposed design. So overall I think we should untangle this series first and throw out as much churn as possible. E.g. the binding rework is very much separate imo. Or someone needs to explain to my why we need all this code reflow. With that out of the way reviewing the changes due to dynamic page table alloc should be fairly simple. My expectation would have been that we'd add a new interface to allocate vm ranges and essentially leave all the current vma binding unchanged. Having the separate vm range extension is somewhat important since the drm_mm allocator has a bit the tendency to just walk up the address space, wasting piles of free space lower down. Or like I've said maybe I just don't see the real issues and have way too naive opinion here. Cheers, Daniel
Thanks for the comments Daniel,I'll prepare another version, trying to just focus in the dynamic alloc part.
-Michel
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
