[PATCH] drm/i915: Add missed MI_BATCH_BUFFER_END in null state batch buffer.

[PATCH] drm/i915: Add missed MI_BATCH_BUFFER_END in null state batch buffer.

[Zhi Wang]

Currently MI_BATCH_BUFFER_END is missed in null state batch buffer.
This fix is trying to append the missed instruction at the end of
null state batch buffer gem bo after it was initialized and filled
with null state commands.

This issue was exposed under full GPU virtualization(Intel GVT-g) environment.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_render_state.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 98dcd94..3495b4b 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -54,7 +54,8 @@ static int render_state_init(struct render_state *so, struct drm_device *dev)
 	if (so->rodata == NULL)
 		return 0;
 
-	if (so->rodata->batch_items * 4 > 4096)
+	/* Leave one dword for MI_BATCH_BUFFER_END. */
+	if ((so->rodata->batch_items * 4 + 1) > 4096)
 		return -EINVAL;
 
 	so->obj = i915_gem_alloc_object(dev, 4096);
@@ -108,6 +109,7 @@ static int render_state_setup(struct render_state *so)
 
 		d[i++] = s;
 	}
+	d[i] = MI_BATCH_BUFFER_END;
 	kunmap(page);
 
 	ret = i915_gem_object_set_to_gtt_domain(so->obj, false);
-- 
1.8.3.2

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

Re: [PATCH] drm/i915: Add missed MI_BATCH_BUFFER_END in null state batch buffer.

[Mika Kuoppala]

Zhi Wang <zhi.a.wang@intel.com> writes:

> Currently MI_BATCH_BUFFER_END is missed in null state batch buffer.
> This fix is trying to append the missed instruction at the end of
> null state batch buffer gem bo after it was initialized and filled
> with null state commands.
>
> This issue was exposed under full GPU virtualization(Intel GVT-g) environment.
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_render_state.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> index 98dcd94..3495b4b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -54,7 +54,8 @@ static int render_state_init(struct render_state *so, struct drm_device *dev)
>  	if (so->rodata == NULL)
>  		return 0;
>  
> -	if (so->rodata->batch_items * 4 > 4096)
> +	/* Leave one dword for MI_BATCH_BUFFER_END. */
> +	if ((so->rodata->batch_items * 4 + 1) > 4096)
>  		return -EINVAL;
>  
>  	so->obj = i915_gem_alloc_object(dev, 4096);
> @@ -108,6 +109,7 @@ static int render_state_setup(struct render_state *so)
>  
>  		d[i++] = s;
>  	}
> +	d[i] = MI_BATCH_BUFFER_END;
>  	kunmap(page);
>

The states themselves have the end explicitly. In i-g-t:

intel-gpu-tools/tools/null_state_gen (master)$ grep BUFFER_END *.c
intel_renderstate_gen6.c:       OUT_BATCH(MI_BATCH_BUFFER_END);
intel_renderstate_gen7.c:       OUT_BATCH(MI_BATCH_BUFFER_END);
intel_renderstate_gen8.c:       OUT_BATCH(MI_BATCH_BUFFER_END);
intel_renderstate_gen9.c:       OUT_BATCH(MI_BATCH_BUFFER_END);

Please check that your state generator emits it correctly.
-Mika

>  	ret = i915_gem_object_set_to_gtt_domain(so->obj, false);
> -- 
> 1.8.3.2
>
> _______________________________________________
> 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

Re: [PATCH] drm/i915: Add missed MI_BATCH_BUFFER_END in null state batch buffer.

[Wang, Zhi A]

Thanks Mika. Just found that command in render state. It mixed the commands with indirect state together.
And my problem is caused by my old drm-intel-nightly branch. Sorry for being annoyed.   

-----Original Message-----
From: Mika Kuoppala [mailto:mika.kuoppala@linux.intel.com] 
Sent: Wednesday, October 29, 2014 8:11 PM
To: Wang, Zhi A; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Add missed MI_BATCH_BUFFER_END in null state batch buffer.

Zhi Wang <zhi.a.wang@intel.com> writes:

