All of lore.kernel.org
 help / color / mirror / Atom feed
From: Horng-Shyang Liao <hs.liao@mediatek.com>
To: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Daniel Kurtz <djkurtz@chromium.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-mediatek@lists.infradead.org>,
	<srv_heupstream@mediatek.com>,
	"Sascha Hauer" <kernel@pengutronix.de>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Nicolas Boichat <drinkcat@chromium.org>,
	CK HU <ck.hu@mediatek.com>,
	"cawa cheng" <cawa.cheng@mediatek.com>,
	Bibby Hsieh <bibby.hsieh@mediatek.com>,
	"YT Shen" <yt.shen@mediatek.com>,
	Daoyuan Huang <daoyuan.huang@mediatek.com>,
	Damon Chu <damon.chu@mediatek.com>,
	Josh-YC Liu <josh-yc.liu@mediatek.com>,
	Glory Hung <glory.hung@mediatek.com>,
	Jiaguang Zhang <jiaguang.zhang@mediatek.com>,
	Monica Wang <monica.wang@mediatek.com>, <hs.liao@mediatek.com>
Subject: Re: [PATCH v8 2/3] CMDQ: Mediatek CMDQ driver
Date: Wed, 8 Jun 2016 13:09:39 +0800	[thread overview]
Message-ID: <1465362579.9963.2.camel@mtksdaap41> (raw)
In-Reply-To: <5756FE9B.6020107@gmail.com>

Hi Matthias,

