* [PATCH] drm/i915: Keep physical cursors pinned while in use
@ 2018-08-17 8:24 Chris Wilson
2018-08-17 8:29 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Chris Wilson @ 2018-08-17 8:24 UTC (permalink / raw)
To: intel-gfx
The optimisation inherent in commit 6a2c4232ece1 ("drm/i915: Make the
physical object coherent with GTT") relies on that once we allocated a
cursor we would have coherent, zero overhead access to the scanout plane
holding the cursor. That is we could then do the very frequent cursor
updates X enjoys with no indirection or kernel involvement. However,
that all hinges on the GGTT mmap of the cursor being pinned and not
require refaulting on each access -- handling such a page fault likely
requires the busy GGTT to be rearranged causing a stall. A very simple
fix is then to handle the physical cursor exactly like other cursors and
keep its vma pinned while active.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600
References: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 592b847db88e..d47ec9fd4af4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12966,8 +12966,11 @@ static int intel_plane_pin_fb(struct intel_plane_state *plane_state)
INTEL_INFO(dev_priv)->cursor_needs_physical) {
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
const int align = intel_cursor_alignment(dev_priv);
+ int err;
- return i915_gem_object_attach_phys(obj, align);
+ err = i915_gem_object_attach_phys(obj, align);
+ if (err)
+ return err;
}
vma = intel_pin_and_fence_fb_obj(fb,
--
2.18.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Keep physical cursors pinned while in use
2018-08-17 8:24 [PATCH] drm/i915: Keep physical cursors pinned while in use Chris Wilson
@ 2018-08-17 8:29 ` Patchwork
2018-08-17 8:47 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-08-17 8:29 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Keep physical cursors pinned while in use
URL : https://patchwork.freedesktop.org/series/48379/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
df341afb28f5 drm/i915: Keep physical cursors pinned while in use
-:21: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#21:
References: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT")
-:21: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT")'
#21:
References: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT")
total: 1 errors, 1 warnings, 0 checks, 12 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Keep physical cursors pinned while in use
2018-08-17 8:24 [PATCH] drm/i915: Keep physical cursors pinned while in use Chris Wilson
2018-08-17 8:29 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-08-17 8:47 ` Patchwork
2018-08-17 11:04 ` ✓ Fi.CI.IGT: " Patchwork
2018-08-22 12:35 ` [PATCH] " Ville Syrjälä
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-08-17 8:47 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Keep physical cursors pinned while in use
URL : https://patchwork.freedesktop.org/series/48379/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4684 -> Patchwork_9966 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/48379/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9966:
=== IGT changes ===
==== Possible regressions ====
{igt@pm_rpm@module-reload}:
fi-bxt-dsi: NOTRUN -> WARN
{fi-icl-u}: NOTRUN -> WARN
{fi-bdw-samus}: NOTRUN -> DMESG-FAIL
== Known issues ==
Here are the changes found in Patchwork_9966 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
{igt@kms_psr@primary_page_flip}:
{fi-icl-u}: NOTRUN -> FAIL (fdo#107383) +3
{igt@kms_psr@sprite_plane_onoff}:
{fi-bdw-samus}: NOTRUN -> FAIL (fdo#107360)
==== Possible fixes ====
igt@drv_selftest@live_hangcheck:
fi-kbl-guc: DMESG-FAIL (fdo#106947, fdo#106560) -> PASS
fi-bxt-j4205: DMESG-FAIL (fdo#106560) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: INCOMPLETE (fdo#103927) -> PASS
{igt@kms_psr@primary_mmap_gtt}:
fi-cnl-psr: DMESG-WARN (fdo#107372) -> PASS
==== Warnings ====
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)
{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#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
fdo#107360 https://bugs.freedesktop.org/show_bug.cgi?id=107360
fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
== Participating hosts (52 -> 48) ==
Additional (2): fi-icl-u fi-bdw-samus
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-bsw-cyan fi-ctg-p8600
== Build changes ==
* Linux: CI_DRM_4684 -> Patchwork_9966
CI_DRM_4684: bb1a6d0044581c5d8867afde39111ea4605c644d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4604: 2a5777f8a694f1f8edcf021afb1ef36192c6762d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9966: df341afb28f5bdd2eaa3eba5a15dc854d08474b5 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
df341afb28f5 drm/i915: Keep physical cursors pinned while in use
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9966/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* ✓ Fi.CI.IGT: success for drm/i915: Keep physical cursors pinned while in use
2018-08-17 8:24 [PATCH] drm/i915: Keep physical cursors pinned while in use Chris Wilson
2018-08-17 8:29 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-08-17 8:47 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-08-17 11:04 ` Patchwork
2018-08-22 12:35 ` [PATCH] " Ville Syrjälä
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-08-17 11:04 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Keep physical cursors pinned while in use
URL : https://patchwork.freedesktop.org/series/48379/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4684_full -> Patchwork_9966_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9966_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9966_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9966_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_params@dr1-dirt:
shard-kbl: PASS -> ( 2 PASS ) +842
igt@gem_exec_schedule@preempt-other-chain-blt:
shard-snb: SKIP -> ( 2 SKIP ) +1076
igt@gem_pwrite_pread@snooped-pwrite-blt-cpu_mmap-correctness:
shard-snb: PASS -> ( 2 PASS ) +1062
igt@gem_userptr_blits@map-fixed-invalidate-overlap:
shard-glk: PASS -> ( 2 PASS ) +1216
igt@kms_concurrent@pipe-d:
shard-hsw: SKIP -> ( 2 SKIP ) +584
igt@kms_flip@2x-plain-flip-ts-check:
shard-apl: SKIP -> ( 2 SKIP ) +516
igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
shard-glk: SKIP -> ( 2 SKIP ) +473
igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc:
shard-kbl: SKIP -> ( 2 SKIP ) +311
igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
shard-apl: PASS -> ( 2 PASS ) +1098
igt@pm_rc6_residency@rc6-accuracy:
shard-kbl: PASS -> ( 1 PASS, 1 SKIP )
igt@syncobj_basic@bad-create-flags:
shard-hsw: PASS -> ( 2 PASS ) +1107
== Known issues ==
Here are the changes found in Patchwork_9966_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_schedule@pi-ringfull-blt:
shard-kbl: NOTRUN -> ( 2 FAIL ) (fdo#103158)
igt@gem_mocs_settings@mocs-rc6-bsd1:
shard-snb: SKIP -> ( 2 INCOMPLETE ) (fdo#105411)
igt@kms_setmode@basic:
shard-kbl: PASS -> FAIL (fdo#99912)
igt@kms_sysfs_edid_timing:
shard-kbl: NOTRUN -> ( 2 FAIL ) (fdo#100047)
igt@perf@polling:
shard-hsw: PASS -> ( 1 FAIL, 1 PASS ) (fdo#102252)
==== Possible fixes ====
igt@prime_vgem@basic-fence-flip:
shard-snb: FAIL (fdo#104008) -> ( 2 PASS )
==== Warnings ====
igt@gem_exec_schedule@pi-ringfull-bsd:
shard-glk: FAIL (fdo#103158) -> ( 2 FAIL ) (fdo#103158) +1
igt@gem_exec_schedule@pi-ringfull-render:
shard-apl: FAIL (fdo#103158) -> ( 2 FAIL ) (fdo#103158) +1
igt@gem_exec_schedule@pi-ringfull-vebox:
shard-kbl: FAIL (fdo#103158) -> ( 2 FAIL ) (fdo#103158)
igt@kms_available_modes_crc@available_mode_test_crc:
shard-hsw: FAIL (fdo#106641) -> ( 2 FAIL ) (fdo#106641)
shard-kbl: FAIL (fdo#106641) -> ( 2 FAIL ) (fdo#106641)
igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
shard-glk: FAIL (fdo#105454, fdo#106509) -> ( 2 FAIL ) (fdo#105454, fdo#106509)
igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
shard-glk: FAIL (fdo#105454) -> ( 1 FAIL, 1 PASS ) (fdo#105454, fdo#106509)
igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-ytiled:
shard-glk: FAIL (fdo#103184) -> ( 1 FAIL, 1 PASS ) (fdo#103184)
igt@kms_rotation_crc@primary-rotation-180:
shard-snb: FAIL (fdo#103925) -> ( 2 FAIL ) (fdo#103925)
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> ( 2 FAIL ) (fdo#99912)
shard-glk: FAIL (fdo#99912) -> ( 2 FAIL ) (fdo#99912)
shard-hsw: FAIL (fdo#99912) -> ( 2 FAIL ) (fdo#99912)
shard-snb: FAIL (fdo#99912) -> ( 2 FAIL ) (fdo#99912)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4684 -> Patchwork_9966
CI_DRM_4684: bb1a6d0044581c5d8867afde39111ea4605c644d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4604: 2a5777f8a694f1f8edcf021afb1ef36192c6762d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9966: df341afb28f5bdd2eaa3eba5a15dc854d08474b5 @ 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_9966/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Keep physical cursors pinned while in use
2018-08-17 8:24 [PATCH] drm/i915: Keep physical cursors pinned while in use Chris Wilson
` (2 preceding siblings ...)
2018-08-17 11:04 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-08-22 12:35 ` Ville Syrjälä
2018-08-31 7:57 ` Chris Wilson
3 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2018-08-22 12:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, Aug 17, 2018 at 09:24:05AM +0100, Chris Wilson wrote:
> The optimisation inherent in commit 6a2c4232ece1 ("drm/i915: Make the
> physical object coherent with GTT") relies on that once we allocated a
> cursor we would have coherent, zero overhead access to the scanout plane
> holding the cursor. That is we could then do the very frequent cursor
> updates X enjoys with no indirection or kernel involvement. However,
> that all hinges on the GGTT mmap of the cursor being pinned and not
> require refaulting on each access -- handling such a page fault likely
> requires the busy GGTT to be rearranged causing a stall. A very simple
> fix is then to handle the physical cursor exactly like other cursors and
> keep its vma pinned while active.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600
I guess this wasn't the thing we wanted. But seems quite harmless to
me anyway, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
in case you still want to land it.
> References: 6a2c4232ece1 ("drm/i915: Make the physical object coherent with GTT")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 592b847db88e..d47ec9fd4af4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12966,8 +12966,11 @@ static int intel_plane_pin_fb(struct intel_plane_state *plane_state)
> INTEL_INFO(dev_priv)->cursor_needs_physical) {
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> const int align = intel_cursor_alignment(dev_priv);
> + int err;
>
> - return i915_gem_object_attach_phys(obj, align);
> + err = i915_gem_object_attach_phys(obj, align);
> + if (err)
> + return err;
> }
>
> vma = intel_pin_and_fence_fb_obj(fb,
> --
> 2.18.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Keep physical cursors pinned while in use
2018-08-22 12:35 ` [PATCH] " Ville Syrjälä
@ 2018-08-31 7:57 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-08-31 7:57 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Quoting Ville Syrjälä (2018-08-22 13:35:52)
> On Fri, Aug 17, 2018 at 09:24:05AM +0100, Chris Wilson wrote:
> > The optimisation inherent in commit 6a2c4232ece1 ("drm/i915: Make the
> > physical object coherent with GTT") relies on that once we allocated a
> > cursor we would have coherent, zero overhead access to the scanout plane
> > holding the cursor. That is we could then do the very frequent cursor
> > updates X enjoys with no indirection or kernel involvement. However,
> > that all hinges on the GGTT mmap of the cursor being pinned and not
> > require refaulting on each access -- handling such a page fault likely
> > requires the busy GGTT to be rearranged causing a stall. A very simple
> > fix is then to handle the physical cursor exactly like other cursors and
> > keep its vma pinned while active.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107600
>
> I guess this wasn't the thing we wanted. But seems quite harmless to
> me anyway, so
It ties neatly in with the ggtt map being used for updates, so I think
it's harmless enough. If I could just get a contiguous page out of
shmemfs, I could follow up with the removal of phys_object. :|
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> in case you still want to land it.
But without the bugzilla since that bug is occurring without cursor
updates, so is even more bizarre.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-31 7:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-17 8:24 [PATCH] drm/i915: Keep physical cursors pinned while in use Chris Wilson
2018-08-17 8:29 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-08-17 8:47 ` ✓ Fi.CI.BAT: success " Patchwork
2018-08-17 11:04 ` ✓ Fi.CI.IGT: " Patchwork
2018-08-22 12:35 ` [PATCH] " Ville Syrjälä
2018-08-31 7:57 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).