On Tue, 2026-07-14 at 13:43 +0200, AngeloGioacchino Del Regno wrote:
> Perform a full migration to HW Type -> HW Instance ID indexing
> for all of the currently supported Display Controller hardware
> sub-components.
>
> This allows to remove the big fixed-size catch-all components
> array (that contains as many element as much as the number of
> supported hardware and number of instances of that), which is
> currently growing bigger and bigger at every sub-ip support
> addition in mediatek-drm.
>
> Note that this cleanup became necessary seen the need to add
> support for new components found in MT8196, MT8994 and other
> variants of it, and of course those SoCs won't be the last
> ones to be upstreamed... so the size of that array would get
> out of control pretty fast (and again, it's already very big).
>
> As a side effect, while saving memory compared to before this
> change, this also allows to have up to 24 (as an extensible fixed
> size declared in this driver) instances of the same HW Type with
> no further increase in memory footprint.
> Similarly, adding support for new hardware will not produce an
> increase of memory footprint on all SoCs (if not only temporarily
> while the probe process is in place) as there's no more catch-all
> array, but only a dynamically allocated list of per display output
> components.
>
> Unfortunately, this change needs to go along with the related one
> in the mtk-mmsys driver, otherwise functionality of the Display
> Controller will regress.
>
> The only way to make this possible in two steps is to duplicate
> all of the routes structure arrays for all of the SoCs, which
> would result in a total of around ~1200 lines changed twice, and
> that ignores the big increase in size for this driver during the
> migration process so, in order to avoid useless bloat, I opted
> for an inter-dependency between the two changes: mediatek-drm
> and mtk-mmsys.
>
> Signed-off-by: AngeloGioacchino Del Regno
> <[email protected]>
> ---
[snip]
> @@ -1143,25 +1158,15 @@ int mtk_crtc_create(struct drm_device *drm_dev,
> crtc_i++;
>
> for (i = 0; i < output_path[i].len; i++) {
> - enum mtk_ddp_comp_id comp_id = output_path->comp[i].type;
> - struct device_node *node;
> + enum mtk_ddp_comp_type comp_type = output_path->comp[i].type;
> + u8 comp_inst = output_path->comp[i].inst_id;
> struct mtk_ddp_comp *comp;
>
> - node = priv->comp_node[comp_id];
> - comp = mtk_ddp_comp_find_by_id(&priv->hlist, comp_id);
> -
> - /* Not all drm components have a DTS device node, such as
> ovl_adaptor,
> - * which is the drm bring up sub driver
> - */
> - if (!node && comp_id != DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> - dev_info(dev,
> - "Not creating crtc %d because component %d is
> disabled or missing\n",
> - crtc_i, comp_id);
> - return 0;
The original behavior for disabled component would return 0, but you change the
behavior here.
Is is safe for legacy SoC?
> - }
> -
> + comp = mtk_ddp_comp_find_by_id(&priv->hlist, comp_type,
> comp_inst);
> if (!comp || !comp->dev) {
> - dev_err(dev, "Component %pOF not initialized\n", node);
> + dev_err(dev,
> + "CRTC%d: Component type=%u inst=%u not
> initialized\n",
> + crtc_i, comp_type, comp_inst);
> return -ENODEV;
> }
> }
>
[snip]
> static bool mtk_ddp_find_comp_dev_in_table(const struct mtk_drm_comp_list
> *hlist,
> - const unsigned int comp_id,
> + const unsigned int comp_type,
> struct device *dev)
> {
> struct mtk_ddp_comp *ddp_comp;
>
> - hash_for_each_possible(hlist->ddp_list, ddp_comp, lnode, comp_id) {
> + hash_for_each_possible(hlist->ddp_list, ddp_comp, lnode, comp_type) {
You change the comp_id to comp_type.
If the correct match is DPI0, after this patch, the DPI1 would match?
> if (ddp_comp->dev == dev)
> return true;
> }
>
[snip]
> int mtk_ddp_comp_get_id(struct device_node *node,
> enum mtk_ddp_comp_type comp_type)
> {
> - int id = of_alias_get_id(node, mtk_ddp_comp_stem[comp_type]);
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(mtk_ddp_matches); i++) {
> - if (comp_type == mtk_ddp_matches[i].type &&
> - (id < 0 || id == mtk_ddp_matches[i].alias_id))
> - return i;
> + /* If there's an alias, return the ID from that */
> + if (mtk_ddp_comp_stem[comp_type]) {
> + int alias_id = of_alias_get_id(node,
> mtk_ddp_comp_stem[comp_type]);
> + if (alias_id >= 0)
> + return alias_id;
> }
>
> - return -EINVAL;
> + return 0;
0 is valid id so this function would not return error.
You change this behavior, so I would like some explain.
> }
>
> int mtk_find_possible_crtcs(struct drm_device *drm, struct device *dev)
>
[snip]
> @@ -16,6 +16,7 @@
> #include <drm/drm_modes.h>
>
> #define MTK_DISP_CONTROLLER_MAX_COMP_PER_PATH 24
> +#define MTK_DISP_CONTROLLER_MAX_HW_COMP_INSTANCE 32
MTK_DISP_CONTROLLER_MAX_HW_COMP_INSTANCE is useless, so drop it.
>
> struct device;
> struct device_node;
>
[snip]
> @@ -924,18 +928,16 @@ static int mtk_drm_probe(struct platform_device *pdev)
> continue;
> }
>
> - comp_id = mtk_ddp_comp_get_id(node, comp_type);
> - if (comp_id < 0) {
> + comp_inst_id = mtk_ddp_comp_get_id(node, comp_type);
> + if (comp_inst_id < 0) {
comp_inst_id is u8, so this would never happen.
Regards,
CK
> dev_warn(dev, "Skipping unknown component %pOF\n",
> node);
> continue;
> }
>
> - if (!mtk_drm_find_mmsys_comp(private, comp_id))
> + if (!mtk_drm_find_mmsys_comp(private, comp_type, comp_inst_id))
> continue;
>
> - private->comp_node[comp_id] = of_node_get(node);
> -
> /*
> * Currently only the AAL, CCORR, COLOR, GAMMA, MERGE, OVL,
> RDMA, DSI, and DPI
> * blocks have separate component platform drivers and
> initialize their own
>
