[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <071256b8cf2a59cb3b52e7fcfa06a2d74293e491.camel@mediatek.com>
Date: Mon, 15 Sep 2025 09:28:51 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, Paul-pl Chen (陳柏霖)
<Paul-pl.Chen@...iatek.com>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
CC: Sunny Shen (沈姍姍) <Sunny.Shen@...iatek.com>,
Sirius Wang (王皓昱) <Sirius.Wang@...iatek.com>,
Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
Xiandong Wang (王先冬)
<Xiandong.Wang@...iatek.com>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"fshao@...omium.org" <fshao@...omium.org>, "p.zabel@...gutronix.de"
<p.zabel@...gutronix.de>, Singo Chang (張興國)
<Singo.Chang@...iatek.com>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
<matthias.bgg@...il.com>, "treapking@...omium.org" <treapking@...omium.org>
Subject: Re: [PATCH v4 17/19] drm/mediatek: add ovlsys_adaptor support for
MT8196
On Thu, 2025-08-28 at 16:07 +0800, Paul Chen wrote:
> From: Nancy Lin <nancy.lin@...iatek.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@...iatek.com>
> Signed-off-by: Paul-pl Chen <paul-pl.chen@...iatek.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);
> + }
> +}
> +
Powered by blists - more mailing lists