Hi, Jason: On Wed, 2024-01-03 at 07:03 +0000, Jason-JH Lin (林睿祥) wrote: > On Thu, 2023-12-28 at 07:37 +0000, CK Hu (胡俊光) wrote: > > Hi, Jason: > > > > On Fri, 2023-12-22 at 12:52 +0800, Jason-JH.Lin wrote: > > > To support secure video path feature, GCE have to read/write > > > registgers > > > in the secure world. GCE will enable the secure access permission > > > to > > > the > > > HW who wants to access the secure content buffer. > > > > > > Add CMDQ secure mailbox driver to make CMDQ client user is able > > > to > > > sending their HW settings to the secure world. So that GCE can > > > execute > > > all instructions to configure HW in the secure world. > > > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@xxxxxxxxxxxx> > > > --- > > > > [snip] > > > > > + > > > +static bool cmdq_sec_irq_handler(struct cmdq_sec_thread > > > *sec_thread, > > > + const u32 cookie, const int err) > > > +{ > > > + struct cmdq_sec_task *sec_task; > > > + struct cmdq_task *task, *temp, *cur_task = NULL; > > > + struct cmdq_sec *cmdq = container_of(sec_thread->thread.chan- > > > > mbox, struct cmdq_sec, mbox); > > > > > > + unsigned long flags; > > > + int done; > > > + > > > + spin_lock_irqsave(&sec_thread->thread.chan->lock, flags); > > > + if (sec_thread->wait_cookie <= cookie) > > > + done = cookie - sec_thread->wait_cookie + 1; > > > + else if (sec_thread->wait_cookie == (cookie + 1) % > > > CMDQ_MAX_COOKIE_VALUE) > > > + done = 0; > > > + else > > > + done = CMDQ_MAX_COOKIE_VALUE - sec_thread->wait_cookie > > > + 1 + cookie + 1; > > > + > > > + list_for_each_entry_safe(task, temp, &sec_thread- > > > > thread.task_busy_list, list_entry) { > > > > > > + if (!done) > > > + break; > > > + > > > + sec_task = container_of(task, struct cmdq_sec_task, > > > task); > > > + cmdq_sec_task_done(sec_task, err); > > > + > > > + if (sec_thread->task_cnt) > > > + sec_thread->task_cnt -= 1; > > > + > > > + done--; > > > + } > > > + > > > + cur_task = list_first_entry_or_null(&sec_thread- > > > > thread.task_busy_list, > > > > > > + struct cmdq_task, > > > list_entry); > > > + if (err && cur_task) { > > > + spin_unlock_irqrestore(&sec_thread->thread.chan->lock, > > > flags); > > > + > > > + sec_task = container_of(cur_task, struct cmdq_sec_task, > > > task); > > > + > > > + /* for error task, cancel, callback and done */ > > > + memset(&cmdq->cancel, 0, sizeof(cmdq->cancel)); > > > + cmdq_sec_task_submit(cmdq, sec_task, > > > CMD_CMDQ_IWC_CANCEL_TASK, > > > + sec_thread->idx, &cmdq->cancel); > > > > cmdq->cancel is useless, so drop it. > > > > Regards, > > CK > > > > I think we should use this to tell CMDQ PTA which thread need to > reset > and update its cookie value, so CMDQ can still move on when error > occurs. CMD_CMDQ_IWC_CANCEL_TASK is used to tell CMDQ PTA which thread need to reset and update its cookie value, and cmdq->cancel is the return value. But you does not process this return value, so drop cmdq->cancel. Regards, CK > > Regards, > Jason-JH.Lin