Quoting Tvrtko Ursulin (2019-09-25 11:57:53)
>
> On 25/09/2019 11:01, Chris Wilson wrote:
> > @@ -423,6 +424,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
> > static int create_mmap_offset(struct drm_i915_gem_object *obj)
> > {
> > struct drm_i915_private *i915 = to_i915(obj->base.dev);
> > + struct intel_gt *gt = &i915->gt;
> > int err;
> >
> > err = drm_gem_create_mmap_offset(&obj->base);
> > @@ -431,7 +433,7 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
> >
> > /* Attempt to reap some mmap space from dead objects */
> > do {
> > - err = i915_gem_wait_for_idle(i915, MAX_SCHEDULE_TIMEOUT);
> > + err = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>
> It may be useful to keep the i915_gem_wait_for_idle as trivial wrapper
> which can then call intel_gt_wait_for_idle. Keeps the separation of GEM
> and GT better, but also..
> > if (err)
> > break;
> >
> > @@ -440,7 +442,7 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
> > if (!err)
> > break;
> >
> > - } while (flush_delayed_work(&i915->gem.retire_work));
> > + } while (flush_delayed_work(>->requests.retire_work));
>
> .. here could be considered a layering violation that GEM is peeking
> into GT internals like this. Shall we have another wrapper like
> i915_gem_retire_requests_sync which would call intel_gt_retire_requests
> and new intel_gt_flush_retire, or something?
This is the most complicated case because we're managing a global
resource. Actually this loop can be eliminated, as it was only trying to
avoid taking struct_mutex here (using the worker instead). Now that
we can call retire directly, it can be reduced.
After which I think the point is moot as the path forward is clearer.
> > @@ -180,8 +178,6 @@ struct drm_i915_private *mock_gem_device(void)
> >
> > mock_init_contexts(i915);
> >
> > - INIT_DELAYED_WORK(&i915->gem.retire_work, mock_retire_work_handler);
> > -
> > i915->gt.awake = -1;
> >
> > intel_timelines_init(i915);
> >
>
> I am still slightly uneasy about having requests, which I see as a GEM
> concept, be retired from GT, but okay, it's not the most important issue
> at the moment.
requests are not a GEM concept, they belong in sched/ :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
