* [PATCH] e1000e: apply burst mode settings only on default
@ 2017-08-25 15:06 Willem de Bruijn
2017-08-27 8:30 ` [Intel-wired-lan] " Neftin, Sasha
2017-09-14 23:48 ` Brown, Aaron F
0 siblings, 2 replies; 6+ messages in thread
From: Willem de Bruijn @ 2017-08-25 15:06 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: intel-wired-lan, netdev, jesse.brandeburg, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Devices that support FLAG2_DMA_BURST have different default values
for RDTR and RADV. Apply burst mode default settings only when no
explicit value was passed at module load.
The RDTR default is zero. If the module is loaded for low latency
operation with RxIntDelay=0, do not override this value with a burst
default of 32.
Move the decision to apply burst values earlier, where explicitly
initialized module variables can be distinguished from defaults.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 98e68888abb1..2311b31bdcac 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -94,10 +94,6 @@ struct e1000_info;
*/
#define E1000_CHECK_RESET_COUNT 25
-#define DEFAULT_RDTR 0
-#define DEFAULT_RADV 8
-#define BURST_RDTR 0x20
-#define BURST_RADV 0x20
#define PCICFG_DESC_RING_STATUS 0xe4
#define FLUSH_DESC_REQUIRED 0x100
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 327dfe5bedc0..47b89aac7969 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -3223,14 +3223,6 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
*/
ew32(RXDCTL(0), E1000_RXDCTL_DMA_BURST_ENABLE);
ew32(RXDCTL(1), E1000_RXDCTL_DMA_BURST_ENABLE);
-
- /* override the delay timers for enabling bursting, only if
- * the value was not set by the user via module options
- */
- if (adapter->rx_int_delay == DEFAULT_RDTR)
- adapter->rx_int_delay = BURST_RDTR;
- if (adapter->rx_abs_int_delay == DEFAULT_RADV)
- adapter->rx_abs_int_delay = BURST_RADV;
}
/* set the Receive Delay Timer Register */
diff --git a/drivers/net/ethernet/intel/e1000e/param.c b/drivers/net/ethernet/intel/e1000e/param.c
index 6d8c39abee16..bb696c98f9b0 100644
--- a/drivers/net/ethernet/intel/e1000e/param.c
+++ b/drivers/net/ethernet/intel/e1000e/param.c
@@ -73,17 +73,25 @@ E1000_PARAM(TxAbsIntDelay, "Transmit Absolute Interrupt Delay");
/* Receive Interrupt Delay in units of 1.024 microseconds
* hardware will likely hang if you set this to anything but zero.
*
+ * Burst variant is used as default if device has FLAG2_DMA_BURST.
+ *
* Valid Range: 0-65535
*/
E1000_PARAM(RxIntDelay, "Receive Interrupt Delay");
+#define DEFAULT_RDTR 0
+#define BURST_RDTR 0x20
#define MAX_RXDELAY 0xFFFF
#define MIN_RXDELAY 0
/* Receive Absolute Interrupt Delay in units of 1.024 microseconds
+ *
+ * Burst variant is used as default if device has FLAG2_DMA_BURST.
*
* Valid Range: 0-65535
*/
E1000_PARAM(RxAbsIntDelay, "Receive Absolute Interrupt Delay");
+#define DEFAULT_RADV 8
+#define BURST_RADV 0x20
#define MAX_RXABSDELAY 0xFFFF
#define MIN_RXABSDELAY 0
@@ -297,6 +305,9 @@ void e1000e_check_options(struct e1000_adapter *adapter)
.max = MAX_RXDELAY } }
};
+ if (adapter->flags2 & FLAG2_DMA_BURST)
+ opt.def = BURST_RDTR;
+
if (num_RxIntDelay > bd) {
adapter->rx_int_delay = RxIntDelay[bd];
e1000_validate_option(&adapter->rx_int_delay, &opt,
@@ -307,7 +318,7 @@ void e1000e_check_options(struct e1000_adapter *adapter)
}
/* Receive Absolute Interrupt Delay */
{
- static const struct e1000_option opt = {
+ static struct e1000_option opt = {
.type = range_option,
.name = "Receive Absolute Interrupt Delay",
.err = "using default of "
@@ -317,6 +328,9 @@ void e1000e_check_options(struct e1000_adapter *adapter)
.max = MAX_RXABSDELAY } }
};
+ if (adapter->flags2 & FLAG2_DMA_BURST)
+ opt.def = BURST_RADV;
+
if (num_RxAbsIntDelay > bd) {
adapter->rx_abs_int_delay = RxAbsIntDelay[bd];
e1000_validate_option(&adapter->rx_abs_int_delay, &opt,
--
2.14.1.342.g6490525c54-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH] e1000e: apply burst mode settings only on default
2017-08-25 15:06 [PATCH] e1000e: apply burst mode settings only on default Willem de Bruijn
@ 2017-08-27 8:30 ` Neftin, Sasha
2017-08-27 8:32 ` Neftin, Sasha
2017-08-28 16:25 ` Alexander Duyck
2017-09-14 23:48 ` Brown, Aaron F
1 sibling, 2 replies; 6+ messages in thread
From: Neftin, Sasha @ 2017-08-27 8:30 UTC (permalink / raw)
To: Willem de Bruijn, "jeffrey.t.kirsher
Cc: netdev, Willem de Bruijn, intel-wired-lan
On 8/25/2017 18:06, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Devices that support FLAG2_DMA_BURST have different default values
> for RDTR and RADV. Apply burst mode default settings only when no
> explicit value was passed at module load.
>
> The RDTR default is zero. If the module is loaded for low latency
> operation with RxIntDelay=0, do not override this value with a burst
> default of 32.
>
> Move the decision to apply burst values earlier, where explicitly
> initialized module variables can be distinguished from defaults.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
> drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
> drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
> 3 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
> index 98e68888abb1..2311b31bdcac 100644
> --- a/drivers/net/ethernet/intel/e1000e/e1000.h
> +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
> @@ -94,10 +94,6 @@ struct e1000_info;
> */
> #define E1000_CHECK_RESET_COUNT 25
>
> -#define DEFAULT_RDTR 0
> -#define DEFAULT_RADV 8
> -#define BURST_RDTR 0x20
> -#define BURST_RADV 0x20
> #define PCICFG_DESC_RING_STATUS 0xe4
> #define FLUSH_DESC_REQUIRED 0x100
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 327dfe5bedc0..47b89aac7969 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -3223,14 +3223,6 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
> */
> ew32(RXDCTL(0), E1000_RXDCTL_DMA_BURST_ENABLE);
> ew32(RXDCTL(1), E1000_RXDCTL_DMA_BURST_ENABLE);
> -
> - /* override the delay timers for enabling bursting, only if
> - * the value was not set by the user via module options
> - */
> - if (adapter->rx_int_delay == DEFAULT_RDTR)
> - adapter->rx_int_delay = BURST_RDTR;
> - if (adapter->rx_abs_int_delay == DEFAULT_RADV)
> - adapter->rx_abs_int_delay = BURST_RADV;
> }
>
> /* set the Receive Delay Timer Register */
> diff --git a/drivers/net/ethernet/intel/e1000e/param.c b/drivers/net/ethernet/intel/e1000e/param.c
> index 6d8c39abee16..bb696c98f9b0 100644
> --- a/drivers/net/ethernet/intel/e1000e/param.c
> +++ b/drivers/net/ethernet/intel/e1000e/param.c
> @@ -73,17 +73,25 @@ E1000_PARAM(TxAbsIntDelay, "Transmit Absolute Interrupt Delay");
> /* Receive Interrupt Delay in units of 1.024 microseconds
> * hardware will likely hang if you set this to anything but zero.
> *
> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
> + *
> * Valid Range: 0-65535
> */
> E1000_PARAM(RxIntDelay, "Receive Interrupt Delay");
> +#define DEFAULT_RDTR 0
> +#define BURST_RDTR 0x20
> #define MAX_RXDELAY 0xFFFF
> #define MIN_RXDELAY 0
>
> /* Receive Absolute Interrupt Delay in units of 1.024 microseconds
> + *
> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
> *
> * Valid Range: 0-65535
> */
> E1000_PARAM(RxAbsIntDelay, "Receive Absolute Interrupt Delay");
> +#define DEFAULT_RADV 8
> +#define BURST_RADV 0x20
> #define MAX_RXABSDELAY 0xFFFF
> #define MIN_RXABSDELAY 0
>
> @@ -297,6 +305,9 @@ void e1000e_check_options(struct e1000_adapter *adapter)
> .max = MAX_RXDELAY } }
> };
>
> + if (adapter->flags2 & FLAG2_DMA_BURST)
> + opt.def = BURST_RDTR;
> +
> if (num_RxIntDelay > bd) {
> adapter->rx_int_delay = RxIntDelay[bd];
> e1000_validate_option(&adapter->rx_int_delay, &opt,
> @@ -307,7 +318,7 @@ void e1000e_check_options(struct e1000_adapter *adapter)
> }
> /* Receive Absolute Interrupt Delay */
> {
> - static const struct e1000_option opt = {
> + static struct e1000_option opt = {
> .type = range_option,
> .name = "Receive Absolute Interrupt Delay",
> .err = "using default of "
> @@ -317,6 +328,9 @@ void e1000e_check_options(struct e1000_adapter *adapter)
> .max = MAX_RXABSDELAY } }
> };
>
> + if (adapter->flags2 & FLAG2_DMA_BURST)
> + opt.def = BURST_RADV;
> +
> if (num_RxAbsIntDelay > bd) {
> adapter->rx_abs_int_delay = RxAbsIntDelay[bd];
> e1000_validate_option(&adapter->rx_abs_int_delay, &opt,
This patch looks good for me, but I would like hear second opinion.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH] e1000e: apply burst mode settings only on default
2017-08-27 8:30 ` [Intel-wired-lan] " Neftin, Sasha
@ 2017-08-27 8:32 ` Neftin, Sasha
2017-08-27 8:34 ` Neftin, Sasha
2017-08-28 16:25 ` Alexander Duyck
1 sibling, 1 reply; 6+ messages in thread
From: Neftin, Sasha @ 2017-08-27 8:32 UTC (permalink / raw)
To: Willem de Bruijn, "jeffrey.t.kirsher
Cc: netdev, Willem de Bruijn, intel-wired-lan
On 8/27/2017 11:30, Neftin, Sasha wrote:
> On 8/25/2017 18:06, Willem de Bruijn wrote:
>> From: Willem de Bruijn <willemb@google.com>
>>
>> Devices that support FLAG2_DMA_BURST have different default values
>> for RDTR and RADV. Apply burst mode default settings only when no
>> explicit value was passed at module load.
>>
>> The RDTR default is zero. If the module is loaded for low latency
>> operation with RxIntDelay=0, do not override this value with a burst
>> default of 32.
>>
>> Move the decision to apply burst values earlier, where explicitly
>> initialized module variables can be distinguished from defaults.
>>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>> ---
>> drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
>> drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
>> drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
>> 3 files changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h
>> b/drivers/net/ethernet/intel/e1000e/e1000.h
>> index 98e68888abb1..2311b31bdcac 100644
>> --- a/drivers/net/ethernet/intel/e1000e/e1000.h
>> +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
>> @@ -94,10 +94,6 @@ struct e1000_info;
>> */
>> #define E1000_CHECK_RESET_COUNT 25
>> -#define DEFAULT_RDTR 0
>> -#define DEFAULT_RADV 8
>> -#define BURST_RDTR 0x20
>> -#define BURST_RADV 0x20
>> #define PCICFG_DESC_RING_STATUS 0xe4
>> #define FLUSH_DESC_REQUIRED 0x100
>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
>> b/drivers/net/ethernet/intel/e1000e/netdev.c
>> index 327dfe5bedc0..47b89aac7969 100644
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -3223,14 +3223,6 @@ static void e1000_configure_rx(struct
>> e1000_adapter *adapter)
>> */
>> ew32(RXDCTL(0), E1000_RXDCTL_DMA_BURST_ENABLE);
>> ew32(RXDCTL(1), E1000_RXDCTL_DMA_BURST_ENABLE);
>> -
>> - /* override the delay timers for enabling bursting, only if
>> - * the value was not set by the user via module options
>> - */
>> - if (adapter->rx_int_delay == DEFAULT_RDTR)
>> - adapter->rx_int_delay = BURST_RDTR;
>> - if (adapter->rx_abs_int_delay == DEFAULT_RADV)
>> - adapter->rx_abs_int_delay = BURST_RADV;
>> }
>> /* set the Receive Delay Timer Register */
>> diff --git a/drivers/net/ethernet/intel/e1000e/param.c
>> b/drivers/net/ethernet/intel/e1000e/param.c
>> index 6d8c39abee16..bb696c98f9b0 100644
>> --- a/drivers/net/ethernet/intel/e1000e/param.c
>> +++ b/drivers/net/ethernet/intel/e1000e/param.c
>> @@ -73,17 +73,25 @@ E1000_PARAM(TxAbsIntDelay, "Transmit Absolute
>> Interrupt Delay");
>> /* Receive Interrupt Delay in units of 1.024 microseconds
>> * hardware will likely hang if you set this to anything but zero.
>> *
>> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
>> + *
>> * Valid Range: 0-65535
>> */
>> E1000_PARAM(RxIntDelay, "Receive Interrupt Delay");
>> +#define DEFAULT_RDTR 0
>> +#define BURST_RDTR 0x20
>> #define MAX_RXDELAY 0xFFFF
>> #define MIN_RXDELAY 0
>> /* Receive Absolute Interrupt Delay in units of 1.024 microseconds
>> + *
>> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
>> *
>> * Valid Range: 0-65535
>> */
>> E1000_PARAM(RxAbsIntDelay, "Receive Absolute Interrupt Delay");
>> +#define DEFAULT_RADV 8
>> +#define BURST_RADV 0x20
>> #define MAX_RXABSDELAY 0xFFFF
>> #define MIN_RXABSDELAY 0
>> @@ -297,6 +305,9 @@ void e1000e_check_options(struct e1000_adapter
>> *adapter)
>> .max = MAX_RXDELAY } }
>> };
>> + if (adapter->flags2 & FLAG2_DMA_BURST)
>> + opt.def = BURST_RDTR;
>> +
>> if (num_RxIntDelay > bd) {
>> adapter->rx_int_delay = RxIntDelay[bd];
>> e1000_validate_option(&adapter->rx_int_delay, &opt,
>> @@ -307,7 +318,7 @@ void e1000e_check_options(struct e1000_adapter
>> *adapter)
>> }
>> /* Receive Absolute Interrupt Delay */
>> {
>> - static const struct e1000_option opt = {
>> + static struct e1000_option opt = {
>> .type = range_option,
>> .name = "Receive Absolute Interrupt Delay",
>> .err = "using default of "
>> @@ -317,6 +328,9 @@ void e1000e_check_options(struct e1000_adapter
>> *adapter)
>> .max = MAX_RXABSDELAY } }
>> };
>> + if (adapter->flags2 & FLAG2_DMA_BURST)
>> + opt.def = BURST_RADV;
>> +
>> if (num_RxAbsIntDelay > bd) {
>> adapter->rx_abs_int_delay = RxAbsIntDelay[bd];
>> e1000_validate_option(&adapter->rx_abs_int_delay, &opt,
>
> This patch looks good for me, but I would like hear second opinion.
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH] e1000e: apply burst mode settings only on default
2017-08-27 8:32 ` Neftin, Sasha
@ 2017-08-27 8:34 ` Neftin, Sasha
0 siblings, 0 replies; 6+ messages in thread
From: Neftin, Sasha @ 2017-08-27 8:34 UTC (permalink / raw)
To: Willem de Bruijn, jeffrey.t.kirsher, alexander.h.duyck,
raanan.avargil, dima.ruinskiy
Cc: netdev, Willem de Bruijn, intel-wired-lan
On 8/27/2017 11:32, Neftin, Sasha wrote:
> On 8/27/2017 11:30, Neftin, Sasha wrote:
>> On 8/25/2017 18:06, Willem de Bruijn wrote:
>>> From: Willem de Bruijn <willemb@google.com>
>>>
>>> Devices that support FLAG2_DMA_BURST have different default values
>>> for RDTR and RADV. Apply burst mode default settings only when no
>>> explicit value was passed at module load.
>>>
>>> The RDTR default is zero. If the module is loaded for low latency
>>> operation with RxIntDelay=0, do not override this value with a burst
>>> default of 32.
>>>
>>> Move the decision to apply burst values earlier, where explicitly
>>> initialized module variables can be distinguished from defaults.
>>>
>>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>>> ---
>>> drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
>>> drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
>>> drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
>>> 3 files changed, 15 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h
>>> b/drivers/net/ethernet/intel/e1000e/e1000.h
>>> index 98e68888abb1..2311b31bdcac 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/e1000.h
>>> +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
>>> @@ -94,10 +94,6 @@ struct e1000_info;
>>> */
>>> #define E1000_CHECK_RESET_COUNT 25
>>> -#define DEFAULT_RDTR 0
>>> -#define DEFAULT_RADV 8
>>> -#define BURST_RDTR 0x20
>>> -#define BURST_RADV 0x20
>>> #define PCICFG_DESC_RING_STATUS 0xe4
>>> #define FLUSH_DESC_REQUIRED 0x100
>>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
>>> b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> index 327dfe5bedc0..47b89aac7969 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>>> @@ -3223,14 +3223,6 @@ static void e1000_configure_rx(struct
>>> e1000_adapter *adapter)
>>> */
>>> ew32(RXDCTL(0), E1000_RXDCTL_DMA_BURST_ENABLE);
>>> ew32(RXDCTL(1), E1000_RXDCTL_DMA_BURST_ENABLE);
>>> -
>>> - /* override the delay timers for enabling bursting, only if
>>> - * the value was not set by the user via module options
>>> - */
>>> - if (adapter->rx_int_delay == DEFAULT_RDTR)
>>> - adapter->rx_int_delay = BURST_RDTR;
>>> - if (adapter->rx_abs_int_delay == DEFAULT_RADV)
>>> - adapter->rx_abs_int_delay = BURST_RADV;
>>> }
>>> /* set the Receive Delay Timer Register */
>>> diff --git a/drivers/net/ethernet/intel/e1000e/param.c
>>> b/drivers/net/ethernet/intel/e1000e/param.c
>>> index 6d8c39abee16..bb696c98f9b0 100644
>>> --- a/drivers/net/ethernet/intel/e1000e/param.c
>>> +++ b/drivers/net/ethernet/intel/e1000e/param.c
>>> @@ -73,17 +73,25 @@ E1000_PARAM(TxAbsIntDelay, "Transmit Absolute
>>> Interrupt Delay");
>>> /* Receive Interrupt Delay in units of 1.024 microseconds
>>> * hardware will likely hang if you set this to anything but zero.
>>> *
>>> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
>>> + *
>>> * Valid Range: 0-65535
>>> */
>>> E1000_PARAM(RxIntDelay, "Receive Interrupt Delay");
>>> +#define DEFAULT_RDTR 0
>>> +#define BURST_RDTR 0x20
>>> #define MAX_RXDELAY 0xFFFF
>>> #define MIN_RXDELAY 0
>>> /* Receive Absolute Interrupt Delay in units of 1.024 microseconds
>>> + *
>>> + * Burst variant is used as default if device has FLAG2_DMA_BURST.
>>> *
>>> * Valid Range: 0-65535
>>> */
>>> E1000_PARAM(RxAbsIntDelay, "Receive Absolute Interrupt Delay");
>>> +#define DEFAULT_RADV 8
>>> +#define BURST_RADV 0x20
>>> #define MAX_RXABSDELAY 0xFFFF
>>> #define MIN_RXABSDELAY 0
>>> @@ -297,6 +305,9 @@ void e1000e_check_options(struct e1000_adapter
>>> *adapter)
>>> .max = MAX_RXDELAY } }
>>> };
>>> + if (adapter->flags2 & FLAG2_DMA_BURST)
>>> + opt.def = BURST_RDTR;
>>> +
>>> if (num_RxIntDelay > bd) {
>>> adapter->rx_int_delay = RxIntDelay[bd];
>>> e1000_validate_option(&adapter->rx_int_delay, &opt,
>>> @@ -307,7 +318,7 @@ void e1000e_check_options(struct e1000_adapter
>>> *adapter)
>>> }
>>> /* Receive Absolute Interrupt Delay */
>>> {
>>> - static const struct e1000_option opt = {
>>> + static struct e1000_option opt = {
>>> .type = range_option,
>>> .name = "Receive Absolute Interrupt Delay",
>>> .err = "using default of "
>>> @@ -317,6 +328,9 @@ void e1000e_check_options(struct e1000_adapter
>>> *adapter)
>>> .max = MAX_RXABSDELAY } }
>>> };
>>> + if (adapter->flags2 & FLAG2_DMA_BURST)
>>> + opt.def = BURST_RADV;
>>> +
>>> if (num_RxAbsIntDelay > bd) {
>>> adapter->rx_abs_int_delay = RxAbsIntDelay[bd];
>>> e1000_validate_option(&adapter->rx_abs_int_delay, &opt,
>>
>> This patch looks good for me, but I would like hear second opinion.
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH] e1000e: apply burst mode settings only on default
2017-08-27 8:30 ` [Intel-wired-lan] " Neftin, Sasha
2017-08-27 8:32 ` Neftin, Sasha
@ 2017-08-28 16:25 ` Alexander Duyck
1 sibling, 0 replies; 6+ messages in thread
From: Alexander Duyck @ 2017-08-28 16:25 UTC (permalink / raw)
To: Neftin, Sasha
Cc: Willem de Bruijn, "jeffrey.t.kirsher, Netdev,
Willem de Bruijn, intel-wired-lan
On Sun, Aug 27, 2017 at 1:30 AM, Neftin, Sasha <sasha.neftin@intel.com> wrote:
> On 8/25/2017 18:06, Willem de Bruijn wrote:
>>
>> From: Willem de Bruijn <willemb@google.com>
>>
>> Devices that support FLAG2_DMA_BURST have different default values
>> for RDTR and RADV. Apply burst mode default settings only when no
>> explicit value was passed at module load.
>>
>> The RDTR default is zero. If the module is loaded for low latency
>> operation with RxIntDelay=0, do not override this value with a burst
>> default of 32.
>>
>> Move the decision to apply burst values earlier, where explicitly
>> initialized module variables can be distinguished from defaults.
>>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> This patch looks good for me, but I would like hear second opinion.
This code is an improvement over what was already there. I am good with this.
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] e1000e: apply burst mode settings only on default
2017-08-25 15:06 [PATCH] e1000e: apply burst mode settings only on default Willem de Bruijn
2017-08-27 8:30 ` [Intel-wired-lan] " Neftin, Sasha
@ 2017-09-14 23:48 ` Brown, Aaron F
1 sibling, 0 replies; 6+ messages in thread
From: Brown, Aaron F @ 2017-09-14 23:48 UTC (permalink / raw)
To: Willem de Bruijn, Kirsher, Jeffrey T
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
Brandeburg, Jesse, Willem de Bruijn
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Willem de Bruijn
> Sent: Friday, August 25, 2017 8:06 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; Willem de Bruijn
> <willemb@google.com>
> Subject: [PATCH] e1000e: apply burst mode settings only on default
>
> From: Willem de Bruijn <willemb@google.com>
>
> Devices that support FLAG2_DMA_BURST have different default values
> for RDTR and RADV. Apply burst mode default settings only when no
> explicit value was passed at module load.
>
> The RDTR default is zero. If the module is loaded for low latency
> operation with RxIntDelay=0, do not override this value with a burst
> default of 32.
>
> Move the decision to apply burst values earlier, where explicitly
> initialized module variables can be distinguished from defaults.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> drivers/net/ethernet/intel/e1000e/e1000.h | 4 ----
> drivers/net/ethernet/intel/e1000e/netdev.c | 8 --------
> drivers/net/ethernet/intel/e1000e/param.c | 16 +++++++++++++++-
> 3 files changed, 15 insertions(+), 13 deletions(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-14 23:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-25 15:06 [PATCH] e1000e: apply burst mode settings only on default Willem de Bruijn
2017-08-27 8:30 ` [Intel-wired-lan] " Neftin, Sasha
2017-08-27 8:32 ` Neftin, Sasha
2017-08-27 8:34 ` Neftin, Sasha
2017-08-28 16:25 ` Alexander Duyck
2017-09-14 23:48 ` Brown, Aaron F
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).