[PATCH v12 1/4] soc: mediatek: mmsys: add CMDQ write register function

moudy.ho moudy.ho at mediatek.com
Wed Mar 9 01:53:41 PST 2022


On Thu, 2022-03-03 at 15:59 +0800, CK Hu wrote:
> Hi, Moudy:
> 
> On Tue, 2022-03-01 at 18:02 +0800, Moudy Ho wrote:
> > Adding the interface of writing MMSYS register via CMDQ,
> > users do not need to parse related dts information.
> > 
> > Signed-off-by: Moudy Ho <moudy.ho at mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno at collabora.com>
> > ---
> >  drivers/soc/mediatek/Kconfig           |  1 +
> >  drivers/soc/mediatek/mtk-mmsys.c       | 30 ++++++++++++++++
> >  drivers/soc/mediatek/mtk-mmsys.h       |  1 +
> >  include/linux/soc/mediatek/mtk-mmsys.h | 50
> > ++++++++++++++++++++++++++
> >  4 files changed, 82 insertions(+)
> > 
> > diff --git a/drivers/soc/mediatek/Kconfig
> > b/drivers/soc/mediatek/Kconfig
> > index fdd8bc08569e..172bc7828aca 100644
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -69,6 +69,7 @@ config MTK_MMSYS
> >  	bool "MediaTek MMSYS Support"
> >  	default ARCH_MEDIATEK
> >  	depends on HAS_IOMEM
> > +	select MTK_CMDQ
> 
> In MT8173, mmsys could work without CMDQ, so I think mmsys should not
> select CMDQ.
> 
Hi CK,

Thanks for the reminder, I will use "IS_REACHABLE" macro instead of
config select operation.

