Quoting Tvrtko Ursulin (2019-10-14 11:17:54)
>
> On 14/10/2019 10:07, Chris Wilson wrote:
> > +static ssize_t
> > +caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> > +{
> > + struct intel_engine_cs *engine = kobj_to_engine(kobj);
> > + const char * const *repr;
> > + int num_repr, n;
> > + ssize_t len;
> > +
> > + switch (engine->class) {
> > + case VIDEO_DECODE_CLASS:
> > + repr = vcs_caps;
> > + num_repr = ARRAY_SIZE(vcs_caps);
> > + break;
> > +
> > + case VIDEO_ENHANCEMENT_CLASS:
> > + repr = vecs_caps;
> > + num_repr = ARRAY_SIZE(vecs_caps);
> > + break;
> > +
> > + default:
> > + repr = NULL;
> > + num_repr = 0;
> > + break;
> > + }
> > +
> > + len = 0;
> > + for_each_set_bit(n,
> > + (unsigned long *)&engine->uabi_capabilities,
> > + BITS_PER_TYPE(typeof(engine->uabi_capabilities))) {
> > + if (GEM_WARN_ON(n >= num_repr || !repr[n]))
> > + len += snprintf(buf + len, PAGE_SIZE - len,
> > + "[%x] ", n);
> > + else
> > + len += snprintf(buf + len, PAGE_SIZE - len,
> > + "%s ", repr[n]);
> > + if (GEM_WARN_ON(len >= PAGE_SIZE))
> > + break;
> > + }
> > + return repr_trim(buf, len);
> > +}
> > +
> > +static struct kobj_attribute caps_attr =
> > +__ATTR(capabilities, 0444, caps_show, NULL);
> > +
> > +static ssize_t
> > +all_caps_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> > +{
> > + struct intel_engine_cs *engine = kobj_to_engine(kobj);
> > + const char * const *repr;
> > + int num_repr, n;
> > + ssize_t len;
> > +
> > + switch (engine->class) {
> > + case VIDEO_DECODE_CLASS:
> > + repr = vcs_caps;
> > + num_repr = ARRAY_SIZE(vcs_caps);
> > + break;
> > +
> > + case VIDEO_ENHANCEMENT_CLASS:
> > + repr = vecs_caps;
> > + num_repr = ARRAY_SIZE(vecs_caps);
> > + break;
> > +
> > + default:
> > + repr = NULL;
> > + num_repr = 0;
> > + break;
> > + }
> > +
> > + len = 0;
> > + for (n = 0; n < num_repr; n++) {
> > + if (!repr[n])
> > + continue;
> > +
> > + len += snprintf(buf + len, PAGE_SIZE - len, "%s ", repr[n]);
> > + if (GEM_WARN_ON(len >= PAGE_SIZE))
> > + break;
> > + }
> > + return repr_trim(buf, len);
> > +}
>
> Would it be worth consolidating like:
>
> __caps_show(engine, buf, len, mask, warn_unknown)
> {
> ...
> switch(engine->class...
> ...
> for_each_set_bit(...mask...) {
> if (n >= repr || !repr[n]) {
> if (warn_unknown)
> GEM_WARN_ON(...);
> else
> len += snprinf...
> } else {
> len += snprintf...
> }
> }
> ...
> }
>
> caps_show()
> {
> ...
> len = __caps_show(engine, buf, len, engine->uabi_capabilities, true);
> ...
> }
>
> all_caps_show()
> {
> ...
> len = __caps_show(engine, buf, len, ~0, false);
> ...
> }
I thought it would look ugly and distract from the intent. One is to
list the bits, the other the array of string.
But there is a certain appeal to having just one setup and loop.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
