drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
Subsystems (2)
PATCH 1/1 drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
Commit b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.") removed the call to wait_for_vblanks and replaced it with flip_done.
Unfortunately legacy_cursor_update was unset too late, and the replacement call drm_atomic_helper_wait_for_flip_done() was a noop. Make sure that its unset before setup_commit() is called to fix this issue.
Changes since v1:
- Force vblank wait for watermarks not yet converted to atomic too. (Ville)
- Use for_each_new_intel_crtc_in_state. (Ville)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
Testcase: kms_cursor_crc
--- drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-)
drivers/gpu/drm/i915/intel_display.c +26 -19
@@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,struct drm_i915_private *dev_priv =to_i915 (dev);int ret =0 ; - ret =drm_atomic_helper_setup_commit (state, nonblock); -if (ret) -return ret; -drm_atomic_state_get (state);i915_sw_fence_init (&intel_state->commit_ready, intel_atomic_commit_ready); - ret =intel_atomic_prepare_commit (dev, state); -if (ret) { -DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); -i915_sw_fence_commit (&intel_state->commit_ready); -return ret; - } -/* * The intel_legacy_cursor_update() fast path takes care * of avoiding the vblank waits for simple cursor
@@ -12540,19 +12529,37 @@ static int intel_atomic_commit(struct drm_device *dev,* updates happen during the correct frames. Gen9+ have * double buffered watermarks and so shouldn't need this. * -* Do this after drm_atomic_helper_setup_commit() and -* intel_atomic_prepare_commit() because we still want -* to skip the flip and fb cleanup waits. Although that -* does risk yanking the mapping from under the display -* engine. +* Unset state->legacy_cursor_update before the call to +* drm_atomic_helper_setup_commit() because otherwise +* drm_atomic_helper_wait_for_flip_done() is a noop and +* we get FIFO underruns because we didn't wait +* for vblank. * * FIXME doing watermarks and fb cleanup from a vblank worker * (assuming we had any) would solve these problems. */ -if (INTEL_GEN (dev_priv) <9 ) - state->legacy_cursor_update = false; +if (INTEL_GEN (dev_priv) <9 && state->legacy_cursor_update) { +struct intel_crtc_state *new_crtc_state; +struct intel_crtc *crtc; +int i; + +for_each_new_intel_crtc_in_state (intel_state, crtc, new_crtc_state, i) +if (new_crtc_state->wm.need_postvbl_update || + new_crtc_state->update_wm_post) + state->legacy_cursor_update = false; + } + + ret =intel_atomic_prepare_commit (dev, state); +if (ret) { +DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); +i915_sw_fence_commit (&intel_state->commit_ready); +return ret; + } + + ret =drm_atomic_helper_setup_commit (state, nonblock); +if (!ret) + ret =drm_atomic_helper_swap_state (state, true); - ret =drm_atomic_helper_swap_state (state, true);if (ret) {i915_sw_fence_commit (&intel_state->commit_ready);
Signature · 6 lines
2.14.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.BAT: failure for drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
== Series Details ==
== Series Details ==
Series: drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
URL : https://patchwork.freedesktop.org/series/30503/
State : failure
== Summary ==
Series 30503v1 drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
https://patchwork.freedesktop.org/api/1.0/series/30503/revisions/1/mbox/
Test gem_exec_reloc:
Subgroup basic-cpu-gtt-noreloc:
pass -> DMESG-WARN (fi-kbl-7500u)
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> INCOMPLETE (fi-kbl-7500u)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
fail -> PASS (fi-snb-2600) fdo#100215
Test pm_rpm:
Subgroup basic-rte:
dmesg-warn -> PASS (fi-cfl-s) fdo#102294fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:448s fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:469s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:418s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:514s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:277s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:506s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:496s fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:488s fi-cfl-s total:286 pass:223 dwarn:31 dfail:0 fail:0 skip:31 fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:412s fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:604s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:426s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:410s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:429s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:485s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:467s fi-kbl-7500u total:118 pass:99 dwarn:2 dfail:0 fail:0 skip:16 fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:580s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:599s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:544s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:452s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:749s fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:496s fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:474s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:564s fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:417s
5299e24e48f3c24e612bcdb997d0dc477cdde0d0 drm-tip: 2017y-09m-18d-08h-44m-15s UTC integration manifest 52dcc46ca752 drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5726/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✓ Fi.CI.BAT: success for drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
In reply to Maarten Lankhorst== Series Details ==
== Series Details ==
Series: drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
URL : https://patchwork.freedesktop.org/series/30503/
State : success
== Summary ==
Series 30503v1 drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
https://patchwork.freedesktop.org/api/1.0/series/30503/revisions/1/mbox/
Test gem_exec_reloc:
Subgroup basic-cpu-read:
dmesg-warn -> PASS (fi-kbl-7500u)
Subgroup basic-gtt-read:
dmesg-warn -> PASS (fi-kbl-7500u)
Test gem_exec_store:
Subgroup basic-all:
dmesg-warn -> PASS (fi-kbl-7500u)
Test gem_exec_suspend:
Subgroup basic-s3:
incomplete -> PASS (fi-kbl-7500u)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass -> FAIL (fi-snb-2600) fdo#100215 +1
Test pm_rpm:
Subgroup basic-rte:
dmesg-warn -> PASS (fi-cfl-s) fdo#102294fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:449s fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:467s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:422s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:517s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:278s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:521s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:491s fi-byt-n2820 total:289 pass:250 dwarn:1 dfail:0 fail:0 skip:38 time:501s fi-cfl-s total:289 pass:223 dwarn:34 dfail:0 fail:0 skip:32 time:541s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:417s fi-glk-2a total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:598s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:428s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:408s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:427s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:491s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:466s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:474s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:588s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:593s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:543s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:455s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:746s fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:494s fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:472s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:563s fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:416s
0bbe2f0d6a1e69d0ee72ea0c1dcdd93269419024 drm-tip: 2017y-09m-18d-13h-52m-45s UTC integration manifest 45883bf694b6 drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5731/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 71 lines · Maarten Lankhorst
Commit
b44d5c0c105a("drm/i915: Always wait forflip_done, v2.") removed the call towait_for_vblanksand replaced it withflip_done.Unfortunately
legacy_cursor_updatewas unset too late, and the replacement calldrm_atomic_helper_wait_for_flip_done()was a noop. Make sure that its unset beforesetup_commit()is called to fix this issue.Changes since v1:
- Force vblank wait for watermarks not yet converted to atomic too. (Ville)
- Usefor_each_new_intel_crtc_in_state. (Ville)Signed-off-by Maarten Lankhorst <[email protected]>Fixes b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
Testcase:kms_cursor_crcCc Daniel Vetter <[email protected]>Cc Jani Nikula <[email protected]>Reported-by Marta Löfstedt <[email protected]>Cc Marta Löfstedt <[email protected]>Tested-by Marta Löfstedt <[email protected]>Cc Ville Syrjälä <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-)drivers/gpu/drm/i915/intel_display.c +5
-17@@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,struct drm_i915_private *dev_priv =to_i915 (dev);int ret =0 ; - ret =drm_atomic_helper_setup_commit (state, nonblock); -if (ret) -return ret; -drm_atomic_state_get (state);i915_sw_fence_init (&intel_state->commit_ready, intel_atomic_commit_ready); - ret =intel_atomic_prepare_commit (dev, state); -if (ret) { -DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); -i915_sw_fence_commit (&intel_state->commit_ready); -return ret; - } -/* * The intel_legacy_cursor_update() fast path takes care * of avoiding the vblank waits for simple cursor @@ -12540,19 +12529,37 @@ static int intel_atomic_commit(struct drm_device *dev,* updates happen during the correct frames. Gen9+ have * double buffered watermarks and so shouldn't need this. * -* Do this after drm_atomic_helper_setup_commit() and -* intel_atomic_prepare_commit() because we still want -* to skip the flip and fb cleanup waits. Although that -* does risk yanking the mapping from under the display -* engine. +* Unset state->legacy_cursor_update before the call to +* drm_atomic_helper_setup_commit() because otherwise +* drm_atomic_helper_wait_for_flip_done() is a noop and +* we get FIFO underruns because we didn't wait +* for vblank. * * FIXME doing watermarks and fb cleanup from a vblank worker * (assuming we had any) would solve these problems. */ -if (INTEL_GEN (dev_priv) <9 )
drivers/gpu/drm/i915/intel_display.c static int intel_atomic_commit(struct drm_device *dev, · line 12549
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +9
-1- state->legacy_cursor_update = false; +if (INTEL_GEN (dev_priv) <9 && state->legacy_cursor_update) { +struct intel_crtc_state *new_crtc_state; +struct intel_crtc *crtc; +int i; + +for_each_new_intel_crtc_in_state (intel_state, crtc, new_crtc_state, i) +if (new_crtc_state->wm.need_postvbl_update || + new_crtc_state->update_wm_post) + state->legacy_cursor_update = false;
Hmm. I guess that's better. But I still don't see why you want to change this bit of code in this patch. AFAICS it's got nothing to do with the fix itself, and instead it's just trying to optimize some cursor updates that were kicked over to the slow path. Or am I missing something?
Quoted 19 lines
drivers/gpu/drm/i915/intel_display.c +12
-1+ } + + ret =intel_atomic_prepare_commit (dev, state); +if (ret) { +DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); +i915_sw_fence_commit (&intel_state->commit_ready); +return ret; + } + + ret =drm_atomic_helper_setup_commit (state, nonblock); +if (!ret) + ret =drm_atomic_helper_swap_state (state, true); - ret =drm_atomic_helper_swap_state (state, true);if (ret) {i915_sw_fence_commit (&intel_state->commit_ready);Signature · 1 line
2.14.1
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 18-09-17 om 17:03 schreef Ville Syrjälä:
Quoted 78 lines · Maarten Lankhorst
Quoted 77 lines
Commit
b44d5c0c105a("drm/i915: Always wait forflip_done, v2.") removed the call towait_for_vblanksand replaced it withflip_done.Unfortunately
legacy_cursor_updatewas unset too late, and the replacement calldrm_atomic_helper_wait_for_flip_done()was a noop. Make sure that its unset beforesetup_commit()is called to fix this issue.Changes since v1:
- Force vblank wait for watermarks not yet converted to atomic too. (Ville)
- Usefor_each_new_intel_crtc_in_state. (Ville)Signed-off-by Maarten Lankhorst <[email protected]>Fixes b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
Testcase:kms_cursor_crcCc Daniel Vetter <[email protected]>Cc Jani Nikula <[email protected]>Reported-by Marta Löfstedt <[email protected]>Cc Marta Löfstedt <[email protected]>Tested-by Marta Löfstedt <[email protected]>Cc Ville Syrjälä <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-)drivers/gpu/drm/i915/intel_display.c +10
-18@@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,struct drm_i915_private *dev_priv =to_i915 (dev);int ret =0 ; - ret =drm_atomic_helper_setup_commit (state, nonblock); -if (ret) -return ret; -drm_atomic_state_get (state);i915_sw_fence_init (&intel_state->commit_ready, intel_atomic_commit_ready); - ret =intel_atomic_prepare_commit (dev, state); -if (ret) { -DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); -i915_sw_fence_commit (&intel_state->commit_ready); -return ret; - } -/* * The intel_legacy_cursor_update() fast path takes care * of avoiding the vblank waits for simple cursor @@ -12540,19 +12529,37 @@ static int intel_atomic_commit(struct drm_device *dev,* updates happen during the correct frames. Gen9+ have * double buffered watermarks and so shouldn't need this. * -* Do this after drm_atomic_helper_setup_commit() and -* intel_atomic_prepare_commit() because we still want -* to skip the flip and fb cleanup waits. Although that -* does risk yanking the mapping from under the display -* engine. +* Unset state->legacy_cursor_update before the call to +* drm_atomic_helper_setup_commit() because otherwise +* drm_atomic_helper_wait_for_flip_done() is a noop and +* we get FIFO underruns because we didn't wait +* for vblank. * * FIXME doing watermarks and fb cleanup from a vblank worker * (assuming we had any) would solve these problems. */ -if (INTEL_GEN (dev_priv) <9 ) - state->legacy_cursor_update = false; +if (INTEL_GEN (dev_priv) <9 && state->legacy_cursor_update) { +struct intel_crtc_state *new_crtc_state; +struct intel_crtc *crtc; +int i; +
Quoted 8 lines
drivers/gpu/drm/i915/intel_display.c static int intel_atomic_commit(struct drm_device *dev, · line 12549
Quoted 4 lines
drivers/gpu/drm/i915/intel_display.c +4
-0+for_each_new_intel_crtc_in_state (intel_state, crtc, new_crtc_state, i) +if (new_crtc_state->wm.need_postvbl_update || + new_crtc_state->update_wm_post) + state->legacy_cursor_update = false;Hmm. I guess that's better. But I still don't see why you want to change this bit of code in this patch. AFAICS it's got nothing to do with the fix itself, and instead it's just trying to optimize some cursor updates that were kicked over to the slow path. Or am I missing something?
We accidentally removed the vblank wait for the slowpath, but I don't think we should reintroduce the vblank except where we need it..
~Maarten
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 88 lines · Maarten Lankhorst
Op 18-09-17 om 17:03 schreef Ville Syrjälä:
Quoted 78 lines · Maarten Lankhorst
Quoted 77 lines
Commit
b44d5c0c105a("drm/i915: Always wait forflip_done, v2.") removed the call towait_for_vblanksand replaced it withflip_done.Unfortunately
legacy_cursor_updatewas unset too late, and the replacement calldrm_atomic_helper_wait_for_flip_done()was a noop. Make sure that its unset beforesetup_commit()is called to fix this issue.Changes since v1:
- Force vblank wait for watermarks not yet converted to atomic too. (Ville)
- Usefor_each_new_intel_crtc_in_state. (Ville)Signed-off-by Maarten Lankhorst <[email protected]>Fixes b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
Testcase:kms_cursor_crcCc Daniel Vetter <[email protected]>Cc Jani Nikula <[email protected]>Reported-by Marta Löfstedt <[email protected]>Cc Marta Löfstedt <[email protected]>Tested-by Marta Löfstedt <[email protected]>Cc Ville Syrjälä <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-)drivers/gpu/drm/i915/intel_display.c +10
-18@@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,struct drm_i915_private *dev_priv =to_i915 (dev);int ret =0 ; - ret =drm_atomic_helper_setup_commit (state, nonblock); -if (ret) -return ret; -drm_atomic_state_get (state);i915_sw_fence_init (&intel_state->commit_ready, intel_atomic_commit_ready); - ret =intel_atomic_prepare_commit (dev, state); -if (ret) { -DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); -i915_sw_fence_commit (&intel_state->commit_ready); -return ret; - } -/* * The intel_legacy_cursor_update() fast path takes care * of avoiding the vblank waits for simple cursor @@ -12540,19 +12529,37 @@ static int intel_atomic_commit(struct drm_device *dev,* updates happen during the correct frames. Gen9+ have * double buffered watermarks and so shouldn't need this. * -* Do this after drm_atomic_helper_setup_commit() and -* intel_atomic_prepare_commit() because we still want -* to skip the flip and fb cleanup waits. Although that -* does risk yanking the mapping from under the display -* engine. +* Unset state->legacy_cursor_update before the call to +* drm_atomic_helper_setup_commit() because otherwise +* drm_atomic_helper_wait_for_flip_done() is a noop and +* we get FIFO underruns because we didn't wait +* for vblank. * * FIXME doing watermarks and fb cleanup from a vblank worker * (assuming we had any) would solve these problems. */ -if (INTEL_GEN (dev_priv) <9 ) - state->legacy_cursor_update = false; +if (INTEL_GEN (dev_priv) <9 && state->legacy_cursor_update) { +struct intel_crtc_state *new_crtc_state; +struct intel_crtc *crtc; +int i; +Quoted 8 lines
drivers/gpu/drm/i915/intel_display.c static int intel_atomic_commit(struct drm_device *dev, · line 12549
Quoted 4 lines
drivers/gpu/drm/i915/intel_display.c +4
-0+for_each_new_intel_crtc_in_state (intel_state, crtc, new_crtc_state, i) +if (new_crtc_state->wm.need_postvbl_update || + new_crtc_state->update_wm_post) + state->legacy_cursor_update = false;Hmm. I guess that's better. But I still don't see why you want to change this bit of code in this patch. AFAICS it's got nothing to do with the fix itself, and instead it's just trying to optimize some cursor updates that were kicked over to the slow path. Or am I missing something?
Quoted 1 line
We accidentally removed the vblank wait for the slowpath, but I don't think we should reintroduce the vblank except where we need it..
IMO any regression fix should ideally get us back exactly where we were.
Signature · 6 lines
Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 19-09-17 om 12:24 schreef Ville Syrjälä:
Quoted 88 lines · Maarten Lankhorst
Quoted 87 lines
Op 18-09-17 om 17:03 schreef Ville Syrjälä:
Quoted 86 lines · Maarten Lankhorst
Quoted 71 lines
Commit
b44d5c0c105a("drm/i915: Always wait forflip_done, v2.") removed the call towait_for_vblanksand replaced it withflip_done.Unfortunately
legacy_cursor_updatewas unset too late, and the replacement calldrm_atomic_helper_wait_for_flip_done()was a noop. Make sure that its unset beforesetup_commit()is called to fix this issue.Changes since v1:
- Force vblank wait for watermarks not yet converted to atomic too. (Ville)
- Usefor_each_new_intel_crtc_in_state. (Ville)Signed-off-by Maarten Lankhorst <[email protected]>Fixes b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
Testcase:kms_cursor_crcCc Daniel Vetter <[email protected]>Cc Jani Nikula <[email protected]>Reported-by Marta Löfstedt <[email protected]>Cc Marta Löfstedt <[email protected]>Tested-by Marta Löfstedt <[email protected]>Cc Ville Syrjälä <[email protected]>--- drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-)drivers/gpu/drm/i915/intel_display.c +5
-17@@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,struct drm_i915_private *dev_priv =to_i915 (dev);int ret =0 ; - ret =drm_atomic_helper_setup_commit (state, nonblock); -if (ret) -return ret; -drm_atomic_state_get (state);i915_sw_fence_init (&intel_state->commit_ready, intel_atomic_commit_ready); - ret =intel_atomic_prepare_commit (dev, state); -if (ret) { -DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); -i915_sw_fence_commit (&intel_state->commit_ready); -return ret; - } -/* * The intel_legacy_cursor_update() fast path takes care * of avoiding the vblank waits for simple cursor @@ -12540,19 +12529,37 @@ static int intel_atomic_commit(struct drm_device *dev,* updates happen during the correct frames. Gen9+ have * double buffered watermarks and so shouldn't need this. * -* Do this after drm_atomic_helper_setup_commit() and -* intel_atomic_prepare_commit() because we still want -* to skip the flip and fb cleanup waits. Although that -* does risk yanking the mapping from under the display -* engine. +* Unset state->legacy_cursor_update before the call to +* drm_atomic_helper_setup_commit() because otherwise +* drm_atomic_helper_wait_for_flip_done() is a noop and +* we get FIFO underruns because we didn't wait +* for vblank. * * FIXME doing watermarks and fb cleanup from a vblank worker * (assuming we had any) would solve these problems. */ -if (INTEL_GEN (dev_priv) <9 )drivers/gpu/drm/i915/intel_display.c static int intel_atomic_commit(struct drm_device *dev, · line 12549
Quoted 10 lines
drivers/gpu/drm/i915/intel_display.c +9
-1- state->legacy_cursor_update = false; +if (INTEL_GEN (dev_priv) <9 && state->legacy_cursor_update) { +struct intel_crtc_state *new_crtc_state; +struct intel_crtc *crtc; +int i; + +for_each_new_intel_crtc_in_state (intel_state, crtc, new_crtc_state, i) +if (new_crtc_state->wm.need_postvbl_update || + new_crtc_state->update_wm_post) + state->legacy_cursor_update = false;Hmm. I guess that's better. But I still don't see why you want to change this bit of code in this patch. AFAICS it's got nothing to do with the fix itself, and instead it's just trying to optimize some cursor updates that were kicked over to the slow path. Or am I missing something?
Quoted 3 lines
Quoted 1 line
We accidentally removed the vblank wait for the slowpath, but I don't think we should reintroduce the vblank except where we need it..
IMO any regression fix should ideally get us back exactly where we were.
Ok I'll send it out as separate patch then..
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
kbuild test robot
build botreproduce:
Hi Maarten,
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.14-rc1 next-20170915]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-i915-Unset-legacy_cursor_update-early-in-intel_atomic_commit-v2/20170918-223150 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-x003-201738 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/intel_display.c: In function 'intel_atomic_commit':
>> drivers/gpu/drm/i915/intel_display.c:12608:3: error: implicit declaration of function 'for_each_new_intel_crtc_in_state' [-Werror=implicit-function-declaration]
for_each_new_intel_crtc_in_state(intel_state, crtc, new_crtc_state, i)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/intel_display.c:12609:4: error: expected ';' before 'if'
if (new_crtc_state->wm.need_postvbl_update ||
^~cc1: some warnings being treated as errors
vim +/for_each_new_intel_crtc_in_state +12608 drivers/gpu/drm/i915/intel_display.c
12561
12562 /**
12563 * intel_atomic_commit - commit validated state object
12564 * @dev: DRM device
12565 * @state: the top-level driver state object
12566 * @nonblock: nonblocking commit
12567 *
12568 * This function commits a top-level state object that has been validated
12569 * with drm_atomic_helper_check().
12570 *
12571 * RETURNS
12572 * Zero for success or -errno.
12573 */
12574 static int intel_atomic_commit(struct drm_device *dev,
12575 struct drm_atomic_state *state,
12576 bool nonblock)
12577 {
12578 struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
12579 struct drm_i915_private *dev_priv = to_i915(dev);
12580 int ret = 0;
12581
12582 drm_atomic_state_get(state);
12583 i915_sw_fence_init(&intel_state->commit_ready,
12584 intel_atomic_commit_ready);
12585
12586 /*
12587 * The intel_legacy_cursor_update() fast path takes care
12588 * of avoiding the vblank waits for simple cursor
12589 * movement and flips. For cursor on/off and size changes,
12590 * we want to perform the vblank waits so that watermark
12591 * updates happen during the correct frames. Gen9+ have
12592 * double buffered watermarks and so shouldn't need this.
12593 *
12594 * Unset state->legacy_cursor_update before the call to
12595 * drm_atomic_helper_setup_commit() because otherwise
12596 * drm_atomic_helper_wait_for_flip_done() is a noop and
12597 * we get FIFO underruns because we didn't wait
12598 * for vblank.
12599 *
12600 * FIXME doing watermarks and fb cleanup from a vblank worker
12601 * (assuming we had any) would solve these problems.
12602 */
12603 if (INTEL_GEN(dev_priv) < 9 && state->legacy_cursor_update) {
12604 struct intel_crtc_state *new_crtc_state;
12605 struct intel_crtc *crtc;
12606 int i;
12607
12608 for_each_new_intel_crtc_in_state(intel_state, crtc, new_crtc_state, i)
12609 if (new_crtc_state->wm.need_postvbl_update ||
12610 new_crtc_state->update_wm_post)
12611 state->legacy_cursor_update = false;
12612 }
12613
12614 ret = intel_atomic_prepare_commit(dev, state);
12615 if (ret) {
12616 DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
12617 i915_sw_fence_commit(&intel_state->commit_ready);
12618 return ret;
12619 }
12620
12621 ret = drm_atomic_helper_setup_commit(state, nonblock);
12622 if (!ret)
12623 ret = drm_atomic_helper_swap_state(state, true);
12624
12625 if (ret) {
12626 i915_sw_fence_commit(&intel_state->commit_ready);
12627
12628 drm_atomic_helper_cleanup_planes(dev, state);
12629 return ret;
12630 }
12631 dev_priv->wm.distrust_bios_wm = false;
12632 intel_shared_dpll_swap_state(state);
12633 intel_atomic_track_fbs(state);
12634
12635 if (intel_state->modeset) {
12636 memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
12637 sizeof(intel_state->min_pixclk));
12638 dev_priv->active_crtcs = intel_state->active_crtcs;
12639 dev_priv->cdclk.logical = intel_state->cdclk.logical;
12640 dev_priv->cdclk.actual = intel_state->cdclk.actual;
12641 }
12642
12643 drm_atomic_state_get(state);
12644 INIT_WORK(&state->commit_work, intel_atomic_commit_work);
12645
12646 i915_sw_fence_commit(&intel_state->commit_ready);
12647 if (nonblock)
12648 queue_work(system_unbound_wq, &state->commit_work);
12649 else
12650 intel_atomic_commit_tail(state);
12651
12652
12653 return 0;
12654 }
12655
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
kbuild test robot
build botreproduce:
Hi Maarten,
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.14-rc1 next-20170918]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-i915-Unset-legacy_cursor_update-early-in-intel_atomic_commit-v2/20170918-223150 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-x012-201738 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/intel_display.c: In function 'intel_atomic_commit':
drivers/gpu/drm/i915/intel_display.c:12608:3: error: implicit declaration of function 'for_each_new_intel_crtc_in_state' [-Werror=implicit-function-declaration]
for_each_new_intel_crtc_in_state(intel_state, crtc, new_crtc_state, i)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/dmi.h:4,
from drivers/gpu/drm/i915/intel_display.c:27:
include/linux/compiler.h:156:2: error: expected ';' before 'if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
>> drivers/gpu/drm/i915/intel_display.c:12609:4: note: in expansion of macro 'if'
if (new_crtc_state->wm.need_postvbl_update ||
^~
drivers/gpu/drm/i915/intel_display.c: At top level:
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcpy' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:390:2: note: in expansion of macro 'if'
if (p_size == (size_t)-1 && q_size == (size_t)-1)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:380:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:378:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:369:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:367:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:358:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:356:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:348:2: note: in expansion of macro 'if'
if (p_size < size || q_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:345:3: note: in expansion of macro 'if'
if (q_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:343:3: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~ include/linux/string.h:342:2: note: in expansion of macro 'if'
vim +/if +12609 drivers/gpu/drm/i915/intel_display.c
12561
12562 /**
12563 * intel_atomic_commit - commit validated state object
12564 * @dev: DRM device
12565 * @state: the top-level driver state object
12566 * @nonblock: nonblocking commit
12567 *
12568 * This function commits a top-level state object that has been validated
12569 * with drm_atomic_helper_check().
12570 *
12571 * RETURNS
12572 * Zero for success or -errno.
12573 */
12574 static int intel_atomic_commit(struct drm_device *dev,
12575 struct drm_atomic_state *state,
12576 bool nonblock)
12577 {
12578 struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
12579 struct drm_i915_private *dev_priv = to_i915(dev);
12580 int ret = 0;
12581
12582 drm_atomic_state_get(state);
12583 i915_sw_fence_init(&intel_state->commit_ready,
12584 intel_atomic_commit_ready);
12585
12586 /*
12587 * The intel_legacy_cursor_update() fast path takes care
12588 * of avoiding the vblank waits for simple cursor
12589 * movement and flips. For cursor on/off and size changes,
12590 * we want to perform the vblank waits so that watermark
12591 * updates happen during the correct frames. Gen9+ have
12592 * double buffered watermarks and so shouldn't need this.
12593 *
12594 * Unset state->legacy_cursor_update before the call to
12595 * drm_atomic_helper_setup_commit() because otherwise
12596 * drm_atomic_helper_wait_for_flip_done() is a noop and
12597 * we get FIFO underruns because we didn't wait
12598 * for vblank.
12599 *
12600 * FIXME doing watermarks and fb cleanup from a vblank worker
12601 * (assuming we had any) would solve these problems.
12602 */
12603 if (INTEL_GEN(dev_priv) < 9 && state->legacy_cursor_update) {
12604 struct intel_crtc_state *new_crtc_state;
12605 struct intel_crtc *crtc;
12606 int i;
12607
12608 for_each_new_intel_crtc_in_state(intel_state, crtc, new_crtc_state, i)
12609 if (new_crtc_state->wm.need_postvbl_update ||
12610 new_crtc_state->update_wm_post)
12611 state->legacy_cursor_update = false;
12612 }
12613
12614 ret = intel_atomic_prepare_commit(dev, state);
12615 if (ret) {
12616 DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
12617 i915_sw_fence_commit(&intel_state->commit_ready);
12618 return ret;
12619 }
12620
12621 ret = drm_atomic_helper_setup_commit(state, nonblock);
12622 if (!ret)
12623 ret = drm_atomic_helper_swap_state(state, true);
12624
12625 if (ret) {
12626 i915_sw_fence_commit(&intel_state->commit_ready);
12627
12628 drm_atomic_helper_cleanup_planes(dev, state);
12629 return ret;
12630 }
12631 dev_priv->wm.distrust_bios_wm = false;
12632 intel_shared_dpll_swap_state(state);
12633 intel_atomic_track_fbs(state);
12634
12635 if (intel_state->modeset) {
12636 memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
12637 sizeof(intel_state->min_pixclk));
12638 dev_priv->active_crtcs = intel_state->active_crtcs;
12639 dev_priv->cdclk.logical = intel_state->cdclk.logical;
12640 dev_priv->cdclk.actual = intel_state->cdclk.actual;
12641 }
12642
12643 drm_atomic_state_get(state);
12644 INIT_WORK(&state->commit_work, intel_atomic_commit_work);
12645
12646 i915_sw_fence_commit(&intel_state->commit_ready);
12647 if (nonblock)
12648 queue_work(system_unbound_wq, &state->commit_work);
12649 else
12650 intel_atomic_commit_tail(state);
12651
12652
12653 return 0;
12654 }
12655
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.IGT: warning for drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
In reply to Maarten Lankhorst== Series Details ==
== Series Details ==
Series: drm/i915: Unset legacy_cursor_update early in intel_atomic_commit, v2.
URL : https://patchwork.freedesktop.org/series/30503/
State : warning
== Summary ==
Test perf:
Subgroup blocking:
pass -> FAIL (shard-hsw) fdo#102252
Test drv_module_reload:
Subgroup basic-reload-inject:
dmesg-warn -> PASS (shard-hsw) fdo#102707
Test kms_setmode:
Subgroup basic:
fail -> PASS (shard-hsw) fdo#99912
Test kms_flip:
Subgroup blt-wf_vblank-vs-modeset-interruptible:
pass -> DMESG-WARN (shard-hsw)
Test kms_plane:
Subgroup plane-panning-bottom-right-suspend-pipe-C-planes:
skip -> PASS (shard-hsw)
Test pm_rpm:
Subgroup gem-mmap-gtt:
pass -> SKIP (shard-hsw)
Test kms_atomic_interruptible:
Subgroup legacy-pageflip:
pass -> SKIP (shard-hsw)fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
shard-hsw total:2313 pass:1242 dwarn:1 dfail:0 fail:13 skip:1057 time:9693s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5731/shards.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 14 lines · Maarten Lankhorst
Commit
b44d5c0c105a("drm/i915: Always wait forflip_done, v2.") removed the call towait_for_vblanksand replaced it withflip_done.Unfortunately
legacy_cursor_updatewas unset too late, and the replacement calldrm_atomic_helper_wait_for_flip_done()was a noop. Make sure that its unset beforesetup_commit()is called to fix this issue.Changes since v1:
- Force vblank wait for watermarks not yet converted to atomic too. (Ville)
- Usefor_each_new_intel_crtc_in_state. (Ville)Signed-off-by Maarten Lankhorst <[email protected]>Fixes b44d5c0c105a ("drm/i915: Always wait for flip_done, v2.")
Quoted 8 lines
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102675
Testcase:kms_cursor_crcCc Daniel Vetter <[email protected]>Cc Jani Nikula <[email protected]>Reported-by Marta Löfstedt <[email protected]>Cc Marta Löfstedt <[email protected]>Tested-by Marta Löfstedt <[email protected]>Cc Ville Syrjälä <[email protected]>
I think the proper fix is switching over to the async plane helpers and unconditionally always clearing the legacy cursor hack. Maybe even outright removing it, since it doesn't really work all that well with most drivers.
-Daniel
Quoted 79 lines
--- drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-)drivers/gpu/drm/i915/intel_display.c +26
-19@@ -12517,21 +12517,10 @@ static int intel_atomic_commit(struct drm_device *dev,struct drm_i915_private *dev_priv =to_i915 (dev);int ret =0 ; - ret =drm_atomic_helper_setup_commit (state, nonblock); -if (ret) -return ret; -drm_atomic_state_get (state);i915_sw_fence_init (&intel_state->commit_ready, intel_atomic_commit_ready); - ret =intel_atomic_prepare_commit (dev, state); -if (ret) { -DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); -i915_sw_fence_commit (&intel_state->commit_ready); -return ret; - } -/* * The intel_legacy_cursor_update() fast path takes care * of avoiding the vblank waits for simple cursor @@ -12540,19 +12529,37 @@ static int intel_atomic_commit(struct drm_device *dev,* updates happen during the correct frames. Gen9+ have * double buffered watermarks and so shouldn't need this. * -* Do this after drm_atomic_helper_setup_commit() and -* intel_atomic_prepare_commit() because we still want -* to skip the flip and fb cleanup waits. Although that -* does risk yanking the mapping from under the display -* engine. +* Unset state->legacy_cursor_update before the call to +* drm_atomic_helper_setup_commit() because otherwise +* drm_atomic_helper_wait_for_flip_done() is a noop and +* we get FIFO underruns because we didn't wait +* for vblank. * * FIXME doing watermarks and fb cleanup from a vblank worker * (assuming we had any) would solve these problems. */ -if (INTEL_GEN (dev_priv) <9 ) - state->legacy_cursor_update = false; +if (INTEL_GEN (dev_priv) <9 && state->legacy_cursor_update) { +struct intel_crtc_state *new_crtc_state; +struct intel_crtc *crtc; +int i; + +for_each_new_intel_crtc_in_state (intel_state, crtc, new_crtc_state, i) +if (new_crtc_state->wm.need_postvbl_update || + new_crtc_state->update_wm_post) + state->legacy_cursor_update = false; + } + + ret =intel_atomic_prepare_commit (dev, state); +if (ret) { +DRM_DEBUG_ATOMIC ("Preparing state failed with %i\n" , ret); +i915_sw_fence_commit (&intel_state->commit_ready); +return ret; + } + + ret =drm_atomic_helper_setup_commit (state, nonblock); +if (!ret) + ret =drm_atomic_helper_swap_state (state, true); - ret =drm_atomic_helper_swap_state (state, true);if (ret) {i915_sw_fence_commit (&intel_state->commit_ready);Signature · 1 line
2.14.1
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx