Re: [Intel-gfx] [alsa-devel] Timing issues between ALSA and i915 drivers
I tried to narrow down the issue further and my current understanding is that the Skylake driver performs link reset operations without the display power turned on - which does not look like a very smart thing to do in hindsight. In other words, it's not really when snd_hdac_i915_init() is called that matters as I assumed initially, but more when snd_hdac_display_power() is invoked. There are two cases where this happens, and for each of them turning the display power on results in HDMI detection. The attached diffs split the initialization from the power on, which provides a better understanding of the issue. OK, this makes some sense, and that's the very reason we have HDA_CODEC_IDX_CONTROLLER for snd_hdac_display_power(). IIRC, we needed to power on the display for probing of the legacy HDA, too. Once after that, for the normal operation, the display power is needed only when you output the HDMI stream. What would be really useful at this point is a confirmation that snd_hdac_i915_init() cannot be called in the initial probe but does need to be executed in a work queue. That would really impact the way the initialization sequence is reworked on the Skylake side as well as modify the way the SOF driver deals with i915 initialization. It's needed to be called in a work queue, yes. Basically you shouldn't call request_module() in the driver's probe callback. When the probe callback is called from the module loading, it blocks the module loading itself, hence loading yet another module can't work. A situation might be easier than the past (which deadlocked), but still it's advised to use either the request_module_nowait() with the callback or call request_module() asynchronously from probe. Thanks Takashi, this is very useful. I guess that will require a complete rework of the Skylake initialization sequence then, my simple code translation isn't enough indeed and the current partition between probe/work queue can't comply with both requirements (request module asynchronously from probe, display turned on before mucking with links). We also need this changed for SOF, the i915_init is done in the probe. -Pierre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] Timing issues between ALSA and i915 drivers
I could use some feedback on HDMI audio issues exposed during the 4.21 merge window. By accident (misleading documentation) we ended up enabling the Skylake driver instead of the HDaudio legacy, and broke audio on a number of Skylake and ApolloLake devices where the HDMI/iDISP codec was not detected (bit 2 not set in the codec_mask). Linus' Dell XPS13 9350 was the first to be impacted of course... After debugging a bit, this issue can be resolved by either a) compiling both SOUND and DRM as built-ins (y instead of m) b) moving the calls snd_hdac_i915_init() to the probe function instead of the worker queue (code at https://github.com/plbossart/sound/commits/fix/skl-hdmi) Both solutions point to timing issues. During internal reviews I was alerted to the fact that the suggested fix essentially reverts patch ab1b732d53c18 ('ASoC: Intel: Skylake: Move i915 registration to worker thread') which was introduced to solve DRM lockup issues. I tried to narrow down the issue further and my current understanding is that the Skylake driver performs link reset operations without the display power turned on - which does not look like a very smart thing to do in hindsight. In other words, it's not really when snd_hdac_i915_init() is called that matters as I assumed initially, but more when snd_hdac_display_power() is invoked. There are two cases where this happens, and for each of them turning the display power on results in HDMI detection. The attached diffs split the initialization from the power on, which provides a better understanding of the issue. What would be really useful at this point is a confirmation that snd_hdac_i915_init() cannot be called in the initial probe but does need to be executed in a work queue. That would really impact the way the initialization sequence is reworked on the Skylake side as well as modify the way the SOF driver deals with i915 initialization. Thanks -Pierre diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 60c94836bf5b..56556d06a17f 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -767,23 +767,6 @@ static const struct hdac_bus_ops bus_core_ops = { .get_response = snd_hdac_bus_get_response, }; -static int skl_i915_init(struct hdac_bus *bus) -{ - int err; - - /* - * The HDMI codec is in GPU so we need to ensure that it is powered - * up and ready for probe - */ - err = snd_hdac_i915_init(bus); - if (err < 0) - return err; - - snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true); - - return 0; -} - static void skl_probe_work(struct work_struct *work) { struct skl *skl = container_of(work, struct skl, probe_work); @@ -791,12 +774,6 @@ static void skl_probe_work(struct work_struct *work) struct hdac_ext_link *hlink = NULL; int err; - if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) { - err = skl_i915_init(bus); - if (err < 0) - return; - } - err = skl_init_chip(bus, true); if (err < 0) { dev_err(bus->dev, "Init chip failed with err: %d\n", err); @@ -899,6 +876,11 @@ static int skl_first_init(struct hdac_bus *bus) unsigned short gcap; int cp_streams, pb_streams, start_idx; + err = snd_hdac_i915_init(bus); + if (err < 0) + return err; + + err = pci_request_regions(pci, "Skylake HD audio"); if (err < 0) return err; @@ -910,7 +892,10 @@ static int skl_first_init(struct hdac_bus *bus) return -ENXIO; } - snd_hdac_bus_reset_link(bus, true); + /* this bus_reset_link is unnecessary, and without the display + * power turned on prevents HDMI from being detected + */ + //snd_hdac_bus_reset_link(bus, true); snd_hdac_bus_parse_capabilities(bus); @@ -962,7 +947,14 @@ static int skl_first_init(struct hdac_bus *bus) /* initialize chip */ skl_init_pci(skl); - return skl_init_chip(bus, true); + /* turning the display power here works */ + snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true); + + err = skl_init_chip(bus, true); + + /* turning the display power here does not work, HDMI not detected */ + + return err; } static int skl_probe(struct pci_dev *pci, ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] Timing issues between ALSA and i915 drivers
a) compiling both SOUND and DRM as built-ins (y instead of m) b) moving the calls snd_hdac_i915_init() to the probe function instead of the worker queue (code at https://github.com/plbossart/sound/commits/fix/skl-hdmi) I added DRM+audio dmesg logs at the following link for reference: https://github.com/thesofproject/linux/issues/549 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Timing issues between ALSA and i915 drivers
I could use some feedback on HDMI audio issues exposed during the 4.21 merge window. By accident (misleading documentation) we ended up enabling the Skylake driver instead of the HDaudio legacy, and broke audio on a number of Skylake and ApolloLake devices where the HDMI/iDISP codec was not detected (bit 2 not set in the codec_mask). Linus' Dell XPS13 9350 was the first to be impacted of course... After debugging a bit, this issue can be resolved by either a) compiling both SOUND and DRM as built-ins (y instead of m) b) moving the calls snd_hdac_i915_init() to the probe function instead of the worker queue (code at https://github.com/plbossart/sound/commits/fix/skl-hdmi) This isn't guaranteed to work, as request_module() might be involved, I'm afraid. Sorry, what would be the impact of the request_module? it'd just delay the probe on the audio side, no? And if the request_module failed then HDMI wouldn't be more broken anyways... Both solutions point to timing issues. During internal reviews I was alerted to the fact that the suggested fix essentially reverts patch ab1b732d53c18 ('ASoC: Intel: Skylake: Move i915 registration to worker thread') which was introduced to solve DRM lockup issues. In other words, we are at a point where we either have DRM lockups or can't detect the HDMI audio codec, none of which are too good. I don't have the background for the DRM lockup stuff, nor do I understand why snd_hdac_i915_init has to be called from a thread context. Is this really a requirement? I also don't get what sort of timing issues would come from an initialization. What happens on the i915 side and is there some sort of mandatory delay between the completion of the i915_init and issuing a snd_hdac_chip_readw(bus, STATESTS) to get the codec masks on the HDaudio links? snd_hdac_i915_init() waits for the binding with the i915 audio component, so a possible solution would be just to delay the audio component registration at the really last stage, like the change below. If this still doesn't work, it'll be more deeply inside, and I have little clue for now... I tried this suggestion and no luck, same error with the iDISP codec not exposed. I added a delay after snd_hdac_i915_init(), doesn't seem to do anything. One possibility is that this is a side effect of the Skylake driver initializing the link twice for some odd reason. And I still don't get what the motivation for moving this init to a work queue was in the first place. Doesn't seem like an easy one to fix... thanks, Takashi --- diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1577,8 +1577,6 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) if (IS_GEN5(dev_priv)) intel_gpu_ips_init(dev_priv); - intel_audio_init(dev_priv); - /* * Some ports require correctly set-up hpd registers for detection to * work properly (leading to ghost connected connector status), e.g. VGA @@ -1597,6 +1595,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) intel_power_domains_enable(dev_priv); intel_runtime_pm_enable(dev_priv); + + intel_audio_init(dev_priv); } /** ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Timing issues between ALSA and i915 drivers
Hi, I could use some feedback on HDMI audio issues exposed during the 4.21 merge window. By accident (misleading documentation) we ended up enabling the Skylake driver instead of the HDaudio legacy, and broke audio on a number of Skylake and ApolloLake devices where the HDMI/iDISP codec was not detected (bit 2 not set in the codec_mask). Linus' Dell XPS13 9350 was the first to be impacted of course... After debugging a bit, this issue can be resolved by either a) compiling both SOUND and DRM as built-ins (y instead of m) b) moving the calls snd_hdac_i915_init() to the probe function instead of the worker queue (code at https://github.com/plbossart/sound/commits/fix/skl-hdmi) Both solutions point to timing issues. During internal reviews I was alerted to the fact that the suggested fix essentially reverts patch ab1b732d53c18 ('ASoC: Intel: Skylake: Move i915 registration to worker thread') which was introduced to solve DRM lockup issues. In other words, we are at a point where we either have DRM lockups or can't detect the HDMI audio codec, none of which are too good. I don't have the background for the DRM lockup stuff, nor do I understand why snd_hdac_i915_init has to be called from a thread context. Is this really a requirement? I also don't get what sort of timing issues would come from an initialization. What happens on the i915 side and is there some sort of mandatory delay between the completion of the i915_init and issuing a snd_hdac_chip_readw(bus, STATESTS) to get the codec masks on the HDaudio links? Thanks for any pointers or comments. -Pierre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2 0/3] Make the audio component binding more generic
On 7/19/18 1:56 PM, Takashi Iwai wrote: On Thu, 19 Jul 2018 15:05:45 +0200, Pierre-Louis Bossart wrote: On 7/19/18 12:50 AM, Takashi Iwai wrote: On Wed, 18 Jul 2018 22:54:35 +0200, Pierre-Louis Bossart wrote: On 07/17/2018 04:26 AM, Takashi Iwai wrote: Hi, this is a preliminiary patch set to convert the existing i915 / HD-audio component binding to be applicable to other drivers like radeon / amdgpu. This patchset itself doesn't change the functionality but only renames and split to a new drm_audio_component stuff from i915_audio_component. The actual usage of the new API will follow once after this one gets reviewed / accepted. The whole patches (including this patchset) are found in topic/hda-acomp branch of sound.git tree. BTW, since the whole stuff is about the audio binding, I suppose these will go through sound git tree. Let me know if anyone has concerns. No objections but a slight concern that this will conflict with the HDAudio+DSP patches that I was about to resubmit on top of your topic/hda-core-intel branch. the two series touch the same files so it'd be a miracle if there is no issue. How do you want to deal with this? Does it conflict severely? If it's trivial, it can be resolved at merge time, too. The changes in my patchset are fairly trivial, so it shouldn't be too hard. I was able to make things work by taking your topic/hda-core-intel, merge it on Mark's for-next, then add my additional changes and these DRM changes. The last two can be done in any order. But I am getting some conflicts if I try to apply these DRM changes first, not sure why git is complaining though, the changes look trivial enough. So yes it looks possible to deal with the two series in parallel, will send my update later today. OK, since my changes are relatively trivial to deal with, I merge the changes to for-next branch now. If your patches can be respinned, maybe it's easier to be rebased on top of these merges. I was planning to resend them tomorrow after internal reviews (mostly changes in the detection/enablement of the HDaudio+DSP case). If you want to take a look in the mean time the patches are here: https://github.com/plbossart/sound/commits/upstream/hda2 I can rebase them as needed, no big deal. Thanks -Pierre Mark, could you merge topic/drm_audio_component branch into yours, if Pierre's patchset won't go in immediately? It's an immutable branch, including already topic/hda-core-intel in itself. thanks, Takashi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2 0/3] Make the audio component binding more generic
On 7/19/18 12:50 AM, Takashi Iwai wrote: On Wed, 18 Jul 2018 22:54:35 +0200, Pierre-Louis Bossart wrote: On 07/17/2018 04:26 AM, Takashi Iwai wrote: Hi, this is a preliminiary patch set to convert the existing i915 / HD-audio component binding to be applicable to other drivers like radeon / amdgpu. This patchset itself doesn't change the functionality but only renames and split to a new drm_audio_component stuff from i915_audio_component. The actual usage of the new API will follow once after this one gets reviewed / accepted. The whole patches (including this patchset) are found in topic/hda-acomp branch of sound.git tree. BTW, since the whole stuff is about the audio binding, I suppose these will go through sound git tree. Let me know if anyone has concerns. No objections but a slight concern that this will conflict with the HDAudio+DSP patches that I was about to resubmit on top of your topic/hda-core-intel branch. the two series touch the same files so it'd be a miracle if there is no issue. How do you want to deal with this? Does it conflict severely? If it's trivial, it can be resolved at merge time, too. The changes in my patchset are fairly trivial, so it shouldn't be too hard. I was able to make things work by taking your topic/hda-core-intel, merge it on Mark's for-next, then add my additional changes and these DRM changes. The last two can be done in any order. But I am getting some conflicts if I try to apply these DRM changes first, not sure why git is complaining though, the changes look trivial enough. So yes it looks possible to deal with the two series in parallel, will send my update later today. Thanks -Pierre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2 0/3] Make the audio component binding more generic
On 07/17/2018 04:26 AM, Takashi Iwai wrote: Hi, this is a preliminiary patch set to convert the existing i915 / HD-audio component binding to be applicable to other drivers like radeon / amdgpu. This patchset itself doesn't change the functionality but only renames and split to a new drm_audio_component stuff from i915_audio_component. The actual usage of the new API will follow once after this one gets reviewed / accepted. The whole patches (including this patchset) are found in topic/hda-acomp branch of sound.git tree. BTW, since the whole stuff is about the audio binding, I suppose these will go through sound git tree. Let me know if anyone has concerns. No objections but a slight concern that this will conflict with the HDAudio+DSP patches that I was about to resubmit on top of your topic/hda-core-intel branch. the two series touch the same files so it'd be a miracle if there is no issue. How do you want to deal with this? Thanks! Takashi === v1->v2: * Change to SPDX for the new drm_audio_component.h * Fix remaining i915 word in drm_audio_component.h comments * Fix NULL dereference in master_bind / _unbind ops === Takashi Iwai (3): drm/i915: Split audio component to a generic type ALSA: hda/i915: Associate audio component with devres ALSA: hda: Make audio component support more generic drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/i915/intel_audio.c | 22 +- include/drm/drm_audio_component.h | 118 ++ include/drm/i915_component.h | 85 +--- include/sound/hda_component.h | 61 ++ include/sound/hda_i915.h | 37 +--- include/sound/hdaudio.h| 6 +- sound/hda/Kconfig | 7 +- sound/hda/Makefile | 1 + sound/hda/hdac_component.c | 335 + sound/hda/hdac_i915.c | 335 ++--- sound/pci/hda/patch_hdmi.c | 57 +++-- sound/soc/codecs/hdac_hdmi.c | 10 +- 13 files changed, 607 insertions(+), 468 deletions(-) create mode 100644 include/drm/drm_audio_component.h create mode 100644 include/sound/hda_component.h create mode 100644 sound/hda/hdac_component.c ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
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.syrj...@linux.intel.com wrote: From: Ville Syrjälä 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 Cc: Pierre-Louis Bossart All look good, and feel free to take my reviewed-by tag Reviewed-by: Takashi Iwai 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: [#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 XX/Cherry Trail FFD, BIOS 5.11 09/28/2016 [5.529736] task: 88007485b780 task.stack: c9bfc000 [5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio] [5.529855] RSP: 0018:c9bffaf0 EFLAGS: 00010246 [5.529904] RAX: RBX: 880079209898 RCX: 88007920f078 [5.529967] RDX: 0014 RSI: c9bffb28 RDI: 0002 [5.530031] RBP: c9bffb70 R08: 0001 R09: [5.530094] R10: 88007441bf00 R11: c9bffb36 R12: 88007920ef20 [5.530159] R13: 88007920ef48 R14: 5688 R15: 0047 [5.530225] FS: 7f627c988640() GS:88007b30() knlGS: [5.530299] CS: 0010 DS: ES: CR0: 80050033 [5.530352] CR2: CR3: 78cb8000 CR4: 001006e0 [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] ? 0xa01cb000 [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:7ffe053eee68 EFLAGS: 0246 ORIG_RAX: 0139 [5.531493] RAX: ffda RBX: 55d6c745b690 RCX: 7f627b5fbbf9 [5.531558] RDX: RSI: 7f627c134995
Re: [Intel-gfx] [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
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.syrj...@linux.intel.com wrote: From: Ville Syrjälä 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 Cc: Pierre-Louis Bossart All look good, and feel free to take my reviewed-by tag Reviewed-by: Takashi Iwai 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: [#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 XX/Cherry Trail FFD, BIOS 5.11 09/28/2016 [5.529736] task: 88007485b780 task.stack: c9bfc000 [5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio] [5.529855] RSP: 0018:c9bffaf0 EFLAGS: 00010246 [5.529904] RAX: RBX: 880079209898 RCX: 88007920f078 [5.529967] RDX: 0014 RSI: c9bffb28 RDI: 0002 [5.530031] RBP: c9bffb70 R08: 0001 R09: [5.530094] R10: 88007441bf00 R11: c9bffb36 R12: 88007920ef20 [5.530159] R13: 88007920ef48 R14: 5688 R15: 0047 [5.530225] FS: 7f627c988640() GS:88007b30() knlGS: [5.530299] CS: 0010 DS: ES: CR0: 80050033 [5.530352] CR2: CR3: 78cb8000 CR4: 001006e0 [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] ? 0xa01cb000 [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:7ffe053eee68 EFLAGS: 0246 ORIG_RAX: 0139 [5.531493] RAX: ffda RBX: 55d6c745b690 RCX: 7f627b5fbbf9 [5.531558] RDX: RSI: 7f627c134995 RDI: 0007 [5.531622] RBP: 7f627c134995 R08: R09: 7ffe053eef80
Re: [Intel-gfx] [alsa-devel] [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
A quick bisect tells me this last patch looks problematic. I don't have time to look further into it today, hope this helps progress in finding the issue. This is on a CHT device with HDMI plugged in and DP left out unconnected for now. $ git bisect good 9af5b5732365b8ea29000d1ad14800bb091a0724 is the first bad commit commit 9af5b5732365b8ea29000d1ad14800bb091a0724 Author: Ville Syrjälä Date: Tue Apr 25 20:42:40 2017 +0300 ALSA: x86: Register multiple PCM devices for the LPE audio card Now that everything is in place let's register a PCM device for each port of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback. v2: Add a PCM per port instead of per pipe Cc: Takashi Iwai Cc: Pierre-Louis Bossart Signed-off-by: Ville Syrjälä :04 04 17f94eecd597b97ccc003e2d27c03eadceb279f5 271d4c3130f9cc1334e79bf60b7fdc7337192655 Mdrivers :04 04 6806ba942f3c0844dcf6ffdfdd751c2007e5680f 8b9e1d1f82a12febe705a771654fadc08c02c90f Minclude :04 04 e1f46a21e1beaf9535b3e807f4cfeea5ad7dbe47 afd86621214380c86769c30d6405d9335143cf6b Msound $ git bisect log git bisect start # bad: [9af5b5732365b8ea29000d1ad14800bb091a0724] ALSA: x86: Register multiple PCM devices for the LPE audio card git bisect bad 9af5b5732365b8ea29000d1ad14800bb091a0724 # good: [8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753] drm-tip: 2017y-04m-27d-13h-10m-59s UTC integration manifest git bisect good 8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753 # good: [63e34e5d2e8aaf85dfe48085e5dbbb7215da80ba] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock git bisect good 63e34e5d2e8aaf85dfe48085e5dbbb7215da80ba # good: [dc0ae8e9c2f395b24cba7a404f2ff49e7272bf4b] drm/i915: Clean up the LPE audio platform data git bisect good dc0ae8e9c2f395b24cba7a404f2ff49e7272bf4b # good: [7d80cfe6f7eafe6cddf36cd6e227d54a45c6f175] ALSA: x86: Split snd_intelhad into card and PCM specific structures git bisect good 7d80cfe6f7eafe6cddf36cd6e227d54a45c6f175 # first bad commit: [9af5b5732365b8ea29000d1ad14800bb091a0724] ALSA: x86: Register multiple PCM devices for the LPE audio card On 04/27/2017 11:02 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä Now that everything is in place let's register a PCM device for each port of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback. v2: Add a PCM per port instead of per pipe Cc: Takashi Iwai Cc: Pierre-Louis Bossart Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_lpe_audio.c | 19 ++--- include/drm/intel_lpe_audio.h | 6 +- sound/x86/intel_hdmi_audio.c | 126 +++-- sound/x86/intel_hdmi_audio.h | 7 +- 4 files changed, 92 insertions(+), 66 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index bdbc235141b5..fa728ed21d1f 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,7 +111,11 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32); - pdata->port.pipe = -1; + pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes; + pdata->num_ports = IS_CHERRYVIEW(dev_priv) ? 3 : 2; /* B,C,D or B,C */ + pdata->port[0].pipe = -1; + pdata->port[1].pipe = -1; + pdata->port[2].pipe = -1; spin_lock_init(&pdata->lpe_audio_slock); platdev = platform_device_register_full(&pinfo); @@ -319,7 +323,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, enum pipe pipe, enum port port, const void *eld, int ls_clock, bool dp_output) { - unsigned long irq_flags; + unsigned long irqflags; struct intel_hdmi_lpe_audio_pdata *pdata; struct intel_hdmi_lpe_audio_port_pdata *ppdata; u32 audio_enable; @@ -328,14 +332,12 @@ 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; + ppdata = &pdata->port[port]; - spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags); + spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags); audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port)); - ppdata->port = port; - if (eld != NULL) { memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); ppdata->p
Re: [Intel-gfx] [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)
On 04/28/2017 03:41 AM, Takashi Iwai wrote: On Thu, 27 Apr 2017 18:02:19 +0200, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä 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 Cc: Pierre-Louis Bossart All look good, and feel free to take my reviewed-by tag Reviewed-by: Takashi Iwai 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: [#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 XX/Cherry Trail FFD, BIOS 5.11 09/28/2016 [5.529736] task: 88007485b780 task.stack: c9bfc000 [5.529793] RIP: 0010:hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio] [5.529855] RSP: 0018:c9bffaf0 EFLAGS: 00010246 [5.529904] RAX: RBX: 880079209898 RCX: 88007920f078 [5.529967] RDX: 0014 RSI: c9bffb28 RDI: 0002 [5.530031] RBP: c9bffb70 R08: 0001 R09: [5.530094] R10: 88007441bf00 R11: c9bffb36 R12: 88007920ef20 [5.530159] R13: 88007920ef48 R14: 5688 R15: 0047 [5.530225] FS: 7f627c988640() GS:88007b30() knlGS: [5.530299] CS: 0010 DS: ES: CR0: 80050033 [5.530352] CR2: CR3: 78cb8000 CR4: 001006e0 [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] ? 0xa01cb000 [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:7ffe053eee68 EFLAGS: 0246 ORIG_RAX: 0139 [5.531493] RAX: ffda RBX: 55d6c745b690 RCX: 7f627b5fbbf9 [5.531558] RDX: RSI: 7f627c134995 RDI: 0007 [5.531622] RBP: 7f627c134995 R08: R09: 7ffe053eef80 [5.531687] R10: 0007 R11: 0246 R12: [5.531751] R13: 00
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts
On 4/25/17 3:27 PM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä vlv_display_irq_postinstall() enables the LPE audio interrupts regardless of whether the LPE audio irq chip has masked/unmasked them. Also the irqchip masking/unmasking doesn't consider the state of the display power well or the device, and hence just leads to dmesg spew when it tries to access the hardware while it's powered down. If the current way works, then we don't need to do anything in the mask/unmask hooks. If it doesn't work, well, then we'd need to properly track whether the irqchip has masked/unmasked the interrupts when we enable display interrupts. And the mask/unmask hooks would need to check whether display interrupts are even enabled before frobbing with he registers. So let's just assume the current way works and neuter the mask/unmask hooks. Also clean up vlv_display_irq_postinstall() a bit and stop it from trying to unmask/enable the LPE C interrupt on VLV since it doesn't exist. No objections, I assumed that we did want to update VLV_IMR and VLV_IIR in the mask/unmask, that was the initial recommendation IIRC There was also a comment where we removed all tests in vlv_display_irq_postinstall: >> + if (IS_LPE_AUDIO_ENABLED(dev_priv)) >> + if (IS_LPE_AUDIO_IRQ_VALID(dev_priv)) > >I think both of these checks can be removed. We won't unmask the >interrupts unless lpe is enabled, so the IIR bits will never be set in >that case. Cc: Takashi Iwai Cc: Pierre-Louis Bossart Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_irq.c| 15 ++ drivers/gpu/drm/i915/intel_lpe_audio.c | 36 -- 2 files changed, 6 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fd97fe00cd0d..190f6aa5d15e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2953,7 +2953,6 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) u32 pipestat_mask; u32 enable_mask; enum pipe pipe; - u32 val; pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV | PIPE_CRC_DONE_INTERRUPT_STATUS; @@ -2964,18 +2963,16 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) enable_mask = I915_DISPLAY_PORT_INTERRUPT | I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | - I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; + I915_DISPLAY_PIPE_B_EVENT_INTERRUPT | + I915_LPE_PIPE_A_INTERRUPT | + I915_LPE_PIPE_B_INTERRUPT; + if (IS_CHERRYVIEW(dev_priv)) - enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT; + enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT | + I915_LPE_PIPE_C_INTERRUPT; WARN_ON(dev_priv->irq_mask != ~0); - val = (I915_LPE_PIPE_A_INTERRUPT | - I915_LPE_PIPE_B_INTERRUPT | - I915_LPE_PIPE_C_INTERRUPT); - - enable_mask |= val; - dev_priv->irq_mask = ~enable_mask; GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask); diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 668f00480d97..292fedf30b00 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -149,44 +149,10 @@ static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv) static void lpe_audio_irq_unmask(struct irq_data *d) { - struct drm_i915_private *dev_priv = d->chip_data; - unsigned long irqflags; - u32 val = (I915_LPE_PIPE_A_INTERRUPT | - I915_LPE_PIPE_B_INTERRUPT); - - if (IS_CHERRYVIEW(dev_priv)) - val |= I915_LPE_PIPE_C_INTERRUPT; - - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - - dev_priv->irq_mask &= ~val; - I915_WRITE(VLV_IIR, val); - I915_WRITE(VLV_IIR, val); - I915_WRITE(VLV_IMR, dev_priv->irq_mask); - POSTING_READ(VLV_IMR); - - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } static void lpe_audio_irq_mask(struct irq_data *d) { - struct drm_i915_private *dev_priv = d->chip_data; - unsigned long irqflags; - u32 val = (I915_LPE_PIPE_A_INTERRUPT | - I915_LPE_PIPE_B_INTERRUPT); - - if (IS_CHERRYVIEW(dev_priv)) - val |= I915_LPE_PIPE_C_INTERRUPT; - - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - - dev_priv->irq_mask |= val; - I915_WRITE(VLV_IMR, dev_priv->irq_mask); - I915_WRITE(VLV_IIR, val); - I915_WRITE(VLV_IIR, val); - POSTING_READ(VLV_IIR); - - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } static struct irq_chip lpe_audio_irqchip = { @@ -330,8 +296
Re: [Intel-gfx] [alsa-devel] [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock
On 4/25/17 3:27 PM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä There's no need to distinguish between the DP link rate and HDMI TMDS clock for the purposes of the LPE audio. Both are actually the same thing more or less, which is the link symbol clock. So let's just call the thing ls_clock and simplify the code. there are still occurences of 'tmds' in sound/x86 and there are are couple of debug messages that don't make sense any longer. Cc: Takashi Iwai Cc: Pierre-Louis Bossart Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h| 4 ++-- drivers/gpu/drm/i915/intel_audio.c | 19 --- drivers/gpu/drm/i915/intel_lpe_audio.c | 14 ++ include/drm/intel_lpe_audio.h | 3 +-- sound/x86/intel_hdmi_audio.c | 11 --- 5 files changed, 21 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 357b6c6c2f04..876eee56a958 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3721,8 +3721,8 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int pipe, int tmds_clk_speed, - bool dp_output, int link_rate); + void *eld, int port, int pipe, int ls_clock, + bool dp_output); /* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 52c207e81f41..79eeef25321f 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -632,20 +632,9 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, (int) port, (int) pipe); } - switch (intel_encoder->type) { - case INTEL_OUTPUT_HDMI: - intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe, - crtc_state->port_clock, - false, 0); - break; - case INTEL_OUTPUT_DP: - intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe, - adjusted_mode->crtc_clock, - true, crtc_state->port_clock); - break; - default: - break; - } + intel_lpe_audio_notify(dev_priv, connector->eld, port, pipe, + crtc_state->port_clock, + intel_encoder->type == INTEL_OUTPUT_DP); } /** @@ -680,7 +669,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder) (int) port, (int) pipe); } - intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false, 0); + intel_lpe_audio_notify(dev_priv, NULL, port, pipe, 0, false); } /** diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 79b9dca985ff..5a1a37e963f1 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -309,13 +309,14 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv) * @eld : ELD data * @pipe: pipe id * @port: port id - * @tmds_clk_speed: tmds clock frequency in Hz + * @ls_clock: Link symbol clock in kHz + * @dp_output: Driving a DP output? * * Notify lpe audio driver of eld change. */ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int pipe, int tmds_clk_speed, - bool dp_output, int link_rate) + void *eld, int port, int pipe, int ls_clock, + bool dp_output) { unsigned long irq_flags; struct intel_hdmi_lpe_audio_pdata *pdata = NULL; @@ -337,12 +338,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, pdata->eld.port_id = port; pdata->eld.pipe_id = pipe; pdata->hdmi_connected = true; - + pdata->ls_clock = ls_clock; pdata->dp_output = dp_output; - if (tmds_clk_speed) - pdata->tmds_clock_speed = tmds_clk_speed; - if (link_rate) - pdata->link_rate = link_rate; /* Unmute the amp for both DP and HDMI */ I915_WRITE(VLV_AUD_PORT_EN_DBG(port), @@ -352,6 +349,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, memset(pdata->eld.eld_data, 0,
Re: [Intel-gfx] [alsa-devel] [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card
On 04/25/2017 03:27 PM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä Now that everything is in place let's register a PCM device for each pipe of the display engine. This will make it possible to actually output audio to multiple displays at the same time. And it avoids modesets on unrelated displays from clobbering up the ELD and whatnot for the display currently doing the playback. The alternative would be to have a PCM device per port, but per-pipe is easier since the hardware actually works that way. Very nice. I just tested on a CHT Zotac box which has two connectors (1 HDMI and 1 DP), and I get sound concurrently on both, with hdmi being listed as device 2 and DP as device 0. I thought there were hardware restrictions but you proved me wrong. Kudos. The only point that I find weird is that the jacks are reported as 'on' on the 3 pipes, is there a way to tie them to an actual cable being used? [plb@ZOTAC ~]$ amixer -Dhw:0 controls | grep Jack numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack' numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack' numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack' [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=5 numid=5,iface=CARD,name='HDMI/DP,pcm=0 Jack' ; type=BOOLEAN,access=r---,values=1 : values=on [plb@ZOTAC ~]$ amixer -Dhw:0 cset numid=5 off amixer: Control hw:0 element write error: Operation not permitted [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=10 numid=10,iface=CARD,name='HDMI/DP,pcm=1 Jack' ; type=BOOLEAN,access=r---,values=1 : values=on [plb@ZOTAC ~]$ amixer -Dhw:0 cget numid=15 numid=15,iface=CARD,name='HDMI/DP,pcm=2 Jack' ; type=BOOLEAN,access=r---,values=1 : values=on The ELD controls do show a null set of values for device 1, maybe the jack value should be set in accordance with the ELD validity? Also I am wondering if the display number could be used for the PCM device number, or as a hint in the device description to help the user know which PCM device to use. Anyway thanks for this patchset, nicely done. Cc: Takashi Iwai Cc: Pierre-Louis Bossart Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_lpe_audio.c | 14 - include/drm/intel_lpe_audio.h | 6 ++-- sound/x86/intel_hdmi_audio.c | 53 +++--- sound/x86/intel_hdmi_audio.h | 3 +- 4 files changed, 34 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index a593fdf73171..270aa3e3f0e2 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -111,6 +111,7 @@ lpe_audio_platdev_create(struct drm_i915_private *dev_priv) pinfo.size_data = sizeof(*pdata); pinfo.dma_mask = DMA_BIT_MASK(32); + pdata->num_pipes = INTEL_INFO(dev_priv)->num_pipes; spin_lock_init(&pdata->lpe_audio_slock); platdev = platform_device_register_full(&pinfo); @@ -318,7 +319,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, enum pipe pipe, enum port port, const void *eld, int ls_clock, bool dp_output) { - unsigned long irq_flags; + unsigned long irqflags; struct intel_hdmi_lpe_audio_pdata *pdata; struct intel_hdmi_lpe_audio_pipe_pdata *ppdata; u32 audio_enable; @@ -327,14 +328,12 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, return; pdata = dev_get_platdata(&dev_priv->lpe_audio.platdev->dev); - ppdata = &pdata->pipe; + ppdata = &pdata->pipe[pipe]; - spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags); + spin_lock_irqsave(&pdata->lpe_audio_slock, irqflags); audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port)); - pdata->pipe_id = pipe; - if (eld != NULL) { memcpy(ppdata->eld, eld, HDMI_MAX_ELD_BYTES); ppdata->port = port; @@ -356,8 +355,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); + pdata->notify_audio_lpe(dev_priv->lpe_audio.platdev, pipe); - spin_unlock_irqrestore(&pdata->lpe_audio_slock, - irq_flags); + spin_unlock_irqrestore(&pdata->lpe_audio_slock, irqflags); } diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 26e569ad8683..b391b2822140 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -39,10 +39,10 @@ struct intel_hdmi_lpe_audio_pipe_pdata { }; struct intel_hdmi_lpe_audio_pdata { - struct intel_hdmi_lpe_audio_pipe_pdata pipe; - int pipe_id; + struct intel_hdmi_lpe_audio_pipe_pdata pipe[3]; + i
Re: [Intel-gfx] [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode
+#define AUD_PORT_EN_B_DBG 0x62F20 +#define AUD_PORT_EN_C_DBG 0x62F28 +#define AUD_PORT_EN_D_DBG 0x62F2C These match the spec. But to match the standard i915 convention they should be called _AUD_PORT_EN_B_DBG etc. Same forthe chicken bit register. Actually they just match one version of the spec I had lying around. Another versions says: AUD_PORT_EN_B_DBG 0x62F20 AUD_PORT_EN_C_DBG 0x62F30 AUD_PORT_EN_D_DBG 0x62F34 That's it! Now finally I can hear the audio from DP3 with the additional patch below. Wow. Thanks Ville for looking into this, we could have lost a lot of time. Do you happen to know if those previous values are due to poor documentation or a different skew we'd need to support, e.g. with a PCI-Id quirk? At any rate, 2 days to get DP audio working is pretty nice, this was a good week. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode
Thanks Jani and Ville for the comments. Couple of precisions needed below: #define GEN6_BSD_RNCID_MMIO(0x12198) #define GEN7_FF_THREAD_MODE _MMIO(0x20a0) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 245523e..b3134ef 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv) goto err_free_irq; } + /* Enable DPAudio debug bits by default */ + if (IS_CHERRYVIEW(dev_priv)) { VLV too. And like I said we might need this in the powerwell code as well. You should make a test to see if the register value is retained across the display power well being turned off. Eg. simply disable all displays, check the log to make sure it really did turn off the display power well, then re-enable some displays, and finally check if the register value was retained or not). VLV has DisplayPort? I thought this was an addition on CHT, and I really don't know of any devices with this combination of HDaudio disabled+DP. I'd rather keep this CHT-only until we find a device where this can be tested. On the powerwell, I could use more guidance. i tried this first solution to see if streaming worked (and it did :-)). I don't mind moving the code somewhere else but I have no idea where. + u32 chicken_bit; + + chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG); + I915_WRITE(VLV_AUD_CHICKEN_BIT_REG, + chicken_bit | CHICKEN_BIT_DBG_ENABLE); + } + return 0; err_free_irq: irq_free_desc(dev_priv->lpe_audio.irq); @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, pdata->tmds_clock_speed = tmds_clk_speed; if (link_rate) pdata->link_rate = link_rate; + + if (dp_output) { /* unmute the amp */ The spec doesn't distinquish DP vs. HDMI here. So I presume we should be able to do this always. I'll try to see if HDMI still works with this. We could tentatively add unmute in all cases but I'll need to add a test for Baytrail (no PORT_D) so in the end it's the same number of tests. And I think we might want to mute things again when disabling audio. I was wondering if there would be side effects of writing to a register controlling a port if that port is not connected any longer. I'll give it a try. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH RFC 1/3] drm/i915: Avoid MST pipe handling for LPE audio
On 01/27/2017 04:36 AM, Takashi Iwai wrote: The pipe gets cleared to -1 for non-MST before the ELD audio notification due to the MST audio support. This makes sense for HD-audio that received the MST handling, but it's useless for LPE audio. Handle the MST pipe hack conditionally only for HD-audio. Reported-by: Pierre-Louis Bossart Signed-off-by: Takashi Iwai --- drivers/gpu/drm/i915/intel_audio.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 1645ce42b898..d4e6d1136cfe 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -624,13 +624,14 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, dev_priv->av_enc_map[pipe] = intel_encoder; mutex_unlock(&dev_priv->av_mutex); - /* audio drivers expect pipe = -1 to indicate Non-MST cases */ - if (intel_encoder->type != INTEL_OUTPUT_DP_MST) - pipe = -1; - - if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) + if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) { + /* audio drivers expect pipe = -1 to indicate Non-MST cases */ + if (intel_encoder->type != INTEL_OUTPUT_DP_MST) + pipe = -1; acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port, (int) pipe); + } + Cool. I missed this part, couldn't figure out where the -1 was coming from. So do you get audio working on both of the DP ports now? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 5/5] ALSA: x86: hdmi: hack to enable DP audio on CHT
Somehow the configuration register that the audio driver needs to use depends on the port/pipe combination. Adding the pipe information to the notify() call isn't helping, the pipe value is -1 (illegal). FIXME: does this require a change in the pdata or a handshake with i915? Signed-off-by: Pierre-Louis Bossart --- sound/x86/intel_hdmi_lpe_audio.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sound/x86/intel_hdmi_lpe_audio.c b/sound/x86/intel_hdmi_lpe_audio.c index 6ef0ff8..23e5b34 100644 --- a/sound/x86/intel_hdmi_lpe_audio.c +++ b/sound/x86/intel_hdmi_lpe_audio.c @@ -563,7 +563,13 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev) if (pci_dev_present(cherryview_ids)) { dev_dbg(&hlpe_pdev->dev, "%s: Cherrytrail LPE - Detected\n", __func__); - ctx->had_config_offset = AUDIO_HDMI_CONFIG_C; + //ctx->had_config_offset = AUDIO_HDMI_CONFIG_C; + /* FIXME: hard-coding to CONFIG_A enables DP audio on CHT, +* how do I find out which config to use ? +* the pipe is -1 (invalid) when the notify function is called, +* so not sure how to go about this +*/ + ctx->had_config_offset = AUDIO_HDMI_CONFIG_A; } else { dev_dbg(&hlpe_pdev->dev, "%s: Baytrail LPE - Assume\n", __func__); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 1/5] drm: i915: add DP support in LPE audio mode
If DisplayPort is detected, pass flag and link rate to audio driver Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/i915_drv.h| 3 ++- drivers/gpu/drm/i915/intel_audio.c | 19 +++ drivers/gpu/drm/i915/intel_lpe_audio.c | 7 ++- include/drm/intel_lpe_audio.h | 2 ++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3e3102c..836d823 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3401,7 +3401,8 @@ int intel_lpe_audio_init(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int tmds_clk_speed); + void *eld, int port, int tmds_clk_speed, + bool dp_output, int link_rate); /* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 364f962..1645ce4 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -631,9 +631,20 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port, (int) pipe); - - intel_lpe_audio_notify(dev_priv, connector->eld, port, - crtc_state->port_clock); + switch (intel_encoder->type) { + case INTEL_OUTPUT_HDMI: + intel_lpe_audio_notify(dev_priv, connector->eld, port, + crtc_state->port_clock, + false, 0); + break; + case INTEL_OUTPUT_DP: + intel_lpe_audio_notify(dev_priv, connector->eld, port, + adjusted_mode->crtc_clock, + true, crtc_state->port_clock); + break; + default: + break; + } } /** @@ -668,7 +679,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder) acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port, (int) pipe); - intel_lpe_audio_notify(dev_priv, NULL, port, 0); + intel_lpe_audio_notify(dev_priv, NULL, port, 0, false, 0); } /** diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 27d9425..245523e 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -332,7 +332,8 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv) * Notify lpe audio driver of eld change. */ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, - void *eld, int port, int tmds_clk_speed) + void *eld, int port, int tmds_clk_speed, + bool dp_output, int link_rate) { unsigned long irq_flags; struct intel_hdmi_lpe_audio_pdata *pdata = NULL; @@ -351,12 +352,16 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, pdata->eld.port_id = port; pdata->hdmi_connected = true; + pdata->dp_output = dp_output; if (tmds_clk_speed) pdata->tmds_clock_speed = tmds_clk_speed; + if (link_rate) + pdata->link_rate = link_rate; } else { memset(pdata->eld.eld_data, 0, HDMI_MAX_ELD_BYTES); pdata->hdmi_connected = false; + pdata->dp_output = false; } if (pdata->notify_audio_lpe) diff --git a/include/drm/intel_lpe_audio.h b/include/drm/intel_lpe_audio.h index 952de05..857e0ea 100644 --- a/include/drm/intel_lpe_audio.h +++ b/include/drm/intel_lpe_audio.h @@ -38,6 +38,8 @@ struct intel_hdmi_lpe_audio_pdata { bool notify_pending; int tmds_clock_speed; bool hdmi_connected; + bool dp_output; + int link_rate; struct intel_hdmi_lpe_audio_eld eld; void (*notify_audio_lpe)(void *audio_ptr); spinlock_t lpe_audio_slock; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 3/5] ALSA: x86: intel_hdmi: set config bitfields for DP mode
These bits were set in legacy and not in upstream code, and are apparently tested for when writing a config in DP mode FIXME: is this even needed? Signed-off-by: Pierre-Louis Bossart --- sound/x86/intel_hdmi_audio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 4155b38..768e6e3 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -396,6 +396,7 @@ static int snd_intelhad_prog_audio_ctrl_v2(struct snd_pcm_substream *substream, else cfg_val.cfg_regx_v2.layout = LAYOUT1; + cfg_val.cfg_regx_v2.val_bit = 1; had_write_register(AUD_CONFIG, cfg_val.cfg_regval); return 0; } @@ -447,6 +448,7 @@ static int snd_intelhad_prog_audio_ctrl_v1(struct snd_pcm_substream *substream, } + cfg_val.cfg_regx.val_bit = 1; had_write_register(AUD_CONFIG, cfg_val.cfg_regval); return 0; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode
Enable chicken bit on LPE mode setup and unmute amp on notification FIXME: should these two phases done somewhere else? Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/i915_reg.h| 12 drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++ 2 files changed, 39 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a9ffc8d..ee90f64 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2061,6 +2061,18 @@ enum skl_disp_power_wells { #define I915_HDMI_LPE_AUDIO_BASE (VLV_DISPLAY_BASE + 0x65000) #define I915_HDMI_LPE_AUDIO_SIZE 0x1000 +/* DisplayPort Audio w/ LPE */ +#define CHICKEN_BIT_DBG_ENABLE (1 << 0) +#define AMP_UNMUTE (1 << 1) +#define AUD_CHICKEN_BIT_REG0x62F38 +#define AUD_PORT_EN_B_DBG 0x62F20 +#define AUD_PORT_EN_C_DBG 0x62F28 +#define AUD_PORT_EN_D_DBG 0x62F2C +#define VLV_AUD_CHICKEN_BIT_REG_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG) +#define VLV_AUD_PORT_EN_B_DBG _MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG) +#define VLV_AUD_PORT_EN_C_DBG _MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG) +#define VLV_AUD_PORT_EN_D_DBG _MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_D_DBG) + #define GEN6_BSD_RNCID _MMIO(0x12198) #define GEN7_FF_THREAD_MODE_MMIO(0x20a0) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 245523e..b3134ef 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv) goto err_free_irq; } + /* Enable DPAudio debug bits by default */ + if (IS_CHERRYVIEW(dev_priv)) { + u32 chicken_bit; + + chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG); + I915_WRITE(VLV_AUD_CHICKEN_BIT_REG, + chicken_bit | CHICKEN_BIT_DBG_ENABLE); + } + return 0; err_free_irq: irq_free_desc(dev_priv->lpe_audio.irq); @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, pdata->tmds_clock_speed = tmds_clk_speed; if (link_rate) pdata->link_rate = link_rate; + + if (dp_output) { /* unmute the amp */ + u32 audio_enable; + + if (port == PORT_B) { + audio_enable = I915_READ(VLV_AUD_PORT_EN_B_DBG); + I915_WRITE(VLV_AUD_PORT_EN_B_DBG, + audio_enable & ~AMP_UNMUTE); + } else if (port == PORT_C) { + audio_enable = I915_READ(VLV_AUD_PORT_EN_C_DBG); + I915_WRITE(VLV_AUD_PORT_EN_C_DBG, + audio_enable & ~AMP_UNMUTE); + } else if (port == PORT_D) { + audio_enable = I915_READ(VLV_AUD_PORT_EN_D_DBG); + I915_WRITE(VLV_AUD_PORT_EN_D_DBG, + audio_enable & ~AMP_UNMUTE); + } + } } else { memset(pdata->eld.eld_data, 0, HDMI_MAX_ELD_BYTES); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 2/5] ALSA: x86: intel_hdmi: add definitions and logic for DP audio
Imported from legacy patches Note: the new code doesn't assume a modified ELD but an explicit notification that DP is present. It appears that the i915 code does change the ELD so we could use the ELD-based tests to check for DP audio Signed-off-by: Pierre-Louis Bossart --- sound/x86/intel_hdmi_audio.c | 172 +-- sound/x86/intel_hdmi_audio.h | 8 +- sound/x86/intel_hdmi_lpe_audio.c | 36 +++- sound/x86/intel_hdmi_lpe_audio.h | 29 +++ 4 files changed, 215 insertions(+), 30 deletions(-) diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index f301554..4155b38 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -548,6 +548,7 @@ void had_build_channel_allocation_map(struct snd_intelhad *intelhaddata) } had_get_caps(HAD_GET_ELD, &intelhaddata->eeld); + had_get_caps(HAD_GET_DP_OUTPUT, &intelhaddata->dp_output); pr_debug("eeld.speaker_allocation_block = %x\n", intelhaddata->eeld.speaker_allocation_block); @@ -685,7 +686,7 @@ static void snd_intelhad_prog_dip_v1(struct snd_pcm_substream *substream, /*Calculte the byte wide checksum for all valid DIP words*/ for (i = 0; i < BYTES_PER_WORD; i++) - checksum += (INFO_FRAME_WORD1 >> i*BITS_PER_BYTE) & MASK_BYTE0; + checksum += (HDMI_INFO_FRAME_WORD1 >> i*BITS_PER_BYTE) & MASK_BYTE0; for (i = 0; i < BYTES_PER_WORD; i++) checksum += (frame2.fr2_val >> i*BITS_PER_BYTE) & MASK_BYTE0; for (i = 0; i < BYTES_PER_WORD; i++) @@ -693,7 +694,7 @@ static void snd_intelhad_prog_dip_v1(struct snd_pcm_substream *substream, frame2.fr2_regx.chksum = -(checksum); - had_write_register(AUD_HDMIW_INFOFR, INFO_FRAME_WORD1); + had_write_register(AUD_HDMIW_INFOFR, HDMI_INFO_FRAME_WORD1); had_write_register(AUD_HDMIW_INFOFR, frame2.fr2_val); had_write_register(AUD_HDMIW_INFOFR, frame3.fr3_val); @@ -722,28 +723,35 @@ static void snd_intelhad_prog_dip_v2(struct snd_pcm_substream *substream, union aud_info_frame2 frame2 = {.fr2_val = 0}; union aud_info_frame3 frame3 = {.fr3_val = 0}; u8 checksum = 0; + u32 info_frame; int channels; channels = substream->runtime->channels; had_write_register(AUD_CNTL_ST, ctrl_state.ctrl_val); - frame2.fr2_regx.chnl_cnt = substream->runtime->channels - 1; + if (intelhaddata->dp_output) { + info_frame = DP_INFO_FRAME_WORD1; + frame2.fr2_val = 1; + } else { + info_frame = HDMI_INFO_FRAME_WORD1; + frame2.fr2_regx.chnl_cnt = substream->runtime->channels - 1; - frame3.fr3_regx.chnl_alloc = snd_intelhad_channel_allocation( - intelhaddata, channels); + frame3.fr3_regx.chnl_alloc = snd_intelhad_channel_allocation( + intelhaddata, channels); - /*Calculte the byte wide checksum for all valid DIP words*/ - for (i = 0; i < BYTES_PER_WORD; i++) - checksum += (INFO_FRAME_WORD1 >> i*BITS_PER_BYTE) & MASK_BYTE0; - for (i = 0; i < BYTES_PER_WORD; i++) - checksum += (frame2.fr2_val >> i*BITS_PER_BYTE) & MASK_BYTE0; - for (i = 0; i < BYTES_PER_WORD; i++) - checksum += (frame3.fr3_val >> i*BITS_PER_BYTE) & MASK_BYTE0; + /*Calculte the byte wide checksum for all valid DIP words*/ + for (i = 0; i < BYTES_PER_WORD; i++) + checksum += (info_frame >> i*BITS_PER_BYTE) & MASK_BYTE0; + for (i = 0; i < BYTES_PER_WORD; i++) + checksum += (frame2.fr2_val >> i*BITS_PER_BYTE) & MASK_BYTE0; + for (i = 0; i < BYTES_PER_WORD; i++) + checksum += (frame3.fr3_val >> i*BITS_PER_BYTE) & MASK_BYTE0; - frame2.fr2_regx.chksum = -(checksum); + frame2.fr2_regx.chksum = -(checksum); + } - had_write_register(AUD_HDMIW_INFOFR_v2, INFO_FRAME_WORD1); + had_write_register(AUD_HDMIW_INFOFR_v2, info_frame); had_write_register(AUD_HDMIW_INFOFR_v2, frame2.fr2_val); had_write_register(AUD_HDMIW_INFOFR_v2, frame3.fr3_val); @@ -839,6 +847,85 @@ int snd_intelhad_read_len(struct snd_intelhad *intelhaddata) return retval; } +static int had_calculate_maud_value(u32 aud_samp_freq, u32 link_rate) +{ + u32 maud_val; + + /* Select maud according to DP 1.2 spec*/ + if (link_rate == DP_2_7_GHZ) { + switch (aud_samp_freq) { + case AUD_SAMPLE_RATE_32: + maud_val = AUD_SAMPLE_RATE_32_DP_2_7_MAUD_VAL; + break; +
[Intel-gfx] [RFC PATCH 0/5] DisplayPort Audio on Cherrytrail
The following patches enable DisplayPort Audio on Cherrytrail machines when applied on top of Takashi's topic/intel-lpe-audio branch (tested on Zotac PI330) There are a couple of opens where I could use some help: - is it necessary to set a valid_bit which is used only for DP audio? - is the sequence to set the chicken bits and unmute the amplifier ok or can it be improved by being moved somewhere else in the i915 driver? - the register offset to be used by the audio driver depends on a combination of port/pipe/output type. Do we need to get access to the pipe information and when is it available (initial trials showed the pipe is still invalid when the audio notification happens) Feedback welcome! Pierre-Louis Bossart (5): drm: i915: add DP support in LPE audio mode ALSA: x86: intel_hdmi: add definitions and logic for DP audio ALSA: x86: intel_hdmi: set config bitfields for DP mode drm: i915: add DisplayPort amp unmute for LPE audio mode ALSA: x86: hdmi: hack to enable DP audio on CHT drivers/gpu/drm/i915/i915_drv.h| 3 +- drivers/gpu/drm/i915/i915_reg.h| 12 +++ drivers/gpu/drm/i915/intel_audio.c | 19 +++- drivers/gpu/drm/i915/intel_lpe_audio.c | 34 ++- include/drm/intel_lpe_audio.h | 2 + sound/x86/intel_hdmi_audio.c | 174 - sound/x86/intel_hdmi_audio.h | 8 +- sound/x86/intel_hdmi_lpe_audio.c | 44 - sound/x86/intel_hdmi_lpe_audio.h | 29 ++ 9 files changed, 288 insertions(+), 37 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 0/5] Add support for Legacy HDMI audio drivers
On 1/25/17 3:21 PM, Takashi Iwai wrote: On Tue, 24 Jan 2017 23:57:48 +0100, Jerome Anand wrote: This patch series has only been tested on hardware with a single HDMI connector/pipe and additional work may be needed for newer machines with 2 connectors BTW, I have such a machine, CHV with two DP outputs. If you have a test patch (not necessarily in a mergeable form), I'll be glad to test. This will make me easier to work on further cleanups. I also have a CHT machine with 1 DP and 1 HDMI connector. I'll start working on DP once I manage to find some time. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 2/5] drm/i915: Add support for audio driver notifications
#include @@ -630,6 +631,10 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port, (int) pipe); + + if (HAS_LPE_AUDIO(dev_priv)) + intel_lpe_audio_notify(dev_priv, connector->eld, port, + crtc_state->port_clock); Seems unnecessary to check for HAS_LPE_AUDIO (which you'll change to dev_priv->lpe_audio.platdev, right ;) both in the caller and callee. Pick one. If we test inside of the function, it'd mean an unconditional jump to test a feature that exists on only two platforms out of the dozen or so that this i915 driver handles. No objection to do the change but is this really desired? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v4 4/5] ALSA: x86: hdmi: Add audio support for BYT and CHT
On 1/20/17 5:15 AM, Takashi Iwai wrote: On Fri, 20 Jan 2017 23:22:31 +0100, Jerome Anand wrote: + had_ops_v1 = had_ops_v1;/* unused */ Until now I didn't realize that the whole v1 stuff is never used in the current patchset. Will it be ever used in future? If not, can't we clean it up? It's a bunch of codes, including the messy union definitions. If there is no v3 or whatever, we can even get rid of the whole indirect calls. And if v1 (and the indirect ops calls) should be kept, actually what is the difference between v1 and v2, why both implementations do exist? Please elaborate in comments. v1 refers to Medfield/Clovertrail, v2 to Baytrail/CHT. The differences are minor and centered on different register definitions or additional features/bug corrections. We left the v1 code in so far but we could probably remove it since it's not tested anyway. The question is if we remove this v1 code and indirect calls now or later, I was planning to add DP audio support and making more changes would make the integration more difficult. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH V2 7/7] ALSA: x86: hdmi: continue playback even when display resolution changes
On 1/6/17 7:21 PM, Jerome Anand wrote: When the display resolution changes, the drm disables the display pipes due to which audio rendering stops. At this time, we need to ensure the existing audio pointers and buffers are cleared out so that the playback can restarted once the display pipe is enabled with a different N/CTS values Add comment that this patch series has only been tested on hardware with a single HDMI connector/pipe and additional work may be needed for newer machines with 2 connectors. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand --- sound/x86/intel_hdmi_audio.c | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 91efbeb..f4042f8 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -43,6 +43,7 @@ static DEFINE_MUTEX(had_mutex); static int hdmi_card_index = SNDRV_DEFAULT_IDX1; static char *hdmi_card_id = SNDRV_DEFAULT_STR1; static struct snd_intelhad *had_data; +static int underrun_count; module_param_named(index, hdmi_card_index, int, 0444); MODULE_PARM_DESC(index, @@ -1114,6 +1115,7 @@ static int snd_intelhad_open(struct snd_pcm_substream *substream) intelhaddata = snd_pcm_substream_chip(substream); had_stream = intelhaddata->private_data; runtime = substream->runtime; + underrun_count = 0; pm_runtime_get(intelhaddata->dev); @@ -1503,10 +1505,23 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( buf_id = intelhaddata->curr_buf % 4; had_read_register(AUD_BUF_A_LENGTH + (buf_id * HAD_REG_WIDTH), &t); - if (t == 0) { - pr_debug("discovered buffer done for buf %d\n", buf_id); - /* had_process_buffer_done(intelhaddata); */ + + if ((t == 0) || (t == ((u32)-1L))) { + underrun_count++; + pr_debug("discovered buffer done for buf %d, count = %d\n", + buf_id, underrun_count); + + if (underrun_count > (HAD_MIN_PERIODS/2)) { + pr_debug("assume audio_codec_reset, underrun = %d - do xrun\n", + underrun_count); + underrun_count = 0; + return SNDRV_PCM_POS_XRUN; + } + } else { + /* Reset Counter */ + underrun_count = 0; } + t = intelhaddata->buf_info[buf_id].buf_size - t; if (intelhaddata->stream_info.buffer_rendered) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH V2 6/7] ALSA: x86: hdmi: Fixup some monitor
On 1/6/17 7:21 PM, Jerome Anand wrote: This change was given to Canonical apparently to fix an issue with on some monitor brand. It's not clear what this patch does but it doesn't seem to have side effects. From Takashi:please fold into the main patch and give the comments in the code instead. Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand --- sound/x86/intel_hdmi_audio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index d2036bc..91efbeb 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -337,6 +337,7 @@ static void snd_intelhad_reset_audio_v2(u8 reset) static int had_prog_status_reg(struct snd_pcm_substream *substream, struct snd_intelhad *intelhaddata) { + union aud_cfg cfg_val = {.cfg_regval = 0}; union aud_ch_status_0 ch_stat0 = {.status_0_regval = 0}; union aud_ch_status_1 ch_stat1 = {.status_1_regval = 0}; int format; @@ -347,6 +348,7 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream, IEC958_AES0_NONAUDIO)>>1; ch_stat0.status_0_regx.clk_acc = (intelhaddata->aes_bits & IEC958_AES3_CON_CLOCK)>>4; + cfg_val.cfg_regx.val_bit = ch_stat0.status_0_regx.lpcm_id; switch (substream->runtime->rate) { case AUD_SAMPLE_RATE_32: @@ -426,7 +428,6 @@ int snd_intelhad_prog_audio_ctrl_v2(struct snd_pcm_substream *substream, else cfg_val.cfg_regx_v2.layout = LAYOUT1; - cfg_val.cfg_regx_v2.val_bit = 1; had_write_register(AUD_CONFIG, cfg_val.cfg_regval); return 0; } @@ -482,7 +483,6 @@ int snd_intelhad_prog_audio_ctrl_v1(struct snd_pcm_substream *substream, } - cfg_val.cfg_regx.val_bit = 1; had_write_register(AUD_CONFIG, cfg_val.cfg_regval); return 0; } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH V2 5/7] ALSA: x86: hdmi: Improve position reporting
On 1/6/17 7:21 PM, Jerome Anand wrote: Use a hw register to calculate sub-period position reports. This makes PulseAudio happier. From Takashi: There is no big merit to keep this a separate patch. Please fold into the main patch. You can put some comment in the code for explanation. Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand --- sound/x86/intel_hdmi_audio.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index d7b57658..d2036bc 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1489,6 +1489,8 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( { struct snd_intelhad *intelhaddata; u32 bytes_rendered = 0; + u32 t; + int buf_id; /* pr_debug("snd_intelhad_pcm_pointer called\n"); */ @@ -1499,6 +1501,14 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( return SNDRV_PCM_POS_XRUN; } + buf_id = intelhaddata->curr_buf % 4; + had_read_register(AUD_BUF_A_LENGTH + (buf_id * HAD_REG_WIDTH), &t); + if (t == 0) { + pr_debug("discovered buffer done for buf %d\n", buf_id); + /* had_process_buffer_done(intelhaddata); */ + } + t = intelhaddata->buf_info[buf_id].buf_size - t; + if (intelhaddata->stream_info.buffer_rendered) div_u64_rem(intelhaddata->stream_info.buffer_rendered, intelhaddata->stream_info.ring_buf_size, @@ -1506,7 +1516,7 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( intelhaddata->stream_info.buffer_ptr = bytes_to_frames( substream->runtime, - bytes_rendered); + bytes_rendered + t); return intelhaddata->stream_info.buffer_ptr; } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH V2 3/7] ALSA: add shell for Intel HDMI LPE audio driver
Minor misses here as well. On 1/6/17 7:21 PM, Jerome Anand wrote: On Baytrail and Cherrytrail, HDaudio may be fused out or disabled by the BIOS. This driver enables an alternate path to the i915 display registers and DMA. Although there is no hardware path between i915 display and LPE/SST audio clusters, this HDMI capability is referred to in the documentation as "HDMI LPE Audio" so we keep the name for consistency. There is no hardware path or control dependencies with the LPE/SST DSP functionality. The hdmi-lpe-audio driver will be probed when the i915 driver creates a child platform device. Since this driver is neither SoC nor PCI, a new x86 folder is added Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand Change the commit title to remove 'shell', e.g. 'add BYT/CHT-T HDMI LPE audio driver' and mention that indirect calls will be removed later (to help with DP integration) diff --git a/sound/x86/Kconfig b/sound/x86/Kconfig new file mode 100644 index 000..e9297d0 --- /dev/null +++ b/sound/x86/Kconfig @@ -0,0 +1,16 @@ +menuconfig SND_X86 + tristate "X86 sound devices" + ---help--- + + X86 sound devices that don't fall under SoC or PCI categories + +if SND_X86 + +config HDMI_LPE_AUDIO + tristate "HDMI audio without HDaudio on Intel Atom platforms" + depends on DRM_I915 + default n + help +Choose this option to support HDMI LPE Audio mode + +endif # SND_X86 diff --git a/sound/x86/Makefile b/sound/x86/Makefile new file mode 100644 index 000..baa6333 --- /dev/null +++ b/sound/x86/Makefile @@ -0,0 +1,6 @@ +ccflags-y += -Idrivers/gpu/drm/i915 from Takashi: Is it just for intel_lpe_audio.h? Then rather put intel_lpe_audio.h to include/drm. JA: OK + +snd-hdmi-lpe-audio-objs += \ + intel_hdmi_lpe_audio.o + +obj-$(CONFIG_HDMI_LPE_AUDIO) += snd-hdmi-lpe-audio.o diff --git a/sound/x86/intel_hdmi_lpe_audio.c b/sound/x86/intel_hdmi_lpe_audio.c new file mode 100644 index 000..61347ab --- /dev/null +++ b/sound/x86/intel_hdmi_lpe_audio.c @@ -0,0 +1,623 @@ +/* + * intel_hdmi_lpe_audio.c - Intel HDMI LPE audio driver for Atom platforms + * + * Copyright (C) 2016 Intel Corp + * Authors: + * Jerome Anand + * Aravind Siddappaji + * ~~ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * ~~ + */ + +#define pr_fmt(fmt)"hdmi_lpe_audio: " fmt From Takashi: Better to use dev_*() variant. JA: OK +static inline int hdmi_get_eld(void *eld) +{ + memcpy(eld, (void *)&hlpe_eld, sizeof(hlpe_eld)); + + { + int i; + uint8_t *eld_data = (uint8_t *)&hlpe_eld; + + pr_debug("hdmi_get_eld:\n{{"); + + for (i = 0; i < sizeof(hlpe_eld); i++) + pr_debug("0x%x, ", eld_data[i]); + + pr_debug("}}\n"); + } + return 0; From Takashi: There is a hexdump debug print helper, too. JA: OK ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH V2 2/7] drm/i915: Add support for audio driver notifications
Same here, missing fixes on agreed comments? On 1/6/17 7:21 PM, Jerome Anand wrote: Notifiations like mode change, hot plug and edid to the audio driver are added. This is inturn used by the audio driver for its functionality. A new interface file capturing the notifications needed by the audio driver is added Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand --- drivers/gpu/drm/i915/i915_drv.h| 3 +++ drivers/gpu/drm/i915/intel_audio.c | 8 ++ drivers/gpu/drm/i915/intel_hdmi.c | 1 + drivers/gpu/drm/i915/intel_lpe_audio.c | 46 ++ include/drm/intel_lpe_audio.h | 1 + 5 files changed, 59 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2f8165e..263bc48 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3634,6 +3634,9 @@ int intel_lpe_audio_setup(struct drm_i915_private *dev_priv); void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); bool intel_lpe_audio_detect(struct drm_i915_private *dev_priv); +void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, + void *eld, int port, int tmds_clk_speed, + bool connected); /* intel_i2c.c */ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 16c2027..aeb37c2 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "intel_drv.h" #include @@ -630,6 +631,10 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder, if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port, (int) pipe); + + if (HAS_LPE_AUDIO(dev_priv)) + intel_lpe_audio_notify(dev_priv, connector->eld, port, + crtc_state->port_clock, true); } /** @@ -663,6 +668,9 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder) if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port, (int) pipe); + + if (HAS_LPE_AUDIO(dev_priv)) + intel_lpe_audio_notify(dev_priv, NULL, port, 0, false); } From Ville: The entire 'connected' parameter seems superfluous. Also why aren't we passing 'pipe' along here? How is the audio driver supposed to find the right thing to use? /** diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 0bcfead..377584e1 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -36,6 +36,7 @@ #include #include "intel_drv.h" #include +#include #include "i915_drv.h" static struct drm_device *intel_hdmi_to_dev(struct intel_hdmi *intel_hdmi) diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 05f5e4e..2a3c1e8 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -353,3 +353,49 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } + + +/** + * intel_lpe_audio_notify() - notify lpe audio event + * audio driver and i915 + * @dev_priv: the i915 drm device private data + * @eld : ELD data + * @port: port id + * @tmds_clk_speed: tmds clock frequency in Hz + * @connected: hdmi connected/disconnected + * + * Notify lpe audio driver of eld change. + */ +void intel_lpe_audio_notify(struct drm_i915_private *dev_priv, + void *eld, int port, int tmds_clk_speed, + bool connected) +{ + unsigned long irq_flags; + struct intel_hdmi_lpe_audio_pdata *pdata = NULL; + + if (!HAS_LPE_AUDIO(dev_priv)) + return; + + pdata = dev_get_platdata( + &(dev_priv->lpe_audio.platdev->dev)); + + spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags); + + if (eld != NULL) { + memcpy(pdata->eld.eld_data, eld, + HDMI_MAX_ELD_BYTES); + pdata->eld.port_id = port; + + if (tmds_clk_speed) + pdata->tmds_clock_speed = tmds_clk_speed; + } From Takashi: If eld==NULL means that no ELD is found (or disconnected), it's better to clear pdata eld as well, so that we don't leak the previous ELD. JA: OK +
Re: [Intel-gfx] [alsa-devel] [PATCH V2 1/7] drm/i915: setup bridge for HDMI LPE audio driver
Thanks for the update Jerome. Looks like you missed a couple of agreed comments from the last round? On 1/6/17 7:21 PM, Jerome Anand wrote: Enable support for HDMI LPE audio mode on Baytrail and Cherrytrail when HDaudio controller is not detected Setup minimum required resources during i915_driver_load: 1. Create a platform device to share MMIO/IRQ resources 2. Make the platform device child of i915 device for runtime PM. 3. Create IRQ chip to forward HDMI LPE audio irqs. HDMI LPE audio driver (a standalone sound driver) probes the LPE audio device and creates a new sound card. Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand --- Documentation/gpu/i915.rst | 9 + drivers/gpu/drm/i915/Makefile | 3 + drivers/gpu/drm/i915/i915_drv.c| 8 +- drivers/gpu/drm/i915/i915_drv.h| 15 ++ drivers/gpu/drm/i915/i915_irq.c| 16 ++ drivers/gpu/drm/i915/i915_reg.h| 3 + drivers/gpu/drm/i915/intel_lpe_audio.c | 355 + include/drm/intel_lpe_audio.h | 45 + 8 files changed, 452 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_lpe_audio.c create mode 100644 include/drm/intel_lpe_audio.h diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst index 104296d..bd9b767 100644 --- a/Documentation/gpu/i915.rst +++ b/Documentation/gpu/i915.rst @@ -225,6 +225,15 @@ Display PLLs .. kernel-doc:: drivers/gpu/drm/i915/intel_dpll_mgr.h :internal: +intel hdmi lpe audio support + + +.. kernel-doc:: drivers/gpu/drm/i915/intel_lpe_audio.c + :doc: LPE Audio integration for HDMI or DP playback + +.. kernel-doc:: drivers/gpu/drm/i915/intel_lpe_audio.c + :internal: + Memory Management and Command Submission diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 5196509..2bca239 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -127,6 +127,9 @@ i915-y += intel_gvt.o include $(src)/gvt/Makefile endif +# LPE Audio for VLV and CHT +i915-y += intel_lpe_audio.o + obj-$(CONFIG_DRM_I915) += i915.o CFLAGS_i915_trace_points.o := -I$(src) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 4d22b4b..70d728b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1131,7 +1131,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) if (IS_GEN5(dev_priv)) intel_gpu_ips_init(dev_priv); - i915_audio_component_init(dev_priv); + if (intel_lpe_audio_init(dev_priv) < 0) + i915_audio_component_init(dev_priv); /* * Some ports require correctly set-up hpd registers for detection to @@ -1149,7 +1150,10 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) */ static void i915_driver_unregister(struct drm_i915_private *dev_priv) { - i915_audio_component_cleanup(dev_priv); + if (HAS_LPE_AUDIO(dev_priv)) + intel_lpe_audio_teardown(dev_priv); + else + i915_audio_component_cleanup(dev_priv); intel_gpu_ips_teardown(); acpi_video_unregister(); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7b43662..2f8165e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2460,6 +2460,12 @@ struct drm_i915_private { /* Used to save the pipe-to-encoder mapping for audio */ struct intel_encoder *av_enc_map[I915_MAX_PIPES]; + /* necessary resource sharing with HDMI LPE audio driver. */ + struct { + struct platform_device *platdev; + int irq; + } lpe_audio; + /* * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch * will be rejected. Instead look for a better place. @@ -2859,6 +2865,8 @@ intel_info(const struct drm_i915_private *dev_priv) #define HAS_POOLED_EU(dev_priv)((dev_priv)->info.has_pooled_eu) +#define HAS_LPE_AUDIO(dev_priv) ((dev_priv)->lpe_audio.platdev != NULL) + #define INTEL_PCH_DEVICE_ID_MASK 0xff00 #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00 #define INTEL_PCH_CPT_DEVICE_ID_TYPE 0x1c00 @@ -3620,6 +3628,13 @@ extern int i915_restore_state(struct drm_i915_private *dev_priv); void i915_setup_sysfs(struct drm_i915_private *dev_priv); void i915_teardown_sysfs(struct drm_i915_private *dev_priv); +/* i915_lpe_audio.c */ +int intel_lpe_audio_init(struct drm_i915_private *dev_priv); +int intel_lpe_audio_setup(struct drm_i915_private *dev_priv); +void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv); +void intel_lpe_audio_irq_handler(struct drm_i915_private *dev_priv); +bool intel_lpe_audio_detect(struct drm_i915_private *dev_priv); + /* intel_i2c.c */ extern int intel_setu
Re: [Intel-gfx] [alsa-devel] [PATCH 7/7] ALSA: x86: hdmi: continue playback even when display resolution changes
This change itself is OK, but this made me wonder about the driver implementation: the current MAX_PB=1 is the hardware limitation or the soft limitation? That is, can't we play back two streams when we connect two HDMI monitors? Two streams was never validated from hardware per se. So setting the limitation in software. To be clearer, the IP itself does support 2 streams going to two different pipes in parallel but we never had a validation platform for Atom gizmos with two connectors and no use case for tablets/phone. Never say never as usual, now we see CHT mini-PC devices with 2 connectors, either both DP or DP+HDMI so we'll have to see how things go when both are enabled. It's on the TODO list. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 3/7] ALSA: add shell for Intel HDMI LPE audio driver
Subject: Re: [PATCH 3/7] ALSA: add shell for Intel HDMI LPE audio driver Why do we need a "shell" and indirect calls? This is a small driver set, so it's not utterly unacceptable, but it still makes things a bit more complicated than necessary, so I'd like to understand the idea behind it. The 'shell' comes from an early prototype which didn't do much except create the device. The title of the commit should be changed if it threw you off. This solution does not use component interface to talk between Audio and i915 driver. The reason for that is the HDMI audio device is created by i915 driver with only one callback pointer passed as pdata. Unlike HDA, the HDMI audio driver does not get loaded if the i915 does not create child device. Since there is only one callback needed we are not using the component interface to make things simpler. This design was co-worked with i915 folks to makes interaction simpler. The interaction between i915 and audio is simple, yes, it just exposes a few things, mmio ptr, irq, etc. But still I don't understand why multiple layers of indirect accesses are needed *inside* lpe audio driver itself. For example, suspend/resume action is cascaded to yet another ops. I would understand if this "shell" provides a few thin accessors to the raw i915 registers. But another layering over a single driver implementation makes little sense in regard of abstraction. (If there were multiple class inherits, it's a different story, of course.) No disagreement here Takashi. I was planning to remove this abstraction in a second incremental pass that would only be on the audio side, for now keeping this layer would allow me to add the DisplayPort changes faster. If we simplify this now then I will have so many differences with the legacy code it'll be a nightmare. I have no emotional attachment to the legacy but it's just pragmatic to avoid changing everything at once. As you also mentioned it, this second pass could also reuse all the ELD parsing that is still duplicated. These HDMI patches are not a single-shot contribution, you'll have updates coming your way. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Add support for audio driver notifications
On 12/14/16 7:13 AM, Takashi Iwai wrote: On Wed, 14 Dec 2016 13:55:52 +0100, Daniel Vetter wrote: Only noticed it here, but why again do we need to re-roll our intel-only hdmi/eld notification? The one we have for hda is somewhat justified since it went in at roughly the same time as the new shared one across a bunch of soc. But this one here is just a reinvented wheel. And yes this code has been hanging around internally for years, but that's kinda not a good excuse. Obviously this comment applies to patch 1 too. Yeah, a unified common interface would be better, really. I'm basically OK with the current implementation, though, as long as it works. But if we can sort it out quickly, it's better. That said, we may reuse i915_audio_component stuff -- or at least, reuse the object used there without actual component binding (the lpe driver doesn't need the component because it's a strong dependency unlike HD-audio case), and just add a few more fields there. Instead of exposing the resource, we can provide the register accessor there, too. It's just an idea, so not necessarily serious taken. But we can think of unification more easily now than later. BTW, now one thing came to my mind: don't we need the power control (pm and power well domain) when accessing from the sound driver side? The HD-audio component object has the gfx side callback points for such. The code hasn't actually been around for years. We threw away the legacy driver and restarted the i915 integration pretty much from scratch using the feedback on the intel-gfx mailing list, specifically Jesse Barnes and Ville Syrjala, with only basic programming sequences and register definitions kept on the audio side. The volume of code required on the i915 side was reduced by probably 50%, with useless stuff taken out left and right. We're down to ~500 lines changed on the i915 side with about 400 just for the interface in the new intel_lpe_audio.c file. The initial idea for the rework was to use the component framework. Then during the initial review it was suggested that component framework wasn't that great and that the design should follow the example of the VED integration with a subdevice created and pdata used to communicate between the i915 and audio sides. I threw the initial component framework patches away and we started in this direction. There is no power domain here and in general no issue with probe happening independently at different times, the subdevice/pdata is simple enough to model the hardware. If there is an agreement to push for a unified interface, that's fine and I will commit to port the code as needed. We can modify the interface, all that's needed is to provide the ELD and let the audio side program a window of register space on the i915 side. But quite frankly I'd rather see the code being merged first to expand the userbase, today HDMI can only be enabled with out-of-tree patches pulled from my github ported on the last 6 kernel versions. I also plan to work an update on DisplayPort support since there are CHT devices on the market now. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 1/7] drm/i915: setup bridge for HDMI LPE audio driver
>>> +static void lpe_audio_irq_unmask(struct irq_data *d) { >>> + struct drm_i915_private *dev_priv = d->chip_data; >>> + unsigned long irqflags; >>> + u32 val = (I915_LPE_PIPE_A_INTERRUPT | >>> + I915_LPE_PIPE_B_INTERRUPT); >> >> PIPE_C missing? >> > > No PIPE_C on vlv Good catch Daniel. We initially had PIPE_C everyhwere, thinking that it's harmless on Baytrail. Ville had a comment that there was no PIPE_C on BYT so we removed it for Baytrail-only code, here it makes no sense to me. we have the same problem in lpe_audio_irq_mask >> Besides the few bikesheds seems all reasonable, with those addressed >> >> Acked-by: Daniel Vetter >> > > Thanks > >> ... for merging through sound tree. Since we're super early in 4.11 a topic >> branch for me to pull in to avoid sync headaches would be good. Thanks for the reviews and ack, we'll fix the comments shortly. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH 5/7] ALSA: x86: hdmi: Improve position reporting
On 12/14/16 8:36 AM, Takashi Iwai wrote: On Wed, 14 Dec 2016 15:09:20 +0100, Pierre-Louis Bossart wrote: On 12/14/16 6:57 AM, Takashi Iwai wrote: On Mon, 12 Dec 2016 19:10:41 +0100, Jerome Anand wrote: Use a hw register to calculate sub-period position reports. This makes PulseAudio happier. Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand There is no big merit to keep this a separate patch. Please fold into the main patch. You can put some comment in the code for explanation. The reason we kept this patch and the next separate is purely procedural: they were contributed by Canonical in their Baytrail Compute Stick kernel, and we didn't want to squash this blindly, especially since David has gone silent. I don't mind folding this code into the Intel patches but I wasn't sure this was appropriate or even allowed. Merging the code must be OK, that's the point of the original patch having David's sign-off. (If the code merge isn't allowed, how can we work on the kernel tree at all? ;) It'd be better, though, to mention about the merged code and his sign-off portion in the commit log. ok, that's fine. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] ALSA: x86: hdmi: Improve position reporting
On 12/14/16 6:57 AM, Takashi Iwai wrote: On Mon, 12 Dec 2016 19:10:41 +0100, Jerome Anand wrote: Use a hw register to calculate sub-period position reports. This makes PulseAudio happier. Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart Signed-off-by: Jerome Anand There is no big merit to keep this a separate patch. Please fold into the main patch. You can put some comment in the code for explanation. The reason we kept this patch and the next separate is purely procedural: they were contributed by Canonical in their Baytrail Compute Stick kernel, and we didn't want to squash this blindly, especially since David has gone silent. I don't mind folding this code into the Intel patches but I wasn't sure this was appropriate or even allowed. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/dp/mst: fix kernel oops when turning off secondary monitor
On 12/5/16 3:39 PM, Pandiyan, Dhinakaran wrote: On Mon, 2016-12-05 at 08:02 +, Chris Wilson wrote: On Sun, Dec 04, 2016 at 07:31:18PM -0600, Pierre-Louis Bossart wrote: 100% reproducible issue found on SKL SkullCanyon NUC with two external DP daisy-chained monitors in DP/MST mode. When turning off or changing the input of the second monitor the machine stops with a kernel oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly. This issue is traced to an inconsistent control flow in drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the same time as'req_payload.num_slots' is set to zero, but the pointer is dereferenced even when req_payload.num_slot is zero. Fix by adding test condition to make sure both variables are used consistently. This removes the kernel oops. There are still annoying cases where the primary display goes black when the secondary display is turned off but it can be recovered from by playing with the monitor inputs and power buttons. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990 Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index aa64448..5481fde 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1815,7 +1815,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr) drm_dp_create_payload_step1(mgr, mgr->proposed_vcpis[i]->vcpi, &req_payload); mgr->payloads[i].num_slots = req_payload.num_slots; mgr->payloads[i].vcpi = req_payload.vcpi; - } else if (mgr->payloads[i].num_slots) { + } else if (mgr->payloads[i].num_slots && port != NULL) { mgr->payloads[i].num_slots = 0; drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]); s/port->vcpi.vcpi/mgr->payloads[i].vcpi/ here looks to be the correct fix. -Chris Hmm, not sure if that is the correct fix either. With port = NULL, doesn't look like we send drm_dp_payload_send_msg(..., pbn = 0). Although, we do update the payload table via DPCD Also, if port is set to NULL, I wonder if we are messing up the reference counting. Because, this is done below. ... if (port) drm_dp_put_port(port); ... Chris' suggested fix works better than my initial proposal, I just sent the update as a V2. It may not be correct or complete but then someone smarter than me needs to take over. I am just an audio guy... ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/dp/mst: fix kernel oops when turning off secondary monitor
100% reproducible issue found on SKL SkullCanyon NUC with two external DP daisy-chained monitors in DP/MST mode. When turning off or changing the input of the second monitor the machine stops with a kernel oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly. This issue is traced to an inconsistent control flow in drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the same time as 'req_payload.num_slots' is set to zero, but the pointer is dereferenced even when req_payload.num_slot is zero. The problematic dereference was introduced in commit dfda0df34 ("drm/mst: rework payload table allocation to conform better") and may impact all versions since v3.18 The fix suggested by Chris Wilson removes the kernel oops and was found to work well after 10mn of monkey-testing with the second monitor power and input buttons Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990 Cc: Dave Airlie Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index aa64448..f59771d 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr) mgr->payloads[i].vcpi = req_payload.vcpi; } else if (mgr->payloads[i].num_slots) { mgr->payloads[i].num_slots = 0; - drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]); + drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]); req_payload.payload_state = mgr->payloads[i].payload_state; mgr->payloads[i].start_slot = 0; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/dp/mst: fix kernel oops when turning off secondary monitor
100% reproducible issue found on SKL SkullCanyon NUC with two external DP daisy-chained monitors in DP/MST mode. When turning off or changing the input of the second monitor the machine stops with a kernel oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly. This issue is traced to an inconsistent control flow in drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the same time as'req_payload.num_slots' is set to zero, but the pointer is dereferenced even when req_payload.num_slot is zero. Fix by adding test condition to make sure both variables are used consistently. This removes the kernel oops. There are still annoying cases where the primary display goes black when the secondary display is turned off but it can be recovered from by playing with the monitor inputs and power buttons. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990 Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index aa64448..5481fde 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1815,7 +1815,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr) drm_dp_create_payload_step1(mgr, mgr->proposed_vcpis[i]->vcpi, &req_payload); mgr->payloads[i].num_slots = req_payload.num_slots; mgr->payloads[i].vcpi = req_payload.vcpi; - } else if (mgr->payloads[i].num_slots) { + } else if (mgr->payloads[i].num_slots && port != NULL) { mgr->payloads[i].num_slots = 0; drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]); req_payload.payload_state = mgr->payloads[i].payload_state; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH v3 2/7] drm/i915: Add support for audio driver notifications
On 11/28/16 1:30 PM, Ville Syrjälä wrote: On Mon, Nov 28, 2016 at 01:13:31PM -0600, Pierre-Louis Bossart wrote: On 11/28/16 11:01 AM, Ville Syrjälä wrote: + if (pdata->notify_audio_lpe) + pdata->notify_audio_lpe( + (eld != NULL) ? &pdata->eld : NULL); + else + pdata->notify_pending = true; Still not sure why the "pending" thing is useful. Can't the audio driver just do its thing (whatever it is) unconditionally? This is added to avoid race when audio driver loads late and the notification from display has already passed. You keep saying that but I can't see it. I have seen this happen - before audio driver is loaded, codec enable completes and notification is sent to the audio driver. Since the audio callbacks are not initialized, notification gets missed. Sure. But what does the extra notification_pending flag buy us? The audio driver could just check the eld/tmds_clock/port directly. When disabling just clear the port to INVALID, eld to zero, and tmds clock to 0, and it should all be fine no? Yes, that's what is being done. Where? Notify callback will have eld to NULL and tmds to zero sent in codec_disable But the driver can look those thigns up directly as well it seems. So this whole thing is a bit of a mess on account of sharing the platform as a communication channel and also trying to pass the things as paraameters to the notify hook. I think we need to pick one or the other approach, not some mismash of both. Indeed it looks weird to have both a parameter for tmds_clock in the pdata AND the notify parameter, this can probably be cleaned-up. That said, I am not sure I completely understand the feedback that the audio driver can get all the eld/tmds/port information directly. We are trying to avoid accessing the data structures of the i915 driver. IIRC what I proposed originally didn't even expose the same structure to both sides, but that's not what we seem to have atm. Are you suggesting a scheme where the i915 driver would just provide a door-bell like notification and the audio driver would use a get_eld/tmds/port interface exposed by the i915 driver on startup and upon receiving this notification? Well, we could do it that way, or we'd do it the other way that the audio driver just calls i915 to triggers a single i915->audio notify call after the audio driver has finished its probe. Or we could do whatever we seem to have now is where the audio driver can just root around directly in the structure (at which point passing any parameters in the notify calls seems redundant as well). Looking at some older emails, i think you recommended a 'register' callback to let the audio driver signal to the i915 side it completed its initialization, with a single notify generated if needed (what you described just above as 'the other way') If you look at the path 4 of the series, Jerome was trying to implement this with a 'notify_pending' field in the platform data set by the i915 side and used during the audio driver probe + if (pdata->notify_pending) { + + pr_debug("%s: handle pending notification\n", __func__); + notify_audio_lpe(&pdata->eld); + pdata->notify_pending = false; + } Maybe an explicit handshake is more self-explanatory and safer? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH v3 2/7] drm/i915: Add support for audio driver notifications
On 11/28/16 11:01 AM, Ville Syrjälä wrote: + if (pdata->notify_audio_lpe) + pdata->notify_audio_lpe( + (eld != NULL) ? &pdata->eld : NULL); + else + pdata->notify_pending = true; Still not sure why the "pending" thing is useful. Can't the audio driver just do its thing (whatever it is) unconditionally? This is added to avoid race when audio driver loads late and the notification from display has already passed. You keep saying that but I can't see it. I have seen this happen - before audio driver is loaded, codec enable completes and notification is sent to the audio driver. Since the audio callbacks are not initialized, notification gets missed. Sure. But what does the extra notification_pending flag buy us? The audio driver could just check the eld/tmds_clock/port directly. When disabling just clear the port to INVALID, eld to zero, and tmds clock to 0, and it should all be fine no? Yes, that's what is being done. Where? Notify callback will have eld to NULL and tmds to zero sent in codec_disable But the driver can look those thigns up directly as well it seems. So this whole thing is a bit of a mess on account of sharing the platform as a communication channel and also trying to pass the things as paraameters to the notify hook. I think we need to pick one or the other approach, not some mismash of both. Indeed it looks weird to have both a parameter for tmds_clock in the pdata AND the notify parameter, this can probably be cleaned-up. That said, I am not sure I completely understand the feedback that the audio driver can get all the eld/tmds/port information directly. We are trying to avoid accessing the data structures of the i915 driver. Are you suggesting a scheme where the i915 driver would just provide a door-bell like notification and the audio driver would use a get_eld/tmds/port interface exposed by the i915 driver on startup and upon receiving this notification? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [RFC PATCH v3 1/7] drm/i915: setup bridge for HDMI LPE audio driver
On 11/24/16 7:31 AM, Ville Syrjälä wrote: +static void lpe_audio_irq_unmask(struct irq_data *d) +{ + struct drm_device *dev = d->chip_data; + struct drm_i915_private *dev_priv = to_i915(dev); + unsigned long irqflags; + u32 val = (I915_LPE_PIPE_A_INTERRUPT | + I915_LPE_PIPE_B_INTERRUPT | + I915_LPE_PIPE_C_INTERRUPT); No pipe C on vlv. Yes but does it hurt to set this bit? If the hardware is not present then there is no side effect, is there? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH v2 0/8] Add support for Legacy Hdmi audio
On 11/9/16 7:19 AM, Mark Brown wrote: On Sun, Nov 06, 2016 at 11:42:31PM -0700, Pierre-Louis Bossart wrote: I am all for convergence when it makes sense but I don't see how hdmi-codec.h provides equivalent functionality for BYT/CHT with what was suggested in this patchset -derived from VED patches- and discussed earlier with intel-gfx folks, specifically how LPE pipe interrupts are masked/unmasked and passed to the audio driver? The BYT/CHT HDMI Without knowing what these things are it's hard to comment but it does seem that if Intel has a reasonable use case for them then so too will other vendors at some point. functionality is also not modeled as an ASoC codec - which seems a dependency from the comments in hdmi-codec.h - since it's really part of the i915 hardware and exposed only as a set of registers+DMA, without any serial link interface typically needed for an external device or the internal HDAudio-display link. None of which is at all unusal. The Intel hardware really doesn't seem like the sort of special snowflake that people appear to believe it to be. I am not sure if this reply was British humor, sarcasm or a proposal? Again the hardware for Baytrail and Cherrytrail is very simple, the display subsystem exposes a DMA interface with a 4 descriptors pointing to buffers in system memory and a window of registers to enable DMA transfers and signal interrupts for DMA events. Rather than adding ALSA related code in the i915 driver, the proposal is to create a subdevice that is given access to the relevant register window and a matching driver in sound/x86 that would take care of creating a card and implementing ALSA-related initializations and buffer/periods management. The interaction between the two gfx and audio parts is based on a very limited set of pdata variables and callbacks. From the gfx perspective this minimizes the code changes and dependencies on ALSA. The hdmi-codec interface seems to assume an ASoC implementation which seems over-engineered in this case. There is no specific power management issues, no real need for cpu- or codec-dai and not physical link such as I2S/SPDIF. The only thing that seems directly useful if the pdata part which I already had. If I missed anything I am all ears. Note that I am not trying to solve HDAudio-gfx interaction, just to get audio support for baytrail and cherrytrail compute sticks in the mainline. I've been porting patches to help users since 4.4, starting from the work Canonical did and internal patches, and it gets tiring. At this point I am looking for either agreement to proceed with the solution suggested in these patches which works fine with minimal impacts to either the drm or alsa domains, or an actual useful/pragmatic counter-proposal. If there is a grand plan of transition to a unified TBD interface for all HDMI cases then I would ask that this is done in a second step after the audio functionality is added. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH v2 0/8] Add support for Legacy Hdmi audio
On 11/3/16 5:01 PM, Daniel Vetter wrote: On Sat, Oct 1, 2016 at 2:22 AM, Jerome Anand wrote: Legacy Hdmi audio drivers are added. Added support for audio/ gfx interface using irq chip framework Just discussed this with Rakesh here at LPC and also with Mark Brown and since earlier this years there's no a standard way to do this in include/sound/hdmi-codec.h. I think it'd be good to port this over to this new interface before merging. And if it makes sense (and ofc afterwards when there's time) to port over the existing i915_component.h to hdmi-codec.h. I am all for convergence when it makes sense but I don't see how hdmi-codec.h provides equivalent functionality for BYT/CHT with what was suggested in this patchset -derived from VED patches- and discussed earlier with intel-gfx folks, specifically how LPE pipe interrupts are masked/unmasked and passed to the audio driver? The BYT/CHT HDMI functionality is also not modeled as an ASoC codec - which seems a dependency from the comments in hdmi-codec.h - since it's really part of the i915 hardware and exposed only as a set of registers+DMA, without any serial link interface typically needed for an external device or the internal HDAudio-display link. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Baytrail MIPI/DSI Black when boot with HDMI connected
On 10/28/16 1:48 AM, Jani Nikula wrote: On Fri, 28 Oct 2016, Fabian Pie wrote: Hi, I'm testing Baytrail device with mipi/dsi display and hdmi output. When I boot without the hdmi connected the mipi/dsi works OK and after connecting the hdmi both mipi/dsi and hdmi display graphics OK. If I boot with the hdmi connected it's selected by the bios as the default display, Ubuntu only boots in hdmi and it works OK in this display but MIPI/DSI is black. It's listed as connected by xrandr but I can't reset in any way. I didn't find this issue in your bug list. Have you seen anything similar ? I'm using Ubuntu 16.10 with kernel 4.8.4 Probably the BIOS skips some DSI initialization in this case, and we fail to bring it up. Please file a bug over at [1], include the above description, add drm.debug=14 module parameter, attach dmesg all the way from boot for both the working and broken cases, plus /sys/kernel/debug/dri/0/i915_vbt. Same issue on Asus T100TAF (Baytrail-CR) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Screen flicker regression on Baytrail
While testing our upcoming HDMI audio patches, I experienced pretty bad screen flicker regressions on my Baytrail compute stick. This happens with both v09-rc2 and drm-intel-nightly. A quick bisect with all the audio patches removed points to the following commit: 2efb813d5388e18255c54afac77bd91acd586908 is the first bad commit commit 2efb813d5388e18255c54afac77bd91acd586908 Author: Chris Wilson Date: Thu Aug 18 17:17:06 2016 +0100 drm/i915: Fallback to using unmappable memory for scanout More logs below. I don't have the background to fix this myself and I don't see an obvious link with Baytrail but I hope someone smarter than me can help fix this -Pierre git bisect start # bad: [d8923dcfa53d59886d432a3fc430e26cb92ce86a] drm/i915: Track display alignment on VMA git bisect bad d8923dcfa53d59886d432a3fc430e26cb92ce86a # good: [03af84fe7f48937941fbb4dcd6cf66c68ae0edd4] drm/i915: Choose partial chunksize based on tile row size git bisect good 03af84fe7f48937941fbb4dcd6cf66c68ae0edd4 # good: [50349247ea807ad0950bbcedb1abb576e6a785db] drm/i915: Drop ORIGIN_GTT for untracked GTT writes git bisect good 50349247ea807ad0950bbcedb1abb576e6a785db # bad: [2efb813d5388e18255c54afac77bd91acd586908] drm/i915: Fallback to using unmappable memory for scanout git bisect bad 2efb813d5388e18255c54afac77bd91acd586908 # good: [821188778b9be2050d45490c4b2b009d51f041e0] drm/i915: Choose not to evict faultable objects from the GGTT git bisect good 821188778b9be2050d45490c4b2b009d51f041e0 # first bad commit: [2efb813d5388e18255c54afac77bd91acd586908] drm/i915: Fallback to using unmappable memory for scanout ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH v2 4/8] drm/i915: Add support for enabling/disabling hdmi audio interrupts
+/* Added for HDMI Audio */ +int i915_enable_hdmi_audio_int(struct drm_i915_private *dev_priv) +{ + unsigned long irqflags; + u32 imr, int_bit; + int pipe = -1; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + imr = I915_READ(VLV_IMR); + + if (IS_CHERRYVIEW(&dev_priv->drm)) { + pipe = PIPE_C; + int_bit = (pipe ? (I915_LPE_PIPE_B_INTERRUPT >> + ((pipe - 1) * 9)) : + I915_LPE_PIPE_A_INTERRUPT); Either parametrize the I915_LPE_PIPE_INTERRUPT macro, or just have eg. a switch here. ok But the bigger issue here is the mess with selecting the right bit. I assume it should either depend on the pipe or port. I can't figure out what is going on here. And actually I don't understand why we even need this function. The irqchip should take care to unmask all the interrupts when the audio device does its request_irq. agree, it's not clear to me either. I'll work with Jerome to figure out if/how this can be removed. @@ -3364,6 +3425,14 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) WARN_ON(dev_priv->irq_mask != ~0); + if (IS_LPE_AUDIO_ENABLED(dev_priv)) { + u32 val = (I915_LPE_PIPE_A_INTERRUPT | + I915_LPE_PIPE_B_INTERRUPT | + I915_LPE_PIPE_C_INTERRUPT); 'val' seems like a rather pointless local variable. + + enable_mask |= val; indeed, will fix. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH v2 1/8] drm/i915: setup bridge for HDMI LPE audio driver
Thanks Ville for the review. A lot of the comments are related to the initial VED code we took pretty much as is, no issues to clean-up further. BTW, it looks like Jerome's patches were stuck for 10+ days on the intel-gfx server for some reason so not everyone saw the initial post? @@ -1141,7 +1141,13 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) if (IS_GEN5(dev_priv)) intel_gpu_ips_init(dev_priv); - i915_audio_component_init(dev_priv); + if (intel_lpe_audio_detect(dev_priv)) { + if (intel_lpe_audio_setup(dev_priv) < 0) + DRM_ERROR("failed to setup LPE Audio bridge\n"); + } I'd move all that into the lpe audio code. No need to have anything here but a single function call IMO. something like intel_lpe_audio_init() for symmetry with the hda component stuff, with both detection and setup handled internally? + + if (!IS_LPE_AUDIO_ENABLED(dev_priv)) I don't like that too much. I think I would just make that HAS_LPE_AUDIO(). The current IS_VLV||IS_CHV check can just be inlined into the init function. ok #define HAS_POOLED_EU(dev) (INTEL_INFO(dev)->has_pooled_eu) +#define HAS_LPE_AUDIO(dev) (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) +#define IS_LPE_AUDIO_ENABLED(dev_priv) \ + (__I915__(dev_priv)->lpe_audio.platdev != NULL) +#define IS_LPE_AUDIO_IRQ_VALID(dev_priv) \ + (__I915__(dev_priv)->lpe_audio.irq >= 0) Seems useless. We should just not register the lpe audio thing if we have no irq. ok --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1827,6 +1827,13 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg) * signalled in iir */ valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats); + if (IS_LPE_AUDIO_ENABLED(dev_priv)) + if (IS_LPE_AUDIO_IRQ_VALID(dev_priv)) I think both of these checks can be removed. We won't unmask the interrupts unless lpe is enabled, so the IIR bits will never be set in that case. + if (iir & (I915_LPE_PIPE_A_INTERRUPT | + I915_LPE_PIPE_B_INTERRUPT | + I915_LPE_PIPE_C_INTERRUPT)) + intel_lpe_audio_irq_handler(dev_priv); + ok. /* * VLV_IIR is single buffered, and reflects the level * from PIPESTAT/PORT_HOTPLUG_STAT, hence clear it last. @@ -1907,6 +1914,13 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg) * signalled in iir */ valleyview_pipestat_irq_ack(dev_priv, iir, pipe_stats); + if (IS_LPE_AUDIO_ENABLED(dev_priv)) + if (IS_LPE_AUDIO_IRQ_VALID(dev_priv)) + if (iir & (I915_LPE_PIPE_A_INTERRUPT | + I915_LPE_PIPE_B_INTERRUPT | + I915_LPE_PIPE_C_INTERRUPT)) + intel_lpe_audio_irq_handler(dev_priv); + ditto ok + platdev = platform_device_alloc("hdmi-lpe-audio", -1); + if (!platdev) { + ret = -ENOMEM; + DRM_ERROR("Failed to allocate LPE audio platform device\n"); + goto err; + } + + /* to work-around check_addr in nommu_map_sg() */ What's this about? Dunno, this was in the original VED series that we used as a baseline. Unless someone has memories from that time, i'll just remove this comment. + DRM_DEBUG("%s: HDMI_AUDIO rsc.start[0] = 0x%x\n", + __func__, (int)(rsc[0].start)); __func__ pointless since DRM_DEBUG alreay adds it. Also saying "rsc.start[0]" in the message doesn't tell anyone anything useful. And I think irq numbers are usually printed in decimal. Same, this was taken from the VED series, no issue to clean-up. + + rsc[1].start= pci_resource_start(dev->pdev, 0) + + I915_HDMI_LPE_AUDIO_BASE; + rsc[1].end = pci_resource_start(dev->pdev, 0) + + I915_HDMI_LPE_AUDIO_BASE + I915_HDMI_LPE_AUDIO_SIZE - 1; + rsc[1].flags= IORESOURCE_MEM; + rsc[1].name = "hdmi-lpe-audio-mmio"; + + DRM_DEBUG("%s: HDMI_AUDIO rsc.start[1] = 0x%x\n", + __func__, (int)(rsc[1].start)); Again "rsc[0].start" doesn't seem like anything useful to print in a debug message. Don't see much point in the whole debug message TBH since the start/end are fixed. so are you saying we should remove the code or move the level to DRM_INFO - for extra verbose debug? + + ret = platform_device_add_resources(platdev, rsc, 2); + if (ret) { + DRM_ERROR("Failed to add resource for platform device: %d\n", + ret); + goto err_put_de
Re: [Intel-gfx] i915 issues with 4.6
On 5/18/16 3:53 PM, Pierre-Louis Bossart wrote: On 5/18/16 2:08 AM, Ville Syrjälä wrote: On Tue, May 17, 2016 at 07:09:58PM -0500, Pierre-Louis Bossart wrote: On 05/17/2016 01:16 PM, Ville Syrjälä wrote: On Tue, May 17, 2016 at 01:00:13PM -0500, Pierre-Louis Bossart wrote: Hi, I was porting the HDMI audio patch to 4.6 as a service to several folks who need them before we finally upstream the reworked code, and I notice that the UI would freeze within 2s to 5mn, with all sorts of GPU hangs and issues reported in dmesg and /sys/calls/drm/card0/error Things work fine with the latest drm-intel-nightly branch from last night Known issue or should I file a bug? One GPU hang generally looks like any other from the symptom POV. Without the error state it's pretty much impossible to speculate. Just filing a new bug is usually the best policy. If it's indeed a kernel regression a bisect would also be helpful. Or in this case perhaps a reverse bisect since you say nightly is fine, so we might want to know what fixed it. bug filed at https://bugs.freedesktop.org/show_bug.cgi?id=95467 V4.5 works fine v4.6 is broken in less than 20s when moving windows around (can still get to virtual console) drm-intel-nightly works fine git bisect says this commit fixes the issue: commit a5e485a95c9c4cdd93b4c6dc53eee3bd1e50de11 Author: Ville Syrjälä mailto:ville.syrj...@linux.intel.com>> Date: Wed Apr 13 21:19:51 2016 +0300 drm/i915: Clear VLV_IER around irq processing Argh. So somehow the broken commit [1] got into 4.6. We'll need to get it reverted. [1] 9dbaab56ac09 ("drm/i915: Exit cherryview_irq_handler() after one pass") git revert 9dbaab56ac09 works fine on my Ara X5 Plus. I see patches being submitted to 4.6 stable now, is anyone on the graphics team planning to send a patch for this one or should I go ahead and do it? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 issues with 4.6
On 5/18/16 2:08 AM, Ville Syrjälä wrote: On Tue, May 17, 2016 at 07:09:58PM -0500, Pierre-Louis Bossart wrote: On 05/17/2016 01:16 PM, Ville Syrjälä wrote: On Tue, May 17, 2016 at 01:00:13PM -0500, Pierre-Louis Bossart wrote: Hi, I was porting the HDMI audio patch to 4.6 as a service to several folks who need them before we finally upstream the reworked code, and I notice that the UI would freeze within 2s to 5mn, with all sorts of GPU hangs and issues reported in dmesg and /sys/calls/drm/card0/error Things work fine with the latest drm-intel-nightly branch from last night Known issue or should I file a bug? One GPU hang generally looks like any other from the symptom POV. Without the error state it's pretty much impossible to speculate. Just filing a new bug is usually the best policy. If it's indeed a kernel regression a bisect would also be helpful. Or in this case perhaps a reverse bisect since you say nightly is fine, so we might want to know what fixed it. bug filed at https://bugs.freedesktop.org/show_bug.cgi?id=95467 V4.5 works fine v4.6 is broken in less than 20s when moving windows around (can still get to virtual console) drm-intel-nightly works fine git bisect says this commit fixes the issue: commit a5e485a95c9c4cdd93b4c6dc53eee3bd1e50de11 Author: Ville Syrjälä mailto:ville.syrj...@linux.intel.com>> Date: Wed Apr 13 21:19:51 2016 +0300 drm/i915: Clear VLV_IER around irq processing Argh. So somehow the broken commit [1] got into 4.6. We'll need to get it reverted. [1] 9dbaab56ac09 ("drm/i915: Exit cherryview_irq_handler() after one pass") git revert 9dbaab56ac09 works fine on my Ara X5 Plus. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface
On 3/15/16 11:21 AM, Vinod Koul wrote: On Tue, Mar 15, 2016 at 03:35:45PM +0200, Ville Syrjälä wrote: I understand the benefits of a parent/child device/subdevice model. What I don't see is whether we need the component framework at all here? It was used in the case of HDaudio since both i915 and HDaudio controllers get probed at different times, here the HDMI interface is just a bunch of registers/DMA from the same hardware so the whole master/client interface exposed by the component framework to 'bind' independent drivers is a bit overkill? I haven't read the patches, but using component.c when you instead can model it with parent/child smells like abuse. Component.c is meant for when you have multiple devices all over the device tree that in aggregate constitute the overall logical driver. This doesn't seem to be the case here. Right I do think that might be the case. We still need the eld notify hooks so that i915 can inform the audio driver about the state of the display. Whether that's best done via the component stuff or something else I don't know. There is already work ongoing by ARM folks for the interface, should we reuse that [1]. I would certainly argue reusing rather than redfeining a new one would be better. Btw this interface seems to rely on display side implemting these ops. My understanding is that it's an interface for external encoders that have an I2S or S/PDIF link. Such external encoders appear as ASoC codecs that can be bound to the SoC with DT bindings. I don't see what we could reuse here? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm: i915: remove intel_hdmi variable declaration
'intel_hdmi' variable was redeclared, use same variable declared in function scope Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/intel_hdmi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index e2dab48..d031be6 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1418,8 +1418,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_hdmi_unset_edid(connector); if (intel_hdmi_set_edid(connector, live_status)) { - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); - hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI; status = connector_status_connected; } else -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface
On 3/14/16 10:30 AM, Ville Syrjälä wrote: On Mon, Mar 14, 2016 at 05:21:54PM +0200, Ville Syrjälä wrote: On Mon, Mar 14, 2016 at 10:13:58AM -0500, Pierre-Louis Bossart wrote: On 3/11/16 1:09 PM, Ville Syrjälä wrote: On Fri, Mar 11, 2016 at 11:27:13AM -0600, Pierre-Louis Bossart wrote: Thanks for the review Ville [snip] Kinda hard to see where everything gets used due to the way the patches are split up. Yes, it's far from great... At least the hotplug/mode change events are not needed. We only have the two points where i915 should inform the audio driver about this stuff, and those are the intel_audio_code_enable/disable(). For that we already have the .pin_eld_notify() hook. The interrupt stuff should mostly vanish from i915 with the subdevice approach. As in i915 would just call the interrupt handler of the audio driver based on the LPE bits in IIR, and the audio driver can then do whatever it wants based on its own status register. Are you saying that the subdevice would provide a read/write interface for the audio driver to look at display registers, and the i915 driver would only provide a notification interface (EDID and interrupts) to the audio driver? The audio driver would simply ioremap the appropriate range of registers itself. If yes, would there be two component framework links, one between i915/audio driver and one between subdevice/audio driver. Yeah sort of. i915 registers the platform device for the audio, the audio driver can then bind to the device via the platform driver .probe callback. It can then register with the audio component stuff at some point to get the relevant notifications on the display state. When i915 gets unloaded we remove the platform device, at which point the audio driver's platform driver .remove() callback gets invoked and it should unregister/cleanup everything. we don't want to expose this interface when HDAudio is present and enabled so we would need to add a test for this. Also it looks like you want the creation of the platform device done in i915, we were thinking of doing this as part of the audio drivers but in the end this looks equivalent. In both cases we would end-up ignoring the HAD022A8 HID and not use acpi for this extension Well, if you have a device you can hang off from then i915 should need to register it I suppose. Though that would make the interrupt forwarding perhaps less nice. There's also the suspend/resume order dependency to deal with if there's no parent/child relationship between the devices. Oh and I suppose you'd end up ioremapping part of the pci dev2 mmio bar to get at the registers, which would look a bit funkly at least. I understand the benefits of a parent/child device/subdevice model. What I don't see is whether we need the component framework at all here? It was used in the case of HDaudio since both i915 and HDaudio controllers get probed at different times, here the HDMI interface is just a bunch of registers/DMA from the same hardware so the whole master/client interface exposed by the component framework to 'bind' independent drivers is a bit overkill? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface
On 3/11/16 1:09 PM, Ville Syrjälä wrote: On Fri, Mar 11, 2016 at 11:27:13AM -0600, Pierre-Louis Bossart wrote: Thanks for the review Ville [snip] Kinda hard to see where everything gets used due to the way the patches are split up. Yes, it's far from great... At least the hotplug/mode change events are not needed. We only have the two points where i915 should inform the audio driver about this stuff, and those are the intel_audio_code_enable/disable(). For that we already have the .pin_eld_notify() hook. The interrupt stuff should mostly vanish from i915 with the subdevice approach. As in i915 would just call the interrupt handler of the audio driver based on the LPE bits in IIR, and the audio driver can then do whatever it wants based on its own status register. Are you saying that the subdevice would provide a read/write interface for the audio driver to look at display registers, and the i915 driver would only provide a notification interface (EDID and interrupts) to the audio driver? The audio driver would simply ioremap the appropriate range of registers itself. If yes, would there be two component framework links, one between i915/audio driver and one between subdevice/audio driver. Yeah sort of. i915 registers the platform device for the audio, the audio driver can then bind to the device via the platform driver .probe callback. It can then register with the audio component stuff at some point to get the relevant notifications on the display state. When i915 gets unloaded we remove the platform device, at which point the audio driver's platform driver .remove() callback gets invoked and it should unregister/cleanup everything. we don't want to expose this interface when HDAudio is present and enabled so we would need to add a test for this. Also it looks like you want the creation of the platform device done in i915, we were thinking of doing this as part of the audio drivers but in the end this looks equivalent. In both cases we would end-up ignoring the HAD022A8 HID and not use acpi for this extension I just tried to frob around with the VED code a bit, and got it to load at least. It's not quite happy about reloading i915 while the ipvr driver was loaded though. Not sure what's going on there, but I do think this approach should work. So the VED patch could serve as a decent enough model to follow. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface
Thanks for the review Ville [snip] Kinda hard to see where everything gets used due to the way the patches are split up. Yes, it's far from great... At least the hotplug/mode change events are not needed. We only have the two points where i915 should inform the audio driver about this stuff, and those are the intel_audio_code_enable/disable(). For that we already have the .pin_eld_notify() hook. The interrupt stuff should mostly vanish from i915 with the subdevice approach. As in i915 would just call the interrupt handler of the audio driver based on the LPE bits in IIR, and the audio driver can then do whatever it wants based on its own status register. Are you saying that the subdevice would provide a read/write interface for the audio driver to look at display registers, and the i915 driver would only provide a notification interface (EDID and interrupts) to the audio driver? If yes, would there be two component framework links, one between i915/audio driver and one between subdevice/audio driver. I am way beyond my comfort zone, bear with me if this is silly. Thanks. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 02/15] drm: i915: remove intel_hdmi variable declaration
On 3/10/16 11:34 AM, Ville Syrjälä wrote: On Fri, Mar 04, 2016 at 08:50:39PM -0600, Pierre-Louis Bossart wrote: 'intel_hdmi' variable is redeclared, use same variable declared in function scope. Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index d8060e6..1beb155 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1418,7 +1418,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_hdmi_unset_edid(connector); if (intel_hdmi_set_edid(connector, live_status)) { - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + intel_hdmi = intel_attached_hdmi(connector); We're still going to get the same answer, so you can just kill the assignment as well. Thanks. I wasn't sure if the set_edid would have side effects. Will resend this one. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 00/15] HDMI Audio support on Atom w/o HDAUdio
Not sure why you skirt around calling the thing by its name. It's called LPE isn't it? No. LPE aka SST is the path to the audio dsp subsystem. This path to HDMI has nothing to do with the audio dsp. Not a single gate is shared. The why are the interrupt bits called LPE_somethingsomething? And what generates the audio data then? I don't think the HAS ever mentioned LPE, it's really unrelated to LPE. LPE cannot even get the interrupts or access the registers. The audio data is generated by the application, written to a ring buffer and fetch by the DMA before they are inserted in the audio islands In most Android implementations this driver is probed using a dedicated ACPI _HID different from the LPEA one. For upstream and machines that don't have this specific device in the BIOS we may piggyback on the LPEA device to complete the probe (that's how it's done on Windows apparently). Hmm. So it's a third audio controller or something? Just someone named the display related bits as LPE just to confuse people on purpose? it's really an interface to the display controller, not a new hardware part. Everything called LPE should be renamed to avoid this confusion. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 01/15] drm: i915: fix inversion of definitions for LPE_PIPE_A/B
On 3/5/16 7:27 AM, Ville Syrjälä wrote: On Fri, Mar 04, 2016 at 08:50:38PM -0600, Pierre-Louis Bossart wrote: Definitions for I915_LPE_PIPE_A_INTERRUPT and I915_LPE_PIPE_B_INTERRUPT are inverted. Not according to the docs. Are the docs wrong? Possibly. I compared with the Android code and flagged this conflict, I didn't dig deeper. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 00/15] HDMI Audio support on Atom w/o HDAUdio
On 3/5/16 7:42 AM, Ville Syrjälä wrote: On Fri, Mar 04, 2016 at 08:50:37PM -0600, Pierre-Louis Bossart wrote: When HDaudio is not enabled or fused-out, an alternate hardware interface can be used to provide audio data to the display/HDMI controller on Atom platforms. The code to control this interface was never submitted upstream but has been used extensively in Android programs on Medfield, Clovertrail, Merrifield, Moorefield, Baytrail-T and CherryTrail-T, as well as the Baytrail Compute Stick w/ Ubuntu. Not sure why you skirt around calling the thing by its name. It's called LPE isn't it? No. LPE aka SST is the path to the audio dsp subsystem. This path to HDMI has nothing to do with the audio dsp. Not a single gate is shared. In most Android implementations this driver is probed using a dedicated ACPI _HID different from the LPEA one. For upstream and machines that don't have this specific device in the BIOS we may piggyback on the LPEA device to complete the probe (that's how it's done on Windows apparently). The main feedback expected in this RFC is on the interaction between audio and display driver, specifically if we can wrap the interface control with the component framework already used between i915 and HDaudio. A short documentation was added to help explain how the hardware works. Yes, using/extending the already existing audio component stuff was my first though when I did a preliminary scan of the patches. I didn't look too closely at what's needed hardware wise, so can't get into details yet. Some other peculiar things that caught my eye: - adds some HDMI live status stuff etc. which we already have if we can reuse existing parts we can prune this driver. - missing pipe C for CHV? yes, none of the CHT changes are included for now. - no clue what that procfs/rpm patch was about it's very likely legacy code, i don't know if it's needed any longer. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 15/15] i915: Fix typo in comment.
From: Toyo Abe Signed-off-by: Toyo Abe Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/i915_irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index dcc1564..25c2d41 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2848,7 +2848,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } -/* Added fo HDMI AUdio */ +/* Added for HDMI Audio */ int i915_enable_hdmi_audio_int(struct drm_device *dev) { struct drm_i915_private *dev_priv = (struct drm_i915_private *) dev->dev_private; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 12/15] hdmi_audio: Fixup some monitor
From: David Henningsson I think this change was given to us, and they claimed it fixed an issue on some monitor brand. I'm not sure what this patch actually does. Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- sound/hdmi_audio/intel_mid_hdmi_audio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sound/hdmi_audio/intel_mid_hdmi_audio.c b/sound/hdmi_audio/intel_mid_hdmi_audio.c index 611c51c..e696c80 100644 --- a/sound/hdmi_audio/intel_mid_hdmi_audio.c +++ b/sound/hdmi_audio/intel_mid_hdmi_audio.c @@ -386,6 +386,7 @@ static void snd_intelhad_reset_audio_v2(u8 reset) static int had_prog_status_reg(struct snd_pcm_substream *substream, struct snd_intelhad *intelhaddata) { + union aud_cfg cfg_val = {.cfg_regval = 0}; union aud_ch_status_0 ch_stat0 = {.status_0_regval = 0}; union aud_ch_status_1 ch_stat1 = {.status_1_regval = 0}; int format; @@ -396,6 +397,8 @@ static int had_prog_status_reg(struct snd_pcm_substream *substream, IEC958_AES0_NONAUDIO)>>1; ch_stat0.status_0_regx.clk_acc = (intelhaddata->aes_bits & IEC958_AES3_CON_CLOCK)>>4; + cfg_val.cfg_regx.val_bit = ch_stat0.status_0_regx.lpcm_id; + switch (substream->runtime->rate) { case AUD_SAMPLE_RATE_32: ch_stat0.status_0_regx.samp_freq = CH_STATUS_MAP_32KHZ; @@ -474,7 +477,6 @@ int snd_intelhad_prog_audio_ctrl_v2(struct snd_pcm_substream *substream, else cfg_val.cfg_regx_v2.layout = LAYOUT1; - cfg_val.cfg_regx_v2.val_bit = 1; had_write_register(AUD_CONFIG, cfg_val.cfg_regval); return 0; } @@ -530,7 +532,6 @@ int snd_intelhad_prog_audio_ctrl_v1(struct snd_pcm_substream *substream, } - cfg_val.cfg_regx.val_bit = 1; had_write_register(AUD_CONFIG, cfg_val.cfg_regval); return 0; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 10/15] add dependency on PM_RUNTIME
Signed-off-by: Pierre-Louis Bossart --- sound/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/Kconfig b/sound/Kconfig index 75c679e..b8b4fce 100644 --- a/sound/Kconfig +++ b/sound/Kconfig @@ -138,6 +138,7 @@ config AC97_BUS config SUPPORT_HDMI bool "SUPPORT_HDMI" depends on DRM_I915 + select PM_RUNTIME default n help Choose this option to support HDMI. -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 14/15] i915: Enable LPE_PIPEA_Interrupt.
From: Toyo Abe To enable interrupt, IER, IIR, and IMR must be configured appropriately. IER setting for hdmi_audio was missing. This fixes the following warning in dmesg. [ 302.369965] had: Driver detected 2 missed buffer done interrupt(s) Signed-off-by: Toyo Abe Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/i915_irq.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 556fa80..dcc1564 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2857,11 +2857,14 @@ int i915_enable_hdmi_audio_int(struct drm_device *dev) int pipe = 1; spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + i915_enable_lpe_pipestat(dev_priv, pipe); + imr = I915_READ(VLV_IMR); /* Audio is on Stream A */ imr &= ~I915_LPE_PIPE_A_INTERRUPT; I915_WRITE(VLV_IMR, imr); - i915_enable_lpe_pipestat(dev_priv, pipe); + I915_WRITE(VLV_IER, ~imr); + POSTING_READ(VLV_IER); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); return 0; @@ -2878,7 +2881,10 @@ int i915_disable_hdmi_audio_int(struct drm_device *dev) spin_lock_irqsave(&dev_priv->irq_lock, irqflags); imr = I915_READ(VLV_IMR); imr |= I915_LPE_PIPE_A_INTERRUPT; + I915_WRITE(VLV_IER, ~imr); I915_WRITE(VLV_IMR, imr); + POSTING_READ(VLV_IMR); + i915_disable_lpe_pipestat(dev_priv, pipe); spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 02/15] drm: i915: remove intel_hdmi variable declaration
'intel_hdmi' variable is redeclared, use same variable declared in function scope. Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/intel_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index d8060e6..1beb155 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1418,7 +1418,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) intel_hdmi_unset_edid(connector); if (intel_hdmi_set_edid(connector, live_status)) { - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); + intel_hdmi = intel_attached_hdmi(connector); hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI; status = connector_status_connected; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 05/15] drm/i915: changes for non-HDAudio HDMI interface
Changes to existing code for interface available on Baytrail and CherryTrail This driver was downloaded from https://github.com/01org/baytrailaudio/ ...and had the changes to .config stripped and the revert on sound/init.c Cleanup, port to 4.4 and intel-drm by Pierre Bossart Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/i915_irq.c | 81 drivers/gpu/drm/i915/intel_display.c | 8 ++ drivers/gpu/drm/i915/intel_hdmi.c| 183 ++- 3 files changed, 271 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d1a46ef..556fa80 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -603,6 +603,31 @@ i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, __i915_disable_pipestat(dev_priv, pipe, enable_mask, status_mask); } +/* Added for HDMI AUDIO */ +void +i915_enable_lpe_pipestat(struct drm_i915_private *dev_priv, int pipe) +{ + u32 mask; + + mask = dev_priv->hdmi_audio_interrupt_mask; + mask |= I915_HDMI_AUDIO_UNDERRUN | I915_HDMI_AUDIO_BUFFER_DONE; + /* Enable the interrupt, clear any pending status */ + I915_WRITE(I915_LPE_AUDIO_HDMI_STATUS_A, mask); + POSTING_READ(I915_LPE_AUDIO_HDMI_STATUS_A); +} + +void +i915_disable_lpe_pipestat(struct drm_i915_private *dev_priv, int pipe) +{ + u32 mask; + + mask = dev_priv->hdmi_audio_interrupt_mask; + mask |= I915_HDMI_AUDIO_UNDERRUN | I915_HDMI_AUDIO_BUFFER_DONE; + /* Disable the interrupt, clear any pending status */ + I915_WRITE(I915_LPE_AUDIO_HDMI_STATUS_A, mask); + POSTING_READ(I915_LPE_AUDIO_HDMI_STATUS_A); +} + /** * i915_enable_asle_pipestat - enable ASLE pipestat for OpRegion * @dev: drm device @@ -1649,6 +1674,7 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir) struct drm_i915_private *dev_priv = dev->dev_private; u32 pipe_stats[I915_MAX_PIPES] = { }; int pipe; + int lpe_stream; spin_lock(&dev_priv->irq_lock); @@ -1719,6 +1745,24 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir) intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe); } + if (iir & I915_LPE_PIPE_A_INTERRUPT) { + lpe_stream = I915_READ(I915_LPE_AUDIO_HDMI_STATUS_A); + if (lpe_stream & I915_HDMI_AUDIO_UNDERRUN) { + I915_WRITE(I915_LPE_AUDIO_HDMI_STATUS_A, + lpe_stream); + mid_hdmi_audio_signal_event(dev, + HAD_EVENT_AUDIO_BUFFER_UNDERRUN); + } + + lpe_stream = I915_READ(I915_LPE_AUDIO_HDMI_STATUS_A); + if (lpe_stream & I915_HDMI_AUDIO_BUFFER_DONE) { + I915_WRITE(I915_LPE_AUDIO_HDMI_STATUS_A, + lpe_stream); + mid_hdmi_audio_signal_event(dev, + HAD_EVENT_AUDIO_BUFFER_DONE); + } + } + if (pipe_stats[0] & PIPE_GMBUS_INTERRUPT_STATUS) gmbus_irq_handler(dev); } @@ -2804,6 +2848,43 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); } +/* Added fo HDMI AUdio */ +int i915_enable_hdmi_audio_int(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = (struct drm_i915_private *) dev->dev_private; + unsigned long irqflags; + u32 imr; + int pipe = 1; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + imr = I915_READ(VLV_IMR); + /* Audio is on Stream A */ + imr &= ~I915_LPE_PIPE_A_INTERRUPT; + I915_WRITE(VLV_IMR, imr); + i915_enable_lpe_pipestat(dev_priv, pipe); + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + return 0; +} + +/* Added for HDMI Audio */ +int i915_disable_hdmi_audio_int(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = (struct drm_i915_private *) dev->dev_private; + unsigned long irqflags; + u32 imr; + int pipe = 1; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + imr = I915_READ(VLV_IMR); + imr |= I915_LPE_PIPE_A_INTERRUPT; + I915_WRITE(VLV_IMR, imr); + i915_disable_lpe_pipestat(dev_priv, pipe); + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + + return 0; +} + static bool ring_idle(struct intel_engine_cs *ring, u32 seqno) { diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 44fcff0..5831af4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8
[Intel-gfx] [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface
Add header files for interface available on Baytrail and CherryTrail Initial code was downloaded from https://github.com/01org/baytrailaudio/ ...and had the changes to .config stripped and the revert on sound/init.c done by David Henningson Clean-up, port to 4.4 and intel-drm by Pierre Bossart Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/hdmi_audio_if.h | 122 +++ drivers/gpu/drm/i915/i915_drv.h | 31 + drivers/gpu/drm/i915/i915_reg.h | 7 ++ drivers/gpu/drm/i915/intel_drv.h | 11 4 files changed, 171 insertions(+) create mode 100644 drivers/gpu/drm/i915/hdmi_audio_if.h diff --git a/drivers/gpu/drm/i915/hdmi_audio_if.h b/drivers/gpu/drm/i915/hdmi_audio_if.h new file mode 100644 index 000..f968028 --- /dev/null +++ b/drivers/gpu/drm/i915/hdmi_audio_if.h @@ -0,0 +1,122 @@ +/* + * Copyright (c) 2010, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * Authors: + * jim liu + * Uma Shankar + */ + + +#ifndef __HDMI_AUDIO_IF_H +#define __HDMI_AUDIO_IF_H + +#include +#include + +/* HDMI AUDIO INTERRUPT TYPE */ +#define HDMI_AUDIO_UNDERRUN (1UL<<0) +#define HDMI_AUDIO_BUFFER_DONE (1UL<<1) + +/* the monitor type HDMI or DVI */ +#define MONITOR_TYPE_HDMI 1 +#define MONITOR_TYPE_DVI 2 + +extern int i915_hdmi_state; +extern int i915_notify_had; + +enum had_caps_list { + HAD_GET_ELD = 1, + HAD_GET_SAMPLING_FREQ, + HAD_GET_DISPLAY_RATE, + HAD_GET_HDCP_STATUS, + HAD_GET_AUDIO_STATUS, + HAD_SET_ENABLE_AUDIO, + HAD_SET_DISABLE_AUDIO, + HAD_SET_ENABLE_AUDIO_INT, + HAD_SET_DISABLE_AUDIO_INT, + OTHERS_TBD, +}; + +enum had_event_type { + HAD_EVENT_HOT_PLUG = 1, + HAD_EVENT_HOT_UNPLUG, + HAD_EVENT_MODE_CHANGING, + HAD_EVENT_PM_CHANGING, + HAD_EVENT_AUDIO_BUFFER_DONE, + HAD_EVENT_AUDIO_BUFFER_UNDERRUN, + HAD_EVENT_QUERY_IS_AUDIO_BUSY, + HAD_EVENT_QUERY_IS_AUDIO_SUSPENDED, +}; + +/* + * HDMI Display Controller Audio Interface + * + */ +typedef int (*had_event_call_back) (enum had_event_type event_type, + void *ctxt_info); + +struct hdmi_audio_registers_ops { + int (*hdmi_audio_read_register)(uint32_t reg_addr, uint32_t *data); + int (*hdmi_audio_write_register)(uint32_t reg_addr, uint32_t data); + int (*hdmi_audio_read_modify)(uint32_t reg_addr, uint32_t data, + uint32_t mask); +}; + +struct hdmi_audio_query_set_ops { + int (*hdmi_audio_get_caps)(enum had_caps_list query_element, + void *capabilties); + int (*hdmi_audio_set_caps)(enum had_caps_list set_element, + void *capabilties); +}; + +typedef struct hdmi_audio_event { + int type; +} hdmi_audio_event_t; + +struct snd_intel_had_interface { + const char *name; + int (*query)(void *had_data, hdmi_audio_event_t event); + int (*suspend)(void *had_data, hdmi_audio_event_t event); + int (*resume)(void *had_data); +}; + +struct hdmi_audio_priv { + struct drm_device *dev; + u32 hdmib_reg; + + bool is_hdcp_supported; + bool hdmi_hpd_connected; + int monitor_type; + void *context; +}; + +extern void i915_hdmi_audio_init(struct hdmi_audio_priv *p_hdmi_priv); + +extern bool mid_hdmi_audio_is_busy(struct drm_device *dev); +extern bool mid_hdmi_audio_suspend(struct drm_device *dev); +extern void mid_hdmi_audio_resume(struct drm_device *dev); +extern void mid_hdmi_audio_signal_event(struct drm_device *dev, + enum had_event_type event); + +/* Added for HDMI Audio */ +extern void hdmi_get_eld(uint8_t *eld); +extern int i915_enable_hdmi_audio_int(struct drm_device *dev); +extern int i915_disable_hdmi_audio_int(struct drm_device *dev); +extern int mid_hdmi_audio_setup( + had_event_call_back audio_callbacks, + struct hdmi_audio_registers_ops *reg_ops, + struct hdmi_audio_query_set_ops *query_ops); +extern int mid_hdmi_audio_register( + struct snd_intel_had_interface *driver, + void *had_data); + +#endif /* __HDMI_AUDIO_IF_H */ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2cb0a41..5dceb5b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -53,6 +53,7 @@ #include #include #include "intel_guc.h" +#include "hdmi_audio_if.h" /* General customization: */ @@ -1196,6 +1197,18 @@ struct intel_gen6_p
[Intel-gfx] [RFC 08/15] drm/i915: enable non-HDAudio HDMI interface Makefile
Makefile for all previous patches This driver was downloaded from https://github.com/01org/baytrailaudio/ ...and had the changes to .config stripped and the revert on sound/init.c clean-up, port to 4.4 and intel-drm by Pierre Bossart Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0851de07..b85d6c2 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -37,6 +37,7 @@ i915-y += i915_cmd_parser.o \ i915_trace_points.o \ intel_lrc.o \ intel_mocs.o \ + i915_rpm.o \ intel_ringbuffer.o \ intel_uncore.o @@ -89,7 +90,8 @@ i915-y += dvo_ch7017.o \ intel_lvds.o \ intel_panel.o \ intel_sdvo.o \ - intel_tv.o + intel_tv.o \ + hdmi_audio_if.o # virtual gpu code i915-y += i915_vgpu.o -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 03/15] Doc: sound: add description of Atom HDMI audio interface
Used when HDaudio is not enabled Signed-off-by: Pierre-Louis Bossart --- Documentation/sound/alsa/Atom-hdmi-audio.txt | 241 +++ 1 file changed, 241 insertions(+) create mode 100644 Documentation/sound/alsa/Atom-hdmi-audio.txt diff --git a/Documentation/sound/alsa/Atom-hdmi-audio.txt b/Documentation/sound/alsa/Atom-hdmi-audio.txt new file mode 100644 index 000..bcc485c --- /dev/null +++ b/Documentation/sound/alsa/Atom-hdmi-audio.txt @@ -0,0 +1,241 @@ +This short blurb describes the support for HDMI audio on Atom +platforms with the DMA/register-based interface (non HDAudio). + +1.Overview + +The HDMI transmitter operates in an image-oriented fashion. The major +part of each frame is used by pixel data; non-pixel data is inserted +in the data islands in the vertical and horizontal blanking +spaces. Null packets are transmitted to fill the gaps when no data is +available. + +Non-pixel data is abstracted though the concept of ‘Data Island’ which +includes packet types such as + +- Audio Clock Regeneration. This packet conveys the information needed +for the receiver to reconstruct a valid audio clock; it is sent every +other frame. + +- Audio data (Audio Sample Packet, One Bit Audio Sample Packet, DST +Audio Packet, HBR Audio packet) + +- Audio InfoFrame. This packets conveys configuration information on +formats, sample rate, channel allocation, etc. + +2 Hardware description + +Although the HDMI block reuses many elements from the ‘normal’ +HDAudio-based design, the insertion of audio and configuration data +into the HDMI frame is done differently, spefically the data is +provided to the display controller with a DMA (instead of a serial +link) and the control interface is done through register access +(instead of HDAudio verbs). The interaction between audio and display +driver remains similar, e.g. to handle hot-plug or EDID information. + +2.1 Data flow + +The audio rate is derived from the video frame rate and software must +program the values of N/CTS registers. In some cases the audio clock is +approximated with an offset that remains within the acceptable bounds +of the standard. Based on N and CTS values and the actual video clock, +the transmitter determines how many audio sample packets need to be +inserted per frame. These packets are read from memory using a DMA +interface and inserted in the data island. In case the display format +or resolution changes the values for N and CTS need to be adjusted. + +2.2 Data formats + +The supported audio frequencies are 32, 44.1, 48, 88.2, 96, 176.4 and +192 kHz. 64 kHz is not supported in the HDMI standard. + +Two to eight channels can be transmitted. Odd number of channels will +required the insertion of an additional dummy channel. This insertion +is handled by software. + +Each transmitted PCM word is represented on 28 bits as described in +the IEC60958 standard. Each sample conveys 24 bits of audio data, +along with 4 configuration bits. + +Unsupported features: +One bit audio sample packet +DST audio packet +High Bit Rate packets + +2.3 DMA operation + +The HDMI audio DMA controller handles a single context. It supports a +four-entry descriptor register for source buffers and handles a single +destination. The address and size of the source buffers can be +programmed in the descriptors. + +The DMA controller transfers bursts of 16 32-bit words from memory (64 +bytes). These bursts are initiated when its FIFO level falls below a +programmable watermark threshold. It is required that the buffers are +aligned on a 64-byte boundary and are allocated from contiguous +physical memory. They also need to be multiples of 64 bytes. + +The DMA controller enables the addition of buffers in descriptor +registers during playback and issues an interrupt when a Transfer +Complete is reached. When it has finished playing a buffer, it will +move to the next descriptor. If no valid descriptors are found after a +complete loop on the 4 descriptors, the hardware will go to a wait +mode and report an under-run condition. As soon as the driver enables +a new transfer the hardware will wake up. Note: it is not possible to +program the DMA to fetch data completely autonomously without +interrupts, after the 4 descriptors are used they need to be +re-enabled by software. + +Should an underflow occur, the hardware can raise an underflow +interrupt, will not transmit any audio packets and will insert null +packets instead. On the software side, the audio device should be +closed and the application restarted from a known position. + +2.4 Data representation in RAM + +Linear or compressed data are represented in the 24 least significant +bits of a 32-bit word. No overlap between words is allowed. The +hardware assumes that the 32-bits are written in little-endian +format. Software needs to take care of format conversions when playing +16-bit PCM (shift-left by eight bits) + +Layout 0 is used for stereo data, with left and right channels
[Intel-gfx] [RFC 13/15] hdmi_audio: Fix mishandling of AUD_HDMI_STATUS_v2 register.
From: Toyo Abe According to the datasheet, write one to clear these UNDERRUN flag bits. This fixes the following warning in dmesg. [15357.574902] had: Unable to clear UNDERRUN bits Signed-off-by: Toyo Abe Signed-off-by: Pierre-Louis Bossart --- sound/hdmi_audio/intel_mid_hdmi_audio.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/hdmi_audio/intel_mid_hdmi_audio.c b/sound/hdmi_audio/intel_mid_hdmi_audio.c index e696c80..b86c37a 100644 --- a/sound/hdmi_audio/intel_mid_hdmi_audio.c +++ b/sound/hdmi_audio/intel_mid_hdmi_audio.c @@ -1135,7 +1135,6 @@ static void had_clear_underrun_intr_v2(struct snd_intelhad *intelhaddata) pr_debug("HDMI status =0x%x\n", hdmi_status); if (hdmi_status & AUD_CONFIG_MASK_UNDERRUN) { i++; - hdmi_status &= ~AUD_CONFIG_MASK_UNDERRUN; had_write_register(AUD_HDMI_STATUS_v2, hdmi_status); } else break; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 06/15] drm/i915: power-related changes non-HDAudio HDMI interface
PM and RPM changes for interface available on Baytrail and CherryTrail This driver was downloaded from https://github.com/01org/baytrailaudio/ ...and had the changes to .config stripped and the revert on sound/init.c Clean-up, port to 4.4 and intel-drm by Pierre Bossart Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/i915_rpm.c | 476 drivers/gpu/drm/i915/intel_pm.c | 53 + 2 files changed, 529 insertions(+) create mode 100644 drivers/gpu/drm/i915/i915_rpm.c diff --git a/drivers/gpu/drm/i915/i915_rpm.c b/drivers/gpu/drm/i915/i915_rpm.c new file mode 100644 index 000..511311c --- /dev/null +++ b/drivers/gpu/drm/i915/i915_rpm.c @@ -0,0 +1,476 @@ +/* + * Copyright 2013 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Author: + * Naresh Kumar Kachhi + */ + +#include "i915_drv.h" +#include "i915_reg.h" +#include "intel_drv.h" +#include +#ifdef CONFIG_PM_RUNTIME +#include +#endif +#include /* Needed for procfs access */ +#include /* For the basic file system */ +#include + +#define RPM_AUTOSUSPEND_DELAY 500 + +#ifdef CONFIG_PM_RUNTIME + +/** + * - Where should we use get/put? + * Get/put should be used very carefully as we might end up in weird states + * if not used properly (see the Note below). We want to cover all the + * acesses that might result in accessing rings/display/registers/gtt etc + * Mostly covering ioctls and drm callbacks should be enough. You can + * avoid those which does not access any HW. + * + * - When should we avoid get/put? + * WQ and interrupts should be taken care in suspend path. We should + * disable all the interrupts and cancel any pending WQs. Never try to + * cover interrupt/WQ with get/put unless you are sure about it. + * + * Note:Following scenarios should be strictly avoided while using get_sync + * 1. Calling get_sync with struct_mutex or mode_config.mutex locked + *- we acquire these locks in runtime_resume, so any call to get_sync + *with these mutex locked might end up in a dead lock. + *check_mutex_current function can be used to debug this scenario. + *- Or let's say thread1 has done get_sync and is currently executing + *runtime_resume function. Before thread1 is able to acquire these + *mutex, thread2 acquires the mutex and does a get_sync. Now thread1 + *is waiting for mutex and thread2 is waiting for dev->power.lock + *resulting in a deadlock. Use check_mutex to debug this. + * 2. Calling get_sync from runtime_resume path + *runtime_resume is called with dev->power.lock held. doing get_sync + *in same path will end up in deadlock + */ + +#define RPM_PROC_ENTRY_FILENAME"i915_rpm_op" +#define RPM_PROC_ENTRY_DIRECTORY "driver/i915rpm" + +int i915_rpm_get_procfs(struct inode *inode, struct file *file); +int i915_rpm_put_procfs(struct inode *inode, struct file *file); +/* proc file operations supported */ +static const struct file_operations rpm_file_ops = { + .owner = THIS_MODULE, + .open = i915_rpm_get_procfs, + .release= i915_rpm_put_procfs, +}; + +bool i915_pm_runtime_enabled(struct device *dev) +{ + return pm_runtime_enabled(dev); +} + +void i915_rpm_enable(struct device *dev) +{ + int cur_status = pm_runtime_enabled(dev); + + if (!cur_status) { + pm_runtime_enable(dev); + pm_runtime_allow(dev); + } +} + +void i915_rpm_disable(struct drm_device *drm_dev) +{ + struct device *dev = drm_dev->dev; + int cur_status = pm_runtime_enabled(dev); + + if (cur_status) { + pm_runtime_forbid(dev); + pm_runtime_disable(dev); + } +} + +static int i915_rpm_procfs_ini
[Intel-gfx] [RFC 11/15] hdmi_audio: Improve position reporting
From: David Henningsson Using a hw register to calculate sub-period position reports. This makes PulseAudio happier. Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- sound/hdmi_audio/intel_mid_hdmi_audio.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sound/hdmi_audio/intel_mid_hdmi_audio.c b/sound/hdmi_audio/intel_mid_hdmi_audio.c index f397435..611c51c 100644 --- a/sound/hdmi_audio/intel_mid_hdmi_audio.c +++ b/sound/hdmi_audio/intel_mid_hdmi_audio.c @@ -1550,6 +1550,8 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( { struct snd_intelhad *intelhaddata; u32 bytes_rendered = 0; + u32 t; + int buf_id; /* pr_debug("snd_intelhad_pcm_pointer called\n"); */ @@ -1560,6 +1562,14 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( return SNDRV_PCM_POS_XRUN; } + buf_id = intelhaddata->curr_buf % 4; + had_read_register(AUD_BUF_A_LENGTH + (buf_id * HAD_REG_WIDTH), &t); + if (t == 0) { + pr_debug("discovered buffer done for buf %d\n", buf_id); + /* had_process_buffer_done(intelhaddata); */ + } + t = intelhaddata->buf_info[buf_id].buf_size - t; + if (intelhaddata->stream_info.buffer_rendered) div_u64_rem(intelhaddata->stream_info.buffer_rendered, intelhaddata->stream_info.ring_buf_size, @@ -1567,7 +1577,7 @@ static snd_pcm_uframes_t snd_intelhad_pcm_pointer( intelhaddata->stream_info.buffer_ptr = bytes_to_frames( substream->runtime, - bytes_rendered); + bytes_rendered + t); return intelhaddata->stream_info.buffer_ptr; } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 07/15] drm/i915: Add API code for non-HDAudio HDMI interface
Add API code for interface available on Baytrail and CherryTrail Initial code was downloaded from https://github.com/01org/baytrailaudio/ ...and had the changes to .config stripped and the revert on sound/init.c done by David Henningson Clean-up, port to 4.4 and intel-drm by Pierre Bossart Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/hdmi_audio_if.c | 410 +++ 1 file changed, 410 insertions(+) create mode 100644 drivers/gpu/drm/i915/hdmi_audio_if.c diff --git a/drivers/gpu/drm/i915/hdmi_audio_if.c b/drivers/gpu/drm/i915/hdmi_audio_if.c new file mode 100644 index 000..d198f39 --- /dev/null +++ b/drivers/gpu/drm/i915/hdmi_audio_if.c @@ -0,0 +1,410 @@ +/* + * Copyright (c) 2010, Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * Authors: + * jim liu + * Uma Shankar + */ + +#include +#include "hdmi_audio_if.h" +#include "i915_drv.h" +#include "i915_reg.h" + +#define CONFIG_SUPPORT_HDMI_AUDIO +#ifdef CONFIG_SUPPORT_HDMI_AUDIO + +int i915_hdmi_state; +int i915_notify_had; + +/* + * Audio register range 0x65000 to 0x65FFF + */ + +#define IS_HDMI_AUDIO_I915(reg) ((reg >= 0x65000) && (reg < 0x65FFF)) + +/* Added for HDMI Audio */ +#define HAD_MAX_ELD_BYTES 84 +uint8_t hdmi_eld[HAD_MAX_ELD_BYTES]; + +static struct hdmi_audio_priv *hdmi_priv; + +void i915_hdmi_audio_init(struct hdmi_audio_priv *p_hdmi_priv) +{ + hdmi_priv = p_hdmi_priv; +} + +/* Added for HDMI Audio */ +void hdmi_get_eld(uint8_t *eld) +{ + struct drm_device *dev = hdmi_priv->dev; + struct drm_i915_private *dev_priv = + (struct drm_i915_private *) dev->dev_private; + memcpy(hdmi_eld, eld, HAD_MAX_ELD_BYTES); + if (i915_notify_had) { + mid_hdmi_audio_signal_event(dev_priv->dev, + HAD_EVENT_HOT_PLUG); + i915_notify_had = 0; + } +} + +static inline int android_hdmi_get_eld(struct drm_device *dev, void *eld) +{ + memcpy(eld, hdmi_eld, HAD_MAX_ELD_BYTES); + return 0; +} + +/* + * return whether HDMI audio device is busy. + */ +bool mid_hdmi_audio_is_busy(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = + (struct drm_i915_private *) dev->dev_private; + int hdmi_audio_busy = 0; + hdmi_audio_event_t hdmi_audio_event; + + if (i915_hdmi_state == connector_status_disconnected) { + /* HDMI is not connected, assuming audio device is idle. */ + return false; + } + + if (dev_priv->had_interface) { + hdmi_audio_event.type = HAD_EVENT_QUERY_IS_AUDIO_BUSY; + hdmi_audio_busy = dev_priv->had_interface->query( + dev_priv->had_pvt_data, + hdmi_audio_event); + return hdmi_audio_busy != 0; + } + return false; +} + +/* + * return whether HDMI audio device is suspended. + */ +bool mid_hdmi_audio_suspend(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = + (struct drm_i915_private *) dev->dev_private; + hdmi_audio_event_t hdmi_audio_event; + int ret = 0; + + if (i915_hdmi_state == connector_status_disconnected) { + /* HDMI is not connected, assuming audio device +* is suspended already. +*/ + return true; + } + DRM_DEBUG_DRIVER("%s: i915_hdmi_state %d", __func__, + i915_hdmi_state); + + if (dev_priv->had_interface) { + hdmi_audio_event.type = 0; + ret = dev_priv->had_interface->suspend(dev_priv->had_pvt_data, + hdmi_audio_event); + return (ret == 0) ? true : false; + } + return true; +} + +void mid_hdmi_audio_resume(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = + (struct drm_i915_private *) dev->dev_private; + + if (i915_hdmi_state == connector_status_disconnected) { + /* HDMI is not connected, there is no need +* to resume audio device. +*/ + return; + } + DRM_DEBUG_DRIVER("%s: i915_hdmi_state %d", __func__, + i915_hdmi_state); + + if (dev_priv->had_interface) + dev_priv->had_interface->resume(dev_priv->h
[Intel-gfx] [RFC 09/15] ALSA: Intel: Atom: add Atom non-HDAudio HDMI interface
Add support interface available on Baytrail and CherryTrail Initial code was downloaded from https://github.com/01org/baytrailaudio/ ...and had the changes to .config stripped and the revert on sound/init.c and printk->pr_debug done by David Henningson Clean-up, port to 4.4 and intel-drm by Pierre Bossart Signed-off-by: David Henningsson Signed-off-by: Pierre-Louis Bossart --- sound/Kconfig |8 + sound/Makefile |1 + sound/hdmi_audio/Makefile |9 + sound/hdmi_audio/intel_mid_hdmi_audio.c| 2029 sound/hdmi_audio/intel_mid_hdmi_audio.h| 740 ++ sound/hdmi_audio/intel_mid_hdmi_audio_if.c | 514 +++ 6 files changed, 3301 insertions(+) create mode 100644 sound/hdmi_audio/Makefile create mode 100644 sound/hdmi_audio/intel_mid_hdmi_audio.c create mode 100644 sound/hdmi_audio/intel_mid_hdmi_audio.h create mode 100644 sound/hdmi_audio/intel_mid_hdmi_audio_if.c diff --git a/sound/Kconfig b/sound/Kconfig index 5a240e0..75c679e 100644 --- a/sound/Kconfig +++ b/sound/Kconfig @@ -134,3 +134,11 @@ config AC97_BUS sound subsystem and other function drivers completely unrelated to sound although they're sharing the AC97 bus. Concerned drivers should "select" this. + +config SUPPORT_HDMI +bool "SUPPORT_HDMI" +depends on DRM_I915 +default n +help + Choose this option to support HDMI. + diff --git a/sound/Makefile b/sound/Makefile index 7732070..f2c5e82 100644 --- a/sound/Makefile +++ b/sound/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DMASOUND) += oss/ obj-$(CONFIG_SND) += core/ i2c/ drivers/ isa/ pci/ ppc/ arm/ sh/ synth/ usb/ \ firewire/ sparc/ spi/ parisc/ pcmcia/ mips/ soc/ atmel/ hda/ obj-$(CONFIG_SND_AOA) += aoa/ +obj-$(CONFIG_SUPPORT_HDMI) += hdmi_audio/ # This one must be compilable even if sound is configured out obj-$(CONFIG_AC97_BUS) += ac97_bus.o diff --git a/sound/hdmi_audio/Makefile b/sound/hdmi_audio/Makefile new file mode 100644 index 000..b28eb3b --- /dev/null +++ b/sound/hdmi_audio/Makefile @@ -0,0 +1,9 @@ +DRIVER_NAME := hdmi_audio + +ccflags-y += -Idrivers/gpu/drm/i915 + +$(DRIVER_NAME)-objs += \ + intel_mid_hdmi_audio.o \ + intel_mid_hdmi_audio_if.o + +obj-m += $(DRIVER_NAME).o diff --git a/sound/hdmi_audio/intel_mid_hdmi_audio.c b/sound/hdmi_audio/intel_mid_hdmi_audio.c new file mode 100644 index 000..f397435 --- /dev/null +++ b/sound/hdmi_audio/intel_mid_hdmi_audio.c @@ -0,0 +1,2029 @@ +/* + * intel_mid_hdmi_audio.c - Intel HDMI audio driver for MID + * + * Copyright (C) 2010 Intel Corp + * Authors: Sailaja Bandarupalli + * Ramesh Babu K V + * Vaibhav Agarwal + * ~~ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * ~~ + * ALSA driver for Intel MID HDMI audio controller + */ + +#define pr_fmt(fmt)"had: " fmt + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "intel_mid_hdmi_audio.h" + +#define PCM_INDEX 0 +#define MAX_PB_STREAMS 1 +#define MAX_CAP_STREAMS0 +#define HDMI_AUDIO_DRIVER "hdmi-audio" +static DEFINE_MUTEX(had_mutex); + +/*standard module options for ALSA. This module supports only one card*/ +static int hdmi_card_index = SNDRV_DEFAULT_IDX1; +static char *hdmi_card_id = SNDRV_DEFAULT_STR1; +static struct snd_intelhad *had_data; + +module_param(hdmi_card_index, int, 0444); +MODULE_PARM_DESC(hdmi_card_index, + "Index value for INTEL Intel HDMI Audio controller."); +module_param(hdmi_card_id, charp, 0444); +MODULE_PARM_DESC(hdmi_card_id, + "ID string for INTEL Intel HDMI Audio controller."); +MODULE_AUTHOR("Sailaja Bandarupalli "); +MODULE_AUTHOR("Ramesh Babu K V "); +MODULE_AUTHOR("Vaibhav Agarwal "); +MODULE_DESCRIPTION("Intel HDMI Audio driver"); +MODULE_LICENSE("GPL v2"); +MODULE_SUPPORTED_DEVICE("{Intel,Intel_HAD}"); +MODULE_VERSION(HAD_DRIVER_VERSION); + +#define INFO_FRAME_WORD1 0x000a0184 +#define FIFO_THRESHOLD 0xFE +#define DMA_FIFO_THRESHOLD 0x7 +#define BYTES_PER_WORD 0x4 + +/* Sampling rate as per IEC60958 Ver 3 */ +#def
[Intel-gfx] [RFC 01/15] drm: i915: fix inversion of definitions for LPE_PIPE_A/B
Definitions for I915_LPE_PIPE_A_INTERRUPT and I915_LPE_PIPE_B_INTERRUPT are inverted. Signed-off-by: Pierre-Louis Bossart --- drivers/gpu/drm/i915/i915_reg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 71abf57..228b22f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2029,8 +2029,8 @@ enum skl_disp_power_wells { #define I915_PM_INTERRUPT (1<<31) #define I915_ISP_INTERRUPT (1<<22) -#define I915_LPE_PIPE_B_INTERRUPT (1<<21) -#define I915_LPE_PIPE_A_INTERRUPT (1<<20) +#define I915_LPE_PIPE_A_INTERRUPT (1<<21) +#define I915_LPE_PIPE_B_INTERRUPT (1<<20) #define I915_MIPIC_INTERRUPT (1<<19) #define I915_MIPIA_INTERRUPT (1<<18) #define I915_PIPE_CONTROL_NOTIFY_INTERRUPT (1<<18) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 00/15] HDMI Audio support on Atom w/o HDAUdio
When HDaudio is not enabled or fused-out, an alternate hardware interface can be used to provide audio data to the display/HDMI controller on Atom platforms. The code to control this interface was never submitted upstream but has been used extensively in Android programs on Medfield, Clovertrail, Merrifield, Moorefield, Baytrail-T and CherryTrail-T, as well as the Baytrail Compute Stick w/ Ubuntu. Jesse Barnes and others at Intel suggested the code be posted on the intex-gfx mailing lists as an RFC for graphics and audio folks to start the conversation on how to support this interface in the mainline kernel. The initial patches used for the Baytrail Compute Stick were split to make them more readable, with changes to drm/i915 and sound/ clearly separated out. A basic cleanup to make checkpatch happy was done but the code needs further updates (we know...). The code was rebased from 3.16 to drm/intel-nightly-build and tested on a Baytrail Compute Stick. CherryTrail changes will be added shortly. The main feedback expected in this RFC is on the interaction between audio and display driver, specifically if we can wrap the interface control with the component framework already used between i915 and HDaudio. A short documentation was added to help explain how the hardware works. The first two patches fix issues that were identified during the rebase process and could be merged without waiting for the interface to be reworked. Comments welcome! David Henningsson (2): hdmi_audio: Improve position reporting hdmi_audio: Fixup some monitor Pierre-Louis Bossart (10): drm: i915: fix inversion of definitions for LPE_PIPE_A/B drm: i915: remove intel_hdmi variable declaration Doc: sound: add description of Atom HDMI audio interface drm/i915: Add headers for non-HDAudio HDMI interface drm/i915: changes for non-HDAudio HDMI interface drm/i915: power-related changes non-HDAudio HDMI interface drm/i915: Add API code for non-HDAudio HDMI interface drm/i915: enable non-HDAudio HDMI interface Makefile ALSA: Intel: Atom: add Atom non-HDAudio HDMI interface add dependency on PM_RUNTIME Toyo Abe (3): hdmi_audio: Fix mishandling of AUD_HDMI_STATUS_v2 register. i915: Enable LPE_PIPEA_Interrupt. i915: Fix typo in comment. Documentation/sound/alsa/Atom-hdmi-audio.txt | 241 +++ drivers/gpu/drm/i915/Makefile|4 +- drivers/gpu/drm/i915/hdmi_audio_if.c | 410 ++ drivers/gpu/drm/i915/hdmi_audio_if.h | 122 ++ drivers/gpu/drm/i915/i915_drv.h | 31 + drivers/gpu/drm/i915/i915_irq.c | 87 ++ drivers/gpu/drm/i915/i915_reg.h | 11 +- drivers/gpu/drm/i915/i915_rpm.c | 476 ++ drivers/gpu/drm/i915/intel_display.c |8 + drivers/gpu/drm/i915/intel_drv.h | 11 + drivers/gpu/drm/i915/intel_hdmi.c| 185 ++- drivers/gpu/drm/i915/intel_pm.c | 53 + sound/Kconfig|9 + sound/Makefile |1 + sound/hdmi_audio/Makefile|9 + sound/hdmi_audio/intel_mid_hdmi_audio.c | 2039 ++ sound/hdmi_audio/intel_mid_hdmi_audio.h | 740 ++ sound/hdmi_audio/intel_mid_hdmi_audio_if.c | 514 +++ 18 files changed, 4946 insertions(+), 5 deletions(-) create mode 100644 Documentation/sound/alsa/Atom-hdmi-audio.txt create mode 100644 drivers/gpu/drm/i915/hdmi_audio_if.c create mode 100644 drivers/gpu/drm/i915/hdmi_audio_if.h create mode 100644 drivers/gpu/drm/i915/i915_rpm.c create mode 100644 sound/hdmi_audio/Makefile create mode 100644 sound/hdmi_audio/intel_mid_hdmi_audio.c create mode 100644 sound/hdmi_audio/intel_mid_hdmi_audio.h create mode 100644 sound/hdmi_audio/intel_mid_hdmi_audio_if.c -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx