Re: [PATCH v2 10/46] drm/mediatek: Introduce and use path/comp definition structures
From: CK Hu (胡俊光)
Date: Wed Oct 28 2026 - 03:05:19 EST
On Tue, 2026-07-14 at 13:43 +0200, AngeloGioacchino Del Regno wrote:
> In preparation for a bigger restructuring of Display Controller
> HW sub-component handing and path building, introduce two new
handling
> mtk_drm_comp_definition and mtk_drm_path definition structures,
> meant to replace all of the {main,ext,third}_path pointers and
> supportive length variables.
>
> Obviously, also change all of the functions that were previously
> using the old path array pointers to use the new structures.
>
> This made possible to loop through the path definition structures,
This made it possible to
> shortening various code paths and hence improving readability but,
> as a nice side effect, now it is possible to easily extend the
> maximum number of outputs that are supported by the driver.
> Of course, even though that could be done by simply extending the
> already present mtk_crtc_path enumeration (which now also indexes
> the mtk_drm_path_definition array in mtk_mmsys_driver_data), doing
> so will increase the memory footprint for all platforms (even the
> ones with a single output): this is currently left unresolved, but
> this change makes performing a future optimization to add dynamic
> CRTC output indexing(/numbers) way easier than before.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> ---
[snip]
>
> -int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> - unsigned int path_len, int priv_data_index,
> +int mtk_crtc_create(struct drm_device *drm_dev,
> + enum mtk_crtc_path path_sel, int priv_data_index,
> const struct mtk_drm_route *conn_routes,
> unsigned int num_conn_routes)
> {
> struct mtk_drm_private *priv = drm_dev->dev_private;
> + const struct mtk_drm_path_definition *output_path;
> struct device *dev = drm_dev->dev;
> struct mtk_ddp_comp *dma_comp;
> struct mtk_crtc *mtk_crtc;
> @@ -1043,16 +1046,14 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> struct drm_crtc *tmp;
> int crtc_i = 0;
>
> - if (!path)
> - return 0;
> -
> priv = priv->all_drm_private[priv_data_index];
> + output_path = &priv->data->output_paths[path_sel];
>
> drm_for_each_crtc(tmp, drm_dev)
> crtc_i++;
>
> - for (i = 0; i < path_len; i++) {
> - enum mtk_ddp_comp_id comp_id = path[i];
> + for (i = 0; i < output_path[i].len; i++) {
i < output_path->len
> + enum mtk_ddp_comp_id comp_id = output_path->comp[i].type;
> struct device_node *node;
> struct mtk_ddp_comp *comp;
>
> @@ -1076,12 +1077,13 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> }
>
> mtk_crtc = devm_kzalloc(dev,
> - struct_size(mtk_crtc, ddp_comp, path_len + (conn_routes ? 1 : 0)),
> + struct_size(mtk_crtc, ddp_comp,
> + output_path->len + (conn_routes ? 1 : 0)),
> GFP_KERNEL);
> if (!mtk_crtc)
> return -ENOMEM;
>
> - mtk_crtc->ddp_comp_nr = path_len;
> + mtk_crtc->ddp_comp_nr = output_path->len;
> mtk_crtc->mmsys_dev = priv->mmsys_dev;
>
> mtk_crtc->mutex = mtk_mutex_get(priv->mutex_dev);
> @@ -1092,7 +1094,7 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> }
>
> for (i = 0, j = 0; i < mtk_crtc->ddp_comp_nr; i++, j++) {
> - unsigned int comp_id = path[i];
> + unsigned int comp_id = output_path->comp[i].type;
> struct mtk_ddp_comp *comp;
>
> comp = mtk_ddp_comp_find_by_id(&priv->hlist, comp_id);
> @@ -1138,7 +1140,7 @@ int mtk_crtc_create(struct drm_device *drm_dev, const unsigned int *path,
> * In the case of ovl_adaptor sub driver, it needs to use the
> * dma_dev_get function to get representative dma dev.
> */
> - dma_comp = mtk_ddp_comp_find_by_id(&priv->hlist, path[0]);
> + dma_comp = mtk_ddp_comp_find_by_id(&priv->hlist, output_path->comp[0].type);
> if (dma_comp == NULL) {
> dev_err(dev, "Could not find appropriate DMA device!\n");
> return -EINVAL;
>
[snip]
>
> -static bool mtk_ddp_path_available(const unsigned int *path,
> - unsigned int path_len,
> +static bool mtk_ddp_path_available(const struct mtk_drm_path_definition *output_path,
> struct device_node **comp_node)
You have remove all the caller of mtk_ddp_path_available(), so remove this function in this patch.
If you need this function in later patch, add this function in that patch.
> {
> unsigned int i;
>
> - if (!path || !path_len)
> - return false;
> -
> - for (i = 0U; i < path_len; i++) {
> + for (i = 0U; i < output_path->len; i++) {
> /* OVL_ADAPTOR doesn't have a device node */
> - if (path[i] == DDP_COMPONENT_DRM_OVL_ADAPTOR)
> + if (output_path->comp[i].type == DDP_COMPONENT_DRM_OVL_ADAPTOR)
> continue;
>
> - if (!comp_node[path[i]])
> + if (!comp_node[output_path->comp[i].type])
> return false;
> }
>
> @@ -576,31 +568,14 @@ int mtk_find_possible_crtcs(struct drm_device *drm, struct device *dev)
> priv_n = private->all_drm_private[j];
> data = priv_n->data;
>
> - if (mtk_ddp_path_available(data->main_path, data->main_len,
> - priv_n->comp_node)) {
> - if (mtk_ddp_comp_find(dev, data->main_path,
> - data->main_len,
> - &priv_n->hlist))
> - return BIT(i);
> - i++;
> - }
> -
> - if (mtk_ddp_path_available(data->ext_path, data->ext_len,
> - priv_n->comp_node)) {
> - if (mtk_ddp_comp_find(dev, data->ext_path,
> - data->ext_len,
> - &priv_n->hlist))
> - return BIT(i);
> - i++;
> - }
> + for (i = 0; i < MAX_CRTC; i++) {
> + if (!data->output_paths[i].len)
> + continue;
>
> - if (mtk_ddp_path_available(data->third_path, data->third_len,
> - priv_n->comp_node)) {
> - if (mtk_ddp_comp_find(dev, data->third_path,
> - data->third_len,
> - &priv_n->hlist))
> + ret = mtk_ddp_comp_find(dev, &data->output_paths[i],
> + &priv_n->hlist);
> + if (ret)
> return BIT(i);
> - i++;
> }
> }
>
[snip]
>
> -static const unsigned int mt7623_mtk_ddp_main[] = {
> - DDP_COMPONENT_OVL0,
> - DDP_COMPONENT_RDMA0,
> - DDP_COMPONENT_COLOR0,
> - DDP_COMPONENT_BLS,
> - DDP_COMPONENT_DPI0,
> +struct mtk_drm_path_definition mt2701_legacy_paths[MAX_CRTC] = {
static const
> + [CRTC_MAIN] = {
> + .comp = mt2701_mtk_ddp_main,
> + .len = ARRAY_SIZE(mt2701_mtk_ddp_main),
> + },
> + [CRTC_EXT] = {
> + .comp = mt2701_mtk_ddp_ext,
> + .len = ARRAY_SIZE(mt2701_mtk_ddp_ext),
> + },
> };
>
[snip]
> @@ -895,8 +966,7 @@ static int mtk_drm_of_get_ddp_ep_cid(struct device_node *node,
> * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path
> * @dev: The mediatek-drm device
> * @cpath: CRTC Path relative to a VDO or MMSYS
> - * @out_path: Pointer to an array that will contain the new pipeline
> - * @out_path_len: Number of entries in the pipeline array
> + * @out_path: Pointer to the structure that will contain the new pipeline
> *
> * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending
> * on the board-specific desired display configuration; this function walks
> @@ -910,20 +980,19 @@ static int mtk_drm_of_get_ddp_ep_cid(struct device_node *node,
> * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller
> */
> static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath,
> - const unsigned int **out_path,
> - unsigned int *out_path_len)
> + struct mtk_drm_path_definition *out_path)
> {
> + struct mtk_drm_comp_definition temp_path[MTK_DISP_CONTROLLER_MAX_COMP_PER_PATH];
You change the temp_path[] size to MTK_DISP_CONTROLLER_MAX_COMP_PER_PATH,
but below do-while check idx < DDP_COMPONENT_DRM_ID_MAX,
you need to change the idx limit.
> struct device_node *next = NULL, *prev, *vdo = dev->parent->of_node;
> - unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 };
> - unsigned int *final_ddp_path;
> - unsigned short int idx = 0;
> bool ovl_adaptor_comp_added = false;
> + unsigned short int idx = 0;
> + size_t final_comp_sz;
> int ret;
>
> /* Get the first entry for the temp_path array */
> ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]);
> if (ret) {
> - if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> + if (next && temp_path[idx].type == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next);
> ovl_adaptor_comp_added = true;
> } else {
> @@ -957,7 +1026,7 @@ static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path
> * to probe that component master driver of which only one instance
> * is needed and possible.
> */
> - if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> + if (temp_path[idx].type == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> if (!ovl_adaptor_comp_added)
> ovl_adaptor_comp_added = true;
> else
> @@ -973,7 +1042,7 @@ static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path
> return ret;
>
> /* If the last entry is not a final display output, the configuration is wrong */
> - switch (temp_path[idx - 1]) {
> + switch (temp_path[idx - 1].type) {
> case DDP_COMPONENT_DP_INTF0:
> case DDP_COMPONENT_DP_INTF1:
> case DDP_COMPONENT_DPI0:
> @@ -985,30 +1054,31 @@ static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path
> break;
> default:
> dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n",
> - temp_path[idx - 1], ret);
> + temp_path[idx - 1].type, ret);
> return -EINVAL;
> }
>
> - final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL);
> - if (!final_ddp_path)
> + /* Pipeline built! */
> + out_path->len = idx;
> + final_comp_sz = out_path->len * sizeof(out_path->comp[0]);
> + out_path->comp = devm_kmemdup(dev, temp_path, final_comp_sz, GFP_KERNEL);
> + if (!out_path->comp)
> return -ENOMEM;
>
> dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx);
>
> - /* Pipeline built! */
> - *out_path = final_ddp_path;
> - *out_path_len = idx;
> -
> return 0;
> }
>
> static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node *node,
> struct mtk_mmsys_driver_data *data)
> {
> + struct mtk_drm_path_definition *output_paths;
> struct device_node *ep_node;
> struct of_endpoint of_ep;
> bool output_present[MAX_CRTC] = { false };
> - int ret;
> + u8 num_outputs_present = 0;
> + int i, ret;
>
> for_each_endpoint_of_node(node, ep_node) {
> ret = of_graph_parse_endpoint(ep_node, &of_ep);
> @@ -1024,33 +1094,30 @@ static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node *nod
> }
>
> output_present[of_ep.id] = true;
> + num_outputs_present++;
> }
> + if (ret == 0 && num_outputs_present == 0)
ret is not used before this line, so remove checking of 'ret == 0'.
> + ret = dev_err_probe(dev, -ENXIO, "No display outputs found.\n");
>
> if (ret) {
> of_node_put(ep_node);
> return ret;
> }
>
> - if (output_present[CRTC_MAIN]) {
> - ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_MAIN,
> - &data->main_path, &data->main_len);
> - if (ret && ret != -ENODEV)
> - return ret;
> - }
> + /* This can be optimized after making CRTC numbers dynamic */
> + output_paths = devm_kcalloc(dev, MAX_CRTC, sizeof(*output_paths), GFP_KERNEL);
> + if (!output_paths)
> + return -ENOMEM;
>
> - if (output_present[CRTC_EXT]) {
> - ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_EXT,
> - &data->ext_path, &data->ext_len);
> - if (ret && ret != -ENODEV)
> - return ret;
> - }
> + for (i = 0; i < MAX_CRTC; i++) {
> + if (!output_present[i])
> + continue;
>
> - if (output_present[CRTC_THIRD]) {
> - ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_THIRD,
> - &data->third_path, &data->third_len);
> + ret = mtk_drm_of_ddp_path_build_one(dev, i, &output_paths[i]);
> if (ret && ret != -ENODEV)
> return ret;
> }
> + data->output_paths = output_paths;
>
> return 0;
> }
>
[snip]
> +
> struct mtk_mmsys_driver_data {
> - const unsigned int *main_path;
> - unsigned int main_len;
> - const unsigned int *ext_path;
> - unsigned int ext_len;
> - const unsigned int *third_path;
> - unsigned int third_len;
> + struct mtk_drm_path_definition *output_paths;
> + u8 num_output_paths;
num_output_paths is not used. Remove it.
Regards,
CK
> const struct mtk_drm_route *conn_routes;
> unsigned int num_conn_routes;
>