Hi Angelo, On Thu May 16, 2024 at 10:11 AM CEST, AngeloGioacchino Del Regno wrote: > 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. paths might be optional, see comment in mtk_drm_kms_init(). But with this patch, you'll get an -EINVAL with a disabled path. See my proposals how to fix that below. With these changes and the following two patches I was able to get DisplayPort working on vdosys1. vdosys0 wasn't used at all. https://lore.kernel.org/r/20240516145824.1669263-1-mwalle@xxxxxxxxxx/ https://lore.kernel.org/r/20240517093024.1702750-1-mwalle@xxxxxxxxxx/ I've already successfully tested a former version with DSI output on vdosys0. Thanks for working on this! > +/** > + * 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_err(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 might not be disabled. In that case, don't check the last * entry but just report the missing device. */ 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); > + of_node_put(ep_node); > + 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) > + 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) 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) likewise > + 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) likewise > + return ret; > + } > + > + return 0; > +} -michael
Attachment:
signature.asc
Description: PGP signature
