Re: [Intel-gfx] [PATCH 08/10] drm/i915/uapi: disable capturing objects on recoverable contexts

From: "Thomas Hellström (Intel)" <thomas_os@shipmail.org>
To: Matthew Auld <matthew.auld@intel.com>, intel-gfx@lists.freedesktop.org
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"Kenneth Graunke" <kenneth@whitecape.org>,
	dri-devel@lists.freedesktop.org,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>
Subject: Re: [Intel-gfx] [PATCH 08/10] drm/i915/uapi: disable capturing objects on recoverable contexts
Date: Fri, 17 Jun 2022 14:28:35 +0200	[thread overview]
Message-ID: <edf3956e-ed30-0716-c6ae-eac7aaadd7df@shipmail.org> (raw)
In-Reply-To: <20220525184337.491763-9-matthew.auld@intel.com>

On 5/25/22 20:43, Matthew Auld wrote:
> A non-recoverable context must be used if the user wants proper error
> capture on discrete platforms. In the future the kernel may want to blit
> the contents of some objects when later doing the capture stage.
>
> Testcase: igt@gem_exec_capture@capture-recoverable-discrete
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index b279588c0672..e27ccfa50dc3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1961,7 +1961,7 @@ eb_find_first_request_added(struct i915_execbuffer *eb)
>   #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
>   
>   /* Stage with GFP_KERNEL allocations before we enter the signaling critical path */
> -static void eb_capture_stage(struct i915_execbuffer *eb)
> +static int eb_capture_stage(struct i915_execbuffer *eb)
>   {
>   	const unsigned int count = eb->buffer_count;
>   	unsigned int i = count, j;
> @@ -1974,6 +1974,10 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
>   		if (!(flags & EXEC_OBJECT_CAPTURE))
>   			continue;
>   
> +		if (i915_gem_context_is_recoverable(eb->gem_context) &&
> +		    IS_DGFX(eb->i915))
> +			return -EINVAL;
> +

We should also require this for future integrated, for capture buffer 
memory allocation purposes.

Otherwise Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>