> Currently MI_BATCH_BUFFER_END is missed in null state batch buffer.
> This fix is trying to append the missed instruction at the end of null 
> state batch buffer gem bo after it was initialized and filled with 
> null state commands.
>
> This issue was exposed under full GPU virtualization(Intel GVT-g) environment.
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_render_state.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c 
> b/drivers/gpu/drm/i915/i915_gem_render_state.c
> index 98dcd94..3495b4b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -54,7 +54,8 @@ static int render_state_init(struct render_state *so, struct drm_device *dev)
>  	if (so->rodata == NULL)
>  		return 0;
>  
> -	if (so->rodata->batch_items * 4 > 4096)
> +	/* Leave one dword for MI_BATCH_BUFFER_END. */
> +	if ((so->rodata->batch_items * 4 + 1) > 4096)
>  		return -EINVAL;
>  
>  	so->obj = i915_gem_alloc_object(dev, 4096); @@ -108,6 +109,7 @@ 
> static int render_state_setup(struct render_state *so)
>  
>  		d[i++] = s;
>  	}
> +	d[i] = MI_BATCH_BUFFER_END;
>  	kunmap(page);
>

The states themselves have the end explicitly. In i-g-t:

intel-gpu-tools/tools/null_state_gen (master)$ grep BUFFER_END *.c
intel_renderstate_gen6.c:       OUT_BATCH(MI_BATCH_BUFFER_END);
intel_renderstate_gen7.c:       OUT_BATCH(MI_BATCH_BUFFER_END);
intel_renderstate_gen8.c:       OUT_BATCH(MI_BATCH_BUFFER_END);
intel_renderstate_gen9.c:       OUT_BATCH(MI_BATCH_BUFFER_END);

Please check that your state generator emits it correctly.
-Mika

>  	ret = i915_gem_object_set_to_gtt_domain(so->obj, false);
> --
> 1.8.3.2
>
> _______________________________________________
> 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

Re: [PATCH] drm/i915: Add missed MI_BATCH_BUFFER_END in null state batch buffer.

[Chris Wilson]

On Wed, Oct 29, 2014 at 02:10:45PM +0200, Mika Kuoppala wrote:
> Zhi Wang <zhi.a.wang@intel.com> writes:
> 
> > Currently MI_BATCH_BUFFER_END is missed in null state batch buffer.
> > This fix is trying to append the missed instruction at the end of
> > null state batch buffer gem bo after it was initialized and filled
> > with null state commands.
> >
> > This issue was exposed under full GPU virtualization(Intel GVT-g) environment.
> >
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_render_state.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> > index 98dcd94..3495b4b 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> > @@ -54,7 +54,8 @@ static int render_state_init(struct render_state *so, struct drm_device *dev)
> >  	if (so->rodata == NULL)
> >  		return 0;
> >  
> > -	if (so->rodata->batch_items * 4 > 4096)
> > +	/* Leave one dword for MI_BATCH_BUFFER_END. */
> > +	if ((so->rodata->batch_items * 4 + 1) > 4096)
> >  		return -EINVAL;
> >  
> >  	so->obj = i915_gem_alloc_object(dev, 4096);
> > @@ -108,6 +109,7 @@ static int render_state_setup(struct render_state *so)
> >  
> >  		d[i++] = s;
> >  	}
> > +	d[i] = MI_BATCH_BUFFER_END;
> >  	kunmap(page);
> >
> 
> The states themselves have the end explicitly. In i-g-t:

The bug is that the MI_BATCH_BUFFER_END doesn't match the batch length
we use.

diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 2b389c2..80bd392 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -33,6 +33,7 @@ struct render_state {
        struct drm_i915_gem_object *obj;
        u64 ggtt_offset;
        int gen;
+       unsigned batch_length;
 };
 
 static const struct intel_renderstate_rodata *
@@ -98,6 +99,9 @@ static int render_state_setup(struct render_state *so)
        while (i < rodata->batch_items) {
                u32 s = rodata->batch[i];
 
+               if (so->batch_length == 0 && s == MI_BATCH_BUFFER_END)
+                       so->batch_length = sizeof(u32) * ALIGN(i, 2);
+
                if (i * 4  == rodata->reloc[reloc_index]) {
                        u64 r = s + so->ggtt_offset;
                        s = lower_32_bits(r);
@@ -155,7 +159,7 @@ int i915_gem_render_state_init(struct i915_gem_request *rq)
 
        ret = i915_request_emit_batchbuffer(rq, NULL,
                                            so.ggtt_offset,
-                                           so.rodata->batch_items * 4,
+                                           so.batch_length,
                                            I915_DISPATCH_SECURE);
        if (ret)
                goto out;

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