Re: [Intel-gfx] [alsa-devel] Timing issues between ALSA and i915 drivers

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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)

[Pierre-Louis Bossart]

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ä <ville.syrj...@linux.intel.com>

Okay, here's the second attempt at getting multiple pipes playing back
audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
that now the PCM devices are associated with ports instead of pipes,
so the audio from one device always gets output on the same display.

I've also tacked on the alsa-lib conf update. No clue whether it's
really correct or not (the config language isn't a close friend
of mine).

BTW I did notice that with LPE audio all the controls say iface=PCM,
whereas on HDA a bunch of them say iface=MIXER. No idea if that's
OK or not, just something I spotted when I was comparing the results
with HDA.

We generally accept both iface types for IEC958 stuff, since
historically many drivers have already mixed them up.  So it's no
problem :)



Entire series available here:
git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2

Cc: Takashi Iwai <ti...@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>

All look good, and feel free to take my reviewed-by tag
Reviewed-by: Takashi Iwai <ti...@suse.de>

As said previously, my only slight concern is the compatibility.
But, in the current situation with PulseAudio, only few people would
use this driver, so it shouldn't be so big impact, I suppose.

BTW, which port is used in general on BYT/CHT?

Oh, also, I suppose you want to carry these over i915 tree?
I don't mind either way, I can take them through sound tree if
preferred, too.

I see frequent oops on startup with this lpe_audio_multipipe_2 branch
with my CHT device not booting or no HDMI audio device created.
Not sure if these issues are due to the new patches or to the rest of
the drm code?

