Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve quirks detection for es8336 #27

Closed
wants to merge 10,000 commits into from

Conversation

mchehab
Copy link

@mchehab mchehab commented Dec 20, 2022

Use _DSM table to detect GPIO level and AMIC. This change has @plbossart patches that apply, and replaces one that doesn't actually work.

Tested on Huawei Matebook D15.

plbossart and others added 30 commits November 23, 2022 20:11
No functionality change, only add indirection for bus management
helpers.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
No functionality change, only add indirection for link power
management helpers.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
No functionality change, only add indirection for in-band wake
management helpers.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
The auxdevice layer is completely generic, it should be split from
intel.c which is only geared to the 'cnl' hw_ops now.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Rander Wang <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
…pt thread

When the code reaches the SoundWire interrupt thread handling, the
interrupt was enabled already, and there is no code that disables it
-> this is a no-op sequence.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Acked-By: Vinod Koul <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
Different generations of Intel hardware rely on different programming
sequences to enable SoundWire IP. In existing hardware, the SoundWire
interrupt is enabled with a register field in the DSP register
space. With HDaudio multi-link extensions registers, the SoundWire
interrupt will be enabled with a generic interrupt enable field in
LCTL, without any dependency on the DSP being enabled.

Add a per-chip callback following the example of the check_sdw_irq()
model already upstream.

Note that the callback is not populated yet for MeteorLake (MTL) since
the interrupts are already enabled in the init. A follow-up patch will
move the functionality to this callback after a couple of cleanups.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
…tions

The offsets and sequences are identical for interrupt enabling and
disabling, we can refactor the code with a single routine and a
boolean.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
There's no real rationale for enabling the SoundWire interrupt in the
init, this can be done from the enable_sdw_irq() callback.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
…tion

The number of links is stored in different registers depending on the
IP version, add sdw_check_lcount() callback. This callback only checks
that the number of links supported in hardware is compatible with the
number of links exposed in ACPI _DSD properties.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
The functionality is implemented with per-chip callbacks, there are no
users of this symbol, remove the code.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Acked-By: Vinod Koul <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
The number of links is checked with a chip-dependent helper in the
caller, remove the check in drivers/soundwire/intel_init.c

This change makes intel_init.c hardware-agnostic - which is quite
fitting for a layer that only creates auxiliary devices.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Acked-By: Vinod Koul <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
There's no reason to delay the multi-link parsing, this can be done
earlier before checking the SoundWire capabilities.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Péter Ujfalusi <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
ALSA callers need to know whether there was a change to the value so
that they can report a control write change correctly.

Signed-off-by: Simon Trimmer <[email protected]>
Signed-off-by: Richard Fitzgerald <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
Functions that update cs_dsp controls need to handle return codes that
indicate whether the control value changed. A return code of 1 indicates
a change, 0 indicates no-change and a negative value is an error
condition.

Acked controls implicitly change value when written so a successful
write shall always report that the value changed.

Signed-off-by: Simon Trimmer <[email protected]>
Signed-off-by: Richard Fitzgerald <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
With incorrect topology file we can end up with a case when swidget->spipe
is NULL and it going to be caught in sof_widget_list_setup().

Check spipe against NULL before dereferencing for the pipe_widget.

Reported-by: Seppo Ingalsuo <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
These fixes were queued for v5.18 but due to me changing my scripting
they never actually got merged - pulling them up now.
As the devm_kcalloc may return NULL, the return value needs to be checked
to avoid NULL poineter dereference.

Fixes: 24caf8d ("ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio")
Signed-off-by: Yuan Can <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
SND_SOC_QCOM_COMMON depends on SOUNDWIRE for some symbols but this
is not explicitly specified using Kconfig depends. On the other hand
SND_SOC_QCOM_COMMON is also directly selected by the sound card
Kconfigs, this could result in various combinations and some symbols
ending up in modules and soundcard that uses those symbols as in-build
driver.

