<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Sep 5, 2014 at 11:28 AM, Paulo Zanoni <span dir="ltr"><<a href="mailto:przanoni@gmail.com" target="_blank">przanoni@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">2014-08-04 7:51 GMT-03:00 Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>>:<br>
<span class="">> According to spec FBC on BDW and HSW are identical without any gaps.<br>
> So let's copy the nuke and let FBC really start compressing stuff.<br>
><br>
> Without this patch we can verify with false color that nothing is being<br>
> compressed. With the nuke in place and false color it is possible<br>
> to see false color debugs.<br>
><br>
> Unfortunatelly on some rings like BCS on BDW we have to avoid Bits 22:18 on<br>
> LRIs due to a high risk of hung. So, when using Blt ring for frontbuffer rend<br>
> cache would never been cleaned and FBC would stop compressing buffer.<br>
> One alternative is to cache clean on software frontbuffer tracking.<br>
><br>
> v2: Fix rebase conflict.<br>
> v3: Do not clean cache on BCS ring. Instead use sw frontbuffer tracking.<br>
<br>
</span>This patch causes lots of WARNs when running igt/pm_rpm/cursor and a<br>
few other similar test cases. Now the interesting this is that we're<br>
trying to do FBC flushing, but:<br></blockquote><div><br></div><div>As I told you on the IRC already, thank you for finding this. It is good to get reasons and arguments.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
- FBC is disabled by default</blockquote><div><br></div><div>Yeah, we need to fix it although it just matter much </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
- The screen is disabled</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
- We're runtime suspended<br></blockquote><div><br></div><div>These are another error. Not caused by this patch.</div><div>Why frontbuffer_flush is being called with screen disabled or on runtime suspend?</div><div> </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
- We're touching cursors when we decide to flush FBC.<br></blockquote><div><br></div><div>This is not actually a flush. But cleaning the cache to allow compression restart.</div><div>I believe we could have some variable to indicate when we touched ring flush than on next frontbuffer we do the mmio cache clean.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
This is the first WARN that happens:<br>
<br>
[ 123.581179] [drm:intel_runtime_suspend] Device suspended<br>
[ 123.680666] ------------[ cut here ]------------<br>
[ 123.680697] WARNING: CPU: 1 PID: 1776 at<br>
drivers/gpu/drm/i915/intel_uncore.c:47<br>
assert_device_not_suspended.isra.8+0x43/0x50 [i915]()<br>
[ 123.680699] Device suspended<br>
[ 123.680700] Modules linked in: intel_rapl x86_pkg_temp_thermal<br>
serio_raw intel_powerclamp efivars btusb iwlmvm iwlwifi mei_me mei<br>
int3403_thermal snd_hda_codec_hdmi snd_hda_intel snd_hda_controller<br>
i2c_designware_platform snd_hda_codec i2c_designware_core snd_hwdep<br>
snd_pcm_oss snd_mixer_oss acpi_pad snd_pcm snd_timer fuse nls_utf8<br>
nls_cp437 vfat fat sd_mod ahci libahci i915 sdhci_pci e1000e<br>
drm_kms_helper drm sdhci_acpi sdhci<br>
[ 123.680733] CPU: 1 PID: 1776 Comm: pm_rpm Not tainted<br>
3.17.0-rc2.1409051503pz+ #1100<br>
[ 123.680736] Hardware name: Intel Corporation Broadwell Client<br>
platform/Wilson Beach SDS, BIOS BDW-E2R1.86C.0072.R03.1405072127<br>
05/07/2014<br>
[ 123.680738] 0000000000000009 ffff88024174fab8 ffffffff816f6d23<br>
ffff88024174fb00<br>
[ 123.680742] ffff88024174faf0 ffffffff8107b368 ffff880037700000<br>
0000000000050380<br>
[ 123.680746] 0000000000050380 ffff880037700068 0000000000000001<br>
ffff88024174fb50<br>
[ 123.680751] Call Trace:<br>
[ 123.680758] [<ffffffff816f6d23>] dump_stack+0x4d/0x66<br>
[ 123.680763] [<ffffffff8107b368>] warn_slowpath_common+0x78/0xa0<br>
[ 123.680766] [<ffffffff8107b3d7>] warn_slowpath_fmt+0x47/0x50<br>
[ 123.680772] [<ffffffff810c0efd>] ? trace_hardirqs_on_caller+0x15d/0x200<br>
[ 123.680791] [<ffffffffa0118c63>]<br>
assert_device_not_suspended.isra.8+0x43/0x50 [i915]<br>
[ 123.680809] [<ffffffffa011cbd5>] gen8_write32+0x35/0x180 [i915]<br>
[ 123.680821] [<ffffffffa00dbb49>] gen8_fbc_sw_flush+0x29/0x30 [i915]<br>
[ 123.680840] [<ffffffffa013150d>] intel_frontbuffer_flush+0x7d/0x90 [i915]<br>
[ 123.680859] [<ffffffffa0135f6b>]<br>
intel_cursor_plane_update+0x13b/0x150 [i915]<br>
[ 123.680876] [<ffffffffa00276b0>] setplane_internal+0x260/0x2a0 [drm]<br>
[ 123.680889] [<ffffffffa002780e>] drm_mode_cursor_common+0x11e/0x310 [drm]<br>
[ 123.680904] [<ffffffffa002ae1c>] drm_mode_cursor_ioctl+0x3c/0x40 [drm]<br>
[ 123.680914] [<ffffffffa001cc9f>] drm_ioctl+0x1df/0x6a0 [drm]<br>
[ 123.680920] [<ffffffff816fe769>] ? mutex_unlock+0x9/0x10<br>
[ 123.680924] [<ffffffff811f3856>] ? seq_read+0xb6/0x3e0<br>
[ 123.680929] [<ffffffff811e2540>] do_vfs_ioctl+0x2e0/0x4e0<br>
[ 123.680934] [<ffffffff81701077>] ? sysret_check+0x1b/0x56<br>
[ 123.680939] [<ffffffff810c0efd>] ? trace_hardirqs_on_caller+0x15d/0x200<br>
[ 123.680943] [<ffffffff811e27c1>] SyS_ioctl+0x81/0xa0<br>
[ 123.680947] [<ffffffff81701052>] system_call_fastpath+0x16/0x1b<br>
[ 123.680949] ---[ end trace 9f389682639d2eb9 ]---<br>
[ 123.680953] ------------[ cut here ]------------<br>
<br>
<br>
So, what we can/could do:<br>
1 - Change gen8_fbc_sw_flush() do to nothing when FBC is disabled.<br></blockquote><div><br></div><div>Agreed!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
2 - At some point of the stack above, avoid calling everything else if<br>
the screen and/or plane and/or crtc is disabled. But when? At<br>
intel_cursor_plane_update? At intel_frontbuffer_flush?<br></blockquote><div><br></div><div>Good question!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
3 - Don't call gen8_fbc_sw_flush if the frontbuffer_bits don't include<br>
the primary plane bits (since, AFAIR, FBC ignores the sprite and<br>
cursors, right?)<br></blockquote><div><br></div><div>not sure. but we could try check primary bits + do only when we did a ring flush on blt ring. Actually other ring than render.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Is anybody against any of the 3 items above?<br></blockquote><div><br></div><div>should I change this patch itself or provide another one on top?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Also notice that the "cursor" subtest is not the only one that causes<br>
these WARNs, so I expect similar backtraces with other similar<br>
solutions for equivalent problems with the other planes.<br></blockquote><div><br></div><div>so check for primary bit is the way!</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Thanks,<br>
Paulo<br></blockquote><div><br></div><div>Thank you!</div><div>Rodrigo.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><br>
><br>
> Signed-off-by: Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>><br>
> ---<br>
> drivers/gpu/drm/i915/i915_drv.h | 1 +<br>
> drivers/gpu/drm/i915/intel_display.c | 3 +++<br>
> drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++<br>
> drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +++++++++-<br>
> 4 files changed, 23 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h<br>
> index 2a372f2..25d7365 100644<br>
> --- a/drivers/gpu/drm/i915/i915_drv.h<br>
> +++ b/drivers/gpu/drm/i915/i915_drv.h<br>
> @@ -2713,6 +2713,7 @@ extern void intel_modeset_setup_hw_state(struct drm_device *dev,<br>
> extern void i915_redisable_vga(struct drm_device *dev);<br>
> extern void i915_redisable_vga_power_on(struct drm_device *dev);<br>
> extern bool intel_fbc_enabled(struct drm_device *dev);<br>
> +extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value);<br>
> extern void intel_disable_fbc(struct drm_device *dev);<br>
> extern bool ironlake_set_drps(struct drm_device *dev, u8 val);<br>
> extern void intel_init_pch_refclk(struct drm_device *dev);<br>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c<br>
> index 883af0b..c8421cd 100644<br>
> --- a/drivers/gpu/drm/i915/intel_display.c<br>
> +++ b/drivers/gpu/drm/i915/intel_display.c<br>
> @@ -9044,6 +9044,9 @@ void intel_frontbuffer_flush(struct drm_device *dev,<br>
> intel_mark_fb_busy(dev, frontbuffer_bits, NULL);<br>
><br>
> intel_edp_psr_flush(dev, frontbuffer_bits);<br>
> +<br>
> + if (IS_GEN8(dev))<br>
> + gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);<br>
> }<br>
><br>
> /**<br>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c<br>
> index 684dc5f..de07d3e 100644<br>
> --- a/drivers/gpu/drm/i915/intel_pm.c<br>
> +++ b/drivers/gpu/drm/i915/intel_pm.c<br>
> @@ -345,6 +345,16 @@ bool intel_fbc_enabled(struct drm_device *dev)<br>
> return dev_priv->display.fbc_enabled(dev);<br>
> }<br>
><br>
> +void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)<br>
> +{<br>
> + struct drm_i915_private *dev_priv = dev->dev_private;<br>
> +<br>
> + if (!IS_GEN8(dev))<br>
> + return;<br>
> +<br>
> + I915_WRITE(MSG_FBC_REND_STATE, value);<br>
> +}<br>
> +<br>
> static void intel_fbc_work_fn(struct work_struct *__work)<br>
> {<br>
> struct intel_fbc_work *work =<br>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c<br>
> index 2908896..2fe871c 100644<br>
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c<br>
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c<br>
> @@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,<br>
> {<br>
> u32 flags = 0;<br>
> u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;<br>
> + int ret;<br>
><br>
> flags |= PIPE_CONTROL_CS_STALL;<br>
><br>
> @@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,<br>
> flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;<br>
> }<br>
><br>
> - return gen8_emit_pipe_control(ring, flags, scratch_addr);<br>
> + ret = gen8_emit_pipe_control(ring, flags, scratch_addr);<br>
> + if (ret)<br>
> + return ret;<br>
> +<br>
> + if (!invalidate_domains && flush_domains)<br>
> + return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);<br>
> +<br>
> + return 0;<br>
> }<br>
><br>
> static void ring_write_tail(struct intel_engine_cs *ring,<br>
> --<br>
> 1.9.3<br>
><br>
> _______________________________________________<br>
> Intel-gfx mailing list<br>
> <a href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx" target="_blank">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br>
<br>
<br>
<br>
</div></div><span class=""><font color="#888888">--<br>
Paulo Zanoni<br>
</font></span><div class=""><div class="h5">_______________________________________________<br>
Intel-gfx mailing list<br>
<a href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx" target="_blank">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Rodrigo Vivi</div><div>Blog: <a href="http://blog.vivi.eng.br" target="_blank">http://blog.vivi.eng.br</a></div><div> </div>
</div></div>