To help avoid evicting already resident buffers from the batch we're processing, perform locking as a separate step.
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 201fed19d120..394eb40c95b5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -922,21 +922,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) return err; }
-static int eb_validate_vmas(struct i915_execbuffer *eb) +static int eb_lock_vmas(struct i915_execbuffer *eb) { unsigned int i; int err;
- INIT_LIST_HEAD(&eb->unbound); - for (i = 0; i < eb->buffer_count; i++) { - struct drm_i915_gem_exec_object2 *entry = &eb->exec[i]; struct eb_vma *ev = &eb->vma[i]; struct i915_vma *vma = ev->vma;
err = i915_gem_object_lock(vma->obj, &eb->ww); if (err) return err; + } + + return 0; +} + +static int eb_validate_vmas(struct i915_execbuffer *eb) +{ + unsigned int i; + int err; + + INIT_LIST_HEAD(&eb->unbound); + + err = eb_lock_vmas(eb); + if (err) + return err; + + for (i = 0; i < eb->buffer_count; i++) { + struct drm_i915_gem_exec_object2 *entry = &eb->exec[i]; + struct eb_vma *ev = &eb->vma[i]; + struct i915_vma *vma = ev->vma;
err = eb_pin_vma(eb, entry, ev); if (err == -EDEADLK)
On 2021-06-15 at 13:36:00 +0200, Thomas Hellström wrote:
To help avoid evicting already resident buffers from the batch we're processing, perform locking as a separate step.
Looks reasonable to me.
Reviewed-by: Ramalingam C ramalingam.c@intel.com
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 201fed19d120..394eb40c95b5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -922,21 +922,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) return err; }
-static int eb_validate_vmas(struct i915_execbuffer *eb) +static int eb_lock_vmas(struct i915_execbuffer *eb) { unsigned int i; int err;
INIT_LIST_HEAD(&eb->unbound);
for (i = 0; i < eb->buffer_count; i++) {
struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
struct eb_vma *ev = &eb->vma[i]; struct i915_vma *vma = ev->vma;
err = i915_gem_object_lock(vma->obj, &eb->ww); if (err) return err;
- }
- return 0;
+}
+static int eb_validate_vmas(struct i915_execbuffer *eb) +{
unsigned int i;
int err;
INIT_LIST_HEAD(&eb->unbound);
err = eb_lock_vmas(eb);
if (err)
return err;
for (i = 0; i < eb->buffer_count; i++) {
struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
struct eb_vma *ev = &eb->vma[i];
struct i915_vma *vma = ev->vma;
err = eb_pin_vma(eb, entry, ev); if (err == -EDEADLK)
-- 2.31.1
On 6/17/21 11:56 AM, Ramalingam C wrote:
On 2021-06-15 at 13:36:00 +0200, Thomas Hellström wrote:
To help avoid evicting already resident buffers from the batch we're processing, perform locking as a separate step.
Looks reasonable to me.
Reviewed-by: Ramalingam C ramalingam.c@intel.com
Thanks for reviewing!
/Thomas
Op 15-06-2021 om 13:36 schreef Thomas Hellström:
To help avoid evicting already resident buffers from the batch we're processing, perform locking as a separate step.
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 201fed19d120..394eb40c95b5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -922,21 +922,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) return err; }
-static int eb_validate_vmas(struct i915_execbuffer *eb) +static int eb_lock_vmas(struct i915_execbuffer *eb) { unsigned int i; int err;
INIT_LIST_HEAD(&eb->unbound);
for (i = 0; i < eb->buffer_count; i++) {
struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
struct eb_vma *ev = &eb->vma[i]; struct i915_vma *vma = ev->vma;
err = i915_gem_object_lock(vma->obj, &eb->ww); if (err) return err;
- }
- return 0;
+}
+static int eb_validate_vmas(struct i915_execbuffer *eb) +{
unsigned int i;
int err;
INIT_LIST_HEAD(&eb->unbound);
err = eb_lock_vmas(eb);
if (err)
return err;
for (i = 0; i < eb->buffer_count; i++) {
struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
struct eb_vma *ev = &eb->vma[i];
struct i915_vma *vma = ev->vma;
err = eb_pin_vma(eb, entry, ev); if (err == -EDEADLK)
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Thomas Hellström Sent: Tuesday, June 15, 2021 4:36 AM To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Thomas Hellström thomas.hellstrom@linux.intel.com; Auld, Matthew matthew.auld@intel.com Subject: [Intel-gfx] [PATCH] drm/i915: Perform execbuffer object locking as a separate step
To help avoid evicting already resident buffers from the batch we're processing, perform locking as a separate step.
Signed-off-by: Thomas Hellström thomas.hellstrom@linux.intel.com
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 25 ++++++++++++++++--
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 201fed19d120..394eb40c95b5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -922,21 +922,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) return err; }
-static int eb_validate_vmas(struct i915_execbuffer *eb) +static int eb_lock_vmas(struct i915_execbuffer *eb) { unsigned int i; int err;
INIT_LIST_HEAD(&eb->unbound);
for (i = 0; i < eb->buffer_count; i++) {
struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
struct eb_vma *ev = &eb->vma[i]; struct i915_vma *vma = ev->vma;
err = i915_gem_object_lock(vma->obj, &eb->ww); if (err) return err;
- }
- return 0;
+}
+static int eb_validate_vmas(struct i915_execbuffer *eb) {
unsigned int i;
int err;
INIT_LIST_HEAD(&eb->unbound);
err = eb_lock_vmas(eb);
if (err)
return err;
for (i = 0; i < eb->buffer_count; i++) {
struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
struct eb_vma *ev = &eb->vma[i];
struct i915_vma *vma = ev->vma;
err = eb_pin_vma(eb, entry, ev); if (err == -EDEADLK)
Thomas, just checked eb_pin_vma(), it calls i915_vma_pin_ww(), if the object is already locked, under what condition these calls still return -EDEADLK?
--CQ
-- 2.31.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
dri-devel@lists.freedesktop.org