Re: [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

Re: [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Daniel Vetter]

On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
>
>> Hm, where do we have the canonical source for all these fourcc codes? I'm
>> asking since we have our own copy in the kernel as drm_fourcc.h, and that
>> one is part of the userspace ABI since we use it to pass around
>> framebuffer formats and format lists.
>
> I think it's the kernel? I really don't know, as the whole notion of
> fourcc codes seems crazy to me...
>
> Feel free to steal this code and stick it in the kernel if you like.

Well, I wasn't ever in favour of using fourcc codes since they're just
not standardized at all, highly redundant in some cases and also miss
lots of stuff we actually need (like all the rgb formats).

Cc'ing the heck out of this to get kernel people to hopefully notice.
Maybe someone takes charge of this ... Otherwise meh.

>> Just afraid to create long-term maintainance madness here with the
>> kernel's iron thou-shalt-not-break-userspace-ever rule ... Not likely
>> we'll ever accept srgb for framebuffers though.
>
> Would suck to collide with something we do want though.

Yeah, it'd suck. But given how fourcc works we probably have that
already, just haven't noticed yet :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Ville Syrjälä]

On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote:
> On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> >> Hm, where do we have the canonical source for all these fourcc codes? I'm
> >> asking since we have our own copy in the kernel as drm_fourcc.h, and that
> >> one is part of the userspace ABI since we use it to pass around
> >> framebuffer formats and format lists.
> >
> > I think it's the kernel? I really don't know, as the whole notion of
> > fourcc codes seems crazy to me...
> >
> > Feel free to steal this code and stick it in the kernel if you like.
> 
> Well, I wasn't ever in favour of using fourcc codes since they're just
> not standardized at all, highly redundant in some cases and also miss
> lots of stuff we actually need (like all the rgb formats).

I also argued against them, but some people wanted them for whatever
reason. And since I didn't want to argue for several years about the
subject, I just gave in and made the drm pixel formats fourcss. But
given that I just pulled the fourccs out of my ass, we can't really
cross use them between different subsystems anyway. So if we just
consider all the different fourcc namespaces totally distinct, we're
not going to have any problems.

Personally I can promise that I will _not_ be checking Mesa/whatever
code for conflicting fourccs when I need to add a new one to drm_fourcc.h.
There, now I've given fair warning and if things explode later it won't be
my fault.

However if someone wants to emulate the drm fourcc style for whatever
reason, there is a distinct pattern how I cooked them up. Well, a few
different patterns depending whether it's RGB,YUV,packed,planar etc.

> 
> Cc'ing the heck out of this to get kernel people to hopefully notice.
> Maybe someone takes charge of this ... Otherwise meh.
> 
> >> Just afraid to create long-term maintainance madness here with the
> >> kernel's iron thou-shalt-not-break-userspace-ever rule ... Not likely
> >> we'll ever accept srgb for framebuffers though.
> >
> > Would suck to collide with something we do want though.
> 
> Yeah, it'd suck. But given how fourcc works we probably have that
> already, just haven't noticed yet :(
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Kristian Høgsberg]

On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote:
> On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> >> Hm, where do we have the canonical source for all these fourcc codes? I'm
> >> asking since we have our own copy in the kernel as drm_fourcc.h, and that
> >> one is part of the userspace ABI since we use it to pass around
> >> framebuffer formats and format lists.
> >
> > I think it's the kernel? I really don't know, as the whole notion of
> > fourcc codes seems crazy to me...
> >
> > Feel free to steal this code and stick it in the kernel if you like.
> 
> Well, I wasn't ever in favour of using fourcc codes since they're just
> not standardized at all, highly redundant in some cases and also miss
> lots of stuff we actually need (like all the rgb formats).

These drm codes are not fourcc codes in any other way than that
they're defined by creating a 32 bit value by picking four characters.
I don't know what PTSD triggers people have from hearing "fourcc", but
it seems severe.  Forget all that, these codes are DRM specific
defines that are not inteded to match anything anybody else does.  It
doesn't matter if these match of conflict with v4l, fourcc.org,
wikipedia.org or what the amiga did.  They're just tokens that let us
define succintly what the pixel format of a kms framebuffer is and
tell the kernel.

I don't know what else you'd propose?  Pass an X visual in the ioctl?
An EGL config?  This is our name space, we can add stuff as we need
(as Keith is doing here). include/uapi/drm/drm_fourcc.h is the
canonical source for these values and we should add
DRM_FORMAT_SARGB8888 there to make sure we don't clash.

Why are these codes in mesa (and gbm and wl_drm protocol) then?
Because it turns out that once you have an stable and established
namespace for pixel formats (and a kernel userspace header is about as
stable and established as it gets) it makes a lot of sense to reuse
those values.

I already explained to Keith why we use different sets of format codes
in the DRI interface, but it's always fun to slam other peoples code.
Anyway, it's pretty simple, the __DRI_IMAGE_FORMAT_* defines predate
the introduction of drm_fourcc.h.  When we later added suport for
planar YUV __DRIimages, the kernel had picked up drm_fourcc.h after a
long sad bikeshedding flamewar, which included the planar formats we
needed.  At this point we could continue using our custom
__DRI_IMAGE_FORMAT_* defines or we could switch to the tokens that we
had finally converged on.  But don't let me ruin a good old snide remark.

> Cc'ing the heck out of this to get kernel people to hopefully notice.
> Maybe someone takes charge of this ... Otherwise meh.

I don't know what you want to change.  These values are already kernel
ABI, we use them in drmAddFB2, and again, I don't understand what
problem you're seeing.

Kristian

> >> Just afraid to create long-term maintainance madness here with the
> >> kernel's iron thou-shalt-not-break-userspace-ever rule ... Not likely
> >> we'll ever accept srgb for framebuffers though.
> >
> > Would suck to collide with something we do want though.
> 
> Yeah, it'd suck. But given how fourcc works we probably have that
> already, just haven't noticed yet :(
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Ville Syrjälä]

On Fri, Nov 22, 2013 at 02:12:13PM -0800, Kristian Høgsberg wrote:
> On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard <keithp@keithp.com> wrote:
> > > Daniel Vetter <daniel@ffwll.ch> writes:
> > >
> > >> Hm, where do we have the canonical source for all these fourcc codes? I'm
> > >> asking since we have our own copy in the kernel as drm_fourcc.h, and that
> > >> one is part of the userspace ABI since we use it to pass around
> > >> framebuffer formats and format lists.
> > >
> > > I think it's the kernel? I really don't know, as the whole notion of
> > > fourcc codes seems crazy to me...
> > >
> > > Feel free to steal this code and stick it in the kernel if you like.
> > 
> > Well, I wasn't ever in favour of using fourcc codes since they're just
> > not standardized at all, highly redundant in some cases and also miss
> > lots of stuff we actually need (like all the rgb formats).
> 
> These drm codes are not fourcc codes in any other way than that
> they're defined by creating a 32 bit value by picking four characters.
> I don't know what PTSD triggers people have from hearing "fourcc", but
> it seems severe.  Forget all that, these codes are DRM specific
> defines that are not inteded to match anything anybody else does.  It
> doesn't matter if these match of conflict with v4l, fourcc.org,
> wikipedia.org or what the amiga did.  They're just tokens that let us
> define succintly what the pixel format of a kms framebuffer is and
> tell the kernel.
> 
> I don't know what else you'd propose?  Pass an X visual in the ioctl?
> An EGL config?  This is our name space, we can add stuff as we need
> (as Keith is doing here). include/uapi/drm/drm_fourcc.h is the
> canonical source for these values and we should add
> DRM_FORMAT_SARGB8888 there to make sure we don't clash.

What is this format anyway? -ENODOCS

If its just an srgb version of ARGB8888, then I wouldn't really want it
in drm_fourcc.h. I expect colorspacy stuff will be handled by various
crtc/plane properties in the kernel so we don't need to encode that
stuff into the fb format.

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

Ville Syrjälä <ville.syrjala@linux.intel.com> writes:

> What is this format anyway? -ENODOCS

Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-)

> If its just an srgb version of ARGB8888, then I wouldn't really want it
> in drm_fourcc.h. I expect colorspacy stuff will be handled by various
> crtc/plane properties in the kernel so we don't need to encode that
> stuff into the fb format.

It's not any different from splitting YUV codes from RGB codes; a
different color encoding with the same bitfields. And, for mesa, it's
necessary to differentiate between ARGB and SARGB within the same format
code given how the API is structured. So, we have choices:

 1) Let Mesa define it's own fourcc codes and risk future accidental
    collisions
 
 2) Rewrite piles of mesa code to split out the color encoding from the
    bitfield information and pass it separately.

 3) Add "reasonable" format codes like this to the kernel fourcc list.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Ville Syrjälä]

On Fri, Nov 22, 2013 at 03:43:13PM -0800, Keith Packard wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> 
> > What is this format anyway? -ENODOCS
> 
> Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-)
> 
> > If its just an srgb version of ARGB8888, then I wouldn't really want it
> > in drm_fourcc.h. I expect colorspacy stuff will be handled by various
> > crtc/plane properties in the kernel so we don't need to encode that
> > stuff into the fb format.
> 
> It's not any different from splitting YUV codes from RGB codes;

Not really. Saying something is YUV (or rather Y'CbCr) doesn't
actually tell you the color space. It just tells you whether the
information is encoded as R+G+B or Y+Cb+Cr. How you convert between
them is another matter. You need to know the gamma, color primaries,
chroma siting for sub-sampled YCbCr formats, etc.

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Geert Uytterhoeven]

