[PATCH v4 02/34] ASoC: Intel: avs: Include CPUID header at file scope

Ahmed S. Darwish posted 34 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 02/34] ASoC: Intel: avs: Include CPUID header at file scope
Posted by Ahmed S. Darwish 1 month, 2 weeks ago
Commit

    cbe37a4d2b3c ("ASoC: Intel: avs: Configure basefw on TGL-based platforms")

includes the main CPUID header from within a C function.  This works by
luck and forbids valid refactorings inside the CPUID header.

Include the CPUID header at file scope instead.

Note, for the CPUID(0x15) leaf number, use CPUID_LEAF_TSC instead of
defining a custom local macro for it.

Signed-off-by: Ahmed S. Darwish <darwi@linutronix.de>
Acked-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/tgl.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/avs/tgl.c b/sound/soc/intel/avs/tgl.c
index 9dbb3ad0954a..cf19d3a7ced2 100644
--- a/sound/soc/intel/avs/tgl.c
+++ b/sound/soc/intel/avs/tgl.c
@@ -10,8 +10,6 @@
 #include "avs.h"
 #include "messages.h"
 
-#define CPUID_TSC_LEAF 0x15
-
 static int avs_tgl_dsp_core_power(struct avs_dev *adev, u32 core_mask, bool power)
 {
 	core_mask &= AVS_MAIN_CORE_MASK;
@@ -39,22 +37,31 @@ static int avs_tgl_dsp_core_stall(struct avs_dev *adev, u32 core_mask, bool stal
 	return avs_dsp_core_stall(adev, core_mask, stall);
 }
 
+#ifdef CONFIG_X86
+#include <asm/cpuid/api.h>
+static unsigned int intel_crystal_freq_hz(void)
+{
+	return cpuid_ecx(CPUID_LEAF_TSC);
+}
+#else
+static unsigned int intel_crystal_freq_hz(void)
+{
+	return 0;
+}
+#endif /* !CONFIG_X86 */
+
 static int avs_tgl_config_basefw(struct avs_dev *adev)
 {
+	unsigned int freq = intel_crystal_freq_hz();
 	struct pci_dev *pci = adev->base.pci;
 	struct avs_bus_hwid hwid;
 	int ret;
-#ifdef CONFIG_X86
-	unsigned int ecx;
 
-#include <asm/cpuid/api.h>
-	ecx = cpuid_ecx(CPUID_TSC_LEAF);
-	if (ecx) {
-		ret = avs_ipc_set_fw_config(adev, 1, AVS_FW_CFG_XTAL_FREQ_HZ, sizeof(ecx), &ecx);
+	if (freq) {
+		ret = avs_ipc_set_fw_config(adev, 1, AVS_FW_CFG_XTAL_FREQ_HZ, sizeof(freq), &freq);
 		if (ret)
 			return AVS_IPC_RET(ret);
 	}
-#endif
 
 	hwid.device = pci->device;
 	hwid.subsystem = pci->subsystem_vendor | (pci->subsystem_device << 16);
-- 
2.50.1
Re: [PATCH v4 02/34] ASoC: Intel: avs: Include CPUID header at file scope
Posted by Borislav Petkov 1 month, 2 weeks ago
On Fri, Aug 15, 2025 at 09:01:55AM +0200, Ahmed S. Darwish wrote:
> Commit
> 
>     cbe37a4d2b3c ("ASoC: Intel: avs: Configure basefw on TGL-based platforms")
> 
> includes the main CPUID header from within a C function.  This works by
> luck and forbids valid refactorings inside the CPUID header.
> 
> Include the CPUID header at file scope instead.
> 
> Note, for the CPUID(0x15) leaf number, use CPUID_LEAF_TSC instead of
> defining a custom local macro for it.
> 
> Signed-off-by: Ahmed S. Darwish <darwi@linutronix.de>
> Acked-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
>  sound/soc/intel/avs/tgl.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)

I'm guessing that:

https://lore.kernel.org/r/aGv8AWwHbi5seDxi@lx-t490

still needs to happen yet?

Because I don't see it in this set...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v4 02/34] ASoC: Intel: avs: Include CPUID header at file scope
Posted by Ahmed S. Darwish 1 month, 2 weeks ago
Hi Boris,

On Mon, 18 Aug 2025, Borislav Petkov wrote:
>
> I'm guessing that:
>
> https://lore.kernel.org/r/aGv8AWwHbi5seDxi@lx-t490
>
> still needs to happen yet?
>
> Because I don't see it in this set...
>

Yeah, I discovered that I need to do the X86_FEATURE integration within
the CPUID model before touching the CPUID(0x15) call sites.

Quoting the cover:

  State of Affairs: By now, all CPUID leaves which do not interact with
  the X86_FEATURE mechanism(s) has been transformed to the CPUID model.
  Remaining leaves have dependencies on X86_FEATURE.

  For example, some callsites _directly_ enumerate CPUID(0x15)
  [ART_CPUID_LEAF / CPUID_LEAF_TSC] even though that leaf has important
  dependencies on X86_FEATURE_ART and its tsc_async_resets flag.

  The CPUID parser work will not propagate such call sites brokedness and
  will integrate the X86_FEATURE infrastructure within it first.

As I mentioned in the Bhyve thread, this shall also include a directed
acyclic graph of dependencies for the X86_FEATURE flags themselves:

  https://lore.kernel.org/lkml/aKL0WlA4wIU8l9RT@lx-t490

v5 should cover this.

Thanks!

--
Ahmed S. Darwish
Linutronix GmbH