Message ID | 20200728111800.77641-2-frank-w@public-files.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | make hdmi work on bananapi-r2 | expand |
Hi, Frank: Frank Wunderlich <frank-w@public-files.de> 於 2020年7月28日 週二 下午7:18寫道: > > From: Bibby Hsieh <bibby.hsieh@mediatek.com> > > We can select output component by decive node port. > Main path default output component is DSI. > External path default output component is DPI. > > without this Patch i get this warning: > > WARNING: CPU: 3 PID: 70 at drivers/gpu/drm/drm_mode_config.c:621 drm_mode_config_validate+0x1d8/0x258 > > Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com> > > added small fixes for warnings > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > Tested-by: Frank Wunderlich <frank-w@public-files.de> > --- [snip] > > -static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = { > +static enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = { > DDP_COMPONENT_OVL1, > DDP_COMPONENT_COLOR1, > DDP_COMPONENT_GAMMA, > @@ -459,6 +466,7 @@ static int mtk_drm_probe(struct platform_device *pdev) > > /* Iterate over sibling DISP function blocks */ > for_each_child_of_node(phandle->parent, node) { > + struct device_node *port, *ep, *remote; > const struct of_device_id *of_id; > enum mtk_ddp_comp_type comp_type; > int comp_id; > @@ -522,6 +530,32 @@ static int mtk_drm_probe(struct platform_device *pdev) > > private->ddp_comp[comp_id] = comp; > } > + > + if (comp_type != MTK_DSI && comp_type != MTK_DPI) { > + port = of_graph_get_port_by_id(node, 0); > + if (!port) > + continue; > + ep = of_get_child_by_name(port, "endpoint"); > + of_node_put(port); > + if (!ep) > + continue; > + remote = of_graph_get_remote_port_parent(ep); > + of_node_put(ep); > + if (!remote) > + continue; > + of_id = of_match_node(mtk_ddp_comp_dt_ids, remote); > + if (!of_id) > + continue; > + comp_type = (enum mtk_ddp_comp_type)of_id->data; > + for (i = 0; i < private->data->main_len - 1; i++) > + if (private->data->main_path[i] == comp_id) > + private->data->main_path[i + 1] = > + mtk_ddp_comp_get_id(node, comp_type); > + for (i = 0; i < private->data->ext_len - 1; i++) > + if (private->data->ext_path[i] == comp_id) > + private->data->ext_path[i + 1] = > + mtk_ddp_comp_get_id(node, comp_type); > + } The port property is not defined in binding document [1], so define it in binding document first. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt?h=v5.8-rc7 Regards, Chun-Kuang. > } > > if (!private->mutex_node) { > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > index b5be63e53176..7fcaab648bf1 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h > @@ -21,9 +21,9 @@ struct drm_property; > struct regmap; > > struct mtk_mmsys_driver_data { > - const enum mtk_ddp_comp_id *main_path; > + enum mtk_ddp_comp_id *main_path; > unsigned int main_len; > - const enum mtk_ddp_comp_id *ext_path; > + enum mtk_ddp_comp_id *ext_path; > unsigned int ext_len; > const enum mtk_ddp_comp_id *third_path; > unsigned int third_len; > -- > 2.25.1 >
> Gesendet: Sonntag, 02. August 2020 um 02:03 Uhr > Von: "Chun-Kuang Hu" <chunkuang.hu@kernel.org> > An: "Frank Wunderlich" <frank-w@public-files.de> > Cc: "Chun-Kuang Hu" <chunkuang.hu@kernel.org>, "Philipp Zabel" <p.zabel@pengutronix.de>, "David Airlie" <airlied@linux.ie>, "linux-kernel" <linux-kernel@vger.kernel.org>, "DRI Development" <dri-devel@lists.freedesktop.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>, "Daniel Vetter" <daniel@ffwll.ch>, "Matthias Brugger" <matthias.bgg@gmail.com>, "Bibby Hsieh" <bibby.hsieh@mediatek.com>, "Linux ARM" <linux-arm-kernel@lists.infradead.org> > Betreff: Re: [PATCH v2 1/5] drm/mediatek: config component output by device node port > > + > > + if (comp_type != MTK_DSI && comp_type != MTK_DPI) { > > + port = of_graph_get_port_by_id(node, 0); > > + if (!port) > > + continue; > > + ep = of_get_child_by_name(port, "endpoint"); > > + of_node_put(port); > > + if (!ep) > > + continue; > > + remote = of_graph_get_remote_port_parent(ep); > > + of_node_put(ep); > > + if (!remote) > > + continue; > > + of_id = of_match_node(mtk_ddp_comp_dt_ids, remote); > > + if (!of_id) > > + continue; > > + comp_type = (enum mtk_ddp_comp_type)of_id->data; > > + for (i = 0; i < private->data->main_len - 1; i++) > > + if (private->data->main_path[i] == comp_id) > > + private->data->main_path[i + 1] = > > + mtk_ddp_comp_get_id(node, comp_type); > > + for (i = 0; i < private->data->ext_len - 1; i++) > > + if (private->data->ext_path[i] == comp_id) > > + private->data->ext_path[i + 1] = > > + mtk_ddp_comp_get_id(node, comp_type); > > + } > > The port property is not defined in binding document [1], so define it > in binding document first. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt?h=v5.8-rc7 hi, would be the following enough for describing the ports? Port binding ============= some nodes like - connector (e.g. hdmi-connector) - bls (mediatek,mt7623-disp-pwm) - hdmix (mediatek,mt7623-hdmi) can have port bindings to connect each other. Each port can have only 1 endpoint more detail about ports/endpoints in ../../media/video-interfaces.txt hdmi-connector is described here: ../connector/hdmi-connector.txt example: connector { compatible = "hdmi-connector"; port { hdmi_connector_in: endpoint { remote-endpoint = <&hdmi0_out>; }; }; }; &bls { status = "okay"; port { bls_out: endpoint { remote-endpoint = <&dpi0_in>; }; }; }; &dpi0 { status = "okay"; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; dpi0_out: endpoint { remote-endpoint = <&hdmi0_in>; }; }; port@1 { reg = <1>; dpi0_in: endpoint { remote-endpoint = <&bls_out>; }; }; }; }; &hdmi0 { ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; hdmi0_in: endpoint { remote-endpoint = <&dpi0_out>; }; }; port@1 { reg = <1>; hdmi0_out: endpoint { remote-endpoint = <&hdmi_connector_in>; }; }; }; }; regards Frank
Hi, Frank: Frank Wunderlich <frank-w@public-files.de> 於 2020年8月2日 週日 下午4:06寫道: > > > Gesendet: Sonntag, 02. August 2020 um 02:03 Uhr > > Von: "Chun-Kuang Hu" <chunkuang.hu@kernel.org> > > An: "Frank Wunderlich" <frank-w@public-files.de> > > Cc: "Chun-Kuang Hu" <chunkuang.hu@kernel.org>, "Philipp Zabel" <p.zabel@pengutronix.de>, "David Airlie" <airlied@linux.ie>, "linux-kernel" <linux-kernel@vger.kernel.org>, "DRI Development" <dri-devel@lists.freedesktop.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>, "Daniel Vetter" <daniel@ffwll.ch>, "Matthias Brugger" <matthias.bgg@gmail.com>, "Bibby Hsieh" <bibby.hsieh@mediatek.com>, "Linux ARM" <linux-arm-kernel@lists.infradead.org> > > Betreff: Re: [PATCH v2 1/5] drm/mediatek: config component output by device node port > > > + > > > + if (comp_type != MTK_DSI && comp_type != MTK_DPI) { > > > + port = of_graph_get_port_by_id(node, 0); > > > + if (!port) > > > + continue; > > > + ep = of_get_child_by_name(port, "endpoint"); > > > + of_node_put(port); > > > + if (!ep) > > > + continue; > > > + remote = of_graph_get_remote_port_parent(ep); > > > + of_node_put(ep); > > > + if (!remote) > > > + continue; > > > + of_id = of_match_node(mtk_ddp_comp_dt_ids, remote); > > > + if (!of_id) > > > + continue; > > > + comp_type = (enum mtk_ddp_comp_type)of_id->data; > > > + for (i = 0; i < private->data->main_len - 1; i++) > > > + if (private->data->main_path[i] == comp_id) > > > + private->data->main_path[i + 1] = > > > + mtk_ddp_comp_get_id(node, comp_type); > > > + for (i = 0; i < private->data->ext_len - 1; i++) > > > + if (private->data->ext_path[i] == comp_id) > > > + private->data->ext_path[i + 1] = > > > + mtk_ddp_comp_get_id(node, comp_type); > > > + } > > > > The port property is not defined in binding document [1], so define it > > in binding document first. > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt?h=v5.8-rc7 > > hi, > would be the following enough for describing the ports? > > Port binding > ============= > > some nodes like > > - connector (e.g. hdmi-connector) > - bls (mediatek,mt7623-disp-pwm) > - hdmix (mediatek,mt7623-hdmi) Now I just care about the bls to dpi. So in mediatek,disp.txt, you just need to add a Optional properties - port (input and output), and modify mediatek,dpi.txt for its input port. Regards, Chun-Kuang. > > can have port bindings to connect each other. Each port can have only 1 endpoint > > more detail about ports/endpoints in ../../media/video-interfaces.txt > hdmi-connector is described here: ../connector/hdmi-connector.txt > > example: > > connector { > compatible = "hdmi-connector"; > > port { > hdmi_connector_in: endpoint { > remote-endpoint = <&hdmi0_out>; > }; > }; > }; > > > &bls { > status = "okay"; > > port { > bls_out: endpoint { > remote-endpoint = <&dpi0_in>; > }; > }; > }; > > &dpi0 { > status = "okay"; > > ports { > #address-cells = <1>; > #size-cells = <0>; > port@0 { > reg = <0>; > dpi0_out: endpoint { > remote-endpoint = <&hdmi0_in>; > }; > }; > > port@1 { > reg = <1>; > dpi0_in: endpoint { > remote-endpoint = <&bls_out>; > }; > }; > }; > }; > > &hdmi0 { > > ports { > #address-cells = <1>; > #size-cells = <0>; > port@0 { > reg = <0>; > hdmi0_in: endpoint { > remote-endpoint = <&dpi0_out>; > }; > }; > > port@1 { > reg = <1>; > hdmi0_out: endpoint { > remote-endpoint = <&hdmi_connector_in>; > }; > }; > }; > }; > > regards Frank
Hi > Gesendet: Montag, 03. August 2020 um 01:47 Uhr > Von: "Chun-Kuang Hu" <chunkuang.hu@kernel.org> > Now I just care about the bls to dpi. So in mediatek,disp.txt, you > just need to add a Optional properties - port (input and output), and > modify mediatek,dpi.txt for its input port. you mean something like this is enough: Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt for details. +Optional properties: +- port (input and output) see ../../media/video-interfaces.txt + Examples: Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt: Optional properties: - pinctrl-names: Contain "default" and "sleep". +- port: Input port node with endpoint definition, this can be connected to <chipid>-disp-pwm Example: should i link to pwm/pwm-mtk-disp.txt in doc? regards Frank
Hi, Frank: Frank Wunderlich <frank-w@public-files.de> 於 2020年8月3日 週一 下午6:43寫道: > > Hi > > Gesendet: Montag, 03. August 2020 um 01:47 Uhr > > Von: "Chun-Kuang Hu" <chunkuang.hu@kernel.org> > > > Now I just care about the bls to dpi. So in mediatek,disp.txt, you > > just need to add a Optional properties - port (input and output), and > > modify mediatek,dpi.txt for its input port. > > you mean something like this is enough: > > Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt > > argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > for details. > > +Optional properties: > +- port (input and output) see ../../media/video-interfaces.txt > + > Examples: > > Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt: > > Optional properties: > - pinctrl-names: Contain "default" and "sleep". > +- port: Input port node with endpoint definition, this can be connected to <chipid>-disp-pwm > > Example: > > should i link to pwm/pwm-mtk-disp.txt in doc? > > regards Frank Yes, this is what I mean, but I think it need not output to pmw. But now I have a solution that you need not to modify binding document. Because now mt7623 has a different routing than mt2701, and this patch's approach is to use different port setting in each device tree. My solution is that these two SoC has different compatible string: "mediatek,mt7623-mmsys" and "mediatek,mt2701-mmsys". For now, "mediatek,mt2701-mmsys" has its data as static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = { .main_path = mt2701_mtk_ddp_main, .main_len = ARRAY_SIZE(mt2701_mtk_ddp_main), .ext_path = mt2701_mtk_ddp_ext, .ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext), .shadow_register = true, }; I think you could create a data for "mediatek,mt7623-mmsys" which has a different routing. Regards, Chun-Kuang.
Am 3. August 2020 18:27:02 MESZ schrieb Chun-Kuang Hu <chunkuang.hu@kernel.org>: >But now I have a solution that you need not to modify binding document. >Because now mt7623 has a different routing than mt2701, and this >patch's approach is to use different port setting in each device tree. >My solution is that these two SoC has different compatible string: >"mediatek,mt7623-mmsys" and "mediatek,mt2701-mmsys". For now, >"mediatek,mt2701-mmsys" has its data as > >static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = { >.main_path = mt2701_mtk_ddp_main, >.main_len = ARRAY_SIZE(mt2701_mtk_ddp_main), >.ext_path = mt2701_mtk_ddp_ext, >.ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext), >.shadow_register = true, >}; > >I think you could create a data for "mediatek,mt7623-mmsys" which has >a different routing. The paths are defined as this: static enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = { DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0, DDP_COMPONENT_COLOR0, DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,}; static enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = { DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,}; First thing i notice is that main=dsi and ext=dpi (hdmi). I guess dpi should be main,right? And bls is actually routed to dpi...how about the other components? The 2 loops are not really clear to me (except 1st overwrites mt2701 main-path and second ext_path based on ports/endpoints) but this only applies from bls-dpi-hdmi-connector,not xdma/color or similar. Or should be main-path (or external) only bls and dpi? It looks like it only swappes dpi and dsi in my case. @Ryder Lee can you explain it? regards Frank
Hi, i've printed the mtk_comp_id after the modification-loops... [ 5.480848] main: [ 5.480851] DDP_COMPONENT_OVL0 [ 5.482776] DDP_COMPONENT_RDMA0 [ 5.485827] DDP_COMPONENT_COLOR0 [ 5.488978] DDP_COMPONENT_BLS [ 5.492206] DDP_COMPONENT_DPI0 [ 5.495170] ext: [ 5.498233] DDP_COMPONENT_RDMA1 [ 5.500068] DDP_COMPONENT_DPI0 so only the main-path was replaced with DPI at the end. so currently the DSI is not connected (or i move it to ext). have now added new structs for mt7623 with swapped DPI0/DSI0 and commented out the code from Patch 1...compatible was already mt7623 with 2701 as fallback, so no dts-change needed... now i need to look which changes in dts can be dropped...at least the bls=>dpi, right? regards Frank > Gesendet: Montag, 03. August 2020 um 18:27 Uhr > Von: "Chun-Kuang Hu" <chunkuang.hu@kernel.org> > Yes, this is what I mean, but I think it need not output to pmw. > But now I have a solution that you need not to modify binding document. > Because now mt7623 has a different routing than mt2701, and this > patch's approach is to use different port setting in each device tree. > My solution is that these two SoC has different compatible string: > "mediatek,mt7623-mmsys" and "mediatek,mt2701-mmsys". For now, > "mediatek,mt2701-mmsys" has its data as > > static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = { > .main_path = mt2701_mtk_ddp_main, > .main_len = ARRAY_SIZE(mt2701_mtk_ddp_main), > .ext_path = mt2701_mtk_ddp_ext, > .ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext), > .shadow_register = true, > }; > > I think you could create a data for "mediatek,mt7623-mmsys" which has > a different routing.
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 6bd369434d9d..ce7abf2743d9 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -24,6 +24,13 @@ #include <drm/drm_of.h> #include <drm/drm_probe_helper.h> #include <drm/drm_vblank.h> +#include <linux/component.h> +#include <linux/iommu.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_graph.h> +#include <linux/of_platform.h> +#include <linux/pm_runtime.h> #include "mtk_drm_crtc.h" #include "mtk_drm_ddp.h" @@ -61,7 +68,7 @@ static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = { .atomic_commit = drm_atomic_helper_commit, }; -static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = { +static enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = { DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0, DDP_COMPONENT_COLOR0, @@ -69,12 +76,12 @@ static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = { DDP_COMPONENT_DSI0, }; -static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = { +static enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = { DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0, }; -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = { +static enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = { DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0, DDP_COMPONENT_AAL0, @@ -84,7 +91,7 @@ static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = { DDP_COMPONENT_PWM0, }; -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = { +static enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = { DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1, DDP_COMPONENT_AAL1, @@ -100,7 +107,7 @@ static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = { DDP_COMPONENT_PWM2, }; -static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = { +static enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = { DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0, DDP_COMPONENT_AAL0, @@ -111,7 +118,7 @@ static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = { DDP_COMPONENT_PWM0, }; -static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = { +static enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = { DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1, DDP_COMPONENT_GAMMA, @@ -459,6 +466,7 @@ static int mtk_drm_probe(struct platform_device *pdev) /* Iterate over sibling DISP function blocks */ for_each_child_of_node(phandle->parent, node) { + struct device_node *port, *ep, *remote; const struct of_device_id *of_id; enum mtk_ddp_comp_type comp_type; int comp_id; @@ -522,6 +530,32 @@ static int mtk_drm_probe(struct platform_device *pdev) private->ddp_comp[comp_id] = comp; } + + if (comp_type != MTK_DSI && comp_type != MTK_DPI) { + port = of_graph_get_port_by_id(node, 0); + if (!port) + continue; + ep = of_get_child_by_name(port, "endpoint"); + of_node_put(port); + if (!ep) + continue; + remote = of_graph_get_remote_port_parent(ep); + of_node_put(ep); + if (!remote) + continue; + of_id = of_match_node(mtk_ddp_comp_dt_ids, remote); + if (!of_id) + continue; + comp_type = (enum mtk_ddp_comp_type)of_id->data; + for (i = 0; i < private->data->main_len - 1; i++) + if (private->data->main_path[i] == comp_id) + private->data->main_path[i + 1] = + mtk_ddp_comp_get_id(node, comp_type); + for (i = 0; i < private->data->ext_len - 1; i++) + if (private->data->ext_path[i] == comp_id) + private->data->ext_path[i + 1] = + mtk_ddp_comp_get_id(node, comp_type); + } } if (!private->mutex_node) { diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index b5be63e53176..7fcaab648bf1 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -21,9 +21,9 @@ struct drm_property; struct regmap; struct mtk_mmsys_driver_data { - const enum mtk_ddp_comp_id *main_path; + enum mtk_ddp_comp_id *main_path; unsigned int main_len; - const enum mtk_ddp_comp_id *ext_path; + enum mtk_ddp_comp_id *ext_path; unsigned int ext_len; const enum mtk_ddp_comp_id *third_path; unsigned int third_len;