| Message ID | 20241007093114.35332-4-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mediatek: Add support for OF graphs | expand |
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@baylibre.com> > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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; > +} > +
Il 08/10/24 09:41, CK Hu (胡俊光) ha scritto: > 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@baylibre.com> >> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> >> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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? > I mean no offense, really, but your reply here is a contradiction... In [1], you explained what the path for the external display should look like and said that the graph in DT should generate a path which, in the driver, shall look like the current mt8195_mtk_ddp_ext[] hardcoded array. In [2], you repeated that you "just want the path to be like mt8195_mtk_ddp_ext[]". Now you're saying that this is not what you expect. I don't understand your intention. [1]: https://lore.kernel.org/all/58ee09aeb5a224dbc8faee236ed1a77ce3fbd011.camel@mediatek.com/ [2]: https://lore.kernel.org/all/04f1506b23b41c775e0735b5b3189b4118500715.camel@mediatek.com/ Regards, Angelo > >> + >> + /* >> + * 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; >> +} >> +
Hi, Angelo: On Tue, 2024-10-08 at 10:03 +0200, AngeloGioacchino Del Regno wrote: > Il 08/10/24 09:41, CK Hu (胡俊光) ha scritto: > > 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@baylibre.com> > > > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > > > Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> > > > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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? > > > > I mean no offense, really, but your reply here is a contradiction... > > In [1], you explained what the path for the external display should look like > and said that the graph in DT should generate a path which, in the driver, shall > look like the current mt8195_mtk_ddp_ext[] hardcoded array. > > In [2], you repeated that you "just want the path to be like mt8195_mtk_ddp_ext[]". > > Now you're saying that this is not what you expect. > I don't understand your intention. In [1] & [2], I want the path to be like mt8195_mtk_ddp_ext[]. I don't know where is the contradiction? mt8195_mtk_ddp_ext[] is: ovl_adaptor -> merge5 but this patch generate the path as ovl_adaptor -> merge (1 ~ 4) -> merge5 it's not the same and this may cause something wrong. I'm sorry my expression make you confused. So what I want is to generate the path as ovl_adaptor -> merge5 Regards, CK > > [1]: > https://lore.kernel.org/all/58ee09aeb5a224dbc8faee236ed1a77ce3fbd011.camel@mediatek.com/ > > [2]: > https://lore.kernel.org/all/04f1506b23b41c775e0735b5b3189b4118500715.camel@mediatek.com/ > > Regards, > Angelo > > > > > > + > > > + /* > > > + * 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; > > > +} > > > + > >
Il 09/10/24 10:43, CK Hu (胡俊光) ha scritto: > Hi, Angelo: > > On Tue, 2024-10-08 at 10:03 +0200, AngeloGioacchino Del Regno wrote: >> Il 08/10/24 09:41, CK Hu (胡俊光) ha scritto: >>> 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@baylibre.com> >>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> >>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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? >>> >> >> I mean no offense, really, but your reply here is a contradiction... >> >> In [1], you explained what the path for the external display should look like >> and said that the graph in DT should generate a path which, in the driver, shall >> look like the current mt8195_mtk_ddp_ext[] hardcoded array. >> >> In [2], you repeated that you "just want the path to be like mt8195_mtk_ddp_ext[]". >> >> Now you're saying that this is not what you expect. >> I don't understand your intention. > > In [1] & [2], I want the path to be like mt8195_mtk_ddp_ext[]. I don't know where is the contradiction? > mt8195_mtk_ddp_ext[] is: > > ovl_adaptor -> merge5 > > but this patch generate the path as > > ovl_adaptor -> merge (1 ~ 4) -> merge5 > > it's not the same and this may cause something wrong. > I'm sorry my expression make you confused. > So what I want is to generate the path as > > ovl_adaptor -> merge5 Ah, okay, no - you're misunderstanding how the OVL_ADAPTOR is treated here, hence we misunderstood each other in the end! The resulting path in mt8195_mtk_ddp_ext[] will be ovl_adaptor->merge5, because the merge1-4 are already taken dynamically by the ovl_adaptor driver so these will be *temporarily omitted* in the graph for MT8195. My intention is to add handling for the additional merge1-4 (and similar) selection with OF Graph support *later*, not in this series (please be aware that it will *not be needed* to change any bindings, and compatibility will be guaranteed with no additional effort). This is because I believe that the ovl_adaptor driver needs more changes than just a basic OF Graph implementation: as of now, that driver is practically specific to just MT8195 and MT8188, as the OVL_ADAPTOR specific MERGE paths are hardcoded. So, I am planning to improve the ovl_adaptor driver, but that will be a separated series as it's probably going to be a relatively (in relation to the size of the ovl_adaptor driver) big set of changes. Regards, Angelo > > Regards, > CK > >> >> [1]: >> https://lore.kernel.org/all/58ee09aeb5a224dbc8faee236ed1a77ce3fbd011.camel@mediatek.com/ >> >> [2]: >> https://lore.kernel.org/all/04f1506b23b41c775e0735b5b3189b4118500715.camel@mediatek.com/ >> >> Regards, >> Angelo >> >>> >>>> + >>>> + /* >>>> + * 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; >>>> +} >>>> + >> >>
Hi, Angelo: On Wed, 2024-10-09 at 12:10 +0200, AngeloGioacchino Del Regno wrote: > Il 09/10/24 10:43, CK Hu (胡俊光) ha scritto: > > Hi, Angelo: > > > > On Tue, 2024-10-08 at 10:03 +0200, AngeloGioacchino Del Regno wrote: > > > Il 08/10/24 09:41, CK Hu (胡俊光) ha scritto: > > > > 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@baylibre.com> > > > > > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > > > > > Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> > > > > > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > > > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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? > > > > > > > > > > I mean no offense, really, but your reply here is a contradiction... > > > > > > In [1], you explained what the path for the external display should look like > > > and said that the graph in DT should generate a path which, in the driver, shall > > > look like the current mt8195_mtk_ddp_ext[] hardcoded array. > > > > > > In [2], you repeated that you "just want the path to be like mt8195_mtk_ddp_ext[]". > > > > > > Now you're saying that this is not what you expect. > > > I don't understand your intention. > > > > In [1] & [2], I want the path to be like mt8195_mtk_ddp_ext[]. I don't know where is the contradiction? > > mt8195_mtk_ddp_ext[] is: > > > > ovl_adaptor -> merge5 > > > > but this patch generate the path as > > > > ovl_adaptor -> merge (1 ~ 4) -> merge5 > > > > it's not the same and this may cause something wrong. > > I'm sorry my expression make you confused. > > So what I want is to generate the path as > > > > ovl_adaptor -> merge5 > > Ah, okay, no - you're misunderstanding how the OVL_ADAPTOR is treated here, hence > we misunderstood each other in the end! > > The resulting path in mt8195_mtk_ddp_ext[] will be ovl_adaptor->merge5, because > the merge1-4 are already taken dynamically by the ovl_adaptor driver so these > will be *temporarily omitted* in the graph for MT8195. For "*temporarily omitted* in the graph for MT8195", do you mean the OF graph is mdp_rdma (0 ~ 7) -> padding (0 ~ 7) -> ethdr -> merge5 So the path would be ovl_adaptor -> merge5. So this patch work fine depending on the tricky way of OF graph. Add comment to describe the tricky way of OF graph. After this, Reviewed-by: CK Hu <ck.hu@mediatek.com> > > My intention is to add handling for the additional merge1-4 (and similar) selection > with OF Graph support *later*, not in this series (please be aware that it will > *not be needed* to change any bindings, and compatibility will be guaranteed with > no additional effort). > > This is because I believe that the ovl_adaptor driver needs more changes than just > a basic OF Graph implementation: as of now, that driver is practically specific to > just MT8195 and MT8188, as the OVL_ADAPTOR specific MERGE paths are hardcoded. > > So, I am planning to improve the ovl_adaptor driver, but that will be a separated > series as it's probably going to be a relatively (in relation to the size of the > ovl_adaptor driver) big set of changes. > > Regards, > Angelo > > > > > Regards, > > CK > > > > > > > > [1]: > > > https://lore.kernel.org/all/58ee09aeb5a224dbc8faee236ed1a77ce3fbd011.camel@mediatek.com/ > > > > > > [2]: > > > https://lore.kernel.org/all/04f1506b23b41c775e0735b5b3189b4118500715.camel@mediatek.com/ > > > > > > Regards, > > > Angelo > > > > > > > > > > > > + > > > > > + /* > > > > > + * 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; > > > > > +} > > > > > + > > > > > > > >
Il 14/10/24 10:19, CK Hu (胡俊光) ha scritto: > Hi, Angelo: > > On Wed, 2024-10-09 at 12:10 +0200, AngeloGioacchino Del Regno wrote: >> Il 09/10/24 10:43, CK Hu (胡俊光) ha scritto: >>> Hi, Angelo: >>> >>> On Tue, 2024-10-08 at 10:03 +0200, AngeloGioacchino Del Regno wrote: >>>> Il 08/10/24 09:41, CK Hu (胡俊光) ha scritto: >>>>> 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@baylibre.com> >>>>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>>> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> >>>>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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? >>>>> >>>> >>>> I mean no offense, really, but your reply here is a contradiction... >>>> >>>> In [1], you explained what the path for the external display should look like >>>> and said that the graph in DT should generate a path which, in the driver, shall >>>> look like the current mt8195_mtk_ddp_ext[] hardcoded array. >>>> >>>> In [2], you repeated that you "just want the path to be like mt8195_mtk_ddp_ext[]". >>>> >>>> Now you're saying that this is not what you expect. >>>> I don't understand your intention. >>> >>> In [1] & [2], I want the path to be like mt8195_mtk_ddp_ext[]. I don't know where is the contradiction? >>> mt8195_mtk_ddp_ext[] is: >>> >>> ovl_adaptor -> merge5 >>> >>> but this patch generate the path as >>> >>> ovl_adaptor -> merge (1 ~ 4) -> merge5 >>> >>> it's not the same and this may cause something wrong. >>> I'm sorry my expression make you confused. >>> So what I want is to generate the path as >>> >>> ovl_adaptor -> merge5 >> >> Ah, okay, no - you're misunderstanding how the OVL_ADAPTOR is treated here, hence >> we misunderstood each other in the end! >> >> The resulting path in mt8195_mtk_ddp_ext[] will be ovl_adaptor->merge5, because >> the merge1-4 are already taken dynamically by the ovl_adaptor driver so these >> will be *temporarily omitted* in the graph for MT8195. > > For "*temporarily omitted* in the graph for MT8195", do you mean the OF graph is > > mdp_rdma (0 ~ 7) -> padding (0 ~ 7) -> ethdr -> merge5 > Yes. > So the path would be > > ovl_adaptor -> merge5. > Yes, exactly! > So this patch work fine depending on the tricky way of OF graph. > Add comment to describe the tricky way of OF graph. After this, > > Reviewed-by: CK Hu <ck.hu@mediatek.com> Thank you! :-) Cheers, Angelo > >> >> My intention is to add handling for the additional merge1-4 (and similar) selection >> with OF Graph support *later*, not in this series (please be aware that it will >> *not be needed* to change any bindings, and compatibility will be guaranteed with >> no additional effort). >> >> This is because I believe that the ovl_adaptor driver needs more changes than just >> a basic OF Graph implementation: as of now, that driver is practically specific to >> just MT8195 and MT8188, as the OVL_ADAPTOR specific MERGE paths are hardcoded. >> >> So, I am planning to improve the ovl_adaptor driver, but that will be a separated >> series as it's probably going to be a relatively (in relation to the size of the >> ovl_adaptor driver) big set of changes. >> >> Regards, >> Angelo >> >>> >>> Regards, >>> CK >>> >>>> >>>> [1]: >>>> https://lore.kernel.org/all/58ee09aeb5a224dbc8faee236ed1a77ce3fbd011.camel@mediatek.com/ >>>> >>>> [2]: >>>> https://lore.kernel.org/all/04f1506b23b41c775e0735b5b3189b4118500715.camel@mediatek.com/ >>>> >>>> Regards, >>>> Angelo >>>> >>>>> >>>>>> + >>>>>> + /* >>>>>> + * 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; >>>>>> +} >>>>>> + >>>> >>>> >> >>
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h index 082ac18fe04a..94843974851f 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h @@ -108,6 +108,7 @@ size_t mtk_ovl_get_num_formats(struct device *dev); void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex *mutex); void mtk_ovl_adaptor_remove_comp(struct device *dev, struct mtk_mutex *mutex); +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node); void mtk_ovl_adaptor_connect(struct device *dev, struct device *mmsys_dev, unsigned int next); void mtk_ovl_adaptor_disconnect(struct device *dev, struct device *mmsys_dev, diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c index c6768210b08b..4e064d3c97cc 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c @@ -490,6 +490,41 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; } +static int ovl_adaptor_of_get_ddp_comp_type(struct device_node *node, + enum mtk_ovl_adaptor_comp_type *ctype) +{ + const struct of_device_id *of_id = of_match_node(mtk_ovl_adaptor_comp_dt_ids, node); + + if (!of_id) + return -EINVAL; + + *ctype = (enum mtk_ovl_adaptor_comp_type)((uintptr_t)of_id->data); + + return 0; +} + +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; +} + static int ovl_adaptor_comp_init(struct device *dev, struct component_match **match) { struct mtk_disp_ovl_adaptor *priv = dev_get_drvdata(dev); @@ -499,12 +534,11 @@ static int ovl_adaptor_comp_init(struct device *dev, struct component_match **ma parent = dev->parent->parent->of_node->parent; for_each_child_of_node_scoped(parent, node) { - const struct of_device_id *of_id; enum mtk_ovl_adaptor_comp_type type; - int id; + int id, ret; - of_id = of_match_node(mtk_ovl_adaptor_comp_dt_ids, node); - if (!of_id) + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); + if (ret) continue; if (!of_device_is_available(node)) { @@ -513,7 +547,6 @@ static int ovl_adaptor_comp_init(struct device *dev, struct component_match **ma continue; } - type = (enum mtk_ovl_adaptor_comp_type)(uintptr_t)of_id->data; id = ovl_adaptor_comp_get_id(dev, node, type); if (id < 0) { dev_warn(dev, "Skipping unknown component %pOF\n", diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index a08d20654954..20a9d589fd75 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -704,6 +704,20 @@ static int mtk_dpi_bridge_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags) { struct mtk_dpi *dpi = bridge_to_dpi(bridge); + int ret; + + dpi->next_bridge = devm_drm_of_get_bridge(dpi->dev, dpi->dev->of_node, 1, -1); + if (IS_ERR(dpi->next_bridge)) { + ret = PTR_ERR(dpi->next_bridge); + if (ret == -EPROBE_DEFER) + return ret; + + /* Old devicetree has only one endpoint */ + dpi->next_bridge = devm_drm_of_get_bridge(dpi->dev, dpi->dev->of_node, 0, 0); + if (IS_ERR(dpi->next_bridge)) + return dev_err_probe(dpi->dev, PTR_ERR(dpi->next_bridge), + "Failed to get bridge\n"); + } return drm_bridge_attach(bridge->encoder, dpi->next_bridge, &dpi->bridge, flags); @@ -1058,13 +1072,6 @@ static int mtk_dpi_probe(struct platform_device *pdev) if (dpi->irq < 0) return dpi->irq; - dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); - if (IS_ERR(dpi->next_bridge)) - return dev_err_probe(dev, PTR_ERR(dpi->next_bridge), - "Failed to get bridge\n"); - - dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node); - platform_set_drvdata(pdev, dpi); dpi->bridge.funcs = &mtk_dpi_bridge_funcs; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index a4594f8873d5..85be035a209a 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -27,6 +27,7 @@ #include "mtk_crtc.h" #include "mtk_ddp_comp.h" +#include "mtk_disp_drv.h" #include "mtk_drm_drv.h" #include "mtk_gem.h" @@ -820,12 +821,235 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = { { } }; +static int mtk_drm_of_get_ddp_comp_type(struct device_node *node, enum mtk_ddp_comp_type *ctype) +{ + const struct of_device_id *of_id = of_match_node(mtk_ddp_comp_dt_ids, node); + + if (!of_id) + return -EINVAL; + + *ctype = (enum mtk_ddp_comp_type)((uintptr_t)of_id->data); + + return 0; +} + +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); + + /* + * 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; +} + +static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node *node, + struct mtk_mmsys_driver_data *data) +{ + struct device_node *ep_node; + struct of_endpoint of_ep; + bool output_present[MAX_CRTC] = { false }; + int ret; + + for_each_endpoint_of_node(node, ep_node) { + ret = of_graph_parse_endpoint(ep_node, &of_ep); + if (ret) { + dev_err_probe(dev, ret, "Cannot parse endpoint\n"); + break; + } + + if (of_ep.id >= MAX_CRTC) { + ret = dev_err_probe(dev, -EINVAL, + "Invalid endpoint%u number\n", of_ep.port); + break; + } + + output_present[of_ep.id] = true; + } + + 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; + } + + 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; + } + + if (output_present[CRTC_THIRD]) { + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_THIRD, + &data->third_path, &data->third_len); + if (ret && ret != -ENODEV) + return ret; + } + + return 0; +} + static int mtk_drm_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *phandle = dev->parent->of_node; const struct of_device_id *of_id; struct mtk_drm_private *private; + struct mtk_mmsys_driver_data *mtk_drm_data; struct device_node *node; struct component_match *match = NULL; struct platform_device *ovl_adaptor; @@ -846,7 +1070,27 @@ static int mtk_drm_probe(struct platform_device *pdev) if (!of_id) return -ENODEV; - private->data = of_id->data; + mtk_drm_data = (struct mtk_mmsys_driver_data *)of_id->data; + if (!mtk_drm_data) + return -EINVAL; + + /* Try to build the display pipeline from devicetree graphs */ + if (of_graph_is_present(phandle)) { + dev_dbg(dev, "Building display pipeline for MMSYS %u\n", + mtk_drm_data->mmsys_id); + private->data = devm_kmemdup(dev, mtk_drm_data, + sizeof(*mtk_drm_data), GFP_KERNEL); + if (!private->data) + return -ENOMEM; + + ret = mtk_drm_of_ddp_path_build(dev, phandle, private->data); + if (ret) + return ret; + } else { + /* No devicetree graphs support: go with hardcoded paths if present */ + dev_dbg(dev, "Using hardcoded paths for MMSYS %u\n", mtk_drm_data->mmsys_id); + private->data = mtk_drm_data; + }; private->all_drm_private = devm_kmalloc_array(dev, private->data->mmsys_dev_num, sizeof(*private->all_drm_private), @@ -868,12 +1112,11 @@ static int mtk_drm_probe(struct platform_device *pdev) /* Iterate over sibling DISP function blocks */ for_each_child_of_node(phandle->parent, node) { - const struct of_device_id *of_id; enum mtk_ddp_comp_type comp_type; int comp_id; - of_id = of_match_node(mtk_ddp_comp_dt_ids, node); - if (!of_id) + ret = mtk_drm_of_get_ddp_comp_type(node, &comp_type); + if (ret) continue; if (!of_device_is_available(node)) { @@ -882,8 +1125,6 @@ static int mtk_drm_probe(struct platform_device *pdev) continue; } - comp_type = (enum mtk_ddp_comp_type)(uintptr_t)of_id->data; - if (comp_type == MTK_DISP_MUTEX) { int id; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index ce897984de51..675cdc90a440 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -63,7 +63,7 @@ struct mtk_drm_private { struct device *mmsys_dev; struct device_node *comp_node[DDP_COMPONENT_DRM_ID_MAX]; struct mtk_ddp_comp ddp_comp[DDP_COMPONENT_DRM_ID_MAX]; - const struct mtk_mmsys_driver_data *data; + struct mtk_mmsys_driver_data *data; struct drm_atomic_state *suspend_state; unsigned int mbox_index; struct mtk_drm_private **all_drm_private; diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index eeec641cab60..33ceeb8d6925 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -988,9 +988,17 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host, dsi->lanes = device->lanes; dsi->format = device->format; dsi->mode_flags = device->mode_flags; - dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); - if (IS_ERR(dsi->next_bridge)) - return PTR_ERR(dsi->next_bridge); + dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0); + if (IS_ERR(dsi->next_bridge)) { + ret = PTR_ERR(dsi->next_bridge); + if (ret == -EPROBE_DEFER) + return ret; + + /* Old devicetree has only one endpoint */ + dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); + if (IS_ERR(dsi->next_bridge)) + return PTR_ERR(dsi->next_bridge); + } drm_bridge_add(&dsi->bridge);