-
Notifications
You must be signed in to change notification settings - Fork 128
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
Merge/sound upstream 20210927 #3177
Merge/sound upstream 20210927 #3177
Conversation
Generic swiotlb code makes sure to keep the slab count a multiple of the number of slabs per segment. Yet even without checking whether any such assumption is made elsewhere, it is easy to see that xen_swiotlb_fixup() might alter unrelated memory when calling xen_create_contiguous_region() for the last segment, when that's not a full one - the function acts on full order-N regions, not individual pages. Align the slab count suitably when halving it for a retry. Add a build time check and a runtime one. Replace the no longer useful local variable "slabs" by an "order" one calculated just once, outside of the loop. Re-use "order" for calculating "dma_bits", and change the type of the latter as well as the one of "i" while touching this anyway. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/dc054cb0-bec4-4db0-fc06-c9fc957b6e66@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
Only on the 2nd of the paths leading to xen_swiotlb_init()'s "error" label it is useful to retry the allocation; the first one did already iterate through all possible order values. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/56477481-87da-4962-9661-5e1b277efde0@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
Due to the use of max(1024, ...) there's no point retrying (and issuing bogus log messages) when the number of slabs is already no larger than this minimum value. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/984fa426-2b7b-4b77-5ce8-766619575b7f@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
Commit a98f565 ("xen-swiotlb: split xen_swiotlb_init") should not only have added __init to the split off function, but also should have dropped __ref from the one left. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/7cd163e1-fe13-270b-384c-2708e8273d34@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
I consider it unhelpful that address and size of the buffer aren't put in the log file; it makes diagnosing issues needlessly harder. The majority of callers of swiotlb_init() also passes 1 for the "verbose" parameter. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/2e3c8e68-36b2-4ae9-b829-bf7f75d39d47@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
It was introduced by 4035b43 ("xen-swiotlb: remove xen_set_nslabs") and then not removed by 2d29960 ("swiotlb: dynamically allocate io_tlb_default_mem"). Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/15259326-209a-1d11-338c-5018dc38abe8@suse.com Signed-off-by: Juergen Gross <jgross@suse.com>
The ICS native driver relies on the IRQ chip data to find the struct 'ics_native' describing the ICS controller but it was removed by commit 248af24 ("powerpc/xics: Rename the map handler in a check handler"). Revert this change to fix the Microwatt SoC platform. Fixes: 248af24 ("powerpc/xics: Rename the map handler in a check handler") Signed-off-by: Cédric Le Goater <clg@kaod.org> Tested-by: Gustavo Romero <gustavo.romero@linaro.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Link: https://lore.kernel.org/r/20210913134056.3761960-1-clg@kaod.org
Fix @buf arg given to hex_dump_to_buffer() and stack address used in dump error output. Fixes: e657c18 ('ASoC: SOF: Add xtensa support') Signed-off-by: Yong Zhi <yong.zhi@intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Link: https://lore.kernel.org/r/20210915063230.29711-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
Add a new quirk SOF_RT5682S_HEADPHONE_CODEC_PRESENT to support ALC5682I-VS headphone codec which driver is a new one, rt5682s, with new macros and functions. Signed-off-by: Brent Lu <brent.lu@intel.com> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20210914101847.778688-2-brent.lu@intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
This patch adds driver data for two ALC1015Q-VB speaker amplifiers on SSP1 and one ALC5682I-VS headphone codec on SSP0 for JSL platform. Topology is leveraged from jsl_rt5682_rt1015p since the capability of two ALC5682 variants is the same. Signed-off-by: Brent Lu <brent.lu@intel.com> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20210914101847.778688-3-brent.lu@intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
This patch adds driver data for two ALC1015Q-CG speaker amplifiers on SSP1 and one ALC5682I-VS headphone codec on SSP0 for JSL platform. Topology is leveraged from jsl_rt5682_rt1015 since the capability of two ALC5682 variants is the same. Signed-off-by: Brent Lu <brent.lu@intel.com> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20210914101847.778688-4-brent.lu@intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
This patch adds driver data for two MAX98360A speaker amplifiers on SSP1 and one ALC5682I-VS headphone codec on SSP0 for JSL platform. Topology is leveraged from jsl_rt5682_mx98360a since the capability of two ALC5682 variants is the same. Signed-off-by: Brent Lu <brent.lu@intel.com> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Link: https://lore.kernel.org/r/20210914101847.778688-5-brent.lu@intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
.resindex_dma_base is not used by the code and in all instances it is set to -1. To make it possible to remove it from the sof_dev_desc struct, first remove all references from the intel drivers (initialization). Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Link: https://lore.kernel.org/r/20210915065541.1178-2-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
…desc resindex_dma_base, dma_engine and dma_size is unused, remove them. There is no hint in the comments how this supposed to be used, when the need arises it can be added back. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Link: https://lore.kernel.org/r/20210915065541.1178-3-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
The following functions can be made static as they are only used locally: hda_dsp_core_reset_enter hda_dsp_core_reset_leave hda_dsp_core_stall_reset hda_dsp_core_power_up hda_dsp_core_power_down hda_dsp_core_is_enabled The hda_dsp_ipc_int_disable is also only used within hda-dsp.c, but for symmetry for hda_dsp_ipc_int_enable (used by hda-loader.c) leave it as it is. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Link: https://lore.kernel.org/r/20210915071805.5704-2-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
…ations hda_dsp_cl_boot_firmware_iccmax_icl and hda_dsp_cl_boot_firmware_skl is no longer backed with an implementation, remove them from the hda.h Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Link: https://lore.kernel.org/r/20210915071805.5704-3-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
…c for sdw Move the only locally needed inline functions to hda.c when CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE is not enabled to make the header file less cluttered with information no needed to be there. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Link: https://lore.kernel.org/r/20210915071805.5704-4-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
The platform_node is returned by of_parse_phandle() should have of_node_put() before return. Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Bixuan Cui <cuibixuan@huawei.com> Link: https://lore.kernel.org/r/20210911081246.33867-1-cuibixuan@huawei.com Signed-off-by: Mark Brown <broonie@kernel.org>
Don't populate the array adda_dai_list on the stack but instead it static const. Also makes the object code smaller by 33 bytes: Before: text data bss dec hex filename 28271 11640 0 39911 9be7 mt8195/mt8195-dai-adda.o After: text data bss dec hex filename 28142 11736 0 39878 9bc6 mt8195/mt8195-dai-adda.o (gcc version 11.2.0) Signed-off-by: Colin Ian King <colin.king@canonical.com> Link: https://lore.kernel.org/r/20210915105027.10805-1-colin.king@canonical.com Signed-off-by: Mark Brown <broonie@kernel.org>
… that Since the load_firmware callback in snd_sof_dsp_ops is mandatory and it is tested during probe. Move the snd_sof_load_firmware() wrapper to ops.h as inline and drop the check of sof_ops(sdev)->load_firmware Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> Link: https://lore.kernel.org/r/20210914125356.19828-1-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie@kernel.org>
mt6359-sound is a MFD driver. Because its regmap is retrieved from its parent, it shouldn't be freed in mt6359-sound driver. snd_soc_component_exit_regmap() will do regmap_exit(), this results in unexpected results if sound card unregister flow is invoked when users try to bind/unbind audio codec. Remove the usage of snd_soc_component_exit_regmap(). Instead, set component->regmap = NULL in the component remove function. Signed-off-by: Trevor Wu <trevor.wu@mediatek.com> Link: https://lore.kernel.org/r/20210915034659.25044-1-trevor.wu@mediatek.com Signed-off-by: Mark Brown <broonie@kernel.org>
The loop checking PDN_DONE doesn't check the return value from regmap_read, nor does it initialise val. This means if regmap_read fails val will be checked for the PDN_DONE bit whilst being uninitialised. Fix this up by switching to regmap_read_poll_timeout which tidies up the code and avoids the uninitialised variable. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Link: https://lore.kernel.org/r/20210914141349.30218-1-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
Just clean up the code a little by using the helper rather than open coding waiting for OTP_BOOT_DONE. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Link: https://lore.kernel.org/r/20210914141349.30218-2-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
cs35l41 is often connected over I2C which is a very slow bus, as such timings can be greatly improved combining writes where acceptable. Update several points where the driver does multiple register writes when a single one would suffice. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Link: https://lore.kernel.org/r/20210914141349.30218-3-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
In multiple places the driver overwrites the error code returned with a static error code, this is not helpful for debugging. Update to pass the error codes straight through. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Link: https://lore.kernel.org/r/20210914141349.30218-4-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
It is not idiomatic for ASoC to print the function name in the error messages, however it is expected to show the return code. Update the error messages to follow these conventions. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Link: https://lore.kernel.org/r/20210914141349.30218-5-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Link: https://lore.kernel.org/r/20210914141349.30218-6-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown <broonie@kernel.org>
We should not walk/touch page tables outside of VMA boundaries when holding only the mmap sem in read mode. Evil user space can modify the VMA layout just before this function runs and e.g., trigger races with page table removal code since commit dd2283f ("mm: mmap: zap pages with read mmap_sem in munmap"). find_vma() does not check if the address is >= the VMA start address; use vma_lookup() instead. Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> Fixes: dd2283f ("mm: mmap: zap pages with read mmap_sem in munmap") Signed-off-by: David Hildenbrand <david@redhat.com> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
Prevent out-of-range access if the returned SCLP SCCB response is smaller in size than the address of the Secure-IPL flag. Fixes: c9896ac ("s390/ipl: Provide has_secure sysfs attribute") Cc: stable@vger.kernel.org # 5.2+ Signed-off-by: Alexander Egorenkov <egorenar@linux.ibm.com> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com> Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
The sparse failure is from upstream patch: |
@brentlu, @plbossart, can you propose a fix for this? include/linux/mod_devicetable.h: #define PLATFORM_NAME_SIZE 20
#define PLATFORM_MODULE_PREFIX "platform:"
struct platform_device_id {
char name[PLATFORM_NAME_SIZE];
kernel_ulong_t driver_data;
};
|
There are four variants MAX98360A/B/C/D . The difference between variants is the DAI format supported and they are all the same to codec driver. I propose removing the last character which does not have meaning to machine driver. jsl_rt5682_mx98360a -> jsl_rt5682_mx98360 |
Can you send it to upstream (please CC me)? I will pick from the list and add to this branch, thank you. |
done. https://patchwork.kernel.org/project/alsa-devel/patch/20210927143249.439129-1-brent.lu@intel.com/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gave this a local spin on a JSL HDA device and all good.
To follow 20-character length limitation of platform device name, we have only 7 character space for amplifier. Therefore, the last character of mx98357a and mx98360a is removed to save space. Signed-off-by: Brent Lu <brent.lu@intel.com> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
@kv2019i, thanks for the testing! Changes since v1: |
SOFCI TEST |
I can reproduce a hw_params issue with this PR on UpExtreme:
Meh. |
possible root cause: |
Confirmed regression due to this commit, sending feedback upstream |
This reverts commit 0c25db3. Explanation from upstream: https://lore.kernel.org/alsa-devel/be6290d1-0682-3d93-98a6-ad0be3ca42c1@linux.intel.com/ This patch breaks our SOF CI tests, ironically in all cases where we have a mixer with a 'Deep buffer' port! The tests with multiple streams all fail with this error: [ 124.366400] Port0 Deep Buffer: ASoC: no backend DAIs enabled for Port0 Deep Buffer [ 124.366406] Port0 Deep Buffer: ASoC: dpcm_fe_dai_prepare() failed (-22) Reverting this patch restore the mixer functionality. I see multiple problems with this patch: At a high-level, there's at least a race condition where this BE state is checked without any protection. That was already a problem that I highlighted in a recent RFC and still working on, when we have multiple FEs we can have START/STOP triggers happening concurrently and it's necessary to serialize these triggers when checking the state, and this patch increases the 'surface' for this race condition. But in addition we'd need to agree on what an 'active BE' is. Why can't we connect a second stream while the first one is already in HW_PARAMS or PAUSED or STOP? It's perfectly legal in ALSA/ASoC to have multiple HW_PARAMS calls, and when we reach STOP we have to do a prepare again. And more fundamentally, the ability to add a second FE on a 'active' BE in START state is a basic requirement for a mixer, e.g. to play a notification on one FE while listening to music on another. What needs to happen is only to make sure that the FE and BE are compatible in terms of HW_PARAMS and not sending a second TRIGGER_STOP, only checking the BE NEW or CLOSE state is way too restrictive. I will agree this sort of mixer use cases is not well supported in soc-pcm.c, but let's not make it worse than it was before this patch, shall we? I can send a revert with the explanations in the commit message if there is a consensus that this patch needs to be revisited. [1] thesofproject#3177 [2] https://sof-ci.01.org/linuxpr/PR3177/build6440/devicetest/ Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Changes since v2:
|
This reverts commit 0c25db3. Explanation from upstream: https://lore.kernel.org/alsa-devel/be6290d1-0682-3d93-98a6-ad0be3ca42c1@linux.intel.com/ This patch breaks our SOF CI tests, ironically in all cases where we have a mixer with a 'Deep buffer' port! The tests with multiple streams all fail with this error: [ 124.366400] Port0 Deep Buffer: ASoC: no backend DAIs enabled for Port0 Deep Buffer [ 124.366406] Port0 Deep Buffer: ASoC: dpcm_fe_dai_prepare() failed (-22) Reverting this patch restore the mixer functionality. I see multiple problems with this patch: At a high-level, there's at least a race condition where this BE state is checked without any protection. That was already a problem that I highlighted in a recent RFC and still working on, when we have multiple FEs we can have START/STOP triggers happening concurrently and it's necessary to serialize these triggers when checking the state, and this patch increases the 'surface' for this race condition. But in addition we'd need to agree on what an 'active BE' is. Why can't we connect a second stream while the first one is already in HW_PARAMS or PAUSED or STOP? It's perfectly legal in ALSA/ASoC to have multiple HW_PARAMS calls, and when we reach STOP we have to do a prepare again. And more fundamentally, the ability to add a second FE on a 'active' BE in START state is a basic requirement for a mixer, e.g. to play a notification on one FE while listening to music on another. What needs to happen is only to make sure that the FE and BE are compatible in terms of HW_PARAMS and not sending a second TRIGGER_STOP, only checking the BE NEW or CLOSE state is way too restrictive. I will agree this sort of mixer use cases is not well supported in soc-pcm.c, but let's not make it worse than it was before this patch, shall we? I can send a revert with the explanations in the commit message if there is a consensus that this patch needs to be revisited. [1] #3177 [2] https://sof-ci.01.org/linuxpr/PR3177/build6440/devicetest/ Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess main question is whether we revert the one ASoC patch or wait until issue is solved in Mark's tree (full revert or a fixup). Given some discussion is still ongoing, maybe we can go forward with the revert for sof-dev.
I'll merge so that this PR is part of the daily tests starting in 50mn. |
Sync up with upstream on this nice autumn day.