Ben Widawsky <ben at bwidawsk.net> writes:
> Make resets optional for fini. fini is only ever called in
> module_unload. It was therefore a good assumption that the GPU reset
> (see the comment in the function) was what we wanted. With an upcoming
> patch, we're going to add a few more callsites, one of which is GPU
> reset, where doing the extra reset isn't usual.
>
> On that same note, with more callers it makes sense to make the default
> context state consistent with the actual state (via NULLing the
> pointer).
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 7 +++++--
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 3b315ba..a141b8a 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1764,7 +1764,7 @@ int i915_driver_unload(struct drm_device *dev)
> mutex_lock(&dev->struct_mutex);
> i915_gem_free_all_phys_object(dev);
> i915_gem_cleanup_ringbuffer(dev);
> - i915_gem_context_fini(dev);
> + i915_gem_context_fini(dev, true);
> mutex_unlock(&dev->struct_mutex);
> i915_gem_cleanup_aliasing_ppgtt(dev);
> i915_gem_cleanup_stolen(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9717c69..618845e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1698,7 +1698,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
>
> /* i915_gem_context.c */
> void i915_gem_context_init(struct drm_device *dev);
> -void i915_gem_context_fini(struct drm_device *dev);
> +void i915_gem_context_fini(struct drm_device *dev, bool reset);
> void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
> int i915_switch_context(struct intel_ring_buffer *ring,
> struct drm_file *file, int to_id);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 411ace0..6030d83 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -259,7 +259,7 @@ void i915_gem_context_init(struct drm_device *dev)
> DRM_DEBUG_DRIVER("HW context support initialized\n");
> }
>
> -void i915_gem_context_fini(struct drm_device *dev)
> +void i915_gem_context_fini(struct drm_device *dev, bool reset)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> @@ -269,11 +269,14 @@ void i915_gem_context_fini(struct drm_device *dev)
> /* The only known way to stop the gpu from accessing the hw context is
> * to reset it. Do this as the very last operation to avoid confusing
> * other code, leading to spurious errors. */
Should we switch to default context in here to be sure that
the running context will get unreferenced correctly?...
> - intel_gpu_reset(dev);
> + if (reset)
> + intel_gpu_reset(dev);
>
> i915_gem_object_unpin(dev_priv->ring[RCS].default_context->obj);
>
...and unreference the default context here so that it will get freed
on module unload.
--Mika
> do_destroy(dev_priv->ring[RCS].default_context);
> +
> + dev_priv->ring[RCS].default_context = NULL;
> }
>
> static int context_idr_cleanup(int id, void *p, void *data)
> --
> 1.8.2.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