> >  	help
> >  	  Say yes here to add support for the MediaTek Multimedia
> >  	  Subsystem (MMSYS).
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.c
> > b/drivers/soc/mediatek/mtk-mmsys.c
> > index 50c797d70ddd..04c0c7de395e 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -8,9 +8,11 @@
> >  #include <linux/device.h>
> >  #include <linux/io.h>
> >  #include <linux/of_device.h>
> > +#include <linux/of_address.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/reset-controller.h>
> >  #include <linux/soc/mediatek/mtk-mmsys.h>
> > +#include <linux/soc/mediatek/mtk-cmdq.h>
> >  
> >  #include "mtk-mmsys.h"
> >  #include "mt8167-mmsys.h"
> > @@ -55,6 +57,7 @@ static const struct mtk_mmsys_driver_data
> > mt8183_mmsys_driver_data = {
> >  	.clk_driver = "clk-mt8183-mm",
> >  	.routes = mmsys_mt8183_routing_table,
> >  	.num_routes = ARRAY_SIZE(mmsys_mt8183_routing_table),
> > +	.has_gce_client_reg = true,
> >  };
> >  
> >  static const struct mtk_mmsys_driver_data mt8186_mmsys_driver_data
> > =
> > {
> > @@ -81,6 +84,8 @@ struct mtk_mmsys {
> >  	const struct mtk_mmsys_driver_data *data;
> >  	spinlock_t lock; /* protects mmsys_sw_rst_b reg */
> >  	struct reset_controller_dev rcdev;
> > +	struct cmdq_client_reg cmdq_base;
> > +	phys_addr_t addr;
> >  };
> >  
> >  void mtk_mmsys_ddp_connect(struct device *dev,
> > @@ -120,6 +125,17 @@ void mtk_mmsys_ddp_disconnect(struct device
> > *dev,
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
> >  
> > +void mtk_mmsys_write_reg_by_cmdq(struct device *dev,
> > +				 struct mmsys_cmdq_cmd *cmd,
> > +				 u32 offset, u32 value, u32 mask)
> 
> I would like mmsys provide high level interface rather than such low
> level interface.
> 

Thank you for your suggestion, we will adjust MDP flow to avoid
frequent use of MMSYS API to operate CMDQ, so we can remove this
unnecessary API in the next version.

> > +{
> > +	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> > +
> > +	cmdq_pkt_write_mask(cmd->pkt, mmsys->cmdq_base.subsys,
> > +			    mmsys->addr + offset, value, mask);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mmsys_write_reg_by_cmdq);
> > +
> >  static int mtk_mmsys_reset_update(struct reset_controller_dev
> > *rcdev, unsigned long id,
> >  				  bool assert)
> >  {
> > @@ -178,6 +194,7 @@ static int mtk_mmsys_probe(struct
> > platform_device
> > *pdev)
> >  	struct platform_device *clks;
> >  	struct platform_device *drm;
> >  	struct mtk_mmsys *mmsys;
> > +	struct resource res;
> >  	int ret;
> >  
> >  	mmsys = devm_kzalloc(dev, sizeof(*mmsys), GFP_KERNEL);
> > @@ -203,6 +220,19 @@ static int mtk_mmsys_probe(struct
> > platform_device *pdev)
> >  		return ret;
> >  	}
> >  
> > +	if (of_address_to_resource(dev->of_node, 0, &res) < 0)
> > +		mmsys->addr = 0L;
> > +	else
> > +		mmsys->addr = res.start;
> > +
> > +	if (mmsys->data->has_gce_client_reg) {
> > +		ret = cmdq_dev_get_client_reg(dev, &mmsys->cmdq_base,
> > 0);
> 
> Add gce client reg in mmsys binding document [1].
> 
> [1] 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml?h=v5.17-rc6
> 

Thanks for the reminder, I will fill in the missing parts in the next
version.
	 
> > +		if (ret) {
> > +			dev_err(dev, "No mediatek,gce-client-reg!\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> >  	mmsys->data = of_device_get_match_data(&pdev->dev);
> >  	platform_set_drvdata(pdev, mmsys);
> >  
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.h
> > b/drivers/soc/mediatek/mtk-mmsys.h
> > index 8b0ed05117ea..9fce400507d2 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.h
> > +++ b/drivers/soc/mediatek/mtk-mmsys.h
> > @@ -92,6 +92,7 @@ struct mtk_mmsys_driver_data {
> >  	const char *clk_driver;
> >  	const struct mtk_mmsys_routes *routes;
> >  	const unsigned int num_routes;
> > +	bool has_gce_client_reg;
> >  };
> >  
> >  /*
> > diff --git a/include/linux/soc/mediatek/mtk-mmsys.h
> > b/include/linux/soc/mediatek/mtk-mmsys.h
> > index 4bba275e235a..7f8ecc98d023 100644
> > --- a/include/linux/soc/mediatek/mtk-mmsys.h
> > +++ b/include/linux/soc/mediatek/mtk-mmsys.h
> > @@ -7,8 +7,14 @@
> >  #define __MTK_MMSYS_H
> >  
> >  enum mtk_ddp_comp_id;
> > +enum mtk_mdp_comp_id;
> >  struct device;
> >  
> > +struct mmsys_cmdq_cmd {
> > +	struct cmdq_pkt *pkt;
> > +	s32 *event;
> > +};
> > +
> >  enum mtk_ddp_comp_id {
> >  	DDP_COMPONENT_AAL0,
> >  	DDP_COMPONENT_AAL1,
> > @@ -45,6 +51,46 @@ enum mtk_ddp_comp_id {
> >  	DDP_COMPONENT_ID_MAX,
> >  };
> >  
> > +enum mtk_mdp_comp_id {
> > +	MDP_COMP_NONE = -1,	/* Invalid engine */
> > +
> > +	/* ISP */
> > +	MDP_COMP_WPEI = 0,
> > +	MDP_COMP_WPEO,		/* 1 */
> > +	MDP_COMP_WPEI2,		/* 2 */
> > +	MDP_COMP_WPEO2,		/* 3 */
> > +	MDP_COMP_ISP_IMGI,	/* 4 */
> > +	MDP_COMP_ISP_IMGO,	/* 5 */
> > +	MDP_COMP_ISP_IMG2O,	/* 6 */
> > +
> > +	/* IPU */
> > +	MDP_COMP_IPUI,		/* 7 */
> > +	MDP_COMP_IPUO,		/* 8 */
> > +
> > +	/* MDP */
> > +	MDP_COMP_CAMIN,		/* 9 */
> > +	MDP_COMP_CAMIN2,	/* 10 */
> > +	MDP_COMP_RDMA0,		/* 11 */
> > +	MDP_COMP_AAL0,		/* 12 */
> > +	MDP_COMP_CCORR0,	/* 13 */
> > +	MDP_COMP_RSZ0,		/* 14 */
> > +	MDP_COMP_RSZ1,		/* 15 */
> > +	MDP_COMP_TDSHP0,	/* 16 */
> > +	MDP_COMP_COLOR0,	/* 17 */
> > +	MDP_COMP_PATH0_SOUT,	/* 18 */
> > +	MDP_COMP_PATH1_SOUT,	/* 19 */
> > +	MDP_COMP_WROT0,		/* 20 */
> > +	MDP_COMP_WDMA,		/* 21 */
> > +
> > +	/* Dummy Engine */
> > +	MDP_COMP_RDMA1,		/* 22 */
> > +	MDP_COMP_RSZ2,		/* 23 */
> > +	MDP_COMP_TDSHP1,	/* 24 */
> > +	MDP_COMP_WROT1,		/* 25 */
> > +
> > +	MDP_MAX_COMP_COUNT	/* ALWAYS keep at the end */
> > +};
> 
> Useless, remove this.
> 
> Regards,
> CK
> 

In order to cooperate with the operation of MUTEX and maintain the MOD
define in the corresponding driver(used in mtk_mutex_get_mdp_mod), it
is necessary to move the MDP component ID to a higher-level module to
communicate across modules.
Can this setting be kept here?

Thanks & Regards,
Moudy Ho

> > +
> >  void mtk_mmsys_ddp_connect(struct device *dev,
> >  			   enum mtk_ddp_comp_id cur,
> >  			   enum mtk_ddp_comp_id next);
> > @@ -53,4 +99,8 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
> >  			      enum mtk_ddp_comp_id cur,
> >  			      enum mtk_ddp_comp_id next);
> >  
> > +void mtk_mmsys_write_reg_by_cmdq(struct device *dev,
> > +				 struct mmsys_cmdq_cmd *cmd,
> > +				 u32 alias_id, u32 value, u32 mask);
> > +
> >  #endif /* __MTK_MMSYS_H */
> 
> 




More information about the Linux-mediatek mailing list