[PATCH] drm/i915: Skip object locking around a no-op set-domain ioctl

[PATCH] drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

If we are already in the desired write domain of a set-domain ioctl,
then there is nothing for us to do and we can quickly return back to
userspace, avoiding any lock contention.

Secondary aspect of this is that we undo the arbitrary fetching and
potential flushing of all pages for a set-domain(.write=CPU) call on a
fresh object -- which was introduced simply because we do the get-pages
before taking the struct_mutex.

References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1a684b7e8c09..e30165536f9a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1484,7 +1484,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
 		return -EINVAL;
 
-	/* Having something in the write domain implies it's in the read
+	/*
+	 * Having something in the write domain implies it's in the read
 	 * domain, and only that read domain.  Enforce that in the request.
 	 */
 	if (write_domain != 0 && read_domains != write_domain)
@@ -1494,7 +1495,14 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
-	/* Try to flush the object off the GPU without holding the lock.
+	/* Already in the desired target write domain? Nothing for us to! */
+	if (READ_ONCE(obj->write_domain) == write_domain) {
+		err = 0;
+		goto out;
+	}
+
+	/*
+	 * Try to flush the object off the GPU without holding the lock.
 	 * We will repeat the flush holding the lock in the normal manner
 	 * to catch cases where we are gazumped.
 	 */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Skip object locking around a no-op set-domain ioctl

[Patchwork]

== Series Details ==

Series: drm/i915: Skip object locking around a no-op set-domain ioctl
URL   : https://patchwork.freedesktop.org/series/58325/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e968397bb2a0 drm/i915: Skip object locking around a no-op set-domain ioctl
-:15: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#15: 
References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")

-:15: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")'
#15: 
References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")

total: 1 errors, 1 warnings, 0 checks, 24 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Skip object locking around a no-op set-domain ioctl

[Patchwork]

== Series Details ==

Series: drm/i915: Skip object locking around a no-op set-domain ioctl
URL   : https://patchwork.freedesktop.org/series/58325/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5785 -> Patchwork_12542
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12542 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12542, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58325/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12542:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_reloc@basic-cpu-active:
    - fi-bsw-kefka:       PASS -> FAIL +7
    - fi-hsw-4770r:       PASS -> FAIL +3
    - fi-cfl-8109u:       PASS -> FAIL +3
    - fi-byt-clapper:     PASS -> FAIL +7
    - fi-ivb-3770:        PASS -> FAIL +3
    - fi-icl-u3:          PASS -> FAIL +3
    - fi-skl-gvtdvm:      PASS -> FAIL +2

  * igt@gem_exec_reloc@basic-gtt-active:
    - fi-snb-2520m:       PASS -> CRASH +1

  * igt@gem_exec_reloc@basic-gtt-cpu:
    - fi-byt-n2820:       PASS -> FAIL +6
    - fi-elk-e7500:       PASS -> FAIL +9
    - fi-ilk-650:         PASS -> FAIL +8

  * igt@gem_exec_reloc@basic-gtt-cpu-active:
    - fi-kbl-8809g:       PASS -> FAIL +3
    - fi-kbl-x1275:       PASS -> FAIL +3
    - fi-skl-6600u:       PASS -> FAIL +3
    - fi-pnv-d510:        PASS -> FAIL +6
    - fi-apl-guc:         PASS -> FAIL +3
    - fi-skl-6700k2:      PASS -> FAIL +3
    - fi-bxt-j4205:       PASS -> FAIL +3

  * igt@gem_exec_reloc@basic-gtt-cpu-noreloc:
    - fi-bwr-2160:        PASS -> FAIL +7
    - fi-gdg-551:         PASS -> FAIL +4

  * igt@gem_exec_reloc@basic-write-cpu-active:
    - fi-hsw-4770:        PASS -> FAIL +3
    - fi-snb-2520m:       PASS -> FAIL +4
    - fi-skl-guc:         PASS -> FAIL +3
    - fi-kbl-7500u:       PASS -> FAIL +2
    - fi-bdw-5557u:       PASS -> FAIL +3
    - fi-bdw-gvtdvm:      PASS -> FAIL +2
    - fi-kbl-guc:         PASS -> FAIL +3
    - fi-whl-u:           PASS -> FAIL +3
    - fi-kbl-7567u:       PASS -> FAIL +3

  * igt@gem_exec_reloc@basic-write-cpu-noreloc:
    - fi-blb-e6850:       PASS -> FAIL +7
    - fi-bsw-n3050:       PASS -> FAIL +6
    - fi-byt-j1900:       PASS -> FAIL +7

  * igt@gem_exec_reloc@basic-write-gtt-active:
    - fi-pnv-d510:        PASS -> CRASH +2
    - fi-ilk-650:         PASS -> CRASH +1

  * igt@prime_vgem@basic-fence-mmap:
    - fi-cfl-8700k:       PASS -> FAIL +3
    - fi-skl-iommu:       PASS -> FAIL +3
    - fi-cfl-guc:         PASS -> FAIL +3
    - fi-skl-6770hq:      PASS -> FAIL +3
    - fi-skl-6260u:       PASS -> FAIL +3
    - fi-kbl-r:           PASS -> FAIL +3

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@prime_vgem@basic-fence-mmap:
    - {fi-skl-lmem}:      PASS -> FAIL +3

  
Known issues
------------

  Here are the changes found in Patchwork_12542 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       PASS -> DMESG-WARN [fdo#107709]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362] +3

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> FAIL [fdo#107709]
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         FAIL [fdo#104008] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720


Participating hosts (47 -> 39)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5785 -> Patchwork_12542

  CI_DRM_5785: 1e3d80c25878b7d97ad6c0680a452d55baeb28e0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4894: fedd92f4022837e2c20e472b65bd7d0849f484a3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12542: e968397bb2a0f670008379e48b8e228dd0780bbd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e968397bb2a0 drm/i915: Skip object locking around a no-op set-domain ioctl

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12542/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

Quoting Patchwork (2019-03-21 09:04:59)
> == Series Details ==
> 
> Series: drm/i915: Skip object locking around a no-op set-domain ioctl
> URL   : https://patchwork.freedesktop.org/series/58325/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5785 -> Patchwork_12542
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_12542 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_12542, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/58325/revisions/1/mbox/
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_12542:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@gem_exec_reloc@basic-cpu-active:
>     - fi-bsw-kefka:       PASS -> FAIL +7
>     - fi-hsw-4770r:       PASS -> FAIL +3
>     - fi-cfl-8109u:       PASS -> FAIL +3
>     - fi-byt-clapper:     PASS -> FAIL +7
>     - fi-ivb-3770:        PASS -> FAIL +3
>     - fi-icl-u3:          PASS -> FAIL +3
>     - fi-skl-gvtdvm:      PASS -> FAIL +2

Intriguing, now that might explain some of the random failure we've
seen here over the years, as we unexpectedly take the slow path.

>   * igt@prime_vgem@basic-fence-mmap:
>     - fi-cfl-8700k:       PASS -> FAIL +3
>     - fi-skl-iommu:       PASS -> FAIL +3
>     - fi-cfl-guc:         PASS -> FAIL +3
>     - fi-skl-6770hq:      PASS -> FAIL +3
>     - fi-skl-6260u:       PASS -> FAIL +3
>     - fi-kbl-r:           PASS -> FAIL +3

Hmm. Decidedly, hmm.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

Quoting Chris Wilson (2019-03-21 08:05:32)
> If we are already in the desired write domain of a set-domain ioctl,
> then there is nothing for us to do and we can quickly return back to
> userspace, avoiding any lock contention.
> 
> Secondary aspect of this is that we undo the arbitrary fetching and
> potential flushing of all pages for a set-domain(.write=CPU) call on a
> fresh object -- which was introduced simply because we do the get-pages
> before taking the struct_mutex.
> 
> References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1a684b7e8c09..e30165536f9a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1484,7 +1484,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>         if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
>                 return -EINVAL;
>  
> -       /* Having something in the write domain implies it's in the read
> +       /*
> +        * Having something in the write domain implies it's in the read
>          * domain, and only that read domain.  Enforce that in the request.
>          */
>         if (write_domain != 0 && read_domains != write_domain)
> @@ -1494,7 +1495,14 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>         if (!obj)
>                 return -ENOENT;
>  
> -       /* Try to flush the object off the GPU without holding the lock.
> +       /* Already in the desired target write domain? Nothing for us to! */
> +       if (READ_ONCE(obj->write_domain) == write_domain) {

This should be write_domain && ...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

If we are already in the desired write domain of a set-domain ioctl,
then there is nothing for us to do and we can quickly return back to
userspace, avoiding any lock contention.

Secondary aspect of this is that we undo the arbitrary fetching and
potential flushing of all pages for a set-domain(.write=CPU) call on a
fresh object -- which was introduced simply because we do the get-pages
before taking the struct_mutex.

References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1a684b7e8c09..4243d760a420 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1484,17 +1484,25 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
 		return -EINVAL;
 
-	/* Having something in the write domain implies it's in the read
+	/*
+	 * Having something in the write domain implies it's in the read
 	 * domain, and only that read domain.  Enforce that in the request.
 	 */
-	if (write_domain != 0 && read_domains != write_domain)
+	if (write_domain && read_domains != write_domain)
 		return -EINVAL;
 
 	obj = i915_gem_object_lookup(file, args->handle);
 	if (!obj)
 		return -ENOENT;
 
-	/* Try to flush the object off the GPU without holding the lock.
+	/* Already in the desired target write domain? Nothing for us to! */
+	if (write_domain && READ_ONCE(obj->write_domain) == write_domain) {
+		err = 0;
+		goto out;
+	}
+
+	/*
+	 * Try to flush the object off the GPU without holding the lock.
 	 * We will repeat the flush holding the lock in the normal manner
 	 * to catch cases where we are gazumped.
 	 */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

Quoting Chris Wilson (2019-03-21 09:11:30)
> Quoting Patchwork (2019-03-21 09:04:59)
> > == Series Details ==
> > 
> > Series: drm/i915: Skip object locking around a no-op set-domain ioctl
> > URL   : https://patchwork.freedesktop.org/series/58325/
> > State : failure
> > 
> > == Summary ==
> > 
> > CI Bug Log - changes from CI_DRM_5785 -> Patchwork_12542
> > ====================================================
> > 
> > Summary
> > -------
> > 
> >   **FAILURE**
> > 
> >   Serious unknown changes coming with Patchwork_12542 absolutely need to be
> >   verified manually.
> >   
> >   If you think the reported changes have nothing to do with the changes
> >   introduced in Patchwork_12542, please notify your bug team to allow them
> >   to document this new failure mode, which will reduce false positives in CI.
> > 
> >   External URL: https://patchwork.freedesktop.org/api/1.0/series/58325/revisions/1/mbox/
> > 
> > Possible new issues
> > -------------------
> > 
> >   Here are the unknown changes that may have been introduced in Patchwork_12542:
> > 
> > ### IGT changes ###
> > 
> > #### Possible regressions ####
> > 
> >   * igt@gem_exec_reloc@basic-cpu-active:
> >     - fi-bsw-kefka:       PASS -> FAIL +7
> >     - fi-hsw-4770r:       PASS -> FAIL +3
> >     - fi-cfl-8109u:       PASS -> FAIL +3
> >     - fi-byt-clapper:     PASS -> FAIL +7
> >     - fi-ivb-3770:        PASS -> FAIL +3
> >     - fi-icl-u3:          PASS -> FAIL +3
> >     - fi-skl-gvtdvm:      PASS -> FAIL +2
> 
> Intriguing, now that might explain some of the random failure we've
> seen here over the years, as we unexpectedly take the slow path.

Sadly, not these are the active variant so the inaccurate test
shortcircuited the required read synchronisation.

And I hope the lack of the read clflush will explain the !llc..
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

If we are already in the desired write domain of a set-domain ioctl,
then there is nothing for us to do and we can quickly return back to
userspace, avoiding any lock contention. By recognising that the
write_domain is always a subset of the read_domains, and excluding the
no-op case of requiring 0 read_domains in the ioctl, we can infer if the
current write_domain matches the target read_domains, there is nothing
for us to do.

Secondary aspect of this is that we undo the arbitrary fetching and
potential flushing of all pages for a set-domain(.write=CPU) call on a
fresh object -- which was introduced simply because we do the get-pages
before taking the struct_mutex.

References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1a684b7e8c09..82f0bd316564 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1484,17 +1484,28 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
 		return -EINVAL;
 
-	/* Having something in the write domain implies it's in the read
+	/*
+	 * Having something in the write domain implies it's in the read
 	 * domain, and only that read domain.  Enforce that in the request.
 	 */
-	if (write_domain != 0 && read_domains != write_domain)
+	if (write_domain && read_domains != write_domain)
 		return -EINVAL;
 
+	if (!read_domains)
+		return 0;
+
 	obj = i915_gem_object_lookup(file, args->handle);
 	if (!obj)
 		return -ENOENT;
 
-	/* Try to flush the object off the GPU without holding the lock.
+	/* Already in the desired target write domain? Nothing for us to! */
+	if (READ_ONCE(obj->write_domain) == read_domains) {
+		err = 0;
+		goto out;
+	}
+
+	/*
+	 * Try to flush the object off the GPU without holding the lock.
 	 * We will repeat the flush holding the lock in the normal manner
 	 * to catch cases where we are gazumped.
 	 */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4] drm/i915: Skip object locking around a no-op set-domain ioctl

[Chris Wilson]

If we are already in the desired write domain of a set-domain ioctl,
then there is nothing for us to do and we can quickly return back to
userspace, avoiding any lock contention. By recognising that the
write_domain is always a subset of the read_domains, and excluding the
no-op case of requiring 0 read_domains in the ioctl, we can infer if the
current write_domain matches the target read_domains, there is nothing
for us to do.

Secondary aspect of this is that we undo the arbitrary fetching and
potential flushing of all pages for a set-domain(.write=CPU) call on a
fresh object -- which was introduced simply because we do the get-pages
before taking the struct_mutex.

References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1a684b7e8c09..85a19b906e72 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1484,17 +1484,37 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if ((write_domain | read_domains) & I915_GEM_GPU_DOMAINS)
 		return -EINVAL;
 
-	/* Having something in the write domain implies it's in the read
+	/*
+	 * Having something in the write domain implies it's in the read
 	 * domain, and only that read domain.  Enforce that in the request.
 	 */
-	if (write_domain != 0 && read_domains != write_domain)
+	if (write_domain && read_domains != write_domain)
 		return -EINVAL;
 
+	if (!read_domains)
+		return 0;
+
 	obj = i915_gem_object_lookup(file, args->handle);
 	if (!obj)
 		return -ENOENT;
 
-	/* Try to flush the object off the GPU without holding the lock.
+	/*
+	 * Already in the desired target write domain? Nothing for us to!
+	 *
+	 * We apply a little bit of cunning here to catch a broader set of
+	 * no-ops. If obj->write_domain is set, we must be in the same
+	 * obj->read_domains, and only that domain. Therefore, if that
+	 * obj->write_domain matches the request read_domains, we are
+	 * already in the same read/write domain and can skip the operation,
+	 * without having to further check the requested write_domain.
+	 */
+	if (READ_ONCE(obj->write_domain) == read_domains) {
+		err = 0;
+		goto out;
+	}
+
+	/*
+	 * Try to flush the object off the GPU without holding the lock.
 	 * We will repeat the flush holding the lock in the normal manner
 	 * to catch cases where we are gazumped.
 	 */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Skip object locking around a no-op set-domain ioctl (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Skip object locking around a no-op set-domain ioctl (rev4)
URL   : https://patchwork.freedesktop.org/series/58325/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f18176be40dd drm/i915: Skip object locking around a no-op set-domain ioctl
-:19: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#19: 
References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")

-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")'
#19: 
References: 40e62d5d6be8 ("drm/i915: Acquire the backing storage outside of struct_mutex in set-domain")

total: 1 errors, 1 warnings, 0 checks, 40 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Skip object locking around a no-op set-domain ioctl (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Skip object locking around a no-op set-domain ioctl (rev4)
URL   : https://patchwork.freedesktop.org/series/58325/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5787 -> Patchwork_12546
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58325/revisions/4/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12546 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@gtt-bsd:
    - fi-bwr-2160:        NOTRUN -> SKIP [fdo#109271] +103

  * igt@gem_exec_basic@readonly-bsd1:
    - fi-snb-2520m:       NOTRUN -> SKIP [fdo#109271] +57

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-7500u:       PASS -> DMESG-WARN [fdo#105128] / [fdo#107139]

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        PASS -> DMESG-FAIL [fdo#110210]

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@basic-flip-c:
    - fi-bwr-2160:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-snb-2520m:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +62

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-bxt-j4205:       NOTRUN -> SKIP [fdo#109271] +47

  * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         PASS -> DMESG-FAIL [fdo#103182]

  
#### Possible fixes ####

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210


Participating hosts (42 -> 37)
------------------------------

  Additional (4): fi-bxt-j4205 fi-bwr-2160 fi-snb-2520m fi-bsw-n3050 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-iommu fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5787 -> Patchwork_12546

  CI_DRM_5787: 8a27af0b94f56d403bc7806a64a50013a2dd9396 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4896: 0f9c061247fb7aba21c9459f19f437927a28f32c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12546: f18176be40dd5c167b35f64bc33444007353a5f8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f18176be40dd drm/i915: Skip object locking around a no-op set-domain ioctl

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12546/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Skip object locking around a no-op set-domain ioctl (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Skip object locking around a no-op set-domain ioctl (rev4)
URL   : https://patchwork.freedesktop.org/series/58325/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5787_full -> Patchwork_12546_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12546_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@drm_read@short-buffer-block:
    - shard-snb:          PASS -> SKIP [fdo#109271]

  * igt@gem_bad_reloc@negative-reloc-lut-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +5

  * igt@gem_create@create-clear:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540]

  * igt@gem_eio@in-flight-suspend:
    - shard-snb:          PASS -> FAIL [fdo#103375]

  * igt@gem_mocs_settings@mocs-reset-dirty-render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#110206]

  * igt@gem_stolen@stolen-pread:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277]

  * igt@gem_wait@wait-bsd2:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +34

  * igt@i915_pm_lpsp@edp-native:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109301]

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109982]

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109308]

  * igt@i915_pm_rpm@system-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107807]

  * igt@kms_atomic_transition@3x-modeset-transitions:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +10

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-e:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +2

  * igt@kms_chamelium@vga-frame-dump:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284]

  * igt@kms_cursor_legacy@cursorb-vs-flipb-legacy:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +1

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-apl:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +16

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-cpu:
    - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247]

  * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
    - shard-skl:          NOTRUN -> FAIL [fdo#105683]

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +38

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-cur-indfb-draw-mmap-wc:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +6

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815]

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-mid:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_psr@cursor_mmap_gtt:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +4

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109441]

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +3

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          NOTRUN -> FAIL [fdo#109016]

  * igt@kms_vrr@flip-basic:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109502]

  * igt@kms_vrr@flip-suspend:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +140

  * igt@prime_vgem@fence-read-hang:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109295]

  
#### Possible fixes ####

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-iclb:         INCOMPLETE [fdo#109766] / [fdo#109801] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
    - shard-iclb:         FAIL [fdo#103355] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-tilingchange:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +11

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@primary_mmap_gtt:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-modeset-rpm:
    - shard-apl:          FAIL [fdo#104894] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
    - shard-iclb:         FAIL [fdo#104894] -> PASS

  
#### Warnings ####

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> FAIL [fdo#109358]

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         SKIP [fdo#109441] -> FAIL [fdo#107383] / [fdo#110215] +1

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109301]: https://bugs.freedesktop.org/show_bug.cgi?id=109301
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109358]: https://bugs.freedesktop.org/show_bug.cgi?id=109358
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109502]: https://bugs.freedesktop.org/show_bug.cgi?id=109502
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109766]: https://bugs.freedesktop.org/show_bug.cgi?id=109766
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#109982]: https://bugs.freedesktop.org/show_bug.cgi?id=109982
  [fdo#110206]: https://bugs.freedesktop.org/show_bug.cgi?id=110206
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5787 -> Patchwork_12546

  CI_DRM_5787: 8a27af0b94f56d403bc7806a64a50013a2dd9396 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4896: 0f9c061247fb7aba21c9459f19f437927a28f32c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12546: f18176be40dd5c167b35f64bc33444007353a5f8 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12546/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx