Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[Horng-Shyang Liao]

Hi CK,

Reply in line.

On Thu, 2016-05-26 at 15:28 +0800, CK Hu wrote:
> Hi, HS:
> 
> Replay inline.
> 
> On Tue, 2016-05-24 at 20:27 +0800, Horng-Shyang Liao wrote:
> > Hi CK,
> > 
> > Reply in line.
> > 
> > On Tue, 2016-05-24 at 11:05 +0800, CK Hu wrote:
> > > Hi, HS:
> > > 
> > > Some comments below.
> > > 
> > ...
> > > > +static void cmdq_thread_irq_handler(struct cmdq *cmdq, int tid)
> > > > +{
> > > > +   struct cmdq_thread *thread = >thread[tid];
> > > > +   unsigned long flags = 0L;
> > > > +   int value;
> > > > +
> > > > +   spin_lock_irqsave(>exec_lock, flags);
> > > > +
> > > > +   /*
> > > > +* it is possible for another CPU core
> > > > +* to run "release task" right before we acquire the spin lock
> > > > +* and thus reset / disable this HW thread
> > > > +* so we check both the IRQ flag and the enable bit of this 
> > > > thread
> > > > +*/
> > > > +   value = cmdq_thread_readl(thread, CMDQ_THR_IRQ_STATUS);
> > > > +   if (!(value & CMDQ_THR_IRQ_MASK)) {
> > > > +   spin_unlock_irqrestore(>exec_lock, flags);
> > > > +   return;
> > > > +   }
> > > 
> > > If this case happen and just return without clearing irq status, the irq
> > > would keep triggering and system hang up. So just remove this checking
> > > and go down to clear irq status.
> > 
> > This case is safe because irq status is cleared.
> > But, next if condition has the problem which you mentioned.
> > 
> > I will change it as below.
> > 
> > if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> >   CMDQ_THR_ENABLED)) {
> > cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
> > spin_unlock_irqrestore(>exec_lock, flags);
> > return;
> > }
> > 
> > If thread is disabled, tasks must be all finished.
> > Therefore, just clear irq status and return.
> > 
> 
> For irq status checking part, if irq status & irq mask is zero, remove
> this checking and let it go down, it still do nothing because value &
> CMDQ_THR_IRQ_ERROR is zero and value & CMDQ_THR_IRQ_DONE is zero. So you
> can just remove this checking and get the same result.
> 
> In general HW design, once a HW is not enable, it does not trigger
> interrupt any more. Be sure that it's necessary to clear irq status even
> though thread is disable.
> 

I Will remove first if condition,
so rewrite first two if condition parts as below.

value = cmdq_thread_readl(thread, CMDQ_THR_IRQ_STATUS);
cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);

if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
CMDQ_THR_ENABLED))
value = 0;

> > > > +
> > > > +   if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> > > > + CMDQ_THR_ENABLED)) {
> > > > +   spin_unlock_irqrestore(>exec_lock, flags);
> > > > +   return;
> > > > +   }
> > > > +
> > > > +   cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
> > > > +
> > > > +   if (value & CMDQ_THR_IRQ_ERROR)
> > > > +   cmdq_handle_error_done(cmdq, thread, true);
> > > > +   else if (value & CMDQ_THR_IRQ_DONE)
> > > > +   cmdq_handle_error_done(cmdq, thread, false);
> > > 
> > > These irq status checking and clearing code here is the same as those in
> > > cmdq_task_handle_error_result(). To move the checking and clearing code
> > > into cmdq_handle_error_done() and here just to call
> > > cmdq_handle_error_done(cmdq, thread) would reduce duplicated code.
> > 
> > Will do.
> > 
> > > > +
> > > > +   spin_unlock_irqrestore(>exec_lock, flags);
> > > > +}
> > ...
> > ...
> > > 
> > > Regards,
> > > CK
> > 
> > Thanks,
> > HS
> > 
> 
> Regards,
> CK
> 
> 

Thanks,
HS

Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[Horng-Shyang Liao]

Hi CK,

Reply in line.

On Thu, 2016-05-26 at 15:28 +0800, CK Hu wrote:
> Hi, HS:
> 
> Replay inline.
> 
> On Tue, 2016-05-24 at 20:27 +0800, Horng-Shyang Liao wrote:
> > Hi CK,
> > 
> > Reply in line.
> > 
> > On Tue, 2016-05-24 at 11:05 +0800, CK Hu wrote:
> > > Hi, HS:
> > > 
> > > Some comments below.
> > > 
> > ...
> > > > +static void cmdq_thread_irq_handler(struct cmdq *cmdq, int tid)
> > > > +{
> > > > +   struct cmdq_thread *thread = >thread[tid];
> > > > +   unsigned long flags = 0L;
> > > > +   int value;
> > > > +
> > > > +   spin_lock_irqsave(>exec_lock, flags);
> > > > +
> > > > +   /*
> > > > +* it is possible for another CPU core
> > > > +* to run "release task" right before we acquire the spin lock
> > > > +* and thus reset / disable this HW thread
> > > > +* so we check both the IRQ flag and the enable bit of this 
> > > > thread
> > > > +*/
> > > > +   value = cmdq_thread_readl(thread, CMDQ_THR_IRQ_STATUS);
> > > > +   if (!(value & CMDQ_THR_IRQ_MASK)) {
> > > > +   spin_unlock_irqrestore(>exec_lock, flags);
> > > > +   return;
> > > > +   }
> > > 
> > > If this case happen and just return without clearing irq status, the irq
> > > would keep triggering and system hang up. So just remove this checking
> > > and go down to clear irq status.
> > 
> > This case is safe because irq status is cleared.
> > But, next if condition has the problem which you mentioned.
> > 
> > I will change it as below.
> > 
> > if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> >   CMDQ_THR_ENABLED)) {
> > cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
> > spin_unlock_irqrestore(>exec_lock, flags);
> > return;
> > }
> > 
> > If thread is disabled, tasks must be all finished.
> > Therefore, just clear irq status and return.
> > 
> 
> For irq status checking part, if irq status & irq mask is zero, remove
> this checking and let it go down, it still do nothing because value &
> CMDQ_THR_IRQ_ERROR is zero and value & CMDQ_THR_IRQ_DONE is zero. So you
> can just remove this checking and get the same result.
> 
> In general HW design, once a HW is not enable, it does not trigger
> interrupt any more. Be sure that it's necessary to clear irq status even
> though thread is disable.
> 

