Re: [PATCH] ASoC: Intel: avs: Depend on CONFIG_ACPI

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

On 4/26/2022 8:26 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 4/26/22 13:25, Mark Brown wrote:
>> On Tue, Apr 26, 2022 at 08:07:22PM +0200, Cezary Rojewski wrote:
>>
>>> Would you like the change proposed by Pierre to be part of this patch or a
>>> separate one?
>>
>> Either is fine.
>>
>>> Also, I assume the above code does not change, just:
>>> s/select SND_SOC_ACPI/select SND_SOC_ACPI if ACPI/
>>
>>> is applied on top of what's already in the patch. Is my deduction correct?
>>
>> AIUI the current patch and associated hard dependency on ACPI could be
>> dropped but I've not checked or thought too deeply about it
> 
> 
> my suggestion was the following:
> 
> depends on PCI
> depends on COMMON_CLK
> select SND_SOC_ACPI if ACPI

Thanks for suggestion, I tested with it, but it doesn't work as we 
access fields from struct nhlt_specific_cfg which is available only if 
ACPI support is enabled:

sound/soc/intel/avs/path.c: In function ‘avs_copier_create’:
sound/soc/intel/avs/path.c:186:17: error: ‘struct nhlt_specific_cfg’ has 
no member named ‘caps’
   186 |   data = ep_blob->caps;
       |                 ^~
sound/soc/intel/avs/path.c:187:22: error: ‘struct nhlt_specific_cfg’ has 
no member named ‘size’
   187 |   data_size = ep_blob->size;
       |                      ^~
sound/soc/intel/avs/path.c:210:17: error: ‘struct nhlt_specific_cfg’ has 
no member named ‘caps’
   210 |   data = ep_blob->caps;
       |                 ^~
sound/soc/intel/avs/path.c:211:22: error: ‘struct nhlt_specific_cfg’ has 
no member named ‘size’
   211 |   data_size = ep_blob->size;
       |                      ^~

Now there are few ways this can be solved:
1. Move the struct nhlt_specific_cfg definition out of define check... 
in include/sound/intel-nhlt.h, but that seems kind of sloppy to me, even 
if it makes sense.
2. NHLT is only needed for I2S and DMIC configuration, so we could in 
theory add #ifdef ACPI around configuration for those endpoints in 
sound/soc/intel/avs/path.c - but it introduces potential problems on x86 
platforms with users doing some weird config.
3. Depend directly on ACPI (current patch) causing NHLT support to be 
always enabled. And then apply change suggested by Pierre when we 
rewrite NHLT support to use NHLT structures in include/acpi/actbl2.h 
header as it as far as I can tell exposes all structures unconditionally.

I've discussed with Cezary and we are leaning towards option 3. but if 
there is any other preference we can prepare patch for that option.