[RFC,09/38] drm/i915: s/pd/pdpe, s/pt/pde

Submitted by Michel Thierry on Oct. 7, 2014, 5:11 p.m.

Details

Message ID 1412701894-28905-10-git-send-email-michel.thierry@intel.com
State New, archived
Headers show

Not browsing as part of any series.

Commit Message

Michel Thierry Oct. 7, 2014, 5:11 p.m.
From: Ben Widawsky <benjamin.widawsky@intel.com>

The actual correct way to think about this with the new style of page
table data structures is as the actual entry that is being indexed into
the array. "pd", and "pt" aren't representative of what the operation is
doing.

The clarity here will improve the readability of future patches.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0ee258b..12da57a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -502,40 +502,40 @@  static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
 }
 
 static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
-					     const int pd)
+					     const int pdpe)
 {
 	dma_addr_t pd_addr;
 	int ret;
 
 	pd_addr = pci_map_page(ppgtt->base.dev->pdev,
-			       &ppgtt->pd_pages[pd], 0,
+			       &ppgtt->pd_pages[pdpe], 0,
 			       PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
 
 	ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr);
 	if (ret)
 		return ret;
 
-	ppgtt->pd_dma_addr[pd] = pd_addr;
+	ppgtt->pd_dma_addr[pdpe] = pd_addr;
 
 	return 0;
 }
 
 static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
-					const int pd,
-					const int pt)
+					const int pdpe,
+					const int pde)
 {
 	dma_addr_t pt_addr;
 	struct page *p;
 	int ret;
 
-	p = ppgtt->gen8_pt_pages[pd][pt];
+	p = ppgtt->gen8_pt_pages[pdpe][pde];
 	pt_addr = pci_map_page(ppgtt->base.dev->pdev,
 			       p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
 	ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
 	if (ret)
 		return ret;
 
-	ppgtt->gen8_pt_dma_addr[pd][pt] = pt_addr;
+	ppgtt->gen8_pt_dma_addr[pdpe][pde] = pt_addr;
 
 	return 0;
 }

Comments

On Tue, Oct 07, 2014 at 06:11:05PM +0100, Michel Thierry wrote:
> From: Ben Widawsky <benjamin.widawsky@intel.com>
>
> The actual correct way to think about this with the new style of page
> table data structures is as the actual entry that is being indexed into
> the array. "pd", and "pt" aren't representative of what the operation is
> doing.
>
> The clarity here will improve the readability of future patches.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>

Given that I don't know what pdpe means, I disagree that this improves
clarity really. And generally in the core vm the p*e are pointers to the
actual entry itself, not the index like here.

So if we want this it needs to at least better sell the advantage.
-Daniel


> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0ee258b..12da57a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -502,40 +502,40 @@ static int gen8_ppgtt_alloc(struct i915_hw_ppgtt *ppgtt,
>  }
>
>  static int gen8_ppgtt_setup_page_directories(struct i915_hw_ppgtt *ppgtt,
> -     const int pd)
> +     const int pdpe)
>  {
>   dma_addr_t pd_addr;
>   int ret;
>
>   pd_addr = pci_map_page(ppgtt->base.dev->pdev,
> -       &ppgtt->pd_pages[pd], 0,
> +       &ppgtt->pd_pages[pdpe], 0,
>         PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
>
>   ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pd_addr);
>   if (ret)
>   return ret;
>
> - ppgtt->pd_dma_addr[pd] = pd_addr;
> + ppgtt->pd_dma_addr[pdpe] = pd_addr;
>
>   return 0;
>  }
>
>  static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt,
> - const int pd,
> - const int pt)
> + const int pdpe,
> + const int pde)
>  {
>   dma_addr_t pt_addr;
>   struct page *p;
>   int ret;
>
> - p = ppgtt->gen8_pt_pages[pd][pt];
> + p = ppgtt->gen8_pt_pages[pdpe][pde];
>   pt_addr = pci_map_page(ppgtt->base.dev->pdev,
>         p, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
>   ret = pci_dma_mapping_error(ppgtt->base.dev->pdev, pt_addr);
>   if (ret)
>   return ret;
>
> - ppgtt->gen8_pt_dma_addr[pd][pt] = pt_addr;
> + ppgtt->gen8_pt_dma_addr[pdpe][pde] = pt_addr;
>
>   return 0;
>  }
> --
> 2.0.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx