[PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Keith Packard]

The __DRIimage createImageFromFds function takes a fourcc code, but there was
no fourcc code that match __DRI_IMAGE_FORMAT_SARGB8. This adds a define for
that format, adds a translation in DRI3 from __DRI_IMAGE_FORMAT_SARGB8 to
__DRI_IMAGE_FOURCC_SARGB8888 and then adds translations *back* to
__IMAGE_FORMAT_SARGB8 in both the i915 and i965 drivers.

I'll refrain from comments on whether I think having two separate sets of
format defines in dri_interface.h is a good idea or not...

Signed-off-by: Keith Packard <keithp@keithp.com>
---

This gets iceweasel running with the GL compositor enabled.

 include/GL/internal/dri_interface.h      | 1 +
 src/glx/dri3_glx.c                       | 1 +
 src/mesa/drivers/dri/i915/intel_screen.c | 3 +++
 src/mesa/drivers/dri/i965/intel_screen.c | 3 +++
 4 files changed, 8 insertions(+)

diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
index b012570..a4387c4 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1034,6 +1034,7 @@ struct __DRIdri2ExtensionRec {
 #define __DRI_IMAGE_FOURCC_XRGB8888	0x34325258
 #define __DRI_IMAGE_FOURCC_ABGR8888	0x34324241
 #define __DRI_IMAGE_FOURCC_XBGR8888	0x34324258
+#define __DRI_IMAGE_FOURCC_SARGB8888    0x83324258
 #define __DRI_IMAGE_FOURCC_YUV410	0x39565559
 #define __DRI_IMAGE_FOURCC_YUV411	0x31315559
 #define __DRI_IMAGE_FOURCC_YUV420	0x32315559
diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index b047cc8..5861317 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -890,6 +890,7 @@ image_format_to_fourcc(int format)
 
    /* Convert from __DRI_IMAGE_FORMAT to __DRI_IMAGE_FOURCC (sigh) */
    switch (format) {
+   case __DRI_IMAGE_FORMAT_SARGB8: return __DRI_IMAGE_FOURCC_SARGB8888;
    case __DRI_IMAGE_FORMAT_RGB565: return __DRI_IMAGE_FOURCC_RGB565;
    case __DRI_IMAGE_FORMAT_XRGB8888: return __DRI_IMAGE_FOURCC_XRGB8888;
    case __DRI_IMAGE_FORMAT_ARGB8888: return __DRI_IMAGE_FOURCC_ARGB8888;
diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
index 7f1fc6b..2dd2bc7 100644
--- a/src/mesa/drivers/dri/i915/intel_screen.c
+++ b/src/mesa/drivers/dri/i915/intel_screen.c
@@ -184,6 +184,9 @@ static struct intel_image_format intel_image_formats[] = {
    { __DRI_IMAGE_FOURCC_ARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
      { { 0, 0, 0, __DRI_IMAGE_FORMAT_ARGB8888, 4 } } },
 
+   { __DRI_IMAGE_FOURCC_SARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
+     { { 0, 0, 0, __DRI_IMAGE_FORMAT_SARGB8, 4 } } },
+
    { __DRI_IMAGE_FOURCC_XRGB8888, __DRI_IMAGE_COMPONENTS_RGB, 1,
      { { 0, 0, 0, __DRI_IMAGE_FORMAT_XRGB8888, 4 }, } },
 
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index e44d0f6..cf8c3e2 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -220,6 +220,9 @@ static struct intel_image_format intel_image_formats[] = {
    { __DRI_IMAGE_FOURCC_ARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
      { { 0, 0, 0, __DRI_IMAGE_FORMAT_ARGB8888, 4 } } },
 
+   { __DRI_IMAGE_FOURCC_SARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
+     { { 0, 0, 0, __DRI_IMAGE_FORMAT_SARGB8, 4 } } },
+
    { __DRI_IMAGE_FOURCC_XRGB8888, __DRI_IMAGE_COMPONENTS_RGB, 1,
      { { 0, 0, 0, __DRI_IMAGE_FORMAT_XRGB8888, 4 }, } },
 
-- 
1.8.4.2

Re: [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Daniel Vetter]

On Thu, Nov 21, 2013 at 08:12:04PM -0800, Keith Packard wrote:
> The __DRIimage createImageFromFds function takes a fourcc code, but there was
> no fourcc code that match __DRI_IMAGE_FORMAT_SARGB8. This adds a define for
> that format, adds a translation in DRI3 from __DRI_IMAGE_FORMAT_SARGB8 to
> __DRI_IMAGE_FOURCC_SARGB8888 and then adds translations *back* to
> __IMAGE_FORMAT_SARGB8 in both the i915 and i965 drivers.
> 
> I'll refrain from comments on whether I think having two separate sets of
> format defines in dri_interface.h is a good idea or not...
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

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.

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.
-Daniel

> ---
> 
> This gets iceweasel running with the GL compositor enabled.
> 
>  include/GL/internal/dri_interface.h      | 1 +
>  src/glx/dri3_glx.c                       | 1 +
>  src/mesa/drivers/dri/i915/intel_screen.c | 3 +++
>  src/mesa/drivers/dri/i965/intel_screen.c | 3 +++
>  4 files changed, 8 insertions(+)
> 
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index b012570..a4387c4 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -1034,6 +1034,7 @@ struct __DRIdri2ExtensionRec {
>  #define __DRI_IMAGE_FOURCC_XRGB8888	0x34325258
>  #define __DRI_IMAGE_FOURCC_ABGR8888	0x34324241
>  #define __DRI_IMAGE_FOURCC_XBGR8888	0x34324258
> +#define __DRI_IMAGE_FOURCC_SARGB8888    0x83324258
>  #define __DRI_IMAGE_FOURCC_YUV410	0x39565559
>  #define __DRI_IMAGE_FOURCC_YUV411	0x31315559
>  #define __DRI_IMAGE_FOURCC_YUV420	0x32315559
> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
> index b047cc8..5861317 100644
> --- a/src/glx/dri3_glx.c
> +++ b/src/glx/dri3_glx.c
> @@ -890,6 +890,7 @@ image_format_to_fourcc(int format)
>  
>     /* Convert from __DRI_IMAGE_FORMAT to __DRI_IMAGE_FOURCC (sigh) */
>     switch (format) {
> +   case __DRI_IMAGE_FORMAT_SARGB8: return __DRI_IMAGE_FOURCC_SARGB8888;
>     case __DRI_IMAGE_FORMAT_RGB565: return __DRI_IMAGE_FOURCC_RGB565;
>     case __DRI_IMAGE_FORMAT_XRGB8888: return __DRI_IMAGE_FOURCC_XRGB8888;
>     case __DRI_IMAGE_FORMAT_ARGB8888: return __DRI_IMAGE_FOURCC_ARGB8888;
> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
> index 7f1fc6b..2dd2bc7 100644
> --- a/src/mesa/drivers/dri/i915/intel_screen.c
> +++ b/src/mesa/drivers/dri/i915/intel_screen.c
> @@ -184,6 +184,9 @@ static struct intel_image_format intel_image_formats[] = {
>     { __DRI_IMAGE_FOURCC_ARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
>       { { 0, 0, 0, __DRI_IMAGE_FORMAT_ARGB8888, 4 } } },
>  
> +   { __DRI_IMAGE_FOURCC_SARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_SARGB8, 4 } } },
> +
>     { __DRI_IMAGE_FOURCC_XRGB8888, __DRI_IMAGE_COMPONENTS_RGB, 1,
>       { { 0, 0, 0, __DRI_IMAGE_FORMAT_XRGB8888, 4 }, } },
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index e44d0f6..cf8c3e2 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -220,6 +220,9 @@ static struct intel_image_format intel_image_formats[] = {
>     { __DRI_IMAGE_FOURCC_ARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
>       { { 0, 0, 0, __DRI_IMAGE_FORMAT_ARGB8888, 4 } } },
>  
> +   { __DRI_IMAGE_FOURCC_SARGB8888, __DRI_IMAGE_COMPONENTS_RGBA, 1,
> +     { { 0, 0, 0, __DRI_IMAGE_FORMAT_SARGB8, 4 } } },
> +
>     { __DRI_IMAGE_FOURCC_XRGB8888, __DRI_IMAGE_COMPONENTS_RGB, 1,
>       { { 0, 0, 0, __DRI_IMAGE_FORMAT_XRGB8888, 4 }, } },
>  
> -- 
> 1.8.4.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
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

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 771 bytes --]

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.

> 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.

-- 
keith.packard@intel.com

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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: [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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Keith Packard]

[-- Attachment #1.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 #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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: [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

Re: [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1017 bytes --]

Keith Packard <keithp@keithp.com> writes:

> The __DRIimage createImageFromFds function takes a fourcc code, but there was
> no fourcc code that match __DRI_IMAGE_FORMAT_SARGB8. This adds a define for
> that format, adds a translation in DRI3 from __DRI_IMAGE_FORMAT_SARGB8 to
> __DRI_IMAGE_FOURCC_SARGB8888 and then adds translations *back* to
> __IMAGE_FORMAT_SARGB8 in both the i915 and i965 drivers.
>
> I'll refrain from comments on whether I think having two separate sets of
> format defines in dri_interface.h is a good idea or not...
>
> Signed-off-by: Keith Packard <keithp@keithp.com>

Reviewed-by: Eric Anholt <eric@anholt.net>

I'd love to see some debug information in whatever path it was that was
silently failing, if we can.  It's so easy to miss places to add format
support.  (I see gallium doesn't use sargb images currently, but might
want this in the future, plus we're still missing an equivalent change
for 2101010 though I don't know if anybody's made it really work
anywhere on dri2 either)

[-- Attachment #1.2: Type: application/pgp-signature, Size: 835 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888

[Kenneth Graunke]

On 11/21/2013 08:12 PM, Keith Packard wrote:
> The __DRIimage createImageFromFds function takes a fourcc code, but there was
> no fourcc code that match __DRI_IMAGE_FORMAT_SARGB8. This adds a define for
> that format, adds a translation in DRI3 from __DRI_IMAGE_FORMAT_SARGB8 to
> __DRI_IMAGE_FOURCC_SARGB8888 and then adds translations *back* to
> __IMAGE_FORMAT_SARGB8 in both the i915 and i965 drivers.
> 
> I'll refrain from comments on whether I think having two separate sets of
> format defines in dri_interface.h is a good idea or not...
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
> 
> This gets iceweasel running with the GL compositor enabled.

I see a huge discussion about this patch, but it's not obvious to me
whether there were actual concerns or just people asking questions.

I see that Eric reviewed it, and that it has not landed.  Are there any
objections to merging it?

--Ken

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

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 567 bytes --]

Kenneth Graunke <kenneth@whitecape.org> writes:

> I see that Eric reviewed it, and that it has not landed.  Are there any
> objections to merging it?

They're on top of a series of DRI3/Present patches, not all of which
have seen review. I was hoping the rest of that series would get
reviewed so that I could merge it all at the same time.

Eric also suggested that we change the switch statements using these
defines to catch unknown values and provide some better indication than
the silent failure I was experiencing.

-- 
keith.packard@intel.com

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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