* [PATCH 2/4] drm/i915/gtt: Warn if the next layer scratch dma is invalid
2015-06-30 15:16 [PATCH 1/4] drm/i915/gtt: Reorder page alloc/free/init functions Mika Kuoppala
@ 2015-06-30 15:16 ` Mika Kuoppala
2015-06-30 16:59 ` Michel Thierry
2015-06-30 15:16 ` [PATCH 3/4] drm/i915/gtt: Return struct i915_scratch_page from alloc_scratch Mika Kuoppala
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Mika Kuoppala @ 2015-06-30 15:16 UTC (permalink / raw)
To: intel-gfx; +Cc: miku
When dma page is setup, warn if we try to point the entries
to a uninitialized (zero) dma address. Like we do with gen6
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 23f5896..78bfb88 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -427,6 +427,8 @@ static void gen8_initialize_pt(struct i915_address_space *vm,
{
gen8_pte_t scratch_pte;
+ WARN_ON(px_dma(vm->scratch_page) == 0);
+
scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
I915_CACHE_LLC, true);
@@ -488,6 +490,8 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
{
gen8_pde_t scratch_pde;
+ WARN_ON(px_dma(vm->scratch_pt) == 0);
+
scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
fill_px(vm->dev, pd, scratch_pde);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/4] drm/i915/gtt: Warn if the next layer scratch dma is invalid
2015-06-30 15:16 ` [PATCH 2/4] drm/i915/gtt: Warn if the next layer scratch dma is invalid Mika Kuoppala
@ 2015-06-30 16:59 ` Michel Thierry
2015-06-30 17:11 ` Chris Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Michel Thierry @ 2015-06-30 16:59 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: miku
On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> When dma page is setup, warn if we try to point the entries
> to a uninitialized (zero) dma address. Like we do with gen6
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
[Mental note to add the same in the future pdp initialize function].
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 23f5896..78bfb88 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -427,6 +427,8 @@ static void gen8_initialize_pt(struct i915_address_space *vm,
> {
> gen8_pte_t scratch_pte;
>
> + WARN_ON(px_dma(vm->scratch_page) == 0);
> +
> scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
> I915_CACHE_LLC, true);
>
> @@ -488,6 +490,8 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
> {
> gen8_pde_t scratch_pde;
>
> + WARN_ON(px_dma(vm->scratch_pt) == 0);
> +
> scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
>
> fill_px(vm->dev, pd, scratch_pde);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/4] drm/i915/gtt: Warn if the next layer scratch dma is invalid
2015-06-30 16:59 ` Michel Thierry
@ 2015-06-30 17:11 ` Chris Wilson
2015-07-01 10:55 ` Mika Kuoppala
0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2015-06-30 17:11 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, miku
On Tue, Jun 30, 2015 at 05:59:27PM +0100, Michel Thierry wrote:
> On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> >When dma page is setup, warn if we try to point the entries
> >to a uninitialized (zero) dma address. Like we do with gen6
Just a note that 0 looks only to be reserved due to a quirk of the
intel-iommu implementation. There is nothing preventing a different
iommu or future implementation using 0 as a valid dma_addr_t afaict.
This should presumably be
WARN_ON(dma_mapping_error(&dev->pdev->dev, px_dma()));
-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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] drm/i915/gtt: Warn if the next layer scratch dma is invalid
2015-06-30 17:11 ` Chris Wilson
@ 2015-07-01 10:55 ` Mika Kuoppala
0 siblings, 0 replies; 15+ messages in thread
From: Mika Kuoppala @ 2015-07-01 10:55 UTC (permalink / raw)
To: Chris Wilson, Michel Thierry; +Cc: intel-gfx, miku
Chris Wilson <chris@chris-wilson.co.uk> writes:
> On Tue, Jun 30, 2015 at 05:59:27PM +0100, Michel Thierry wrote:
>> On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
>> >When dma page is setup, warn if we try to point the entries
>> >to a uninitialized (zero) dma address. Like we do with gen6
>
> Just a note that 0 looks only to be reserved due to a quirk of the
> intel-iommu implementation. There is nothing preventing a different
> iommu or future implementation using 0 as a valid dma_addr_t afaict.
>
> This should presumably be
> WARN_ON(dma_mapping_error(&dev->pdev->dev, px_dma()));
> -Chris
>
Ah yes. This was my paranoia from the times we updated
execlists pdps with zero dma addresses.
Michel pointed out that we do a proper dma_mapping_error
check when we allocate the dma page. So I think this patch
can be ignored.
-Mika
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] drm/i915/gtt: Return struct i915_scratch_page from alloc_scratch
2015-06-30 15:16 [PATCH 1/4] drm/i915/gtt: Reorder page alloc/free/init functions Mika Kuoppala
2015-06-30 15:16 ` [PATCH 2/4] drm/i915/gtt: Warn if the next layer scratch dma is invalid Mika Kuoppala
@ 2015-06-30 15:16 ` Mika Kuoppala
2015-07-01 12:02 ` Michel Thierry
2015-06-30 15:16 ` [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page Mika Kuoppala
2015-06-30 16:58 ` [PATCH 1/4] drm/i915/gtt: Reorder page alloc/free/init functions Michel Thierry
3 siblings, 1 reply; 15+ messages in thread
From: Mika Kuoppala @ 2015-06-30 15:16 UTC (permalink / raw)
To: intel-gfx; +Cc: miku
Every other alloc_* function return the pointer to the page
they alloc. Follow the convention with scratch page also.
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 78 ++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 78bfb88..402d6d3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -384,6 +384,35 @@ static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
fill_page_dma(dev, p, v);
}
+static struct i915_page_scratch *alloc_scratch_page(struct drm_device *dev)
+{
+ struct i915_page_scratch *sp;
+ int ret;
+
+ sp = kzalloc(sizeof(*sp), GFP_KERNEL);
+ if (sp == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ ret = __setup_page_dma(dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
+ if (ret) {
+ kfree(sp);
+ return ERR_PTR(ret);
+ }
+
+ set_pages_uc(px_page(sp), 1);
+
+ return sp;
+}
+
+static void free_scratch_page(struct drm_device *dev,
+ struct i915_page_scratch *sp)
+{
+ set_pages_wb(px_page(sp), 1);
+
+ cleanup_px(dev, sp);
+ kfree(sp);
+}
+
static struct i915_page_table *alloc_pt(struct drm_device *dev)
{
struct i915_page_table *pt;
@@ -497,42 +526,6 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
fill_px(vm->dev, pd, scratch_pde);
}
-static int alloc_scratch_page(struct i915_address_space *vm)
-{
- struct i915_page_scratch *sp;
- int ret;
-
- WARN_ON(vm->scratch_page);
-
- sp = kzalloc(sizeof(*sp), GFP_KERNEL);
- if (sp == NULL)
- return -ENOMEM;
-
- ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
- if (ret) {
- kfree(sp);
- return ret;
- }
-
- set_pages_uc(px_page(sp), 1);
-
- vm->scratch_page = sp;
-
- return 0;
-}
-
-static void free_scratch_page(struct i915_address_space *vm)
-{
- struct i915_page_scratch *sp = vm->scratch_page;
-
- set_pages_wb(px_page(sp), 1);
-
- cleanup_px(vm->dev, sp);
- kfree(sp);
-
- vm->scratch_page = NULL;
-}
-
/* Broadwell Page Directory Pointer Descriptors */
static int gen8_write_pdp(struct drm_i915_gem_request *req,
unsigned entry,
@@ -2248,8 +2241,8 @@ static int ggtt_probe_common(struct drm_device *dev,
size_t gtt_size)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct i915_page_scratch *scratch_page;
phys_addr_t gtt_phys_addr;
- int ret;
/* For Modern GENs the PTEs and register space are split in the BAR */
gtt_phys_addr = pci_resource_start(dev->pdev, 0) +
@@ -2271,14 +2264,17 @@ static int ggtt_probe_common(struct drm_device *dev,
return -ENOMEM;
}
- ret = alloc_scratch_page(&dev_priv->gtt.base);
- if (ret) {
+ scratch_page = alloc_scratch_page(dev);
+ if (IS_ERR(scratch_page)) {
DRM_ERROR("Scratch setup failed\n");
/* iounmap will also get called at remove, but meh */
iounmap(dev_priv->gtt.gsm);
+ return PTR_ERR(scratch_page);
}
- return ret;
+ dev_priv->gtt.base.scratch_page = scratch_page;
+
+ return 0;
}
/* The GGTT and PPGTT need a private PPAT setup in order to handle cacheability
@@ -2450,7 +2446,7 @@ static void gen6_gmch_remove(struct i915_address_space *vm)
struct i915_gtt *gtt = container_of(vm, struct i915_gtt, base);
iounmap(gtt->gsm);
- free_scratch_page(vm);
+ free_scratch_page(vm->dev, vm->scratch_page);
}
static int i915_gmch_probe(struct drm_device *dev,
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/4] drm/i915/gtt: Return struct i915_scratch_page from alloc_scratch
2015-06-30 15:16 ` [PATCH 3/4] drm/i915/gtt: Return struct i915_scratch_page from alloc_scratch Mika Kuoppala
@ 2015-07-01 12:02 ` Michel Thierry
2015-07-01 13:15 ` Daniel Vetter
0 siblings, 1 reply; 15+ messages in thread
From: Michel Thierry @ 2015-07-01 12:02 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: miku
On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> Every other alloc_* function return the pointer to the page
> they alloc. Follow the convention with scratch page also.
>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 78 ++++++++++++++++++-------------------
> 1 file changed, 37 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 78bfb88..402d6d3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -384,6 +384,35 @@ static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
> fill_page_dma(dev, p, v);
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] drm/i915/gtt: Return struct i915_scratch_page from alloc_scratch
2015-07-01 12:02 ` Michel Thierry
@ 2015-07-01 13:15 ` Daniel Vetter
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2015-07-01 13:15 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, miku
On Wed, Jul 01, 2015 at 01:02:09PM +0100, Michel Thierry wrote:
> On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> >Every other alloc_* function return the pointer to the page
> >they alloc. Follow the convention with scratch page also.
> >
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Merged patch 1 and this, thanks.
-Daniel
>
> >Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> >---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 78 ++++++++++++++++++-------------------
> > 1 file changed, 37 insertions(+), 41 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >index 78bfb88..402d6d3 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >@@ -384,6 +384,35 @@ static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
> > fill_page_dma(dev, p, v);
> > }
> >
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page
2015-06-30 15:16 [PATCH 1/4] drm/i915/gtt: Reorder page alloc/free/init functions Mika Kuoppala
2015-06-30 15:16 ` [PATCH 2/4] drm/i915/gtt: Warn if the next layer scratch dma is invalid Mika Kuoppala
2015-06-30 15:16 ` [PATCH 3/4] drm/i915/gtt: Return struct i915_scratch_page from alloc_scratch Mika Kuoppala
@ 2015-06-30 15:16 ` Mika Kuoppala
2015-07-01 14:05 ` Michel Thierry
2015-07-02 14:34 ` shuang.he
2015-06-30 16:58 ` [PATCH 1/4] drm/i915/gtt: Reorder page alloc/free/init functions Michel Thierry
3 siblings, 2 replies; 15+ messages in thread
From: Mika Kuoppala @ 2015-06-30 15:16 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, miku
Previously we have pointed the page where the individual ppgtt
scratch structures refer to, to be the instance which GGTT setup have
allocated. So it has been shared.
To achive full isolation between ppgtts also in this regard,
allocate per ppgtt scratch page.
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 94 +++++++++++++++++++++++++++++--------
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 402d6d3..b1a8fc4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -682,6 +682,42 @@ static void gen8_free_page_tables(struct drm_device *dev,
}
}
+static int gen8_init_scratch(struct i915_address_space *vm)
+{
+ struct drm_device *dev = vm->dev;
+
+ vm->scratch_page = alloc_scratch_page(dev);
+ if (IS_ERR(vm->scratch_page))
+ return PTR_ERR(vm->scratch_page);
+
+ vm->scratch_pt = alloc_pt(dev);
+ if (IS_ERR(vm->scratch_pt)) {
+ free_scratch_page(dev, vm->scratch_page);
+ return PTR_ERR(vm->scratch_pt);
+ }
+
+ vm->scratch_pd = alloc_pd(dev);
+ if (IS_ERR(vm->scratch_pd)) {
+ free_pt(dev, vm->scratch_pt);
+ free_scratch_page(dev, vm->scratch_page);
+ return PTR_ERR(vm->scratch_pd);
+ }
+
+ gen8_initialize_pt(vm, vm->scratch_pt);
+ gen8_initialize_pd(vm, vm->scratch_pd);
+
+ return 0;
+}
+
+static void gen8_free_scratch(struct i915_address_space *vm)
+{
+ struct drm_device *dev = vm->dev;
+
+ free_pd(dev, vm->scratch_pd);
+ free_pt(dev, vm->scratch_pt);
+ free_scratch_page(dev, vm->scratch_page);
+}
+
static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
{
struct i915_hw_ppgtt *ppgtt =
@@ -697,8 +733,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
}
- free_pd(vm->dev, vm->scratch_pd);
- free_pt(vm->dev, vm->scratch_pt);
+ gen8_free_scratch(vm);
}
/**
@@ -985,16 +1020,11 @@ err_out:
*/
static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
{
- ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
- if (IS_ERR(ppgtt->base.scratch_pt))
- return PTR_ERR(ppgtt->base.scratch_pt);
-
- ppgtt->base.scratch_pd = alloc_pd(ppgtt->base.dev);
- if (IS_ERR(ppgtt->base.scratch_pd))
- return PTR_ERR(ppgtt->base.scratch_pd);
+ int ret;
- gen8_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
- gen8_initialize_pd(&ppgtt->base, ppgtt->base.scratch_pd);
+ ret = gen8_init_scratch(&ppgtt->base);
+ if (ret)
+ return ret;
ppgtt->base.start = 0;
ppgtt->base.total = 1ULL << 32;
@@ -1410,6 +1440,33 @@ unwind_out:
return ret;
}
+static int gen6_init_scratch(struct i915_address_space *vm)
+{
+ struct drm_device *dev = vm->dev;
+
+ vm->scratch_page = alloc_scratch_page(dev);
+ if (IS_ERR(vm->scratch_page))
+ return PTR_ERR(vm->scratch_page);
+
+ vm->scratch_pt = alloc_pt(dev);
+ if (IS_ERR(vm->scratch_pt)) {
+ free_scratch_page(dev, vm->scratch_page);
+ return PTR_ERR(vm->scratch_pt);
+ }
+
+ gen6_initialize_pt(vm, vm->scratch_pt);
+
+ return 0;
+}
+
+static void gen6_free_scratch(struct i915_address_space *vm)
+{
+ struct drm_device *dev = vm->dev;
+
+ free_pt(dev, vm->scratch_pt);
+ free_scratch_page(dev, vm->scratch_page);
+}
+
static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
{
struct i915_hw_ppgtt *ppgtt =
@@ -1424,11 +1481,12 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
free_pt(ppgtt->base.dev, pt);
}
- free_pt(vm->dev, vm->scratch_pt);
+ gen6_free_scratch(vm);
}
static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
{
+ struct i915_address_space *vm = &ppgtt->base;
struct drm_device *dev = ppgtt->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
bool retried = false;
@@ -1439,11 +1497,10 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
* size. We allocate at the top of the GTT to avoid fragmentation.
*/
BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
- ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
- if (IS_ERR(ppgtt->base.scratch_pt))
- return PTR_ERR(ppgtt->base.scratch_pt);
- gen6_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
+ ret = gen6_init_scratch(vm);
+ if (ret)
+ return ret;
alloc:
ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
@@ -1474,7 +1531,7 @@ alloc:
return 0;
err_out:
- free_pt(ppgtt->base.dev, ppgtt->base.scratch_pt);
+ gen6_free_scratch(vm);
return ret;
}
@@ -1548,10 +1605,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
ppgtt->base.dev = dev;
- ppgtt->base.scratch_page = dev_priv->gtt.base.scratch_page;
if (INTEL_INFO(dev)->gen < 8)
return gen6_ppgtt_init(ppgtt);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page
2015-06-30 15:16 ` [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page Mika Kuoppala
@ 2015-07-01 14:05 ` Michel Thierry
2015-07-01 14:26 ` Daniel Vetter
2015-07-02 14:34 ` shuang.he
1 sibling, 1 reply; 15+ messages in thread
From: Michel Thierry @ 2015-07-01 14:05 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx, Daniel Vetter; +Cc: miku
On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> Previously we have pointed the page where the individual ppgtt
> scratch structures refer to, to be the instance which GGTT setup have
> allocated. So it has been shared.
>
> To achive full isolation between ppgtts also in this regard,
^^^^^achieve
> allocate per ppgtt scratch page.
>
Maybe also say that it moved scratch page/pt/pd operations together
(genx_init/free_scratch functions).
Daniel, since you requested this, should it get yours r-b?
It looks ok to me.
-Michel
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 94 +++++++++++++++++++++++++++++--------
> 1 file changed, 74 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 402d6d3..b1a8fc4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -682,6 +682,42 @@ static void gen8_free_page_tables(struct drm_device *dev,
> }
> }
>
> +static int gen8_init_scratch(struct i915_address_space *vm)
> +{
> + struct drm_device *dev = vm->dev;
> +
> + vm->scratch_page = alloc_scratch_page(dev);
> + if (IS_ERR(vm->scratch_page))
> + return PTR_ERR(vm->scratch_page);
> +
> + vm->scratch_pt = alloc_pt(dev);
> + if (IS_ERR(vm->scratch_pt)) {
> + free_scratch_page(dev, vm->scratch_page);
> + return PTR_ERR(vm->scratch_pt);
> + }
> +
> + vm->scratch_pd = alloc_pd(dev);
> + if (IS_ERR(vm->scratch_pd)) {
> + free_pt(dev, vm->scratch_pt);
> + free_scratch_page(dev, vm->scratch_page);
> + return PTR_ERR(vm->scratch_pd);
> + }
> +
> + gen8_initialize_pt(vm, vm->scratch_pt);
> + gen8_initialize_pd(vm, vm->scratch_pd);
> +
> + return 0;
> +}
> +
> +static void gen8_free_scratch(struct i915_address_space *vm)
> +{
> + struct drm_device *dev = vm->dev;
> +
> + free_pd(dev, vm->scratch_pd);
> + free_pt(dev, vm->scratch_pt);
> + free_scratch_page(dev, vm->scratch_page);
> +}
> +
> static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> {
> struct i915_hw_ppgtt *ppgtt =
> @@ -697,8 +733,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> free_pd(ppgtt->base.dev, ppgtt->pdp.page_directory[i]);
> }
>
> - free_pd(vm->dev, vm->scratch_pd);
> - free_pt(vm->dev, vm->scratch_pt);
> + gen8_free_scratch(vm);
> }
>
> /**
> @@ -985,16 +1020,11 @@ err_out:
> */
> static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> {
> - ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
> - if (IS_ERR(ppgtt->base.scratch_pt))
> - return PTR_ERR(ppgtt->base.scratch_pt);
> -
> - ppgtt->base.scratch_pd = alloc_pd(ppgtt->base.dev);
> - if (IS_ERR(ppgtt->base.scratch_pd))
> - return PTR_ERR(ppgtt->base.scratch_pd);
> + int ret;
>
> - gen8_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
> - gen8_initialize_pd(&ppgtt->base, ppgtt->base.scratch_pd);
> + ret = gen8_init_scratch(&ppgtt->base);
> + if (ret)
> + return ret;
>
> ppgtt->base.start = 0;
> ppgtt->base.total = 1ULL << 32;
> @@ -1410,6 +1440,33 @@ unwind_out:
> return ret;
> }
>
> +static int gen6_init_scratch(struct i915_address_space *vm)
> +{
> + struct drm_device *dev = vm->dev;
> +
> + vm->scratch_page = alloc_scratch_page(dev);
> + if (IS_ERR(vm->scratch_page))
> + return PTR_ERR(vm->scratch_page);
> +
> + vm->scratch_pt = alloc_pt(dev);
> + if (IS_ERR(vm->scratch_pt)) {
> + free_scratch_page(dev, vm->scratch_page);
> + return PTR_ERR(vm->scratch_pt);
> + }
> +
> + gen6_initialize_pt(vm, vm->scratch_pt);
> +
> + return 0;
> +}
> +
> +static void gen6_free_scratch(struct i915_address_space *vm)
> +{
> + struct drm_device *dev = vm->dev;
> +
> + free_pt(dev, vm->scratch_pt);
> + free_scratch_page(dev, vm->scratch_page);
> +}
> +
> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> {
> struct i915_hw_ppgtt *ppgtt =
> @@ -1424,11 +1481,12 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> free_pt(ppgtt->base.dev, pt);
> }
>
> - free_pt(vm->dev, vm->scratch_pt);
> + gen6_free_scratch(vm);
> }
>
> static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
> {
> + struct i915_address_space *vm = &ppgtt->base;
> struct drm_device *dev = ppgtt->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> bool retried = false;
> @@ -1439,11 +1497,10 @@ static int gen6_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt)
> * size. We allocate at the top of the GTT to avoid fragmentation.
> */
> BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
> - ppgtt->base.scratch_pt = alloc_pt(ppgtt->base.dev);
> - if (IS_ERR(ppgtt->base.scratch_pt))
> - return PTR_ERR(ppgtt->base.scratch_pt);
>
> - gen6_initialize_pt(&ppgtt->base, ppgtt->base.scratch_pt);
> + ret = gen6_init_scratch(vm);
> + if (ret)
> + return ret;
>
> alloc:
> ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
> @@ -1474,7 +1531,7 @@ alloc:
> return 0;
>
> err_out:
> - free_pt(ppgtt->base.dev, ppgtt->base.scratch_pt);
> + gen6_free_scratch(vm);
> return ret;
> }
>
> @@ -1548,10 +1605,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>
> static int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> ppgtt->base.dev = dev;
> - ppgtt->base.scratch_page = dev_priv->gtt.base.scratch_page;
>
> if (INTEL_INFO(dev)->gen < 8)
> return gen6_ppgtt_init(ppgtt);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page
2015-07-01 14:05 ` Michel Thierry
@ 2015-07-01 14:26 ` Daniel Vetter
2015-07-01 14:25 ` Michel Thierry
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2015-07-01 14:26 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, miku, Daniel Vetter
On Wed, Jul 01, 2015 at 03:05:44PM +0100, Michel Thierry wrote:
> On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> >Previously we have pointed the page where the individual ppgtt
> >scratch structures refer to, to be the instance which GGTT setup have
> >allocated. So it has been shared.
> >
> >To achive full isolation between ppgtts also in this regard,
> ^^^^^achieve
>
> >allocate per ppgtt scratch page.
> >
> Maybe also say that it moved scratch page/pt/pd operations together
> (genx_init/free_scratch functions).
>
> Daniel, since you requested this, should it get yours r-b?
> It looks ok to me.
Does that count as an r-b? Doing a detailed review is more work than just
acking the overall idea ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page
2015-07-01 14:26 ` Daniel Vetter
@ 2015-07-01 14:25 ` Michel Thierry
2015-07-01 14:49 ` Daniel Vetter
0 siblings, 1 reply; 15+ messages in thread
From: Michel Thierry @ 2015-07-01 14:25 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, miku, Daniel Vetter
On 7/1/2015 3:26 PM, Daniel Vetter wrote:
> On Wed, Jul 01, 2015 at 03:05:44PM +0100, Michel Thierry wrote:
>> On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
>>> Previously we have pointed the page where the individual ppgtt
>>> scratch structures refer to, to be the instance which GGTT setup have
>>> allocated. So it has been shared.
>>>
>>> To achive full isolation between ppgtts also in this regard,
>> ^^^^^achieve
>>
>>> allocate per ppgtt scratch page.
>>>
>> Maybe also say that it moved scratch page/pt/pd operations together
>> (genx_init/free_scratch functions).
>>
>> Daniel, since you requested this, should it get yours r-b?
>> It looks ok to me.
>
> Does that count as an r-b? Doing a detailed review is more work than just
> acking the overall idea ;-)
Yes, it'd be great if you fix the typo while merging.
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
> -Daniel
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page
2015-07-01 14:25 ` Michel Thierry
@ 2015-07-01 14:49 ` Daniel Vetter
0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2015-07-01 14:49 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, miku, Daniel Vetter
On Wed, Jul 01, 2015 at 03:25:09PM +0100, Michel Thierry wrote:
> On 7/1/2015 3:26 PM, Daniel Vetter wrote:
> >On Wed, Jul 01, 2015 at 03:05:44PM +0100, Michel Thierry wrote:
> >>On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> >>>Previously we have pointed the page where the individual ppgtt
> >>>scratch structures refer to, to be the instance which GGTT setup have
> >>>allocated. So it has been shared.
> >>>
> >>>To achive full isolation between ppgtts also in this regard,
> >> ^^^^^achieve
> >>
> >>>allocate per ppgtt scratch page.
> >>>
> >>Maybe also say that it moved scratch page/pt/pd operations together
> >>(genx_init/free_scratch functions).
> >>
> >>Daniel, since you requested this, should it get yours r-b?
> >>It looks ok to me.
> >
> >Does that count as an r-b? Doing a detailed review is more work than just
> >acking the overall idea ;-)
>
> Yes, it'd be great if you fix the typo while merging.
Done.
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page
2015-06-30 15:16 ` [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page Mika Kuoppala
2015-07-01 14:05 ` Michel Thierry
@ 2015-07-02 14:34 ` shuang.he
1 sibling, 0 replies; 15+ messages in thread
From: shuang.he @ 2015-07-02 14:34 UTC (permalink / raw)
To: shuang.he, lei.a.liu, intel-gfx, mika.kuoppala
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6688
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 302/302 302/302
SNB 312/316 312/316
IVB 343/343 343/343
BYT -3 287/287 284/287
HSW 380/380 380/380
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_partial_pwrite_pread@reads PASS(1) FAIL(1)
*BYT igt@gem_partial_pwrite_pread@reads-display PASS(1) FAIL(1)
*BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] drm/i915/gtt: Reorder page alloc/free/init functions
2015-06-30 15:16 [PATCH 1/4] drm/i915/gtt: Reorder page alloc/free/init functions Mika Kuoppala
` (2 preceding siblings ...)
2015-06-30 15:16 ` [PATCH 4/4] drm/i915/gtt: Per ppgtt scratch page Mika Kuoppala
@ 2015-06-30 16:58 ` Michel Thierry
3 siblings, 0 replies; 15+ messages in thread
From: Michel Thierry @ 2015-06-30 16:58 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: miku
On 6/30/2015 4:16 PM, Mika Kuoppala wrote:
> Maintain base page handling functions in order of
> alloc, free, init. No functional changes.
>
> v2: s/Introduce/Maintain (Michel)
> v3: Rebase
>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com> (v2)
v3 too.
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 166 ++++++++++++++++++------------------
> 1 file changed, 83 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8cfa390..23f5896 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -384,24 +384,6 @@ static void fill_page_dma_32(struct drm_device *dev, struct i915_page_dma *p,
> fill_page_dma(dev, p, v);
> }
>
> -static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
> -{
> - cleanup_px(dev, pt);
> - kfree(pt->used_ptes);
> - kfree(pt);
> -}
> -
> -static void gen8_initialize_pt(struct i915_address_space *vm,
> - struct i915_page_table *pt)
> -{
> - gen8_pte_t scratch_pte;
> -
> - scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
> - I915_CACHE_LLC, true);
> -
> - fill_px(vm->dev, pt, scratch_pte);
> -}
> -
> static struct i915_page_table *alloc_pt(struct drm_device *dev)
> {
> struct i915_page_table *pt;
> @@ -433,13 +415,35 @@ fail_bitmap:
> return ERR_PTR(ret);
> }
>
> -static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
> +static void free_pt(struct drm_device *dev, struct i915_page_table *pt)
> {
> - if (px_page(pd)) {
> - cleanup_px(dev, pd);
> - kfree(pd->used_pdes);
> - kfree(pd);
> - }
> + cleanup_px(dev, pt);
> + kfree(pt->used_ptes);
> + kfree(pt);
> +}
> +
> +static void gen8_initialize_pt(struct i915_address_space *vm,
> + struct i915_page_table *pt)
> +{
> + gen8_pte_t scratch_pte;
> +
> + scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
> + I915_CACHE_LLC, true);
> +
> + fill_px(vm->dev, pt, scratch_pte);
> +}
> +
> +static void gen6_initialize_pt(struct i915_address_space *vm,
> + struct i915_page_table *pt)
> +{
> + gen6_pte_t scratch_pte;
> +
> + WARN_ON(px_dma(vm->scratch_page) == 0);
> +
> + scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
> + I915_CACHE_LLC, true, 0);
> +
> + fill32_px(vm->dev, pt, scratch_pte);
> }
>
> static struct i915_page_directory *alloc_pd(struct drm_device *dev)
> @@ -470,6 +474,61 @@ fail_bitmap:
> return ERR_PTR(ret);
> }
>
> +static void free_pd(struct drm_device *dev, struct i915_page_directory *pd)
> +{
> + if (px_page(pd)) {
> + cleanup_px(dev, pd);
> + kfree(pd->used_pdes);
> + kfree(pd);
> + }
> +}
> +
> +static void gen8_initialize_pd(struct i915_address_space *vm,
> + struct i915_page_directory *pd)
> +{
> + gen8_pde_t scratch_pde;
> +
> + scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
> +
> + fill_px(vm->dev, pd, scratch_pde);
> +}
> +
> +static int alloc_scratch_page(struct i915_address_space *vm)
> +{
> + struct i915_page_scratch *sp;
> + int ret;
> +
> + WARN_ON(vm->scratch_page);
> +
> + sp = kzalloc(sizeof(*sp), GFP_KERNEL);
> + if (sp == NULL)
> + return -ENOMEM;
> +
> + ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
> + if (ret) {
> + kfree(sp);
> + return ret;
> + }
> +
> + set_pages_uc(px_page(sp), 1);
> +
> + vm->scratch_page = sp;
> +
> + return 0;
> +}
> +
> +static void free_scratch_page(struct i915_address_space *vm)
> +{
> + struct i915_page_scratch *sp = vm->scratch_page;
> +
> + set_pages_wb(px_page(sp), 1);
> +
> + cleanup_px(vm->dev, sp);
> + kfree(sp);
> +
> + vm->scratch_page = NULL;
> +}
> +
> /* Broadwell Page Directory Pointer Descriptors */
> static int gen8_write_pdp(struct drm_i915_gem_request *req,
> unsigned entry,
> @@ -609,16 +668,6 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
> kunmap_px(ppgtt, pt_vaddr);
> }
>
> -static void gen8_initialize_pd(struct i915_address_space *vm,
> - struct i915_page_directory *pd)
> -{
> - gen8_pde_t scratch_pde;
> -
> - scratch_pde = gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC);
> -
> - fill_px(vm->dev, pd, scratch_pde);
> -}
> -
> static void gen8_free_page_tables(struct drm_device *dev,
> struct i915_page_directory *pd)
> {
> @@ -1274,19 +1323,6 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
> kunmap_px(ppgtt, pt_vaddr);
> }
>
> -static void gen6_initialize_pt(struct i915_address_space *vm,
> - struct i915_page_table *pt)
> -{
> - gen6_pte_t scratch_pte;
> -
> - WARN_ON(px_dma(vm->scratch_page) == 0);
> -
> - scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
> - I915_CACHE_LLC, true, 0);
> -
> - fill32_px(vm->dev, pt, scratch_pte);
> -}
> -
> static int gen6_alloc_va_range(struct i915_address_space *vm,
> uint64_t start_in, uint64_t length_in)
> {
> @@ -2126,42 +2162,6 @@ void i915_global_gtt_cleanup(struct drm_device *dev)
> vm->cleanup(vm);
> }
>
> -static int alloc_scratch_page(struct i915_address_space *vm)
> -{
> - struct i915_page_scratch *sp;
> - int ret;
> -
> - WARN_ON(vm->scratch_page);
> -
> - sp = kzalloc(sizeof(*sp), GFP_KERNEL);
> - if (sp == NULL)
> - return -ENOMEM;
> -
> - ret = __setup_page_dma(vm->dev, px_base(sp), GFP_DMA32 | __GFP_ZERO);
> - if (ret) {
> - kfree(sp);
> - return ret;
> - }
> -
> - set_pages_uc(px_page(sp), 1);
> -
> - vm->scratch_page = sp;
> -
> - return 0;
> -}
> -
> -static void free_scratch_page(struct i915_address_space *vm)
> -{
> - struct i915_page_scratch *sp = vm->scratch_page;
> -
> - set_pages_wb(px_page(sp), 1);
> -
> - cleanup_px(vm->dev, sp);
> - kfree(sp);
> -
> - vm->scratch_page = NULL;
> -}
> -
> static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
> {
> snb_gmch_ctl >>= SNB_GMCH_GGMS_SHIFT;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread