13 Jan, 2020
1 commit
-
In the commit 8e85def5723e ("ALSA: hda: enable regmap internal
locking"), we re-enabled the regmap lock due to the reported
regression that showed the possible concurrent accesses. It was a
temporary workaround, and there are still a few opened races even
after the revert. In this patch, we cover those still opened windows
with a proper mutex lock and disable the regmap internal lock again.First off, the patch introduces a new snd_hdac_device.regmap_lock
mutex that is applied for each snd_hdac_regmap_*() call, including
read, write and update helpers. The mutex is applied carefully so
that it won't block the self-power-up procedure in the helper
function. Also, this assures the protection for the accesses without
regmap, too.The snd_hdac_regmap_update_raw() is refactored to use the standard
regmap_update_bits_check() function instead of the open-code. The
non-regmap case is still open-coded but it's an easy part. The all
read and write operations are in the single mutex protection, so it's
now race-free.In addition, a couple of new helper functions are added:
snd_hdac_regmap_update_raw_once() and snd_hdac_regmap_sync(). Both
are called from HD-audio legacy driver. The former is to initialize
the given verb bits but only once when it's not initialized yet. Due
to this condition, the function invokes regcache_cache_only(), and
it's now performed inside the regmap_lock (formerly it was racy) too.
The latter function is for simply invoking regcache_sync() inside the
regmap_lock, which is called from the codec resume call path.
Along with that, the HD-audio codec driver code is slightly modified /
simplified to adapt those new functions.And finally, snd_hdac_regmap_read_raw(), *_write_raw(), etc are
rewritten with the helper macro. It's just for simplification because
the code logic is identical among all those functions.Tested-by: Kai Vehmanen
Link: https://lore.kernel.org/r/20200109090104.26073-1-tiwai@suse.de
Signed-off-by: Takashi Iwai
11 Jan, 2020
1 commit
-
Sync 5.5-devel branch once again for applying the HD-audio fixes.
Signed-off-by: Takashi Iwai
09 Jan, 2020
1 commit
-
This reverts commit 42ec336f1f9d ("ALSA: hda: Disable regmap
internal locking").Without regmap locking, there is a race between snd_hda_codec_amp_init()
and PM callbacks issuing regcache_sync(). This was caught by
following kernel warning trace:[358.080081] WARNING: CPU: 2 PID: 4157 at drivers/base/regmap/regcache.c:498 regcache_cache_only+0xf5/0x130
[...]
[358.080148] Call Trace:
[358.080158] snd_hda_codec_amp_init+0x4e/0x100 [snd_hda_codec]
[358.080169] snd_hda_codec_amp_init_stereo+0x40/0x80 [snd_hda_codec]Suggested-by: Takashi Iwai
BugLink: https://gitlab.freedesktop.org/drm/intel/issues/592
Signed-off-by: Kai Vehmanen
Link: https://lore.kernel.org/r/20200108180856.5194-1-kai.vehmanen@linux.intel.com
Signed-off-by: Takashi Iwai
08 Jan, 2020
1 commit
-
Link: https://lore.kernel.org/r/20200108153430.31456-1-tiwai@suse.de
Signed-off-by: Takashi Iwai
05 Nov, 2019
1 commit
-
Since we apply the own mutex (bus->cmd_mutex) in HDA core side, the
internal locking in regmap is superfluous. This patch adds the flag
to indicate that.Also, an infamous side-effect by this change is that it disables the
regmap debugfs, too, and this is seen rather good; the regmap debugfs
isn't quite useful for HD-audio as it provides the very sparse
registers and its debugfs access tends to lead to the way too high
resource usages or sometimes hang up. So it'd be rather safe to
disable it altogether.Link: https://lore.kernel.org/r/2029139028.10333037.1572874551626.JavaMail.zimbra@redhat.com
Link: https://lore.kernel.org/r/20191105081806.4896-1-tiwai@suse.de
Signed-off-by: Takashi Iwai
15 Aug, 2019
1 commit
-
Drop EXPORT_SYMBOL*() from a few more stuff in HD-audio core that
aren't used outside. Particular the unsol event handler can be
staticized now because the recent change removed all external
callers.Signed-off-by: Takashi Iwai
21 May, 2019
1 commit
-
Add SPDX license identifiers to all files which:
- Have no license information of any form
- Have EXPORT_.*_SYMBOL_GPL inside which was used in the
initial scan/conversion to ignore the fileThese files fall under the project license, GPL v2 only. The resulting SPDX
license identifier is:GPL-2.0-only
Signed-off-by: Thomas Gleixner
Signed-off-by: Greg Kroah-Hartman
07 Sep, 2018
1 commit
-
Split regmap_config.use_single_rw into use_single_read and
use_single_write. This change enables drivers of devices which only
support bulk operations in one direction to use the regmap_bulk_*()
functions for both directions and have their bulk operation split into
single operations only when necessary.Update all struct regmap_config instances where use_single_rw==true to
instead set both use_single_read and use_single_write. No attempt was
made to evaluate whether it is possible to set only one of
use_single_read or use_single_write.Signed-off-by: David Frey
Signed-off-by: Mark Brown
24 Apr, 2018
1 commit
-
Introduce a new helper macro, snd_array_for_each(), to iterate for
each snd_array element. It slightly improves the readability than
lengthy open codes at each place.Along with it, add const prefix to some obvious places.
There should be no functional changes by this.
Signed-off-by: Takashi Iwai
17 Jun, 2016
1 commit
-
Call path:
1) snd_hdac_power_up_pm()
2) snd_hdac_power_up()
3) pm_runtime_get_sync()
4) __pm_runtime_resume()
5) rpm_resume()The rpm_resume() returns 1 when the device is already active.
Because the return value is unmodified, the hdac regmap read/write
functions should allow this value for the retry I/O operation, too.Signed-off-by: Jaroslav Kysela
Cc:
Signed-off-by: Takashi Iwai
21 Apr, 2016
1 commit
-
HD-audio driver uses regmap cache bypass feature for reading a raw
value without the cache. But this is racy since both the cached and
the uncached reads may occur concurrently. The former is done via the
normal control API access while the latter comes from the proc file
read.Even though the regmap itself has the protection against the
concurrent accesses, the flag set/reset is done without the
protection, so it may lead to inconsistent state of bypass flag that
doesn't match with the current read and occasionally result in a
kernel WARNING like:
WARNING: CPU: 3 PID: 2731 at drivers/base/regmap/regcache.c:499 regcache_cache_only+0x78/0x93One way to work around such a problem is to wrap with a mutex. But in
this case, the solution is simpler: for the uncached read, we just
skip the regmap and directly calls its accessor. The verb execution
there is protected by itself, so basically it's safe to call
individually.Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=116171
Signed-off-by: Takashi Iwai
08 Mar, 2016
1 commit
-
HD-audio driver has a mechanism to trigger the runtime resume
automatically at accessing the verbs. This auto-resume, however,
causes the mutex deadlock when invoked from the regmap handler since
the regmap keeps the mutex while auto-resuming. For avoiding that,
there is some tricky check in the HDA regmap handler to return -EAGAIN
error to back-off when the codec is powered down. Then the caller of
regmap r/w will retry after properly turning on the codec power.This works in most cases, but there seems a slight race between the
codec power check and the actual on-demand auto-resume trigger. This
resulted in the lockdep splat, eventually leading to a real deadlock.This patch tries to address the race window by getting the runtime PM
refcount at the check time using pm_runtime_get_if_in_use(). With
this call, we can keep the power on only when the codec has been
already turned on, and back off if not.For keeping the code consistency, the code touching the runtime PM is
stored in hdac_device.c although it's used only locally in
hdac_regmap.c.Reported-by: Jiri Slaby
Cc:
Signed-off-by: Takashi Iwai
28 Oct, 2015
1 commit
-
Give some readable comment in kernel doc style for each exported
function, as I promised in the previous meetings. While we're at it,
fix the wrong comments, too.Signed-off-by: Takashi Iwai
17 Jul, 2015
1 commit
-
This patch changes the return type of snd_hdac_power_up/down() and
variants to pass the error code from the underlying
pm_runtime_get/put() calls. Currently they are ignored, but in most
places, these should be handled properly.As an example, the regmap handler is updated to check the return value
and accesses the register only when the wakeup succeeds.Signed-off-by: Takashi Iwai
11 Jun, 2015
1 commit
-
Yet another regression by the transition to regmap cache; for better
usability, we had the fake mute control using the zero amp value for
Conexant codecs, and this was forgotten in the new hda core code.Since the bits 4-7 are unused for the amp registers (as we follow the
syntax of AMP_GET verb), the bit 4 is now used to indicate the fake
mute. For setting this flag, snd_hda_codec_amp_update() becomes a
function from a simple macro. The bonus is that it gained a proper
function description.Signed-off-by: Takashi Iwai
10 Jun, 2015
1 commit
-
Along with the transition to regmap for managing the cached parameter
reads, the caps overwrite was also moved to regmap cache. The cache
change itself works, but it still tries to write the non-existing verb
(the HDA parameter is read-only) wrongly. It's harmless in most
cases, but some chips are picky and may result in the codec
communication stall.This patch avoids it just by adding the missing flag check in
reg_write ops.Signed-off-by: Takashi Iwai
14 Apr, 2015
1 commit
-
Some HD-A codecs may add their own vendor 'set' verb to the regmap, thru func
snd_hdac_add_vendor_verb(). This patch sets the GET bit (bit 11) when adding
the verb so that its peer vendor 'get' verb is actually added. This can avoid
I/O error when writing the 'set' verb thru remap, since HD-A regmap internally
looks up a writable vendor verb with GET bit set at first.Signed-off-by: Mengdong Lin
Signed-off-by: Takashi Iwai
09 Apr, 2015
1 commit
-
The hdac regmap code checks whether the codec is powered on while
accessing, but there must be an exception -- the verbs to control the
power state.Currently HD-audio driver doesn't access them via regmap, so this
patch doesn't fix any current behavior, but it's just for future.Signed-off-by: Takashi Iwai
08 Apr, 2015
1 commit
-
Currently, snd_hdac_power_up()/down() helpers checks whether the codec
is being in pm (suspend/resume), and skips the call of runtime get/put
during it. This is needed as there are lots of power up/down
sequences called in the paths that are also used in the PM itself. An
example is found in hda_codec.c::codec_exec_verb(), where this can
power up the codec while it may be called again in its power up
sequence, too.The above works in most cases, but sometimes we really want to wait
for the real power up. For example, the control element get/put may
want explicit power up so that the value change is assured to reach to
the hardware. Using the current snd_hdac_power_up(), however,
results in a race, e.g. when it's called during the runtime suspend is
being performed. In the worst case, as found in patch_ca0132.c, it
can even lead to the deadlock because the code assumes the power up
while it was skipped due to the check above.For dealing with such cases, this patch makes snd_hdac_power_up() and
_down() to two variants: with and without in_pm flag check. The
version with pm flag check is named as snd_hdac_power_up_pm() while
the version without pm flag check is still kept as
snd_hdac_power_up(). (Just because the usage of the former is fewer.)Then finally, the patch replaces each call potentially done in PM with
the new _pm() variant.In theory, we can implement a unified version -- if we can distinguish
the current context whether it's in the pm path. But such an
implementation is cumbersome, so leave the code like this a bit messy
way for now...Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96271
Signed-off-by: Takashi Iwai
27 Mar, 2015
1 commit
-
HD-audio doesn't support the bulk access. Currently it works even
without this flag as implicitly assumed, but it's safer to set
explicitly.Signed-off-by: Takashi Iwai
26 Mar, 2015
1 commit
-
Although they can be written, handle a few verbs as read-only in
regmap interface: CONFIG_DEFAULT, CONV and CVT_CHAN_COUNT. These are
either updated in PCM or HDMI management code in a volatile manner, or
just needed only as parameter, thus they don't need to be written at
resume sync.Signed-off-by: Takashi Iwai
23 Mar, 2015
6 commits
-
The 16bit COEF read/write is pretty standard for many codecs, and they
can be cached in most cases -- more importantly, they need to be
restored at resume. For making this easier, add the cache support to
regmap. If the codec driver wants to cache the COEF access, set
codec->cache_coef flag and issue AC_VERB_GET_PROC_COEF with the coef
index in LSB 8 bits.Signed-off-by: Takashi Iwai
-
HD-audio has quite a few asymmetrical ways of accessing verbs, and one
of typical ones is GET/SET_POWER_STATE verbs. While it takes only the
power state for setting, it returns a combination of states for
getting. For making the state handling simpler, this patch adds a
code to translate the value returned from GET_POWER_STATE to return
only the actual state or -1 for error. In that way, the driver can
simplify the power state management.Signed-off-by: Takashi Iwai
-
HD-audio spec is inconvenient regarding the handling of stereo volume
controls. It can set and get only single channel at once (although
there is a special option to set the same value to both channels).
This patch provides a fake pseudo-register via the regmap access so
that the stereo channels can be read and written by a single call.
It'd be useful, for example, for implementing DAPM widgets.A stereo amp pseudo register consists of the encoding like the normal
amp verbs but it has both SET_LEFT (bit 13) and SET_RIGHT (bit 12)
bits set. The regmap reads and writes a 16bit value for this pseudo
register where the upper 8bit is for the right chanel and the lower
8bit for the left channel.Note that the driver doesn't recognize conflicts when both stereo and
mono channel registers are mixed. Mixing them would certainly confuse
the operation. So, use carefully.Signed-off-by: Takashi Iwai
-
Codecs may have own vendor-specific verbs, and we need to allow each
driver to give such verbs for cached accesses. Here a verb can be put
into a single array and looked through it at readable and writeable
callbacks.Signed-off-by: Takashi Iwai
-
The amp hash table was used for recording the cached reads of some
capability values like pin caps or amp caps. Now all these are moved
to regmap as well.One addition to the regmap helper is codec->caps_overwriting flag.
This is set in snd_hdac_override_parm(), and the regmap helper accepts
any register while this flag is set, so that it can overwrite even the
read-only verb like AC_VERB_PARAMETERS. The flag is cleared
immediately in snd_hdac_override_parm(), as it's a once-off flag.Along with these changes, the no longer needed amp hash and relevant
fields are removed from hda_codec struct now.Signed-off-by: Takashi Iwai
-
This patch adds an infrastructure to support regmap-based verb
accesses. Because o the asymmetric nature of HD-audio verbs,
especially the amp verbs, we need to translate the verbs as a sort of
pseudo registers to be mapped uniquely in regmap.In this patch, a pseudo register is built from the NID, the
AC_VERB_GET_* and 8bit parameters, i.e. almost in the form to be sent
to HD-audio bus but without codec address field. OTOH, for writing,
the same pseudo register is translated to AC_VERB_SET_* automatically.
The AC_VERB_SET_AMP_* verb is re-encoded from the corresponding
AC_VERB_GET_AMP_* verb and parameter at writing.Some verbs has a single command for read but multiple for writes. A
write for such a verb is split automatically to multiple verbs.The patch provides also a few handy helper functions. They are
designed to be accessible even without regmap. When no regmap is set
up (e.g. before the codec device instantiation), the direct hardware
access is used. Also, it tries to avoid the unnecessary power-up.
The power up/down sequence is performed only on demand.The codec driver needs to call snd_hdac_regmap_exit() and
snd_hdac_regmap_exit() at probe and remove if it wants the regmap
access.There is one flag added to hdac_device. When the flag lazy_cache is
set, regmap helper ignores a write for a suspended device and returns
as if it was actually written. It reduces the hardware access pretty
much, e.g. when adjusting the mixer volume while in idle. This
assumes that the driver will sync the cache later at resume properly,
so use it carefully.Signed-off-by: Takashi Iwai