[PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Arun Siluvery]

The number of DWords should be even when doing ring emits as
command sequences require QWord alignment.

Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 12a546f..79211ae 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -680,7 +680,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
 	if (ret)
 		return ret;
 
-	ret = intel_ring_begin(ring, w->count * 3);
+	ret = intel_ring_begin(ring, w->count * 4);
 	if (ret)
 		return ret;
 
@@ -688,6 +688,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
 		intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
 		intel_ring_emit(ring, w->reg[i].addr);
 		intel_ring_emit(ring, w->reg[i].value);
+		intel_ring_emit(ring, MI_NOOP);
 	}
 
 	intel_ring_advance(ring);
-- 
2.1.2

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Damien Lespiau]

On Wed, Oct 22, 2014 at 10:09:49AM +0100, Arun Siluvery wrote:
> The number of DWords should be even when doing ring emits as
> command sequences require QWord alignment.

It looks like we could just pad at the end of the batch instead of one
NOP per register write?

Also, It looks like we could use the LRI variant that can write more
than one register in one go (separate patch)?.

-- 
Damien

> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 12a546f..79211ae 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -680,7 +680,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
>  	if (ret)
>  		return ret;
>  
> -	ret = intel_ring_begin(ring, w->count * 3);
> +	ret = intel_ring_begin(ring, w->count * 4);
>  	if (ret)
>  		return ret;
>  
> @@ -688,6 +688,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
>  		intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
>  		intel_ring_emit(ring, w->reg[i].addr);
>  		intel_ring_emit(ring, w->reg[i].value);
> +		intel_ring_emit(ring, MI_NOOP);
>  	}
>  
>  	intel_ring_advance(ring);
> -- 
> 2.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Damien Lespiau]

On Wed, Oct 22, 2014 at 11:40:30AM +0100, Damien Lespiau wrote:
> On Wed, Oct 22, 2014 at 10:09:49AM +0100, Arun Siluvery wrote:
> > The number of DWords should be even when doing ring emits as
> > command sequences require QWord alignment.
> 
> It looks like we could just pad at the end of the batch instead of one
> NOP per register write?
> 
> Also, It looks like we could use the LRI variant that can write more
> than one register in one go (separate patch)?.

Note that if you use the multi-register LRI variant, the number of
dwords will always be odd, so we'll always need to pad a NOP, that makes
it easy.

-- 
Damien

[PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Arun Siluvery]

The number of DWords should be even when doing ring emits as
command sequences require QWord alignment.

v2: user LRI variant that can write multiple regs in one go (Damien).
We can simply insert one NOP at the end instead of one per register write.

Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 497b836..a8f72e8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
 	if (ret)
 		return ret;
 
-	ret = intel_ring_begin(ring, w->count * 3);
+	ret = intel_ring_begin(ring, (w->count * 2 + 2));
 	if (ret)
 		return ret;
 
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
 	for (i = 0; i < w->count; i++) {
-		intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
 		intel_ring_emit(ring, w->reg[i].addr);
 		intel_ring_emit(ring, w->reg[i].value);
 	}
+	intel_ring_emit(ring, MI_NOOP);
 
 	intel_ring_advance(ring);
 
-- 
2.1.2

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Damien Lespiau]

On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
> The number of DWords should be even when doing ring emits as
> command sequences require QWord alignment.
> 
> v2: user LRI variant that can write multiple regs in one go (Damien).
> We can simply insert one NOP at the end instead of one per register write.
> 
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>

Looks good to me (maybe without the extra '()' outlined below).

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 497b836..a8f72e8 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
>  	if (ret)
>  		return ret;
>  
> -	ret = intel_ring_begin(ring, w->count * 3);
> +	ret = intel_ring_begin(ring, (w->count * 2 + 2));

No need of the () around the second argument.

