Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1534554 > unrolled thread
| Started by | Grygorii Strashko <grygorii.strashko@ti.com> |
|---|---|
| First post | 2016-12-02 00:40 +0100 |
| Last post | 2016-12-07 20:50 +0100 |
| Articles | 6 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size Grygorii Strashko <grygorii.strashko@ti.com> - 2016-12-02 00:40 +0100
Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> - 2016-12-02 12:30 +0100
Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size Grygorii Strashko <grygorii.strashko@ti.com> - 2016-12-02 18:30 +0100
Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> - 2016-12-03 01:40 +0100
Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size Grygorii Strashko <grygorii.strashko@ti.com> - 2016-12-02 20:50 +0100
Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size Grygorii Strashko <grygorii.strashko@ti.com> - 2016-12-07 20:50 +0100
| From | Grygorii Strashko <grygorii.strashko@ti.com> |
|---|---|
| Date | 2016-12-02 00:40 +0100 |
| Subject | [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size |
| Message-ID | <sJJ7k-3WJ-15@gated-at.bofh.it> |
Add optional property "descs_pool_size" to specify buffer descriptor's pool size. The "descs_pool_size" should define total number of CPDMA CPPI descriptors to be used for both ingress/egress packets processing. If not specified - the default value 256 will be used which will allow to place descriptor's pool into the internal CPPI RAM on most of TI SoC. Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> --- Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt index 5ad439f..b99d196 100644 --- a/Documentation/devicetree/bindings/net/cpsw.txt +++ b/Documentation/devicetree/bindings/net/cpsw.txt @@ -35,6 +35,11 @@ Optional properties: For example in dra72x-evm, pcf gpio has to be driven low so that cpsw slave 0 and phy data lines are connected via mux. +- descs_pool_size : total number of CPDMA CPPI descriptors to be used for + both ingress/egress packets processing. if not + specified the default value 256 will be used which + will allow to place descriptors pool into the + internal CPPI RAM. Slave Properties: -- 2.10.1
[toc] | [next] | [standalone]
| From | Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> |
|---|---|
| Date | 2016-12-02 12:30 +0100 |
| Subject | Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size |
| Message-ID | <sJUcq-3bc-27@gated-at.bofh.it> |
| In reply to | #1534554 |
On Thu, Dec 01, 2016 at 05:34:30PM -0600, Grygorii Strashko wrote: > Add optional property "descs_pool_size" to specify buffer descriptor's > pool size. The "descs_pool_size" should define total number of CPDMA > CPPI descriptors to be used for both ingress/egress packets > processing. If not specified - the default value 256 will be used > which will allow to place descriptor's pool into the internal CPPI > RAM on most of TI SoC. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > --- > Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt > index 5ad439f..b99d196 100644 > --- a/Documentation/devicetree/bindings/net/cpsw.txt > +++ b/Documentation/devicetree/bindings/net/cpsw.txt > @@ -35,6 +35,11 @@ Optional properties: > For example in dra72x-evm, pcf gpio has to be > driven low so that cpsw slave 0 and phy data > lines are connected via mux. > +- descs_pool_size : total number of CPDMA CPPI descriptors to be used for > + both ingress/egress packets processing. if not > + specified the default value 256 will be used which > + will allow to place descriptors pool into the > + internal CPPI RAM. Does it describe h/w? Why now module parameter? or even smth like ethtool num ring entries? > > Slave Properties: > -- > 2.10.1 >
[toc] | [prev] | [next] | [standalone]
| From | Grygorii Strashko <grygorii.strashko@ti.com> |
|---|---|
| Date | 2016-12-02 18:30 +0100 |
| Subject | Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size |
| Message-ID | <sJZON-6LQ-1@gated-at.bofh.it> |
| In reply to | #1534854 |
On 12/02/2016 05:28 AM, Ivan Khoronzhuk wrote:
> On Thu, Dec 01, 2016 at 05:34:30PM -0600, Grygorii Strashko wrote:
>> Add optional property "descs_pool_size" to specify buffer descriptor's
>> pool size. The "descs_pool_size" should define total number of CPDMA
>> CPPI descriptors to be used for both ingress/egress packets
>> processing. If not specified - the default value 256 will be used
>> which will allow to place descriptor's pool into the internal CPPI
>> RAM on most of TI SoC.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>> Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>> index 5ad439f..b99d196 100644
>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>> @@ -35,6 +35,11 @@ Optional properties:
>> For example in dra72x-evm, pcf gpio has to be
>> driven low so that cpsw slave 0 and phy data
>> lines are connected via mux.
>> +- descs_pool_size : total number of CPDMA CPPI descriptors to be used for
>> + both ingress/egress packets processing. if not
>> + specified the default value 256 will be used which
>> + will allow to place descriptors pool into the
>> + internal CPPI RAM.
> Does it describe h/w? Why now module parameter? or even smth like ethtool num
> ring entries?
>
It can be module parameter too. in general this is expected to be
one-time boot setting only.
----- OR
So, do you propose to use
ethtool -g ethX
ethtool -G ethX [rx N] [tx N]
?
Now cpdma has one pool for all RX/TX channels, so changing this settings
by ethtool will require: pause interfaces, reallocate cpdma pool,
re-arrange buffers between channels, resume interface. Correct?
How do you think - we can move forward with one pool or better to have two (Rx and Tx)?
Wouldn't it be reasonable to still have DT (or module) parameter to avoid
cpdma reconfiguration on system startup (pause/resume interfaces) (faster boot)?
How about cpdma re-allocation policy (with expectation that is shouldn't happen too often)?
- increasing of Rx, Tx will grow total number of physically allocated buffers (total_desc_num)
- decreasing of Rx, Tx will just change number of available buffers (no memory re-allocation)
----- OR ----
Can we move forward with current patch (total number of CPDMA CPPI descriptors defined in DT)
and add ethtool -G ethX [rx N] [tx N] which will allow to re-split descs between RX and TX?
--
regards,
-grygorii
[toc] | [prev] | [next] | [standalone]
| From | Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> |
|---|---|
| Date | 2016-12-03 01:40 +0100 |
| Subject | Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size |
| Message-ID | <sK6wV-2ve-7@gated-at.bofh.it> |
| In reply to | #1535093 |
On Fri, Dec 02, 2016 at 11:22:28AM -0600, Grygorii Strashko wrote: > > > On 12/02/2016 05:28 AM, Ivan Khoronzhuk wrote: > > On Thu, Dec 01, 2016 at 05:34:30PM -0600, Grygorii Strashko wrote: > >> Add optional property "descs_pool_size" to specify buffer descriptor's > >> pool size. The "descs_pool_size" should define total number of CPDMA > >> CPPI descriptors to be used for both ingress/egress packets > >> processing. If not specified - the default value 256 will be used > >> which will allow to place descriptor's pool into the internal CPPI > >> RAM on most of TI SoC. > >> > >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > >> --- > >> Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt > >> index 5ad439f..b99d196 100644 > >> --- a/Documentation/devicetree/bindings/net/cpsw.txt > >> +++ b/Documentation/devicetree/bindings/net/cpsw.txt > >> @@ -35,6 +35,11 @@ Optional properties: > >> For example in dra72x-evm, pcf gpio has to be > >> driven low so that cpsw slave 0 and phy data > >> lines are connected via mux. > >> +- descs_pool_size : total number of CPDMA CPPI descriptors to be used for > >> + both ingress/egress packets processing. if not > >> + specified the default value 256 will be used which > >> + will allow to place descriptors pool into the > >> + internal CPPI RAM. > > Does it describe h/w? Why now module parameter? or even smth like ethtool num > > ring entries? > > > > It can be module parameter too. in general this is expected to be > one-time boot setting only. > > ----- OR > So, do you propose to use > ethtool -g ethX > > ethtool -G ethX [rx N] [tx N] > ? It has a little different names, but yes, why not? No need, maybe, but....It's just a proposition, at least I was thinking about it after proposition from +cc Schuyler Patton to leave rx desc num property. In this case it's possible to tune tx/rx desc num ratio, even with SRAM descs. > > Now cpdma has one pool for all RX/TX channels, so changing this settings > by ethtool will require: pause interfaces, reallocate cpdma pool, Pause can lead to losts only for rx, and only for very short time, so it's not very bad, especially when user knows what he is doing. > re-arrange buffers between channels, resume interface. Correct? correct. But, some alternative variants can be used, like replacing descriptors. Shrink num of desc for every channels to 1, replace/add others, and expand. In this case no losts, but it's harder to debug issues after.... > > How do you think - we can move forward with one pool or better to have two (Rx and Tx)? I think one is enough, just split, if no harm on perf. > > Wouldn't it be reasonable to still have DT (or module) parameter to avoid > cpdma reconfiguration on system startup (pause/resume interfaces) (faster boot)? Would be, your choice, but it's not flexible. > > How about cpdma re-allocation policy (with expectation that is shouldn't happen too often)? > - increasing of Rx, Tx will grow total number of physically allocated buffers (total_desc_num) > - decreasing of Rx, Tx will just change number of available buffers (no memory re-allocation) > > ----- OR ---- > Can we move forward with current patch (total number of CPDMA CPPI descriptors defined in DT) > and add ethtool -G ethX [rx N] [tx N] which will allow to re-split descs between RX and TX? No objections, It anyway requires re-allocations. Re-split of Rx and Tx will not have a lot changes as most code exists already. > > > > -- > regards, > -grygorii
[toc] | [prev] | [next] | [standalone]
| From | Grygorii Strashko <grygorii.strashko@ti.com> |
|---|---|
| Date | 2016-12-02 20:50 +0100 |
| Subject | Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size |
| Message-ID | <sK20i-82t-27@gated-at.bofh.it> |
| In reply to | #1534854 |
On 12/02/2016 05:28 AM, Ivan Khoronzhuk wrote:
> On Thu, Dec 01, 2016 at 05:34:30PM -0600, Grygorii Strashko wrote:
>> Add optional property "descs_pool_size" to specify buffer descriptor's
>> pool size. The "descs_pool_size" should define total number of CPDMA
>> CPPI descriptors to be used for both ingress/egress packets
>> processing. If not specified - the default value 256 will be used
>> which will allow to place descriptor's pool into the internal CPPI
>> RAM on most of TI SoC.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>> Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>> index 5ad439f..b99d196 100644
>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>> @@ -35,6 +35,11 @@ Optional properties:
>> For example in dra72x-evm, pcf gpio has to be
>> driven low so that cpsw slave 0 and phy data
>> lines are connected via mux.
>> +- descs_pool_size : total number of CPDMA CPPI descriptors to be used for
>> + both ingress/egress packets processing. if not
>> + specified the default value 256 will be used which
>> + will allow to place descriptors pool into the
>> + internal CPPI RAM.
> Does it describe h/w? Why now module parameter? or even smth like ethtool num
> ring entries?
>
It can be module parameter too. for the use cases i'm aware of -
this is one-time boot setting only.
----- OR
So, do you propose to use
ethtool -g ethX
ethtool -G ethX [rx N] [tx N]
?
Now cpdma has one pool for all RX/TX channels, so changing this settings
by ethtool will require: pause interfaces, reallocate cpdma pool,
re-arrange buffers between channels, resume interface. Correct?
How do you think - we can move forward with one pool or better to have two (Rx and Tx)?
Wouldn't it be reasonable to still have DT (or module) parameter to avoid
cpdma reconfiguration on system startup (pause/resume interfaces) (faster boot)?
How about cpdma re-allocation policy (with expectation that is shouldn't happen too often)?
- increasing of Rx, Tx will grow total number of physically allocated buffers (total_desc_num)
- decreasing of Rx, Tx will just change number of available buffers (no memory re-allocation)
----- OR ----
Can we move forward with current patch (total number of CPDMA CPPI descriptors defined in DT)
and add ethtool -G ethX [rx N] [tx N] which will allow to re-split descs between RX and TX?
--
regards,
-grygorii
[toc] | [prev] | [next] | [standalone]
| From | Grygorii Strashko <grygorii.strashko@ti.com> |
|---|---|
| Date | 2016-12-07 20:50 +0100 |
| Subject | Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size |
| Message-ID | <sLQo2-4iZ-13@gated-at.bofh.it> |
| In reply to | #1535171 |
On 12/02/2016 11:21 AM, Grygorii Strashko wrote: > > > On 12/02/2016 05:28 AM, Ivan Khoronzhuk wrote: >> On Thu, Dec 01, 2016 at 05:34:30PM -0600, Grygorii Strashko wrote: >>> Add optional property "descs_pool_size" to specify buffer descriptor's >>> pool size. The "descs_pool_size" should define total number of CPDMA >>> CPPI descriptors to be used for both ingress/egress packets >>> processing. If not specified - the default value 256 will be used >>> which will allow to place descriptor's pool into the internal CPPI >>> RAM on most of TI SoC. >>> >>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >>> --- >>> Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt >>> index 5ad439f..b99d196 100644 >>> --- a/Documentation/devicetree/bindings/net/cpsw.txt >>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt >>> @@ -35,6 +35,11 @@ Optional properties: >>> For example in dra72x-evm, pcf gpio has to be >>> driven low so that cpsw slave 0 and phy data >>> lines are connected via mux. >>> +- descs_pool_size : total number of CPDMA CPPI descriptors to be used for >>> + both ingress/egress packets processing. if not >>> + specified the default value 256 will be used which >>> + will allow to place descriptors pool into the >>> + internal CPPI RAM. >> Does it describe h/w? Why now module parameter? or even smth like ethtool num >> ring entries? >> > > It can be module parameter too. for the use cases i'm aware of - > this is one-time boot setting only. > > ----- OR > So, do you propose to use > ethtool -g ethX > > ethtool -G ethX [rx N] [tx N] > ? > > Now cpdma has one pool for all RX/TX channels, so changing this settings > by ethtool will require: pause interfaces, reallocate cpdma pool, > re-arrange buffers between channels, resume interface. Correct? > > How do you think - we can move forward with one pool or better to have two (Rx and Tx)? > > Wouldn't it be reasonable to still have DT (or module) parameter to avoid > cpdma reconfiguration on system startup (pause/resume interfaces) (faster boot)? > > How about cpdma re-allocation policy (with expectation that is shouldn't happen too often)? > - increasing of Rx, Tx will grow total number of physically allocated buffers (total_desc_num) > - decreasing of Rx, Tx will just change number of available buffers (no memory re-allocation) > > ----- OR ---- > Can we move forward with current patch (total number of CPDMA CPPI descriptors defined in DT) > and add ethtool -G ethX [rx N] [tx N] which will allow to re-split descs between RX and TX? > > if no comments here, I'll rework patches to use module parameter for descs_pool_size and will add possibility to re-split RX/TX buffers using ethtool -G ethX [rx N] [tx N] -- regards, -grygorii
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web