Fix these issues by explicitly specifying the dependencies of
SND_SOC_QCOM_COMMON and also use imply a to select SND_SOC_QCOM_COMMON
so that the symbol is selected based on its dependencies.

Also remove dummy stubs in common.c around CONFIG_SOUNDWIRE

Fixes: 3bd975f ("ASoC: qcom: sm8250: move some code to common")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
Add new acpi id and compatible id for nau8315.

Signed-off-by: David Lin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
The audio amplifier NAU8318 is almost functionally identical to NAU8315.
Adds compatible string "nuvoton,nau8318" for driver reuse.

Signed-off-by: David Lin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
In mt8186 platform, I2S2 should be the main I2S port that provide
the clock, on the contrary I2S3 should be the second I2S port that
use this clock.

Fixes: 9986bda ("ASoC: mediatek: mt8186: Configure shared clocks")
Signed-off-by: Jiaxin Yu <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
If the DSP crashes before the system suspends, the setting of target state
will be skipped because the firmware state will no longer be
SOF_FW_BOOT_COMPLETE. This leads to the incorrect assumption that the
DSP should suspend to D0I3 instead of suspending to D3. To fix this,
set the target_state before we skip to DSP suspend even when the DSP has
crashed.

Signed-off-by: Ranjani Sridharan <[email protected]>
When the DSP is suspended while the firmware is in the crashed state, we
skip tearing down the pipelines. This means that the widget reference
counts will not get to reset to 0 before suspend. This will lead to
errors with resuming audio after system resume. To fix this, invoke the
tear_down_all_pipelines op before skipping to DSP suspend.

Signed-off-by: Ranjani Sridharan <[email protected]>
The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
.probe_new() doesn't get the i2c_device_id * parameter, so determine
that explicitly in the probe function.

Signed-off-by: Uwe Kleine-König <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Reviewed-by: Matt Flax <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
Qualify the KConfig symbol for cs_dsp by adding a FW_ prefix so that
it is more explicit what is being referred to. This is preparation for
using the symbol to namespace the exports.

Signed-off-by: Richard Fitzgerald <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
Move all the exports into a namespace.
This also adds the MODULE_IMPORT_NS to the 3 drivers that use the
exported functions.

Signed-off-by: Richard Fitzgerald <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Mark Brown <[email protected]>
plbossart and others added 19 commits December 13, 2022 09:34
The amplifier may provide hardware support for I/V feedback, or
alternatively the firmware may generate an echo reference attached to
the SSP and dailink used for the amplifier.

To avoid any issues with invalid/NULL substreams in the latter case,
always unconditionally set dpcm_capture.

Link: thesofproject#4083
Signed-off-by: Pierre-Louis Bossart <[email protected]>
…stored

If the pipeline setup fails at the first widget/pipeline then we will have
no spipe stored under the pipeline_list->pipelines, the
pipeline_list->count is 0.

If this is the case we would have a NULL pointer dereference if the
execution is allowed to proceed.

Check for this condition along with the pipeline_list->pipelines check

Signed-off-by: Peter Ujfalusi <[email protected]>
The create pipeline message can carry the target code_id which is set to
0 at the moment.

Add macros to set the core_id in the message extension.

Signed-off-by: Peter Ujfalusi <[email protected]>
Token SOF_TKN_SCHED_CORE in topology file can specify the target core for
the pipeline, if it is missing it is going to be 0 (as it is right now).

Firmware will double-check all information retrieved by topology and
report errors if required. This will allow policy and changes in
topologies without a need for a synchronized kernel change.

Signed-off-by: Peter Ujfalusi <[email protected]>
… error

The sof_widget_free() on the error path will decrement the use count and if
we jump to widget_free: then the use_count will be decremented by two,
which is not correct as we only incremented once with 1.

Signed-off-by: Peter Ujfalusi <[email protected]>
…race

The use_count of the swidget is protect by ALSA core PCM locking with the
exception when an associated kcontrol is changed.

It has been observed that a rightly timed kcontrol access during stream
stop can result of an attempt to send a control update to a widget which
has been freed up between the check of the use_count and the message
sending.

