Re: [Intel-gfx] [PATCH 14/26] drm/i915: Complete page table structures

2014-03-22 Thread Ben Widawsky
On Tue, Mar 18, 2014 at 09:09:45AM +, Chris Wilson wrote:
 On Mon, Mar 17, 2014 at 10:48:46PM -0700, Ben Widawsky wrote:
  Move the remaining members over to the new page table structures.
  
  This can be squashed with the previous commit if desire. The reasoning
  is the same as that patch. I simply felt it is easier to review if split.
 
 I'm not liking the shorter names much. Is there precedence elsewhere
 (e.g. daddr)?
 -Chris
 

I'm not particularly attached to daddr. It was fun to say in my head.
A lot of code does use daddr but it seems to vary between dma,
device, data, destination Not exactly precedence.

Initially I had the prefix p[td]_daddr, but I thought you might complain
about it because it's implicit. dma_addr seemed kinda redundant to me.

Recommendation?

 -- 
 Chris Wilson, Intel Open Source Technology Centre
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 14/26] drm/i915: Complete page table structures

2014-03-22 Thread Chris Wilson
On Sat, Mar 22, 2014 at 01:10:47PM -0700, Ben Widawsky wrote:
 On Tue, Mar 18, 2014 at 09:09:45AM +, Chris Wilson wrote:
  On Mon, Mar 17, 2014 at 10:48:46PM -0700, Ben Widawsky wrote:
   Move the remaining members over to the new page table structures.
   
   This can be squashed with the previous commit if desire. The reasoning
   is the same as that patch. I simply felt it is easier to review if split.
  
  I'm not liking the shorter names much. Is there precedence elsewhere
  (e.g. daddr)?
  -Chris
  
 
 I'm not particularly attached to daddr. It was fun to say in my head.
 A lot of code does use daddr but it seems to vary between dma,
 device, data, destination Not exactly precedence.
 
 Initially I had the prefix p[td]_daddr, but I thought you might complain
 about it because it's implicit. dma_addr seemed kinda redundant to me.
 
 Recommendation?

I am still attached to dma_addr, as it seems to be common (or
dma_address) for dma-mapped addresses. But by the end, I had stopped
caring, so

Acked-by: Chris Wilson ch...@chris-wilson.co.uk

I'll come back to this if nobody has a better idea and see if I can make
some sensisble suggestions, or just give in completely.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 14/26] drm/i915: Complete page table structures

2014-03-18 Thread Chris Wilson
On Mon, Mar 17, 2014 at 10:48:46PM -0700, Ben Widawsky wrote:
 Move the remaining members over to the new page table structures.
 
 This can be squashed with the previous commit if desire. The reasoning
 is the same as that patch. I simply felt it is easier to review if split.

I'm not liking the shorter names much. Is there precedence elsewhere
(e.g. daddr)?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 14/26] drm/i915: Complete page table structures

2014-03-17 Thread Ben Widawsky
Move the remaining members over to the new page table structures.

This can be squashed with the previous commit if desire. The reasoning
is the same as that patch. I simply felt it is easier to review if split.

Signed-off-by: Ben Widawsky b...@bwidawsk.net

Conflicts:
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem_gtt.c
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c   | 85 +--
 drivers/gpu/drm/i915/i915_gem_gtt.h   | 15 +++
 drivers/gpu/drm/i915/i915_gpu_error.c |  1 -
 4 files changed, 38 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index b226788..5f3666a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1788,7 +1788,7 @@ static void gen8_ppgtt_info(struct seq_file *m, struct 
drm_device *dev, int verb
 static void print_ppgtt(struct seq_file *m, struct i915_hw_ppgtt *ppgtt, const 
char *name)
 {
seq_printf(m, %s:\n, name);
-   seq_printf(m, pd gtt offset: 0x%08x\n, ppgtt-pd_offset);
+   seq_printf(m, pd gtt offset: 0x%08x\n, ppgtt-pd.pd_offset);
 }
 
 static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev, bool 
verbose)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5b283f2..d91a545 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -223,7 +223,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
int used_pd = ppgtt-num_pd_entries / I915_PDES_PER_PD;
 
for (i = used_pd - 1; i = 0; i--) {
-   dma_addr_t addr = ppgtt-pd_dma_addr[i];
+   dma_addr_t addr = ppgtt-pdp.pagedir[i].daddr;
ret = gen8_write_pdp(ring, i, addr, synchronous);
if (ret)
return ret;
@@ -341,7 +341,6 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt)
for (i = 0; i  ppgtt-num_pd_pages; i++) {
gen8_free_page_tables(ppgtt-pdp.pagedir[i]);
gen8_free_page_directories(ppgtt-pdp.pagedir[i]);
-   kfree(ppgtt-gen8_pt_dma_addr[i]);
}
 }
 
@@ -353,14 +352,14 @@ static void gen8_ppgtt_dma_unmap_pages(struct 
i915_hw_ppgtt *ppgtt)
for (i = 0; i  ppgtt-num_pd_pages; i++) {
/* TODO: In the future we'll support sparse mappings, so this
 * will have to change. */
-   if (!ppgtt-pd_dma_addr[i])
+   if (!ppgtt-pdp.pagedir[i].daddr)
continue;
 
-   pci_unmap_page(hwdev, ppgtt-pd_dma_addr[i], PAGE_SIZE,
+   pci_unmap_page(hwdev, ppgtt-pdp.pagedir[i].daddr, PAGE_SIZE,
   PCI_DMA_BIDIRECTIONAL);
 
for (j = 0; j  I915_PDES_PER_PD; j++) {
-   dma_addr_t addr = ppgtt-gen8_pt_dma_addr[i][j];
+   dma_addr_t addr = 
ppgtt-pdp.pagedir[i].page_tables[j].daddr;
if (addr)
pci_unmap_page(hwdev, addr, PAGE_SIZE,
   PCI_DMA_BIDIRECTIONAL);
@@ -380,31 +379,18 @@ static void gen8_ppgtt_cleanup(struct i915_address_space 
*vm)
gen8_ppgtt_free(ppgtt);
 }
 
-static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt)
-{
-   int i;
-
-   for (i = 0; i  ppgtt-num_pd_pages; i++) {
-   ppgtt-gen8_pt_dma_addr[i] = kcalloc(I915_PDES_PER_PD,
-sizeof(dma_addr_t),
-GFP_KERNEL);
-   if (!ppgtt-gen8_pt_dma_addr[i])
-   return -ENOMEM;
-   }
-
-   return 0;
-}
-
 static int gen8_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt)
 {
int i, j;
 
for (i = 0; i  ppgtt-num_pd_pages; i++) {
+   struct i915_pagedir *pd = ppgtt-pdp.pagedir[i];
for (j = 0; j  I915_PDES_PER_PD; j++) {
-   struct i915_pagetab *pt = 
ppgtt-pdp.pagedir[i].page_tables[j];
+   struct i915_pagetab *pt = pd-page_tables[j];
pt-page = alloc_page(GFP_KERNEL | __GFP_ZERO);
if (!pt-page)
goto unwind_out;
+
}
}
 
@@ -464,9 +450,7 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
 
ppgtt-num_pd_entries = max_pdp * I915_PDES_PER_PD;
 
-   ret = gen8_ppgtt_allocate_dma(ppgtt);
-   if (!ret)
-   return ret;
+   return 0;
 
/* TODO: Check this for all cases */
 err_out:
@@ -488,7 +472,7 @@ static int gen8_ppgtt_setup_page_directories(struct 
i915_hw_ppgtt *ppgtt,
if (ret)
return ret;
 
-   ppgtt-pd_dma_addr[pdpe] = pd_addr;
+   ppgtt-pdp.pagedir[pdpe].daddr = pd_addr;
 
return 0;