Re: [PATCH v4 17/19] drm/mediatek: add ovlsys_adaptor support for MT8196

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

On Thu, 2025-08-28 at 16:07 +0800, Paul Chen wrote:
> From: Nancy Lin <nancy.lin@mediatek.com>
> 
> Ovlsys_adaptor is an encapsulated module designed to
> simplify the DRM control flow. This module is composed
> of 20 EXDMAs, 20 BLENDERs, and 12 OUTPROCs.
> Two EXDMAs merge into one layer, allowing the module
> to support 20 layers for 3 display paths.
> Ovlsys_adaptor driver is integrated within the
> mtk_ddp_comp framework.
> 
> Signed-off-by: Nancy Lin <nancy.lin@mediatek.com>
> Signed-off-by: Paul-pl Chen <paul-pl.chen@mediatek.com>
> ---

[snip]

> diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> index 7f9facb5574b..cba996f99dd7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
> @@ -33,6 +33,7 @@ enum mtk_ddp_comp_type {
>  	MTK_DISP_MUTEX,
>  	MTK_DISP_OD,
>  	MTK_DISP_OVL,
> +	MTK_DISP_OVLSYS_ADAPTOR,
>  	MTK_DISP_OVL_2L,
>  	MTK_DISP_OVL_ADAPTOR,

Move MTK_DISP_OVLSYS_ADAPTOR here?

>  	MTK_DISP_POSTMASK,
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index cecee77a8590..3457c33ded57 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> 

[snip]

> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_print.h>
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_device.h>

Alphabetic order.

> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <linux/soc/mediatek/mtk-cmdq.h>
> +#include <linux/soc/mediatek/mtk-mmsys.h>
> +#include <linux/soc/mediatek/mtk-mutex.h>
> +

[snip]

> +int mtk_ovlsys_adaptor_power_on(struct device *dev)
> +{
> +	struct mtk_disp_ovlsys_adaptor *priv = dev_get_drvdata(dev);
> +	struct device *comp;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < priv->path_size; i++) {
> +		comp = priv->ovl_adaptor_comp[priv->path[i]];
> +		if (!comp || get_type(priv->path[i]) != OVLSYS_ADAPTOR_TYPE_EXDMA)

I would like that in ovlsys_adaptor_comp_init() it make sure that all comp exist once,
so these interface would not check comp exist or not.
Without this checking,

/* power on EXDMA only */
for (i = 0; i < (priv->path_size - 1); i += 2) {

> +			continue;
> +
> +		ret = pm_runtime_get_sync(comp);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to enable power domain %d, err %d\n", i, ret);
> +			goto pwr_err;
> +		}
> +	}
> +
> +	return 0;
> +
> +pwr_err:
> +	while (--i >= 0) {
> +		comp = priv->ovl_adaptor_comp[i];
> +		if (!comp || get_type(priv->path[i]) != OVLSYS_ADAPTOR_TYPE_EXDMA)
> +			continue;

drop this checking and,

do {
	i -= 2;
	if (i < 0)
		break;

	...
}

> +
> +		pm_runtime_put(priv->ovl_adaptor_comp[i]);
> +	}
> +
> +	return ret;
> +}
> +
> +void mtk_ovlsys_adaptor_power_off(struct device *dev)
> +{
> +	struct mtk_disp_ovlsys_adaptor *priv = dev_get_drvdata(dev);
> +	struct device *comp;
> +	int i;
> +
> +	for (i = 0; i < priv->path_size; i++) {
> +		comp = priv->ovl_adaptor_comp[priv->path[i]];
> +		if (!comp || get_type(priv->path[i]) != OVLSYS_ADAPTOR_TYPE_EXDMA)
> +			continue;

drop this checking, and

/* power off EXDMA only */
for (i = 0; i < (priv->path_size - 1); i += 2) {

> +
> +		pm_runtime_put(comp);
> +	}
> +}
> +
> +void mtk_ovlsys_adaptor_layer_config(struct device *dev, unsigned int idx,
> +				     struct mtk_plane_state *state,
> +				     struct cmdq_pkt *cmdq_pkt)
> +{
> +	struct mtk_plane_pending_state *pending = &state->pending;
> +	struct mtk_disp_ovlsys_adaptor *priv = dev_get_drvdata(dev);
> +	struct device *exdma;
> +	struct device *blender;
> +	const struct drm_format_info *fmt_info = drm_format_info(pending->format);
> +
> +	DRM_DEV_DEBUG_DRIVER(dev, "idx:%d enable:%d fmt:0x%x addr:0x%pad fb_w:%d {%d,%d,%d,%d}\n",
> +			     idx, pending->enable, pending->format,
> +			     &pending->addr, (pending->pitch / fmt_info->cpp[0]),
> +			     pending->x, pending->y, pending->width, pending->height);
> +
> +	exdma = priv->ovl_adaptor_comp[priv->path[idx * 2]];
> +	if (!exdma) {

Make sure that all comp exist once in ovlsys_adaptor_comp_init(),
and drop this checking.

> +		dev_err(dev, "%s: exdma%d comp not found\n", __func__, idx);
> +		return;
> +	}
> +
> +	blender = priv->ovl_adaptor_comp[priv->path[idx * 2 + 1]];
> +	if (!blender) {

Make sure that all comp exist once in ovlsys_adaptor_comp_init(),
and drop this checking.

> +		dev_err(dev, "%s: blender%d comp not found\n", __func__, idx);
> +		return;
> +	}
> +
> +	mtk_disp_exdma_layer_config(exdma, state, cmdq_pkt);
> +	mtk_disp_blender_layer_config(blender, state, cmdq_pkt);
> +}
> +

[snip]

> +int mtk_ovlsys_adaptor_clk_enable(struct device *dev)
> +{
> +	struct mtk_disp_ovlsys_adaptor *priv = dev_get_drvdata(dev);
> +	struct device *comp;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < priv->path_size; i++) {
> +		comp = priv->ovl_adaptor_comp[priv->path[i]];
> +
> +		if (get_type(priv->path[i]) == OVLSYS_ADAPTOR_TYPE_EXDMA)
> +			ret = mtk_disp_exdma_clk_enable(comp);
> +		else if (get_type(priv->path[i]) == OVLSYS_ADAPTOR_TYPE_BLENDER)
> +			ret = mtk_disp_blender_clk_enable(comp);
> +		else
> +			ret = mtk_disp_outproc_clk_enable(comp);

Drop the compare, and

for (i = 0; i < (priv->path_size - 1); i += 2) {
	exdma = priv->ovl_adaptor_comp[priv->path[i * 2]];
	blender = priv->ovl_adaptor_comp[priv->path[i * 2 + 1]];
}
outproc = priv->ovl_adaptor_comp[priv->path[i * 2]];

> +
> +		if (ret) {
> +			dev_err(dev, "Failed to enable clock %d, err %d\n", i, ret);
> +			goto clk_err;
> +		}
> +	}
> +
> +	return 0;
> +
> +clk_err:
> +	while (--i >= 0) {
> +		comp = priv->ovl_adaptor_comp[priv->path[i]];
> +		if (!comp)
> +			continue;
> +
> +		if (get_type(priv->path[i]) == OVLSYS_ADAPTOR_TYPE_EXDMA)
> +			mtk_disp_exdma_clk_disable(comp);
> +		else if (get_type(priv->path[i]) == OVLSYS_ADAPTOR_TYPE_BLENDER)
> +			mtk_disp_blender_clk_disable(comp);
> +		else
> +			mtk_disp_outproc_clk_disable(comp);
> +	}
> +
> +	return ret;
> +}
> +
> +void mtk_ovlsys_adaptor_clk_disable(struct device *dev)
> +{
> +	struct mtk_disp_ovlsys_adaptor *priv = dev_get_drvdata(dev);
> +	struct device *comp;
> +	int i;
> +
> +	for (i = 0; i < priv->path_size; i++) {
> +		comp = priv->ovl_adaptor_comp[priv->path[i]];
> +		if (!comp)
> +			continue;
> +
> +		if (get_type(priv->path[i]) == OVLSYS_ADAPTOR_TYPE_EXDMA)
> +			mtk_disp_exdma_clk_disable(comp);
> +		else if (get_type(priv->path[i]) == OVLSYS_ADAPTOR_TYPE_BLENDER)
> +			mtk_disp_blender_clk_disable(comp);
> +		else
> +			mtk_disp_outproc_clk_disable(comp);
> +	}

Drop the compare, and

for (i = 0; i < (priv->path_size - 1); i += 2) {
	exdma = priv->ovl_adaptor_comp[priv->path[i * 2]];
	blender = priv->ovl_adaptor_comp[priv->path[i * 2 + 1]];
}
outproc = priv->ovl_adaptor_comp[priv->path[i * 2]];

> +}
> +
> +unsigned int mtk_ovlsys_adaptor_layer_nr(struct device *dev)
> +{
> +	struct mtk_disp_ovlsys_adaptor *priv = dev_get_drvdata(dev);
> +
> +	return priv->layer_nr;
> +}
> +

