On 2020-01-29 10:46, Daniel Baluta wrote:
On Wed, 2020-01-29 at 10:24 +0100, Cezary Rojewski wrote:
Thanks for the review Daniel!
Tthe example you provided is not very clear to me - same condition
is
added for both assignments, but I'll try to answer your question.
Existing "sof_compressed_ops" don't even exist, as you said it
yourself
so nothing is lost. Changes within pcm.c are gated by _DEBUG_PROBES
anyway so the solution is actually very clean.
While DAI can have different cops, platform driver cannot. I'm aware
of
the problem but till actual compress support for SOF comes out,
minimal,
probe-only changes were a better choice IMHO. After all, that's not
a
problem to make this code smarter in the future - not a farseer
though,
can't predict what you're going to add for SOF-compr :)
Indeed, we can make the code smarter later but I want to do the code
cleaner now.
I think I had a copy paste error when providing the example.
So, my proposal is to override the platform driver compr_ops field
with probe compressed ops when CONFIG_SND_SOC_SOF_DEBUG_PROBES is set.
The code could look like this in the end:
#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
pd->compr_ops = &sof_compressed_ops;
#endif
/* Add a comment explaining that we are doing override in case of
probes */
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
pd->compr_ops = &sof_probe_compressed_ops;
#endif
Also, I think there is no need to define the probe compressed ops
inside pcm.c
So, instead of
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
#include "compress.h"
struct snd_compr_ops sof_compressed_ops = {
.copy = sof_probe_compr_copy,
};
EXPORT_SYMBOL(sof_compressed_ops);
#endif
We can do:
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
extern snd_compr_ops sof_probe_compressed_ops;
#endif
or even better include a header with the declaration.
And add the definition of sof_probe_compressed_ops would be somewhere
in the compressed probe file.
Your comments have been addressed in v4. Non-existant cops usage have
been re-added and is now overridden by probes when enabled.
sof_probe_compressed_ops declaration moved to compress.c file as requested.
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
