-
Notifications
You must be signed in to change notification settings - Fork 131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG][JSL] SOF doesn't work after S3 #1719
Comments
@kv2019i does this ring a bell? |
@plbossart no, this looks like a new case. I wonder if graphics came back ok from S3. Only error in the log before the audio register sync error is this: @keqiaozhang Would be interesting to check when this started happening? Perhaps a recent upstream merge? |
@kv2019i We didn't do a regular test on JSL, because no JSL devices available in PRC now. We will get one next week. |
@keqiaozhang I believe this same bug as #1763 (now fixed in sof-dev) and is triggered by difference in BIOS settings for HDA link values versus hw defaults. This happen on JSL/TGL/ICL/GLK platforms and newer. |
@keqiaozhang is this bug still current? it's been going on for ~2 months now. |
@emilchudzik |
Issue exists on my JSL (HDA mode). Steps:
full dmesg: |
@emilchudzik Does HDMI work before S3 on this platform? The error: [ 714.317335] snd_hda_codec_hdmi ehdaudio0D2: Unable to sync register 0x2f8100. -5 .. is typically seen when the hardware is something our HDMI codec driver does not identify. We recently added one JSL variant more, but I wonder if there is still more, or there is an issue with your kernel config. To debug this further, can you also share me output of "cat /proc/asound/card0/codec#2" before S3 is attempted And also, please share the config file (.config) that was used to compile the kernel. |
@kv2019i I haved just check issue again with headphones connected to monitor. Logs: for building kernel I used: kconfig-sof-default.sh |
Thanks @emilchudzik ! The kernel config is ok, as is the proc asound dump. This looks very close to #1763 but the fix for that is included in the kernel tree you quoted (24d9781 ), so it is not helping. So it seem on audio side everything is as it should, but graphics side is not coming back alive after S3. Let's see if we could have other people reprouce this. It would help if you could additionally take drm traces. Before going to S3 and back, run following as root: echo 0x1f > /sys/module/drm/parameters/debug You should see plenty of drm logs in dmesg. There's so much traces, so you might overrun dmesg This log would help to confirm whether graphics driver is programming the link correctly after S3. @keqiaozhang @yongzhi1 Can you reproduce this on JSL? |
after enter command for extend log buffer to 1M and then I enter S3: then run aplay - which "freeze", doesn't work |
Thanks @emilchudzik ! First thing that pops from the logs that you are missing i915 firmware on the test device: ... you should fetch one from linux-firmware. Latest linux-firmware on Ubuntu 18.04LTS still doesn't have it, so you might need to manually fetch it from: The firmware update probably should be done first as the i915 logs don't seem quite right. I would be expecting to see traces like below when aplay is used and/or when system resumes from S3: [245682.136006] [drm:intel_power_well_enable [i915]] enabling power well 2 |
thanks @kv2019i for hints, below logs: I can see now: but issue still exists, aplay doesn't work after wake up from S3. |
Thanks @emilchudzik ! After a bit more debug, it starts to seem this is a GFX problem. A peek at /sys/kernel/debug/dri/0/i915_capabilities shows that the i915 platform is ELK, not ICL, so I think there are some i915-side driver workarounds that are not yet applied on this platform. Let me put together a patch to verify. |
@emilchudzik Ok, I have a potential fix in PR1881 . Could you give it a try? |
@plbossart |
@plbossart @keqiaozhang This is clearly a bug, no question about it. #1881 is a potential fix. |
Excellent, that's great news, thanks @emilchudzik ! I'll post this patch to i915 driver. Once reviews pass and patch is merged to i915, we can backport it to sof-dev as otherwise it would take a looong time to get the patch back. |
Patch sent to intel-gfx list: |
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Still in review, not yet merged: |
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
OK, now merged to upstream: |
…g/drm/drm-intel into drm-next UAPI Changes: - drm/i915/perf: introduce global sseu pinning Allow userspace to request at perf/OA open full SSEU configuration on the system to be able to benchmark 3D workloads, at the cost of not being able to run media workloads. (Lionel) Userspace changes: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4021 - drm/i915/perf: add new open param to configure polling of OA buffer Let application choose how often the OA buffer should be checked on the CPU side for data availability for choosig between CPU overhead and realtime nature of data. Userspace changes: https://patchwork.freedesktop.org/series/74655/ (i915 perf recorder is a tool to capture i915 perf data for viewing in GPUVis.) - drm/i915/perf: remove generated code Removal of the machine generated perf/OA test configurations from i915. Used by Mesa v17.1-18.0, and shortly replaced by userspace supplied OA configurations. Removal of configs causes affected Mesa versions to fall back to earlier kernel behaviour (potentially missing metrics). (Lionel) Cross-subsystem Changes: - Backmerge of drm-next - Includes tag 'topic/phy-compliance-2020-04-08' from git://anongit.freedesktop.org/drm/drm-misc Driver Changes: - Fix for GitLab issue torvalds#27: Support 5k tiled dual DP display on SKL (Ville) - Fix thesofproject#1719: Broken audio after S3 resume on JSL platforms. (Kai) - Add new Tigerlake PCI IDs (Swathi D.) - Add missing Tigerlake W/As (Matt R.) - Extended Wa_2006604312 to EHL (Matt A) - Add DPCD link_rate quirk for Apple 15" MBP 2017 (v3) (Mario) - Make Wa_14010229206 apply to all Tigerlake steppings (Swathi d) - Extend hotplug detect retry on TypeC connectors to 5 seconds (Imre) - Yield the timeslice if caught waiting on a user semaphore (Chris) - Limit the residual W/A batch to Haswell due to instability on IVB/BYT (Chris) - TBT AUX should use TC power well ops on Tigerlake (Matt R) - Update PMINTRMSK holding fw to make it effective for RPS (Francisco, Chris) - Add YUV444 packed format support for skl+ (Stanislav) - Invalidate OA TLB when closing perf stream to avoid corruption (Umesh) - HDCP: fix Ri prime check done during link check (Oliver) - Rearm heartbeat on sysfs interval change (Chris) - Fix crtc nv12 etc. plane bitmasks for DPMS off (Ville) - Treat idling as a RPS downclock event (Chris) - Leave rps->cur_freq on unpark (Chris) - Ignore short pulse when EDP panel powered off (Anshuman) - Keep the engine awake until the next jiffie, to avoid ping-pong on moderate load (Chris) - Select the deepest available parking mode for rc6 on IVB (Chris) - Optimizations to direct submission execlist path (Chris) - Avoid NULL pointer dereference at intel_read_infoframe() (Chris) - Fix mode private_flags comparison at atomic_check (Uma, Ville) - Use forced codec wake on all gen9+ platforms (Kai) - Schedule oa_config after modifying the contexts (Chris, Lionel) - Explicitly reset both reg and context runtime on GPU reset (Chris) - Don't enable DDI IO power on a TypeC port in TBT mode (Imre) - Fixes to TGL, ICL and EHL vswing tables (Jose) - Fill all the unused space in the GGTT (Chris, imre) - Ignore readonly failures when updating relocs (Chris) - Attempt to find free space earlier for non-pinned VMAs (Chris) - Only wait for GPU activity before unbinding a GGTT fence (Chris) - Avoid data loss on small userspace perf OA polling (Ashutosh) - Watch out for unevictable nodes during eviction (Matt A) - Reinforce the barrier after GTT updates for Ironlake (Chris) - Convert various parts of driver to use drm_device based logging (Wambui, Jani) - Avoid dereferencing already closed context for engine (Chris) - Enable non-contiguous pipe fusing (Anshuman) - Add HW readout of Gamma LUT on ICL (Swati S.) - Use explicit flag to mark unreachable intel_context (Chris) - Cancel a hung context if already closed (Chris) - Add DP VSC/HDR SDP data structures and write routines (Gwan-gyeong) - Report context-is-closed prior to pinning at execbuf (Chris) - Mark timeline->cacheline as destroyed after rcu grace period (Chris) - Avoid live-lock with i915_vma_parked() (Chris) - Avoid gem_context->mutex for simple vma lookup (Chris) - Rely on direct submission to the queue (Chris) - Configure DSI transcoder to operate in TE GATE command mode (Vandita) - Add DI vblank calculation for command mode (Vandita) - Disable periodic command mode if programmed by GOP (Vandita) - Use private flags to indicate TE in cmd mode (Vandita) - Make fences a nice-to-have for FBC on GEN9+ (Jose) - Fix work queuing issue with mixed virtual engine/physical engine submissions (Chris) - Drop final few uses of drm_i915_private.engine (Chris) - Return early after MISSING_CASE for write_dp_sdp (Chris) - Include port sync state in the state dump (Ville) - ELSP workaround switching back to a completed context (Chris) - Include priority info in trace_ports (Chris) - Allow for different modes of interruptible i915_active_wait (Chris) - Split eb_vma into its own allocation (Chris) - Don't read perf head/tail pointers outside critical section (Lionel) - Pause CS flow before execlists reset (Chris) - Make fence revocation unequivocal (Chris) - Drop cached obj->bind_count (Chris) - Peek at the next submission for error interrupts (Chris) - Utilize rcu iteration of context engines (Chris) - Keep a per-engine request pool for power management ops (Chris) - Refactor port sync code into normal modeset flow (Ville) - Check current i915_vma.pin_count status first on unbind (Chris) - Free request pool from virtual engines (Chris) - Flush all the reloc_gpu batch (Chris) - Make exclusive awaits on i915_active optional and allow async waits (Chris) - Wait until the context is finally retired before releasing engines (Chris) - Prefer '%ps' for printing function symbol names (Chris) - Allow setting generic data pointer on intel GT debugfs (Andi) - Constify DP link computation code more (Ville) - Simplify MST master transcoder computation (Ville) - Move TRANS_DDI_FUNC_CTL2 programming where it belongs (Ville) - Move icl_get_trans_port_sync_config() into the DDI code (Ville) - Add definitions for VRR registers and bits (Aditya) - Refactor hardware fence code (Chris) - Start passing latency as parameter to WM calculation (Stanislav) - Kernel selftest and debug tracing improvements (Matt A, Chris, Mika) - Fixes to CI found corner cases and lockdep splats (Chris) - Overall fixes and refactoring to GEM code (Chris) - Overall fixes and refactoring to display code (Ville) - GuC/HuC code improvements (Daniele, Michal Wa) - Static code checker fixes (Nathan, Ville, Colin, Chris) - Fix spelling mistake (Chen) Signed-off-by: Dave Airlie <airlied@redhat.com> From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200417111548.GA15033@jlahtine-desk.ger.corp.intel.com
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: #1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: #1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Jaska Uimonen <jaska.uimonen@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: #1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Jaska Uimonen <jaska.uimonen@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: #1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Jaska Uimonen <jaska.uimonen@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694533c9 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject/linux#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694533c9 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject/linux#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694533c9 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject/linux#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694533c9 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject/linux#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Replace the TGL/ICL specific platform checks with a more generic check using INTEL_GEN(). Fixes bug with broken audio after S3 resume on JSL platforms. An initial version of state save and restore of AUD_FREQ_CNTRL register was added for subset of platforms in commit 87c1694533c9 ("drm/i915: save AUD_FREQ_CNTRL state at audio domain suspend"). The state save has proven to work well and it is needed in newer platforms, so needs to be extended. Although the logic is not in practise needed on GEN9/10 systems, follow the hardware specification and apply state and restore on all gen9+ platforms. Bspec: 49281 Link: thesofproject/linux#1719 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20200330144421.11632-1-kai.vehmanen@linux.intel.com Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Describe the bug
Perform S3 test first, then do playback or capture, aplay/arecord will stuck there without output and process cannot be killed.
This issue only happens on HDA mode, on I2S mode, sof works well after S3.
To Reproduce
rtcwake -m mem -s 5
aplay -Dhw:0,1 -f dat -c 2 -vvv /dev/zero
Reproduction Rate
100%
Expected behavior
SOF works well after S3
Environment
Branch jsl-001-drop-stable
Commit: 921fd95
kernel:
Branch:sof-dev
Commit: 0e05b6e
dmesg
dmesg_s3.txt
The text was updated successfully, but these errors were encountered: