[Mesa-dev] [PATCH] i965: Don't enable reset notification support on Gen4-5.

[Kenneth Graunke]

On 04/07/2014 08:47 AM, Ian Romanick wrote:
> On 03/13/2014 08:15 AM, Mika Kuoppala wrote:
>> Ian Romanick <idr at freedesktop.org> writes:
>>
>>> On 03/12/2014 01:43 AM, Kenneth Graunke wrote:
>>>> arekm reported that using Chrome with GPU acceleration enabled on GM45
>>>> triggered the hw_ctx != NULL assertion in brw_get_graphics_reset_status.
>>>>
>>>> We definitely do not want to advertise reset notification support on
>>>> Gen4-5 systems, since it needs hardware contexts, and we never even
>>>> request a hardware context on those systems.
>>>>
>>>> Cc: Ian Romanick <idr at freedesktop.org>
>>>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>>>> ---
>>>>  src/mesa/drivers/dri/i965/intel_screen.c | 13 +++++++++----
>>>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> This may not be the best solution; if someone would like to send patches for
>>>> a better one, I'm more than happy to go with something else.  Checking for
>>>> hardware contexts is a bit awkward - we need to decide whether to advertise
>>>> support here in the screen, but we don't create hardware contexts until
>>>> context creation time, which is a fair bit later.  So, I'm not sure how to
>>>> do better than the generation check, for now.
>>>
>>> I'm confused.  I thought the ioctl was was supposed to fail with EINVAL
>>> in that case.  Mika, what's your suggestion?
>>
>> I think two ioctl queries are needed to catch the combinations,
>> unfortunately :(
>>
>> For gens without hw_contexts, we keep statistics per file
>> descriptor and thus we can ban batch submission through
>> file desc if has been sending hanging batches. This is
>> tied to reset stats so that if there is no hw_contexts,
>> you get statistics for your fd.
>>
>> Here is how I do it in tests:
> 
> So... in the reset tests, you do something like:
> 
>     if (gem_has_hw_contexts(fd) && gem_has_reset_stats(fd))
>         do_reset_with_context_test();
> 
> I guess that wouldn't be a terrible way to handle it in Mesa...

I've gone ahead and pushed my patch without review, since nobody came up
with a better solution, it's been over a month, and users machines have
been broken.

As far as I can tell, the reset stats ioctl is not useful without a
context.  We could pass 0 (the default context), but that raises -EPERM
unless you are root (CAP_SYS_ADMIN).  Root-only APIs aren't useful for
implementing general GL features.  I don't see any other API to
determine whether there were resets (say, using an FD alone).

Making reset statistics root-only seems bizarre to me, seeing as we
report GPU hangs in dmesg, which any normal user is allowed to run.
Locking down the convenient API when anyone can parse dmesg seems...odd.

--Ken

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140430/09e6b67c/attachment.sig>