Fix/legacy sst coexistence by plbossart · Pull Request #480 · thesofproject/linux

plbossart · 2018-12-21T17:40:02Z

Now that we have a mechanism upstream to deal with HDaudio legacy, let's use it.
Note that SOF still needs to enable the PCI class/subclass as listed in #468

…r selected Now that the SST/Skylake driver supports per platform selectors, we can add logic to automatically select the right driver. If the Skylake driver is selected for a specific platform, and the DSP is detected at run-time based on the PCI class/subclass/prog-if information, the legacy HDaudio driver aborts the probe. This will result in a single driver probing and remove the need for modprobe blacklists. Follow-up patches will add a module parameter to bypass the logic if this automatic detection fails, or if the Skylake driver is unable to actually support the platform (firmware authentication, missing topology file, hardware issue, etc). The same mechanism will be used to conflicts generated by the same PCI ID being registered by both legacy HDAuudio and SOF drivers for Intel platforms. In other words SOF will not require changes to the HDaudio legacy. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Takashi Iwai <tiwai@suse.de> (cherry picked from commit c337104)

For HDaudio and Skylake drivers, add module parameter "pci_binding" When pci_binding == 0 (AUTO), the PCI class/subclass info is used to select drivers based on the presence of the DSP. pci_binding == 1 (LEGACY) forces the use of the HDAudio legacy driver, even if the DSP is present. pci_binding == 2 (ASOC) forces the use of the ASOC driver. The information on the DSP presence is bypassed. The value for the module parameter needs to be identical for both drivers. This parameter is intended as a back-up solution if the automatic detection fails or when the DSP usage fails. Such cases should be reported on the alsa-devel mailing list for analysis. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Takashi Iwai <tiwai@suse.de> (cherry picked from commit d82b51c)

We don't have I2S based plaforms yet so use CannonLake information Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Use existing mechanism to prevent HDaudio legacy from probing when SOF Intel drivers are used. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The choice is done on a platform-per-platform basis, e.g. it's perfectly fine to use the SST/Skylake driver for Kabylake but SOF for CoffeeLake. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

lyakh · 2019-01-04T07:44:09Z

+			dev_info(&pci->dev, "Module parameter forced binding with SKL+ ASoC driver, aborting probe\n");
+			return -ENODEV;
+		default:
+			dev_err(&pci->dev, "invalid value for skl_pci_binding module parameter, ignored\n");


Shouldn't ignoring an invalid value be equivalent to automatic detection, to also fall back to the pre-patch behaviour? Similar for the ASoC code below.

I don't get your point. In both cases an invalid module parameter is ignored and the previous behavior is used.

lyakh

I didn't find a way to comment on the commit message... I suspect two typos there (highlighted in bold):
The same mechanism will be used to resolve conflicts generated by the same PCI
ID being registered by both legacy HDAuudio and SOF drivers for Intel

lyakh · 2019-01-04T08:07:44Z

+
+config SND_HDA_INTEL_DSP_DETECTION_SKL
+	bool
+	help


wouldn't it be better to hide these Kconfig entries by removing their help entries, if they are anyway not for humans to toggle?

There are different opinions on this. I used comments in the past but was asked to add a help message.

I also don't like the fact that commit messages are out of scope in github. Thanks for the typo report.

plbossart · 2019-01-25T19:07:37Z

Closing this PR for now. The autodetection isn't straighforward and until we support SKL and devices using the Intel private key this is premature.

[Why] If we have a single MST display and we disconnect it, we dont disable that link. This causes the old link settings to still exist Now on a replug for MST we think its a link loss and will try to reallocate mst payload which will fail, throwing warning below. [ 129.374192] [drm] Failed to updateMST allocation table forpipe idx:0 [ 129.374206] ------------[ cut here ]------------ [ 129.374284] WARNING: CPU: 14 PID: 1710 at drivers/gpu/drm/amd/amdgpu/../dal-dev/dc/core/dc_link.c:3153 dc_link_allocate_mst_payload+0x1f7/0x220 [amdgpu] [ 129.374285] Modules linked in: amdgpu(OE) amd_iommu_v2 gpu_sched ttm drm_kms_helper drm fb_sys_fops syscopyarea sysfillrect sysimgblt binfmt_misc nls_iso8859_1 edac_mce_amd snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio kvm snd_hda_codec_hdmi snd_hda_intel snd_intel_nhlt snd_hda_codec irqbypass snd_hda_core snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi crct10dif_pclmul snd_seq crc32_pclmul ghash_clmulni_intel snd_seq_device snd_timer snd aesni_intel eeepc_wmi crypto_simd asus_wmi joydev cryptd sparse_keymap input_leds soundcore video glue_helper wmi_bmof mxm_wmi k10temp ccp mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 hid_generic usbhid hid igb i2c_algo_bit ahci dca i2c_piix4 libahci gpio_amdpt wmi gpio_generic [ 129.374318] CPU: 14 PID: 1710 Comm: kworker/14:2 Tainted: G W OE 5.4.0-rc7bhawan+ thesofproject#480 [ 129.374318] Hardware name: System manufacturer System Product Name/PRIME X370-PRO, BIOS 0515 03/30/2017 [ 129.374397] Workqueue: events dm_irq_work_func [amdgpu] [ 129.374468] RIP: 0010:dc_link_allocate_mst_payload+0x1f7/0x220 [amdgpu] [ 129.374470] Code: 52 20 e8 1c 63 ad f4 48 8b 5d d0 65 48 33 1c 25 28 00 00 00 b8 01 00 00 00 75 16 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 <0f> 0b e9 fa fe ff ff e8 ed 5b d6 f3 41 0f b6 b6 c4 02 00 00 48 c7 [ 129.374471] RSP: 0018:ffff9f9141e7fcc0 EFLAGS: 00010246 [ 129.374472] RAX: 0000000000000000 RBX: ffff91ef0762f800 RCX: 0000000000000000 [ 129.374473] RDX: 0000000000000005 RSI: ffffffffc0c4a988 RDI: 0000000000000004 [ 129.374474] RBP: ffff9f9141e7fd10 R08: 0000000000000005 R09: 0000000000000000 [ 129.374475] R10: 0000000000000002 R11: 0000000000000001 R12: ffff91eebd510c00 [ 129.374475] R13: ffff91eebd510e58 R14: ffff91ef052c01b8 R15: 0000000000000006 [ 129.374476] FS: 0000000000000000(0000) GS:ffff91ef0ef80000(0000) knlGS:0000000000000000 [ 129.374477] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 129.374478] CR2: 000055623ea01d50 CR3: 0000000408a8c000 CR4: 00000000003406e0 [ 129.374479] Call Trace: [ 129.374550] dc_link_reallocate_mst_payload+0x12e/0x150 [amdgpu] [ 129.374617] dc_link_handle_hpd_rx_irq+0x6d4/0x6e0 [amdgpu] [ 129.374693] handle_hpd_rx_irq+0x77/0x310 [amdgpu] [ 129.374768] dm_irq_work_func+0x53/0x70 [amdgpu] [ 129.374774] process_one_work+0x1fd/0x3f0 [ 129.374776] worker_thread+0x255/0x410 [ 129.374778] kthread+0x121/0x140 [ 129.374780] ? process_one_work+0x3f0/0x3f0 [ 129.374781] ? kthread_park+0x90/0x90 [ 129.374785] ret_from_fork+0x22/0x40 [How] when we disable MST we should clear the cur link settings (lane_count=0 is good enough). This will cause us to not reallocate payloads earlier than expected and not throw the warning Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com> Reviewed-by: Hersen Wu <hersenxs.wu@amd.com> Acked-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

