CPF_CURRENTPITCH starts swerving towards PTRX_PITCHTARGET as soon as
that is set. In practice this means that CPF_FRACADDRESS may acquire a
non-zero value before we manage to force CPF_CURRENTPITCH to the final
value, which would prevent bit-for-bit reproduction.
To avoid that this state persists, we now reset CPF_FRACADDRESS when
setting CPF_CURRENTPITCH, and to (mostly) avoid that it progresses too
far in the first place (possibly even reaching CCCA_CURRADDR), we write
PTRX and CPF in one critical section (though NMIs, etc. still make this
unreliable).
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518140339.3722279-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Make terminate_voice() actually do at all what it's supposed to do:
instantly and completely shut down the note.
The bogus behavior was mostly harmless, as usually the voice is freed
right afterwards, which implicitly terminates it anyway.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518140339.3722308-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Compensate for the cache delay, and actually populate the cache.
Without these, the playback would start with garbage (which would be
(mostly?) masqueraded by the attack phase).
Unlike for the PCM voices, this doesn't try to compensate for the
interpolator read-ahead, because it's pointless to be super-exact here.
Note that this code is probably still broken for particularly short
samples, because we ignore the loop-related parts of CCR. But I'm not
going to reverse-engineer that now ...
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518140339.3722308-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
smatch reports
sound/pci/emu10k1/emumixer.c:519:39: warning: symbol
'emu1010_routing_info' was not declared. Should it be static?
sound/pci/emu10k1/emumixer.c:859:36: warning: symbol
'emu1010_pads_info' was not declared. Should it be static?
These variables are only used in their defining file, so it should be static
Signed-off-by: Tom Rix <trix@redhat.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518123826.925752-1-trix@redhat.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
While this nicely denoises the code, the real intent is being able to
write many registers pseudo-atomically, which will come in handy later.
Idea stolen from kX-project.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518093134.3697955-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
We'd try to iterate the voices twice without resetting the pointer.
This went unnoticed, because the code isn't actually in use.
Amends commit 27ae958cf6 ("emu10k1 driver - add multichannel device
hw:x,3 [2-8/8]").
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518093047.3697887-4-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Handle the "timeout" (actually the retry counter) such that it's more
obvious and causes less cost in the normal case.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518093047.3697887-3-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Remove weird INTE_* clearing code. The bits were a subset of the
actually handled interrupts, which kind of contradicted the stated
purpose. I suppose it would make sense to complete the set and negate
it, but interrupts being enabled out of the blue is neither something
that happens a lot, nor should it result in just one error message, IMO.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518093047.3697887-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
IPR_CHANNELNUMBERMASK cannot be non-zero when IPR_CHANNELLOOP is unset,
so join marking them as handled.
This logically reverts part of commit f453e20d8a0 ("ALSA update
0.9.3a"), which made the inverse change with no explanation.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518093047.3697887-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The period_bytes_min parameter and the buffer_bytes minimum constraint
made no sense at all, as they didn't reflect any hardware limitation.
Instead, apply a frame-based period_size minimum constraint, which is
derived from the cache size (it would be actually possible to go below
that, but it would require special handling, and it would be practically
impossible to keep up with the IRQ rate anyway).
Sync up the constraints of the EFX playback with those of the regular
playback, as there is no reason for them to diverge.
N.b., the maximum buffer size is actually arbitrary - the hardware could
go waay beyond 128 KiB.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230518092224.3696958-9-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Pull the special handling of extra voices out of
snd_emu10k1_pcm_init_voice(), simplify snd_emu10k1_playback_pointer(),
and make the logic overall clearer. Also, add verbose comments.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230517174256.3657060-9-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Rename snd_emu10k1_playback_invalidate_cache() to the more apt
snd_emu10k1_playback_fill_cache(), and factor out
snd_emu10k1_playback_prepare_voices(), which calls the former for all
channels.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230517174256.3657060-8-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Originally, there was a 1:1 relationship between the PCM streams' and
the low-level voices' parameters. The addition of multi-channel playback
partially invalidated that, but didn't introduce proper layering, so
things kept working only by virtue of the multi-channel device never
having two channels (yet). The upcoming addition of 32-bit playback
would complete upending the relationships.
So this patch detaches the low-level parameters from the high-level
ones: we pass pre-calculated bit width and stereo flags to the low-level
manipulation functions instead of calculating them in-place from the
stream parameters.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230517174256.3657060-7-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The cache causes a fixed delay regardless of stream parameters.
Consequently, all that "cache invalidate size" calculation stuff was
garbage (which can be traced right back to Creative's OSS driver).
This also removes the definitions of registers CD1..CDF, because they
are accessed only relative to CD0 anyway.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230517174256.3657060-5-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Given that the data is going to be ignored anyway, and that the cache
does not influence interrupt timing (which is the purpose of the extra
voices), it's pointless to pre-fill the cache.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230517174256.3657060-4-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The idea is to make the extra voice lag behind the "real" voices, but
moving the buffer address around doesn't contribute to that, as the CCCA
write below uses the same address. The exact address is unimportant, as
the data is discarded anyway.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230517174256.3657060-3-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This workaround fails to address the underlying problem, which is
actually wholly self-made. Subsequent patches will fix it.
This reverts commit 56385a12d9.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230517174256.3657060-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Shrink the {in,out}put_source arrays and their data type to what is
actually necessary.
To be still on the safe side, add some static asserts.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536508-11-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Unlike the other models, this is actually a distinct card, rather than
an E-MU 1010 with different "dongles". It is stereo only, and supports
no ADAT (there is no trace of ADAT in the manual, switching the output
mode to ADAT has no effect, and switching the input mode to ADAT just
breaks input (presumably ... my only ADAT source is the card's output)).
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536508-10-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The more card models are handled separately, the more code duplication
this saves.
add_emu1010_source_mixers() is factored out the save duplication in a
later commit.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536508-8-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
... and move it to the mixer init, as it's logically part of it.
As a side effect, this fixes the initial values of the input destination
mixer controls, which would have previously remained at "Silent" despite
different defaults. This didn't really matter, though, as ALSA state
restoration would hide that bug beyond first use.
Note that this completely does away with clearing the output routing
registers, as it was rather pointless - we just programmed the FPGA
(resetting it first if necessary), so everything is zeroed anyway
(that's documented by Xilinx, and as further evidence, some of the loops
terminated too early, and we didn't bother clearing the high channels of
the input routes at all, all with no observed adverse effects).
As a drive-by, this also fixes some capture channel defaults - any
EMU_SRC_*2 isn't a sensible value in 1x clock mode.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536508-7-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Instead of hard-coding the card-specific arrays and their sizes in each
function, use a more data-driven approach.
As a drive-by, also hide the unavailable I2S input destinations on the
1616 cardbus card.
Also as a drive-by, use more assignments at variable declaration for
brevity. This also removes the pointless masking of kctl.private_value.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536508-5-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Define arrays of strings instead of snd_kcontrol_new.
While at it, move the E-MU source & destination enum defs next to their
hardware defs, which is a lot more logical and will come in handy in a
followup commit. And add some static asserts to verify that the array
sizes match.
This also applies the compactization from the previous commit to the
destination registers.
While reshuffling the arrays anyway, switch the order of the HAMOA_DAC
& HANA_SPDIF output destinations for the 1010 card, so they follow a
more regular pattern. This should have no functional impact.
The code is somewhat de-duplicated by the extraction of add_ctls().
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536508-4-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Use macros to avoid duplication. Arguably, this is somewhat less
legible, but future additions would grow this part of the file to
completely unmanageable dimensions.
The EMU*_COMMON_TEXTS macros will save duplication in a future commit;
I pulled them ahead to reduce churn.
While rewriting the tables anyway, rearrange them such that each card's
strings and registers are adjacent.
Also, add some static asserts to verify that the array sizes match.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536508-3-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Many registers are meaningless for stereo slaves and the extra voices.
This patch cleans up these unnecessary register writes.
snd_emu10k1_playback_{trigger,stop}_voice() is not called for stereo
slaves any more.
snd_emu10k1_playback_prepare_voice() is renamed to
snd_emu10k1_playback_unmute_voice(), as this better reflects its
remaining function. It's not called for the extra voices any more.
Accordingly, snd_emu10k1_playback_mute_voice() is factored out from
snd_emu10k1_playback_stop_voice(), and is called selectively as well.
This doesn't add conditionals which would avoid initializing
sub-registers, as that wouldn't pull its weight.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536451-6-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
We now enable ints even before triggering, and disable them only after
stopping - otherwise there is a race condition we may plausibly run into
when we pause/resume near the end of the buffer.
Updating the epcm->running flag is moved the same way, as it affects the
*_pointer() functions, which are called by the interrupt handler.
Also, factor these out to own functions, for clarity.
For multi-channel, the extra voice is now triggered after all regular
voices - we wouldn't want to receive an int before all channels have
passed the period boundary.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536451-5-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
We initialize them at card init and don't touch them later, so there is
no need to reset them again at voice start.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536451-4-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
We (rightfully) don't enable the envelope engine for PCM voices, so any
related setup is entirely pointless - the EMU8K documentation makes that
very clear, and the fact that the various open drivers all use different
values to no observable detriment pretty much confirms it.
The remaining initializations are regrouped for clarity.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536451-3-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The mixer structures were filled in two places: on driver init, and when
the devices are opened. The latter made the former pointless, so we
remove the former. This implies that mixer dumps may now return all
zeroes, which is OK, as restoring them is meaningless as well.
Things were even weirder for the (generally unused) secondary sends:
Some of the initialization loops were forgotten when support for Audigy
was added, thus creating the technically illegal state of multiple sends
being routed to the same FX accumulator (though it apparently doesn't
matter when the amount is zero).
The global multi-channel init used some rather bizarre values for the
secondary sends, and the init on open actually forgot to re-initialize
them. We now use a not really more useful, but simpler formula.
The direct register init was also bogus. This doesn't really matter, as
the value is overwritten when a voice comes into use, but still.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230516093612.3536451-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
SNDRV_CTL_ELEM_TYPE_* are type of snd_ctl_elem_type_t, we
have to __force cast them to int when comparing them with int
to fix the following sparse warnings.
sound/core/control_compat.c:203:14: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
sound/core/control_compat.c:205:14: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
sound/core/control_compat.c:207:14: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
sound/core/control_compat.c:209:14: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
sound/core/control_compat.c:237:21: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
sound/core/control_compat.c:238:21: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
sound/core/control_compat.c:270:21: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
sound/core/control_compat.c:271:21: sparse: warning: restricted snd_ctl_elem_type_t degrades to integer
Signed-off-by: Min-Hua Chen <minhuadotchen@gmail.com>
Link: https://lore.kernel.org/r/20230516223806.185683-1-minhuadotchen@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The voice volume is a raw fractional multiplier that can't actually
represent 1.0. To still enable real pass-through, we now set the volume
to 0.5 (which results in no loss of precision, as the FX bus provides
fractional values) and scale up the samples in DSP code.
To maintain backwards compatibility with existing configuration files,
we rescale the values in the mixer controls. The range is extended
upwards from 0xffff to 0x1fffd, which actually introduces the
possibility of specifying an amplification.
There is still a minor incompatibility with user space, namely if
someone loaded custom DSP code. They'll just get half the volume, so
this doesn't seem like a big deal.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408834-8-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Fractional multiplication with the maximal value 2^31-1 causes some tiny
distortion. Instead, we want to multiply with the full 2^31. The catch
is of course that this cannot be represented in the DSP's signed 32 bit
registers.
One way to deal with this is to encode 1.0 as a negative number and
special-case it. As a matter of fact, the SbLive! code path already
contained such code, though the controls never actually exercised it.
A more efficient approach is to use negative values, which actually
extend to -2^31. Accordingly, for all the volume adjustments we now use
the MAC1 instruction which negates the X operand.
The range of the controls in highres mode is extended downwards, so -1
is the new zero/mute. At maximal excursion, real zero is not mute any
more, but I don't think anyone will notice this behavior change. ;-)
That also required making the min/max/values in the control structs
signed. This technically changes the user space interface, but it seems
implausible that someone would notice - the numbers were actually
treated as if they were signed anyway (and in the actual mixer iface
they _are_). And without this change, the min value didn't even make
sense in the first place (and no-one noticed, because it was always 0).
Tested-by: Jonathan Dowland <jon@dow.land>
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408834-7-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The microphone capture device is a feature of the AC97 codec, so its
availability should be coupled to the presence of that codec.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408834-6-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The E-MU cards don't try very hard to be Sound Blasters. All sound I/O
goes through the Hana FPGA, thus making the regular extin/out controls
useless. Still showing them just serves to clutter up the interface and
confuse the user.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408834-5-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
User space could pass arbitrary ranges, which were uncritically
accepted. This could lead to table lookups out of range.
I don't think that this is a security issue, as it only allowed someone
with CAP_SYS_ADMIN to crash the kernel, but still.
Setting an invalid translation mode will also be rejected now. That did
no harm, but it's still better to detect errors.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408834-4-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
- Pull ahead all fixed allocations, so we don't rely on the semi-
dynamic ones not crossing the arbitrarily determined limit
- Use an enum for the fixed allocations
- Stop arbitrarily wasting registers on unexplained "reservations"
- Don't reserve two regs for the master volume control - it's mono
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408834-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Rather than applying masks to the provided values, make assertions
about them being valid - otherwise we'd just try to paper over bugs.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408798-2-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The idea to encode the bitfield manipulation in the register address is
quite clever, but doing that by hand is ugly and error-prone. So derive
it automatically from the mask instead.
Macros cannot #define other macros, so we now declare enums instead.
This also adds macros for decoding the register definitions. These will
be used by later commits.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230514170323.3408798-1-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Instead of using lots of instructions to mix wet and dry signals,
simply skip over the whole code block if tone control is disabled.
This also allows us doing away with the "shadow" playback channels.
Tested-by: Jonathan Dowland <jon@dow.land>
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230510173917.3073107-7-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Evidently, the channel delay bug exists in all E-MU cards; it's in the
Hana FPGA program, and was never fixed.
Note that the implementation is somewhat lazy - to localize the code
paths, we actually waste a GPR and a DSP instruction by keeping two
delay registers for the same physical source.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230510173917.3073107-6-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Instead of spending lots of instructions on masking and transplanting
the sign bit, sidestep the issue by replacing the last bit shift with
a wrapping addition to self.
Solution stolen from kX-project, after I pondered other ideas first.
Also, the function really doesn't need to return a constant int value.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Link: https://lore.kernel.org/r/20230510173917.3073107-5-oswald.buddenhagen@gmx.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