On Tue, 2016-06-07 at 19:04 +0200, Matthias Brugger wrote:
> 
> On 30/05/16 05:19, 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 <hs.liao@mediatek.com>
> > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > ---
> 
> [...]
> 
> > +static void cmdq_handle_error_done(struct cmdq *cmdq,
> > +				   struct cmdq_thread *thread, u32 irq_flag)
> > +{
> > +	struct cmdq_task *task, *tmp, *curr_task = NULL;
> > +	u32 curr_pa;
> > +	struct cmdq_cb_data cmdq_cb_data;
> > +	bool err;
> > +
> > +	if (irq_flag & CMDQ_THR_IRQ_ERROR)
> > +		err = true;
> > +	else if (irq_flag & CMDQ_THR_IRQ_DONE)
> > +		err = false;
> > +	else
> > +		return;
> > +
> > +	curr_pa = cmdq_thread_readl(thread, CMDQ_THR_CURR_ADDR);
> > +
> > +	list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > +				 list_entry) {
> > +		if (curr_pa >= task->pa_base &&
> > +		    curr_pa < (task->pa_base + task->command_size))
> > +			curr_task = task;
> > +		if (task->cb.cb) {
> > +			cmdq_cb_data.err = curr_task ? err : false;
> > +			cmdq_cb_data.data = task->cb.data;
> > +			task->cb.cb(cmdq_cb_data);
> > +		}
> 
> I think this is not right. If we got an IRQ_DONE, then the current task 
> is in execution, we should not call the callback until it has finished.

Thanks for your finding. This is a bug from CMDQ v6.
I will fix it in next version (CMDQ v9).

> 
> Regards,
> Matthias

Thanks,
HS

WARNING: multiple messages have this Message-ID (diff)
From: Horng-Shyang Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
To: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org,
	Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	Nicolas Boichat
	<drinkcat-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	CK HU <ck.hu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	cawa cheng <cawa.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Bibby Hsieh <bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	YT Shen <yt.shen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Daoyuan Huang
	<daoyuan.huang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Damon Chu <damon.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Josh-YC Liu <josh-yc.liu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Glory Hung <glory.hung-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Jiaguang Zhang
	<jiaguang.zhang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	Monica Wang <monica.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
	hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org
Subject: Re: [PATCH v8 2/3] CMDQ: Mediatek CMDQ driver
Date: Wed, 8 Jun 2016 13:09:39 +0800	[thread overview]
Message-ID: <1465362579.9963.2.camel@mtksdaap41> (raw)
In-Reply-To: <5756FE9B.6020107-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Matthias,

On Tue, 2016-06-07 at 19:04 +0200, Matthias Brugger wrote:
> 
> On 30/05/16 05:19, 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 <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: CK Hu <ck.hu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> 
> [...]
> 
> > +static void cmdq_handle_error_done(struct cmdq *cmdq,
> > +				   struct cmdq_thread *thread, u32 irq_flag)
> > +{
> > +	struct cmdq_task *task, *tmp, *curr_task = NULL;
> > +	u32 curr_pa;
> > +	struct cmdq_cb_data cmdq_cb_data;
> > +	bool err;
> > +
> > +	if (irq_flag & CMDQ_THR_IRQ_ERROR)
> > +		err = true;
> > +	else if (irq_flag & CMDQ_THR_IRQ_DONE)
> > +		err = false;
> > +	else
> > +		return;
> > +
> > +	curr_pa = cmdq_thread_readl(thread, CMDQ_THR_CURR_ADDR);
> > +
> > +	list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > +				 list_entry) {
> > +		if (curr_pa >= task->pa_base &&
> > +		    curr_pa < (task->pa_base + task->command_size))
> > +			curr_task = task;
> > +		if (task->cb.cb) {
> > +			cmdq_cb_data.err = curr_task ? err : false;
> > +			cmdq_cb_data.data = task->cb.data;
> > +			task->cb.cb(cmdq_cb_data);
> > +		}
> 
> I think this is not right. If we got an IRQ_DONE, then the current task 
> is in execution, we should not call the callback until it has finished.

Thanks for your finding. This is a bug from CMDQ v6.
I will fix it in next version (CMDQ v9).

> 
> Regards,
> Matthias

Thanks,
HS

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: hs.liao@mediatek.com (Horng-Shyang Liao)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v8 2/3] CMDQ: Mediatek CMDQ driver
Date: Wed, 8 Jun 2016 13:09:39 +0800	[thread overview]
Message-ID: <1465362579.9963.2.camel@mtksdaap41> (raw)
In-Reply-To: <5756FE9B.6020107@gmail.com>

Hi Matthias,

On Tue, 2016-06-07 at 19:04 +0200, Matthias Brugger wrote:
> 
> On 30/05/16 05:19, 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 <hs.liao@mediatek.com>
> > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > ---
> 
> [...]
> 
> > +static void cmdq_handle_error_done(struct cmdq *cmdq,
> > +				   struct cmdq_thread *thread, u32 irq_flag)
> > +{
> > +	struct cmdq_task *task, *tmp, *curr_task = NULL;
> > +	u32 curr_pa;
> > +	struct cmdq_cb_data cmdq_cb_data;
> > +	bool err;
> > +
> > +	if (irq_flag & CMDQ_THR_IRQ_ERROR)
> > +		err = true;
> > +	else if (irq_flag & CMDQ_THR_IRQ_DONE)
> > +		err = false;
> > +	else
> > +		return;
> > +
> > +	curr_pa = cmdq_thread_readl(thread, CMDQ_THR_CURR_ADDR);
> > +
> > +	list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > +				 list_entry) {
> > +		if (curr_pa >= task->pa_base &&
> > +		    curr_pa < (task->pa_base + task->command_size))
> > +			curr_task = task;
> > +		if (task->cb.cb) {
> > +			cmdq_cb_data.err = curr_task ? err : false;
> > +			cmdq_cb_data.data = task->cb.data;
> > +			task->cb.cb(cmdq_cb_data);
> > +		}
> 
> I think this is not right. If we got an IRQ_DONE, then the current task 
> is in execution, we should not call the callback until it has finished.

Thanks for your finding. This is a bug from CMDQ v6.
I will fix it in next version (CMDQ v9).

> 
> Regards,
> Matthias

Thanks,
HS

  reply	other threads:[~2016-06-08  5:09 UTC|newest]

Thread overview: 125+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-30  3:19 [PATCH v8 0/3] Mediatek MT8173 CMDQ support HS Liao
2016-05-30  3:19 ` HS Liao
2016-05-30  3:19 ` HS Liao
2016-05-30  3:19 ` [PATCH v8 1/3] dt-bindings: soc: Add documentation for the MediaTek GCE unit HS Liao
2016-05-30  3:19   ` HS Liao
2016-05-30  3:19   ` HS Liao
2016-05-30  3:19 ` [PATCH v8 2/3] CMDQ: Mediatek CMDQ driver HS Liao
2016-05-30  3:19   ` HS Liao
2016-05-30  3:19   ` HS Liao
2016-05-30  6:49   ` CK Hu
2016-05-30  6:49     ` CK Hu
2016-05-30  6:49     ` CK Hu
2016-05-30  9:38     ` Horng-Shyang Liao
2016-05-30  9:38       ` Horng-Shyang Liao
2016-05-30  9:38       ` Horng-Shyang Liao
2016-05-30 15:31   ` Matthias Brugger
2016-05-30 15:31     ` Matthias Brugger
2016-05-31  8:36     ` Horng-Shyang Liao
2016-05-31  8:36       ` Horng-Shyang Liao
2016-05-31  8:36       ` Horng-Shyang Liao
2016-05-31 20:04       ` Matthias Brugger
2016-05-31 20:04         ` Matthias Brugger
2016-06-01  9:57         ` Horng-Shyang Liao
2016-06-01  9:57           ` Horng-Shyang Liao
2016-06-01  9:57           ` Horng-Shyang Liao
2016-06-02  8:46           ` Matthias Brugger
2016-06-02  8:46             ` Matthias Brugger
2016-06-02  8:46             ` Matthias Brugger
2016-06-03  6:12             ` Horng-Shyang Liao
2016-06-03  6:12               ` Horng-Shyang Liao
2016-06-03  6:12               ` Horng-Shyang Liao
2016-06-03 11:18               ` Matthias Brugger
2016-06-03 11:18                 ` Matthias Brugger
2016-06-03 12:13                 ` Horng-Shyang Liao
2016-06-03 12:13                   ` Horng-Shyang Liao
2016-06-03 12:13                   ` Horng-Shyang Liao
2016-06-03 13:11                   ` Matthias Brugger
2016-06-03 13:11                     ` Matthias Brugger
2016-06-03 13:11                     ` Matthias Brugger
2016-06-07 16:59                     ` Matthias Brugger
2016-06-07 16:59                       ` Matthias Brugger
2016-06-08  5:40                       ` Horng-Shyang Liao
2016-06-08  5:40                         ` Horng-Shyang Liao
2016-06-08  5:40                         ` Horng-Shyang Liao
2016-06-08 10:45                         ` Matthias Brugger
2016-06-08 10:45                           ` Matthias Brugger
2016-06-08 10:45                           ` Matthias Brugger
2016-06-08 12:25                           ` Horng-Shyang Liao
2016-06-08 12:25                             ` Horng-Shyang Liao
2016-06-08 12:25                             ` Horng-Shyang Liao
2016-06-08 15:35                             ` Matthias Brugger
2016-06-08 15:35                               ` Matthias Brugger
2016-06-14  7:44                               ` Horng-Shyang Liao
2016-06-14  7:44                                 ` Horng-Shyang Liao
2016-06-14  7:44                                 ` Horng-Shyang Liao
2016-06-14 10:17                                 ` Matthias Brugger
2016-06-14 10:17                                   ` Matthias Brugger
2016-06-14 12:07                                   ` Horng-Shyang Liao
2016-06-14 12:07                                     ` Horng-Shyang Liao
2016-06-14 12:07                                     ` Horng-Shyang Liao
2016-06-17  8:28                                     ` Horng-Shyang Liao
2016-06-17  8:28                                       ` Horng-Shyang Liao
2016-06-17  8:28                                       ` Horng-Shyang Liao
2016-06-17 15:57                                       ` Matthias Brugger
2016-06-17 15:57                                         ` Matthias Brugger
2016-06-17 15:57                                         ` Matthias Brugger
2016-06-21  5:52                                         ` Horng-Shyang Liao
2016-06-21  5:52                                           ` Horng-Shyang Liao
2016-06-21  5:52                                           ` Horng-Shyang Liao
2016-06-21 13:41                                           ` Matthias Brugger
2016-06-21 13:41                                             ` Matthias Brugger
2016-06-21 13:41                                             ` Matthias Brugger
2016-06-22  5:43                                             ` Horng-Shyang Liao
2016-06-22  5:43                                               ` Horng-Shyang Liao
2016-06-22  5:43                                               ` Horng-Shyang Liao
2016-06-22  9:58                                               ` Matthias Brugger
2016-06-22  9:58                                                 ` Matthias Brugger
2016-06-22  9:58                                                 ` Matthias Brugger
2016-06-17 16:14                                     ` Matthias Brugger
2016-06-17 16:14                                       ` Matthias Brugger
2016-06-03 13:11                 ` Jassi Brar
2016-06-03 13:11                   ` Jassi Brar
2016-06-03 13:11                   ` Jassi Brar
2016-06-06  9:33                   ` Horng-Shyang Liao
2016-06-06  9:33                     ` Horng-Shyang Liao
2016-06-06  9:33                     ` Horng-Shyang Liao
2016-06-06 13:17                     ` Jassi Brar
2016-06-07  2:45                   ` Horng-Shyang Liao
2016-06-07  2:45                     ` Horng-Shyang Liao
2016-06-07  2:45                     ` Horng-Shyang Liao
2016-06-07 17:04   ` Matthias Brugger
2016-06-07 17:04     ` Matthias Brugger
2016-06-08  5:09     ` Horng-Shyang Liao [this message]
2016-06-08  5:09       ` Horng-Shyang Liao
2016-06-08  5:09       ` Horng-Shyang Liao
2016-06-20 10:41   ` CK Hu
2016-06-20 10:41     ` CK Hu
2016-06-20 10:41     ` CK Hu
2016-06-20 11:22     ` Horng-Shyang Liao
2016-06-20 11:22       ` Horng-Shyang Liao
2016-06-20 11:22       ` Horng-Shyang Liao
2016-06-21  2:03       ` CK Hu
2016-06-21  2:03         ` CK Hu
2016-06-21  2:03         ` CK Hu
2016-06-21  7:46         ` Horng-Shyang Liao
2016-06-21  7:46           ` Horng-Shyang Liao
2016-06-21  7:46           ` Horng-Shyang Liao
2016-06-24 11:39           ` Horng-Shyang Liao
2016-06-24 11:39             ` Horng-Shyang Liao
2016-06-24 11:39             ` Horng-Shyang Liao
2016-06-27  2:00             ` CK Hu
2016-06-27  2:00               ` CK Hu
2016-06-27  2:00               ` CK Hu
2016-06-23  6:03   ` CK Hu
2016-06-23  6:03     ` CK Hu
2016-06-23  6:03     ` CK Hu
2016-06-23  7:54     ` Horng-Shyang Liao
2016-06-23  7:54       ` Horng-Shyang Liao
2016-06-23  7:54       ` Horng-Shyang Liao
2016-06-23 11:44       ` CK Hu
2016-06-23 11:44         ` CK Hu
2016-06-23 11:44         ` CK Hu
2016-05-30  3:19 ` [PATCH v8 3/3] arm64: dts: mt8173: Add GCE node HS Liao
2016-05-30  3:19   ` HS Liao
2016-05-30  3:19   ` HS Liao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1465362579.9963.2.camel@mtksdaap41 \
    --to=hs.liao@mediatek.com \
    --cc=bibby.hsieh@mediatek.com \
    --cc=cawa.cheng@mediatek.com \
    --cc=ck.hu@mediatek.com \
    --cc=damon.chu@mediatek.com \
    --cc=daoyuan.huang@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=djkurtz@chromium.org \
    --cc=drinkcat@chromium.org \
    --cc=glory.hung@mediatek.com \
    --cc=jiaguang.zhang@mediatek.com \
    --cc=josh-yc.liu@mediatek.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=monica.wang@mediatek.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=srv_heupstream@mediatek.com \
    --cc=yt.shen@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.