>  	if (ret)
>  		return ret;
>  
> +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
>  	for (i = 0; i < w->count; i++) {
> -		intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
>  		intel_ring_emit(ring, w->reg[i].addr);
>  		intel_ring_emit(ring, w->reg[i].value);
>  	}
> +	intel_ring_emit(ring, MI_NOOP);
>  
>  	intel_ring_advance(ring);
>  
> -- 
> 2.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Daniel Vetter]

On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
> The number of DWords should be even when doing ring emits as
> command sequences require QWord alignment.
> 
> v2: user LRI variant that can write multiple regs in one go (Damien).
> We can simply insert one NOP at the end instead of one per register write.
> 
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 497b836..a8f72e8 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
>  	if (ret)
>  		return ret;
>  
> -	ret = intel_ring_begin(ring, w->count * 3);
> +	ret = intel_ring_begin(ring, (w->count * 2 + 2));
>  	if (ret)
>  		return ret;
>  
> +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));

Afaik there's a limit to the size of an MI_LRI. Where's the check for
that (probably with a WARN_ON for now to avoid unecessary complexity)?
-Daniel

>  	for (i = 0; i < w->count; i++) {
> -		intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
>  		intel_ring_emit(ring, w->reg[i].addr);
>  		intel_ring_emit(ring, w->reg[i].value);
>  	}
> +	intel_ring_emit(ring, MI_NOOP);
>  
>  	intel_ring_advance(ring);
>  
> -- 
> 2.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Damien Lespiau]

On Thu, Oct 23, 2014 at 02:21:02PM +0200, Daniel Vetter wrote:
> On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
> > The number of DWords should be even when doing ring emits as
> > command sequences require QWord alignment.
> > 
> > v2: user LRI variant that can write multiple regs in one go (Damien).
> > We can simply insert one NOP at the end instead of one per register write.
> > 
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 497b836..a8f72e8 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = intel_ring_begin(ring, w->count * 3);
> > +	ret = intel_ring_begin(ring, (w->count * 2 + 2));
> >  	if (ret)
> >  		return ret;
> >  
> > +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
> 
> Afaik there's a limit to the size of an MI_LRI. Where's the check for
> that (probably with a WARN_ON for now to avoid unecessary complexity)?

I guess there's always the size of the length field, I don't see any
other indication. Note that I can find the documentation of the
multi-registers version of LRI either. So, well, we probably should
double check it does work.

-- 
Damien

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Chris Wilson]

On Thu, Oct 23, 2014 at 01:42:38PM +0100, Damien Lespiau wrote:
> On Thu, Oct 23, 2014 at 02:21:02PM +0200, Daniel Vetter wrote:
> > On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
> > > The number of DWords should be even when doing ring emits as
> > > command sequences require QWord alignment.
> > > 
> > > v2: user LRI variant that can write multiple regs in one go (Damien).
> > > We can simply insert one NOP at the end instead of one per register write.
> > > 
> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > index 497b836..a8f72e8 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > -	ret = intel_ring_begin(ring, w->count * 3);
> > > +	ret = intel_ring_begin(ring, (w->count * 2 + 2));
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
> > 
> > Afaik there's a limit to the size of an MI_LRI. Where's the check for
> > that (probably with a WARN_ON for now to avoid unecessary complexity)?
> 
> I guess there's always the size of the length field, I don't see any
> other indication. Note that I can find the documentation of the
> multi-registers version of LRI either. So, well, we probably should
> double check it does work.

It does work. The max is around 60 iirc (the max length of the
command).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Ville Syrjälä]