We need to protect the entire sof_widget_setup() and sof_widget_free()
execution to make it safe to rely on the use_count.
Move the code under an _unlocked() function and use a mutex to protect
the execution of the functions for concurrency.
On the control path we need to use the lock only for the kcontrol access,
the widget_kcontrol_setup() op is called with the lock already held.

Reported-by: Guennadi Liakhovetski <[email protected]>
Signed-off-by: Peter Ujfalusi <[email protected]>
When the DSP is disabled in the BIOS, the DSP_BAR and PP_BAR cannot be
accessed.

One possible objection noted in initial reviews is that this patch
adds a number of branches. However the number of branches is actually
limited in probe/suspend/resume routines mostly, so there isn't really
a degradation in terms of readability and maintainability. Adding yet
another level of abstraction/ops/callbacks would increase complexity
and not really help in terms of code reuse or readability and
maintainability. A split between controller and DSP driver would be
even more invasive.

Signed-off-by: Pierre-Louis Bossart <[email protected]>
Add support of SOF on MediaTek MT8188 SoC.
MT8188 ADSP integrates with a single core Cadence HiFi-5 DSP.
The IPC communication between AP and DSP is based on shared DRAM and
mailbox interrupt.

The change in the mt8186.h is compatible on both mt8186 and
mt8188. The register controls booting the DSP core with the
default address or the user specified address. Both mt8186
and mt8188 should boot with the user specified boot in the driver.
The usage of the register is the same on both SoC, but the control bit is
different on mt8186 and mt8188, which is bit 1 on mt8186 and bit 0 on mt8188.
Configure the redundant bit has no side effect on both SoCs.

Signed-off-by: Tinghan Shen <[email protected]>
Set the generic iomem callback for debugfs_add_region_item to support
sof-logger.

Signed-off-by: Tinghan Shen <[email protected]>
Just a couple of checkpatch fixes.

Signed-off-by: Ranjani Sridharan <[email protected]>
Most of the ES83xx codec configuration is exposed in the DSDT table
and accessible via a _DSM method. Start adding basic definitions and
helpers to dump the information.

Reviewed-by: Mauro Carvalho Chehab <[email protected]>
Co-developed-by: David Yang <[email protected]>
Signed-off-by: David Yang <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Read jack detection information from _DSM and pre-set value. The value
may be overridden with a property set by the machine driver.

Reviewed-by: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Read jack detection information from _DSM and pre-set value. The value
may be overridden with a property set by the machine driver.

Reviewed-by: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Pierre-Louis Bossart <[email protected]>
The ACPI entry for es83xx should contain its configuration at
_DSM. It is helpful to debug problems by looking on its contents.

So, add a debug function that outputs its contents as read by the
Kernel.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
The enable GPIOs can either be low or high level activated.

Add a logic to detect it from _DSM.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
The _DSM method has an entry to detect the microphone type,
and where they're attached. Add support for checking it.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Now that we can read GPIO enable level from ACPI _DSM, use it
to properly set the right GPIO levels.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
This is already detected from _DSM, so no need to have a quirk
inside the driver anymore.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Use the _DSM data to fill the analog microphone ports,
removing another quirk.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
@mchehab mchehab changed the base branch from topic/sof-v4.14 to topic/sof-dev December 21, 2022 12:14
@mchehab mchehab changed the base branch from topic/sof-dev to topic/sof-next December 21, 2022 12:18
@mchehab mchehab changed the base branch from topic/sof-next to fix/sof-es8336-quirk December 21, 2022 12:21
@mchehab mchehab changed the base branch from fix/sof-es8336-quirk to topic/sof-dev December 21, 2022 12:23
@mchehab
Copy link
Author

mchehab commented Dec 21, 2022

Hmm... basing it on the top of topic/sof-dev didn't work... it still shows 10k+ commits. I'll just cancel it and open PR against https://github.com/thesofproject/linux/.

@mchehab mchehab closed this Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.