Re: [PATCH v1 6/6] drm/mediatek: apply CMDQ control flow

Hi, Bibby:

On Tue, 2019-12-03 at 13:58 +0800, Bibby Hsieh wrote:

On Tue, 2019-12-03 at 09:38 +0800, CK Hu wrote:

...

Hi, Bibby:

On Thu, 2019-11-28 at 10:42 +0800, Bibby Hsieh wrote:

...

Unlike other SoCs, MT8183 does not have "shadow" registers for performaing an atomic video mode set or page flip at vblank/vsync.

The CMDQ (Commend Queue) in MT8183 is used to help update all relevant display controller registers with critical time limation.

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_drm_crtc.c | 86 ++++++++++++++++++++- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 31 ++++++++ 2 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index fcf4e755e0bd..1b4e537ac3c1 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -12,6 +12,8 @@ #include <drm/drm_plane_helper.h> #include <drm/drm_probe_helper.h> #include <drm/drm_vblank.h> +#include <linux/of_address.h> +#include <linux/soc/mediatek/mtk-cmdq.h>

#include "mtk_drm_drv.h" #include "mtk_drm_crtc.h" @@ -42,6 +44,9 @@ struct mtk_drm_crtc { unsigned int layer_nr; bool pending_planes;

• struct cmdq_client *cmdq_client;
• u32 cmdq_event;
• void __iomem *config_regs; const struct mtk_mmsys_reg_data *mmsys_reg_data; struct mtk_disp_mutex *mutex;

@@ -56,6 +61,11 @@ struct mtk_crtc_state { unsigned int pending_width; unsigned int pending_height; unsigned int pending_vrefresh;

• struct cmdq_pkt *cmdq_handle;

+};

+struct mtk_cmdq_cb_data {

• struct cmdq_pkt *cmdq_handle;

};

static inline struct mtk_drm_crtc *to_mtk_crtc(struct drm_crtc *c) @@ -229,6 +239,46 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc, return NULL; }

+#ifdef CONFIG_MTK_CMDQ +static void ddp_cmdq_cb(struct cmdq_cb_data data) +{

• struct mtk_cmdq_cb_data *cb_data = data.data;
• cmdq_pkt_destroy(cb_data->cmdq_handle);
• kfree(cb_data);

+}

+static void mtk_cmdq_acquire(struct drm_crtc *crtc) +{

• struct mtk_crtc_state *mtk_crtc_state =

• 	to_mtk_crtc_state(crtc->state);
  

• struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
• mtk_crtc_state->cmdq_handle =

• 	cmdq_pkt_create(mtk_crtc->cmdq_client,
  

• 			PAGE_SIZE);
  

I would like to remove atomic feature in cmdq driver and drm driver could reuse the pkt. Please refer to [1] for detail.

[1] http://lists.infradead.org/pipermail/linux-mediatek/2019-January/016866.html

Hi, CK,

Thanks for all the comments. I will change them in my next version.

About the atomic feature removing, I think we already made note at here. Let's fix them after the atomic feature removed really.

Removing atomic feature would modify the device tree, and I'm worry that device tree setting could not be modified at that time. I think we could modify the device tree now because there no client use cmdq in upstream.

Regards, CK

Bibby

...

...

• cmdq_pkt_clear_event(mtk_crtc_state->cmdq_handle,

• 	     mtk_crtc->cmdq_event);
  

• cmdq_pkt_wfe(mtk_crtc_state->cmdq_handle, mtk_crtc->cmdq_event);

+}

+static void mtk_cmdq_release(struct drm_crtc *crtc) +{

• struct mtk_crtc_state *mtk_crtc_state =

• 	to_mtk_crtc_state(crtc->state);
  

• struct mtk_cmdq_cb_data *cb_data;
• cb_data = kmalloc(sizeof(*cb_data), GFP_KERNEL);
• if (!cb_data) {

• DRM_DEV_ERROR(crtc->dev->dev, "Failed to alloc cb_data\n");
  

• return;
  

• }
• cb_data->cmdq_handle = mtk_crtc_state->cmdq_handle;
• cmdq_pkt_flush_async(mtk_crtc_state->cmdq_handle,

• 	     ddp_cmdq_cb, cb_data);
  

+} +#endif static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) { struct drm_crtc *crtc = &mtk_crtc->base; @@ -383,7 +433,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc) if (state->pending_config) { mtk_ddp_comp_config(comp, state->pending_width, state->pending_height,

• 		    state->pending_vrefresh, 0, NULL);
  

• 		    state->pending_vrefresh, 0,
  

• 		    state->cmdq_handle);
  
  

  state->pending_config = false; }

@@ -403,7 +454,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)

	if (comp)
		mtk_ddp_comp_layer_config(comp, local_layer,

• 					  plane_state, NULL);
  

• 					  plane_state,
  

• 					  state->cmdq_handle);
  plane_state->pending.config = false;
  

  } mtk_crtc->pending_planes = false;

@@ -454,6 +506,13 @@ void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane, mtk_crtc_ddp_config(crtc); mtk_disp_mutex_release(mtk_crtc->mutex); } +#ifdef CONFIG_MTK_CMDQ

• if (mtk_crtc->cmdq_client) {

• mtk_cmdq_acquire(crtc);
  

• mtk_crtc_ddp_config(crtc);
  

• mtk_cmdq_release(crtc);
  

• }

+#endif mutex_unlock(&priv->hw_lock); }

@@ -570,6 +629,13 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc, mtk_crtc_ddp_config(crtc); mtk_disp_mutex_release(mtk_crtc->mutex); } +#ifdef CONFIG_MTK_CMDQ

• if (mtk_crtc->cmdq_client) {

• mtk_cmdq_acquire(crtc);
  

• mtk_crtc_ddp_config(crtc);
  

• mtk_cmdq_release(crtc);
  

• }

+#endif

This part is almost the same as the one in mtk_drm_crtc_cursor_update(), try to merge them.

...

}

static const struct drm_crtc_funcs mtk_crtc_funcs = { @@ -619,7 +685,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *comp) struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); struct mtk_drm_private *priv = crtc->dev->dev_private;

• if (!priv->data->shadow_register)

• if (!priv->data->shadow_register && !mtk_crtc->cmdq_client) mtk_crtc_ddp_config(crtc);

  mtk_drm_finish_page_flip(mtk_crtc);

@@ -761,6 +827,18 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, drm_mode_crtc_set_gamma_size(&mtk_crtc->base, MTK_LUT_SIZE); drm_crtc_enable_color_mgmt(&mtk_crtc->base, 0, false, MTK_LUT_SIZE); priv->num_pipes++;

+#ifdef CONFIG_MTK_CMDQ

• mtk_crtc->cmdq_client =

• 	cmdq_mbox_create(dev, drm_crtc_index(&mtk_crtc->base),
  

• 			 2000);
  

• of_property_read_u32_index(dev->of_node, "mediatek,gce-events",

• 		   drm_crtc_index(&mtk_crtc->base),
  

• 		   &mtk_crtc->cmdq_event);
  

• if (IS_ERR(mtk_crtc->cmdq_client)) {

• dev_dbg(dev, "mtk_crtc %d failed to create mailbox client, writing register by CPU now\n",
  

• 	drm_crtc_index(&mtk_crtc->base));
  

• mtk_crtc->cmdq_client = NULL;
  

• }

+#endif return 0; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c index 6d0f349ddf82..9cc12af2bc06 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -370,6 +370,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;

• int ret = 0;

  if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX) return -EINVAL;

@@ -404,6 +407,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

I would like to move this part to the patch "drm/mediatek: support CMDQ interface in ddp component".

Regards, CK

...

return 0; }