We try using cancel_delayed_work_sync() to prevent the work from enabling NAPI. This is insufficient since we don't disable the source of the refill work scheduling. This means an NAPI poll callback after cancel_delayed_work_sync() can schedule the refill work then can re-enable the NAPI that leads to use-after-free [1]. Since the work can enable NAPI, we can't simply disable NAPI before calling cancel_delayed_work_sync(). So fix this by introducing a dedicated boolean to control whether or not the work could be scheduled from NAPI. [1] ================================================================== BUG: KASAN: use-after-free in refill_work+0x43/0xd4 Read of size 2 at addr ffff88810562c92e by task kworker/2:1/42 CPU: 2 PID: 42 Comm: kworker/2:1 Not tainted 5.19.0-rc1+ #480 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014 Workqueue: events refill_work Call Trace: <TASK> dump_stack_lvl+0x34/0x44 print_report.cold+0xbb/0x6ac ? _printk+0xad/0xde ? refill_work+0x43/0xd4 kasan_report+0xa8/0x130 ? refill_work+0x43/0xd4 refill_work+0x43/0xd4 process_one_work+0x43d/0x780 worker_thread+0x2a0/0x6f0 ? process_one_work+0x780/0x780 kthread+0x167/0x1a0 ? kthread_exit+0x50/0x50 ret_from_fork+0x22/0x30 </TASK> ... Fixes: b2baed6 ("virtio_net: set/cancel work on ndo_open/ndo_stop") Signed-off-by: Jason Wang <jasowang@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Signed-off-by: David S. Miller <davem@davemloft.net>

Fix checkpatch code style warnings: WARNING: quoted string split across lines thesofproject#480: FILE: net/core/pktgen.c:480: + "Packet Generator for packet performance testing. " + "Version: " VERSION "\n"; WARNING: quoted string split across lines thesofproject#632: FILE: net/core/pktgen.c:632: + " udp_src_min: %d udp_src_max: %d" + " udp_dst_min: %d udp_dst_max: %d\n", Signed-off-by: Peter Seiderer <ps.report@gmx.net> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

plbossart mentioned this pull request Dec 21, 2018

CI: Work out default kconfig for WHL to build daily image for testing #463

Closed

plbossart added 5 commits December 26, 2018 12:44

ASoC: SOF: Intel: add placeholder for CoffeeLake

5510151

We don't have I2S based plaforms yet so use CannonLake information Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

ASoC: SOF: Intel: enable HDaudio legacy DSP detection

50a6888

Use existing mechanism to prevent HDaudio legacy from probing when SOF Intel drivers are used. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

ASoC: Intel: Add mutual exclusion with SOF drivers

a74e976

The choice is done on a platform-per-platform basis, e.g. it's perfectly fine to use the SST/Skylake driver for Kabylake but SOF for CoffeeLake. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

plbossart force-pushed the fix/legacy-sst-coexistence branch from 7d3a5c4 to a74e976 Compare December 26, 2018 18:47

lyakh reviewed Jan 4, 2019

View reviewed changes

plbossart closed this Jan 25, 2019

Navigation Menu

Repository navigation

Fix/legacy sst coexistence - #480

Fix/legacy sst coexistence#480
plbossart wants to merge 5 commits into
thesofproject:topic/sof-devfrom
plbossart:fix/legacy-sst-coexistence

plbossart commented Dec 21, 2018

Uh oh!

lyakh Jan 4, 2019

Uh oh!

plbossart Jan 4, 2019

Uh oh!

lyakh left a comment

Uh oh!

lyakh Jan 4, 2019

Uh oh!

plbossart Jan 4, 2019

Uh oh!

plbossart Jan 4, 2019

Uh oh!

plbossart commented Jan 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

plbossart commented Dec 21, 2018

Uh oh!

lyakh Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

plbossart Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

lyakh left a comment

Choose a reason for hiding this comment

Uh oh!

lyakh Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

plbossart Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

plbossart Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

plbossart commented Jan 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants