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

[Mika Kuoppala]

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:

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;
>>