[PATCH] drm/i915: Do not re-allocate vmas in aliasing ppgtt

[PATCH] drm/i915: Do not re-allocate vmas in aliasing ppgtt

[Michel Thierry]

Aliasing ppgtt is fully allocated right after creation, thus shouldn't
need to call allocate_va_range in i915_vma_bind.

This duplication started after commit 5c5f645773b6d147bf68c350674dc3ef4f8de83d
("drm/i915: drm/i915: Unify aliasing ppgtt handling"), as aliasing ppgtt
now also uses allocate_va_range.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7b13273..e8c0ab0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3242,7 +3242,7 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 {
 	int ret;
 
-	if (vma->vm->allocate_va_range) {
+	if (vma->vm->allocate_va_range && USES_FULL_PPGTT(dev)) {
 		trace_i915_va_alloc(vma->vm, vma->node.start,
 				    vma->node.size,
 				    VM_TO_TRACE_NAME(vma->vm));
-- 
2.1.1

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

Re: [PATCH] drm/i915: Do not re-allocate vmas in aliasing ppgtt

[Chris Wilson]

On Wed, Apr 22, 2015 at 05:13:38PM +0100, Michel Thierry wrote:
> Aliasing ppgtt is fully allocated right after creation, thus shouldn't
> need to call allocate_va_range in i915_vma_bind.

And?
 
> This duplication started after commit 5c5f645773b6d147bf68c350674dc3ef4f8de83d
> ("drm/i915: drm/i915: Unify aliasing ppgtt handling"), as aliasing ppgtt
> now also uses allocate_va_range.

The choice would be either to allow calling the no-op function, or to
not install a vfunc.

I would move the tracepoint to the actual allocation side, presumably a
real allocate_va_range() will do some filtering and we only really want
to know when the available space is updated.
-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: [PATCH] drm/i915: Do not re-allocate vmas in aliasing ppgtt

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6248
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  301/301              301/301
SNB                                  318/318              318/318
IVB                                  345/345              345/345
BYT                 -1              285/285              284/285
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@kms_setmode@invalid-clone-exclusive-crtc      PASS(5)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
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

Re: [PATCH] drm/i915: Do not re-allocate vmas in aliasing ppgtt

[Mika Kuoppala]

Michel Thierry <michel.thierry@intel.com> writes:

> Aliasing ppgtt is fully allocated right after creation, thus shouldn't
> need to call allocate_va_range in i915_vma_bind.
>
> This duplication started after commit 5c5f645773b6d147bf68c350674dc3ef4f8de83d
> ("drm/i915: drm/i915: Unify aliasing ppgtt handling"), as aliasing ppgtt
> now also uses allocate_va_range.
>

I understood that Daniel's intention was to unify the initialization
and the handling of ppgtt vm areas. The only special case for
aliasing ppgtt would be that the whole vm area would be preallocated
after generic ppgtt_init (what the Unify patch does).

Even with full ppgtt, we get calls to allocate_va_range where
there is already structure in place (as we dont teardown vm space).

I would prefer to keep the aliasing like that. There might
be small performance cost if we omit checking for aliasing
early. But we gain common code path and aliasing looks like
less special and only contained in init (in this case).

Thanks,
-Mika

> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7b13273..e8c0ab0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3242,7 +3242,7 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
>  {
>  	int ret;
>  
> -	if (vma->vm->allocate_va_range) {
> +	if (vma->vm->allocate_va_range && USES_FULL_PPGTT(dev)) {
>  		trace_i915_va_alloc(vma->vm, vma->node.start,
>  				    vma->node.size,
>  				    VM_TO_TRACE_NAME(vma->vm));
> -- 
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx