Skip to content

LL scheduler domain refinement #4089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 7 commits into from
May 13, 2021
Merged

LL scheduler domain refinement #4089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0851d45
schedule: add a core index for schedule_data
keyonjie May 7, 2021
a2188d8
cavs: timer: remove the wrong timer_disable
keyonjie May 7, 2021
79e2923
ll_schedule_domain: add return value of domain_unregister()
keyonjie Apr 26, 2021
d17e28d
ll_schedule: rename num_clients to registered_cores
keyonjie Apr 27, 2021
72310da
ll_schedule_domain: add flag to store the enabled cores
keyonjie Apr 26, 2021
d3d290c
ll_schedule: domain tasks/cores refinement
keyonjie Apr 26, 2021
2efcb00
ll_schedule: add new_target_tick to ll_schedule_domain
keyonjie May 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/drivers/intel/cavs/timer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,6 @@ static int platform_timer_register(struct timer *timer,
/* enable timer interrupt */
interrupt_enable(timer->logical_irq, arg);

/* disable timer interrupt on core level */
timer_disable(timer, arg, cpu_get_id());

return err;
}

Expand Down
89 changes: 59 additions & 30 deletions src/include/sof/schedule/ll_schedule_domain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ struct ll_schedule_domain_ops {
int (*domain_register)(struct ll_schedule_domain *domain,
uint64_t period, struct task *task,
void (*handler)(void *arg), void *arg);
void (*domain_unregister)(struct ll_schedule_domain *domain,
struct task *task, uint32_t num_tasks);
int (*domain_unregister)(struct ll_schedule_domain *domain,
struct task *task, uint32_t num_tasks);
void (*domain_enable)(struct ll_schedule_domain *domain, int core);
void (*domain_disable)(struct ll_schedule_domain *domain, int core);
void (*domain_set)(struct ll_schedule_domain *domain, uint64_t start);
Expand All @@ -43,9 +43,11 @@ struct ll_schedule_domain_ops {

struct ll_schedule_domain {
uint64_t next_tick; /**< ticks just set for next run */
uint64_t new_target_tick; /**< for the next set, used during the reschedule stage */
spinlock_t lock; /**< standard lock */
atomic_t total_num_tasks; /**< total number of registered tasks */
atomic_t num_clients; /**< number of registered cores */
atomic_t registered_cores; /**< number of registered cores */
atomic_t enabled_cores; /**< number of enabled cores */
uint32_t ticks_per_ms; /**< number of clock ticks per ms */
int type; /**< domain type */
int clk; /**< source clock */
Expand Down Expand Up @@ -86,14 +88,35 @@ static inline struct ll_schedule_domain *domain_init
domain->ops = ops;
/* maximum value means no tick has been set to timer */
domain->next_tick = UINT64_MAX;
domain->new_target_tick = UINT64_MAX;

spinlock_init(&domain->lock);
atomic_init(&domain->total_num_tasks, 0);
atomic_init(&domain->num_clients, 0);
atomic_init(&domain->registered_cores, 0);
atomic_init(&domain->enabled_cores, 0);

return domain;
}

/* configure the next interrupt for domain */
static inline void domain_set(struct ll_schedule_domain *domain, uint64_t start)
{
if (domain->ops->domain_set)
domain->ops->domain_set(domain, start);
else
domain->next_tick = start;
}

/* clear the interrupt for domain */
static inline void domain_clear(struct ll_schedule_domain *domain)
{
if (domain->ops->domain_clear)
domain->ops->domain_clear(domain);

/* reset to denote no tick/interrupt is set */
domain->next_tick = UINT64_MAX;
}

static inline int domain_register(struct ll_schedule_domain *domain,
uint64_t period, struct task *task,
void (*handler)(void *arg), void *arg)
Expand All @@ -104,51 +127,57 @@ static inline int domain_register(struct ll_schedule_domain *domain,

ret = domain->ops->domain_register(domain, period, task, handler, arg);

if (!ret) {
Comment thread
keyonjie marked this conversation as resolved.
Outdated
/* registered one more task, increase the count */
atomic_add(&domain->total_num_tasks, 1);

if (!domain->registered[cpu_get_id()]) {
/* first task of the core, new client registered */
domain->registered[cpu_get_id()] = true;
atomic_add(&domain->registered_cores, 1);
}
}

return ret;
}

static inline void domain_unregister(struct ll_schedule_domain *domain,
struct task *task, uint32_t num_tasks)
{
int ret;

assert(domain->ops->domain_unregister);

domain->ops->domain_unregister(domain, task, num_tasks);
ret = domain->ops->domain_unregister(domain, task, num_tasks);

if (!ret) {
Comment thread
keyonjie marked this conversation as resolved.
Outdated
/* unregistered the task, decrease the count */
atomic_sub(&domain->total_num_tasks, 1);

/* the last task of the core, unregister the client/core */
if (!num_tasks && domain->registered[cpu_get_id()]) {
domain->registered[cpu_get_id()] = false;
atomic_sub(&domain->registered_cores, 1);
}
}
}

static inline void domain_enable(struct ll_schedule_domain *domain, int core)
{
if (domain->ops->domain_enable)
if (!domain->enabled[core] && domain->ops->domain_enable) {
domain->ops->domain_enable(domain, core);

domain->enabled[core] = true;
atomic_add(&domain->enabled_cores, 1);
}
}

static inline void domain_disable(struct ll_schedule_domain *domain, int core)
{
if (domain->ops->domain_disable)
if (domain->enabled[core] && domain->ops->domain_disable) {
domain->ops->domain_disable(domain, core);

}

/* configure the next interrupt for domain */
static inline void domain_set(struct ll_schedule_domain *domain, uint64_t start)
{
if (domain->ops->domain_set)
domain->ops->domain_set(domain, start);
else
domain->next_tick = start;

}

/* clear the interrupt for domain */
static inline void domain_clear(struct ll_schedule_domain *domain)
{
if (domain->ops->domain_clear)
domain->ops->domain_clear(domain);

/* reset to denote no tick/interrupt is set */
domain->next_tick = UINT64_MAX;

domain->enabled[core] = false;
atomic_sub(&domain->enabled_cores, 1);
}
}

static inline bool domain_is_pending(struct ll_schedule_domain *domain,
Expand Down
8 changes: 6 additions & 2 deletions src/include/sof/schedule/schedule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#define __SOF_SCHEDULE_SCHEDULE_H__

#include <sof/common.h>
#include <sof/lib/cpu.h>
#include <sof/list.h>
#include <sof/schedule/task.h>
#include <sof/trace/trace.h>
Expand Down Expand Up @@ -122,6 +123,7 @@ struct scheduler_ops {
struct schedule_data {
struct list_item list; /**< list of schedulers */
int type; /**< SOF_SCHEDULE_ type */
uint32_t core; /**< the index of the core the scheduler run on */
const struct scheduler_ops *ops; /**< scheduler operations */
void *data; /**< pointer to private data */
};
Expand Down Expand Up @@ -210,7 +212,8 @@ static inline int schedule_task(struct task *task, uint64_t start,

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type && sch->ops->schedule_task)
if (task->type == sch->type && sch->core == cpu_get_id() &&
sch->ops->schedule_task)
return sch->ops->schedule_task(sch->data, task, start,
period);
}
Expand Down Expand Up @@ -249,7 +252,8 @@ static inline int schedule_task_cancel(struct task *task)

list_for_item(slist, &schedulers->list) {
sch = container_of(slist, struct schedule_data, list);
if (task->type == sch->type && sch->ops->schedule_task_cancel)
if (task->type == sch->type && sch->core == cpu_get_id() &&
sch->ops->schedule_task_cancel)
return sch->ops->schedule_task_cancel(sch->data, task);
}

Expand Down
14 changes: 9 additions & 5 deletions src/schedule/dma_multi_chan_domain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,11 @@ static void dma_multi_chan_domain_irq_unregister(struct dma_domain_data *data)
* \param[in,out] domain Pointer to schedule domain.
* \param[in,out] task Task to be unregistered from the domain..
* \param[in] num_tasks Number of currently scheduled tasks.
* \return Error code.
*/
static void dma_multi_chan_domain_unregister(struct ll_schedule_domain *domain,
struct task *task,
uint32_t num_tasks)
static int dma_multi_chan_domain_unregister(struct ll_schedule_domain *domain,
struct task *task,
uint32_t num_tasks)
{
struct dma_domain *dma_domain = ll_sch_domain_get_pdata(domain);
struct pipeline_task *pipe_task = pipeline_task_get(task);
Expand All @@ -201,7 +202,7 @@ static void dma_multi_chan_domain_unregister(struct ll_schedule_domain *domain,

/* check if task should be unregistered */
if (!pipe_task->registrable)
return;
return 0;

for (i = 0; i < dma_domain->num_dma; ++i) {
for (j = 0; j < dmas[i].plat_data.channels; ++j) {
Expand Down Expand Up @@ -236,9 +237,12 @@ static void dma_multi_chan_domain_unregister(struct ll_schedule_domain *domain,
else if (!dma_domain->channel_mask[i][core])
dma_multi_chan_domain_irq_unregister(
dma_domain->arg[i][core]);
return;
return 0;
}
}

/* task in running or unregistered at all, can't unregister it */
return -EINVAL;

@lyakh lyakh Apr 28, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

presumably we'd end up here if the domain hadn't been registered or maybe is still busy?

@keyonjie keyonjie Apr 30, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it could be either, so I use '-EINVAL' here.

}

/**
Expand Down
17 changes: 10 additions & 7 deletions src/schedule/dma_single_chan_domain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,11 @@ static void dma_domain_unregister_owner(struct ll_schedule_domain *domain,
* \param[in,out] domain Pointer to schedule domain.
* \param[in,out] task Task to be unregistered from the domain.
* \param[in] num_tasks Number of currently scheduled tasks.
* \return Error code.
*/
static void dma_single_chan_domain_unregister(struct ll_schedule_domain *domain,
struct task *task,
uint32_t num_tasks)
static int dma_single_chan_domain_unregister(struct ll_schedule_domain *domain,
struct task *task,
uint32_t num_tasks)
{
struct dma_domain *dma_domain = ll_sch_domain_get_pdata(domain);
struct pipeline_task *pipe_task = pipeline_task_get(task);
Expand All @@ -370,27 +371,29 @@ static void dma_single_chan_domain_unregister(struct ll_schedule_domain *domain,

/* check if task should be unregistered */
if (!pipe_task->registrable)
return;
return 0;

/* channel not registered */
if (!data->channel)
return;
return -EINVAL;

/* unregister domain owner */
if (dma_domain->owner == core) {
dma_domain_unregister_owner(domain, data);
return;
return 0;
}

/* some channel still running, so return */
if (dma_chan_is_any_running(dmas, dma_domain->num_dma))
return;
return -EBUSY;

/* no more transfers scheduled on this core */
dma_single_chan_domain_irq_unregister(data);
data->channel = NULL;

notifier_unregister(domain, NULL, NOTIFIER_ID_DMA_DOMAIN_CHANGE);

return 0;
}

/**
Expand Down
Loading