I Will remove first if condition,
so rewrite first two if condition parts as below.

value = cmdq_thread_readl(thread, CMDQ_THR_IRQ_STATUS);
cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);

if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
CMDQ_THR_ENABLED))
value = 0;

> > > > +
> > > > +   if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> > > > + CMDQ_THR_ENABLED)) {
> > > > +   spin_unlock_irqrestore(>exec_lock, flags);
> > > > +   return;
> > > > +   }
> > > > +
> > > > +   cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
> > > > +
> > > > +   if (value & CMDQ_THR_IRQ_ERROR)
> > > > +   cmdq_handle_error_done(cmdq, thread, true);
> > > > +   else if (value & CMDQ_THR_IRQ_DONE)
> > > > +   cmdq_handle_error_done(cmdq, thread, false);
> > > 
> > > These irq status checking and clearing code here is the same as those in
> > > cmdq_task_handle_error_result(). To move the checking and clearing code
> > > into cmdq_handle_error_done() and here just to call
> > > cmdq_handle_error_done(cmdq, thread) would reduce duplicated code.
> > 
> > Will do.
> > 
> > > > +
> > > > +   spin_unlock_irqrestore(>exec_lock, flags);
> > > > +}
> > ...
> > ...
> > > 
> > > Regards,
> > > CK
> > 
> > Thanks,
> > HS
> > 
> 
> Regards,
> CK
> 
> 

Thanks,
HS

Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[CK Hu]

Hi, HS:

Replay inline.

On Tue, 2016-05-24 at 20:27 +0800, Horng-Shyang Liao wrote:
> Hi CK,
> 
> Reply in line.
> 
> On Tue, 2016-05-24 at 11:05 +0800, CK Hu wrote:
> > Hi, HS:
> > 
> > Some comments below.
> > 
> ...
> > > +static void cmdq_thread_irq_handler(struct cmdq *cmdq, int tid)
> > > +{
> > > + struct cmdq_thread *thread = >thread[tid];
> > > + unsigned long flags = 0L;
> > > + int value;
> > > +
> > > + spin_lock_irqsave(>exec_lock, flags);
> > > +
> > > + /*
> > > +  * it is possible for another CPU core
> > > +  * to run "release task" right before we acquire the spin lock
> > > +  * and thus reset / disable this HW thread
> > > +  * so we check both the IRQ flag and the enable bit of this thread
> > > +  */
> > > + value = cmdq_thread_readl(thread, CMDQ_THR_IRQ_STATUS);
> > > + if (!(value & CMDQ_THR_IRQ_MASK)) {
> > > + spin_unlock_irqrestore(>exec_lock, flags);
> > > + return;
> > > + }
> > 
> > If this case happen and just return without clearing irq status, the irq
> > would keep triggering and system hang up. So just remove this checking
> > and go down to clear irq status.
> 
> This case is safe because irq status is cleared.
> But, next if condition has the problem which you mentioned.
> 
> I will change it as below.
> 
>   if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> CMDQ_THR_ENABLED)) {
>   cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
>   spin_unlock_irqrestore(>exec_lock, flags);
>   return;
>   }
> 
> If thread is disabled, tasks must be all finished.
> Therefore, just clear irq status and return.
> 

For irq status checking part, if irq status & irq mask is zero, remove
this checking and let it go down, it still do nothing because value &
CMDQ_THR_IRQ_ERROR is zero and value & CMDQ_THR_IRQ_DONE is zero. So you
can just remove this checking and get the same result.

In general HW design, once a HW is not enable, it does not trigger
interrupt any more. Be sure that it's necessary to clear irq status even
though thread is disable.


> > > +
> > > + if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> > > +   CMDQ_THR_ENABLED)) {
> > > + spin_unlock_irqrestore(>exec_lock, flags);
> > > + return;
> > > + }
> > > +
> > > + cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
> > > +
> > > + if (value & CMDQ_THR_IRQ_ERROR)
> > > + cmdq_handle_error_done(cmdq, thread, true);
> > > + else if (value & CMDQ_THR_IRQ_DONE)
> > > + cmdq_handle_error_done(cmdq, thread, false);
> > 
> > These irq status checking and clearing code here is the same as those in
> > cmdq_task_handle_error_result(). To move the checking and clearing code
> > into cmdq_handle_error_done() and here just to call
> > cmdq_handle_error_done(cmdq, thread) would reduce duplicated code.
> 
> Will do.
> 
> > > +
> > > + spin_unlock_irqrestore(>exec_lock, flags);
> > > +}
> ...
> ...
> > 
> > Regards,
> > CK
> 
> Thanks,
> HS
> 

Regards,
CK

Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[CK Hu]

Hi, HS:

Replay inline.

On Tue, 2016-05-24 at 20:27 +0800, Horng-Shyang Liao wrote:
> Hi CK,
> 
> Reply in line.
> 
> On Tue, 2016-05-24 at 11:05 +0800, CK Hu wrote:
> > Hi, HS:
> > 
> > Some comments below.
> > 
> ...
> > > +static void cmdq_thread_irq_handler(struct cmdq *cmdq, int tid)
> > > +{
> > > + struct cmdq_thread *thread = >thread[tid];
> > > + unsigned long flags = 0L;
> > > + int value;
> > > +
> > > + spin_lock_irqsave(>exec_lock, flags);
> > > +
> > > + /*
> > > +  * it is possible for another CPU core
> > > +  * to run "release task" right before we acquire the spin lock
> > > +  * and thus reset / disable this HW thread
> > > +  * so we check both the IRQ flag and the enable bit of this thread
> > > +  */
> > > + value = cmdq_thread_readl(thread, CMDQ_THR_IRQ_STATUS);
> > > + if (!(value & CMDQ_THR_IRQ_MASK)) {
> > > + spin_unlock_irqrestore(>exec_lock, flags);
> > > + return;
> > > + }
> > 
> > If this case happen and just return without clearing irq status, the irq
> > would keep triggering and system hang up. So just remove this checking
> > and go down to clear irq status.
> 
> This case is safe because irq status is cleared.
> But, next if condition has the problem which you mentioned.
> 
> I will change it as below.
> 
>   if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> CMDQ_THR_ENABLED)) {
>   cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
>   spin_unlock_irqrestore(>exec_lock, flags);
>   return;
>   }
> 
> If thread is disabled, tasks must be all finished.
> Therefore, just clear irq status and return.
> 

For irq status checking part, if irq status & irq mask is zero, remove
this checking and let it go down, it still do nothing because value &
CMDQ_THR_IRQ_ERROR is zero and value & CMDQ_THR_IRQ_DONE is zero. So you
can just remove this checking and get the same result.

In general HW design, once a HW is not enable, it does not trigger
interrupt any more. Be sure that it's necessary to clear irq status even
though thread is disable.


> > > +
> > > + if (!(cmdq_thread_readl(thread, CMDQ_THR_ENABLE_TASK) &
> > > +   CMDQ_THR_ENABLED)) {
> > > + spin_unlock_irqrestore(>exec_lock, flags);
> > > + return;
> > > + }
> > > +
> > > + cmdq_thread_writel(thread, ~value, CMDQ_THR_IRQ_STATUS);
> > > +
> > > + if (value & CMDQ_THR_IRQ_ERROR)
> > > + cmdq_handle_error_done(cmdq, thread, true);
> > > + else if (value & CMDQ_THR_IRQ_DONE)
> > > + cmdq_handle_error_done(cmdq, thread, false);
> > 
> > These irq status checking and clearing code here is the same as those in
> > cmdq_task_handle_error_result(). To move the checking and clearing code
> > into cmdq_handle_error_done() and here just to call
> > cmdq_handle_error_done(cmdq, thread) would reduce duplicated code.
> 
> Will do.
> 
> > > +
> > > + spin_unlock_irqrestore(>exec_lock, flags);
> > > +}
> ...
> ...
> > 
> > Regards,
> > CK
> 
> Thanks,
> HS
> 

Regards,
CK

Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[Horng-Shyang Liao]

Hi CK,

Reply in line.

