| Message ID | 20161019081228.25533-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2016.10.19 09:12:28 +0100, Chris Wilson wrote: > For whatever reason, the gvt scheduler runs synchronously. At the very > least, lets run synchronously without holding the struct_mutex. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> This will break current device model which assume to serve one VM context on hw at each time but can't switch to another one between for emulation issue. This is not good but its current implementation limit we'll address later. > --- > drivers/gpu/drm/i915/gvt/scheduler.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c > index 4cedd3274da7..aa83dd3381a3 100644 > --- a/drivers/gpu/drm/i915/gvt/scheduler.c > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c > @@ -420,10 +420,6 @@ static int workload_thread(void *priv) > > intel_runtime_pm_get(gvt->dev_priv); > > - /* > - * Always take i915 big lock first > - */ > - mutex_lock(&gvt->dev_priv->drm.struct_mutex); > > gvt_dbg_sched("ring id %d will dispatch workload %p\n", > workload->ring_id, workload); > @@ -432,7 +428,10 @@ static int workload_thread(void *priv) > intel_uncore_forcewake_get(gvt->dev_priv, > FORCEWAKE_ALL); > > + mutex_lock(&gvt->dev_priv->drm.struct_mutex); > ret = dispatch_workload(workload); > + mutex_lock(&gvt->dev_priv->drm.struct_mutex); > + > if (ret) { > gvt_err("fail to dispatch workload, skip\n"); > goto complete; > @@ -442,8 +441,7 @@ static int workload_thread(void *priv) > workload->ring_id, workload); > > workload->status = i915_wait_request(workload->req, > - I915_WAIT_LOCKED, > - NULL, NULL); > + 0, NULL, NULL); > if (workload->status != 0) > gvt_err("fail to wait workload, skip\n"); > > @@ -451,13 +449,14 @@ static int workload_thread(void *priv) > gvt_dbg_sched("will complete workload %p\n, status: %d\n", > workload, workload->status); > > + mutex_lock(&gvt->dev_priv->drm.struct_mutex); > complete_current_workload(gvt, ring_id); > + mutex_unlock(&gvt->dev_priv->drm.struct_mutex); > > if (need_force_wake) > intel_uncore_forcewake_put(gvt->dev_priv, > FORCEWAKE_ALL); > > - mutex_unlock(&gvt->dev_priv->drm.struct_mutex); > > intel_runtime_pm_put(gvt->dev_priv); > } > -- > 2.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 19 October 2016 at 09:12, Chris Wilson <chris@chris-wilson.co.uk> wrote: > For whatever reason, the gvt scheduler runs synchronously. At the very > least, lets run synchronously without holding the struct_mutex. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gvt/scheduler.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c > index 4cedd3274da7..aa83dd3381a3 100644 > --- a/drivers/gpu/drm/i915/gvt/scheduler.c > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c > @@ -420,10 +420,6 @@ static int workload_thread(void *priv) > > intel_runtime_pm_get(gvt->dev_priv); > > - /* > - * Always take i915 big lock first > - */ > - mutex_lock(&gvt->dev_priv->drm.struct_mutex); > > gvt_dbg_sched("ring id %d will dispatch workload %p\n", > workload->ring_id, workload); > @@ -432,7 +428,10 @@ static int workload_thread(void *priv) > intel_uncore_forcewake_get(gvt->dev_priv, > FORCEWAKE_ALL); > > + mutex_lock(&gvt->dev_priv->drm.struct_mutex); > ret = dispatch_workload(workload); > + mutex_lock(&gvt->dev_priv->drm.struct_mutex); mutex_unlock...
On Wed, Oct 19, 2016 at 04:54:28PM +0800, Zhenyu Wang wrote: > On 2016.10.19 09:12:28 +0100, Chris Wilson wrote: > > For whatever reason, the gvt scheduler runs synchronously. At the very > > least, lets run synchronously without holding the struct_mutex. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > This will break current device model which assume to serve one VM context > on hw at each time but can't switch to another one between for emulation > issue. That's a scheduling issue, which should not require struct_mutex. So what is struct_mutex being used for? -Chris
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 4cedd3274da7..aa83dd3381a3 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -420,10 +420,6 @@ static int workload_thread(void *priv) intel_runtime_pm_get(gvt->dev_priv); - /* - * Always take i915 big lock first - */ - mutex_lock(&gvt->dev_priv->drm.struct_mutex); gvt_dbg_sched("ring id %d will dispatch workload %p\n", workload->ring_id, workload); @@ -432,7 +428,10 @@ static int workload_thread(void *priv) intel_uncore_forcewake_get(gvt->dev_priv, FORCEWAKE_ALL); + mutex_lock(&gvt->dev_priv->drm.struct_mutex); ret = dispatch_workload(workload); + mutex_lock(&gvt->dev_priv->drm.struct_mutex); + if (ret) { gvt_err("fail to dispatch workload, skip\n"); goto complete; @@ -442,8 +441,7 @@ static int workload_thread(void *priv) workload->ring_id, workload); workload->status = i915_wait_request(workload->req, - I915_WAIT_LOCKED, - NULL, NULL); + 0, NULL, NULL); if (workload->status != 0) gvt_err("fail to wait workload, skip\n"); @@ -451,13 +449,14 @@ static int workload_thread(void *priv) gvt_dbg_sched("will complete workload %p\n, status: %d\n", workload, workload->status); + mutex_lock(&gvt->dev_priv->drm.struct_mutex); complete_current_workload(gvt, ring_id); + mutex_unlock(&gvt->dev_priv->drm.struct_mutex); if (need_force_wake) intel_uncore_forcewake_put(gvt->dev_priv, FORCEWAKE_ALL); - mutex_unlock(&gvt->dev_priv->drm.struct_mutex); intel_runtime_pm_put(gvt->dev_priv); }
For whatever reason, the gvt scheduler runs synchronously. At the very least, lets run synchronously without holding the struct_mutex. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gvt/scheduler.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)