On 03/13/2014 08:15 AM, Mika Kuoppala wrote:
> Ian Romanick <i...@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 <i...@freedesktop.org>
>>> Signed-off-by: Kenneth Graunke <kenn...@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...
> static bool gem_has_hw_contexts(int fd)
> {
> struct local_drm_i915_gem_context_create create;
> int ret;
>
> memset(&create, 0, sizeof(create));
> ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
>
> if (ret == 0) {
> drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &create);
> return true;
> }
>
> return false;
> }
>
> static bool gem_has_reset_stats(int fd)
> {
> struct local_drm_i915_reset_stats rs;
> int ret;
>
> /* Carefully set flags and pad to zero, otherwise
> we get -EINVAL
> */
> memset(&rs, 0, sizeof(rs));
>
> ret = drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs);
> if (ret == 0)
> return true;
>
> /* If we get EPERM, we have support but did not
> have CAP_SYSADM */
> if (ret == -1 && errno == EPERM)
> return true;
>
> return false;
> }
>
> -Mika
>
>
>>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
>>> b/src/mesa/drivers/dri/i965/intel_screen.c
>>> index 464cebf..12006be 100644
>>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>>> @@ -1342,13 +1342,18 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>>> * no error. If the ioctl is not supported, it always generate EINVAL.
>>> * Use this to determine whether to advertise the __DRI2_ROBUSTNESS
>>> * extension to the loader.
>>> + *
>>> + * Don't even try on pre-Gen6, since we don't attempt to use contexts
>>> there.
>>> */
>>> - struct drm_i915_reset_stats stats;
>>> - memset(&stats, 0, sizeof(stats));
>>> + if (intelScreen->devinfo->gen >= 6) {
>>> + struct drm_i915_reset_stats stats;
>>> + memset(&stats, 0, sizeof(stats));
>>>
>>> - const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS,
>>> &stats);
>>> + const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS,
>>> &stats);
>>>
>>> - intelScreen->has_context_reset_notification = (ret != -1 || errno !=
>>> EINVAL);
>>> + intelScreen->has_context_reset_notification =
>>> + (ret != -1 || errno != EINVAL);
>>> + }
>>>
>>> psp->extensions = !intelScreen->has_context_reset_notification
>>> ? intelScreenExtensions : intelRobustScreenExtensions;
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
