Re: [PATCH 03/24] drm/i915: Rename to GEN8_LEGACY_PDPES

[Dave Gordon <david.s.gordon@intel.com>]

On 18/12/14 20:44, Daniel Vetter wrote:
> On Thu, Dec 18, 2014 at 09:40:51PM +0100, Daniel Vetter wrote:
>> On Thu, Dec 18, 2014 at 05:10:00PM +0000, Michel Thierry wrote:
>>> From: Ben Widawsky <benjamin.widawsky@intel.com>
>>>
>>> In gen8, 32b PPGTT has always had one "pdp" (it doesn't actually have
>>> one, but it resembles having one). The #define was confusing as is, and
>>> using "PDPE" is a much better description.
>>>
>>> sed -i 's/GEN8_LEGACY_PDPS/GEN8_LEGACY_PDPES/' drivers/gpu/drm/i915/*.[ch]
>>
>> Hm generally I've thought the abbreviations are pdp (for the page itself)
>> and pde (for the entries within). I still have no idea what pdpe means ...
>>
>> So either please explain that or pick one of the others.
> 
> In case you fear the rebase pain of renaming this:
> 
> 1. Export entire series as patches with git format-patch.
> 2. sed -e 's/PDPE/PDE/g' on all the patch files
> 3. Import the changed patches into a new fresh branch.'
> 
> That's all. Feels really crazy the first time you do it, but after having
> done this a lot with the internal branch when something random (function
> name or so) changed in upstream it's a fairly simple trick to pull off ;-)
> 
> Cheers, Daniel

The specific #define is inconsistent with the naming used for other
#defines and in the associated comments. Here's the relevant chunk of
i915_gem_gtt.h:

/* GEN8 legacy style address is defined as a 3 level page table:
 * 31:30 | 29:21 | 20:12 |  11:0
 * PDPE  |  PDE  |  PTE  | offset
 * The difference as compared to normal x86 3 level page table is the
PDPEs are
 * programmed via register.
 */
#define GEN8_PDPE_SHIFT                 30
#define GEN8_PDPE_MASK                  0x3
#define GEN8_PDE_SHIFT                  21
#define GEN8_PDE_MASK                   0x1ff
#define GEN8_PTE_SHIFT                  12
#define GEN8_PTE_MASK                   0x1ff
#define GEN8_LEGACY_PDPS                4
#define GEN8_PTES_PER_PAGE              (PAGE_SIZE / sizeof(gen8_gtt_pte_t))
#define GEN8_PDES_PER_PAGE              (PAGE_SIZE /
sizeof(gen8_ppgtt_pde_t))

So 'LEGACY_PDPS' is inconsistent with 'PDPE_SHIFT/MASK'.

PTE  = Page Table Entry
PDE  = Page Directory Entry
PDPE = Page Directory Pointer Entry

See http://www.pagetable.com/?p=308

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