Hi Gabriel,
Comments inline
On Tue, Nov 28, 2017 at 11:40:06AM -0600, Erik Gabriel Carrillo wrote:
<snip>
> +struct rte_event_timer_adapter *
> +rte_event_timer_adapter_create(const struct rte_event_timer_adapter_conf
> *conf)
> +{
> + /* default port conf values */
> + struct rte_event_port_conf port_conf = {
> + .new_event_threshold = 128,
> + .dequeue_depth = 32,
> + .enqueue_depth = 32
> + };
Instead of harcoding get the port conf values from
rte_event_port_default_conf_get().
> +
> + return rte_event_timer_adapter_create_ext(conf, default_port_conf_cb,
> + &port_conf);
> +}
> +
> +struct rte_event_timer_adapter *
> +rte_event_timer_adapter_create_ext(
> + const struct rte_event_timer_adapter_conf *conf,
> + rte_event_timer_adapter_port_conf_cb_t conf_cb,
> + void *conf_arg)
> +{
> + uint16_t adapter_id;
> + struct rte_event_timer_adapter *adapter;
> + const struct rte_memzone *mz;
> + char mz_name[DATA_MZ_NAME_MAX_LEN];
> + int n, ret;
> + struct rte_eventdev *dev;
> +
> + if (conf == NULL) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + /* Check eventdev ID */
> + if (!rte_event_pmd_is_valid_dev(conf->event_dev_id)) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> + dev = &rte_eventdevs[conf->event_dev_id];
> +
> + adapter_id = conf->timer_adapter_id;
> +
> + /* Check adapter ID not already allocated */
> + adapter = &adapters[adapter_id];
> + if (adapter->allocated) {
> + rte_errno = -EEXIST;
> + return NULL;
> + }
> +
> + /* Create shared data area. */
> + n = snprintf(mz_name, sizeof(mz_name), DATA_MZ_NAME_FORMAT, adapter_id);
> + if (n >= (int)sizeof(mz_name)) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> + mz = rte_memzone_reserve(mz_name,
> + sizeof(struct rte_event_timer_adapter_data),
> + conf->socket_id, 0);
> + if (mz == NULL)
> + /* rte_errno set by rte_memzone_reserve */
> + return NULL;
> +
> + adapter->data = mz->addr;
> + memset(adapter->data, 0, sizeof(struct rte_event_timer_adapter_data));
> +
> + adapter->data->mz = mz;
> + adapter->data->event_dev_id = conf->event_dev_id;
> + adapter->data->id = adapter_id;
> + adapter->data->socket_id = conf->socket_id;
> + adapter->data->conf = *conf; /* copy conf structure */
> +
> + /* Query eventdev PMD for timer adapter capabilities and ops */
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + &adapter->data->caps,
> + &adapter->ops);
The underlying driver needs info about the adapter flags i.e.
RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES and RTE_EVENT_TIMER_ADAPTER_F_SP_PUT so we
need to pass those too conf->flags.
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + if (!(adapter->data->caps &
> + RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)) {
> + FUNC_PTR_OR_NULL_RET_WITH_ERRNO(conf_cb, -EINVAL);
> + ret = conf_cb(adapter->data->id, adapter->data->event_dev_id,
> + &adapter->data->event_port_id, conf_arg);
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> + }
> +
> + /* Allow driver to do some setup */
> + FUNC_PTR_OR_NULL_RET_WITH_ERRNO(adapter->ops->init, -ENOTSUP);
> + ret = adapter->ops->init(adapter);
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + /* Set fast-path function pointers */
> + adapter->arm_burst = adapter->ops->arm_burst;
> + adapter->arm_tmo_tick_burst = adapter->ops->arm_tmo_tick_burst;
> + adapter->cancel_burst = adapter->ops->cancel_burst;
> +
> + adapter->allocated = 1;
> +
> + return adapter;
> +}
> +
> +struct rte_event_timer_adapter *
> +rte_event_timer_adapter_lookup(uint16_t adapter_id)
> +{
> + char name[DATA_MZ_NAME_MAX_LEN];
> + const struct rte_memzone *mz;
> + struct rte_event_timer_adapter_data *data;
> + struct rte_event_timer_adapter *adapter;
> + int ret;
> + struct rte_eventdev *dev;
> +
> + if (adapters[adapter_id].allocated)
> + return &adapters[adapter_id]; /* Adapter is already loaded */
> +
> + snprintf(name, DATA_MZ_NAME_MAX_LEN, DATA_MZ_NAME_FORMAT, adapter_id);
> + mz = rte_memzone_lookup(name);
> + if (mz == NULL) {
> + rte_errno = -ENOENT;
> + return NULL;
> + }
> +
> + data = mz->addr;
> +
> + adapter = &adapters[data->id];
> + adapter->data = data;
> +
> + dev = &rte_eventdevs[adapter->data->event_dev_id];
> +
> + /* Query eventdev PMD for timer adapter capabilities and ops */
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + &adapter->data->caps,
> + &adapter->ops);
Same as above.
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + /* Set fast-path function pointers */
> + adapter->arm_burst = adapter->ops->arm_burst;
> + adapter->arm_tmo_tick_burst = adapter->ops->arm_tmo_tick_burst;
> + adapter->cancel_burst = adapter->ops->cancel_burst;
> +
> + adapter->allocated = 1;
> +
> + return adapter;
> +}
> +
<snip>
> +int
> +rte_event_timer_arm_burst(const struct rte_event_timer_adapter *adapter,
> + struct rte_event_timer **event_timers,
> + uint16_t nb_event_timers)
> +{
> + int ret;
> +
> + ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> + FUNC_PTR_OR_ERR_RET(adapter->arm_burst, -EINVAL);
> +
> + if (!adapter->data->started)
> + return -EAGAIN;
These checks are datapath heavy so enable them under debug compilation.
> +
> + ret = adapter->arm_burst(adapter, event_timers, nb_event_timers);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
when burst is called we need to return the number of timers successfully set
and free the failures. Return the result directly.
> +}
> +
> +int
> +rte_event_timer_arm_tmo_tick_burst(
> + const struct rte_event_timer_adapter *adapter,
> + struct rte_event_timer **event_timers,
> + const uint64_t timeout_ticks,
> + const uint16_t nb_event_timers)
> +{
> + int ret;
> +
> + ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> + FUNC_PTR_OR_ERR_RET(adapter->arm_tmo_tick_burst, -EINVAL);
> +
> + if (!adapter->data->started)
> + return -EAGAIN;
> +
> + for (int i = 0; i < nb_event_timers; i++)
> + event_timers[i]->timeout_ticks = timeout_ticks;
> +
> + ret = adapter->arm_tmo_tick_burst(adapter, event_timers, timeout_ticks,
> + nb_event_timers);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
Same as above.
> +}
> +
> +int
> +rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
> + struct rte_event_timer **event_timers,
> + uint16_t nb_event_timers)
> +{
> + int ret;
> +
> + ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> + FUNC_PTR_OR_ERR_RET(adapter->cancel_burst, -EINVAL);
> +
> + if (!adapter->data->started)
> + return -EAGAIN;
> +
> + ret = adapter->cancel_burst(adapter, event_timers, nb_event_timers);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
Same as above.
> +}
> diff --git a/lib/librte_eventdev/rte_event_timer_adapter_driver.h
> b/lib/librte_eventdev/rte_event_timer_adapter_driver.h
Consider naming this _pmd.h for consistency
> new file mode 100644
> index 0000000..485fad1
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_timer_adapter_driver.h
> @@ -0,0 +1,159 @@
Regards,
Pavan.
