On Tue, Jan 23, 2024 at 10:18:53AM +0100, Mattias Rönnblom wrote: > On 2024-01-19 18:43, Bruce Richardson wrote: > > Update the device capability docs, to: > > > > * include more cross-references > > * split longer text into paragraphs, in most cases with each flag having > > a single-line summary at the start of the doc block > > * general comment rewording and clarification as appropriate > > > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com> > > --- > > lib/eventdev/rte_eventdev.h | 130 ++++++++++++++++++++++++++---------- > > 1 file changed, 93 insertions(+), 37 deletions(-) > > <snip> > > * If this capability is not set, the queue only supports events of the > > - * *RTE_SCHED_TYPE_* type that it was created with. > > + * *RTE_SCHED_TYPE_* type that it was created with. > > + * Any events of other types scheduled to the queue will handled in an > > + * implementation-dependent manner. They may be dropped by the > > + * event device, or enqueued with the scheduling type adjusted to the > > + * correct/supported value. > > Having the application setting sched_type when it was already set on a the > level of the queue never made sense to me. > > I can't see any reasons why this field shouldn't be ignored by the event > device on non-RTE_EVENT_QUEUE_CFG_ALL_TYPES queues. > > If the behavior is indeed undefined, I think it's better to just say > "undefined" rather than the above speculation. >
Updating in v3 to just say it's undefined. > > * > > - * @see RTE_SCHED_TYPE_* values <snip> > > #define RTE_EVENT_DEV_CAP_RUNTIME_QUEUE_ATTR (1ULL << 11) > > /**< Event device is capable of changing the queue attributes at runtime > > i.e > > - * after rte_event_queue_setup() or rte_event_start() call sequence. If > > this > > - * flag is not set, eventdev queue attributes can only be configured during > > + * after rte_event_queue_setup() or rte_event_dev_start() call sequence. > > + * > > + * If this flag is not set, eventdev queue attributes can only be > > configured during > > * rte_event_queue_setup(). > > "event queue" or just "queue". > Ack. > > + * > > + * @see rte_event_queue_setup > > */ > > #define RTE_EVENT_DEV_CAP_PROFILE_LINK (1ULL << 12) > > -/**< Event device is capable of supporting multiple link profiles per > > event port > > - * i.e., the value of `rte_event_dev_info::max_profiles_per_port` is > > greater > > - * than one. > > +/**< Event device is capable of supporting multiple link profiles per > > event port. > > + * > > + * > > + * When set, the value of `rte_event_dev_info::max_profiles_per_port` is > > greater > > + * than one, and multiple profiles may be configured and then switched at > > runtime. > > + * If not set, only a single profile may be configured, which may itself be > > + * runtime adjustable (if @ref RTE_EVENT_DEV_CAP_RUNTIME_PORT_LINK is set). > > + * > > + * @see rte_event_port_profile_links_set rte_event_port_profile_links_get > > + * @see rte_event_port_profile_switch > > + * @see RTE_EVENT_DEV_CAP_RUNTIME_PORT_LINK > > */ > > /* Event device priority levels */ > > #define RTE_EVENT_DEV_PRIORITY_HIGHEST 0 > > -/**< Highest priority expressed across eventdev subsystem > > +/**< Highest priority expressed across eventdev subsystem. > > "The highest priority an event device may support." > or > "The highest priority any event device may support." > > Maybe this is a further improvement, beyond punctuation? "across eventdev > subsystem" sounds awkward. > Still not very clear. Talking about device support implies that its possible some devices may not support it. How about: "highest priority level for events and queues".
