On Wed, 23 Jul 2014 16:10:32 +0100
John Harrison <John.C.Harrison@xxxxxxxxx> wrote:
> On 02/07/2014 19:20, Jesse Barnes wrote:
> > On Thu, 26 Jun 2014 18:24:04 +0100
> > John.C.Harrison@xxxxxxxxx wrote:
> >
> >> From: John Harrison <John.C.Harrison@xxxxxxxxx>
> >>
> >> The scheduler decouples the submission of batch buffers to the driver with
> >> submission of batch buffers to the hardware. Thus it is possible for an
> >> application to submit work, then close the DRM handle and free up all the
> >> resources that piece of work wishes to use before the work has even been
> >> submitted to the hardware. To prevent this, the scheduler needs to be informed
> >> of the DRM close event so that it can force through any outstanding work
> >> attributed to that file handle.
> >> ---
> >> drivers/gpu/drm/i915/i915_dma.c | 3 +++
> >> drivers/gpu/drm/i915/i915_scheduler.c | 18 ++++++++++++++++++
> >> drivers/gpu/drm/i915/i915_scheduler.h | 2 ++
> >> 3 files changed, 23 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> >> index 494b156..6c9ce82 100644
> >> --- a/drivers/gpu/drm/i915/i915_dma.c
> >> +++ b/drivers/gpu/drm/i915/i915_dma.c
> >> @@ -42,6 +42,7 @@
> >> #include <linux/vga_switcheroo.h>
> >> #include <linux/slab.h>
> >> #include <acpi/video.h>
> >> +#include "i915_scheduler.h"
> >> #include <linux/pm.h>
> >> #include <linux/pm_runtime.h>
> >> #include <linux/oom.h>
> >> @@ -1930,6 +1931,8 @@ void i915_driver_lastclose(struct drm_device * dev)
> >>
> >> void i915_driver_preclose(struct drm_device *dev, struct drm_file *file)
> >> {
> >> + i915_scheduler_closefile(dev, file);
> >> +
> >> mutex_lock(&dev->struct_mutex);
> >> i915_gem_context_close(dev, file);
> >> i915_gem_release(dev, file);
> >> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> >> index d9c1879..66a6568 100644
> >> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> >> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> >> @@ -78,6 +78,19 @@ bool i915_scheduler_is_seqno_in_flight(struct intel_engine_cs *ring,
> >> return found;
> >> }
> >>
> >> +int i915_scheduler_closefile(struct drm_device *dev, struct drm_file *file)
> >> +{
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + struct i915_scheduler *scheduler = dev_priv->scheduler;
> >> +
> >> + if (!scheduler)
> >> + return 0;
> >> +
> >> + /* Do stuff... */
> >> +
> >> + return 0;
> >> +}
> >> +
> >> #else /* CONFIG_DRM_I915_SCHEDULER */
> >>
> >> int i915_scheduler_init(struct drm_device *dev)
> >> @@ -85,4 +98,9 @@ int i915_scheduler_init(struct drm_device *dev)
> >> return 0;
> >> }
> >>
> >> +int i915_scheduler_closefile(struct drm_device *dev, struct drm_file *file)
> >> +{
> >> + return 0;
> >> +}
> >> +
> >> #endif /* CONFIG_DRM_I915_SCHEDULER */
> >> diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
> >> index 4044b6e..95641f6 100644
> >> --- a/drivers/gpu/drm/i915/i915_scheduler.h
> >> +++ b/drivers/gpu/drm/i915/i915_scheduler.h
> >> @@ -27,6 +27,8 @@
> >>
> >> bool i915_scheduler_is_enabled(struct drm_device *dev);
> >> int i915_scheduler_init(struct drm_device *dev);
> >> +int i915_scheduler_closefile(struct drm_device *dev,
> >> + struct drm_file *file);
> >>
> >> #ifdef CONFIG_DRM_I915_SCHEDULER
> >>
> > Yeah I guess the client could have passed a ref to some other process
> > for tracking the outstanding work, so we need to complete it.
> >
> > But shouldn't that happen as part of the clearing of the outstanding
> > requests in i915_gem_suspend() which is called from lastclose()? We do
> > a gpu_idle() and retire_requests() in there already...
> >
>
> Note that this is per file close not the global close. Individual DRM
> file handles are closed whenever a user land app stops using DRM. When
> that happens, the scheduler needs to clean up all references to that
> handle. It is not just to ensure all work belonging to that handle has
> completed but also to ensure the scheduler does not attempt to deference
> dodgy file pointers later on.
Ah yeah sorry, mixed it up with lastclose. Looks fine for per-client
close.
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
