Re: [PATCH libdrm 3/3] util: Add SMPTE pattern support for C4 format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ilia,

On Mon, Mar 14, 2022 at 3:39 PM Ilia Mirkin <imirkin@xxxxxxxxxxxx> wrote:
> On Mon, Mar 14, 2022 at 10:06 AM Geert Uytterhoeven
> <geert@xxxxxxxxxxxxxx> wrote:
> > On Mon, Mar 14, 2022 at 2:44 PM Ilia Mirkin <imirkin@xxxxxxxxxxxx> wrote:
> > > On Mon, Mar 14, 2022 at 9:07 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > > On Tue, Mar 8, 2022 at 8:57 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > > > On Mon, Mar 7, 2022 at 10:23 PM Ilia Mirkin <imirkin@xxxxxxxxxxxx> wrote:
> > > > > > On Mon, Mar 7, 2022 at 3:53 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > > > > > diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> > > > > > > index 953bf95492ee150c..42d75d700700dc3d 100644
> > > > > > > --- a/tests/util/pattern.c
> > > > > > > +++ b/tests/util/pattern.c
> > > > > > > @@ -608,6 +608,46 @@ static void fill_smpte_rgb16fp(const struct util_rgb_info *rgb, void *mem,
> > > > > > >  static unsigned int smpte_middle[7] = { 6, 7, 4, 7, 2, 7, 0 };
> > > > > > >  static unsigned int smpte_bottom[8] = { 8, 9, 10, 7, 11, 7, 12, 7 };
> > > > > > >
> > > > > > > +static void write_pixel_4(uint8_t *mem, unsigned int x, unsigned int pixel)
> > > > > > > +{
> > > > > > > +       if (x & 1)
> > > > > > > +               mem[x / 2] = (mem[x / 2] & 0xf0) | (pixel & 0x0f);
> > > > > > > +       else
> > > > > > > +               mem[x / 2] = (mem[x / 2] & 0x0f) | (pixel << 4);
> > > > > > > +}
> > > > > >
> > > > > > The standard layout is MSB? i.e. first pixel goes in the upper bits of
> > > > > > the first byte? It's been ages since I've dealt with C4 (or perhaps I
> > > > > > never even touched it), but this seems a bit surprising.

> > > > Turns out I was wrong: fbdev ordering follows native ordering, and
> > > > there's also FBINFO_FOREIGN_ENDIAN  :-(
> > >
> > > I haven't double-checked the meaning in fbdev, but ENDIAN-ness
> > > generally refers to the layout of *bytes*, not *bits*. Although one
> > > could also argue that it's the layout of "elements", and so in that
> > > way, upper/lower values could be considered flipped. I've never gone
> > > that far though.
> >
> > Yes, usually it refers to the ordering of bytes in a word.
> > Here, it's about the ordering of sub-byte pixels in a byte.
> > Note that with C2 and C4, there's a third ordering that comes into
> > play.
> > So we have:
> >   1. Ordering of bytes in a word, for bpp > 8,
> >   2. Ordering of pixels in a byte, for bpp < 8,
> >   3. Ordering of bits in a pixel, for bpp > 1.
> >
> > 1. Is handled by DRM_FORMAT_BIG_ENDIAN.
>
> OK. Note that DRM_FORMAT_BIG_ENDIAN flag for formats other than
> RGBX8888 and very similar formats is basically broken in drm. So ...
> watch out. There are two setups supported for big-endian currently:
>
> 1. Legacy: radeon/nouveau, ignore the "little endian" comment about
> formats and only supports AddFB, not AddFB2. The former only has
> depth/bpp, not the actual format, anyways. This matches what current
> user-space expects too. (quirk_addfb_prefer_host_byte_order = 1)
> 2. AddFB2 support with proper formats. Only used for vmwgfx and virgl
> in practice for BE, IIRC. Only supports 32-bit 8bpc formats, and uses
> some helpers to just flip around DRM_FORMAT_BIG_ENDIAN bit to an
> equivalent format in the frontend api handling. This obviously won't
> work for other formats, but none of the helpers are ready to receive
> the BIG_ENDIAN bit.

I'm fully aware the DRM_FORMAT_BIG_ENDIAN flag is broken,
and it's on my list of things to fix (for 16-bpp Atari).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux