This patch support gce on mt6779 platform.
Change since v4:
- do not clear disp event again in drm driver
- symbolize value 1 to jump relative
Change since v3:
- refine code for local variable usage
- use cmdq error code to consistent with current design
- return error directly after send if error code return
- also modify drm driver which uses cmdq_pkt_wfe api
- add finalize in drm driver
[... snip ...]
Dennis YC Hsieh (13):
dt-binding: gce: add gce header file for mt6779
mailbox: cmdq: variablize address shift in platform
mailbox: cmdq: support mt6779 gce platform definition
mailbox: mediatek: cmdq: clear task in channel before shutdown
soc: mediatek: cmdq: return send msg error code
soc: mediatek: cmdq: add assign function
soc: mediatek: cmdq: add write_s function
soc: mediatek: cmdq: add read_s function
soc: mediatek: cmdq: add write_s value function
soc: mediatek: cmdq: export finalize function
soc: mediatek: cmdq: add jump function
soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api
soc: mediatek: cmdq: add set event function
.../devicetree/bindings/mailbox/mtk-gce.txt | 8 +-
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +-
drivers/mailbox/mtk-cmdq-mailbox.c | 101 ++++++--
drivers/soc/mediatek/mtk-cmdq-helper.c | 144 +++++++++++-
include/dt-bindings/gce/mt6779-gce.h | 222 ++++++++++++++++++
include/linux/mailbox/mtk-cmdq-mailbox.h | 10 +-
include/linux/soc/mediatek/mtk-cmdq.h | 94 +++++++-
7 files changed, 549 insertions(+), 33 deletions(-)
create mode 100644 include/dt-bindings/gce/mt6779-gce.h
--
2.18.0
Add set event function in cmdq helper functions to set specific event.
Signed-off-by: Dennis YC Hsieh <[email protected]>
Reviewed-by: CK Hu <[email protected]>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
include/linux/soc/mediatek/mtk-cmdq.h | 9 +++++++++
3 files changed, 25 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index ec5637d43254..3294c9285994 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
}
EXPORT_SYMBOL(cmdq_pkt_clear_event);
+int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
+{
+ struct cmdq_instruction inst = { {0} };
+
+ if (event >= CMDQ_MAX_EVENT)
+ return -EINVAL;
+
+ inst.op = CMDQ_CODE_WFE;
+ inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
+ inst.event = event;
+
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_set_event);
+
int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value)
{
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 42d2a30e6a70..ba2d811183a9 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -17,6 +17,7 @@
#define CMDQ_JUMP_PASS CMDQ_INST_SIZE
#define CMDQ_WFE_UPDATE BIT(31)
+#define CMDQ_WFE_UPDATE_VALUE BIT(16)
#define CMDQ_WFE_WAIT BIT(15)
#define CMDQ_WFE_WAIT_VALUE 0x1
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index d63749440697..ca70296ae120 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
*/
int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
+/**
+ * cmdq_pkt_set_event() - append set event command to the CMDQ packet
+ * @pkt: the CMDQ packet
+ * @event: the desired event to be set
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event);
+
/**
* cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to
* execute an instruction that wait for a specified
--
2.18.0
Export finalize function to client which helps append eoc and jump
command to pkt. Let client decide call finalize or not.
Signed-off-by: Dennis YC Hsieh <[email protected]>
Reviewed-by: CK Hu <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-----
include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 0dfcd1787e65..7daaabc26eb1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
mtk_crtc_ddp_config(crtc, cmdq_handle);
+ cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
}
#endif
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index a9ebbabb7439..59bc1164b411 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
}
EXPORT_SYMBOL(cmdq_pkt_assign);
-static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
+int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
{
struct cmdq_instruction inst = { {0} };
int err;
@@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
return err;
}
+EXPORT_SYMBOL(cmdq_pkt_finalize);
static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
{
@@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
unsigned long flags = 0;
struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
- err = cmdq_pkt_finalize(pkt);
- if (err < 0)
- return err;
-
pkt->cb.cb = cb;
pkt->cb.data = data;
pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index fec292aac83c..99e77155f967 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
*/
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
+/**
+ * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
+ * @pkt: the CMDQ packet
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
+
/**
* cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
* packet and call back at the end of done packet
--
2.18.0
On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> Export finalize function to client which helps append eoc and jump
> command to pkt. Let client decide call finalize or not.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> Reviewed-by: CK Hu <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
> drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-----
> include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++
> 3 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 0dfcd1787e65..7daaabc26eb1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> mtk_crtc_ddp_config(crtc, cmdq_handle);
> + cmdq_pkt_finalize(cmdq_handle);
> cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> }
> #endif
This should be a independent patch.
Other then that patch looks good.
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index a9ebbabb7439..59bc1164b411 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> }
> EXPORT_SYMBOL(cmdq_pkt_assign);
>
> -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> {
> struct cmdq_instruction inst = { {0} };
> int err;
> @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>
> return err;
> }
> +EXPORT_SYMBOL(cmdq_pkt_finalize);
>
> static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
> {
> @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
> unsigned long flags = 0;
> struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
>
> - err = cmdq_pkt_finalize(pkt);
> - if (err < 0)
> - return err;
> -
> pkt->cb.cb = cb;
> pkt->cb.data = data;
> pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index fec292aac83c..99e77155f967 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> */
> int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
>
> +/**
> + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
> + * @pkt: the CMDQ packet
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
> +
> /**
> * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
> * packet and call back at the end of done packet
>
On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> Add set event function in cmdq helper functions to set specific event.
>
> Signed-off-by: Dennis YC Hsieh <[email protected]>
> Reviewed-by: CK Hu <[email protected]>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> include/linux/soc/mediatek/mtk-cmdq.h | 9 +++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index ec5637d43254..3294c9285994 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
> }
> EXPORT_SYMBOL(cmdq_pkt_clear_event);
>
> +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
> +{
> + struct cmdq_instruction inst = { {0} };
> +
> + if (event >= CMDQ_MAX_EVENT)
> + return -EINVAL;
> +
> + inst.op = CMDQ_CODE_WFE;
> + inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
> + inst.event = event;
> +
> + return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_set_event);
> +
> int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
> u16 offset, u32 value)
> {
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 42d2a30e6a70..ba2d811183a9 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -17,6 +17,7 @@
> #define CMDQ_JUMP_PASS CMDQ_INST_SIZE
>
> #define CMDQ_WFE_UPDATE BIT(31)
> +#define CMDQ_WFE_UPDATE_VALUE BIT(16)
> #define CMDQ_WFE_WAIT BIT(15)
> #define CMDQ_WFE_WAIT_VALUE 0x1
>
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index d63749440697..ca70296ae120 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
> */
> int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
>
> +/**
> + * cmdq_pkt_set_event() - append set event command to the CMDQ packet
> + * @pkt: the CMDQ packet
> + * @event: the desired event to be set
Can we add the events and their code, so that later on, when a consumer calls
cmdq_pkt_set_event() we don't have any magic values that are hard to understand?
Regards,
Matthias
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event);
> +
> /**
> * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to
> * execute an instruction that wait for a specified
>
Hi Matthias,
Thanks for your comment.
On Sat, 2020-05-16 at 20:22 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > Export finalize function to client which helps append eoc and jump
> > command to pkt. Let client decide call finalize or not.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > Reviewed-by: CK Hu <[email protected]>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-----
> > include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++
> > 3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 0dfcd1787e65..7daaabc26eb1 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> > mtk_crtc_ddp_config(crtc, cmdq_handle);
> > + cmdq_pkt_finalize(cmdq_handle);
> > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> > }
> > #endif
>
> This should be a independent patch.
> Other then that patch looks good.
ok, I'll separate this part.
Regards,
Dennis
>
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index a9ebbabb7439..59bc1164b411 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> > }
> > EXPORT_SYMBOL(cmdq_pkt_assign);
> >
> > -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > {
> > struct cmdq_instruction inst = { {0} };
> > int err;
> > @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> >
> > return err;
> > }
> > +EXPORT_SYMBOL(cmdq_pkt_finalize);
> >
> > static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
> > {
> > @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
> > unsigned long flags = 0;
> > struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
> >
> > - err = cmdq_pkt_finalize(pkt);
> > - if (err < 0)
> > - return err;
> > -
> > pkt->cb.cb = cb;
> > pkt->cb.data = data;
> > pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index fec292aac83c..99e77155f967 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> > */
> > int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
> >
> > +/**
> > + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
> > + * @pkt: the CMDQ packet
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
> > +
> > /**
> > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
> > * packet and call back at the end of done packet
> >
Hi Matthias,
Thanks for your comment.
On Sat, 2020-05-16 at 20:32 +0200, Matthias Brugger wrote:
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > Add set event function in cmdq helper functions to set specific event.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > Reviewed-by: CK Hu <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> > include/linux/soc/mediatek/mtk-cmdq.h | 9 +++++++++
> > 3 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index ec5637d43254..3294c9285994 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
> > }
> > EXPORT_SYMBOL(cmdq_pkt_clear_event);
> >
> > +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > +
> > + if (event >= CMDQ_MAX_EVENT)
> > + return -EINVAL;
> > +
> > + inst.op = CMDQ_CODE_WFE;
> > + inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
> > + inst.event = event;
> > +
> > + return cmdq_pkt_append_command(pkt, inst);
> > +}
> > +EXPORT_SYMBOL(cmdq_pkt_set_event);
> > +
> > int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
> > u16 offset, u32 value)
> > {
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 42d2a30e6a70..ba2d811183a9 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -17,6 +17,7 @@
> > #define CMDQ_JUMP_PASS CMDQ_INST_SIZE
> >
> > #define CMDQ_WFE_UPDATE BIT(31)
> > +#define CMDQ_WFE_UPDATE_VALUE BIT(16)
> > #define CMDQ_WFE_WAIT BIT(15)
> > #define CMDQ_WFE_WAIT_VALUE 0x1
> >
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index d63749440697..ca70296ae120 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
> > */
> > int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
> >
> > +/**
> > + * cmdq_pkt_set_event() - append set event command to the CMDQ packet
> > + * @pkt: the CMDQ packet
> > + * @event: the desired event to be set
>
> Can we add the events and their code, so that later on, when a consumer calls
> cmdq_pkt_set_event() we don't have any magic values that are hard to understand?
Please see patch 02/13:
http://lists.infradead.org/pipermail/linux-mediatek/2020-March/027801.html
Definitions begin with CMDQ_EVENT_ is the event id to this function.
Since the event id is different between platform, client must parse it
from device tree. So no magic values require when call this function.
Regard,
Dennis
>
> Regards,
> Matthias
>
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event);
> > +
> > /**
> > * cmdq_pkt_poll() - Append polling command to the CMDQ packet, ask GCE to
> > * execute an instruction that wait for a specified
> >
On 24/05/2020 19:39, Dennis-YC Hsieh wrote:
> Hi Matthias,
>
> Thanks for your comment.
>
>
> On Sat, 2020-05-16 at 20:32 +0200, Matthias Brugger wrote:
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> Add set event function in cmdq helper functions to set specific event.
>>>
>>> Signed-off-by: Dennis YC Hsieh <[email protected]>
>>> Reviewed-by: CK Hu <[email protected]>
>>> ---
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++++
>>> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
>>> include/linux/soc/mediatek/mtk-cmdq.h | 9 +++++++++
>>> 3 files changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index ec5637d43254..3294c9285994 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -327,6 +327,21 @@ int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event)
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_clear_event);
>>>
>>> +int cmdq_pkt_set_event(struct cmdq_pkt *pkt, u16 event)
>>> +{
>>> + struct cmdq_instruction inst = { {0} };
>>> +
>>> + if (event >= CMDQ_MAX_EVENT)
>>> + return -EINVAL;
>>> +
>>> + inst.op = CMDQ_CODE_WFE;
>>> + inst.value = CMDQ_WFE_UPDATE | CMDQ_WFE_UPDATE_VALUE;
>>> + inst.event = event;
>>> +
>>> + return cmdq_pkt_append_command(pkt, inst);
>>> +}
>>> +EXPORT_SYMBOL(cmdq_pkt_set_event);
>>> +
>>> int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>>> u16 offset, u32 value)
>>> {
>>> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> index 42d2a30e6a70..ba2d811183a9 100644
>>> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
>>> @@ -17,6 +17,7 @@
>>> #define CMDQ_JUMP_PASS CMDQ_INST_SIZE
>>>
>>> #define CMDQ_WFE_UPDATE BIT(31)
>>> +#define CMDQ_WFE_UPDATE_VALUE BIT(16)
>>> #define CMDQ_WFE_WAIT BIT(15)
>>> #define CMDQ_WFE_WAIT_VALUE 0x1
>>>
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index d63749440697..ca70296ae120 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -168,6 +168,15 @@ int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
>>> */
>>> int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event);
>>>
>>> +/**
>>> + * cmdq_pkt_set_event() - append set event command to the CMDQ packet
>>> + * @pkt: the CMDQ packet
>>> + * @event: the desired event to be set
>>
>> Can we add the events and their code, so that later on, when a consumer calls
>> cmdq_pkt_set_event() we don't have any magic values that are hard to understand?
>
> Please see patch 02/13:
> http://lists.infradead.org/pipermail/linux-mediatek/2020-March/027801.html
>
> Definitions begin with CMDQ_EVENT_ is the event id to this function.
> Since the event id is different between platform, client must parse it
> from device tree. So no magic values require when call this function.
>
>
Got it, thanks for clarification.
Matthias
Hi, Matthias:
Matthias Brugger <[email protected]> 於 2020年5月17日 週日 上午2:22寫道：
>
>
>
> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> > Export finalize function to client which helps append eoc and jump
> > command to pkt. Let client decide call finalize or not.
> >
> > Signed-off-by: Dennis YC Hsieh <[email protected]>
> > Reviewed-by: CK Hu <[email protected]>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-----
> > include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++
> > 3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 0dfcd1787e65..7daaabc26eb1 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> > cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> > mtk_crtc_ddp_config(crtc, cmdq_handle);
> > + cmdq_pkt_finalize(cmdq_handle);
> > cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> > }
> > #endif
>
> This should be a independent patch.
> Other then that patch looks good.
Apply only drm part or only cmdq helpr part, it would be abnormal.
Shall we seperate this patch?
Or seperate it but make sure these two patches be in the same tree?
Regards,
Chun-Kuang.
>
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index a9ebbabb7439..59bc1164b411 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> > }
> > EXPORT_SYMBOL(cmdq_pkt_assign);
> >
> > -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> > {
> > struct cmdq_instruction inst = { {0} };
> > int err;
> > @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> >
> > return err;
> > }
> > +EXPORT_SYMBOL(cmdq_pkt_finalize);
> >
> > static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
> > {
> > @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
> > unsigned long flags = 0;
> > struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
> >
> > - err = cmdq_pkt_finalize(pkt);
> > - if (err < 0)
> > - return err;
> > -
> > pkt->cb.cb = cb;
> > pkt->cb.data = data;
> > pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> > index fec292aac83c..99e77155f967 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> > */
> > int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
> >
> > +/**
> > + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
> > + * @pkt: the CMDQ packet
> > + *
> > + * Return: 0 for success; else the error code is returned
> > + */
> > +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
> > +
> > /**
> > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
> > * packet and call back at the end of done packet
> >
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 25/05/2020 02:23, Chun-Kuang Hu wrote:
> Hi, Matthias:
>
> Matthias Brugger <[email protected]> 於 2020年5月17日 週日 上午2:22寫道：
>>
>>
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> Export finalize function to client which helps append eoc and jump
>>> command to pkt. Let client decide call finalize or not.
>>>
>>> Signed-off-by: Dennis YC Hsieh <[email protected]>
>>> Reviewed-by: CK Hu <[email protected]>
>>> ---
>>> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
>>> drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-----
>>> include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++
>>> 3 files changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>>> index 0dfcd1787e65..7daaabc26eb1 100644
>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>>> @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
>>> cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
>>> cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
>>> mtk_crtc_ddp_config(crtc, cmdq_handle);
>>> + cmdq_pkt_finalize(cmdq_handle);
>>> cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
>>> }
>>> #endif
>>
>> This should be a independent patch.
>> Other then that patch looks good.
>
> Apply only drm part or only cmdq helpr part, it would be abnormal.
Right it would break DRM driver (if only applied to cmdq) or compilation if only
applied to DRM.
> Shall we seperate this patch?
After thinking twice, I think we can leave it as it is. If you provide your
Acked-by I can take it thorugh my tree, if that's OK for you.
Regards,
Matthias
> Or seperate it but make sure these two patches be in the same tree?
>
> Regards,
> Chun-Kuang.
>
>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index a9ebbabb7439..59bc1164b411 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
>>> }
>>> EXPORT_SYMBOL(cmdq_pkt_assign);
>>>
>>> -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>>> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>>> {
>>> struct cmdq_instruction inst = { {0} };
>>> int err;
>>> @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>>>
>>> return err;
>>> }
>>> +EXPORT_SYMBOL(cmdq_pkt_finalize);
>>>
>>> static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
>>> {
>>> @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
>>> unsigned long flags = 0;
>>> struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
>>>
>>> - err = cmdq_pkt_finalize(pkt);
>>> - if (err < 0)
>>> - return err;
>>> -
>>> pkt->cb.cb = cb;
>>> pkt->cb.data = data;
>>> pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index fec292aac83c..99e77155f967 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>>> */
>>> int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
>>>
>>> +/**
>>> + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
>>> + * @pkt: the CMDQ packet
>>> + *
>>> + * Return: 0 for success; else the error code is returned
>>> + */
>>> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
>>> +
>>> /**
>>> * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
>>> * packet and call back at the end of done packet
>>>
>> _______________________________________________
>> dri-devel mailing list
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi, Matthias:
Matthias Brugger <[email protected]> 於 2020年5月25日 週一 下午4:38寫道：
>
>
>
> On 25/05/2020 02:23, Chun-Kuang Hu wrote:
> > Hi, Matthias:
> >
> > Matthias Brugger <[email protected]> 於 2020年5月17日 週日 上午2:22寫道：
> >>
> >>
> >>
> >> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
> >>> Export finalize function to client which helps append eoc and jump
> >>> command to pkt. Let client decide call finalize or not.
> >>>
> >>> Signed-off-by: Dennis YC Hsieh <[email protected]>
> >>> Reviewed-by: CK Hu <[email protected]>
> >>> ---
> >>> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
> >>> drivers/soc/mediatek/mtk-cmdq-helper.c | 7 ++-----
> >>> include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++
> >>> 3 files changed, 11 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> >>> index 0dfcd1787e65..7daaabc26eb1 100644
> >>> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> >>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> >>> @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> >>> cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> >>> cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
> >>> mtk_crtc_ddp_config(crtc, cmdq_handle);
> >>> + cmdq_pkt_finalize(cmdq_handle);
> >>> cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
> >>> }
> >>> #endif
> >>
> >> This should be a independent patch.
> >> Other then that patch looks good.
> >
> > Apply only drm part or only cmdq helpr part, it would be abnormal.
>
> Right it would break DRM driver (if only applied to cmdq) or compilation if only
> applied to DRM.
>
> > Shall we seperate this patch?
>
> After thinking twice, I think we can leave it as it is. If you provide your
> Acked-by I can take it thorugh my tree, if that's OK for you.
This is OK for me, so
Acked-by: Chun-Kuang Hu <[email protected]>
>
> Regards,
> Matthias
>
> > Or seperate it but make sure these two patches be in the same tree?
> >
> > Regards,
> > Chun-Kuang.
> >
> >>
> >>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>> index a9ebbabb7439..59bc1164b411 100644
> >>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> >>> @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> >>> }
> >>> EXPORT_SYMBOL(cmdq_pkt_assign);
> >>>
> >>> -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> >>> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> >>> {
> >>> struct cmdq_instruction inst = { {0} };
> >>> int err;
> >>> @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
> >>>
> >>> return err;
> >>> }
> >>> +EXPORT_SYMBOL(cmdq_pkt_finalize);
> >>>
> >>> static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
> >>> {
> >>> @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
> >>> unsigned long flags = 0;
> >>> struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
> >>>
> >>> - err = cmdq_pkt_finalize(pkt);
> >>> - if (err < 0)
> >>> - return err;
> >>> -
> >>> pkt->cb.cb = cb;
> >>> pkt->cb.data = data;
> >>> pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
> >>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> >>> index fec292aac83c..99e77155f967 100644
> >>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> >>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> >>> @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> >>> */
> >>> int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
> >>>
> >>> +/**
> >>> + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
> >>> + * @pkt: the CMDQ packet
> >>> + *
> >>> + * Return: 0 for success; else the error code is returned
> >>> + */
> >>> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
> >>> +
> >>> /**
> >>> * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
> >>> * packet and call back at the end of done packet
> >>>
> >> _______________________________________________
> >> dri-devel mailing list
> >> [email protected]
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel