* Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ
[not found] <72f7feabe22064365457b5b72dfb1972ba2b3151.camel@mediatek.com>
@ 2022-03-18 8:38 ` Miles Chen
0 siblings, 0 replies; 6+ messages in thread
From: Miles Chen @ 2022-03-18 8:38 UTC (permalink / raw)
To: moudy.ho
Cc: Project_Global_Chrome_Upstream_Group, acourbot, allen-kh.cheng,
angelogioacchino.delregno, chunkuang.hu, daoyuan.huang,
devicetree, drinkcat, geert+renesas, hsinyi, hverkuil-cisco,
jason-jh.lin, jernej.skrabec, laurent.pinchart, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, maoguang.meng,
matthias.bgg, mchehab, menghui.lin, miles.chen, pihsun,
ping-hsun.wu, randy.wu, river.cheng, rob, robh+dt, roy-cw.yeh,
sj.huang, srv_heupstream, tfiga
Hi Moudy,
>> > }
>> > }
>> >
>> > + if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
>> > + mtx->addr = 0L;
>>
>> addr is phys_addr_t, which means that it holds a physical address.
>> 0 may be a valid address in some platforms, so I think
>> we should not set mtx->addr = 0L when of_address_to_resource fails.
>> Maybe probe fail by returning dev_err_probe()?
>>
>> Thanks,
>> Miles
>
>Hi Miles,
>
>This does lead to the situation you mentioned, and sorry for the
>deficient dts parsing flow. Maybe we should follow "gce-client" usage
>and add a check condition when needed to avoid compatibility issues?
>
>Thanks,
>Moudy
According to Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml,
it is required to have a reg field in the mutex node. So I think we can
fail in this case.
Thanks,
Miles
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ
2022-04-11 7:24 ` [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ Moudy Ho
@ 2022-04-13 8:41 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 6+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-04-13 8:41 UTC (permalink / raw)
To: Moudy Ho, Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Hans Verkuil, Jernej Skrabec
Cc: Chun-Kuang Hu, Geert Uytterhoeven, Rob Landley, Laurent Pinchart,
linux-media, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, Alexandre Courbot, tfiga, drinkcat, pihsun, hsinyi,
Benjamin Gaignard, daoyuan huang, Ping-Hsun Wu, allen-kh.cheng,
xiandong.wang, randy.wu, jason-jh.lin, roy-cw.yeh, river.cheng,
srv_heupstream, Project_Global_Chrome_Upstream_Group
Il 11/04/22 09:24, Moudy Ho ha scritto:
> Due to HW limitations, MDP3 is necessary to enable MUTEX in each frame
> for SOF triggering and cooperate with CMDQ control to reduce the amount
> of interrupts generated(also, reduce frame latency).
>
> In response to the above situation, a new interface
> "mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.
>
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
> drivers/soc/mediatek/mtk-mutex.c | 42 +++++++++++++++++++++++++-
> include/linux/soc/mediatek/mtk-mutex.h | 2 ++
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
> index fc9ba2749946..1811beaf399d 100644
> --- a/drivers/soc/mediatek/mtk-mutex.c
> +++ b/drivers/soc/mediatek/mtk-mutex.c
> @@ -7,10 +7,12 @@
> #include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> #include <linux/soc/mediatek/mtk-mmsys.h>
> #include <linux/soc/mediatek/mtk-mutex.h>
> +#include <linux/soc/mediatek/mtk-cmdq.h>
>
> #define MT2701_MUTEX0_MOD0 0x2c
> #define MT2701_MUTEX0_SOF0 0x30
> @@ -176,6 +178,9 @@ struct mtk_mutex_ctx {
> void __iomem *regs;
> struct mtk_mutex mutex[10];
> const struct mtk_mutex_data *data;
> + phys_addr_t addr;
> + struct cmdq_client_reg cmdq_reg;
> + bool has_gce_client_reg;
> };
>
> static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> @@ -618,6 +623,28 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
> }
> EXPORT_SYMBOL_GPL(mtk_mutex_enable);
>
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> +{
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> + struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
> + mutex[mutex->id]);
> + struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> +
> + WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> + if (!mtx->has_gce_client_reg) {
> + dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
> + return;
Is this really supposed to happen?
Reiterating: when the gce-client-reg is not set, this function should either never
be called from the principle, or it should actually fail.
If a driver relies on mtk_mutex_enable_by_cmdq() and does *not* fall back to
register write from cpu, then no change will occur at all, leading to a random
breakage with no apparent reason.
This means that - for safety - this function should return -EINVAL when it gets
called while no gce client reg is declared.
Of course, this would also mean that the dev_dbg() should be a dev_err(), and
that efforts should be done to avoid triggering this error by adding fallbacks
in the drivers that will call this.
Another option would be to keep this function void, keep the print a dev_dbg(),
but automatically fallback to mtk_mutex_enable(), so that here:
if (!mtx->has_gce_client_reg) {
dev_dbg(mtx->dev,
"No GCE client register found: falling back to cpu write.\n");
mtk_mutex_enable(mutex);
return;
}
...you're free to choose whichever of the two options, but this has to be fixed
to remove this fragility.
> + }
> +
> + cmdq_pkt_write(cmdq_pkt, mtx->cmdq_reg.subsys,
> + mtx->addr + DISP_REG_MUTEX_EN(mutex->id), 1);
> +#else
> + dev_err(mtx->dev, "Not support for enable MUTEX by CMDQ");
...obviously, if mtk_cmdq not reachable, instead of simply letting drivers break,
you should, also here, fall back to the less performant and very suboptimal (for
the specific case of mdp3 and some others) mtk_mutex_enable().
Thanks,
Angelo
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ
2022-04-11 7:23 [PATCH v14 0/6] Add mutex support for MDP Moudy Ho
@ 2022-04-11 7:24 ` Moudy Ho
2022-04-13 8:41 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 6+ messages in thread
From: Moudy Ho @ 2022-04-11 7:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Hans Verkuil, Jernej Skrabec
Cc: Chun-Kuang Hu, Geert Uytterhoeven, Rob Landley, Laurent Pinchart,
linux-media, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, Alexandre Courbot, tfiga, drinkcat, pihsun, hsinyi,
Benjamin Gaignard, AngeloGioacchino Del Regno, daoyuan huang,
Ping-Hsun Wu, allen-kh.cheng, xiandong.wang, randy.wu, moudy.ho,
jason-jh.lin, roy-cw.yeh, river.cheng, srv_heupstream,
Project_Global_Chrome_Upstream_Group
Due to HW limitations, MDP3 is necessary to enable MUTEX in each frame
for SOF triggering and cooperate with CMDQ control to reduce the amount
of interrupts generated(also, reduce frame latency).
In response to the above situation, a new interface
"mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.
Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
drivers/soc/mediatek/mtk-mutex.c | 42 +++++++++++++++++++++++++-
include/linux/soc/mediatek/mtk-mutex.h | 2 ++
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
index fc9ba2749946..1811beaf399d 100644
--- a/drivers/soc/mediatek/mtk-mutex.c
+++ b/drivers/soc/mediatek/mtk-mutex.c
@@ -7,10 +7,12 @@
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/soc/mediatek/mtk-mmsys.h>
#include <linux/soc/mediatek/mtk-mutex.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
#define MT2701_MUTEX0_MOD0 0x2c
#define MT2701_MUTEX0_SOF0 0x30
@@ -176,6 +178,9 @@ struct mtk_mutex_ctx {
void __iomem *regs;
struct mtk_mutex mutex[10];
const struct mtk_mutex_data *data;
+ phys_addr_t addr;
+ struct cmdq_client_reg cmdq_reg;
+ bool has_gce_client_reg;
};
static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
@@ -618,6 +623,28 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
}
EXPORT_SYMBOL_GPL(mtk_mutex_enable);
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
+{
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+ struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+ mutex[mutex->id]);
+ struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
+
+ WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+ if (!mtx->has_gce_client_reg) {
+ dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
+ return;
+ }
+
+ cmdq_pkt_write(cmdq_pkt, mtx->cmdq_reg.subsys,
+ mtx->addr + DISP_REG_MUTEX_EN(mutex->id), 1);
+#else
+ dev_err(mtx->dev, "Not support for enable MUTEX by CMDQ");
+#endif
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
+
void mtk_mutex_disable(struct mtk_mutex *mutex)
{
struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
@@ -656,7 +683,7 @@ static int mtk_mutex_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct mtk_mutex_ctx *mtx;
- struct resource *regs;
+ struct resource *regs, addr;
int i;
mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
@@ -677,6 +704,19 @@ static int mtk_mutex_probe(struct platform_device *pdev)
}
}
+ if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
+ mtx->addr = 0L;
+ else
+ mtx->addr = addr.start;
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+ ret = cmdq_dev_get_client_reg(dev, &mtx->cmdq_reg, 0);
+ if (ret)
+ dev_dbg(dev, "No mediatek,gce-client-reg!\n");
+ else
+ mtx->has_gce_client_reg = true;
+#endif
+
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
mtx->regs = devm_ioremap_resource(dev, regs);
if (IS_ERR(mtx->regs)) {
diff --git a/include/linux/soc/mediatek/mtk-mutex.h b/include/linux/soc/mediatek/mtk-mutex.h
index 200f4365c950..17eea55b6809 100644
--- a/include/linux/soc/mediatek/mtk-mutex.h
+++ b/include/linux/soc/mediatek/mtk-mutex.h
@@ -33,6 +33,8 @@ void mtk_mutex_set_mod(struct mtk_mutex *mutex,
void mtk_mutex_set_sof(struct mtk_mutex *mutex,
enum mtk_mutex_table_index idx);
void mtk_mutex_enable(struct mtk_mutex *mutex);
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex,
+ void *pkt);
void mtk_mutex_disable(struct mtk_mutex *mutex);
void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
enum mtk_ddp_comp_id id);
--
2.18.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ
2022-03-17 14:39 ` [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ Moudy Ho
2022-03-17 23:31 ` Miles Chen
@ 2022-03-22 10:04 ` CK Hu
1 sibling, 0 replies; 6+ messages in thread
From: CK Hu @ 2022-03-22 10:04 UTC (permalink / raw)
To: Moudy Ho, Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Hans Verkuil, Jernej Skrabec
Cc: Chun-Kuang Hu, Geert Uytterhoeven, Rob Landley, Laurent Pinchart,
linux-media, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, Alexandre Courbot, tfiga, drinkcat, pihsun, hsinyi,
AngeloGioacchino Del Regno, Maoguang Meng, daoyuan huang,
Ping-Hsun Wu, menghui.lin, sj.huang, allen-kh.cheng, randy.wu,
jason-jh.lin, roy-cw.yeh, river.cheng, srv_heupstream,
Project_Global_Chrome_Upstream_Group
Hi, Moudy:
On Thu, 2022-03-17 at 22:39 +0800, Moudy Ho wrote:
> Due to HW limitations, MDP3 is necessary to enable MUTEX in each
> frame
> for SOF triggering and cooperate with CMDQ control to reduce the
> amount
> of interrupts generated(also, reduce frame latency).
>
> In response to the above situation, a new interface
> "mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.
>
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
> drivers/soc/mediatek/mtk-mutex.c | 45
> +++++++++++++++++++++++++-
> include/linux/soc/mediatek/mtk-mutex.h | 2 ++
> 2 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-mutex.c
> b/drivers/soc/mediatek/mtk-mutex.c
> index c72e9f6ee4cc..32c87b8ed077 100644
> --- a/drivers/soc/mediatek/mtk-mutex.c
> +++ b/drivers/soc/mediatek/mtk-mutex.c
> @@ -7,10 +7,14 @@
> #include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> #include <linux/soc/mediatek/mtk-mmsys.h>
> #include <linux/soc/mediatek/mtk-mutex.h>
> +#include <linux/soc/mediatek/mtk-cmdq.h>
> +
> +#define MTK_MUTEX_ENABLE BIT(0)
If this is necessary, separate this to another patch and apply to
mtk_mutex_enable() also.
>
> #define MT2701_MUTEX0_MOD0 0x2c
> #define MT2701_MUTEX0_SOF0 0x30
> @@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
> void __iomem *regs;
> struct mtk_mutex mutex[10];
> const struct mtk_mutex_data *data;
> + phys_addr_t addr;
> + struct cmdq_client_reg cmdq_reg;
> + bool has_gce_client_reg;
> };
>
> static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> @@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
> }
> EXPORT_SYMBOL_GPL(mtk_mutex_enable);
>
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> +{
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> + struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> + mutex[mutex->id]);
> + struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> +
> + WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> + if (!mtx->has_gce_client_reg) {
> + dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been
> set in dts");
dev_err()
> + return;
> + }
> +
> + cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
> + mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
+ MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);
According to mtk_mutex_enable(),
cmdq_pkt_write(cmdq_pkt, mtx->cmdq_reg.subsys,
mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
1);
> +#else
> + dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");
dev_err()
Regards,
CK
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
> +
> void mtk_mutex_disable(struct mtk_mutex *mutex)
> {
> struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> @@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct platform_device
> *pdev)
> {
> struct device *dev = &pdev->dev;
> struct mtk_mutex_ctx *mtx;
> - struct resource *regs;
> + struct resource *regs, addr;
> int i;
>
> mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
> @@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct
> platform_device *pdev)
> }
> }
>
> + if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
> + mtx->addr = 0L;
> + else
> + mtx->addr = addr.start;
> +
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> + ret = cmdq_dev_get_client_reg(dev, &mtx->cmdq_reg, 0);
> + if (ret)
> + dev_dbg(dev, "No mediatek,gce-client-reg!\n");
> + else
> + mtx->has_gce_client_reg = true;
> +#endif
> +
> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> mtx->regs = devm_ioremap_resource(dev, regs);
> if (IS_ERR(mtx->regs)) {
> diff --git a/include/linux/soc/mediatek/mtk-mutex.h
> b/include/linux/soc/mediatek/mtk-mutex.h
> index 200f4365c950..17eea55b6809 100644
> --- a/include/linux/soc/mediatek/mtk-mutex.h
> +++ b/include/linux/soc/mediatek/mtk-mutex.h
> @@ -33,6 +33,8 @@ void mtk_mutex_set_mod(struct mtk_mutex *mutex,
> void mtk_mutex_set_sof(struct mtk_mutex *mutex,
> enum mtk_mutex_table_index idx);
> void mtk_mutex_enable(struct mtk_mutex *mutex);
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex,
> + void *pkt);
> void mtk_mutex_disable(struct mtk_mutex *mutex);
> void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
> enum mtk_ddp_comp_id id);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ
2022-03-17 14:39 ` [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ Moudy Ho
@ 2022-03-17 23:31 ` Miles Chen
2022-03-22 10:04 ` CK Hu
1 sibling, 0 replies; 6+ messages in thread
From: Miles Chen @ 2022-03-17 23:31 UTC (permalink / raw)
To: moudy.ho
Cc: Project_Global_Chrome_Upstream_Group, acourbot, allen-kh.cheng,
angelogioacchino.delregno, chunkuang.hu, daoyuan.huang,
devicetree, drinkcat, geert+renesas, hsinyi, hverkuil-cisco,
jason-jh.lin, jernej.skrabec, laurent.pinchart, linux-arm-kernel,
linux-kernel, linux-media, linux-mediatek, maoguang.meng,
matthias.bgg, mchehab, menghui.lin, pihsun, ping-hsun.wu,
randy.wu, river.cheng, rob, robh+dt, roy-cw.yeh, sj.huang,
srv_heupstream, tfiga
Hi Moudy,
> #define MT2701_MUTEX0_MOD0 0x2c
> #define MT2701_MUTEX0_SOF0 0x30
> @@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
> void __iomem *regs;
> struct mtk_mutex mutex[10];
> const struct mtk_mutex_data *data;
> + phys_addr_t addr;
> + struct cmdq_client_reg cmdq_reg;
> + bool has_gce_client_reg;
> };
>
...snip...
> static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> @@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
> }
> EXPORT_SYMBOL_GPL(mtk_mutex_enable);
>
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> +{
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> + struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
> + mutex[mutex->id]);
> + struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> +
> + WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> + if (!mtx->has_gce_client_reg) {
> + dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
> + return;
> + }
> +
> + cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
> + mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
> + MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);
We do not check if mtx->addr is valid.
> +#else
> + dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
> +
> void mtk_mutex_disable(struct mtk_mutex *mutex)
> {
> struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
> @@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct mtk_mutex_ctx *mtx;
> - struct resource *regs;
> + struct resource *regs, addr;
> int i;
>
> mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
> @@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct platform_device *pdev)
> }
> }
>
> + if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
> + mtx->addr = 0L;
addr is phys_addr_t, which means that it holds a physical address.
0 may be a valid address in some platforms, so I think
we should not set mtx->addr = 0L when of_address_to_resource fails.
Maybe probe fail by returning dev_err_probe()?
Thanks,
Miles
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ
2022-03-17 14:39 [PATCH v14 0/6] Add mutex support for MDP Moudy Ho
@ 2022-03-17 14:39 ` Moudy Ho
2022-03-17 23:31 ` Miles Chen
2022-03-22 10:04 ` CK Hu
0 siblings, 2 replies; 6+ messages in thread
From: Moudy Ho @ 2022-03-17 14:39 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Hans Verkuil, Jernej Skrabec
Cc: Chun-Kuang Hu, Geert Uytterhoeven, Rob Landley, Laurent Pinchart,
linux-media, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, Alexandre Courbot, tfiga, drinkcat, pihsun, hsinyi,
AngeloGioacchino Del Regno, Maoguang Meng, daoyuan huang,
Ping-Hsun Wu, menghui.lin, sj.huang, allen-kh.cheng, randy.wu,
moudy.ho, jason-jh.lin, roy-cw.yeh, river.cheng, srv_heupstream,
Project_Global_Chrome_Upstream_Group
Due to HW limitations, MDP3 is necessary to enable MUTEX in each frame
for SOF triggering and cooperate with CMDQ control to reduce the amount
of interrupts generated(also, reduce frame latency).
In response to the above situation, a new interface
"mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.
Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
drivers/soc/mediatek/mtk-mutex.c | 45 +++++++++++++++++++++++++-
include/linux/soc/mediatek/mtk-mutex.h | 2 ++
2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
index c72e9f6ee4cc..32c87b8ed077 100644
--- a/drivers/soc/mediatek/mtk-mutex.c
+++ b/drivers/soc/mediatek/mtk-mutex.c
@@ -7,10 +7,14 @@
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/soc/mediatek/mtk-mmsys.h>
#include <linux/soc/mediatek/mtk-mutex.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
+
+#define MTK_MUTEX_ENABLE BIT(0)
#define MT2701_MUTEX0_MOD0 0x2c
#define MT2701_MUTEX0_SOF0 0x30
@@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
void __iomem *regs;
struct mtk_mutex mutex[10];
const struct mtk_mutex_data *data;
+ phys_addr_t addr;
+ struct cmdq_client_reg cmdq_reg;
+ bool has_gce_client_reg;
};
static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
@@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
}
EXPORT_SYMBOL_GPL(mtk_mutex_enable);
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
+{
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+ struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+ mutex[mutex->id]);
+ struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
+
+ WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+ if (!mtx->has_gce_client_reg) {
+ dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
+ return;
+ }
+
+ cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
+ mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
+ MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);
+#else
+ dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");
+#endif
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
+
void mtk_mutex_disable(struct mtk_mutex *mutex)
{
struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
@@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct mtk_mutex_ctx *mtx;
- struct resource *regs;
+ struct resource *regs, addr;
int i;
mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
@@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct platform_device *pdev)
}
}
+ if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
+ mtx->addr = 0L;
+ else
+ mtx->addr = addr.start;
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+ ret = cmdq_dev_get_client_reg(dev, &mtx->cmdq_reg, 0);
+ if (ret)
+ dev_dbg(dev, "No mediatek,gce-client-reg!\n");
+ else
+ mtx->has_gce_client_reg = true;
+#endif
+
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
mtx->regs = devm_ioremap_resource(dev, regs);
if (IS_ERR(mtx->regs)) {
diff --git a/include/linux/soc/mediatek/mtk-mutex.h b/include/linux/soc/mediatek/mtk-mutex.h
index 200f4365c950..17eea55b6809 100644
--- a/include/linux/soc/mediatek/mtk-mutex.h
+++ b/include/linux/soc/mediatek/mtk-mutex.h
@@ -33,6 +33,8 @@ void mtk_mutex_set_mod(struct mtk_mutex *mutex,
void mtk_mutex_set_sof(struct mtk_mutex *mutex,
enum mtk_mutex_table_index idx);
void mtk_mutex_enable(struct mtk_mutex *mutex);
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex,
+ void *pkt);
void mtk_mutex_disable(struct mtk_mutex *mutex);
void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
enum mtk_ddp_comp_id id);
--
2.18.0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-13 8:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <72f7feabe22064365457b5b72dfb1972ba2b3151.camel@mediatek.com>
2022-03-18 8:38 ` [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ Miles Chen
2022-04-11 7:23 [PATCH v14 0/6] Add mutex support for MDP Moudy Ho
2022-04-11 7:24 ` [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ Moudy Ho
2022-04-13 8:41 ` AngeloGioacchino Del Regno
-- strict thread matches above, loose matches on Subject: below --
2022-03-17 14:39 [PATCH v14 0/6] Add mutex support for MDP Moudy Ho
2022-03-17 14:39 ` [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ Moudy Ho
2022-03-17 23:31 ` Miles Chen
2022-03-22 10:04 ` CK Hu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®