[1/3] igt/gem_cpu_reloc: Check HW exists before attempting to use it

Submitted by Chris Wilson on May 17, 2018, 8:23 a.m.

Details

Message ID 20180517082330.32760-1-chris@chris-wilson.co.uk
State Accepted
Commit 0c6f14ca970ba969d8834eb50b642a9c35723949
Headers show
Series "Series without cover letter" ( rev: 2 1 ) in IGT (deprecated)

Not browsing as part of any series.

Commit Message

Chris Wilson May 17, 2018, 8:23 a.m.
Confirm we have the available HW before asserting it succeeds.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/gem_cpu_reloc.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/tests/gem_cpu_reloc.c b/tests/gem_cpu_reloc.c
index 882c312d4..e3bbcd239 100644
--- a/tests/gem_cpu_reloc.c
+++ b/tests/gem_cpu_reloc.c
@@ -167,6 +167,7 @@  static void run_test(int fd, int count)
 	use_blt = 0;
 	if (intel_gen(noop) >= 6)
 		use_blt = I915_EXEC_BLT;
+	gem_require_ring(fd, use_blt);
 
 	if (intel_gen(noop) >= 8) {
 		batch = gen8_batch;

Comments

On 17/05/18 01:23, Chris Wilson wrote:
> Confirm we have the available HW before asserting it succeeds.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/gem_cpu_reloc.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tests/gem_cpu_reloc.c b/tests/gem_cpu_reloc.c
> index 882c312d4..e3bbcd239 100644
> --- a/tests/gem_cpu_reloc.c
> +++ b/tests/gem_cpu_reloc.c
> @@ -167,6 +167,7 @@ static void run_test(int fd, int count)
>   	use_blt = 0;

Is this^ meant to be EXEC_DEFAULT?

>   	if (intel_gen(noop) >= 6)
>   		use_blt = I915_EXEC_BLT;
> +	gem_require_ring(fd, use_blt);

Are any gens 6+ that do not have a BLT ring? if that is the case 
shouldn't we use '0' like we do for 5- gens?

Thanks,
Antonio

>   
>   	if (intel_gen(noop) >= 8) {
>   		batch = gen8_batch;
>
Quoting Antonio Argenziano (2018-05-17 16:08:14)
> 
> 
> On 17/05/18 01:23, Chris Wilson wrote:
> > Confirm we have the available HW before asserting it succeeds.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   tests/gem_cpu_reloc.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/gem_cpu_reloc.c b/tests/gem_cpu_reloc.c
> > index 882c312d4..e3bbcd239 100644
> > --- a/tests/gem_cpu_reloc.c
> > +++ b/tests/gem_cpu_reloc.c
> > @@ -167,6 +167,7 @@ static void run_test(int fd, int count)
> >       use_blt = 0;
> 
> Is this^ meant to be EXEC_DEFAULT?

Depends on your viewpoint. EXEC_DEFAULT is zero.

> >       if (intel_gen(noop) >= 6)
> >               use_blt = I915_EXEC_BLT;
> > +     gem_require_ring(fd, use_blt);
> 
> Are any gens 6+ that do not have a BLT ring? if that is the case 
> shouldn't we use '0' like we do for 5- gens?

No, it has to match the engine for which the blitter commands are valid. If
that engine does not exist, there is no alternative except to rewrite the
test not to use those commands. If there was, it indeed would be included
in the selection above.
-Chris
On 17/05/18 08:37, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-05-17 16:08:14)
>>
>>
>> On 17/05/18 01:23, Chris Wilson wrote:
>>> Confirm we have the available HW before asserting it succeeds.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    tests/gem_cpu_reloc.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/tests/gem_cpu_reloc.c b/tests/gem_cpu_reloc.c
>>> index 882c312d4..e3bbcd239 100644
>>> --- a/tests/gem_cpu_reloc.c
>>> +++ b/tests/gem_cpu_reloc.c
>>> @@ -167,6 +167,7 @@ static void run_test(int fd, int count)
>>>        use_blt = 0;
>>
>> Is this^ meant to be EXEC_DEFAULT?
> 
> Depends on your viewpoint. EXEC_DEFAULT is zero.

Just wandering if it should enforce EXEC_RENDER. Which I think is what 
we want for gen 5-.

> 
>>>        if (intel_gen(noop) >= 6)
>>>                use_blt = I915_EXEC_BLT;
>>> +     gem_require_ring(fd, use_blt);
>>
>> Are any gens 6+ that do not have a BLT ring? if that is the case
>> shouldn't we use '0' like we do for 5- gens?
> 
> No, it has to match the engine for which the blitter commands are valid. If
> that engine does not exist, there is no alternative except to rewrite the
> test not to use those commands. If there was, it indeed would be included
> in the selection above.

So, just to wrap my head around it, the commands we are talking about 
here are allowed on render for gen5- but only on blitter on 6+. Right?

Thanks,
Antonio

> -Chris
>
Quoting Antonio Argenziano (2018-05-17 17:29:26)
> 
> 
> On 17/05/18 08:37, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2018-05-17 16:08:14)
> >>
> >>
> >> On 17/05/18 01:23, Chris Wilson wrote:
> >>> Confirm we have the available HW before asserting it succeeds.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> ---
> >>>    tests/gem_cpu_reloc.c | 1 +
> >>>    1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/tests/gem_cpu_reloc.c b/tests/gem_cpu_reloc.c
> >>> index 882c312d4..e3bbcd239 100644
> >>> --- a/tests/gem_cpu_reloc.c
> >>> +++ b/tests/gem_cpu_reloc.c
> >>> @@ -167,6 +167,7 @@ static void run_test(int fd, int count)
> >>>        use_blt = 0;
> >>
> >> Is this^ meant to be EXEC_DEFAULT?
> > 
> > Depends on your viewpoint. EXEC_DEFAULT is zero.
> 
> Just wandering if it should enforce EXEC_RENDER. Which I think is what 
> we want for gen 5-.

Not really. It just wants the default mixed ring. You definitely don't
want to suggest sending blitter commands down the 3D pipe, that would be
even more confusing.

> >>>        if (intel_gen(noop) >= 6)
> >>>                use_blt = I915_EXEC_BLT;
> >>> +     gem_require_ring(fd, use_blt);
> >>
> >> Are any gens 6+ that do not have a BLT ring? if that is the case
> >> shouldn't we use '0' like we do for 5- gens?
> > 
> > No, it has to match the engine for which the blitter commands are valid. If
> > that engine does not exist, there is no alternative except to rewrite the
> > test not to use those commands. If there was, it indeed would be included
> > in the selection above.
> 
> So, just to wrap my head around it, the commands we are talking about 
> here are allowed on render for gen5- but only on blitter on 6+. Right?

There is no render for gen5- either. There is a universal ringbuffer
that can handle multiple different client commands, and on the odd
machine a bit stream decoder.
-Chris
On 17/05/18 09:52, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-05-17 17:29:26)
>>
>>
>> On 17/05/18 08:37, Chris Wilson wrote:
>>> Quoting Antonio Argenziano (2018-05-17 16:08:14)
>>>>
>>>>
>>>> On 17/05/18 01:23, Chris Wilson wrote:
>>>>> Confirm we have the available HW before asserting it succeeds.
>>>>>
>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>> ---
>>>>>     tests/gem_cpu_reloc.c | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/tests/gem_cpu_reloc.c b/tests/gem_cpu_reloc.c
>>>>> index 882c312d4..e3bbcd239 100644
>>>>> --- a/tests/gem_cpu_reloc.c
>>>>> +++ b/tests/gem_cpu_reloc.c
>>>>> @@ -167,6 +167,7 @@ static void run_test(int fd, int count)
>>>>>         use_blt = 0;
>>>>
>>>> Is this^ meant to be EXEC_DEFAULT?
>>>
>>> Depends on your viewpoint. EXEC_DEFAULT is zero.
>>
>> Just wandering if it should enforce EXEC_RENDER. Which I think is what
>> we want for gen 5-.
> 
> Not really. It just wants the default mixed ring. You definitely don't
> want to suggest sending blitter commands down the 3D pipe, that would be
> even more confusing.

The comment below answers this question as well.

> 
>>>>>         if (intel_gen(noop) >= 6)
>>>>>                 use_blt = I915_EXEC_BLT;
>>>>> +     gem_require_ring(fd, use_blt);
>>>>
>>>> Are any gens 6+ that do not have a BLT ring? if that is the case
>>>> shouldn't we use '0' like we do for 5- gens?
>>>
>>> No, it has to match the engine for which the blitter commands are valid. If
>>> that engine does not exist, there is no alternative except to rewrite the
>>> test not to use those commands. If there was, it indeed would be included
>>> in the selection above.
>>
>> So, just to wrap my head around it, the commands we are talking about
>> here are allowed on render for gen5- but only on blitter on 6+. Right?
> 
> There is no render for gen5- either. There is a universal ringbuffer
> that can handle multiple different client commands, and on the odd
> machine a bit stream decoder.

I see, I wasn't making any sense then :).

Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

> -Chris
>