On Sat, Nov 23, 2013 at 2:10 AM, Ville Syrj채l채
<ville.syrjala@linux.intel.com> wrote:
> On Fri, Nov 22, 2013 at 03:43:13PM -0800, Keith Packard wrote:
>> Ville Syrj채l채 <ville.syrjala@linux.intel.com> writes:
>>
>> > What is this format anyway? -ENODOCS
>>
>> Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-)
>>
>> > If its just an srgb version of ARGB8888, then I wouldn't really want it
>> > in drm_fourcc.h. I expect colorspacy stuff will be handled by various
>> > crtc/plane properties in the kernel so we don't need to encode that
>> > stuff into the fb format.
>>
>> It's not any different from splitting YUV codes from RGB codes;
>
> Not really. Saying something is YUV (or rather Y'CbCr) doesn't
> actually tell you the color space. It just tells you whether the
> information is encoded as R+G+B or Y+Cb+Cr. How you convert between
> them is another matter. You need to know the gamma, color primaries,
> chroma siting for sub-sampled YCbCr formats, etc.

Yep. Fbdev has a separation of type (how pixel values are laid out in memory),
fb_bitfield structs (how tuples are formed into pixels), and visual (how to
interprete the tuples).

The fb_bitfield structs do have RGB-centric names, but you could use them
for e.g. Y, Cb, Cr, and alpha, giving a proper visual. Unfortunately the
YCbCr visuals haven't made it into mainline.

FOURCC unifies all of that in (not so) unique 32-bit IDs.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]

Kristian Høgsberg <hoegsberg@gmail.com> writes:

> I already explained to Keith why we use different sets of format codes
> in the DRI interface, but it's always fun to slam other peoples code.

As we discussed, my complaint isn't so much about __DRI_IMAGE_FOURCC,
but the fact that the __DRIimage interfaces use *both*
__DRI_IMAGE_FOURCC and __DRI_IMAGE_FORMAT at different times.

Ok, here's a fine thing we can actually fix -- the pattern that mesa
uses all over the place in converting formats looks like this (not to
pick on anyone, it's repeated everywhere, this is just the first one I
found in gbm_dri.c):

	static uint32_t
        gbm_dri_to_gbm_format(uint32_t dri_format)
	{
	   uint32_t ret = 0;
	
	   switch (dri_format) {
	   case __DRI_IMAGE_FORMAT_RGB565:
	      ret = GBM_FORMAT_RGB565;
	      break;
	   case __DRI_IMAGE_FORMAT_XRGB8888:
	      ret = GBM_FORMAT_XRGB8888;
	      break;
	   case __DRI_IMAGE_FORMAT_ARGB8888:
	      ret = GBM_FORMAT_ARGB8888;
	      break;
	   case __DRI_IMAGE_FORMAT_ABGR8888:
	      ret = GBM_FORMAT_ABGR8888;
	      break;
	   default:
	      ret = 0;
	      break;
	   }
	
	   return ret;
	}

The problem here is that any unknown incoming formats get translated to
garbage (0) outgoing. With fourcc codes, there is the slight advantage
that 0 is never a legal value, but it sure would be nice to print a
warning or even abort if you get a format code you don't understand as
there's no way 0 is ever going to do what you want.

Anyone have a preference? Abort? Print an error? Silently continue to do
the wrong thing?

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Daniel Vetter]

On Fri, Nov 22, 2013 at 02:12:13PM -0800, Kristian Høgsberg wrote:
> I don't know what else you'd propose?  Pass an X visual in the ioctl?
> An EGL config?  This is our name space, we can add stuff as we need
> (as Keith is doing here). include/uapi/drm/drm_fourcc.h is the
> canonical source for these values and we should add
> DRM_FORMAT_SARGB8888 there to make sure we don't clash.

Well that's kinda the problem. If you don't expect the kernel to clash
with whatever mesa is using internally then we should add it to the
kernel, first. That's kinda what Dave's recent rant has all been about.

The other issue was that originally the idea behind fourcc was to have one
formate namespace shared between drm, v4l and whomever else cares. If
people are happy to drop that idea on the floor I won't shed a single
tear.

In the end I'll expect that someone will make a funny collision between
all the different projects anyway, so meh.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Ville Syrjälä]

On Mon, Nov 25, 2013 at 09:57:23AM +0100, Daniel Vetter wrote:
> On Fri, Nov 22, 2013 at 02:12:13PM -0800, Kristian Høgsberg wrote:
> > I don't know what else you'd propose?  Pass an X visual in the ioctl?
> > An EGL config?  This is our name space, we can add stuff as we need
> > (as Keith is doing here). include/uapi/drm/drm_fourcc.h is the
> > canonical source for these values and we should add
> > DRM_FORMAT_SARGB8888 there to make sure we don't clash.
> 
> Well that's kinda the problem. If you don't expect the kernel to clash
> with whatever mesa is using internally then we should add it to the
> kernel, first. That's kinda what Dave's recent rant has all been about.
> 
> The other issue was that originally the idea behind fourcc was to have one
> formate namespace shared between drm, v4l and whomever else cares. If
> people are happy to drop that idea on the floor I won't shed a single
> tear.

I broke that idea alredy when I cooked up the current drm fourccs.
I didn't cross check them with any other fourcc source, so I'm 100%
sure most of them don't match anything else.

-- 
Ville Syrjälä
Intel OTC