| Message ID | 20220921070945.27764-3-niranjana.vishwanathapura@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/vm_bind: Add VM_BIND functionality | expand |
On 21/09/2022 08:09, Niranjana Vishwanathapura wrote: > Add function __i915_sw_fence_await_reservation() for > asynchronous wait on a dma-resv object with specified > dma_resv_usage. This is required for async vma unbind > with vm_bind. > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> > --- > drivers/gpu/drm/i915/i915_sw_fence.c | 25 ++++++++++++++++++------- > drivers/gpu/drm/i915/i915_sw_fence.h | 7 ++++++- > 2 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c > index 6fc0d1b89690..0ce8f4efc1ed 100644 > --- a/drivers/gpu/drm/i915/i915_sw_fence.c > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c > @@ -569,12 +569,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, > return ret; > } > > -int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > - struct dma_resv *resv, > - const struct dma_fence_ops *exclude, > - bool write, > - unsigned long timeout, > - gfp_t gfp) > +int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > + struct dma_resv *resv, > + enum dma_resv_usage usage, > + unsigned long timeout, > + gfp_t gfp) > { > struct dma_resv_iter cursor; > struct dma_fence *f; > @@ -583,7 +582,7 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > debug_fence_assert(fence); > might_sleep_if(gfpflags_allow_blocking(gfp)); > > - dma_resv_iter_begin(&cursor, resv, dma_resv_usage_rw(write)); > + dma_resv_iter_begin(&cursor, resv, usage); > dma_resv_for_each_fence_unlocked(&cursor, f) { > pending = i915_sw_fence_await_dma_fence(fence, f, timeout, > gfp); > @@ -598,6 +597,18 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > return ret; > } > > +int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > + struct dma_resv *resv, > + const struct dma_fence_ops *exclude, > + bool write, > + unsigned long timeout, > + gfp_t gfp) > +{ > + return __i915_sw_fence_await_reservation(fence, resv, > + dma_resv_usage_rw(write), > + timeout, gfp); > +} Drive by observation - it looked dodgy that you create a wrapper here which ignores one function parameter. On a more detailed look it seems no callers actually use exclude and it's even unused inside this function since 1b5bdf071e62 ("drm/i915: use the new iterator in i915_sw_fence_await_reservation v3"). So a cleanup patch before this one? Regards, Tvrtko > + > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > #include "selftests/lib_sw_fence.c" > #include "selftests/i915_sw_fence.c" > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h > index 619fc5a22f0c..3cf4b6e16f35 100644 > --- a/drivers/gpu/drm/i915/i915_sw_fence.h > +++ b/drivers/gpu/drm/i915/i915_sw_fence.h > @@ -10,13 +10,13 @@ > #define _I915_SW_FENCE_H_ > > #include <linux/dma-fence.h> > +#include <linux/dma-resv.h> > #include <linux/gfp.h> > #include <linux/kref.h> > #include <linux/notifier.h> /* for NOTIFY_DONE */ > #include <linux/wait.h> > > struct completion; > -struct dma_resv; > struct i915_sw_fence; > > enum i915_sw_fence_notify { > @@ -89,6 +89,11 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, > unsigned long timeout, > gfp_t gfp); > > +int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > + struct dma_resv *resv, > + enum dma_resv_usage usage, > + unsigned long timeout, > + gfp_t gfp); > int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > struct dma_resv *resv, > const struct dma_fence_ops *exclude,
On Wed, Sep 21, 2022 at 10:06:48AM +0100, Tvrtko Ursulin wrote: > >On 21/09/2022 08:09, Niranjana Vishwanathapura wrote: >>Add function __i915_sw_fence_await_reservation() for >>asynchronous wait on a dma-resv object with specified >>dma_resv_usage. This is required for async vma unbind >>with vm_bind. >> >>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> >>--- >> drivers/gpu/drm/i915/i915_sw_fence.c | 25 ++++++++++++++++++------- >> drivers/gpu/drm/i915/i915_sw_fence.h | 7 ++++++- >> 2 files changed, 24 insertions(+), 8 deletions(-) >> >>diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c >>index 6fc0d1b89690..0ce8f4efc1ed 100644 >>--- a/drivers/gpu/drm/i915/i915_sw_fence.c >>+++ b/drivers/gpu/drm/i915/i915_sw_fence.c >>@@ -569,12 +569,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, >> return ret; >> } >>-int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, >>- struct dma_resv *resv, >>- const struct dma_fence_ops *exclude, >>- bool write, >>- unsigned long timeout, >>- gfp_t gfp) >>+int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, >>+ struct dma_resv *resv, >>+ enum dma_resv_usage usage, >>+ unsigned long timeout, >>+ gfp_t gfp) >> { >> struct dma_resv_iter cursor; >> struct dma_fence *f; >>@@ -583,7 +582,7 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, >> debug_fence_assert(fence); >> might_sleep_if(gfpflags_allow_blocking(gfp)); >>- dma_resv_iter_begin(&cursor, resv, dma_resv_usage_rw(write)); >>+ dma_resv_iter_begin(&cursor, resv, usage); >> dma_resv_for_each_fence_unlocked(&cursor, f) { >> pending = i915_sw_fence_await_dma_fence(fence, f, timeout, >> gfp); >>@@ -598,6 +597,18 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, >> return ret; >> } >>+int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, >>+ struct dma_resv *resv, >>+ const struct dma_fence_ops *exclude, >>+ bool write, >>+ unsigned long timeout, >>+ gfp_t gfp) >>+{ >>+ return __i915_sw_fence_await_reservation(fence, resv, >>+ dma_resv_usage_rw(write), >>+ timeout, gfp); >>+} > >Drive by observation - it looked dodgy that you create a wrapper here >which ignores one function parameter. > >On a more detailed look it seems no callers actually use exclude and >it's even unused inside this function since 1b5bdf071e62 ("drm/i915: >use the new iterator in i915_sw_fence_await_reservation v3"). > >So a cleanup patch before this one? > Thanks Tvrtko. Yah, I noticed it, but did not want to fix that here. Sure, will post a patch beforehand to fix that. Niranjana >Regards, > >Tvrtko > > >>+ >> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) >> #include "selftests/lib_sw_fence.c" >> #include "selftests/i915_sw_fence.c" >>diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h >>index 619fc5a22f0c..3cf4b6e16f35 100644 >>--- a/drivers/gpu/drm/i915/i915_sw_fence.h >>+++ b/drivers/gpu/drm/i915/i915_sw_fence.h >>@@ -10,13 +10,13 @@ >> #define _I915_SW_FENCE_H_ >> #include <linux/dma-fence.h> >>+#include <linux/dma-resv.h> >> #include <linux/gfp.h> >> #include <linux/kref.h> >> #include <linux/notifier.h> /* for NOTIFY_DONE */ >> #include <linux/wait.h> >> struct completion; >>-struct dma_resv; >> struct i915_sw_fence; >> enum i915_sw_fence_notify { >>@@ -89,6 +89,11 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, >> unsigned long timeout, >> gfp_t gfp); >>+int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, >>+ struct dma_resv *resv, >>+ enum dma_resv_usage usage, >>+ unsigned long timeout, >>+ gfp_t gfp); >> int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, >> struct dma_resv *resv, >> const struct dma_fence_ops *exclude,
On Wed, 21 Sep 2022, Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> wrote: > Add function __i915_sw_fence_await_reservation() for > asynchronous wait on a dma-resv object with specified > dma_resv_usage. This is required for async vma unbind > with vm_bind. > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> > --- > drivers/gpu/drm/i915/i915_sw_fence.c | 25 ++++++++++++++++++------- > drivers/gpu/drm/i915/i915_sw_fence.h | 7 ++++++- > 2 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c > index 6fc0d1b89690..0ce8f4efc1ed 100644 > --- a/drivers/gpu/drm/i915/i915_sw_fence.c > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c > @@ -569,12 +569,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, > return ret; > } > > -int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > - struct dma_resv *resv, > - const struct dma_fence_ops *exclude, > - bool write, > - unsigned long timeout, > - gfp_t gfp) > +int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > + struct dma_resv *resv, > + enum dma_resv_usage usage, > + unsigned long timeout, > + gfp_t gfp) > { > struct dma_resv_iter cursor; > struct dma_fence *f; > @@ -583,7 +582,7 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > debug_fence_assert(fence); > might_sleep_if(gfpflags_allow_blocking(gfp)); > > - dma_resv_iter_begin(&cursor, resv, dma_resv_usage_rw(write)); > + dma_resv_iter_begin(&cursor, resv, usage); > dma_resv_for_each_fence_unlocked(&cursor, f) { > pending = i915_sw_fence_await_dma_fence(fence, f, timeout, > gfp); > @@ -598,6 +597,18 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > return ret; > } > > +int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > + struct dma_resv *resv, > + const struct dma_fence_ops *exclude, > + bool write, > + unsigned long timeout, > + gfp_t gfp) > +{ > + return __i915_sw_fence_await_reservation(fence, resv, > + dma_resv_usage_rw(write), > + timeout, gfp); > +} > + > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > #include "selftests/lib_sw_fence.c" > #include "selftests/i915_sw_fence.c" > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h > index 619fc5a22f0c..3cf4b6e16f35 100644 > --- a/drivers/gpu/drm/i915/i915_sw_fence.h > +++ b/drivers/gpu/drm/i915/i915_sw_fence.h > @@ -10,13 +10,13 @@ > #define _I915_SW_FENCE_H_ > > #include <linux/dma-fence.h> > +#include <linux/dma-resv.h> As a GCC extension you can drop this and forward declare enum dma_resv_usage. We use it extensively. > #include <linux/gfp.h> > #include <linux/kref.h> > #include <linux/notifier.h> /* for NOTIFY_DONE */ > #include <linux/wait.h> > > struct completion; > -struct dma_resv; > struct i915_sw_fence; > > enum i915_sw_fence_notify { > @@ -89,6 +89,11 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, > unsigned long timeout, > gfp_t gfp); > > +int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > + struct dma_resv *resv, > + enum dma_resv_usage usage, > + unsigned long timeout, > + gfp_t gfp); > int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, > struct dma_resv *resv, > const struct dma_fence_ops *exclude,
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c index 6fc0d1b89690..0ce8f4efc1ed 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.c +++ b/drivers/gpu/drm/i915/i915_sw_fence.c @@ -569,12 +569,11 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, return ret; } -int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, - struct dma_resv *resv, - const struct dma_fence_ops *exclude, - bool write, - unsigned long timeout, - gfp_t gfp) +int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, + struct dma_resv *resv, + enum dma_resv_usage usage, + unsigned long timeout, + gfp_t gfp) { struct dma_resv_iter cursor; struct dma_fence *f; @@ -583,7 +582,7 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, debug_fence_assert(fence); might_sleep_if(gfpflags_allow_blocking(gfp)); - dma_resv_iter_begin(&cursor, resv, dma_resv_usage_rw(write)); + dma_resv_iter_begin(&cursor, resv, usage); dma_resv_for_each_fence_unlocked(&cursor, f) { pending = i915_sw_fence_await_dma_fence(fence, f, timeout, gfp); @@ -598,6 +597,18 @@ int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, return ret; } +int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, + struct dma_resv *resv, + const struct dma_fence_ops *exclude, + bool write, + unsigned long timeout, + gfp_t gfp) +{ + return __i915_sw_fence_await_reservation(fence, resv, + dma_resv_usage_rw(write), + timeout, gfp); +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/lib_sw_fence.c" #include "selftests/i915_sw_fence.c" diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h b/drivers/gpu/drm/i915/i915_sw_fence.h index 619fc5a22f0c..3cf4b6e16f35 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence.h +++ b/drivers/gpu/drm/i915/i915_sw_fence.h @@ -10,13 +10,13 @@ #define _I915_SW_FENCE_H_ #include <linux/dma-fence.h> +#include <linux/dma-resv.h> #include <linux/gfp.h> #include <linux/kref.h> #include <linux/notifier.h> /* for NOTIFY_DONE */ #include <linux/wait.h> struct completion; -struct dma_resv; struct i915_sw_fence; enum i915_sw_fence_notify { @@ -89,6 +89,11 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence *fence, unsigned long timeout, gfp_t gfp); +int __i915_sw_fence_await_reservation(struct i915_sw_fence *fence, + struct dma_resv *resv, + enum dma_resv_usage usage, + unsigned long timeout, + gfp_t gfp); int i915_sw_fence_await_reservation(struct i915_sw_fence *fence, struct dma_resv *resv, const struct dma_fence_ops *exclude,
Add function __i915_sw_fence_await_reservation() for asynchronous wait on a dma-resv object with specified dma_resv_usage. This is required for async vma unbind with vm_bind. Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> --- drivers/gpu/drm/i915/i915_sw_fence.c | 25 ++++++++++++++++++------- drivers/gpu/drm/i915/i915_sw_fence.h | 7 ++++++- 2 files changed, 24 insertions(+), 8 deletions(-)