Re: [PATCHv2 - CA0132 HDA Codec 1/2] ALSA: Update Creative CA0132 codec to add DSP features.

[Takashi Iwai <tiwai@suse.de>]

At Wed, 25 Jul 2012 11:01:19 -0700,
Ian Minett wrote:
> 
> From: Ian Minett <ian_minett@creativelabs.com>
> 
> Hi,
> Thanks again for the feedback - we've reworked the patch accordingly, to remove
> floats, tidy the use of macros and add the MODULE_FIRMWARE() lines.
> 
> You are right - since it is a pretty big change introducing all the DSP
> functionality it isn't trivial to break it into multiple small patches.
> I've added more info on what has been changed below in the git commit message.
> 
> The reason we need to modify the trigger callback in hda_intel.c is that the DSP
> code is downloaded from the host (driver) to the CA0132 chip via the HD-link.
> This means that the trigger callback is called from the codec driver.

Well, this is the biggest problem.  What the patch does is a layer
violation.  Such an operation must not be performed in the codec
driver.  I guess you know it very well when you look again how ugly
hacks you need for setting up a fake PCM instance and so on.

One possible way would be to move these DSP loading code into
hda_intel.c.  But then it bloats up the generic controller driver.

Another possibility is to fork hda_intel.c and move the Creative's
controller chip driver there (not meant the code in patch_ca0132.c).
You can strip down many workarounds found in hda_intel.c
(e.g. bdl_pos_adj, position_fix, probing errors, vga_switcheroo, etc)
as a bonus.  And the new controller driver can still use the same
hda_codec.c and the rest as long as it uses the same structure.

> I will submit the DSP firmware blob (ctefx.bin) as a separate patch against the 
> alsa-firmware repository, for intended upstreaming to linux-firmware package. 
> The CA0132 codec uses request_firmware() loader to find the DSP binary in the 
> /lib/firmware directory (the firmware isn't required for building the CA0132
> module). If this isn't the correct way to submit firmware binaries, we'd 
> appreciate any info on what the correct method for this is.

Yes, this is the correct way.

But, still one concern is that people may very likely build the kernel
without taking the firmware.  Then suddenly it breaks just by the
kernel update -- this shouldn't happen.

So, we'd need a kernel config, or at least a module parameter, to
enable/disable the new DSP stuff.  When it's off, the whole DSP isn't
requested and it behaves just likes a standard HD-audio.


thanks,

Takashi