Re: [PATCH v3 5/6] drm/mediatek: support CMDQ interface in ddp component

CK Hu
2019-12-05T04:51:07Z

Hi, Bibby:

On Wed, 2019-12-04 at 17:44 +0800, Bibby Hsieh wrote:
> The CMDQ (Command Queue) in MT8183 is used to help
> update all relevant display controller registers
> with critical time limation.
> This patch add cmdq interface in ddp_comp interface,
> let all ddp_comp interface can support cpu/cmdq function
> at the same time.
>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_color.c | 7 +-
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 65 +++++-----
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 43 ++++---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 +-
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 136 +++++++++++++++-----
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 31 +++--
> 6 files changed, 197 insertions(+), 93 deletions(-)
>

[snip]

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 3407d38aff8f..e93e46726de6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -12,7 +12,8 @@
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> -
> +#include <drm/drmP.h>
> +#include <linux/soc/mediatek/mtk-cmdq.h>
> #include "mtk_drm_drv.h"
> #include "mtk_drm_plane.h"
> #include "mtk_drm_ddp_comp.h"
> @@ -76,36 +77,82 @@
> #define DITHER_ADD_LSHIFT_G(x) (((x) & 0x7) << 4)
> #define DITHER_ADD_RSHIFT_G(x) (((x) & 0x7) << 0)
>
> +void mtk_ddp_write(struct cmdq_pkt *cmdq_pkt, unsigned int value,
> + struct mtk_ddp_comp *comp, unsigned int offset)
> +{
> + if (cmdq_pkt)
> +#ifdef CONFIG_MTK_CMDQ
> + cmdq_pkt_write(cmdq_pkt, comp->subsys,
> + comp->regs_pa + offset, value);
> +#endif
> + else
> + writel(value, comp->regs + offset);

If CONFIG_MTK_CMDQ is not defined, this code would become

if (cmdq_pkt)

else
writel(value, comp->regs + offset);

> +}
> +
> +void mtk_ddp_write_relaxed(struct cmdq_pkt *cmdq_pkt, unsigned int value,
> + struct mtk_ddp_comp *comp,
> + unsigned int offset)
> +{
> + if (cmdq_pkt)
> +#ifdef CONFIG_MTK_CMDQ
> + cmdq_pkt_write(cmdq_pkt, comp->subsys,
> + comp->regs_pa + offset, value);
> +#endif
> + else
> + writel_relaxed(value, comp->regs + offset);
> +}
> +
> +void mtk_ddp_write_mask(struct cmdq_pkt *cmdq_pkt,
> + unsigned int value,
> + struct mtk_ddp_comp *comp,
> + unsigned int offset,
> + unsigned int mask)
> +{
> + if (cmdq_pkt) {
> +#ifdef CONFIG_MTK_CMDQ
> + cmdq_pkt_write_mask(cmdq_pkt, comp->subsys,
> + comp->regs_pa + offset, value, mask);
> +#endif
> + } else {
> + u32 tmp = readl(comp->regs + offset);
> +
> + tmp = (tmp & ~mask) | (value & mask);
> + writel(tmp, comp->regs + offset);
> + }
> +}
> +

[snip]

>
> static void mtk_gamma_start(struct mtk_ddp_comp *comp)
> @@ -324,6 +371,9 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
> const struct mtk_ddp_comp_funcs *funcs)
> {
> struct platform_device *comp_pdev;
> + struct resource res;
> + struct cmdq_client_reg *cmdq_reg;

I think you could simply defined as below, so you need not to allocate
and free.

struct cmdq_client_reg cmdq_reg;

Regards,
CK

> + int ret = 0;
>
> if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX)
> return -EINVAL;
> @@ -358,6 +408,34 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
> }
> comp->dev = &comp_pdev->dev;
>
> +#ifdef CONFIG_MTK_CMDQ
> + if (of_address_to_resource(node, 0, &res) != 0) {
> + dev_err(dev, "Missing reg in %s node\n",
> + node->full_name);
> + return -EINVAL;
> + }
> + comp->regs_pa = res.start;
> +
> + comp_pdev = of_find_device_by_node(node);
> + if (!comp_pdev) {
> + dev_warn(dev, "Waiting for component device %s\n",
> + node->full_name);
> + return -EPROBE_DEFER;
> + }
> +
> + cmdq_reg = kzalloc(sizeof(*cmdq_reg), GFP_KERNEL);
> + if (!cmdq_reg)
> + return -EINVAL;
> +
> + ret = cmdq_dev_get_client_reg(&comp_pdev->dev, cmdq_reg, 0);
> + if (ret != 0)
> + dev_dbg(&comp_pdev->dev,
> + "get mediatek,gce-client-reg fail!\n");
> + else
> + comp->subsys = cmdq_reg->subsys;
> +
> + kfree(cmdq_reg);
> +#endif
> return 0;
> }
>