[5.529023] BUG: unable to handle kernel NULL pointer dereference
at   (null)
[5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
[5.529202] PGD 0

[5.529242] Oops:  [#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: 55d6c745b6

Re: [Intel-gfx] [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Pierre-Louis Bossart]

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ä <ville.syrj...@linux.intel.com>

Okay, here's the second attempt at getting multiple pipes playing back
audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
that now the PCM devices are associated with ports instead of pipes,
so the audio from one device always gets output on the same display.

I've also tacked on the alsa-lib conf update. No clue whether it's
really correct or not (the config language isn't a close friend
of mine).

BTW I did notice that with LPE audio all the controls say iface=PCM,
whereas on HDA a bunch of them say iface=MIXER. No idea if that's
OK or not, just something I spotted when I was comparing the results
with HDA.

We generally accept both iface types for IEC958 stuff, since
historically many drivers have already mixed them up.  So it's no
problem :)



Entire series available here:
git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2

Cc: Takashi Iwai <ti...@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>

All look good, and feel free to take my reviewed-by tag
Reviewed-by: Takashi Iwai <ti...@suse.de>

As said previously, my only slight concern is the compatibility.
But, in the current situation with PulseAudio, only few people would
use this driver, so it shouldn't be so big impact, I suppose.

BTW, which port is used in general on BYT/CHT?

Oh, also, I suppose you want to carry these over i915 tree?
I don't mind either way, I can take them through sound tree if
preferred, too.

I see frequent oops on startup with this lpe_audio_multipipe_2 branch
with my CHT device not booting or no HDMI audio device created.
Not sure if these issues are due to the new patches or to the rest of
the drm code?

[5.529023] BUG: unable to handle kernel NULL pointer dereference
at   (null)
[5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
[5.529202] PGD 0

[5.529242] Oops:  [#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:
000

Re: [Intel-gfx] [alsa-devel] [PATCH v2 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card

[Pierre-Louis Bossart]

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ä <ville.syrj...@linux.intel.com>
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 <ti...@suse.de>
    Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

: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ä <ville.syrj...@linux.intel.com>

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 <ti...@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
  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(>lpe_audio_slock);
  
  	platdev = platform_device_register_full();

@@ -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(_priv->lpe_audio.platdev->dev);

-   ppdata = >port;
+   ppdata = >port[port];
  
-	spin_lock_irqsave(>lpe_audio_slock, irq_flags);

+   spin_lock_irqsave(>lpe_audio_slock, irqflags);
  
  	audio_enable = 

Re: [Intel-gfx] [alsa-devel] [PATCH v2 00/11] drm/i915: LPE audio runtime PM and multipipe (v2)

[Pierre-Louis Bossart]

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ä <ville.syrj...@linux.intel.com>

Okay, here's the second attempt at getting multiple pipes playing back
audio on the VLV/CHV HDMI LPE audio device. The main change from v1 is
that now the PCM devices are associated with ports instead of pipes,
so the audio from one device always gets output on the same display.

I've also tacked on the alsa-lib conf update. No clue whether it's
really correct or not (the config language isn't a close friend
of mine).

BTW I did notice that with LPE audio all the controls say iface=PCM,
whereas on HDA a bunch of them say iface=MIXER. No idea if that's
OK or not, just something I spotted when I was comparing the results
with HDA.

We generally accept both iface types for IEC958 stuff, since
historically many drivers have already mixed them up.  So it's no
problem :)



Entire series available here:
git://github.com/vsyrjala/linux.git lpe_audio_multipipe_2

Cc: Takashi Iwai <ti...@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>

All look good, and feel free to take my reviewed-by tag
   Reviewed-by: Takashi Iwai <ti...@suse.de>

As said previously, my only slight concern is the compatibility.
But, in the current situation with PulseAudio, only few people would
use this driver, so it shouldn't be so big impact, I suppose.

BTW, which port is used in general on BYT/CHT?

Oh, also, I suppose you want to carry these over i915 tree?
I don't mind either way, I can take them through sound tree if
preferred, too.
I see frequent oops on startup with this lpe_audio_multipipe_2 branch 
with my CHT device not booting or no HDMI audio device created.
Not sure if these issues are due to the new patches or to the rest of 
the drm code?


[5.529023] BUG: unable to handle kernel NULL pointer dereference 
at   (null)

[5.529143] IP: hdmi_lpe_audio_probe+0x40f/0x650 [snd_hdmi_lpe_audio]
[5.529202] PGD 0

[5.529242] Oops:  [#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] R

Re: [Intel-gfx] [PATCH 03/11] drm/i915: Stop pretending to mask/unmask LPE audio interrupts

[Pierre-Louis Bossart]

On 4/25/17 3:27 PM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä <ville.syrj...@linux.intel.com>

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 <ti...@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
 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(_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(_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(_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(_priv->irq_

Re: [Intel-gfx] [alsa-devel] [PATCH 05/11] drm/i915: Replace tmds_clock_speed and link_rate with just ls_clock

[Pierre-Louis Bossart]

On 4/25/17 3:27 PM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä <ville.syrj...@linux.intel.com>

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 <ti...@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
 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 

Re: [Intel-gfx] [alsa-devel] [PATCH 11/11] ALSA: x86: Register multiple PCM devices for the LPE audio card

[Pierre-Louis Bossart]

On 04/25/2017 03:27 PM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä <ville.syrj...@linux.intel.com>

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 <ti...@suse.de>
Cc: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
  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(>lpe_audio_slock);
  
  	platdev = platform_device_register_full();

@@ -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(_priv->lpe_audio.platdev->dev);

-   ppdata = >pipe;
+   ppdata = >pipe[pipe];
  
-	spin_lock_irqsave(>lpe_audio_slock, irq_flags);

+   spin_lock_irqsave(>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(>lpe_audio_slock,

-   irq_flags);
+   spin_unlock_irqrestore(>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];
+   int num_pipes;
  
-	void (*notify_a

Re: [Intel-gfx] [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Pierre-Louis Bossart]

+#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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Takashi Iwai <ti...@suse.de>
---
  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(_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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
---
 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(_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(_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

[Pierre-Louis Bossart]

If DisplayPort is detected, pass flag and link rate to audio driver

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
---
 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 0/5] DisplayPort Audio on Cherrytrail

[Pierre-Louis Bossart]

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

[Intel-gfx] [RFC PATCH 2/5] ALSA: x86: intel_hdmi: add definitions and logic for DP audio

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
---
 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, >eeld);
+   had_get_caps(HAD_GET_DP_OUTPUT, >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;
+  

Re: [Intel-gfx] [PATCH v5 0/5] Add support for Legacy HDMI audio drivers

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

 #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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>
---
 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), );
-   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

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>
---
 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

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>
---
 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), );
+   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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>


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 <jerome.an...@intel.com>
+ * Aravind Siddappaji <aravindx.siddapp...@intel.com>
+ *  ~~
+ *
+ *  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 *)_eld, sizeof(hlpe_eld));
+
+   {
+   int i;
+   uint8_t *eld_data = (uint8_t *)_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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>
---
 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(_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(>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 t

Re: [Intel-gfx] [alsa-devel] [PATCH V2 1/7] drm/i915: setup bridge for HDMI LPE audio driver

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>
---
 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_i

Re: [Intel-gfx] [alsa-devel] [PATCH 7/7] ALSA: x86: hdmi: continue playback even when display resolution changes

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

>>> +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

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>


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

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
Signed-off-by: Jerome Anand <jerome.an...@intel.com>


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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
---
 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, _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, >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

[Pierre-Louis Bossart]

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 <airl...@redhat.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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, >payloads[i]);
+   drm_dp_destroy_payload_step1(mgr, port, 
mgr->payloads[i].vcpi, >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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
---
 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, _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, >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

[Pierre-Louis Bossart]

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) ? >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(>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

[Pierre-Louis Bossart]

On 11/28/16 11:01 AM, Ville Syrjälä wrote:

+   if (pdata->notify_audio_lpe)
+   pdata->notify_audio_lpe(
+   (eld != NULL) ? >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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

+/* 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(_priv->irq_lock, irqflags);
+
+   imr = I915_READ(VLV_IMR);
+
+   if (IS_CHERRYVIEW(_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

[Pierre-Louis Bossart]

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 

Re: [Intel-gfx] i915 issues with 4.6

[Pierre-Louis Bossart]

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ä <ville.syrj...@linux.intel.com
<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

[Pierre-Louis Bossart]

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ä <ville.syrj...@linux.intel.com 
<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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

'intel_hdmi' variable was redeclared, use same variable declared in
function scope

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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 <pierre-louis.boss...@linux.intel.com>
---
  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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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

[Pierre-Louis Bossart]

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.

[Pierre-Louis Bossart]

From: Toyo Abe <toyo@gmail.com>

Signed-off-by: Toyo Abe <toyo@gmail.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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(_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

[Pierre-Louis Bossart]

From: David Henningsson <david.hennings...@canonical.com>

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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.

[Pierre-Louis Bossart]

From: Toyo Abe <toyo@gmail.com>

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 <toyo@gmail.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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(_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(_priv->irq_lock, irqflags);
 
return 0;
@@ -2878,7 +2881,10 @@ int i915_disable_hdmi_audio_int(struct drm_device *dev)
spin_lock_irqsave(_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(_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

[Pierre-Louis Bossart]

'intel_hdmi' variable is redeclared, use same variable declared in
function scope.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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(_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(_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(_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(_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(_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(_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/drive

[Intel-gfx] [RFC 04/15] drm/i915: Add headers for non-HDAudio HDMI interface

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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 <jim@intel.com>
+ * Uma Shankar <uma.shan...@intel.com>
+ */
+
+
+#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 &quo

[Intel-gfx] [RFC 08/15] drm/i915: enable non-HDAudio HDMI interface Makefile

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

Used when HDaudio is not enabled

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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 stere

[Intel-gfx] [RFC 13/15] hdmi_audio: Fix mishandling of AUD_HDMI_STATUS_v2 register.

[Pierre-Louis Bossart]

From: Toyo Abe <toyo@gmail.com>

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 <toyo@gmail.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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 <naresh.kumar.kac...@intel.com>
+ */
+
+#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) {
+ 

[Intel-gfx] [RFC 11/15] hdmi_audio: Improve position reporting

[Pierre-Louis Bossart]

From: David Henningsson <david.hennings...@canonical.com>

Using a hw register to calculate sub-period position reports.

This makes PulseAudio happier.

Signed-off-by: David Henningsson <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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), );
+   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

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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 <jim@intel.com>
+ * Uma Shankar <uma.shan...@intel.com>
+ */
+
+#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__,
+

[Intel-gfx] [RFC 09/15] ALSA: Intel: Atom: add Atom non-HDAudio HDMI interface

[Pierre-Louis Bossart]

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 <david.hennings...@canonical.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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 <sailaja.bandarupa...@intel.com>
+ * Ramesh Babu K V <ramesh.b...@intel.com>
+ * Vaibhav Agarwal <vaibhav.agar...@intel.com>
+ *  ~~
+ *
+ *  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 <sailaja.bandarupa...@intel.com>");
+MODULE_AUTHOR("Ramesh Babu K V <ramesh.b...@intel.com>");
+MODULE_AUTHOR("Vaibhav Agarwal <vaibhav.agar...@intel.com>");
+MODULE_DESCRIPTION("Intel HDMI Audio driver");
+MODULE_LICENSE("GPL v2");
+MODULE_SUPPORTED_DEVI

[Intel-gfx] [RFC 01/15] drm: i915: fix inversion of definitions for LPE_PIPE_A/B

[Pierre-Louis Bossart]

Definitions for I915_LPE_PIPE_A_INTERRUPT and I915_LPE_PIPE_B_INTERRUPT
are inverted.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.boss...@linux.intel.com>
---
 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

[Pierre-Louis Bossart]

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