| Message ID | 20240618101726.110416-1-angelogioacchino.delregno@collabora.com |
|---|---|
| Headers | show |
| Series | drm/mediatek: Add support for OF graphs | expand |
On Tue Jun 18, 2024 at 12:17 PM CEST, AngeloGioacchino Del Regno wrote: > The display IPs in MediaTek SoCs are *VERY* flexible and those support > being interconnected with different instances of DDP IPs (for example, > merge0 or merge1) and/or with different DDP IPs (for example, rdma can > be connected with either color, dpi, dsi, merge, etc), forming a full > Display Data Path that ends with an actual display. > > This series was born because of an issue that I've found while enabling > support for MT8195/MT8395 boards with DSI output as main display: the > current mtk_drm_route variations would not work as currently, the driver > hardcodes a display path for Chromebooks, which have a DisplayPort panel > with DSC support, instead of a DSI panel without DSC support. > > There are other reasons for which I wrote this series, and I find that > hardcoding those paths - when a HW path is clearly board-specific - is > highly suboptimal. Also, let's not forget about keeping this driver from > becoming a huge list of paths for each combination of SoC->board->disp > and... this and that. > > For more information, please look at the commit description for each of > the commits included in this series. > > This series is essential to enable support for the MT8195/MT8395 EVK, > Kontron i1200, Radxa NIO-12L and, mainly, for non-Chromebook boards > and Chromebooks to co-exist without conflicts. > > Besides, this is also a valid option for MT8188 Chromebooks which might > have different DSI-or-eDP displays depending on the model (as far as I > can see from the mtk_drm_route attempt for this SoC that is already > present in this driver). > > This series was tested on MT8195 Cherry Tomato and on MT8395 Radxa > NIO-12L with both hardcoded paths, OF graph support and partially > hardcoded paths, and pure OF graph support including pipelines that > require OVL_ADAPTOR support. > > AngeloGioacchino Del Regno (3): > dt-bindings: display: mediatek: Add OF graph support for board path > dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path > drm/mediatek: Implement OF graphs support for display paths Thanks! Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 -michael
Hi, On 6/18/24 18:17, 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> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Il 18/06/24 12:17, AngeloGioacchino Del Regno ha scritto: > Changes in v8: > - Rebased on next-20240617 > - Changed to allow probing a VDO with no available display outputs > Hello CK, At the time of writing, this series was well reviewed and tested by multiple people on multiple SoCs and boards. We've got a bunch of series that are waiting for this to get upstreamed, including the addition of support for MT8365-EVK (already on mailing lists), MT8395 Radxa NIO 12L, MT8395 Kontron SBC i1200 (not on mailing lists yet, waiting for this to get merged), other than some other conversion commits for other MediaTek DTs from myself. As for the MT8195/NIO12L commits, I'm planning to send them on the lists tomorrow, along with some code to properly support devicetree overlays (DTBO) generation for MediaTek boards. Alexandre tested it on MT8365-EVK; Michael tested on Kontron SBC-i1200; I tested on Radxa NIO-12L, Cherry Tomato Chromebook, MT6795 Sony Xperia M5 (dsi video panel) smartphone and MT8192 Asurada Chromebook. So, is there anything else to address on this, or can we proceed? Many thanks, Angelo > Changes in v7: > - Fix typo in patch 3/3 > > Changes in v6: > - Added EPROBE_DEFER check to fix dsi/dpi false positive DT fallback case > - Dropped refcount of ep_out in mtk_drm_of_get_ddp_ep_cid() > - Fixed double refcount drop during path building > - Removed failure upon finding a DT-disabled path as requested > - Tested again on MT8195, MT8395 boards > > Changes in v5: > - Fixed commit [2/3], changed allOf -> anyOf to get the > intended allowance in the binding > > Changes in v4: > - Fixed a typo that caused pure OF graphs pipelines multiple > concurrent outputs to not get correctly parsed (port->id); > - Added OVL_ADAPTOR support for OF graph specified pipelines; > - Now tested with fully OF Graph specified pipelines on MT8195 > Chromebooks and MT8395 boards; > - Rebased on next-20240516 > > Changes in v3: > - Rebased on next-20240502 because of renames in mediatek-drm > > Changes in v2: > - Fixed wrong `required` block indentation in commit [2/3] > > > The display IPs in MediaTek SoCs are *VERY* flexible and those support > being interconnected with different instances of DDP IPs (for example, > merge0 or merge1) and/or with different DDP IPs (for example, rdma can > be connected with either color, dpi, dsi, merge, etc), forming a full > Display Data Path that ends with an actual display. > > This series was born because of an issue that I've found while enabling > support for MT8195/MT8395 boards with DSI output as main display: the > current mtk_drm_route variations would not work as currently, the driver > hardcodes a display path for Chromebooks, which have a DisplayPort panel > with DSC support, instead of a DSI panel without DSC support. > > There are other reasons for which I wrote this series, and I find that > hardcoding those paths - when a HW path is clearly board-specific - is > highly suboptimal. Also, let's not forget about keeping this driver from > becoming a huge list of paths for each combination of SoC->board->disp > and... this and that. > > For more information, please look at the commit description for each of > the commits included in this series. > > This series is essential to enable support for the MT8195/MT8395 EVK, > Kontron i1200, Radxa NIO-12L and, mainly, for non-Chromebook boards > and Chromebooks to co-exist without conflicts. > > Besides, this is also a valid option for MT8188 Chromebooks which might > have different DSI-or-eDP displays depending on the model (as far as I > can see from the mtk_drm_route attempt for this SoC that is already > present in this driver). > > This series was tested on MT8195 Cherry Tomato and on MT8395 Radxa > NIO-12L with both hardcoded paths, OF graph support and partially > hardcoded paths, and pure OF graph support including pipelines that > require OVL_ADAPTOR support. > > AngeloGioacchino Del Regno (3): > dt-bindings: display: mediatek: Add OF graph support for board path > dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path > drm/mediatek: Implement OF graphs support for display paths > > .../bindings/arm/mediatek/mediatek,mmsys.yaml | 28 ++ > .../display/mediatek/mediatek,aal.yaml | 40 +++ > .../display/mediatek/mediatek,ccorr.yaml | 21 ++ > .../display/mediatek/mediatek,color.yaml | 22 ++ > .../display/mediatek/mediatek,dither.yaml | 22 ++ > .../display/mediatek/mediatek,dpi.yaml | 25 +- > .../display/mediatek/mediatek,dsc.yaml | 24 ++ > .../display/mediatek/mediatek,dsi.yaml | 27 +- > .../display/mediatek/mediatek,ethdr.yaml | 22 ++ > .../display/mediatek/mediatek,gamma.yaml | 19 ++ > .../display/mediatek/mediatek,merge.yaml | 23 ++ > .../display/mediatek/mediatek,od.yaml | 22 ++ > .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++ > .../display/mediatek/mediatek,ovl.yaml | 22 ++ > .../display/mediatek/mediatek,postmask.yaml | 21 ++ > .../display/mediatek/mediatek,rdma.yaml | 22 ++ > .../display/mediatek/mediatek,ufoe.yaml | 21 ++ > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 + > .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 40 ++- > drivers/gpu/drm/mediatek/mtk_dpi.c | 21 +- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 291 ++++++++++++++++-- > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +- > drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +- > 23 files changed, 731 insertions(+), 41 deletions(-) >
Il 19/06/24 12:56, AngeloGioacchino Del Regno ha scritto: > Il 18/06/24 12:17, AngeloGioacchino Del Regno ha scritto: >> Changes in v8: >> - Rebased on next-20240617 >> - Changed to allow probing a VDO with no available display outputs >> > > Hello CK, > > At the time of writing, this series was well reviewed and tested by multiple people > on multiple SoCs and boards. > > We've got a bunch of series that are waiting for this to get upstreamed, including > the addition of support for MT8365-EVK (already on mailing lists), MT8395 Radxa > NIO 12L, MT8395 Kontron SBC i1200 (not on mailing lists yet, waiting for this to > get merged), other than some other conversion commits for other MediaTek DTs from > myself. > > As for the MT8195/NIO12L commits, I'm planning to send them on the lists tomorrow, > along with some code to properly support devicetree overlays (DTBO) generation for > MediaTek boards. > > Alexandre tested it on MT8365-EVK; > Michael tested on Kontron SBC-i1200; > I tested on Radxa NIO-12L, Cherry Tomato Chromebook, MT6795 Sony Xperia > M5 (dsi video panel) smartphone and MT8192 Asurada Chromebook. > > So, is there anything else to address on this, or can we proceed? > Gentle ping Regards, Angelo > Many thanks, > Angelo > >> Changes in v7: >> - Fix typo in patch 3/3 >> >> Changes in v6: >> - Added EPROBE_DEFER check to fix dsi/dpi false positive DT fallback case >> - Dropped refcount of ep_out in mtk_drm_of_get_ddp_ep_cid() >> - Fixed double refcount drop during path building >> - Removed failure upon finding a DT-disabled path as requested >> - Tested again on MT8195, MT8395 boards >> >> Changes in v5: >> - Fixed commit [2/3], changed allOf -> anyOf to get the >> intended allowance in the binding >> >> Changes in v4: >> - Fixed a typo that caused pure OF graphs pipelines multiple >> concurrent outputs to not get correctly parsed (port->id); >> - Added OVL_ADAPTOR support for OF graph specified pipelines; >> - Now tested with fully OF Graph specified pipelines on MT8195 >> Chromebooks and MT8395 boards; >> - Rebased on next-20240516 >> >> Changes in v3: >> - Rebased on next-20240502 because of renames in mediatek-drm >> >> Changes in v2: >> - Fixed wrong `required` block indentation in commit [2/3] >> >> >> The display IPs in MediaTek SoCs are *VERY* flexible and those support >> being interconnected with different instances of DDP IPs (for example, >> merge0 or merge1) and/or with different DDP IPs (for example, rdma can >> be connected with either color, dpi, dsi, merge, etc), forming a full >> Display Data Path that ends with an actual display. >> >> This series was born because of an issue that I've found while enabling >> support for MT8195/MT8395 boards with DSI output as main display: the >> current mtk_drm_route variations would not work as currently, the driver >> hardcodes a display path for Chromebooks, which have a DisplayPort panel >> with DSC support, instead of a DSI panel without DSC support. >> >> There are other reasons for which I wrote this series, and I find that >> hardcoding those paths - when a HW path is clearly board-specific - is >> highly suboptimal. Also, let's not forget about keeping this driver from >> becoming a huge list of paths for each combination of SoC->board->disp >> and... this and that. >> >> For more information, please look at the commit description for each of >> the commits included in this series. >> >> This series is essential to enable support for the MT8195/MT8395 EVK, >> Kontron i1200, Radxa NIO-12L and, mainly, for non-Chromebook boards >> and Chromebooks to co-exist without conflicts. >> >> Besides, this is also a valid option for MT8188 Chromebooks which might >> have different DSI-or-eDP displays depending on the model (as far as I >> can see from the mtk_drm_route attempt for this SoC that is already >> present in this driver). >> >> This series was tested on MT8195 Cherry Tomato and on MT8395 Radxa >> NIO-12L with both hardcoded paths, OF graph support and partially >> hardcoded paths, and pure OF graph support including pipelines that >> require OVL_ADAPTOR support. >> >> AngeloGioacchino Del Regno (3): >> dt-bindings: display: mediatek: Add OF graph support for board path >> dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path >> drm/mediatek: Implement OF graphs support for display paths >> >> .../bindings/arm/mediatek/mediatek,mmsys.yaml | 28 ++ >> .../display/mediatek/mediatek,aal.yaml | 40 +++ >> .../display/mediatek/mediatek,ccorr.yaml | 21 ++ >> .../display/mediatek/mediatek,color.yaml | 22 ++ >> .../display/mediatek/mediatek,dither.yaml | 22 ++ >> .../display/mediatek/mediatek,dpi.yaml | 25 +- >> .../display/mediatek/mediatek,dsc.yaml | 24 ++ >> .../display/mediatek/mediatek,dsi.yaml | 27 +- >> .../display/mediatek/mediatek,ethdr.yaml | 22 ++ >> .../display/mediatek/mediatek,gamma.yaml | 19 ++ >> .../display/mediatek/mediatek,merge.yaml | 23 ++ >> .../display/mediatek/mediatek,od.yaml | 22 ++ >> .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++ >> .../display/mediatek/mediatek,ovl.yaml | 22 ++ >> .../display/mediatek/mediatek,postmask.yaml | 21 ++ >> .../display/mediatek/mediatek,rdma.yaml | 22 ++ >> .../display/mediatek/mediatek,ufoe.yaml | 21 ++ >> drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 + >> .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 40 ++- >> drivers/gpu/drm/mediatek/mtk_dpi.c | 21 +- >> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 291 ++++++++++++++++-- >> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +- >> drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +- >> 23 files changed, 731 insertions(+), 41 deletions(-) >> > >
Hi, > > We've got a bunch of series that are waiting for this to get upstreamed, including > > the addition of support for MT8365-EVK (already on mailing lists), MT8395 Radxa > > NIO 12L, MT8395 Kontron SBC i1200 (not on mailing lists yet, waiting for this to > > get merged), other than some other conversion commits for other MediaTek DTs from > > myself. Yes this is the missing piece to finally get DisplayPort output working on our board. -michael
Hi, Angelo: On Tue, 2024-06-18 at 12:17 +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> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- [snip] > +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); This looks not an error. > + 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; > @@ -824,7 +1048,31 @@ 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; > + > + private->data = kmemdup(mtk_drm_data, sizeof(*mtk_drm_data), GFP_KERNEL); This is redundant. You would assign private->data below. > + if (!private->data) > + return -ENOMEM; > + > + /* 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), > @@ -846,12 +1094,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)) { > @@ -860,8 +1107,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; > > @@ -890,22 +1135,24 @@ static int mtk_drm_probe(struct platform_device *pdev) > * blocks have separate component platform drivers and initialize their own > * DDP component structure. The others are initialized here. > */ > - if (comp_type == MTK_DISP_AAL || > - comp_type == MTK_DISP_CCORR || > - comp_type == MTK_DISP_COLOR || > - comp_type == MTK_DISP_GAMMA || > - comp_type == MTK_DISP_MERGE || > - comp_type == MTK_DISP_OVL || > - comp_type == MTK_DISP_OVL_2L || > - comp_type == MTK_DISP_OVL_ADAPTOR || > - comp_type == MTK_DISP_RDMA || > - comp_type == MTK_DP_INTF || > - comp_type == MTK_DPI || > - comp_type == MTK_DSI) { > - dev_info(dev, "Adding component match for %pOF\n", > - node); > - drm_of_component_match_add(dev, &match, component_compare_of, > - node); > + switch (comp_type) { > + default: > + break; > + case MTK_DISP_AAL: > + case MTK_DISP_CCORR: > + case MTK_DISP_COLOR: > + case MTK_DISP_GAMMA: > + case MTK_DISP_MERGE: > + case MTK_DISP_OVL: > + case MTK_DISP_OVL_2L: > + case MTK_DISP_OVL_ADAPTOR: > + case MTK_DISP_RDMA: > + case MTK_DP_INTF: > + case MTK_DPI: > + case MTK_DSI: > + dev_info(dev, "Adding component match for %pOF\n", node); > + drm_of_component_match_add(dev, &match, component_compare_of, node); > + break; This modification seems not related to OF graphs support. If you need this, separate this to another patch. > } > > ret = mtk_ddp_comp_init(node, &private->ddp_comp[comp_id], comp_id); > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > index 78d698ede1bf..7e54d86e25a3 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > @@ -59,7 +59,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 c255559cc56e..880ea37937da 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -904,9 +904,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); >
Hi, On Thu Jul 4, 2024 at 10:29 AM CEST, AngeloGioacchino Del Regno wrote: > Il 19/06/24 12:56, AngeloGioacchino Del Regno ha scritto: > > Il 18/06/24 12:17, AngeloGioacchino Del Regno ha scritto: > >> Changes in v8: > >> - Rebased on next-20240617 > >> - Changed to allow probing a VDO with no available display outputs > >> > > > > Hello CK, > > > > At the time of writing, this series was well reviewed and tested by multiple people > > on multiple SoCs and boards. > > > > We've got a bunch of series that are waiting for this to get upstreamed, including > > the addition of support for MT8365-EVK (already on mailing lists), MT8395 Radxa > > NIO 12L, MT8395 Kontron SBC i1200 (not on mailing lists yet, waiting for this to > > get merged), other than some other conversion commits for other MediaTek DTs from > > myself. > > > > As for the MT8195/NIO12L commits, I'm planning to send them on the lists tomorrow, > > along with some code to properly support devicetree overlays (DTBO) generation for > > MediaTek boards. > > > > Alexandre tested it on MT8365-EVK; > > Michael tested on Kontron SBC-i1200; > > I tested on Radxa NIO-12L, Cherry Tomato Chromebook, MT6795 Sony Xperia > > M5 (dsi video panel) smartphone and MT8192 Asurada Chromebook. > > > > So, is there anything else to address on this, or can we proceed? > > > > Gentle ping Any news here? Angelo, maybe you can just resend the patches? Because there is already a new kernel release, I'm not sure how this is handled. -michael
Il 08/08/24 05:48, CK Hu (胡俊光) ha scritto: > Hi, Angelo: > > On Tue, 2024-06-18 at 12:17 +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> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- > > [snip] > >> +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); > > This looks not an error. > Thanks for catching that, I used that line for debugging and forgot to change it back to dev_dbg(). Will fix. >> + 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; >> @@ -824,7 +1048,31 @@ 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; >> + >> + private->data = kmemdup(mtk_drm_data, sizeof(*mtk_drm_data), GFP_KERNEL); > > This is redundant. You would assign private->data below. > Right. Will remove. >> + if (!private->data) >> + return -ENOMEM; >> + >> + /* 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), >> @@ -846,12 +1094,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)) { >> @@ -860,8 +1107,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; >> >> @@ -890,22 +1135,24 @@ static int mtk_drm_probe(struct platform_device *pdev) >> * blocks have separate component platform drivers and initialize their own >> * DDP component structure. The others are initialized here. >> */ >> - if (comp_type == MTK_DISP_AAL || >> - comp_type == MTK_DISP_CCORR || >> - comp_type == MTK_DISP_COLOR || >> - comp_type == MTK_DISP_GAMMA || >> - comp_type == MTK_DISP_MERGE || >> - comp_type == MTK_DISP_OVL || >> - comp_type == MTK_DISP_OVL_2L || >> - comp_type == MTK_DISP_OVL_ADAPTOR || >> - comp_type == MTK_DISP_RDMA || >> - comp_type == MTK_DP_INTF || >> - comp_type == MTK_DPI || >> - comp_type == MTK_DSI) { >> - dev_info(dev, "Adding component match for %pOF\n", >> - node); >> - drm_of_component_match_add(dev, &match, component_compare_of, >> - node); >> + switch (comp_type) { >> + default: >> + break; >> + case MTK_DISP_AAL: >> + case MTK_DISP_CCORR: >> + case MTK_DISP_COLOR: >> + case MTK_DISP_GAMMA: >> + case MTK_DISP_MERGE: >> + case MTK_DISP_OVL: >> + case MTK_DISP_OVL_2L: >> + case MTK_DISP_OVL_ADAPTOR: >> + case MTK_DISP_RDMA: >> + case MTK_DP_INTF: >> + case MTK_DPI: >> + case MTK_DSI: >> + dev_info(dev, "Adding component match for %pOF\n", node); >> + drm_of_component_match_add(dev, &match, component_compare_of, node); >> + break; > > This modification seems not related to OF graphs support. If you need this, separate this to another patch. > Will do for v9. Thanks, Angelo >> } >> >> ret = mtk_ddp_comp_init(node, &private->ddp_comp[comp_id], comp_id); >> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h >> index 78d698ede1bf..7e54d86e25a3 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h >> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h >> @@ -59,7 +59,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 c255559cc56e..880ea37937da 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c >> @@ -904,9 +904,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); >>