[PATCH v2 03/11] eventdev: update documentation on device capability flags

Bruce Richardson bruce.richardson at intel.com
Wed Jan 31 15:09:02 CET 2024


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.richardson at 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".



More information about the dev mailing list