[snip]

> +static int mtk_disp_ovlsys_adaptor_probe(struct platform_device *pdev)
> +{
> +	struct mtk_disp_ovlsys_adaptor *priv;
> +	struct device *dev = &pdev->dev;
> +	struct component_match *match = NULL;
> +	struct mtk_ovlsys_platform_data *ovlsys_priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	ovlsys_priv = pdev->dev.platform_data;
> +	priv->mmsys_dev = ovlsys_priv->mmsys_dev;
> +
> +	if (ovlsys_priv->use_path == CRTC_MAIN) {
> +		priv->path = mt8196_mtk_ovl_main;
> +		priv->path_size = ARRAY_SIZE(mt8196_mtk_ovl_main);
> +	} else if (ovlsys_priv->use_path == CRTC_EXT) {
> +		priv->path = mt8196_mtk_ovl_ext;
> +		priv->path_size = ARRAY_SIZE(mt8196_mtk_ovl_ext);
> +	} else if (ovlsys_priv->use_path == CRTC_THIRD) {
> +		priv->path = mt8196_mtk_ovl_third;
> +		priv->path_size = ARRAY_SIZE(mt8196_mtk_ovl_third);
> +	}
> +
> +	ret = ovlsys_adaptor_comp_init(dev, &match);
> +	if (ret < 0)
> +		return ret;
> +
> +	component_master_add_with_match(dev, &mtk_disp_ovlsys_adaptor_master_ops, match);
> +
> +	pm_runtime_enable(dev);

ovlsys_adaptor itself has no power.
Drop this.
ovl_adaptor has the same problem.

> +
> +	ret = component_add(dev, &mtk_disp_ovlsys_adaptor_comp_ops);
> +	if (ret != 0) {
> +		pm_runtime_disable(dev);
> +		dev_err(dev, "Failed to add component: %d\n", ret);
> +	}
> +
> +	return ret;
> +}
> +

[snip]

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index cf5f9b8fc335..7972a0506616 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -428,32 +428,41 @@ static bool mtk_drm_get_all_drm_priv(struct device *dev)
>  	return false;
>  }
>  
> -static bool mtk_drm_find_mmsys_comp(struct mtk_drm_private *private, int comp_id)
> +static bool mtk_drm_find_mmsys_comp(struct mtk_drm_private *private, int comp_id, int *use_path)

Separate mtk_drm_find_mmsys_comp() modification to independent patch which is a prepare patch for this patch.

>  {
>  	const struct mtk_mmsys_driver_data *drv_data = private->data;
> -	int i;
> +	int i, path = MAX_CRTC;
>  
>  	if (drv_data->main_path)
>  		for (i = 0; i < drv_data->main_len; i++)
> -			if (drv_data->main_path[i] == comp_id)
> -				return true;
> +			if (drv_data->main_path[i] == comp_id) {
> +				path = CRTC_MAIN;
> +				goto found;
> +			}
>  
>  	if (drv_data->ext_path)
>  		for (i = 0; i < drv_data->ext_len; i++)
> -			if (drv_data->ext_path[i] == comp_id)
> -				return true;
> -
> +			if (drv_data->ext_path[i] == comp_id) {
> +				path = CRTC_EXT;
> +				goto found;
> +			}
>  	if (drv_data->third_path)
>  		for (i = 0; i < drv_data->third_len; i++)
> -			if (drv_data->third_path[i] == comp_id)
> -				return true;
> +			if (drv_data->third_path[i] == comp_id) {
> +				path = CRTC_THIRD;
> +				goto found;
> +			}
>  
>  	if (drv_data->num_conn_routes)
>  		for (i = 0; i < drv_data->num_conn_routes; i++)
>  			if (drv_data->conn_routes[i].route_ddp == comp_id)
> -				return true;
> +				goto found;
>  
>  	return false;
> +found:
> +	if (use_path)
> +		*use_path = path;
> +	return true;
>  }
>  
>  static int mtk_drm_kms_init(struct drm_device *drm)
> @@ -1069,6 +1078,26 @@ static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node *nod
>  	return 0;
>  }
>  
> +static void mtk_drm_ovl_adaptor_probe(struct device *dev, struct mtk_drm_private *private,
> +				      struct component_match **match, enum mtk_ddp_comp_id id)

Separate mtk_drm_ovl_adaptor_probe() modification to independent patch which is a prepare patch for this patch.

Regards,
CK

> +{
> +	struct platform_device *ovl_adaptor;
> +	struct mtk_ovlsys_platform_data ovlsys_priv;
> +	bool is_ovlsys = (id != DDP_COMPONENT_DRM_OVL_ADAPTOR);
> +	char *dev_name = is_ovlsys ? "mediatek-disp-ovlsys-adaptor" : "mediatek-disp-ovl-adaptor";
> +	void *drv_data = is_ovlsys ? (void *)&ovlsys_priv : (void *)private->mmsys_dev;
> +	size_t data_size = is_ovlsys ? sizeof(ovlsys_priv) : sizeof(*private->mmsys_dev);
> +
> +	if (mtk_drm_find_mmsys_comp(private, id, &ovlsys_priv.use_path)) {
> +		ovlsys_priv.mmsys_dev = private->mmsys_dev;
> +		ovl_adaptor = platform_device_register_data(dev, dev_name, PLATFORM_DEVID_AUTO,
> +							    drv_data, data_size);
> +		private->ddp_comp[id].dev = &ovl_adaptor->dev;
> +		mtk_ddp_comp_init(NULL, &private->ddp_comp[id], id);
> +		component_match_add(dev, match, compare_dev, &ovl_adaptor->dev);
> +	}
> +}
> +