Skip to content

Commit b2fc4d1

Browse files
checkupupcheckupup
committed
dax: use atomic flags to avoid race conditions
In DP domain, sof_dax_process runs in a DP thread, other sof_dax_* functions run in a different LL thread. Using a atomic flags instead of original dax_ctx->update_flags to avoid potential race conditions when updating flags. Since dax_inf.h should not have any dependencies on SOF, we define aotmic flags in dax.c Signed-off-by: Jun Lai <jun.lai@dolby.com>
1 parent 1e4400a commit b2fc4d1

File tree

2 files changed

+90
-35
lines changed
  • src/audio/module_adapter/module/dolby
  • third_party/include

2 files changed

+90
-35
lines changed

‎src/audio/module_adapter/module/dolby/dax.c‎

Lines changed: 86 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include <stdio.h>
1111

12+
#include <rtos/atomic.h>
1213
#include <rtos/init.h>
1314
#include <sof/audio/data_blob.h>
1415
#include <sof/audio/module_adapter/module/generic.h>
@@ -37,6 +38,41 @@ SOF_DEFINE_REG_UUID(dolby_dax_audio_processing);
3738
#define DAX_ENUM_PROFILE_CONTROL_ID 0
3839
#define DAX_ENUM_DEVICE_CONTROL_ID 1
3940

41+
/* dax_adapter_data can be only used within dax module adapter
42+
* and is not visible in dax_inf.h
43+
*/
44+
struct dax_adapter_data {
45+
atomic_t flags;
46+
};
47+
48+
enum dax_flag_opt_mode {
49+
DAX_FLAG_READ = 0,
50+
DAX_FLAG_ADD,
51+
DAX_FLAG_CLEAR,
52+
};
53+
54+
static uint32_t flag_process(atomic_t *flags,
55+
uint32_t flag,
56+
enum dax_flag_opt_mode opt_mode)
57+
{
58+
switch (opt_mode) {
59+
case DAX_FLAG_READ:
60+
return atomic_read(flags) & flag;
61+
case DAX_FLAG_ADD:
62+
if ((atomic_read(flags) & flag) == 0)
63+
atomic_add(flags, flag);
64+
break;
65+
case DAX_FLAG_CLEAR:
66+
if ((atomic_read(flags) & flag) == flag)
67+
atomic_sub(flags, flag);
68+
break;
69+
default:
70+
break;
71+
}
72+
73+
return atomic_read(flags);
74+
}
75+
4076
static int itostr(int num, char *str)
4177
{
4278
int index = 0, digit_count = 0;
@@ -198,6 +234,7 @@ static int establish_instance(struct processing_module *mod)
198234
int ret = 0;
199235
struct comp_dev *dev = mod->dev;
200236
struct sof_dax *dax_ctx = module_get_private_data(mod);
237+
struct dax_adapter_data *adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
201238
uint32_t persist_sz;
202239
uint32_t scratch_sz;
203240

@@ -220,7 +257,7 @@ static int establish_instance(struct processing_module *mod)
220257
}
221258

222259
/* set DAX_ENABLE_MASK bit to trigger the fully update of kcontrol values */
223-
dax_ctx->update_flags |= DAX_ENABLE_MASK;
260+
flag_process(&adapter_data->flags, DAX_ENABLE_MASK, DAX_FLAG_ADD);
224261

225262
comp_info(dev, "allocated: persist %u, scratch %u. version: %s",
226263
persist_sz, scratch_sz, dax_get_version());
@@ -374,6 +411,7 @@ static int dax_set_param_wrapper(struct processing_module *mod,
374411
int ret = 0;
375412
struct comp_dev *dev = mod->dev;
376413
struct sof_dax *dax_ctx = module_get_private_data(mod);
414+
struct dax_adapter_data *adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
377415
int32_t tmp_val;
378416

379417
switch (id) {
@@ -385,41 +423,41 @@ static int dax_set_param_wrapper(struct processing_module *mod,
385423
tmp_val = !!tmp_val;
386424
if (dax_ctx->enable != tmp_val) {
387425
dax_ctx->enable = tmp_val;
388-
dax_ctx->update_flags |= DAX_ENABLE_MASK;
426+
flag_process(&adapter_data->flags, DAX_ENABLE_MASK, DAX_FLAG_ADD);
389427
}
390428
break;
391429
case DAX_PARAM_ID_ABSOLUTE_VOLUME:
392430
dax_ctx->volume = *((int32_t *)value);
393-
dax_ctx->update_flags |= DAX_VOLUME_MASK;
431+
flag_process(&adapter_data->flags, DAX_VOLUME_MASK, DAX_FLAG_ADD);
394432
break;
395433
case DAX_PARAM_ID_OUT_DEVICE:
396434
tmp_val = *((int32_t *)value);
397435
if (dax_ctx->out_device != tmp_val) {
398436
dax_ctx->out_device = tmp_val;
399-
dax_ctx->update_flags |= DAX_DEVICE_MASK;
437+
flag_process(&adapter_data->flags, DAX_DEVICE_MASK, DAX_FLAG_ADD);
400438
}
401439
break;
402440
case DAX_PARAM_ID_PROFILE:
403441
tmp_val = *((int32_t *)value);
404442
if (dax_ctx->profile != tmp_val) {
405443
dax_ctx->profile = tmp_val;
406-
dax_ctx->update_flags |= DAX_PROFILE_MASK;
444+
flag_process(&adapter_data->flags, DAX_PROFILE_MASK, DAX_FLAG_ADD);
407445
}
408446
break;
409447
case DAX_PARAM_ID_CP_ENABLE:
410448
tmp_val = *((int32_t *)value);
411449
tmp_val = !!tmp_val;
412450
if (dax_ctx->content_processing_enable != tmp_val) {
413451
dax_ctx->content_processing_enable = tmp_val;
414-
dax_ctx->update_flags |= DAX_CP_MASK;
452+
flag_process(&adapter_data->flags, DAX_CP_MASK, DAX_FLAG_ADD);
415453
}
416454
break;
417455
case DAX_PARAM_ID_CTC_ENABLE:
418456
tmp_val = *((int32_t *)value);
419457
tmp_val = !!tmp_val;
420458
if (dax_ctx->ctc_enable != tmp_val) {
421459
dax_ctx->ctc_enable = tmp_val;
422-
dax_ctx->update_flags |= DAX_CTC_MASK;
460+
flag_process(&adapter_data->flags, DAX_CTC_MASK, DAX_FLAG_ADD);
423461
}
424462
break;
425463
case DAX_PARAM_ID_ENDPOINT:
@@ -472,55 +510,58 @@ static int update_params_from_buffer(struct processing_module *mod, void *data,
472510
static void check_and_update_settings(struct processing_module *mod)
473511
{
474512
struct sof_dax *dax_ctx = module_get_private_data(mod);
513+
struct dax_adapter_data *adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
475514

476-
if (dax_ctx->update_flags & DAX_ENABLE_MASK) {
515+
if (flag_process(&adapter_data->flags, DAX_ENABLE_MASK, DAX_FLAG_READ)) {
516+
flag_process(&adapter_data->flags, DAX_ENABLE_MASK, DAX_FLAG_CLEAR);
477517
set_enable(mod, dax_ctx->enable);
478518
if (dax_ctx->enable) {
479-
dax_ctx->update_flags |= DAX_DEVICE_MASK;
480-
dax_ctx->update_flags |= DAX_VOLUME_MASK;
519+
flag_process(&adapter_data->flags, DAX_DEVICE_MASK, DAX_FLAG_ADD);
520+
flag_process(&adapter_data->flags, DAX_VOLUME_MASK, DAX_FLAG_ADD);
481521
}
482-
dax_ctx->update_flags &= ~DAX_ENABLE_MASK;
483522
return;
484523
}
485-
if (dax_ctx->update_flags & DAX_DEVICE_MASK) {
524+
if (flag_process(&adapter_data->flags, DAX_DEVICE_MASK, DAX_FLAG_READ)) {
525+
flag_process(&adapter_data->flags, DAX_DEVICE_MASK, DAX_FLAG_CLEAR);
486526
set_device(mod, dax_ctx->out_device);
487527
set_tuning_device(mod, dax_ctx->tuning_device);
488-
dax_ctx->update_flags |= DAX_PROFILE_MASK;
489-
dax_ctx->update_flags &= ~DAX_DEVICE_MASK;
528+
flag_process(&adapter_data->flags, DAX_PROFILE_MASK, DAX_FLAG_ADD);
490529
return;
491530
}
492-
if (dax_ctx->update_flags & DAX_CTC_MASK) {
531+
if (flag_process(&adapter_data->flags, DAX_CTC_MASK, DAX_FLAG_READ)) {
532+
flag_process(&adapter_data->flags, DAX_CTC_MASK, DAX_FLAG_CLEAR);
493533
set_crosstalk_cancellation_enable(mod, dax_ctx->ctc_enable);
494-
dax_ctx->update_flags |= DAX_PROFILE_MASK;
495-
dax_ctx->update_flags &= ~DAX_CTC_MASK;
534+
flag_process(&adapter_data->flags, DAX_PROFILE_MASK, DAX_FLAG_ADD);
496535
return;
497536
}
498-
if (dax_ctx->update_flags & DAX_PROFILE_MASK) {
537+
if (flag_process(&adapter_data->flags, DAX_PROFILE_MASK, DAX_FLAG_READ)) {
538+
flag_process(&adapter_data->flags, DAX_PROFILE_MASK, DAX_FLAG_CLEAR);
499539
set_profile(mod, dax_ctx->profile);
500540
if (!dax_ctx->content_processing_enable)
501-
dax_ctx->update_flags |= DAX_CP_MASK;
502-
dax_ctx->update_flags &= ~DAX_PROFILE_MASK;
541+
flag_process(&adapter_data->flags, DAX_CP_MASK, DAX_FLAG_ADD);
503542
return;
504543
}
505-
if (dax_ctx->update_flags & DAX_CP_MASK) {
544+
if (flag_process(&adapter_data->flags, DAX_CP_MASK, DAX_FLAG_READ)) {
545+
flag_process(&adapter_data->flags, DAX_CP_MASK, DAX_FLAG_CLEAR);
506546
set_content_processing_enable(mod, dax_ctx->content_processing_enable);
507-
dax_ctx->update_flags &= ~DAX_CP_MASK;
508547
return;
509548
}
510-
if (dax_ctx->update_flags & DAX_VOLUME_MASK) {
549+
if (flag_process(&adapter_data->flags, DAX_VOLUME_MASK, DAX_FLAG_READ)) {
550+
flag_process(&adapter_data->flags, DAX_VOLUME_MASK, DAX_FLAG_CLEAR);
511551
set_volume(mod, dax_ctx->volume);
512-
dax_ctx->update_flags &= ~DAX_VOLUME_MASK;
513552
}
514553
}
515554

516555
static int sof_dax_reset(struct processing_module *mod)
517556
{
518557
struct sof_dax *dax_ctx = module_get_private_data(mod);
558+
struct dax_adapter_data *adapter_data;
519559

520560
/* dax instance will be established on prepare(), and destroyed on reset() */
521561
if (dax_ctx) {
522-
if (dax_ctx->update_flags & DAX_PROCESSING_MASK) {
523-
dax_ctx->update_flags |= DAX_RESET_MASK;
562+
adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
563+
if (flag_process(&adapter_data->flags, DAX_PROCESSING_MASK, DAX_FLAG_READ)) {
564+
flag_process(&adapter_data->flags, DAX_PROCESSING_MASK, DAX_FLAG_ADD);
524565
} else {
525566
destroy_instance(mod);
526567
dax_buffer_release(mod, &dax_ctx->input_buffer);
@@ -534,10 +575,12 @@ static int sof_dax_reset(struct processing_module *mod)
534575
static int sof_dax_free(struct processing_module *mod)
535576
{
536577
struct sof_dax *dax_ctx = module_get_private_data(mod);
578+
struct dax_adapter_data *adapter_data;
537579

538580
if (dax_ctx) {
539-
if (dax_ctx->update_flags & DAX_PROCESSING_MASK) {
540-
dax_ctx->update_flags |= DAX_FREE_MASK;
581+
adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
582+
if (flag_process(&adapter_data->flags, DAX_PROCESSING_MASK, DAX_FLAG_READ)) {
583+
flag_process(&adapter_data->flags, DAX_FREE_MASK, DAX_FLAG_ADD);
541584
} else {
542585
sof_dax_reset(mod);
543586
dax_buffer_release(mod, &dax_ctx->tuning_file_buffer);
@@ -553,15 +596,18 @@ static int sof_dax_free(struct processing_module *mod)
553596
static void check_and_update_state(struct processing_module *mod)
554597
{
555598
struct sof_dax *dax_ctx = module_get_private_data(mod);
599+
struct dax_adapter_data *adapter_data;
556600

557601
if (!dax_ctx)
558602
return;
559603

560-
if (dax_ctx->update_flags & DAX_FREE_MASK) {
604+
adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
605+
if (flag_process(&adapter_data->flags, DAX_FREE_MASK, DAX_FLAG_READ)) {
606+
flag_process(&adapter_data->flags, DAX_FREE_MASK, DAX_FLAG_CLEAR);
561607
sof_dax_free(mod);
562-
} else if (dax_ctx->update_flags & DAX_RESET_MASK) {
608+
} else if (flag_process(&adapter_data->flags, DAX_RESET_MASK, DAX_FLAG_READ)) {
609+
flag_process(&adapter_data->flags, DAX_RESET_MASK, DAX_FLAG_CLEAR);
563610
sof_dax_reset(mod);
564-
dax_ctx->update_flags &= ~DAX_RESET_MASK;
565611
}
566612
}
567613

@@ -570,8 +616,9 @@ static int sof_dax_init(struct processing_module *mod)
570616
struct comp_dev *dev = mod->dev;
571617
struct module_data *md = &mod->priv;
572618
struct sof_dax *dax_ctx;
619+
struct dax_adapter_data *adapter_data;
573620

574-
md->private = mod_zalloc(mod, sizeof(struct sof_dax));
621+
md->private = mod_zalloc(mod, sizeof(struct sof_dax) + sizeof(struct dax_adapter_data));
575622
if (!md->private) {
576623
comp_err(dev, "failed to allocate %u bytes for initialization",
577624
sizeof(struct sof_dax));
@@ -586,6 +633,9 @@ static int sof_dax_init(struct processing_module *mod)
586633
dax_ctx->volume = 1 << 23;
587634
dax_ctx->update_flags = 0;
588635

636+
adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
637+
atomic_init(&adapter_data->flags, 0);
638+
589639
dax_ctx->blob_handler = mod_data_blob_handler_new(mod);
590640
if (!dax_ctx->blob_handler) {
591641
comp_err(dev, "create blob handler failed");
@@ -734,6 +784,7 @@ static int sof_dax_process(struct processing_module *mod, struct sof_source **so
734784
int num_of_sources, struct sof_sink **sinks, int num_of_sinks)
735785
{
736786
struct sof_dax *dax_ctx = module_get_private_data(mod);
787+
struct dax_adapter_data *adapter_data;
737788
struct sof_source *source = sources[0];
738789
struct sof_sink *sink = sinks[0];
739790
uint8_t *buf, *bufstart, *bufend, *dax_buf;
@@ -747,7 +798,8 @@ static int sof_dax_process(struct processing_module *mod, struct sof_source **so
747798
return -EINVAL;
748799
}
749800

750-
dax_ctx->update_flags |= DAX_PROCESSING_MASK;
801+
adapter_data = (struct dax_adapter_data *)dax_ctx->reserved;
802+
flag_process(&adapter_data->flags, DAX_PROCESSING_MASK, DAX_FLAG_ADD);
751803
/* source stream -> internal input buffer */
752804
consumed_bytes = MIN(source_get_data_available(source), dax_input_buffer->free);
753805
source_get_data(source, consumed_bytes, (void *)&buf, (void *)&bufstart, &bufsz);
@@ -779,7 +831,7 @@ static int sof_dax_process(struct processing_module *mod, struct sof_source **so
779831
dax_buffer_consume(dax_output_buffer, produced_bytes);
780832
sink_commit_buffer(sink, produced_bytes);
781833
}
782-
dax_ctx->update_flags &= ~DAX_PROCESSING_MASK;
834+
flag_process(&adapter_data->flags, DAX_PROCESSING_MASK, DAX_FLAG_CLEAR);
783835
check_and_update_state(mod);
784836

785837
return 0;

‎third_party/include/dax_inf.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,17 @@ struct sof_dax {
8383
int32_t ctc_enable;
8484
int32_t content_processing_enable;
8585
int32_t volume;
86-
uint32_t update_flags;
86+
uint32_t update_flags; /* Deprecated */
8787

8888
/* DAX buffers */
8989
struct dax_buffer persist_buffer; /* Used for dax instance */
9090
struct dax_buffer scratch_buffer; /* Used for dax process */
9191
struct dax_buffer input_buffer;
9292
struct dax_buffer output_buffer;
9393
struct dax_buffer tuning_file_buffer;
94+
95+
/* Other data */
96+
uint8_t reserved[];
9497
};
9598

9699
/**

0 commit comments

Comments
 (0)