From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: "Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>,
"jassisinghbrar@gmail.com" <jassisinghbrar@gmail.com>,
"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
"chunkuang.hu@kernel.org" <chunkuang.hu@kernel.org>
Cc: "linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"Singo Chang (張興國)" <Singo.Chang@mediatek.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Jason-ch Chen (陳建豪)" <Jason-ch.Chen@mediatek.com>,
"Shawn Sung (宋孝謙)" <Shawn.Sung@mediatek.com>,
"Nancy Lin (林欣螢)" <Nancy.Lin@mediatek.com>,
Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@mediatek.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 5/5] mailbox: mtk-cmdq: Add support runtime get and set GCE event
Date: Tue, 5 Mar 2024 11:20:55 +0100 [thread overview]
Message-ID: <e9cc42d7-db7f-4bd8-978e-72b97cfa8d41@collabora.com> (raw)
In-Reply-To: <35b6915dd195abba009dab64dc6002362292351c.camel@mediatek.com>
Il 05/03/24 04:09, Jason-JH Lin (林睿祥) ha scritto:
> Hi Angelo,
>
> Thanks for the reviews.
>
> On Mon, 2024-03-04 at 11:06 +0100, AngeloGioacchino Del Regno wrote:
>> Il 01/03/24 12:11, Jason-JH.Lin ha scritto:
>>> ISP drivers need to get and set GCE event in their runtime contorl
>>> flow.
>>> So add these functions to support get and set GCE by CPU.
>>>
>>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
>>> Change-Id: I494c34ebc5ec26c82213f2bc03d2033d60652523
>>
>> Change-Id makes no sense upstream. Please drop.
>
> OK, I'll drop it.
>
>>
>>> ---
>>> drivers/mailbox/mtk-cmdq-mailbox.c | 37
>>> ++++++++++++++++++++++++
>>> include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
>>> 2 files changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> index ead2200f39ba..d7c08249c898 100644
>>> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> @@ -25,7 +25,11 @@
>>> #define CMDQ_GCE_NUM_MAX (2)
>>>
>>> #define CMDQ_CURR_IRQ_STATUS 0x10
>>> +#define CMDQ_SYNC_TOKEN_ID 0x60
>>> +#define CMDQ_SYNC_TOKEN_VALUE 0x64
>>> +#define CMDQ_TOKEN_ID_MASK GENMASK(9, 0)
>>> #define CMDQ_SYNC_TOKEN_UPDATE 0x68
>>> +#define CMDQ_TOKEN_UPDATE_VALUE BIT(16)
>>> #define CMDQ_THR_SLOT_CYCLES 0x30
>>> #define CMDQ_THR_BASE 0x100
>>> #define CMDQ_THR_SIZE 0x80
>>> @@ -83,6 +87,7 @@ struct cmdq {
>>> struct cmdq_thread *thread;
>>> struct clk_bulk_data clocks[CMDQ_GCE_NUM_MAX];
>>> bool suspended;
>>> + spinlock_t event_lock; /* lock for gce event */
>>> };
>>>
>>> struct gce_plat {
>>> @@ -113,6 +118,38 @@ u8 cmdq_get_shift_pa(struct mbox_chan *chan)
>>> }
>>> EXPORT_SYMBOL(cmdq_get_shift_pa);
>>>
>>> +void cmdq_set_event(void *chan, u16 event_id)
>>> +{
>>> + struct cmdq *cmdq = container_of(((struct mbox_chan *)chan)-
>>>> mbox,
>>> + typeof(*cmdq), mbox);
>>
>> struct mbox_chan *mbc = chan;
>> struct cmdq *cmdq = container_of(mbc->mbox, ... etc); (and this fits
>> in one line)
>>
> OK, I'll change it.
>
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&cmdq->event_lock, flags);
>>
>> Why do you need irqsave/irqrestore? I think I know, but please
>> explain.
>>
> Because ISP driver may call cmdq_get_event() first than use
> cmdq_set_event() to update the event status in one
> mtk_imgsys_setevent() function frequently.
>
> And mtk_imgsys_setevent() will be called in SW multi-thread after cmdq
> callback from cmdq_irq_handler, so we use the spin_lock_irqsave to
> avoid the race condition.
I was imagining something like that, yes - thank you for explaining.
Cheers,
Angelo
WARNING: multiple messages have this Message-ID (diff)
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: "Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>,
"jassisinghbrar@gmail.com" <jassisinghbrar@gmail.com>,
"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
"chunkuang.hu@kernel.org" <chunkuang.hu@kernel.org>
Cc: "linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"Singo Chang (張興國)" <Singo.Chang@mediatek.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Jason-ch Chen (陳建豪)" <Jason-ch.Chen@mediatek.com>,
"Shawn Sung (宋孝謙)" <Shawn.Sung@mediatek.com>,
"Nancy Lin (林欣螢)" <Nancy.Lin@mediatek.com>,
Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@mediatek.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 5/5] mailbox: mtk-cmdq: Add support runtime get and set GCE event
Date: Tue, 5 Mar 2024 11:20:55 +0100 [thread overview]
Message-ID: <e9cc42d7-db7f-4bd8-978e-72b97cfa8d41@collabora.com> (raw)
In-Reply-To: <35b6915dd195abba009dab64dc6002362292351c.camel@mediatek.com>
Il 05/03/24 04:09, Jason-JH Lin (林睿祥) ha scritto:
> Hi Angelo,
>
> Thanks for the reviews.
>
> On Mon, 2024-03-04 at 11:06 +0100, AngeloGioacchino Del Regno wrote:
>> Il 01/03/24 12:11, Jason-JH.Lin ha scritto:
>>> ISP drivers need to get and set GCE event in their runtime contorl
>>> flow.
>>> So add these functions to support get and set GCE by CPU.
>>>
>>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
>>> Change-Id: I494c34ebc5ec26c82213f2bc03d2033d60652523
>>
>> Change-Id makes no sense upstream. Please drop.
>
> OK, I'll drop it.
>
>>
>>> ---
>>> drivers/mailbox/mtk-cmdq-mailbox.c | 37
>>> ++++++++++++++++++++++++
>>> include/linux/mailbox/mtk-cmdq-mailbox.h | 2 ++
>>> 2 files changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> index ead2200f39ba..d7c08249c898 100644
>>> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> @@ -25,7 +25,11 @@
>>> #define CMDQ_GCE_NUM_MAX (2)
>>>
>>> #define CMDQ_CURR_IRQ_STATUS 0x10
>>> +#define CMDQ_SYNC_TOKEN_ID 0x60
>>> +#define CMDQ_SYNC_TOKEN_VALUE 0x64
>>> +#define CMDQ_TOKEN_ID_MASK GENMASK(9, 0)
>>> #define CMDQ_SYNC_TOKEN_UPDATE 0x68
>>> +#define CMDQ_TOKEN_UPDATE_VALUE BIT(16)
>>> #define CMDQ_THR_SLOT_CYCLES 0x30
>>> #define CMDQ_THR_BASE 0x100
>>> #define CMDQ_THR_SIZE 0x80
>>> @@ -83,6 +87,7 @@ struct cmdq {
>>> struct cmdq_thread *thread;
>>> struct clk_bulk_data clocks[CMDQ_GCE_NUM_MAX];
>>> bool suspended;
>>> + spinlock_t event_lock; /* lock for gce event */
>>> };
>>>
>>> struct gce_plat {
>>> @@ -113,6 +118,38 @@ u8 cmdq_get_shift_pa(struct mbox_chan *chan)
>>> }
>>> EXPORT_SYMBOL(cmdq_get_shift_pa);
>>>
>>> +void cmdq_set_event(void *chan, u16 event_id)
>>> +{
>>> + struct cmdq *cmdq = container_of(((struct mbox_chan *)chan)-
>>>> mbox,
>>> + typeof(*cmdq), mbox);
>>
>> struct mbox_chan *mbc = chan;
>> struct cmdq *cmdq = container_of(mbc->mbox, ... etc); (and this fits
>> in one line)
>>
> OK, I'll change it.
>
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&cmdq->event_lock, flags);
>>
>> Why do you need irqsave/irqrestore? I think I know, but please
>> explain.
>>
> Because ISP driver may call cmdq_get_event() first than use
> cmdq_set_event() to update the event status in one
> mtk_imgsys_setevent() function frequently.
>
> And mtk_imgsys_setevent() will be called in SW multi-thread after cmdq
> callback from cmdq_irq_handler, so we use the spin_lock_irqsave to
> avoid the race condition.
I was imagining something like that, yes - thank you for explaining.
Cheers,
Angelo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-03-05 10:20 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-01 11:11 [PATCH 0/5] Add CMDQ API for upcoming ISP feature Jason-JH.Lin
2024-03-01 11:11 ` Jason-JH.Lin
2024-03-01 11:11 ` [PATCH 1/5] soc: mediatek: mtk-cmdq: Add specific purpose register definitions for GCE Jason-JH.Lin
2024-03-01 11:11 ` Jason-JH.Lin
2024-03-01 11:11 ` [PATCH 2/5] soc: mediatek: mtk-cmdq: Add cmdq_pkt_mem_move() function Jason-JH.Lin
2024-03-01 11:11 ` Jason-JH.Lin
2024-03-04 10:05 ` AngeloGioacchino Del Regno
2024-03-04 10:05 ` AngeloGioacchino Del Regno
2024-03-05 1:39 ` Jason-JH Lin (林睿祥)
2024-03-05 1:39 ` Jason-JH Lin (林睿祥)
2024-03-01 11:11 ` [PATCH 3/5] soc: mediatek: mtk-cmdq: Add cmdq_pkt_poll_addr() function Jason-JH.Lin
2024-03-01 11:11 ` Jason-JH.Lin
2024-03-04 10:05 ` AngeloGioacchino Del Regno
2024-03-04 10:05 ` AngeloGioacchino Del Regno
2024-03-05 1:32 ` Jason-JH Lin (林睿祥)
2024-03-05 1:32 ` Jason-JH Lin (林睿祥)
2024-03-01 11:11 ` [PATCH 4/5] soc: mediatek: mtk-cmdq: Add cmdq_pkt_acquire_event() function Jason-JH.Lin
2024-03-01 11:11 ` Jason-JH.Lin
2024-03-04 10:05 ` AngeloGioacchino Del Regno
2024-03-04 10:05 ` AngeloGioacchino Del Regno
2024-03-05 1:41 ` Jason-JH Lin (林睿祥)
2024-03-05 1:41 ` Jason-JH Lin (林睿祥)
2024-03-01 11:11 ` [PATCH 5/5] mailbox: mtk-cmdq: Add support runtime get and set GCE event Jason-JH.Lin
2024-03-01 11:11 ` Jason-JH.Lin
2024-03-04 10:06 ` AngeloGioacchino Del Regno
2024-03-04 10:06 ` AngeloGioacchino Del Regno
2024-03-05 3:09 ` Jason-JH Lin (林睿祥)
2024-03-05 3:09 ` Jason-JH Lin (林睿祥)
2024-03-05 10:20 ` AngeloGioacchino Del Regno [this message]
2024-03-05 10:20 ` AngeloGioacchino Del Regno
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=e9cc42d7-db7f-4bd8-978e-72b97cfa8d41@collabora.com \
--to=angelogioacchino.delregno@collabora.com \
--cc=Jason-JH.Lin@mediatek.com \
--cc=Jason-ch.Chen@mediatek.com \
--cc=Nancy.Lin@mediatek.com \
--cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
--cc=Shawn.Sung@mediatek.com \
--cc=Singo.Chang@mediatek.com \
--cc=chunkuang.hu@kernel.org \
--cc=jassisinghbrar@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.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.