On Tue, 2016-05-24 at 11:05 +0800, CK Hu wrote:
> Hi, HS:
> 
> Some comments below.
> 
> On Mon, 2016-05-23 at 20:23 +0800, HS Liao wrote:
...
> > +struct cmdq_task {
> > +   struct cmdq *cmdq;
> > +   struct list_headlist_entry;
> > +   enum cmdq_task_statetask_state;
> > +   void*va_base; /* va */
> 
> It's obviously that va_base is va, so the comment is redundant.

Will remove comment.

> > +   dma_addr_t  mva_base; /* pa */
> > +   u64 engine_flag;
> > +   size_t  command_size;
> > +   u32 num_cmd;
> > +   struct cmdq_thread  *thread;
> > +   struct cmdq_task_cb cb;
> > +   struct work_struct  release_work;
> > +};
...
> > +static struct cmdq_task *cmdq_task_acquire(struct cmdq_rec *rec,
> > +  struct cmdq_task_cb cb)
> > +{
> > +   struct cmdq *cmdq = rec->cmdq;
> > +   struct device *dev = cmdq->dev;
> > +   struct cmdq_task *task;
> > +
> > +   task = kzalloc(sizeof(*task), GFP_KERNEL);
> > +   INIT_LIST_HEAD(>list_entry);
> > +   task->va_base = dma_alloc_coherent(dev, rec->command_size,
> > +  >mva_base, GFP_KERNEL);
> > +   if (!task->va_base) {
> > +   dev_err(dev, "allocate command buffer failed\n");
> > +   kfree(task);
> > +   return NULL;
> > +   }
> > +
> > +   task->cmdq = cmdq;
> > +   task->command_size = rec->command_size;
> > +   task->engine_flag = rec->engine_flag;
> > +   task->task_state = TASK_STATE_BUSY;
> > +   task->cb = cb;
> > +   memcpy(task->va_base, rec->buf, rec->command_size);
> > +   task->num_cmd = task->command_size / sizeof(u64);
> 
> Already define CMDQ_INST_SIZE, so use it and the readability is better.

Will use CMDQ_INST_SIZE.

> > +   return task;
> > +}
...
> > +static void cmdq_task_exec(struct cmdq_task *task, struct cmdq_thread 
> > *thread)
> > +{
> > +   struct cmdq *cmdq = task->cmdq;
> > +   unsigned long flags;
> > +   unsigned long curr_pa, end_pa;
> > +
> > +   WARN_ON(clk_prepare_enable(cmdq->clock) < 0);
> > +   spin_lock_irqsave(>exec_lock, flags);
> > +   task->thread = thread;
> > +   task->task_state = TASK_STATE_BUSY;
> 
> Once a task is created, its state is already BUSY, so this assignment is
> redundant.

I prefer to keep this because task may add, remove, or reorder states in
the future. If we remove this, it is easy to cause a bug here.

> > +   if (list_empty(>task_busy_list)) {
> > +   WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> > +
> > +   cmdq_thread_writel(thread, task->mva_base, CMDQ_THR_CURR_ADDR);
> > +   cmdq_thread_writel(thread, task->mva_base + task->command_size,
> > +  CMDQ_THR_END_ADDR);
> > +   cmdq_thread_writel(thread, CMDQ_THR_PRIORITY, CMDQ_THR_CFG);
> > +   cmdq_thread_writel(thread, CMDQ_THR_IRQ_EN,
> > +  CMDQ_THR_IRQ_ENABLE);
> > +
> > +   list_move_tail(>list_entry,
> > +  >task_busy_list);
> 
> Moving this statement to just before spin_unlock_irqrestore() can reduce
> the duplicated code in else part.

Will do.

> > +
> > +   cmdq_thread_writel(thread, CMDQ_THR_ENABLED,
> > +  CMDQ_THR_ENABLE_TASK);
> > +   } else {
> > +   WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > +
> > +   /*
> > +* check boundary condition
> > +* PC = END - 8, EOC is executed
> > +* PC = END, all CMDs are executed
> > +*/
> > +   curr_pa = cmdq_thread_readl(thread, CMDQ_THR_CURR_ADDR);
> > +   end_pa = cmdq_thread_readl(thread, CMDQ_THR_END_ADDR);
> > +   if (curr_pa == end_pa - 8 || curr_pa == end_pa) {
> > +   /* set to this task directly */
> > +   cmdq_thread_writel(thread, task->mva_base,
> > +  CMDQ_THR_CURR_ADDR);
> > +   } else {
> > +   cmdq_task_insert_into_thread(task);
> > +
> > +   if (thread == >thread[CMDQ_THR_DISP_MAIN_IDX] ||
> > +   thread == >thread[CMDQ_THR_DISP_SUB_IDX])
> > +   cmdq_task_remove_wfe(task);
> > +
> > +   smp_mb(); /* modify jump before enable thread */
> > +   }
> > +
> > +   cmdq_thread_writel(thread, task->mva_base + task->command_size,
> > +  CMDQ_THR_END_ADDR);
> > +   list_move_tail(>list_entry, >task_busy_list);
> > +   cmdq_thread_resume(thread);
> > +   }
> > +   spin_unlock_irqrestore(>exec_lock, flags);
> > +}
...
> > +static void cmdq_thread_irq_handler(struct cmdq *cmdq, int tid)
> > +{
> > +   struct cmdq_thread *thread = >thread[tid];
> > +   unsigned long flags = 0L;
> > +   int value;
> > +
> > +   spin_lock_irqsave(>exec_lock, flags);
> > +
> > +   /*
> > +* it 

Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[Horng-Shyang Liao]

Hi CK,

Reply in line.

On Tue, 2016-05-24 at 11:05 +0800, CK Hu wrote:
> Hi, HS:
> 
> Some comments below.
> 
> On Mon, 2016-05-23 at 20:23 +0800, HS Liao wrote:
...
> > +struct cmdq_task {
> > +   struct cmdq *cmdq;
> > +   struct list_headlist_entry;
> > +   enum cmdq_task_statetask_state;
> > +   void*va_base; /* va */
> 
> It's obviously that va_base is va, so the comment is redundant.

Will remove comment.

> > +   dma_addr_t  mva_base; /* pa */
> > +   u64 engine_flag;
> > +   size_t  command_size;
> > +   u32 num_cmd;
> > +   struct cmdq_thread  *thread;
> > +   struct cmdq_task_cb cb;
> > +   struct work_struct  release_work;
> > +};
...
> > +static struct cmdq_task *cmdq_task_acquire(struct cmdq_rec *rec,
> > +  struct cmdq_task_cb cb)
> > +{
> > +   struct cmdq *cmdq = rec->cmdq;
> > +   struct device *dev = cmdq->dev;
> > +   struct cmdq_task *task;
> > +
> > +   task = kzalloc(sizeof(*task), GFP_KERNEL);
> > +   INIT_LIST_HEAD(>list_entry);
> > +   task->va_base = dma_alloc_coherent(dev, rec->command_size,
> > +  >mva_base, GFP_KERNEL);
> > +   if (!task->va_base) {
> > +   dev_err(dev, "allocate command buffer failed\n");
> > +   kfree(task);
> > +   return NULL;
> > +   }
> > +
> > +   task->cmdq = cmdq;
> > +   task->command_size = rec->command_size;
> > +   task->engine_flag = rec->engine_flag;
> > +   task->task_state = TASK_STATE_BUSY;
> > +   task->cb = cb;
> > +   memcpy(task->va_base, rec->buf, rec->command_size);
> > +   task->num_cmd = task->command_size / sizeof(u64);
> 
> Already define CMDQ_INST_SIZE, so use it and the readability is better.

Will use CMDQ_INST_SIZE.

> > +   return task;
> > +}
...
> > +static void cmdq_task_exec(struct cmdq_task *task, struct cmdq_thread 
> > *thread)
> > +{
> > +   struct cmdq *cmdq = task->cmdq;
> > +   unsigned long flags;
> > +   unsigned long curr_pa, end_pa;
> > +
> > +   WARN_ON(clk_prepare_enable(cmdq->clock) < 0);
> > +   spin_lock_irqsave(>exec_lock, flags);
> > +   task->thread = thread;
> > +   task->task_state = TASK_STATE_BUSY;
> 
> Once a task is created, its state is already BUSY, so this assignment is
> redundant.

I prefer to keep this because task may add, remove, or reorder states in
the future. If we remove this, it is easy to cause a bug here.

> > +   if (list_empty(>task_busy_list)) {
> > +   WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
> > +
> > +   cmdq_thread_writel(thread, task->mva_base, CMDQ_THR_CURR_ADDR);
> > +   cmdq_thread_writel(thread, task->mva_base + task->command_size,
> > +  CMDQ_THR_END_ADDR);
> > +   cmdq_thread_writel(thread, CMDQ_THR_PRIORITY, CMDQ_THR_CFG);
> > +   cmdq_thread_writel(thread, CMDQ_THR_IRQ_EN,
> > +  CMDQ_THR_IRQ_ENABLE);
> > +
> > +   list_move_tail(>list_entry,
> > +  >task_busy_list);
> 
> Moving this statement to just before spin_unlock_irqrestore() can reduce
> the duplicated code in else part.

Will do.

> > +
> > +   cmdq_thread_writel(thread, CMDQ_THR_ENABLED,
> > +  CMDQ_THR_ENABLE_TASK);
> > +   } else {
> > +   WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
> > +
> > +   /*
> > +* check boundary condition
> > +* PC = END - 8, EOC is executed
> > +* PC = END, all CMDs are executed
> > +*/
> > +   curr_pa = cmdq_thread_readl(thread, CMDQ_THR_CURR_ADDR);
> > +   end_pa = cmdq_thread_readl(thread, CMDQ_THR_END_ADDR);
> > +   if (curr_pa == end_pa - 8 || curr_pa == end_pa) {
> > +   /* set to this task directly */
> > +   cmdq_thread_writel(thread, task->mva_base,
> > +  CMDQ_THR_CURR_ADDR);
> > +   } else {
> > +   cmdq_task_insert_into_thread(task);
> > +
> > +   if (thread == >thread[CMDQ_THR_DISP_MAIN_IDX] ||
> > +   thread == >thread[CMDQ_THR_DISP_SUB_IDX])
> > +   cmdq_task_remove_wfe(task);
> > +
> > +   smp_mb(); /* modify jump before enable thread */
> > +   }
> > +
> > +   cmdq_thread_writel(thread, task->mva_base + task->command_size,
> > +  CMDQ_THR_END_ADDR);
> > +   list_move_tail(>list_entry, >task_busy_list);
> > +   cmdq_thread_resume(thread);
> > +   }
> > +   spin_unlock_irqrestore(>exec_lock, flags);
> > +}
...
> > +static void cmdq_thread_irq_handler(struct cmdq *cmdq, int tid)
> > +{
> > +   struct cmdq_thread *thread = >thread[tid];
> > +   unsigned long flags = 0L;
> > +   int value;
> > +
> > +   spin_lock_irqsave(>exec_lock, flags);
> > +
> > +   /*
> > +* it 

Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[CK Hu]

Hi, HS:

Some comments below.

On Mon, 2016-05-23 at 20:23 +0800, HS Liao wrote:
> This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> CMDQ is used to help read/write registers with critical time limitation,
> such as updating display configuration during the vblank. It controls
> Global Command Engine (GCE) hardware to achieve this requirement.
> Currently, CMDQ only supports display related hardwares, but we expect
> it can be extended to other hardwares for future requirements.
> 
> Signed-off-by: HS Liao 
> Signed-off-by: CK Hu 
> ---
>  drivers/soc/mediatek/Kconfig|  10 +
>  drivers/soc/mediatek/Makefile   |   1 +
>  drivers/soc/mediatek/mtk-cmdq.c | 904 
> 
>  include/soc/mediatek/cmdq.h | 197 +
>  4 files changed, 1112 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mtk-cmdq.c
>  create mode 100644 include/soc/mediatek/cmdq.h
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 0a4ea80..c4ad75c 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -1,6 +1,16 @@
>  #
>  # MediaTek SoC drivers
>  #
> +config MTK_CMDQ
> + bool "MediaTek CMDQ Support"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + select MTK_INFRACFG
> + help
> +   Say yes here to add support for the MediaTek Command Queue (CMDQ)
> +   driver. The CMDQ is used to help read/write registers with critical
> +   time limitation, such as updating display configuration during the
> +   vblank.
> +
>  config MTK_INFRACFG
>   bool "MediaTek INFRACFG Support"
>   depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 12998b0..f7397ef 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq.o
>  obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> diff --git a/drivers/soc/mediatek/mtk-cmdq.c b/drivers/soc/mediatek/mtk-cmdq.c
> new file mode 100644
> index 000..f8c5d02
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-cmdq.c
> @@ -0,0 +1,904 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define CMDQ_INITIAL_CMD_BLOCK_SIZE  PAGE_SIZE
> +#define CMDQ_INST_SIZE   8 /* instruction is 64-bit */
> +
> +#define CMDQ_DEFAULT_TIMEOUT_MS  1000
> +
> +#define CMDQ_DRIVER_DEVICE_NAME  "mtk_cmdq"
> +#define CMDQ_CLK_NAME"gce"
> +
> +#define CMDQ_CURR_IRQ_STATUS 0x010
> +#define CMDQ_THR_SLOT_CYCLES 0x030
> +
> +#define CMDQ_THR_BASE0x100
> +#define CMDQ_THR_SHIFT   0x080
> +#define CMDQ_THR_WARM_RESET  0x00
> +#define CMDQ_THR_ENABLE_TASK 0x04
> +#define CMDQ_THR_SUSPEND_TASK0x08
> +#define CMDQ_THR_CURR_STATUS 0x0c
> +#define CMDQ_THR_IRQ_STATUS  0x10
> +#define CMDQ_THR_IRQ_ENABLE  0x14
> +#define CMDQ_THR_CURR_ADDR   0x20
> +#define CMDQ_THR_END_ADDR0x24
> +#define CMDQ_THR_CFG 0x40
> +
> +#define CMDQ_IRQ_MASK0x
> +
> +#define CMDQ_THR_ENABLED 0x1
> +#define CMDQ_THR_DISABLED0x0
> +#define CMDQ_THR_SUSPEND 0x1
> +#define CMDQ_THR_RESUME  0x0
> +#define CMDQ_THR_STATUS_SUSPENDEDBIT(1)
> +#define CMDQ_THR_DO_WARM_RESET   BIT(0)
> +#define CMDQ_THR_ACTIVE_SLOT_CYCLES  0x3200
> +#define CMDQ_THR_PRIORITY3
> +#define CMDQ_THR_IRQ_DONE0x1
> +#define CMDQ_THR_IRQ_ERROR   0x12
> +#define CMDQ_THR_IRQ_EN  0x13 /* done + error */
> +#define CMDQ_THR_IRQ_MASK0x13
> +#define CMDQ_THR_EXECUTING   BIT(31)
> +
> +#define CMDQ_ARG_A_WRITE_MASK0x
> +#define CMDQ_SUBSYS_MASK 0x1f
> +
> +#define CMDQ_OP_CODE_SHIFT   24
> +#define CMDQ_SUBSYS_SHIFT16
> +
> +#define CMDQ_JUMP_BY_OFFSET  0x1000
> +#define CMDQ_JUMP_BY_PA  0x1001

Re: [PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[CK Hu]

Hi, HS:

Some comments below.

On Mon, 2016-05-23 at 20:23 +0800, HS Liao wrote:
> This patch is first version of Mediatek Command Queue(CMDQ) driver. The
> CMDQ is used to help read/write registers with critical time limitation,
> such as updating display configuration during the vblank. It controls
> Global Command Engine (GCE) hardware to achieve this requirement.
> Currently, CMDQ only supports display related hardwares, but we expect
> it can be extended to other hardwares for future requirements.
> 
> Signed-off-by: HS Liao 
> Signed-off-by: CK Hu 
> ---
>  drivers/soc/mediatek/Kconfig|  10 +
>  drivers/soc/mediatek/Makefile   |   1 +
>  drivers/soc/mediatek/mtk-cmdq.c | 904 
> 
>  include/soc/mediatek/cmdq.h | 197 +
>  4 files changed, 1112 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mtk-cmdq.c
>  create mode 100644 include/soc/mediatek/cmdq.h
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 0a4ea80..c4ad75c 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -1,6 +1,16 @@
>  #
>  # MediaTek SoC drivers
>  #
> +config MTK_CMDQ
> + bool "MediaTek CMDQ Support"
> + depends on ARCH_MEDIATEK || COMPILE_TEST
> + select MTK_INFRACFG
> + help
> +   Say yes here to add support for the MediaTek Command Queue (CMDQ)
> +   driver. The CMDQ is used to help read/write registers with critical
> +   time limitation, such as updating display configuration during the
> +   vblank.
> +
>  config MTK_INFRACFG
>   bool "MediaTek INFRACFG Support"
>   depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 12998b0..f7397ef 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq.o
>  obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> diff --git a/drivers/soc/mediatek/mtk-cmdq.c b/drivers/soc/mediatek/mtk-cmdq.c
> new file mode 100644
> index 000..f8c5d02
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-cmdq.c
> @@ -0,0 +1,904 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define CMDQ_INITIAL_CMD_BLOCK_SIZE  PAGE_SIZE
> +#define CMDQ_INST_SIZE   8 /* instruction is 64-bit */
> +
> +#define CMDQ_DEFAULT_TIMEOUT_MS  1000
> +
> +#define CMDQ_DRIVER_DEVICE_NAME  "mtk_cmdq"
> +#define CMDQ_CLK_NAME"gce"
> +
> +#define CMDQ_CURR_IRQ_STATUS 0x010
> +#define CMDQ_THR_SLOT_CYCLES 0x030
> +
> +#define CMDQ_THR_BASE0x100
> +#define CMDQ_THR_SHIFT   0x080
> +#define CMDQ_THR_WARM_RESET  0x00
> +#define CMDQ_THR_ENABLE_TASK 0x04
> +#define CMDQ_THR_SUSPEND_TASK0x08
> +#define CMDQ_THR_CURR_STATUS 0x0c
> +#define CMDQ_THR_IRQ_STATUS  0x10
> +#define CMDQ_THR_IRQ_ENABLE  0x14
> +#define CMDQ_THR_CURR_ADDR   0x20
> +#define CMDQ_THR_END_ADDR0x24
> +#define CMDQ_THR_CFG 0x40
> +
> +#define CMDQ_IRQ_MASK0x
> +
> +#define CMDQ_THR_ENABLED 0x1
> +#define CMDQ_THR_DISABLED0x0
> +#define CMDQ_THR_SUSPEND 0x1
> +#define CMDQ_THR_RESUME  0x0
> +#define CMDQ_THR_STATUS_SUSPENDEDBIT(1)
> +#define CMDQ_THR_DO_WARM_RESET   BIT(0)
> +#define CMDQ_THR_ACTIVE_SLOT_CYCLES  0x3200
> +#define CMDQ_THR_PRIORITY3
> +#define CMDQ_THR_IRQ_DONE0x1
> +#define CMDQ_THR_IRQ_ERROR   0x12
> +#define CMDQ_THR_IRQ_EN  0x13 /* done + error */
> +#define CMDQ_THR_IRQ_MASK0x13
> +#define CMDQ_THR_EXECUTING   BIT(31)
> +
> +#define CMDQ_ARG_A_WRITE_MASK0x
> +#define CMDQ_SUBSYS_MASK 0x1f
> +
> +#define CMDQ_OP_CODE_SHIFT   24
> +#define CMDQ_SUBSYS_SHIFT16
> +
> +#define CMDQ_JUMP_BY_OFFSET  0x1000
> +#define CMDQ_JUMP_BY_PA  0x1001
> +#define CMDQ_JUMP_PASS  

[PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[HS Liao]

This patch is first version of Mediatek Command Queue(CMDQ) driver. The
CMDQ is used to help read/write registers with critical time limitation,
such as updating display configuration during the vblank. It controls
Global Command Engine (GCE) hardware to achieve this requirement.
Currently, CMDQ only supports display related hardwares, but we expect
it can be extended to other hardwares for future requirements.

Signed-off-by: HS Liao 
Signed-off-by: CK Hu 
---
 drivers/soc/mediatek/Kconfig|  10 +
 drivers/soc/mediatek/Makefile   |   1 +
 drivers/soc/mediatek/mtk-cmdq.c | 904 
 include/soc/mediatek/cmdq.h | 197 +
 4 files changed, 1112 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-cmdq.c
 create mode 100644 include/soc/mediatek/cmdq.h

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 0a4ea80..c4ad75c 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -1,6 +1,16 @@
 #
 # MediaTek SoC drivers
 #
+config MTK_CMDQ
+   bool "MediaTek CMDQ Support"
+   depends on ARCH_MEDIATEK || COMPILE_TEST
+   select MTK_INFRACFG
+   help
+ Say yes here to add support for the MediaTek Command Queue (CMDQ)
+ driver. The CMDQ is used to help read/write registers with critical
+ time limitation, such as updating display configuration during the
+ vblank.
+
 config MTK_INFRACFG
bool "MediaTek INFRACFG Support"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 12998b0..f7397ef 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq.o
 obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
 obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
 obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
diff --git a/drivers/soc/mediatek/mtk-cmdq.c b/drivers/soc/mediatek/mtk-cmdq.c
new file mode 100644
index 000..f8c5d02
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-cmdq.c
@@ -0,0 +1,904 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CMDQ_INITIAL_CMD_BLOCK_SIZEPAGE_SIZE
+#define CMDQ_INST_SIZE 8 /* instruction is 64-bit */
+
+#define CMDQ_DEFAULT_TIMEOUT_MS1000
+
+#define CMDQ_DRIVER_DEVICE_NAME"mtk_cmdq"
+#define CMDQ_CLK_NAME  "gce"
+
+#define CMDQ_CURR_IRQ_STATUS   0x010
+#define CMDQ_THR_SLOT_CYCLES   0x030
+
+#define CMDQ_THR_BASE  0x100
+#define CMDQ_THR_SHIFT 0x080
+#define CMDQ_THR_WARM_RESET0x00
+#define CMDQ_THR_ENABLE_TASK   0x04
+#define CMDQ_THR_SUSPEND_TASK  0x08
+#define CMDQ_THR_CURR_STATUS   0x0c
+#define CMDQ_THR_IRQ_STATUS0x10
+#define CMDQ_THR_IRQ_ENABLE0x14
+#define CMDQ_THR_CURR_ADDR 0x20
+#define CMDQ_THR_END_ADDR  0x24
+#define CMDQ_THR_CFG   0x40
+
+#define CMDQ_IRQ_MASK  0x
+
+#define CMDQ_THR_ENABLED   0x1
+#define CMDQ_THR_DISABLED  0x0
+#define CMDQ_THR_SUSPEND   0x1
+#define CMDQ_THR_RESUME0x0
+#define CMDQ_THR_STATUS_SUSPENDED  BIT(1)
+#define CMDQ_THR_DO_WARM_RESET BIT(0)
+#define CMDQ_THR_ACTIVE_SLOT_CYCLES0x3200
+#define CMDQ_THR_PRIORITY  3
+#define CMDQ_THR_IRQ_DONE  0x1
+#define CMDQ_THR_IRQ_ERROR 0x12
+#define CMDQ_THR_IRQ_EN0x13 /* done + error */
+#define CMDQ_THR_IRQ_MASK  0x13
+#define CMDQ_THR_EXECUTING BIT(31)
+
+#define CMDQ_ARG_A_WRITE_MASK  0x
+#define CMDQ_SUBSYS_MASK   0x1f
+
+#define CMDQ_OP_CODE_SHIFT 24
+#define CMDQ_SUBSYS_SHIFT  16
+
+#define CMDQ_JUMP_BY_OFFSET0x1000
+#define CMDQ_JUMP_BY_PA0x1001
+#define CMDQ_JUMP_PASS CMDQ_INST_SIZE
+
+#define CMDQ_WFE_UPDATEBIT(31)
+#define CMDQ_WFE_WAIT  BIT(15)
+#define CMDQ_WFE_WAIT_VALUE0x1
+
+#define CMDQ_EOC_IRQ_ENBIT(0)
+
+#define CMDQ_ENABLE_MASK   BIT(0)
+

[PATCH v7 2/4] CMDQ: Mediatek CMDQ driver

[HS Liao]

This patch is first version of Mediatek Command Queue(CMDQ) driver. The
CMDQ is used to help read/write registers with critical time limitation,
such as updating display configuration during the vblank. It controls
Global Command Engine (GCE) hardware to achieve this requirement.
Currently, CMDQ only supports display related hardwares, but we expect
it can be extended to other hardwares for future requirements.

Signed-off-by: HS Liao 
Signed-off-by: CK Hu 
---
 drivers/soc/mediatek/Kconfig|  10 +
 drivers/soc/mediatek/Makefile   |   1 +
 drivers/soc/mediatek/mtk-cmdq.c | 904 
 include/soc/mediatek/cmdq.h | 197 +
 4 files changed, 1112 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-cmdq.c
 create mode 100644 include/soc/mediatek/cmdq.h

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 0a4ea80..c4ad75c 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -1,6 +1,16 @@
 #
 # MediaTek SoC drivers
 #
+config MTK_CMDQ
+   bool "MediaTek CMDQ Support"
+   depends on ARCH_MEDIATEK || COMPILE_TEST
+   select MTK_INFRACFG
+   help
+ Say yes here to add support for the MediaTek Command Queue (CMDQ)
+ driver. The CMDQ is used to help read/write registers with critical
+ time limitation, such as updating display configuration during the
+ vblank.
+
 config MTK_INFRACFG
bool "MediaTek INFRACFG Support"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 12998b0..f7397ef 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq.o
 obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
 obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
 obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
diff --git a/drivers/soc/mediatek/mtk-cmdq.c b/drivers/soc/mediatek/mtk-cmdq.c
new file mode 100644
index 000..f8c5d02
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-cmdq.c
@@ -0,0 +1,904 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CMDQ_INITIAL_CMD_BLOCK_SIZEPAGE_SIZE
+#define CMDQ_INST_SIZE 8 /* instruction is 64-bit */
+
+#define CMDQ_DEFAULT_TIMEOUT_MS1000
+
+#define CMDQ_DRIVER_DEVICE_NAME"mtk_cmdq"
+#define CMDQ_CLK_NAME  "gce"
+
+#define CMDQ_CURR_IRQ_STATUS   0x010
+#define CMDQ_THR_SLOT_CYCLES   0x030
+
+#define CMDQ_THR_BASE  0x100
+#define CMDQ_THR_SHIFT 0x080
+#define CMDQ_THR_WARM_RESET0x00
+#define CMDQ_THR_ENABLE_TASK   0x04
+#define CMDQ_THR_SUSPEND_TASK  0x08
+#define CMDQ_THR_CURR_STATUS   0x0c
+#define CMDQ_THR_IRQ_STATUS0x10
+#define CMDQ_THR_IRQ_ENABLE0x14
+#define CMDQ_THR_CURR_ADDR 0x20
+#define CMDQ_THR_END_ADDR  0x24
+#define CMDQ_THR_CFG   0x40
+
+#define CMDQ_IRQ_MASK  0x
+
+#define CMDQ_THR_ENABLED   0x1
+#define CMDQ_THR_DISABLED  0x0
+#define CMDQ_THR_SUSPEND   0x1
+#define CMDQ_THR_RESUME0x0
+#define CMDQ_THR_STATUS_SUSPENDED  BIT(1)
+#define CMDQ_THR_DO_WARM_RESET BIT(0)
+#define CMDQ_THR_ACTIVE_SLOT_CYCLES0x3200
+#define CMDQ_THR_PRIORITY  3
+#define CMDQ_THR_IRQ_DONE  0x1
+#define CMDQ_THR_IRQ_ERROR 0x12
+#define CMDQ_THR_IRQ_EN0x13 /* done + error */
+#define CMDQ_THR_IRQ_MASK  0x13
+#define CMDQ_THR_EXECUTING BIT(31)
+
+#define CMDQ_ARG_A_WRITE_MASK  0x
+#define CMDQ_SUBSYS_MASK   0x1f
+
+#define CMDQ_OP_CODE_SHIFT 24
+#define CMDQ_SUBSYS_SHIFT  16
+
+#define CMDQ_JUMP_BY_OFFSET0x1000
+#define CMDQ_JUMP_BY_PA0x1001
+#define CMDQ_JUMP_PASS CMDQ_INST_SIZE
+
+#define CMDQ_WFE_UPDATEBIT(31)
+#define CMDQ_WFE_WAIT  BIT(15)
+#define CMDQ_WFE_WAIT_VALUE0x1
+
+#define CMDQ_EOC_IRQ_ENBIT(0)
+
+#define CMDQ_ENABLE_MASK   BIT(0)
+
+#define CMDQ_OP_CODE_MASK