[PATCH v2 1/2] drm/i915/gem: Allow EXEC_CAPTURE on recoverable contexts on DG1

[PATCH v2 1/2] drm/i915/gem: Allow EXEC_CAPTURE on recoverable contexts on DG1

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The intel-media-driver is currently broken on DG1 because
it uses EXEC_CAPTURE with recovarable contexts. Relax the
check to allow that.

I've also submitted a fix for the intel-media-driver:
https://github.com/intel/media-driver/pull/1920

Cc: stable@vger.kernel.org
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Testcase: igt/gem_exec_capture/capture-invisible
Fixes: 71b1669ea9bd ("drm/i915/uapi: tweak error capture on recoverable contexts")
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index ca7e9216934a..ea9d5063ce78 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2013,7 +2013,7 @@ static int eb_capture_stage(struct i915_execbuffer *eb)
 			continue;
 
 		if (i915_gem_context_is_recoverable(eb->gem_context) &&
-		    (IS_DGFX(eb->i915) || GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 0)))
+		    GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 10))
 			return -EINVAL;
 
 		for_each_batch_create_order(eb, j) {
-- 
2.49.0

Re: [PATCH v2 1/2] drm/i915/gem: Allow EXEC_CAPTURE on recoverable contexts on DG1

[Joonas Lahtinen]

(+ Tvrkto)

Quoting Ville Syrjala (2025-04-11 17:43:12)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The intel-media-driver is currently broken on DG1 because
> it uses EXEC_CAPTURE with recovarable contexts. Relax the
> check to allow that.
> 
> I've also submitted a fix for the intel-media-driver:
> https://github.com/intel/media-driver/pull/1920
> 
> Cc: stable@vger.kernel.org
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Testcase: igt/gem_exec_capture/capture-invisible
> Fixes: 71b1669ea9bd ("drm/i915/uapi: tweak error capture on recoverable contexts")
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index ca7e9216934a..ea9d5063ce78 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -2013,7 +2013,7 @@ static int eb_capture_stage(struct i915_execbuffer *eb)
>                         continue;
>  
>                 if (i915_gem_context_is_recoverable(eb->gem_context) &&
> -                   (IS_DGFX(eb->i915) || GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 0)))
> +                   GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 10))

The IS_DGFX check was there because the error capture is expected to be
broken on anything with VRAM.

If we have already submitted an userspace fix to remove that flag, that
would be the right way to go.

So reverting this for now.

Regards, Joonas

>                         return -EINVAL;
>  
>                 for_each_batch_create_order(eb, j) {
> -- 
> 2.49.0
>

Re: [PATCH v2 1/2] drm/i915/gem: Allow EXEC_CAPTURE on recoverable contexts on DG1

[Andi Shyti]

Hi Joonas,

On Thu, May 22, 2025 at 09:25:04AM +0300, Joonas Lahtinen wrote:
> (+ Tvrkto)
> 
> Quoting Ville Syrjala (2025-04-11 17:43:12)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The intel-media-driver is currently broken on DG1 because
> > it uses EXEC_CAPTURE with recovarable contexts. Relax the
> > check to allow that.
> > 
> > I've also submitted a fix for the intel-media-driver:
> > https://github.com/intel/media-driver/pull/1920
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Testcase: igt/gem_exec_capture/capture-invisible
> > Fixes: 71b1669ea9bd ("drm/i915/uapi: tweak error capture on recoverable contexts")
> > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index ca7e9216934a..ea9d5063ce78 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -2013,7 +2013,7 @@ static int eb_capture_stage(struct i915_execbuffer *eb)
> >                         continue;
> >  
> >                 if (i915_gem_context_is_recoverable(eb->gem_context) &&
> > -                   (IS_DGFX(eb->i915) || GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 0)))
> > +                   GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 10))
> 
> The IS_DGFX check was there because the error capture is expected to be
> broken on anything with VRAM.
> 
> If we have already submitted an userspace fix to remove that flag, that
> would be the right way to go.

Oh! We need to add a comment there, though, because in the
future I expect someone else might decide to send the same patch.

Andi

Re: [PATCH v2 1/2] drm/i915/gem: Allow EXEC_CAPTURE on recoverable contexts on DG1

[Ville Syrjälä]

On Thu, May 22, 2025 at 09:25:04AM +0300, Joonas Lahtinen wrote:
> (+ Tvrkto)
> 
> Quoting Ville Syrjala (2025-04-11 17:43:12)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The intel-media-driver is currently broken on DG1 because
> > it uses EXEC_CAPTURE with recovarable contexts. Relax the
> > check to allow that.
> > 
> > I've also submitted a fix for the intel-media-driver:
> > https://github.com/intel/media-driver/pull/1920
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Testcase: igt/gem_exec_capture/capture-invisible
> > Fixes: 71b1669ea9bd ("drm/i915/uapi: tweak error capture on recoverable contexts")
> > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index ca7e9216934a..ea9d5063ce78 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -2013,7 +2013,7 @@ static int eb_capture_stage(struct i915_execbuffer *eb)
> >                         continue;
> >  
> >                 if (i915_gem_context_is_recoverable(eb->gem_context) &&
> > -                   (IS_DGFX(eb->i915) || GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 0)))
> > +                   GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 10))
> 
> The IS_DGFX check was there because the error capture is expected to be
> broken on anything with VRAM.

I don't care. It's a regression that prevents current userspace
from working.

> 
> If we have already submitted an userspace fix to remove that flag, that
> would be the right way to go.

There has a been an open pull request for that for who knows how long
without any action.

> 
> So reverting this for now.

*You* make sure a userspace fix actually gets released then.

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 1/2] drm/i915/gem: Allow EXEC_CAPTURE on recoverable contexts on DG1

[Joonas Lahtinen]

Quoting Ville Syrjälä (2025-05-22 12:35:05)
> On Thu, May 22, 2025 at 09:25:04AM +0300, Joonas Lahtinen wrote:
> > (+ Tvrkto)
> > 
> > Quoting Ville Syrjala (2025-04-11 17:43:12)

<SNIP>

> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > @@ -2013,7 +2013,7 @@ static int eb_capture_stage(struct i915_execbuffer *eb)
> > >                         continue;
> > >  
> > >                 if (i915_gem_context_is_recoverable(eb->gem_context) &&
> > > -                   (IS_DGFX(eb->i915) || GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 0)))
> > > +                   GRAPHICS_VER_FULL(eb->i915) > IP_VER(12, 10))
> > 
> > The IS_DGFX check was there because the error capture is expected to be
> > broken on anything with VRAM.
> 
> I don't care. It's a regression that prevents current userspace
> from working.

(Spoiler: The userspace fix seems to be accepted and released.)

It's always bit murky when a platform stays under force_probe for
extended period of time, but it was never considered to be finalized from
memory management perspective at the time of adding this check.

Now you are just unblocking codepaths that are simply not expected to work,
and as it's in rather fragile part of the device resets so that's bit of
a no-go.

So if you really would prefer to drop this check for DG1, options would be to
implement the page copying for VRAM (probably bit much work) or alternatively
we could just ignore the flag for DG1 specifically.

> > If we have already submitted an userspace fix to remove that flag, that
> > would be the right way to go.
> 
> There has a been an open pull request for that for who knows how long
> without any action.
> 
> > 
> > So reverting this for now.
> 
> *You* make sure a userspace fix actually gets released then.

Per GIT history[1] it should already be part of media-driver release 25.2.3
which was released last week. Or am I looking at the wrong fix?

Regards, Joonas

[1] https://github.com/intel/media-driver/commit/93c07d9b4b96a78bab21f6acd4eb863f4313ea4a