[alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Tue May 2 22:15:20 CEST 2017
On 5/2/17 1:27 PM, Ville Syrjälä wrote:
> On Mon, May 01, 2017 at 08:29:10PM -0500, Pierre-Louis Bossart wrote:
>>
>>
>> On 04/28/2017 02:37 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 28, 2017 at 12:10:31PM -0500, Pierre-Louis Bossart wrote:
>>>>
>>>> On 04/28/2017 03:41 AM, Takashi Iwai wrote:
>>>>> On Thu, 27 Apr 2017 18:02:19 +0200,
>>>>> ville.syrjala at linux.intel.com wrote:
>>>>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>>>>
>>>>>> Okay, here's the second attempt at getting multiple pipes playing back
>>>>>> audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
>>>>>> that now the PCM devices are associated with ports instead of pipes,
>>>>>> so the audio from one device always gets output on the same display.
>>>>>>
>>>>>> I've also tacked on the alsa-lib conf update. No clue whether it's
>>>>>> really correct or not (the config language isn't a close friend
>>>>>> of mine).
>>>>>>
>>>>>> BTW I did notice that with LPE audio all the controls say iface=PCM,
>>>>>> whereas on HDA a bunch of them say iface=MIXER. No idea if that's
>>>>>> OK or not, just something I spotted when I was comparing the results
>>>>>> with HDA.
>>>>> We generally accept both iface types for IEC958 stuff, since
>>>>> historically many drivers have already mixed them up. So it's no
>>>>> problem :)
>>>>>
>>>>>
>>>>>> Entire series available here:
>>>>>> git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2
>>>>>>
>>>>>> Cc: Takashi Iwai <tiwai at suse.de>
>>>>>> Cc: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
>>>>> All look good, and feel free to take my reviewed-by tag
>>>>> Reviewed-by: Takashi Iwai <tiwai at suse.de>
>>>>>
>>>>> As said previously, my only slight concern is the compatibility.
>>>>> But, in the current situation with PulseAudio, only few people would
>>>>> use this driver, so it shouldn't be so big impact, I suppose.
>>>>>
>>>>> BTW, which port is used in general on BYT/CHT?
>>>>>
>>>>> Oh, also, I suppose you want to carry these over i915 tree?
>>>>> I don't mind either way, I can take them through sound tree if
>>>>> preferred, too.
>>>> I see frequent oops on startup with this lpe_audio_multipipe_2 branch
>>>> with my CHT device not booting or no HDMI audio device created.
>>>> Not sure if these issues are due to the new patches or to the rest of
>>>> the drm code?
>>>>
>>>> [ 5.529023] BUG: unable to handle kernel NULL pointer dereference
>>>> at (null)
>>>> [ 5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
>>>> [ 5.529202] PGD 0
>>>>
>>>> [ 5.529242] Oops: 0000 [#1] SMP
>>>> [ 5.529274] Modules linked in: snd_soc_sst_atom_hifi2_platform
>>>> snd_soc_sst_match snd_soc_core snd_compress lpc_ich snd_seq
>>>> snd_seq_device shpchp snd_hdmi_lpe_audio(+) snd_pcm snd_timer dw_dmac
>>>> snd soundcore i2c_designware_platform(+) i2c_designware_core
>>>> spi_pxa2xx_platform acpi_pad mac_hid nfsd auth_rpcgss nfs_acl lockd
>>>> grace sunrpc ip_tables x_tables hid_generic mmc_block i2c_hid usbhid hid
>>>> autofs4
>>>> [ 5.529605] CPU: 2 PID: 512 Comm: systemd-udevd Not tainted
>>>> 4.11.0-rc8-test+ #11
>>>> [ 5.529671] Hardware name: ZOTAC XXXXXX/Cherry Trail FFD, BIOS 5.11
>>>> 09/28/2016
>>>> [ 5.529736] task: ffff88007485b780 task.stack: ffffc90000bfc000
>>>> [ 5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650
>>>> [snd_hdmi_lpe_audio]
>>>> [ 5.529855] RSP: 0018:ffffc90000bffaf0 EFLAGS: 00010246
>>>> [ 5.529904] RAX: 0000000000000000 RBX: ffff880079209898 RCX:
>>>> ffff88007920f078
>>>> [ 5.529967] RDX: 0000000000000014 RSI: ffffc90000bffb28 RDI:
>>>> 0000000000000002
>>>> [ 5.530031] RBP: ffffc90000bffb70 R08: 0000000000000001 R09:
>>>> 0000000000000000
>>>> [ 5.530094] R10: ffff88007441bf00 R11: ffffc90000bffb36 R12:
>>>> ffff88007920ef20
>>>> [ 5.530159] R13: ffff88007920ef48 R14: 0000000000005688 R15:
>>>> 0000000000000047
>>>> [ 5.530225] FS: 00007f627c988640(0000) GS:ffff88007b300000(0000)
>>>> knlGS:0000000000000000
>>>> [ 5.530299] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [ 5.530352] CR2: 0000000000000000 CR3: 0000000078cb8000 CR4:
>>>> 00000000001006e0
>>>> [ 5.530416] Call Trace:
>>>> [ 5.530452] platform_drv_probe+0x3b/0xa0
>>>> [ 5.530494] driver_probe_device+0x2bb/0x460
>>>> [ 5.530538] __driver_attach+0xdf/0xf0
>>>> [ 5.530576] ? driver_probe_device+0x460/0x460
>>>> [ 5.530620] bus_for_each_dev+0x60/0xa0
>>>> [ 5.530658] driver_attach+0x1e/0x20
>>>> [ 5.530693] bus_add_driver+0x170/0x270
>>>> [ 5.530731] driver_register+0x60/0xe0
>>>> [ 5.530769] ? 0xffffffffa01cb000
>>>> [ 5.530803] __platform_driver_register+0x36/0x40
>>>> [ 5.530851] hdmi_lpe_audio_driver_init+0x17/0x1000 [snd_hdmi_lpe_audio]
>>>> [ 5.530915] do_one_initcall+0x43/0x180
>>>> [ 5.530956] ? __vunmap+0x81/0xd0
>>>> [ 5.530991] ? kfree+0x14c/0x160
>>>> [ 5.531024] ? kmem_cache_alloc_trace+0x38/0x150
>>>> [ 5.531070] do_init_module+0x5f/0x1f8
>>>> [ 5.531108] load_module+0x271e/0x2bd0
>>>> [ 5.531147] ? kernel_read_file+0x1a3/0x1c0
>>>> [ 5.531188] SYSC_finit_module+0xbc/0xf0
>>>> [ 5.531226] ? SYSC_finit_module+0xbc/0xf0
>>>> [ 5.531267] SyS_finit_module+0xe/0x10
>>>> [ 5.531305] do_syscall_64+0x6e/0x180
>>>> [ 5.531345] entry_SYSCALL64_slow_path+0x25/0x25
>>>> [ 5.531389] RIP: 0033:0x7f627b5fbbf9
>>>> [ 5.531424] RSP: 002b:00007ffe053eee68 EFLAGS: 00000246 ORIG_RAX:
>>>> 0000000000000139
>>>> [ 5.531493] RAX: ffffffffffffffda RBX: 000055d6c745b690 RCX:
>>>> 00007f627b5fbbf9
>>>> [ 5.531558] RDX: 0000000000000000 RSI: 00007f627c134995 RDI:
>>>> 0000000000000007
>>>> [ 5.531622] RBP: 00007f627c134995 R08: 0000000000000000 R09:
>>>> 00007ffe053eef80
>>>> [ 5.531687] R10: 0000000000000007 R11: 0000000000000246 R12:
>>>> 0000000000000000
>>>> [ 5.531751] R13: 000055d6c7459ae0 R14: 0000000000020000 R15:
>>>> 000055d6c745b690
>>>> [ 5.531816] Code: 48 8b 45 b0 8b 48 18 e8 e0 cb 22 e1 49 8b 44 24 28
>>>> 4a 8d 8c 33 58 01 00 00 48 8d 75 b8 45 31 c9 41 b8 01 00 00 00 ba 14 00
>>>> 00 00 <48> 8b 38 e8 a9 2a ff ff 85 c0 0f 88 09 01 00 00 49 8b 84 24 58
>>> Disassembling that I get:
>>>
>>> 4005a0: 48 8b 45 b0 mov -0x50(%rbp),%rax
>>> 4005a4: 8b 48 18 mov 0x18(%rax),%ecx
>>> 4005a7: e8 e0 cb 22 e1 callq ffffffffe162d18c <_end+0xffffffffe102c14c>
>>> 4005ac: 49 8b 44 24 28 mov 0x28(%r12),%rax
>>> 4005b1: 4a 8d 8c 33 58 01 00 lea 0x158(%rbx,%r14,1),%rcx
>>> 4005b8: 00
>>> 4005b9: 48 8d 75 b8 lea -0x48(%rbp),%rsi
>>> 4005bd: 45 31 c9 xor %r9d,%r9d
>>> 4005c0: 41 b8 01 00 00 00 mov $0x1,%r8d
>>> 4005c6: ba 14 00 00 00 mov $0x14,%edx
>>> 4005cb: 48 8b 38 mov (%rax),%rdi
>>> 4005ce: e8 a9 2a ff ff callq 3f307c <_init-0xd32c>
>>> 4005d3: 85 c0 test %eax,%eax
>>> 4005d5: 0f 88 09 01 00 00 js 4006e4 <__GNU_EH_FRAME_HDR+0x100>
>>> 4005db: 49 rex.WB
>>> 4005dc: 8b .byte 0x8b
>>> 4005dd: 84 24 58 test %ah,(%rax,%rbx,2)
>>>
>>> Comparing that with the disassembly from my build, that
>>> first call looks like the snprintf() and the second one would
>>> then be snd_jack_new(). So it seems to blow in the
>>> ctx->card_ctx->card dereference. But I don't really see how
>>> a NULL pointer could sneak in there.
>>
>> There was an standard PORT_B off-by-one error which resulted in memory
>> corruption.
>
> Doh.
>
>> the patch below seems to fix the problem, I can get the same
>> functionality as before with concurrent playback on HDMI and DP, and the
>> jack values are correctly set.
>
> Thanks. I do wonder a bit which side should apply this offset.
> But I guess it doesn't really matter in the end as long as both
> sides know what the deal is. So unless there are good reasons
> for anything else I'll just squash this into my patch(es).
Fine with me.
One open I still have is that it's not self-explanatory for the user to
figure out which output the devices correspond to, e.g. on my CHT device
HDMI is device 2 and DP is device 0.
I may be mistaken but in the previous pipe-based solution, audio
playback would progress at a normal rate even if nothing was connected.
With the port-based, it looks like playback doesn't progress if there is
nothing connected - speaker-test is stuck. Not sure if the open or
hw_params should fail in that case? One could argue that the user should
know better and check the jack status, but in reality I don't think any
userspace layer will ever do this.
>
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c
>> b/drivers/gpu/drm/i915/intel_lpe_audio.c
>> index fa728ed..3bf6528 100644
>> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
>> @@ -332,7 +332,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
>> *dev_priv,
>> return;
>>
>> pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev);
>> - ppdata = &pdata->port[port];
>> + ppdata = &pdata->port[port - PORT_B];
>>
>> spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags);
>>
>> @@ -359,7 +359,7 @@ void intel_lpe_audio_notify(struct drm_i915_private
>> *dev_priv,
>> }
>>
>> if (pdata->notify_audio_lpe)
>> - pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port);
>> + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, port - PORT_B);
>>
>> spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags);
>> }
>> diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
>> index 909391d..0153439 100644
>> --- a/sound/x86/intel_hdmi_audio.c
>> +++ b/sound/x86/intel_hdmi_audio.c
>> @@ -1579,7 +1579,7 @@ static irqreturn_t
>> display_pipe_interrupt_handler(int irq, void *dev_id)
>> static void notify_audio_lpe(struct platform_device *pdev, int port)
>> {
>> struct snd_intelhad_card *card_ctx = platform_get_drvdata(pdev);
>> - struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port];
>> + struct snd_intelhad *ctx = &card_ctx->pcm_ctx[port]; /* warning:
>> offset with -PORT_B */
>>
>> schedule_work(&ctx->hdmi_audio_wq);
>> }
>>
>>
>
More information about the Alsa-devel
mailing list