| Message ID | 20240821031348.6837-2-quic_taozha@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | source filtering for multi-port output | expand |
On 22/08/2024 08:08, Krzysztof Kozlowski wrote: > On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote: >> On 21/08/2024 04:13, Tao Zhang wrote: >>> The is some "magic" hard coded filtering in the replicators, >>> which only passes through trace from a particular "source". Add >>> a new property "filter-src" to label a phandle to the coresight >>> trace source device matching the hard coded filtering for the port. >> >> Minor nit: Please do not use abbreviate "source" in the bindings. >> I am not an expert on other changes below and will leave it to >> Rob/Krzysztof to comment. >> >> Rob, Krzysztof, >> >> We need someway to "link" (add a phandle) from a "port". The patch below >> is extending "standard" port to add a phandle. Please let us know if >> there is a better way. >> >> e.g.: >> >> filters = list of tuples of port, phandle. ? >> >> e.g.: >> >> filters = < 0, <&tpdm_video>, >> 1, <&tpdm_mdss> >> > >> > > Current solution feels like band-aid - what if next time you need some > second filter? Or "wall"? Or whatever? Next property? > > Isn't filter just one endpoint in the graph? > > A <--> filter <--> B To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a multi output ports). For clearer example: A0 <--> .. <--> ..\ p0 / --> Filtered for (A1) <--> B1 A1 <--> .. <--> .. - < L(filters> p1 - --> Filtered for (A2) <--> B2 A2 <--> .. <--> ../ p2 \ --> Unfiltered <--> B0 > Instead of > > A <----through-filter----> B? The problem is we need to know the components in the path from A0 to X through, (Not just A0 and L). And also we need to know "which port (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out of the link "L". So ideally, we need a way to tie p0 -> A1, p1 -> A2. would we need something else in the future ? I don't know for sure. People could design their own things ;-). But this was the first time ever in the last 12yrs since we supported coresight in the kernel. (there is always a first time). Fundamentally, the "ports" cannot have additional properties today. Not sure if there are other usecases (I don't see why). So, we have to manually extend like above, which I think is not nice. Happy to proceed with anything that seems acceptable for you folks. Suzuki > > Best regards, > Krzysztof >
On 22/08/2024 11:34, Suzuki K Poulose wrote: > On 22/08/2024 08:08, Krzysztof Kozlowski wrote: >> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote: >>> On 21/08/2024 04:13, Tao Zhang wrote: >>>> The is some "magic" hard coded filtering in the replicators, >>>> which only passes through trace from a particular "source". Add >>>> a new property "filter-src" to label a phandle to the coresight >>>> trace source device matching the hard coded filtering for the port. >>> >>> Minor nit: Please do not use abbreviate "source" in the bindings. >>> I am not an expert on other changes below and will leave it to >>> Rob/Krzysztof to comment. >>> >>> Rob, Krzysztof, >>> >>> We need someway to "link" (add a phandle) from a "port". The patch below >>> is extending "standard" port to add a phandle. Please let us know if >>> there is a better way. >>> >>> e.g.: >>> >>> filters = list of tuples of port, phandle. ? >>> >>> e.g.: >>> >>> filters = < 0, <&tpdm_video>, >>> 1, <&tpdm_mdss> >>> > >>> >> >> Current solution feels like band-aid - what if next time you need some >> second filter? Or "wall"? Or whatever? Next property? > > > >> >> Isn't filter just one endpoint in the graph? >> >> A <--> filter <--> B > > To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a > multi output ports). > > For clearer example: > > A0 <--> .. <--> ..\ p0 / --> Filtered for (A1) <--> B1 > A1 <--> .. <--> .. - < L(filters> p1 - --> Filtered for (A2) <--> B2 > A2 <--> .. <--> ../ p2 \ --> Unfiltered <--> B0 > > > >> Instead of >> >> A <----through-filter----> B? > > The problem is we need to know the components in the path from A0 to X > through, (Not just A0 and L). And also we need to know "which port (p0 > vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out of the > link "L". > > So ideally, we need a way to tie p0 -> A1, p1 -> A2. > > would we need something else in the future ? I don't know for sure. > People could design their own things ;-). But this was the first time > ever in the last 12yrs since we supported coresight in the kernel. > (there is always a first time). > > Fundamentally, the "ports" cannot have additional properties today. > Not sure if there are other usecases (I don't see why). So, we have > to manually extend like above, which I think is not nice. Replying to the other thread [0], made me realize that the above is not true. Indeed it is possible to add properties for endpoints, e.g: e.g.: media/video-interfaces.yaml So extending the endpoint node is indeed acceptable (unlike I thought). May be the we it is achieved in this patch is making it look otherwise. Suzuki [0] https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com > > Happy to proceed with anything that seems acceptable for you folks. > > Suzuki > > > >> >> Best regards, >> Krzysztof >> >
Krzysztof On 22/08/2024 12:50, Suzuki K Poulose wrote: > On 22/08/2024 11:34, Suzuki K Poulose wrote: >> On 22/08/2024 08:08, Krzysztof Kozlowski wrote: >>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote: >>>> On 21/08/2024 04:13, Tao Zhang wrote: >>>>> The is some "magic" hard coded filtering in the replicators, >>>>> which only passes through trace from a particular "source". Add >>>>> a new property "filter-src" to label a phandle to the coresight >>>>> trace source device matching the hard coded filtering for the port. >>>> >>>> Minor nit: Please do not use abbreviate "source" in the bindings. >>>> I am not an expert on other changes below and will leave it to >>>> Rob/Krzysztof to comment. >>>> >>>> Rob, Krzysztof, >>>> >>>> We need someway to "link" (add a phandle) from a "port". The patch >>>> below >>>> is extending "standard" port to add a phandle. Please let us know if >>>> there is a better way. >>>> >>>> e.g.: >>>> >>>> filters = list of tuples of port, phandle. ? >>>> >>>> e.g.: >>>> >>>> filters = < 0, <&tpdm_video>, >>>> 1, <&tpdm_mdss> >>>> > >>>> >>> >>> Current solution feels like band-aid - what if next time you need some >>> second filter? Or "wall"? Or whatever? Next property? >> >> >> >>> >>> Isn't filter just one endpoint in the graph? >>> >>> A <--> filter <--> B >> >> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a >> multi output ports). >> >> For clearer example: >> >> A0 <--> .. <--> ..\ p0 / --> Filtered for (A1) <--> B1 >> A1 <--> .. <--> .. - < L(filters> p1 - --> Filtered for (A2) <--> B2 >> A2 <--> .. <--> ../ p2 \ --> Unfiltered <--> B0 >> >> >> >>> Instead of >>> >>> A <----through-filter----> B? >> >> The problem is we need to know the components in the path from A0 to X >> through, (Not just A0 and L). And also we need to know "which port (p0 >> vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out of the >> link "L". >> >> So ideally, we need a way to tie p0 -> A1, p1 -> A2. >> >> would we need something else in the future ? I don't know for sure. >> People could design their own things ;-). But this was the first time >> ever in the last 12yrs since we supported coresight in the kernel. >> (there is always a first time). >> >> Fundamentally, the "ports" cannot have additional properties today. >> Not sure if there are other usecases (I don't see why). So, we have >> to manually extend like above, which I think is not nice. > > Replying to the other thread [0], made me realize that the above is not > true. Indeed it is possible to add properties for endpoints, e.g: > > e.g.: media/video-interfaces.yaml > > So extending the endpoint node is indeed acceptable (unlike I thought). > May be the we it is achieved in this patch is making it look otherwise. > > Suzuki > [0] https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com Please could you let us know if it is acceptable to extend "endpoint" node to have an optional property ? Suzuki > > > >> >> Happy to proceed with anything that seems acceptable for you folks. >> >> Suzuki >> >> >> >>> >>> Best regards, >>> Krzysztof >>> >> >
On 10/9/2024 6:52 PM, Suzuki K Poulose wrote: > Krzysztof > > On 22/08/2024 12:50, Suzuki K Poulose wrote: >> On 22/08/2024 11:34, Suzuki K Poulose wrote: >>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote: >>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote: >>>>> On 21/08/2024 04:13, Tao Zhang wrote: >>>>>> The is some "magic" hard coded filtering in the replicators, >>>>>> which only passes through trace from a particular "source". Add >>>>>> a new property "filter-src" to label a phandle to the coresight >>>>>> trace source device matching the hard coded filtering for the port. >>>>> >>>>> Minor nit: Please do not use abbreviate "source" in the bindings. >>>>> I am not an expert on other changes below and will leave it to >>>>> Rob/Krzysztof to comment. >>>>> >>>>> Rob, Krzysztof, >>>>> >>>>> We need someway to "link" (add a phandle) from a "port". The patch >>>>> below >>>>> is extending "standard" port to add a phandle. Please let us know if >>>>> there is a better way. >>>>> >>>>> e.g.: >>>>> >>>>> filters = list of tuples of port, phandle. ? >>>>> >>>>> e.g.: >>>>> >>>>> filters = < 0, <&tpdm_video>, >>>>> 1, <&tpdm_mdss> >>>>> > >>>>> >>>> >>>> Current solution feels like band-aid - what if next time you need some >>>> second filter? Or "wall"? Or whatever? Next property? >>> >>> >>> >>>> >>>> Isn't filter just one endpoint in the graph? >>>> >>>> A <--> filter <--> B >>> >>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a >>> multi output ports). >>> >>> For clearer example: >>> >>> A0 <--> .. <--> ..\ p0 / --> Filtered for (A1) >>> <--> B1 >>> A1 <--> .. <--> .. - < L(filters> p1 - --> Filtered for (A2) >>> <--> B2 >>> A2 <--> .. <--> ../ p2 \ --> Unfiltered >>> <--> B0 >>> >>> >>> >>>> Instead of >>>> >>>> A <----through-filter----> B? >>> >>> The problem is we need to know the components in the path from A0 to X >>> through, (Not just A0 and L). And also we need to know "which port >>> (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out >>> of the >>> link "L". >>> >>> So ideally, we need a way to tie p0 -> A1, p1 -> A2. >>> >>> would we need something else in the future ? I don't know for sure. >>> People could design their own things ;-). But this was the first time >>> ever in the last 12yrs since we supported coresight in the kernel. >>> (there is always a first time). >>> >>> Fundamentally, the "ports" cannot have additional properties today. >>> Not sure if there are other usecases (I don't see why). So, we have >>> to manually extend like above, which I think is not nice. >> >> Replying to the other thread [0], made me realize that the above is not >> true. Indeed it is possible to add properties for endpoints, e.g: >> >> e.g.: media/video-interfaces.yaml >> >> So extending the endpoint node is indeed acceptable (unlike I thought). >> May be the we it is achieved in this patch is making it look otherwise. >> >> Suzuki >> [0] >> https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com > > Please could you let us know if it is acceptable to extend "endpoint" > node to have an optional property ? Hi Krzysztof, Kindly reminder, could you help comment on this? Best, Tao > > Suzuki > > >> >> >> >>> >>> Happy to proceed with anything that seems acceptable for you folks. >>> >>> Suzuki >>> >>> >>> >>>> >>>> Best regards, >>>> Krzysztof >>>> >>> >> > > _______________________________________________ > CoreSight mailing list -- coresight@lists.linaro.org > To unsubscribe send an email to coresight-leave@lists.linaro.org
On 18/10/2024 12:08, Suzuki K Poulose wrote: > On 18/10/2024 11:05, Krzysztof Kozlowski wrote: >> On 17/10/2024 09:23, Tao Zhang wrote: >>> >>> On 10/9/2024 6:52 PM, Suzuki K Poulose wrote: >>>> Krzysztof >>>> >>>> On 22/08/2024 12:50, Suzuki K Poulose wrote: >>>>> On 22/08/2024 11:34, Suzuki K Poulose wrote: >>>>>> On 22/08/2024 08:08, Krzysztof Kozlowski wrote: >>>>>>> On Wed, Aug 21, 2024 at 11:38:55AM +0100, Suzuki K Poulose wrote: >>>>>>>> On 21/08/2024 04:13, Tao Zhang wrote: >>>>>>>>> The is some "magic" hard coded filtering in the replicators, >>>>>>>>> which only passes through trace from a particular "source". Add >>>>>>>>> a new property "filter-src" to label a phandle to the coresight >>>>>>>>> trace source device matching the hard coded filtering for the port. >>>>>>>> >>>>>>>> Minor nit: Please do not use abbreviate "source" in the bindings. >>>>>>>> I am not an expert on other changes below and will leave it to >>>>>>>> Rob/Krzysztof to comment. >>>>>>>> >>>>>>>> Rob, Krzysztof, >>>>>>>> >>>>>>>> We need someway to "link" (add a phandle) from a "port". The patch >>>>>>>> below >>>>>>>> is extending "standard" port to add a phandle. Please let us know if >>>>>>>> there is a better way. >>>>>>>> >>>>>>>> e.g.: >>>>>>>> >>>>>>>> filters = list of tuples of port, phandle. ? >>>>>>>> >>>>>>>> e.g.: >>>>>>>> >>>>>>>> filters = < 0, <&tpdm_video>, >>>>>>>> 1, <&tpdm_mdss> >>>>>>>> > >>>>>>>> >>>>>>> >>>>>>> Current solution feels like band-aid - what if next time you need some >>>>>>> second filter? Or "wall"? Or whatever? Next property? >>>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> Isn't filter just one endpoint in the graph? >>>>>>> >>>>>>> A <--> filter <--> B >>>>>> >>>>>> To be more precise, "Filter" is a "port (p0, p1, p2 below)" (among a >>>>>> multi output ports). >>>>>> >>>>>> For clearer example: >>>>>> >>>>>> A0 <--> .. <--> ..\ p0 / --> Filtered for (A1) >>>>>> <--> B1 >>>>>> A1 <--> .. <--> .. - < L(filters> p1 - --> Filtered for (A2) >>>>>> <--> B2 >>>>>> A2 <--> .. <--> ../ p2 \ --> Unfiltered >>>>>> <--> B0 >>>>>> >>>>>> >>>>>> >>>>>>> Instead of >>>>>>> >>>>>>> A <----through-filter----> B? >>>>>> >>>>>> The problem is we need to know the components in the path from A0 to X >>>>>> through, (Not just A0 and L). And also we need to know "which port >>>>>> (p0 vs p1 vs p2)" does the traffic take from a source (A0/A1/A2) out >>>>>> of the >>>>>> link "L". >>>>>> >>>>>> So ideally, we need a way to tie p0 -> A1, p1 -> A2. >>>>>> >>>>>> would we need something else in the future ? I don't know for sure. >>>>>> People could design their own things ;-). But this was the first time >>>>>> ever in the last 12yrs since we supported coresight in the kernel. >>>>>> (there is always a first time). >>>>>> >>>>>> Fundamentally, the "ports" cannot have additional properties today. >>>>>> Not sure if there are other usecases (I don't see why). So, we have >>>>>> to manually extend like above, which I think is not nice. >>>>> >>>>> Replying to the other thread [0], made me realize that the above is not >>>>> true. Indeed it is possible to add properties for endpoints, e.g: >>>>> >>>>> e.g.: media/video-interfaces.yaml >>>>> >>>>> So extending the endpoint node is indeed acceptable (unlike I thought). >>>>> May be the we it is achieved in this patch is making it look otherwise. >>>>> >>>>> Suzuki >>>>> [0] >>>>> https://lkml.kernel.org/r/4b51d5a9-3706-4630-83c1-01b01354d9a4@arm.com >>>> >>>> Please could you let us know if it is acceptable to extend "endpoint" >>>> node to have an optional property ? >>> >>> Hi Krzysztof, >>> >>> >>> Kindly reminder, could you help comment on this? >> >> I don't have any smart ideas and with earlier explanation sounds ok. > > Just to confirm, are you OK with adding a property to the "endpoint" > node that will indicate a phandle that the device allows on this > endpoint ? You mean the filter property in endpoint? if so, then yes. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml index 1892a091ac35..0d258c79eb94 100644 --- a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml +++ b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml @@ -45,7 +45,22 @@ properties: patternProperties: '^port@[01]$': description: Output connections to CoreSight Trace bus - $ref: /schemas/graph.yaml#/properties/port + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + + properties: + endpoint: + $ref: /schemas/graph.yaml#/$defs/endpoint-base + unevaluatedProperties: false + + properties: + filter-src: + $ref: /schemas/types.yaml#/definitions/phandle + description: + phandle to the coresight trace source device matching the + hard coded filtering for this port + + remote-endpoint: true required: - compatible @@ -72,6 +87,7 @@ examples: reg = <0>; replicator_out_port0: endpoint { remote-endpoint = <&etb_in_port>; + filter-src = <&tpdm_video>; }; }; @@ -79,6 +95,7 @@ examples: reg = <1>; replicator_out_port1: endpoint { remote-endpoint = <&tpiu_in_port>; + filter-src = <&tpdm_mdss>; }; }; };
The is some "magic" hard coded filtering in the replicators, which only passes through trace from a particular "source". Add a new property "filter-src" to label a phandle to the coresight trace source device matching the hard coded filtering for the port. Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> --- .../arm/arm,coresight-static-replicator.yaml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)