| Message ID | 1457693986-6892-15-git-send-email-zhi.a.wang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 11, 2016 at 06:59:45PM +0800, Zhi Wang wrote: > Factor out and expose fence stealing functionality for GVT-g. GVT-g > will use i915_find_fence_reg() to find a free/unpin fence register > and use i915_steal_fence() to steal it. > > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_fence.c | 36 ++++++++++++++++++++++++++--------- > 2 files changed, 28 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 26106e5..deb7143b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3247,6 +3247,7 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj) > /* i915_gem_fence.c */ > int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj); > int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj); > +int i915_steal_fence(struct drm_i915_fence_reg *reg); > > bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj); > void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj); > diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c > index 5981985..dd897c6 100644 > --- a/drivers/gpu/drm/i915/i915_gem_fence.c > +++ b/drivers/gpu/drm/i915/i915_gem_fence.c > @@ -346,6 +346,30 @@ deadlock: > } > > /** > + * i915_steal_fence - steal a fence from a GEM object > + * @reg: the fence register to be stolen > + * > + * Returns: > + * > + * 0 on success, negative error code on failure. > + */ > +int i915_steal_fence(struct drm_i915_fence_reg *reg) > +{ > + int ret; > + No, this is not safe. -Chris
Hi Chris:
Do you mean I should also check the fence pin count in this API like i915_find_fence_reg, then it will be safe here? :)
-----Original Message-----
From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
Sent: Friday, March 11, 2016 7:22 PM
To: Wang, Zhi A
Cc: intel-gfx@lists.freedesktop.org; igvt-g@lists.01.org; Tian, Kevin; Lv, Zhiyuan; Niu, Bing; Song, Jike; daniel.vetter@ffwll.ch; Cowperthwaite, David J; joonas.lahtinen@linux.intel.com
Subject: Re: [RFCv3 14/15] drm/i915: factor out and expose i915_steal_fence()
On Fri, Mar 11, 2016 at 06:59:45PM +0800, Zhi Wang wrote:
> Factor out and expose fence stealing functionality for GVT-g. GVT-g
> will use i915_find_fence_reg() to find a free/unpin fence register
> and use i915_steal_fence() to steal it.
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem_fence.c | 36 ++++++++++++++++++++++++++---------
> 2 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 26106e5..deb7143b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3247,6 +3247,7 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj)
> /* i915_gem_fence.c */
> int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
> int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
> +int i915_steal_fence(struct drm_i915_fence_reg *reg);
>
> bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj);
> void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj);
> diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c
> index 5981985..dd897c6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_fence.c
> +++ b/drivers/gpu/drm/i915/i915_gem_fence.c
> @@ -346,6 +346,30 @@ deadlock:
> }
>
> /**
> + * i915_steal_fence - steal a fence from a GEM object
> + * @reg: the fence register to be stolen
> + *
> + * Returns:
> + *
> + * 0 on success, negative error code on failure.
> + */
> +int i915_steal_fence(struct drm_i915_fence_reg *reg)
> +{
> + int ret;
> +
No, this is not safe.
-Chris
On Fri, Mar 11, 2016 at 12:29:06PM +0000, Wang, Zhi A wrote: > Hi Chris: > Do you mean I should also check the fence pin count in this API like i915_find_fence_reg, then it will be safe here? :) Along those lines. I don't like the prospect of exporting this API and without seeing the consumer I cannot advise on a better method. -Chris
OK. I see. Previously the fence registers for host is partitioned. For example, host i915 will only take 4 fence registers, others are kept by GVT-g resource allocator for vGPUs. Danile and Kevin, they thought we could use i915 fence stealing functionality to prevent this static partition. So my idea is: a. When GVT create a guest, it will call i915_find_fence_regs for available physical fence registers. b. Then it will call i915_steal_fence and pin these fence registers for vGPUs. c. Then unpin them, when a guest is destroyed. So I expose these two APIs. Do you have any better ideas to share? :) And thanks for the comments. :) -----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Friday, March 11, 2016 8:43 PM To: Wang, Zhi A Cc: intel-gfx@lists.freedesktop.org; igvt-g@lists.01.org; Tian, Kevin; Lv, Zhiyuan; Niu, Bing; Song, Jike; daniel.vetter@ffwll.ch; Cowperthwaite, David J; joonas.lahtinen@linux.intel.com Subject: Re: [RFCv3 14/15] drm/i915: factor out and expose i915_steal_fence() On Fri, Mar 11, 2016 at 12:29:06PM +0000, Wang, Zhi A wrote: > Hi Chris: > Do you mean I should also check the fence pin count in this API like i915_find_fence_reg, then it will be safe here? :) Along those lines. I don't like the prospect of exporting this API and without seeing the consumer I cannot advise on a better method. -Chris
BTW, for better review, I didn't send out the whole GVT-g device model here. Just the patches related to i915... :) -----Original Message----- From: Wang, Zhi A Sent: Friday, March 11, 2016 8:54 PM To: 'Chris Wilson' Cc: intel-gfx@lists.freedesktop.org; igvt-g@lists.01.org; Tian, Kevin; Lv, Zhiyuan; Niu, Bing; Song, Jike; daniel.vetter@ffwll.ch; Cowperthwaite, David J; joonas.lahtinen@linux.intel.com Subject: RE: [RFCv3 14/15] drm/i915: factor out and expose i915_steal_fence() OK. I see. Previously the fence registers for host is partitioned. For example, host i915 will only take 4 fence registers, others are kept by GVT-g resource allocator for vGPUs. Danile and Kevin, they thought we could use i915 fence stealing functionality to prevent this static partition. So my idea is: a. When GVT create a guest, it will call i915_find_fence_regs for available physical fence registers. b. Then it will call i915_steal_fence and pin these fence registers for vGPUs. c. Then unpin them, when a guest is destroyed. So I expose these two APIs. Do you have any better ideas to share? :) And thanks for the comments. :) -----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Friday, March 11, 2016 8:43 PM To: Wang, Zhi A Cc: intel-gfx@lists.freedesktop.org; igvt-g@lists.01.org; Tian, Kevin; Lv, Zhiyuan; Niu, Bing; Song, Jike; daniel.vetter@ffwll.ch; Cowperthwaite, David J; joonas.lahtinen@linux.intel.com Subject: Re: [RFCv3 14/15] drm/i915: factor out and expose i915_steal_fence() On Fri, Mar 11, 2016 at 12:29:06PM +0000, Wang, Zhi A wrote: > Hi Chris: > Do you mean I should also check the fence pin count in this API like i915_find_fence_reg, then it will be safe here? :) Along those lines. I don't like the prospect of exporting this API and without seeing the consumer I cannot advise on a better method. -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 26106e5..deb7143b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3247,6 +3247,7 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj) /* i915_gem_fence.c */ int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj); int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj); +int i915_steal_fence(struct drm_i915_fence_reg *reg); bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj); void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj); diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c index 5981985..dd897c6 100644 --- a/drivers/gpu/drm/i915/i915_gem_fence.c +++ b/drivers/gpu/drm/i915/i915_gem_fence.c @@ -346,6 +346,30 @@ deadlock: } /** + * i915_steal_fence - steal a fence from a GEM object + * @reg: the fence register to be stolen + * + * Returns: + * + * 0 on success, negative error code on failure. + */ +int i915_steal_fence(struct drm_i915_fence_reg *reg) +{ + int ret; + + if (reg->obj) { + struct drm_i915_gem_object *old = reg->obj; + + ret = i915_gem_object_wait_fence(old); + if (ret) + return ret; + + i915_gem_object_fence_lost(old); + } + return 0; +} + +/** * i915_gem_object_get_fence - set up fencing for an object * @obj: object to map through a fence reg * @@ -397,15 +421,9 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj) if (IS_ERR(reg)) return PTR_ERR(reg); - if (reg->obj) { - struct drm_i915_gem_object *old = reg->obj; - - ret = i915_gem_object_wait_fence(old); - if (ret) - return ret; - - i915_gem_object_fence_lost(old); - } + ret = i915_steal_fence(reg); + if (ret) + return ret; } else return 0;
Factor out and expose fence stealing functionality for GVT-g. GVT-g will use i915_find_fence_reg() to find a free/unpin fence register and use i915_steal_fence() to steal it. Signed-off-by: Zhi Wang <zhi.a.wang@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_fence.c | 36 ++++++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-)