On Thu, Oct 23, 2014 at 01:50:23PM +0100, Chris Wilson wrote:
> On Thu, Oct 23, 2014 at 01:42:38PM +0100, Damien Lespiau wrote:
> > On Thu, Oct 23, 2014 at 02:21:02PM +0200, Daniel Vetter wrote:
> > > On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
> > > > The number of DWords should be even when doing ring emits as
> > > > command sequences require QWord alignment.
> > > > 
> > > > v2: user LRI variant that can write multiple regs in one go (Damien).
> > > > We can simply insert one NOP at the end instead of one per register write.
> > > > 
> > > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > index 497b836..a8f72e8 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
> > > >  	if (ret)
> > > >  		return ret;
> > > >  
> > > > -	ret = intel_ring_begin(ring, w->count * 3);
> > > > +	ret = intel_ring_begin(ring, (w->count * 2 + 2));
> > > >  	if (ret)
> > > >  		return ret;
> > > >  
> > > > +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
> > > 
> > > Afaik there's a limit to the size of an MI_LRI. Where's the check for
> > > that (probably with a WARN_ON for now to avoid unecessary complexity)?
> > 
> > I guess there's always the size of the length field, I don't see any
> > other indication. Note that I can find the documentation of the
> > multi-registers version of LRI either. So, well, we probably should
> > double check it does work.
> 
> It does work. The max is around 60 iirc (the max length of the
> command).

The maximum length seems to be 0xff on gen6+ and 0x3f before that,
which would mean at most 128 or 32 registers.

Also the context image is full of these multi register LRIs. Based on a
quick glance the longest LRI in there is 0x5f on IVB, 0xcf on HSW, and
0xdf on BDW, which translate to 48, 104, and 108 registers per LRI. So
we know at least those must work or context restore would not work.
Before gen7 the context doesn't seem to resemble a batch, so I can't
tell anything about those platforms based on the context image.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Siluvery, Arun]

On 23/10/2014 14:41, Ville Syrjälä wrote:
> On Thu, Oct 23, 2014 at 01:50:23PM +0100, Chris Wilson wrote:
>> On Thu, Oct 23, 2014 at 01:42:38PM +0100, Damien Lespiau wrote:
>>> On Thu, Oct 23, 2014 at 02:21:02PM +0200, Daniel Vetter wrote:
>>>> On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
>>>>> The number of DWords should be even when doing ring emits as
>>>>> command sequences require QWord alignment.
>>>>>
>>>>> v2: user LRI variant that can write multiple regs in one go (Damien).
>>>>> We can simply insert one NOP at the end instead of one per register write.
>>>>>
>>>>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>>>>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
>>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>>>> index 497b836..a8f72e8 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>>>> @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
>>>>>   	if (ret)
>>>>>   		return ret;
>>>>>
>>>>> -	ret = intel_ring_begin(ring, w->count * 3);
>>>>> +	ret = intel_ring_begin(ring, (w->count * 2 + 2));
>>>>>   	if (ret)
>>>>>   		return ret;
>>>>>
>>>>> +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
>>>>
>>>> Afaik there's a limit to the size of an MI_LRI. Where's the check for
>>>> that (probably with a WARN_ON for now to avoid unecessary complexity)?
>>>
>>> I guess there's always the size of the length field, I don't see any
>>> other indication. Note that I can find the documentation of the
>>> multi-registers version of LRI either. So, well, we probably should
>>> double check it does work.
>>
>> It does work. The max is around 60 iirc (the max length of the
>> command).
>
> The maximum length seems to be 0xff on gen6+ and 0x3f before that,
> which would mean at most 128 or 32 registers.
>
> Also the context image is full of these multi register LRIs. Based on a
> quick glance the longest LRI in there is 0x5f on IVB, 0xcf on HSW, and
> 0xdf on BDW, which translate to 48, 104, and 108 registers per LRI. So
> we know at least those must work or context restore would not work.
> Before gen7 the context doesn't seem to resemble a batch, so I can't
> tell anything about those platforms based on the context image.
>

w->count is already checked against max workarounds which is 16 now so 
we are well within the limit; I think additional check would be 
redundant here and it is unlikely to have more than 128 workarounds.

