[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a7fd78a9fb858552e48339bc4bf3d3423d428f3b.camel@mediatek.com>
Date: Tue, 8 Oct 2024 07:41:16 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"sui.jingfeng@...ux.dev" <sui.jingfeng@...ux.dev>, "wenst@...omium.org"
<wenst@...omium.org>, Sjoerd Simons <sjoerd@...labora.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"tzimmermann@...e.de" <tzimmermann@...e.de>,
Shawn Sung (宋孝謙) <Shawn.Sung@...iatek.com>,
"mripard@...nel.org" <mripard@...nel.org>,
Jitao Shi (石记涛) <jitao.shi@...iatek.com>,
"michael@...le.cc" <michael@...le.cc>, "daniel@...ll.ch" <daniel@...ll.ch>,
"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>, "conor+dt@...nel.org"
<conor+dt@...nel.org>, "maarten.lankhorst@...ux.intel.com"
<maarten.lankhorst@...ux.intel.com>, "robh@...nel.org" <robh@...nel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"airlied@...il.com" <airlied@...il.com>, "krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>, "kernel@...labora.com"
<kernel@...labora.com>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
Yu-chang Lee (李禹璋) <Yu-chang.Lee@...iatek.com>,
"mwalle@...nel.org" <mwalle@...nel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, Alexandre Mergnat
<amergnat@...libre.com>
Subject: Re: [PATCH v11 3/3] drm/mediatek: Implement OF graphs support for
display paths
Hi, Angelo:
On Mon, 2024-10-07 at 11:31 +0200, AngeloGioacchino Del Regno wrote:
> It is impossible to add each and every possible DDP path combination
> for each and every possible combination of SoC and board: right now,
> this driver hardcodes configuration for 10 SoCs and this is going to
> grow larger and larger, and with new hacks like the introduction of
> mtk_drm_route which is anyway not enough for all final routes as the
> DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling
> DSC preventively doesn't work if the display doesn't support it, or
> others.
>
> Since practically all display IPs in MediaTek SoCs support being
> interconnected with different instances of other, or the same, IPs
> or with different IPs and in different combinations, the final DDP
> pipeline is effectively a board specific configuration.
>
> Implement OF graphs support to the mediatek-drm drivers, allowing to
> stop hardcoding the paths, and preventing this driver to get a huge
> amount of arrays for each board and SoC combination, also paving the
> way to share the same mtk_mmsys_driver_data between multiple SoCs,
> making it more straightforward to add support for new chips.
>
> Reviewed-by: Alexandre Mergnat <amergnat@...libre.com>
> Tested-by: Alexandre Mergnat <amergnat@...libre.com>
> Acked-by: Sui Jingfeng <sui.jingfeng@...ux.dev>
> Tested-by: Michael Walle <mwalle@...nel.org> # on kontron-sbc-i1200
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
[snip]
> +
> +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node)
> +{
> + enum mtk_ovl_adaptor_comp_type type;
> + int ret;
> +
> + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type);
> + if (ret)
> + return false;
> +
> + if (type >= OVL_ADAPTOR_TYPE_NUM)
> + return false;
> +
> + /*
> + * In the context of mediatek-drm, ETHDR, MDP_RDMA and Padding are
> + * used exclusively by OVL Adaptor: if this component is not one of
> + * those, it's likely not an OVL Adaptor path.
> + */
> + return type == OVL_ADAPTOR_TYPE_ETHDR ||
> + type == OVL_ADAPTOR_TYPE_MDP_RDMA ||
> + type == OVL_ADAPTOR_TYPE_PADDING;
> +}
> +
[snip]
> +
> +static int mtk_drm_of_get_ddp_ep_cid(struct device_node *node,
> + int output_port, enum mtk_crtc_path crtc_path,
> + struct device_node **next, unsigned int *cid)
> +{
> + struct device_node *ep_dev_node, *ep_out;
> + enum mtk_ddp_comp_type comp_type;
> + int ret;
> +
> + ep_out = of_graph_get_endpoint_by_regs(node, output_port, crtc_path);
> + if (!ep_out)
> + return -ENOENT;
> +
> + ep_dev_node = of_graph_get_remote_port_parent(ep_out);
> + of_node_put(ep_out);
> + if (!ep_dev_node)
> + return -EINVAL;
> +
> + /*
> + * Pass the next node pointer regardless of failures in the later code
> + * so that if this function is called in a loop it will walk through all
> + * of the subsequent endpoints anyway.
> + */
> + *next = ep_dev_node;
> +
> + if (!of_device_is_available(ep_dev_node))
> + return -ENODEV;
> +
> + ret = mtk_drm_of_get_ddp_comp_type(ep_dev_node, &comp_type);
> + if (ret) {
> + if (mtk_ovl_adaptor_is_comp_present(ep_dev_node)) {
> + *cid = (unsigned int)DDP_COMPONENT_DRM_OVL_ADAPTOR;
> + return 0;
> + }
> + return ret;
> + }
> +
> + ret = mtk_ddp_comp_get_id(ep_dev_node, comp_type);
> + if (ret < 0)
> + return ret;
> +
> + /* All ok! Pass the Component ID to the caller. */
> + *cid = (unsigned int)ret;
> +
> + return 0;
> +}
> +
> +/**
> + * 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
> + *
> + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending
> + * on the board-specific desired display configuration; this function walks
> + * through all of the output endpoints starting from a VDO or MMSYS hardware
> + * instance and builds the right pipeline as specified in device trees.
> + *
> + * Return:
> + * * %0 - Display HW Pipeline successfully built and validated
> + * * %-ENOENT - Display pipeline was not specified in device tree
> + * * %-EINVAL - Display pipeline built but validation failed
> + * * %-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 device_node *next, *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;
> + 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) {
> + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next);
> + ovl_adaptor_comp_added = true;
> + } else {
> + if (next)
> + dev_err(dev, "Invalid component %pOF\n", next);
> + else
> + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath);
> +
> + return ret;
> + }
> + }
> + idx++;
> +
> + /*
> + * Walk through port outputs until we reach the last valid mediatek-drm component.
> + * To be valid, this must end with an "invalid" component that is a display node.
> + */
> + do {
> + prev = next;
> + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]);
> + of_node_put(prev);
> + if (ret) {
> + of_node_put(next);
> + break;
> + }
> +
> + /*
> + * If this is an OVL adaptor exclusive component and one of those
> + * was already added, don't add another instance of the generic
> + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether
> + * 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 (!ovl_adaptor_comp_added)
> + ovl_adaptor_comp_added = true;
> + else
> + idx--;
> + }
> + } while (++idx < DDP_COMPONENT_DRM_ID_MAX);
For the mt8195 external display path, the OF graph is
mdp_rdma (0 ~ 7) -> padding (0 ~ 7) -> merge (1 ~ 4) -> ethdr -> merge5
and this function would generate the path as
ovl_adaptor -> merge (1 ~ 4) -> merge 5
This is not what I expect.
Is any thing wrong with me?
Regards,
CK
> +
> + /*
> + * The device component might not be enabled: in that case, don't
> + * check the last entry and just report that the device is missing.
> + */
> + if (ret == -ENODEV)
> + return ret;
> +
> + /* If the last entry is not a final display output, the configuration is wrong */
> + switch (temp_path[idx - 1]) {
> + case DDP_COMPONENT_DP_INTF0:
> + case DDP_COMPONENT_DP_INTF1:
> + case DDP_COMPONENT_DPI0:
> + case DDP_COMPONENT_DPI1:
> + case DDP_COMPONENT_DSI0:
> + case DDP_COMPONENT_DSI1:
> + case DDP_COMPONENT_DSI2:
> + case DDP_COMPONENT_DSI3:
> + break;
> + default:
> + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n",
> + temp_path[idx - 1], ret);
> + return -EINVAL;
> + }
> +
> + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL);
> + if (!final_ddp_path)
> + 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;
> +}
> +
Powered by blists - more mailing lists