On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote:
> On 4/1/20 4:05 PM, Sakari Ailus wrote:
> > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM
> > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so
> > the same implementation can be used.

%p4cc ?

> > +   char ch[2] = { 0 };
> 
> This can just be '{ };'

The latter is GCC extension, while above is C standard. Former is slightly
better I think. Though see below.

> > +   unsigned int i;
> > +
> > +   if (check_pointer(&buf, end, fourcc, spec))
> > +           return buf;
> > +
> > +   switch (fmt[1]) {
> > +   case 'f':

> > +           for (i = 0; i < sizeof(*fourcc); i++) {
> > +                   ch[0] = *fourcc >> (i << 3);
> 
> You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set)
> will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't
> used except to indicate a BE variant.

Why not to do it once by a flag and do reset it once?

        u32 tmp = *fourcc;
        bool be4cc = tmp & BIT(31);

        tmp &= BIT(31);

On top of that, as promised above, why not simple do it in a simpler way, i.e.
using standard idiom:

        for (i = 0; i < sizeof(*fourcc); i++) {
                if (buf < end)
                        *buf = tmp >> (i * 8);
                buf++;
        }
?

> > +                   buf = string(buf, end, ch, spec);
> > +           }
> > +
> > +           if (*fourcc & BIT(31))
> > +                   buf = string(buf, end, "-BE", spec);

Another possibility

        u8 ch[8];

        if (*fourcc & BIT(31)) {
                put_unaligned_be32(tmp, &ch[0]);
                strcpy(&ch[4], "-BE");
        } else {
                put_unaligned_le32(tmp, &ch[0]);
                strcpy(&ch[4], "-LE");
        }
        return string(buf, end, &ch[0], spec);

> > +           return buf;
> > +   default:
> > +           return error_string(buf, end, "(%pp?)", spec);
> > +   }
> > +}

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to