regards
Arun

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Thu, Oct 23, 2014 at 01:42:38PM +0100, Damien Lespiau wrote:
>> On Thu, Oct 23, 2014 at 02:21:02PM +0200, Daniel Vetter wrote:
>> > On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
>> > > The number of DWords should be even when doing ring emits as
>> > > command sequences require QWord alignment.
>> > > 
>> > > v2: user LRI variant that can write multiple regs in one go (Damien).
>> > > We can simply insert one NOP at the end instead of one per register write.
>> > > 
>> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
>> > >  1 file changed, 3 insertions(+), 2 deletions(-)
>> > > 
>> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> > > index 497b836..a8f72e8 100644
>> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> > > @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
>> > >  	if (ret)
>> > >  		return ret;
>> > >  
>> > > -	ret = intel_ring_begin(ring, w->count * 3);
>> > > +	ret = intel_ring_begin(ring, (w->count * 2 + 2));
>> > >  	if (ret)
>> > >  		return ret;
>> > >  
>> > > +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
>> > 
>> > Afaik there's a limit to the size of an MI_LRI. Where's the check for
>> > that (probably with a WARN_ON for now to avoid unecessary complexity)?
>> 
>> I guess there's always the size of the length field, I don't see any
>> other indication. Note that I can find the documentation of the
>> multi-registers version of LRI either. So, well, we probably should
>> double check it does work.
>
> It does work. The max is around 60 iirc (the max length of the
> command).
> -Chris
>

I did some test with bdw:

The maximum is 128 writes, resulting the 8 bit length 
field of the command being 0xff, thus following the spec.
The 128'th write went through.

Perhaps the max command length is then less in older gens?

Perhaps WARN_ON(x > 128) in MI_LOAD_REGISTER_IMM would be in place
but one needs minor tweak to command parser a bit also then.

#define I915_MAX_WA_REGS 16

keeps us safe for now atleast.

-Mika

> -- 
> Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Emit even number of dwords when emitting LRIs

[Daniel Vetter]

On Thu, Oct 23, 2014 at 05:42:47PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > On Thu, Oct 23, 2014 at 01:42:38PM +0100, Damien Lespiau wrote:
> >> On Thu, Oct 23, 2014 at 02:21:02PM +0200, Daniel Vetter wrote:
> >> > On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
> >> > > The number of DWords should be even when doing ring emits as
> >> > > command sequences require QWord alignment.
> >> > > 
> >> > > v2: user LRI variant that can write multiple regs in one go (Damien).
> >> > > We can simply insert one NOP at the end instead of one per register write.
> >> > > 
> >> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >> > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> >> > > ---
> >> > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
> >> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> >> > > 
> >> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> > > index 497b836..a8f72e8 100644
> >> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> > > @@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
> >> > >  	if (ret)
> >> > >  		return ret;
> >> > >  
> >> > > -	ret = intel_ring_begin(ring, w->count * 3);
> >> > > +	ret = intel_ring_begin(ring, (w->count * 2 + 2));
> >> > >  	if (ret)
> >> > >  		return ret;
> >> > >  
> >> > > +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w->count));
> >> > 
> >> > Afaik there's a limit to the size of an MI_LRI. Where's the check for
> >> > that (probably with a WARN_ON for now to avoid unecessary complexity)?
> >> 
> >> I guess there's always the size of the length field, I don't see any
> >> other indication. Note that I can find the documentation of the
> >> multi-registers version of LRI either. So, well, we probably should
> >> double check it does work.
> >
> > It does work. The max is around 60 iirc (the max length of the
> > command).
> > -Chris
> >
> 
> I did some test with bdw:
> 
> The maximum is 128 writes, resulting the 8 bit length 
> field of the command being 0xff, thus following the spec.
> The 128'th write went through.
> 
> Perhaps the max command length is then less in older gens?
> 
> Perhaps WARN_ON(x > 128) in MI_LOAD_REGISTER_IMM would be in place
> but one needs minor tweak to command parser a bit also then.
> 
> #define I915_MAX_WA_REGS 16
> 
> keeps us safe for now atleast.

Ok, that's good enough I think. I've summarized the discussion a bit in
the commit message and merged the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch