> -----Original Message-----
> From: McDaniel, Timothy <timothy.mcdan...@intel.com>
> Sent: Friday, September 11, 2020 3:26 PM
> Cc: dev@dpdk.org; Carrillo, Erik G <erik.g.carri...@intel.com>; Eads, Gage
> <gage.e...@intel.com>; Van Haaren, Harry <harry.van.haa...@intel.com>;
> jer...@marvell.com
> Subject: [PATCH 03/22] event/dlb2: add private data structures and constants
>
> The header file dlb2_priv.h is used internally by the PMD.
> It include constants, macros for device resources,
> structure definitions for hardware interfaces and
> software state, and various forward-declarations.
> The header file rte_pmd_dlb2.h will be exported in a
> subsequent patch, but is included here due to a data
> structure dependency.
>
> Signed-off-by: Timothy McDaniel <timothy.mcdan...@intel.com>
> ---
> drivers/event/dlb2/dlb2_priv.h | 614
> ++++++++++++++++++++++++++++++++++++++
> drivers/event/dlb2/rte_pmd_dlb2.h | 59 ++++
> 2 files changed, 673 insertions(+)
> create mode 100644 drivers/event/dlb2/dlb2_priv.h
> create mode 100644 drivers/event/dlb2/rte_pmd_dlb2.h
>
> diff --git a/drivers/event/dlb2/dlb2_priv.h b/drivers/event/dlb2/dlb2_priv.h
> new file mode 100644
> index 0000000..7bec835
> --- /dev/null
> +++ b/drivers/event/dlb2/dlb2_priv.h
> @@ -0,0 +1,614 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2016-2020 Intel Corporation
> + */
> +
> +#ifndef _DLB2_PRIV_H_
> +#define _DLB2_PRIV_H_
> +
> +#include <emmintrin.h>
> +#include <stdbool.h>
> +
> +#include <rte_eventdev.h>
> +
> +#include "rte_config.h"
This should be included with angle brackets and immediately follow
rte_eventdev.h.
[...]
> +/* Do not change - must match hardware! */
> +enum dlb2_hw_sched_type {
> + DLB2_SCHED_ATOMIC = 0,
> + DLB2_SCHED_UNORDERED,
> + DLB2_SCHED_ORDERED,
> + DLB2_SCHED_DIRECTED,
> + /* DLB2_NUM_HW_SCHED_TYPES must be last */
> + DLB2_NUM_HW_SCHED_TYPES
> +};
> +
> +/* TODO - these structs have been converted from qm - some fields may not
> be
> + * required
> + */
Leftover TODO -- still valid?
> +
> +struct dlb2_hw_rsrcs {
> + int32_t nb_events_limit;
> + uint32_t num_queues; /**> Total queues (lb + dir) */
I don't believe "**>" is correct doxygen format (angle bracket's facing the
wrong
direction), but since this is an internal/private header file it won't be
included in
the doxygen output anyway. I'd recommend either correcting the inconsistency
in angle brackets or just dropping the doxygen style here altogether.
> + uint32_t num_ldb_queues; /**> Number of available ldb queues */
> + uint32_t num_ldb_ports; /**< Number of load balanced ports */
> + uint32_t num_dir_ports; /**< Number of directed ports */
> + uint32_t num_ldb_credits; /**< Number of load balanced credits */
> + uint32_t num_dir_credits; /**< Number of directed credits */
> + uint32_t reorder_window_size; /**< Size of reorder window */
> +};
> +
> +struct dlb2_hw_resource_info {
> + /**> Max resources that can be provided */
> + struct dlb2_hw_rsrcs hw_rsrc_max;
> + int num_sched_domains;
> + uint32_t socket_id;
> + /**> EAL flags passed to this QM instance, allowing the application to
> + * identify the pmd backend indicating hardware or software.
> + */
> + const char *eal_flags;
Doesn't look like eal_flags is used in any of the later patches.
[...]
> +
> +enum DLB2_PORT_STATE {
> + PORT_CLOSED,
> + PORT_STARTED,
> + PORT_STOPPED
> +};
Nit: other enum names in this file are lower-case.
> +
> +enum dlb2_configuration_state {
> + /* The resource has not been configured */
> + DLB2_NOT_CONFIGURED,
> + /* The resource was configured, but the device was stopped */
> + DLB2_PREV_CONFIGURED,
> + /* The resource is currently configured */
> + DLB2_CONFIGURED
> +};
> +
> +struct dlb2_port {
> + uint32_t id;
> + bool is_directed;
> + bool gen_bit;
> + uint16_t dir_credits;
> + uint32_t dequeue_depth;
> + enum dlb2_token_pop_mode token_pop_mode;
> + union dlb2_port_config cfg;
> + uint32_t *credit_pool[DLB2_NUM_QUEUE_TYPES]; /* use __atomic
> builtins */
> + uint16_t cached_ldb_credits;
> + uint16_t ldb_credits;
> + uint16_t cached_dir_credits;
> + bool int_armed;
> + uint16_t owed_tokens;
> + int16_t issued_releases;
> + int16_t token_pop_thresh;
> + int cq_depth;
> + uint16_t cq_idx;
> + uint16_t cq_idx_unmasked;
> + uint16_t cq_depth_mask;
> + uint16_t gen_bit_shift;
> + enum DLB2_PORT_STATE state;
> + enum dlb2_configuration_state config_state;
> + int num_mapped_qids;
> + uint8_t *qid_mappings;
> + struct dlb2_enqueue_qe *qe4; /* Cache line's worth of QEs (4) */
> + struct dlb2_enqueue_qe *int_arm_qe;
> + struct dlb2_cq_pop_qe *consume_qe;
> + struct dlb2_eventdev *dlb2; /* back ptr */
> + struct dlb2_eventdev_port *ev_port; /* back ptr */
> +};
> +
> +/* Per-process per-port mmio and memory pointers */
> +struct process_local_port_data {
> + uint64_t *pp_addr;
> + struct dlb2_dequeue_qe *cq_base;
> + bool mmaped;
> +};
> +
> +struct dlb2_eventdev;
> +
> +struct dlb2_port_low_level_io_functions {
> + void (*pp_enqueue_four)(void *qe4,
> + void *pp_addr);
In the DLB PMD v4 patchset, I noticed the pp_enqueue_four indirection is dropped
in favor of directly calling the MOVDIR64 function -- can that be done here as
well?
> +};
> +
> +/* TODO - Optimize memory use and layout */
Leftover TODO
> +struct dlb2_config {
> + int configured;
> + int reserved;
> + uint32_t num_ldb_credits;
> + uint32_t num_dir_credits;
> + struct dlb2_create_sched_domain_args resources;
> +};
> +
> +enum dlb2_cos {
> + DLB2_COS_DEFAULT = -1,
> + DLB2_COS_0 = 0,
> + DLB2_COS_1,
> + DLB2_COS_2,
> + DLB2_COS_3
> +};
> +
> +struct dlb2_hw_dev {
> + char device_name[DLB2_NAME_SIZE];
> + char device_path[DLB2_MAX_DEVICE_PATH];
> + int device_path_id;
> + struct dlb2_config cfg;
> + struct dlb2_hw_resource_info info;
> + void *pf_dev; /* opaque pointer to PF PMD dev (struct dlb2_dev) */
> + int device_id;
> + uint32_t domain_id;
> + int domain_id_valid;
> + enum dlb2_cos cos_id;
> + rte_spinlock_t resource_lock; /* for MP support */
> +}; __rte_cache_aligned
Attribute needs to come before the semi-colon.
> +
> +/* End HW related defines and structs */
> +
> +/* Begin DLB2 PMD Eventdev related defines and structs */
> +
> +#define DLB2_MAX_NUM_QUEUES \
> + (DLB2_MAX_NUM_DIR_QUEUES + DLB2_MAX_NUM_LDB_QUEUES)
> +
> +#define DLB2_MAX_NUM_PORTS (DLB2_MAX_NUM_DIR_PORTS +
> DLB2_MAX_NUM_LDB_PORTS)
> +#define DLB2_MAX_INPUT_QUEUE_DEPTH 256
> +
> +/* Used for parsing dir ports/queues. */
Does this comment refer to the following code? Seems out of place.
> +
> +/* Note: eventdev currently is limited to 64 ports, but DLB hardware has 128
> + * directed ports/queues.
> + */
Does this refer to RTE_EVENT_MAX_QUEUES_PER_DEV? If so, looks like the change
in the DLB PMD to increase that to 255 makes this comment unnecessary.
> +struct dlb2_dir_resource_list {
> + int entries;
> + uint32_t id[DLB2_MAX_NUM_DIR_PORTS];
> +};
Doesn't look this structure is used elsewhere in this series.
[...]
> +struct dlb2_eventdev_queue {
> + struct dlb2_queue qm_queue;
> + struct rte_event_queue_conf conf; /* User config */
> + int depth_threshold; /* use default if 0 */
> + uint32_t id;
> + bool setup_done;
> + uint8_t num_links;
> +};
> +
> +struct dlb2_device_stats {
> + /* Device specific */
> +};
Any need for this, since it's empty?
> +
> +enum dlb2_run_state {
> + DLB2_RUN_STATE_STOPPED = 0,
> + DLB2_RUN_STATE_STOPPING,
> + DLB2_RUN_STATE_STARTING,
> + DLB2_RUN_STATE_STARTED
> +};
> +
> +struct dlb2_eventdev {
> + struct dlb2_eventdev_port ev_ports[DLB2_MAX_NUM_PORTS];
> + struct dlb2_eventdev_queue ev_queues[DLB2_MAX_NUM_QUEUES];
> + uint8_t qm_ldb_to_ev_queue_id[DLB2_MAX_NUM_QUEUES];
> + uint8_t qm_dir_to_ev_queue_id[DLB2_MAX_NUM_QUEUES];
> + /* store num stats and offset of the stats for each queue */
> + uint16_t xstats_count_per_qid[DLB2_MAX_NUM_QUEUES];
> + uint16_t xstats_offset_for_qid[DLB2_MAX_NUM_QUEUES];
> + /* store num stats and offset of the stats for each port */
> + uint16_t xstats_count_per_port[DLB2_MAX_NUM_PORTS];
> + uint16_t xstats_offset_for_port[DLB2_MAX_NUM_PORTS];
> + struct dlb2_get_num_resources_args hw_rsrc_query_results;
> + uint32_t xstats_count_mode_queue;
> + struct dlb2_hw_dev qm_instance; /* strictly hw related */
> + uint64_t global_dequeue_wait_ticks;
> + struct dlb2_xstats_entry *xstats;
> + struct rte_eventdev *event_dev; /* backlink to dev */
> + uint32_t xstats_count_mode_dev;
> + uint32_t xstats_count_mode_port;
> + uint32_t xstats_count;
> + uint32_t inflights; /* use __atomic builtins */
> + uint32_t new_event_limit;
> + int max_num_events_override;
> + int num_dir_credits_override;
> + volatile enum dlb2_run_state run_state;
> + uint16_t num_dir_queues; /* total num of evdev dir queues requested
> */
> + uint16_t num_dir_credits;
> + uint16_t num_ldb_credits;
> + uint16_t num_queues; /* total queues */
> + uint16_t num_ldb_queues; /* total num of evdev ldb queues requested
> */
> + uint16_t num_ports; /* total num of evdev ports requested */
> + uint16_t num_ldb_ports; /* total num of ldb ports requested */
> + uint16_t num_dir_ports; /* total num of dir ports requested */
> + bool umwait_allowed;
> + bool global_dequeue_wait; /* Not using per dequeue wait if true */
> + bool defer_sched;
> + enum dlb2_cq_poll_modes poll_mode;
> + uint8_t revision;
> + bool configured;
> + uint16_t max_ldb_credits;
> + uint16_t max_dir_credits;
> +
> + /* force hw credit pool counters into exclusive cache lines */
> +
> + /* use __atomic builtins */ /* shared hw cred */
> + uint32_t ldb_credit_pool __rte_cache_aligned;
> + /* use __atomic builtins */ /* shared hw cred */
> + uint32_t dir_credit_pool __rte_cache_aligned;
> +
> + /* Device stats */
> + struct dlb2_device_stats stats __rte_cache_aligned;
> +};
> +
> +/* used for collecting and passing around the dev args */
> +struct dlb2_qid_depth_thresholds {
> + int val[DLB2_MAX_NUM_QUEUES];
> +};
Nit: whitespace between the struct definitions, to match the rest of the file.
> +struct dlb2_devargs {
> + int socket_id;
> + int max_num_events;
> + int num_dir_credits_override;
> + int dev_id;
> + int defer_sched;
> + struct dlb2_qid_depth_thresholds qid_depth_thresholds;
> + enum dlb2_cos cos_id;
> +};
> +
> +/* End Eventdev related defines and structs */
> +
> +/* Forwards for non-inlined functions */
> +
> +int dlb2_uninit(const char *name);
This function is never defined
> +
> +void dlb2_eventdev_dump(struct rte_eventdev *dev, FILE *f);
> +
> +int dlb2_xstats_init(struct dlb2_eventdev *dlb2);
> +
> +void dlb2_xstats_uninit(struct dlb2_eventdev *dlb2);
> +
> +int dlb2_eventdev_xstats_get(const struct rte_eventdev *dev,
> + enum rte_event_dev_xstats_mode mode, uint8_t
> queue_port_id,
> + const unsigned int ids[], uint64_t values[], unsigned int n);
> +
> +int dlb2_eventdev_xstats_get_names(const struct rte_eventdev *dev,
> + enum rte_event_dev_xstats_mode mode, uint8_t
> queue_port_id,
> + struct rte_event_dev_xstats_name *xstat_names,
> + unsigned int *ids, unsigned int size);
> +
> +uint64_t dlb2_eventdev_xstats_get_by_name(const struct rte_eventdev *dev,
> + const char *name, unsigned int *id);
> +
> +int dlb2_eventdev_xstats_reset(struct rte_eventdev *dev,
> + enum rte_event_dev_xstats_mode mode,
> + int16_t queue_port_id,
> + const uint32_t ids[],
> + uint32_t nb_ids);
> +
> +int test_dlb2_eventdev(void);
> +
> +int dlb2_primary_eventdev_probe(struct rte_eventdev *dev,
> + const char *name,
> + struct dlb2_devargs *dlb2_args);
> +
> +int dlb2_secondary_eventdev_probe(struct rte_eventdev *dev,
> + const char *name);
> +
> +uint32_t dlb2_get_queue_depth(struct dlb2_eventdev *dlb2,
> + struct dlb2_eventdev_queue *queue);
> +
> +/* arg parsing helper functions */
> +int dlb2_parse_params(const char *params,
> + const char *name,
> + struct dlb2_devargs *dlb2_args);
> +
> +int dlb2_string_to_int(int *result, const char *str);
Looks like this is only used in dlb2.c, is the forward declaration needed?
Thanks,
Gage
