- 20 May, 2023 4 commits
-
-
Gustavo A. R. Silva authored
One-element arrays are deprecated, and we are replacing them with flexible array members instead. However, in this case it seems those one-element arrays have never actually been used as fake flexible arrays. See this code that dates from Linux-2.6.12-rc2 initial git repository build (commit 1da177e4 ("Linux-2.6.12-rc2")): sound/pci/mixart/mixart_core.h: 215 struct mixart_stream_state_req 216 { 217 u32 delayed; 218 u64 scheduler; 219 u32 reserved4np[3]; 220 u32 stream_count; /* set to 1 for instance */ 221 struct mixart_flow_info stream_info; /* could be an array[stream_count] */ 222 } __attribute__((packed)); sound/pci/mixart/mixart.c: 388 389 memset(&stream_state_req, 0, sizeof(stream_state_req)); 390 stream_state_req.stream_count = 1; 391 stream_state_req.stream_info.stream_desc.uid_pipe = stream->pipe->group_uid; 392 stream_state_req.stream_info.stream_desc.stream_idx = stream->substream->number; 393 So, taking the code above as example, replace multiple one-element arrays with simple object declarations, and refactor the rest of the code, accordingly. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1]. This results in no differences in binary output. Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/296 Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1] Signed-off-by:
Gustavo A. R. Silva <gustavoars@kernel.org> Link: https://lore.kernel.org/r/ZGfiFjcL8+r3mayq@workSigned-off-by:
Takashi Iwai <tiwai@suse.de>
-
Oswald Buddenhagen authored
Instead of separate voices, we now allocate non-interleaved channels, which may in turn contain two interleaved voices each. The higher-level code keeps only one pointer per channel. The channels are not allocated in one block any more, as there is no reason to do that. As a consequence of that, and because it is cleaner regardless, we now let the allocator store these pointers at a specified location, rather than returning only the first one and having the calling code deduce the remaining ones. Signed-off-by:
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> Link: https://lore.kernel.org/r/20230518140947.3725394-8-oswald.buddenhagen@gmx.deSigned-off-by:
-
Oswald Buddenhagen authored
The voice allocator clearly knows about the field (it resets it), so it's more consistent (and leads to less duplicated code) to have the constructor take it as a parameter. Signed-off-by:
-
Oswald Buddenhagen authored
This saves some code duplication; no functional change. Signed-off-by:
-
- 18 May, 2023 23 commits
-
-
Oswald Buddenhagen authored
This allows us to drop the code that tries to preserve already allocated voices upon repeated hw_param callback invocations. Getting it right for multi-channel voices would otherwise get a bit hairy. Signed-off-by:
-
Oswald Buddenhagen authored
Eliminate the MIDI type, as there is no such thing - the MPU401 port doesn't have anything to do with voices. For clarity, differentiate between regular and extra voices. Don't atomize the enum into bits in the table display. Simplify/optimize the storage. Signed-off-by:
-
Oswald Buddenhagen authored
The subsequent allocation may still fail after freeing some voices, so we shouldn't leave them in their programmed state. Signed-off-by:
-
Oswald Buddenhagen authored
snd_emu10k1_voice_free() resets the hardware itself, so doing that in the calling function as well is redundant. Signed-off-by:
-
Oswald Buddenhagen authored
On Audigy, the send amounts are merely targets, presumably to avoid sound distortion due to sudden changes, which the EMU8K docu explicitly warns about. However, that "soft-start" would prevent bit-for-bit reproduction, so we now force the current send amounts to their final values at PCM playback init. One might want to do that for the MIDI synthesizer as well, though it seems mostly pointless due to the attack phase each note has anyway. Signed-off-by:
-
Oswald Buddenhagen authored
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 authored
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 authored
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:
-
Tom Rix authored
ALSA: emu10k1: set variables emu1010_routing_info and emu1010_pads_info storage-class-specifier to static 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 authored
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 authored
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 27ae958c ("emu10k1 driver - add multichannel device hw:x,3 [2-8/8]"). Signed-off-by:
-
Oswald Buddenhagen authored
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 authored
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 authored
IPR_CHANNELNUMBERMASK cannot be non-zero when IPR_CHANNELLOOP is unset, so join marking them as handled. This logically reverts part of commit f453e20d ("ALSA update 0.9.3a"), which made the inverse change with no explanation. Signed-off-by:
-
Oswald Buddenhagen authored
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 authored
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 authored
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 authored
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 authored
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 authored
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 authored
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 authored
This workaround fails to address the underlying problem, which is actually wholly self-made. Subsequent patches will fix it. This reverts commit 56385a12. Signed-off-by:
-
Oswald Buddenhagen authored
... to snd_emu10k1_pcm_init_voice(). This makes the code arguably less convoluted. Signed-off-by:
-
- 17 May, 2023 13 commits
-
-
Oswald Buddenhagen authored
32-bit platforms don't like these. As we're actually dealing with constants, factor out the calculations and pass them in as additional arguments. To keep the call sites clean, wrap the actual functions in macros which generate the arguments. Fixes: bb5ceb43 ("ALSA: emu10k1: fix non-zero mixer control defaults in highres mode") Fixes: 1298bc97 ("ALSA: emu10k1: enable bit-exact playback, part 1: DSP attenuation") Reported-by:
kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202305171622.jKTovBvy-lkp@intel.com/Reported-by:
Linux Kernel Functional Testing <lkft@linaro.org> Reported-by:
Christophe Leroy <christophe.leroy@csgroup.eu> Closes: https://lore.kernel.org/r/CA+G9fYsShNP=LALHdMd-Btx3PBtO_CjyBNgpStr9fPGXNbRvdg@mail.gmail.comSigned-off-by:
-
Oswald Buddenhagen authored
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 authored
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 authored
This card has rather different inputs/outputs due to switching from the AudioDock to the MicroDock. Signed-off-by:
-
Oswald Buddenhagen authored
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 authored
... 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 authored
It returned zero even if the value had changed. Signed-off-by:
-
Oswald Buddenhagen authored
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 authored
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 authored
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 authored
- Use bit fields in struct snd_emu_chip_details - Use shorts in the E-MU routing register arrays Signed-off-by:
-
Oswald Buddenhagen authored
Saves a bit of code duplication. Signed-off-by:
-
Oswald Buddenhagen authored
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:
-
