All of lore.kernel.org
 help / color / mirror / Atom feed
* drm pixel formats update
@ 2011-11-16 18:42 ville.syrjala
  2011-11-16 18:42 ` [PATCH 1/2] drm: Add a missing ')' ville.syrjala
                   ` (4 more replies)
  0 siblings, 5 replies; 24+ messages in thread
From: ville.syrjala @ 2011-11-16 18:42 UTC (permalink / raw)
  To: dri-devel

I decided to go all out with the pixel format definitions. Added pretty
much all of the possible RGB/BGR variations. Just left out ones with
16bit components and floats. Also added a whole bunch of YUV formats,
and 8 bit pseudocolor for good measure.

I'm sure some of the fourccs now clash with the ones used by v4l2,
but that's life.

If anyone has problems with the way the formats are defined, please
speak up now! Since only Jesse has bothered to comment on my rantings
I can only assume people are happy with my approach to things.

These patches should apply on top of Jesse's v3+v5 set... I think.
I sort of lost track of things when the patches started having
different version numbers. At least we're not yet into two digits
numbers ;)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/2] drm: Add a missing ')'
  2011-11-16 18:42 drm pixel formats update ville.syrjala
@ 2011-11-16 18:42 ` ville.syrjala
  2011-11-16 18:42 ` [PATCH 2/2] drm: Redefine pixel formats ville.syrjala
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 24+ messages in thread
From: ville.syrjala @ 2011-11-16 18:42 UTC (permalink / raw)
  To: dri-devel

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

The code happened to compile because the flag wasn't actually used yet.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_mode.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 094da8a..27d7faf 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -266,7 +266,7 @@ struct drm_mode_fb_cmd {
 	__u32 handle;
 };
 
-#define DRM_MODE_FB_INTERLACED	(1<<0 /* for interlaced framebuffers */
+#define DRM_MODE_FB_INTERLACED	(1<<0) /* for interlaced framebuffers */
 
 struct drm_mode_fb_cmd2 {
 	__u32 fb_id;
-- 
1.7.3.4

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/2] drm: Redefine pixel formats
  2011-11-16 18:42 drm pixel formats update ville.syrjala
  2011-11-16 18:42 ` [PATCH 1/2] drm: Add a missing ')' ville.syrjala
@ 2011-11-16 18:42 ` ville.syrjala
  2011-11-16 19:16   ` Ilyes Gouta
  2011-11-17  7:52   ` Michel Dänzer
  2011-11-16 19:13 ` drm pixel formats update James Simmons
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 24+ messages in thread
From: ville.syrjala @ 2011-11-16 18:42 UTC (permalink / raw)
  To: dri-devel

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

Name the formats as DRM_FORMAT_X instead of DRM_FOURCC_X. Use consistent
names, especially for the RGB formats. Component order and byte order are
now strictly specified for each format.

The RGB format naming follows a convention where the components names
and sizes are listed from left to right, matching the order within a
single pixel from most significant bit to least significant bit. Lower
case letters are used when listing the components to improve
readablility. I believe this convention matches the one used by pixman.

The YUV format names vary more. For the 4:2:2 packed formats and 2
plane formats use the fourcc. For the three plane formats the
name includes the plane order and subsampling information using the
standard subsampling notation. Some of those also happen to match
the official fourcc definition.

The fourccs for for all the RGB formats and some of the YUV formats
I invented myself. The idea was that looking at just the fourcc you
get some idea what the format is about without having to decode it
using some external reference.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c           |   18 +++---
 drivers/gpu/drm/drm_crtc_helper.c    |   39 ++++++++++++--
 drivers/gpu/drm/i915/intel_display.c |   18 ++++---
 include/drm/drm_fourcc.h             |   96 ++++++++++++++++++++++++----------
 4 files changed, 121 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 30a70a4..761f265 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1918,28 +1918,28 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
 
 	switch (bpp) {
 	case 8:
-		fmt = DRM_FOURCC_RGB332;
+		fmt = DRM_FORMAT_r3g3b2;
 		break;
 	case 16:
 		if (depth == 15)
-			fmt = DRM_FOURCC_RGB555;
+			fmt = DRM_FORMAT_x1r5g5b5;
 		else
-			fmt = DRM_FOURCC_RGB565;
+			fmt = DRM_FORMAT_r5g6b5;
 		break;
 	case 24:
-		fmt = DRM_FOURCC_RGB24;
+		fmt = DRM_FORMAT_r8g8b8;
 		break;
 	case 32:
 		if (depth == 24)
-			fmt = DRM_FOURCC_RGB24;
+			fmt = DRM_FORMAT_x8r8g8b8;
 		else if (depth == 30)
-			fmt = DRM_INTEL_RGB30;
+			fmt = DRM_FORMAT_x2r10g10b10;
 		else
-			fmt = DRM_FOURCC_RGB32;
+			fmt = DRM_FORMAT_a8r8g8b8;
 		break;
 	default:
-		DRM_ERROR("bad bpp, assuming RGB24 pixel format\n");
-		fmt = DRM_FOURCC_RGB24;
+		DRM_ERROR("bad bpp, assuming x8r8g8b8 pixel format\n");
+		fmt = DRM_FORMAT_x8r8g8b8;
 		break;
 	}
 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 3e0645c..4ef19d37 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -816,27 +816,54 @@ void drm_helper_get_fb_bpp_depth(uint32_t format, unsigned int *depth,
 				 int *bpp)
 {
 	switch (format) {
-	case DRM_FOURCC_RGB332:
+	case DRM_FORMAT_r3g3b2:
+	case DRM_FORMAT_b2g3r3:
 		*depth = 8;
 		*bpp = 8;
 		break;
-	case DRM_FOURCC_RGB555:
+	case DRM_FORMAT_x1r5g5b5:
+	case DRM_FORMAT_x1b5g5r5:
+	case DRM_FORMAT_r5g5b5x1:
+	case DRM_FORMAT_b5g5r5x1:
+	case DRM_FORMAT_a1r5g5b5:
+	case DRM_FORMAT_a1b5g5r5:
+	case DRM_FORMAT_r5g5b5a1:
+	case DRM_FORMAT_b5g5r5a1:
 		*depth = 15;
 		*bpp = 16;
 		break;
-	case DRM_FOURCC_RGB565:
+	case DRM_FORMAT_r5g6b5:
+	case DRM_FORMAT_b5g6r5:
 		*depth = 16;
 		*bpp = 16;
 		break;
-	case DRM_FOURCC_RGB24:
+	case DRM_FORMAT_r8g8b8:
+	case DRM_FORMAT_b8g8r8:
+		*depth = 24;
+		*bpp = 24;
+		break;
+	case DRM_FORMAT_x8r8g8b8:
+	case DRM_FORMAT_x8b8g8r8:
+	case DRM_FORMAT_r8g8b8x8:
+	case DRM_FORMAT_b8g8r8x8:
 		*depth = 24;
 		*bpp = 32;
 		break;
-	case DRM_INTEL_RGB30:
+	case DRM_FORMAT_x2r10g10b10:
+	case DRM_FORMAT_x2b10g10r10:
+	case DRM_FORMAT_r10g10b10x2:
+	case DRM_FORMAT_b10g10r10x2:
+	case DRM_FORMAT_a2r10g10b10:
+	case DRM_FORMAT_a2b10g10r10:
+	case DRM_FORMAT_r10g10b10a2:
+	case DRM_FORMAT_b10g10r10a2:
 		*depth = 30;
 		*bpp = 32;
 		break;
-	case DRM_FOURCC_RGB32:
+	case DRM_FORMAT_a8r8g8b8:
+	case DRM_FORMAT_a8b8g8r8:
+	case DRM_FORMAT_r8g8b8a8:
+	case DRM_FORMAT_b8g8r8a8:
 		*depth = 32;
 		*bpp = 32;
 		break;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 50ae915..62c224a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7585,16 +7585,18 @@ int intel_framebuffer_init(struct drm_device *dev,
 		return -EINVAL;
 
 	switch (mode_cmd->pixel_format) {
-	case DRM_FOURCC_RGB332:
-	case DRM_FOURCC_RGB565:
-	case DRM_FOURCC_RGB24:
-	case DRM_INTEL_RGB30:
+	case DRM_FORMAT_r3g3b2:
+	case DRM_FORMAT_r5g6b5:
+	case DRM_FORMAT_x8r8g8b8:
+	case DRM_FORMAT_a8r8g8b8:
+	case DRM_FORMAT_x2r10g10b10:
+	case DRM_FORMAT_a2r10g10b10:
 		/* RGB formats are common across chipsets */
 		break;
-	case DRM_FOURCC_YUYV:
-	case DRM_FOURCC_UYVY:
-	case DRM_FOURCC_YVYU:
-	case DRM_FOURCC_VYUY:
+	case DRM_FORMAT_yuyv:
+	case DRM_FORMAT_uyvy:
+	case DRM_FORMAT_yvyu:
+	case DRM_FORMAT_vyuy:
 		break;
 	default:
 		DRM_ERROR("unsupported pixel format\n");
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 48c3d10..8192275 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -24,40 +24,82 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H
 
-/*
- * We don't use the V4L header because
- * 1) the fourcc codes are well defined and trivial to construct
- * 2) we don't want user apps to have to pull in v4l headers just for fourcc
- * 3) the v4l fourcc codes are mixed up with a bunch of other code and are
- *    part of the v4l API, so changing them to something linux-generic isn't
- *    feasible
- *
- * So the below includes the fourcc codes used by the DRM and its drivers,
- * along with potential device specific codes.
- */
-
 #include <linux/types.h>
 
 #define fourcc_code(a,b,c,d) ((u32)(a) | ((u32)(b) << 8) | \
 			      ((u32)(c) << 16) | ((u32)(d) << 24))
 
-/* RGB codes */
-#define DRM_FOURCC_RGB332 fourcc_code('R','G','B','1')
-#define DRM_FOURCC_RGB555 fourcc_code('R','G','B','O')
-#define DRM_FOURCC_RGB565 fourcc_code('R','G','B','P')
-#define DRM_FOURCC_RGB24  fourcc_code('R','G','B','3')
-#define DRM_FOURCC_RGB32  fourcc_code('R','G','B','4')
+/* color index */
+#define DRM_FORMAT_c8		fourcc_code('C','8',' ',' ') /* [7:0] C */
+
+/* 8 bpp RGB */
+#define DRM_FORMAT_r3g3b2	fourcc_code('R','G','B','8') /* [7:0] R:G:B 3:3:2 */
+#define DRM_FORMAT_b2g3r3	fourcc_code('B','G','R','8') /* [7:0] B:G:R 2:3:3 */
+
+/* 16 bpp RGB */
+#define DRM_FORMAT_x4r4g4b4	fourcc_code('X','R','1','2') /* [15:0] x:R:G:B 4:4:4:4 native endian */
+#define DRM_FORMAT_x4b4g4r4	fourcc_code('X','B','1','2') /* [15:0] x:B:G:R 4:4:4:4 native endian */
+#define DRM_FORMAT_r4g4b4x4	fourcc_code('R','X','1','2') /* [15:0] R:G:B:x 4:4:4:4 native endian */
+#define DRM_FORMAT_b4g4r4x4	fourcc_code('B','X','1','2') /* [15:0] B:G:R:x 4:4:4:4 native endian */
+
+#define DRM_FORMAT_x1r5g5b5	fourcc_code('X','R','1','5') /* [15:0] x:R:G:B 1:5:5:5 native endian */
+#define DRM_FORMAT_x1b5g5r5	fourcc_code('X','B','1','5') /* [15:0] x:B:G:R 1:5:5:5 native endian */
+#define DRM_FORMAT_r5g5b5x1	fourcc_code('R','X','1','5') /* [15:0] R:G:B:x 5:5:5:1 native endian */
+#define DRM_FORMAT_b5g5r5x1	fourcc_code('B','X','1','5') /* [15:0] B:G:R:x 5:5:5:1 native endian */
+
+#define DRM_FORMAT_a1r5g5b5	fourcc_code('A','R','1','5') /* [15:0] A:R:G:B 1:5:5:5 native endian */
+#define DRM_FORMAT_a1b5g5r5	fourcc_code('A','B','1','5') /* [15:0] A:B:G:R 1:5:5:5 native endian */
+#define DRM_FORMAT_r5g5b5a1	fourcc_code('R','A','1','5') /* [15:0] R:G:B:A 5:5:5:1 native endian */
+#define DRM_FORMAT_b5g5r5a1	fourcc_code('B','A','1','5') /* [15:0] B:G:R:A 5:5:5:1 native endian */
+
+#define DRM_FORMAT_r5g6b5	fourcc_code('R','G','1','6') /* [15:0] R:G:B 5:6:5 native endian */
+#define DRM_FORMAT_b5g6r5	fourcc_code('B','G','1','6') /* [15:0] B:G:R 5:6:5 native endian */
+
+/* 24 bpp RGB */
+#define DRM_FORMAT_r8g8b8	fourcc_code('R','G','2','4') /* [23:0] R:G:B native endian */
+#define DRM_FORMAT_b8g8r8	fourcc_code('B','G','2','4') /* [23:0] B:G:R native endian */
+
+/* 32 bpp RGB */
+#define DRM_FORMAT_x8r8g8b8	fourcc_code('X','R','2','4') /* [31:0] x:R:G:B 8:8:8:8 native endian */
+#define DRM_FORMAT_x8b8g8r8	fourcc_code('X','B','2','4') /* [31:0] x:B:G:R 8:8:8:8 native endian */
+#define DRM_FORMAT_r8g8b8x8	fourcc_code('R','X','2','4') /* [31:0] R:G:B:x 8:8:8:8 native endian */
+#define DRM_FORMAT_b8g8r8x8	fourcc_code('B','X','2','4') /* [31:0] B:G:R:x 8:8:8:8 native endian */
+
+#define DRM_FORMAT_a8r8g8b8	fourcc_code('A','R','2','4') /* [31:0] A:R:G:B 8:8:8:8 native endian */
+#define DRM_FORMAT_a8b8g8r8	fourcc_code('A','B','2','4') /* [31:0] A:B:G:R 8:8:8:8 native endian */
+#define DRM_FORMAT_r8g8b8a8	fourcc_code('R','A','2','4') /* [31:0] R:G:B:A 8:8:8:8 native endian */
+#define DRM_FORMAT_b8g8r8a8	fourcc_code('B','A','2','4') /* [31:0] B:G:R:A 8:8:8:8 native endian */
+
+#define DRM_FORMAT_x2r10g10b10	fourcc_code('X','R','3','0') /* [31:0] x:R:G:B 2:10:10:10 native endian */
+#define DRM_FORMAT_x2b10g10r10	fourcc_code('X','B','3','0') /* [31:0] x:B:G:R 2:10:10:10 native endian */
+#define DRM_FORMAT_r10g10b10x2	fourcc_code('R','X','3','0') /* [31:0] R:G:B:x 10:10:10:2 native endian */
+#define DRM_FORMAT_b10g10r10x2	fourcc_code('B','X','3','0') /* [31:0] B:G:R:x 10:10:10:2 native endian */
+
+#define DRM_FORMAT_a2r10g10b10	fourcc_code('A','R','3','0') /* [31:0] A:R:G:B 2:10:10:10 native endian */
+#define DRM_FORMAT_a2b10g10r10	fourcc_code('A','B','3','0') /* [31:0] A:B:G:R 2:10:10:10 native endian */
+#define DRM_FORMAT_r10g10b10a2	fourcc_code('R','A','3','0') /* [31:0] R:G:B:A 10:10:10:2 native endian */
+#define DRM_FORMAT_b10g10r10a2	fourcc_code('B','A','3','0') /* [31:0] B:G:R:A 10:10:10:2 native endian */
 
-#define DRM_FOURCC_BGR24  fourcc_code('B','G','R','3')
-#define DRM_FOURCC_BGR32  fourcc_code('B','G','R','4')
+/* packed YCbCr */
+#define DRM_FORMAT_yuyv		fourcc_code('Y', 'U', 'Y', 'V') /* [31:0] Cr:Y1:Cb:Y0 8:8:8:8 little endian */
+#define DRM_FORMAT_yvyu		fourcc_code('Y', 'V', 'Y', 'U') /* [31:0] Cb:Y1:Cr:Y0 8:8:8:8 little endian */
+#define DRM_FORMAT_uyvy		fourcc_code('U', 'Y', 'V', 'Y') /* [31:0] Y1:Cr:Y0:Cb 8:8:8:8 little endian */
+#define DRM_FORMAT_vyuy		fourcc_code('V', 'Y', 'U', 'Y') /* [31:0] Y1:Cb:Y0:Cr 8:8:8:8 little endian */
 
-/* YUV codes */
-#define DRM_FOURCC_YUYV   fourcc_code('Y', 'U', 'Y', 'V')
-#define DRM_FOURCC_YVYU   fourcc_code('Y', 'V', 'Y', 'U')
-#define DRM_FOURCC_UYVY   fourcc_code('U', 'Y', 'V', 'Y')
-#define DRM_FOURCC_VYUY   fourcc_code('V', 'Y', 'U', 'Y')
+/* 2 plane YCbCr */
+#define DRM_FORMAT_nv12		fourcc_code('N', 'V', '1', '2') /* Y plane and 2x2 subsampled [15:0] Cr:Cb 8:8 little endian plane */
+#define DRM_FORMAT_nv21		fourcc_code('N', 'V', '2', '1') /* Y plane and 2x2 subsampled [15:0] Cb:Cr 8:8 little endian plane */
+#define DRM_FORMAT_nv16		fourcc_code('N', 'V', '1', '6') /* Y plane and 2x1 subsampled [15:0] Cr:Cb 8:8 little endian plane */
+#define DRM_FORMAT_nv61		fourcc_code('N', 'V', '6', '1') /* Y plane and 2x1 subsampled [15:0] Cb:Cr 8:8 little endian plane */
 
-/* DRM specific codes */
-#define DRM_INTEL_RGB30   fourcc_code('R','G','B','0') /* RGB x:10:10:10 */
+/* 3 plane YCbCr */
+#define DRM_FORMAT_yuv410	fourcc_code('Y', 'U', 'V', '9') /* Y plane and 4x4 subsampled Cb and Cr planes */
+#define DRM_FORMAT_yvu410	fourcc_code('Y', 'V', 'U', '9') /* Y plane and 4x4 subsampled Cr and Cb planes */
+#define DRM_FORMAT_yuv411	fourcc_code('Y', 'U', '1', '1') /* Y plane and 4x1 subsampled Cb and Cr planes */
+#define DRM_FORMAT_yvu411	fourcc_code('Y', 'V', '1', '1') /* Y plane and 4x1 subsampled Cr and Cb planes */
+#define DRM_FORMAT_yuv420	fourcc_code('Y', 'U', '1', '2') /* Y plane and 2x2 subsampled Cb and Cr planes */
+#define DRM_FORMAT_yvu420	fourcc_code('Y', 'V', '1', '2') /* Y plane and 2x2 subsampled Cr and Cb planes */
+#define DRM_FORMAT_yuv422	fourcc_code('Y', 'U', '1', '6') /* Y plane and 2x1 subsampled Cb and Cr planes */
+#define DRM_FORMAT_yvu422	fourcc_code('Y', 'V', '1', '6') /* Y plane and 2x1 subsampled Cr and Cb planes */
 
 #endif /* DRM_FOURCC_H */
-- 
1.7.3.4

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 18:42 drm pixel formats update ville.syrjala
  2011-11-16 18:42 ` [PATCH 1/2] drm: Add a missing ')' ville.syrjala
  2011-11-16 18:42 ` [PATCH 2/2] drm: Redefine pixel formats ville.syrjala
@ 2011-11-16 19:13 ` James Simmons
  2011-11-16 19:54 ` Alan Cox
  2011-11-29 12:10   ` Laurent Pinchart
  4 siblings, 0 replies; 24+ messages in thread
From: James Simmons @ 2011-11-16 19:13 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel


> I decided to go all out with the pixel format definitions. Added pretty
> much all of the possible RGB/BGR variations. Just left out ones with
> 16bit components and floats. Also added a whole bunch of YUV formats,
> and 8 bit pseudocolor for good measure.

Thank you for including the pseudocolor as well.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm: Redefine pixel formats
  2011-11-16 18:42 ` [PATCH 2/2] drm: Redefine pixel formats ville.syrjala
@ 2011-11-16 19:16   ` Ilyes Gouta
  2011-11-16 22:28     ` Ville Syrjälä
  2011-11-17  7:52   ` Michel Dänzer
  1 sibling, 1 reply; 24+ messages in thread
From: Ilyes Gouta @ 2011-11-16 19:16 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel

Hi Ville,

Regarding 3 plane YCbCr, DRM_FORMAT_yuv444 (non sub-sampled YCbCr)
would also be useful.

-Ilyes

On Wed, Nov 16, 2011 at 7:42 PM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Name the formats as DRM_FORMAT_X instead of DRM_FOURCC_X. Use consistent
> names, especially for the RGB formats. Component order and byte order are
> now strictly specified for each format.
>
> The RGB format naming follows a convention where the components names
> and sizes are listed from left to right, matching the order within a
> single pixel from most significant bit to least significant bit. Lower
> case letters are used when listing the components to improve
> readablility. I believe this convention matches the one used by pixman.
>
> The YUV format names vary more. For the 4:2:2 packed formats and 2
> plane formats use the fourcc. For the three plane formats the
> name includes the plane order and subsampling information using the
> standard subsampling notation. Some of those also happen to match
> the official fourcc definition.
>
> The fourccs for for all the RGB formats and some of the YUV formats
> I invented myself. The idea was that looking at just the fourcc you
> get some idea what the format is about without having to decode it
> using some external reference.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c           |   18 +++---
>  drivers/gpu/drm/drm_crtc_helper.c    |   39 ++++++++++++--
>  drivers/gpu/drm/i915/intel_display.c |   18 ++++---
>  include/drm/drm_fourcc.h             |   96 ++++++++++++++++++++++++----------
>  4 files changed, 121 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 30a70a4..761f265 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1918,28 +1918,28 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
>
>        switch (bpp) {
>        case 8:
> -               fmt = DRM_FOURCC_RGB332;
> +               fmt = DRM_FORMAT_r3g3b2;
>                break;
>        case 16:
>                if (depth == 15)
> -                       fmt = DRM_FOURCC_RGB555;
> +                       fmt = DRM_FORMAT_x1r5g5b5;
>                else
> -                       fmt = DRM_FOURCC_RGB565;
> +                       fmt = DRM_FORMAT_r5g6b5;
>                break;
>        case 24:
> -               fmt = DRM_FOURCC_RGB24;
> +               fmt = DRM_FORMAT_r8g8b8;
>                break;
>        case 32:
>                if (depth == 24)
> -                       fmt = DRM_FOURCC_RGB24;
> +                       fmt = DRM_FORMAT_x8r8g8b8;
>                else if (depth == 30)
> -                       fmt = DRM_INTEL_RGB30;
> +                       fmt = DRM_FORMAT_x2r10g10b10;
>                else
> -                       fmt = DRM_FOURCC_RGB32;
> +                       fmt = DRM_FORMAT_a8r8g8b8;
>                break;
>        default:
> -               DRM_ERROR("bad bpp, assuming RGB24 pixel format\n");
> -               fmt = DRM_FOURCC_RGB24;
> +               DRM_ERROR("bad bpp, assuming x8r8g8b8 pixel format\n");
> +               fmt = DRM_FORMAT_x8r8g8b8;
>                break;
>        }
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 3e0645c..4ef19d37 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -816,27 +816,54 @@ void drm_helper_get_fb_bpp_depth(uint32_t format, unsigned int *depth,
>                                 int *bpp)
>  {
>        switch (format) {
> -       case DRM_FOURCC_RGB332:
> +       case DRM_FORMAT_r3g3b2:
> +       case DRM_FORMAT_b2g3r3:
>                *depth = 8;
>                *bpp = 8;
>                break;
> -       case DRM_FOURCC_RGB555:
> +       case DRM_FORMAT_x1r5g5b5:
> +       case DRM_FORMAT_x1b5g5r5:
> +       case DRM_FORMAT_r5g5b5x1:
> +       case DRM_FORMAT_b5g5r5x1:
> +       case DRM_FORMAT_a1r5g5b5:
> +       case DRM_FORMAT_a1b5g5r5:
> +       case DRM_FORMAT_r5g5b5a1:
> +       case DRM_FORMAT_b5g5r5a1:
>                *depth = 15;
>                *bpp = 16;
>                break;
> -       case DRM_FOURCC_RGB565:
> +       case DRM_FORMAT_r5g6b5:
> +       case DRM_FORMAT_b5g6r5:
>                *depth = 16;
>                *bpp = 16;
>                break;
> -       case DRM_FOURCC_RGB24:
> +       case DRM_FORMAT_r8g8b8:
> +       case DRM_FORMAT_b8g8r8:
> +               *depth = 24;
> +               *bpp = 24;
> +               break;
> +       case DRM_FORMAT_x8r8g8b8:
> +       case DRM_FORMAT_x8b8g8r8:
> +       case DRM_FORMAT_r8g8b8x8:
> +       case DRM_FORMAT_b8g8r8x8:
>                *depth = 24;
>                *bpp = 32;
>                break;
> -       case DRM_INTEL_RGB30:
> +       case DRM_FORMAT_x2r10g10b10:
> +       case DRM_FORMAT_x2b10g10r10:
> +       case DRM_FORMAT_r10g10b10x2:
> +       case DRM_FORMAT_b10g10r10x2:
> +       case DRM_FORMAT_a2r10g10b10:
> +       case DRM_FORMAT_a2b10g10r10:
> +       case DRM_FORMAT_r10g10b10a2:
> +       case DRM_FORMAT_b10g10r10a2:
>                *depth = 30;
>                *bpp = 32;
>                break;
> -       case DRM_FOURCC_RGB32:
> +       case DRM_FORMAT_a8r8g8b8:
> +       case DRM_FORMAT_a8b8g8r8:
> +       case DRM_FORMAT_r8g8b8a8:
> +       case DRM_FORMAT_b8g8r8a8:
>                *depth = 32;
>                *bpp = 32;
>                break;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 50ae915..62c224a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7585,16 +7585,18 @@ int intel_framebuffer_init(struct drm_device *dev,
>                return -EINVAL;
>
>        switch (mode_cmd->pixel_format) {
> -       case DRM_FOURCC_RGB332:
> -       case DRM_FOURCC_RGB565:
> -       case DRM_FOURCC_RGB24:
> -       case DRM_INTEL_RGB30:
> +       case DRM_FORMAT_r3g3b2:
> +       case DRM_FORMAT_r5g6b5:
> +       case DRM_FORMAT_x8r8g8b8:
> +       case DRM_FORMAT_a8r8g8b8:
> +       case DRM_FORMAT_x2r10g10b10:
> +       case DRM_FORMAT_a2r10g10b10:
>                /* RGB formats are common across chipsets */
>                break;
> -       case DRM_FOURCC_YUYV:
> -       case DRM_FOURCC_UYVY:
> -       case DRM_FOURCC_YVYU:
> -       case DRM_FOURCC_VYUY:
> +       case DRM_FORMAT_yuyv:
> +       case DRM_FORMAT_uyvy:
> +       case DRM_FORMAT_yvyu:
> +       case DRM_FORMAT_vyuy:
>                break;
>        default:
>                DRM_ERROR("unsupported pixel format\n");
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 48c3d10..8192275 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -24,40 +24,82 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
>
> -/*
> - * We don't use the V4L header because
> - * 1) the fourcc codes are well defined and trivial to construct
> - * 2) we don't want user apps to have to pull in v4l headers just for fourcc
> - * 3) the v4l fourcc codes are mixed up with a bunch of other code and are
> - *    part of the v4l API, so changing them to something linux-generic isn't
> - *    feasible
> - *
> - * So the below includes the fourcc codes used by the DRM and its drivers,
> - * along with potential device specific codes.
> - */
> -
>  #include <linux/types.h>
>
>  #define fourcc_code(a,b,c,d) ((u32)(a) | ((u32)(b) << 8) | \
>                              ((u32)(c) << 16) | ((u32)(d) << 24))
>
> -/* RGB codes */
> -#define DRM_FOURCC_RGB332 fourcc_code('R','G','B','1')
> -#define DRM_FOURCC_RGB555 fourcc_code('R','G','B','O')
> -#define DRM_FOURCC_RGB565 fourcc_code('R','G','B','P')
> -#define DRM_FOURCC_RGB24  fourcc_code('R','G','B','3')
> -#define DRM_FOURCC_RGB32  fourcc_code('R','G','B','4')
> +/* color index */
> +#define DRM_FORMAT_c8          fourcc_code('C','8',' ',' ') /* [7:0] C */
> +
> +/* 8 bpp RGB */
> +#define DRM_FORMAT_r3g3b2      fourcc_code('R','G','B','8') /* [7:0] R:G:B 3:3:2 */
> +#define DRM_FORMAT_b2g3r3      fourcc_code('B','G','R','8') /* [7:0] B:G:R 2:3:3 */
> +
> +/* 16 bpp RGB */
> +#define DRM_FORMAT_x4r4g4b4    fourcc_code('X','R','1','2') /* [15:0] x:R:G:B 4:4:4:4 native endian */
> +#define DRM_FORMAT_x4b4g4r4    fourcc_code('X','B','1','2') /* [15:0] x:B:G:R 4:4:4:4 native endian */
> +#define DRM_FORMAT_r4g4b4x4    fourcc_code('R','X','1','2') /* [15:0] R:G:B:x 4:4:4:4 native endian */
> +#define DRM_FORMAT_b4g4r4x4    fourcc_code('B','X','1','2') /* [15:0] B:G:R:x 4:4:4:4 native endian */
> +
> +#define DRM_FORMAT_x1r5g5b5    fourcc_code('X','R','1','5') /* [15:0] x:R:G:B 1:5:5:5 native endian */
> +#define DRM_FORMAT_x1b5g5r5    fourcc_code('X','B','1','5') /* [15:0] x:B:G:R 1:5:5:5 native endian */
> +#define DRM_FORMAT_r5g5b5x1    fourcc_code('R','X','1','5') /* [15:0] R:G:B:x 5:5:5:1 native endian */
> +#define DRM_FORMAT_b5g5r5x1    fourcc_code('B','X','1','5') /* [15:0] B:G:R:x 5:5:5:1 native endian */
> +
> +#define DRM_FORMAT_a1r5g5b5    fourcc_code('A','R','1','5') /* [15:0] A:R:G:B 1:5:5:5 native endian */
> +#define DRM_FORMAT_a1b5g5r5    fourcc_code('A','B','1','5') /* [15:0] A:B:G:R 1:5:5:5 native endian */
> +#define DRM_FORMAT_r5g5b5a1    fourcc_code('R','A','1','5') /* [15:0] R:G:B:A 5:5:5:1 native endian */
> +#define DRM_FORMAT_b5g5r5a1    fourcc_code('B','A','1','5') /* [15:0] B:G:R:A 5:5:5:1 native endian */
> +
> +#define DRM_FORMAT_r5g6b5      fourcc_code('R','G','1','6') /* [15:0] R:G:B 5:6:5 native endian */
> +#define DRM_FORMAT_b5g6r5      fourcc_code('B','G','1','6') /* [15:0] B:G:R 5:6:5 native endian */
> +
> +/* 24 bpp RGB */
> +#define DRM_FORMAT_r8g8b8      fourcc_code('R','G','2','4') /* [23:0] R:G:B native endian */
> +#define DRM_FORMAT_b8g8r8      fourcc_code('B','G','2','4') /* [23:0] B:G:R native endian */
> +
> +/* 32 bpp RGB */
> +#define DRM_FORMAT_x8r8g8b8    fourcc_code('X','R','2','4') /* [31:0] x:R:G:B 8:8:8:8 native endian */
> +#define DRM_FORMAT_x8b8g8r8    fourcc_code('X','B','2','4') /* [31:0] x:B:G:R 8:8:8:8 native endian */
> +#define DRM_FORMAT_r8g8b8x8    fourcc_code('R','X','2','4') /* [31:0] R:G:B:x 8:8:8:8 native endian */
> +#define DRM_FORMAT_b8g8r8x8    fourcc_code('B','X','2','4') /* [31:0] B:G:R:x 8:8:8:8 native endian */
> +
> +#define DRM_FORMAT_a8r8g8b8    fourcc_code('A','R','2','4') /* [31:0] A:R:G:B 8:8:8:8 native endian */
> +#define DRM_FORMAT_a8b8g8r8    fourcc_code('A','B','2','4') /* [31:0] A:B:G:R 8:8:8:8 native endian */
> +#define DRM_FORMAT_r8g8b8a8    fourcc_code('R','A','2','4') /* [31:0] R:G:B:A 8:8:8:8 native endian */
> +#define DRM_FORMAT_b8g8r8a8    fourcc_code('B','A','2','4') /* [31:0] B:G:R:A 8:8:8:8 native endian */
> +
> +#define DRM_FORMAT_x2r10g10b10 fourcc_code('X','R','3','0') /* [31:0] x:R:G:B 2:10:10:10 native endian */
> +#define DRM_FORMAT_x2b10g10r10 fourcc_code('X','B','3','0') /* [31:0] x:B:G:R 2:10:10:10 native endian */
> +#define DRM_FORMAT_r10g10b10x2 fourcc_code('R','X','3','0') /* [31:0] R:G:B:x 10:10:10:2 native endian */
> +#define DRM_FORMAT_b10g10r10x2 fourcc_code('B','X','3','0') /* [31:0] B:G:R:x 10:10:10:2 native endian */
> +
> +#define DRM_FORMAT_a2r10g10b10 fourcc_code('A','R','3','0') /* [31:0] A:R:G:B 2:10:10:10 native endian */
> +#define DRM_FORMAT_a2b10g10r10 fourcc_code('A','B','3','0') /* [31:0] A:B:G:R 2:10:10:10 native endian */
> +#define DRM_FORMAT_r10g10b10a2 fourcc_code('R','A','3','0') /* [31:0] R:G:B:A 10:10:10:2 native endian */
> +#define DRM_FORMAT_b10g10r10a2 fourcc_code('B','A','3','0') /* [31:0] B:G:R:A 10:10:10:2 native endian */
>
> -#define DRM_FOURCC_BGR24  fourcc_code('B','G','R','3')
> -#define DRM_FOURCC_BGR32  fourcc_code('B','G','R','4')
> +/* packed YCbCr */
> +#define DRM_FORMAT_yuyv                fourcc_code('Y', 'U', 'Y', 'V') /* [31:0] Cr:Y1:Cb:Y0 8:8:8:8 little endian */
> +#define DRM_FORMAT_yvyu                fourcc_code('Y', 'V', 'Y', 'U') /* [31:0] Cb:Y1:Cr:Y0 8:8:8:8 little endian */
> +#define DRM_FORMAT_uyvy                fourcc_code('U', 'Y', 'V', 'Y') /* [31:0] Y1:Cr:Y0:Cb 8:8:8:8 little endian */
> +#define DRM_FORMAT_vyuy                fourcc_code('V', 'Y', 'U', 'Y') /* [31:0] Y1:Cb:Y0:Cr 8:8:8:8 little endian */
>
> -/* YUV codes */
> -#define DRM_FOURCC_YUYV   fourcc_code('Y', 'U', 'Y', 'V')
> -#define DRM_FOURCC_YVYU   fourcc_code('Y', 'V', 'Y', 'U')
> -#define DRM_FOURCC_UYVY   fourcc_code('U', 'Y', 'V', 'Y')
> -#define DRM_FOURCC_VYUY   fourcc_code('V', 'Y', 'U', 'Y')
> +/* 2 plane YCbCr */
> +#define DRM_FORMAT_nv12                fourcc_code('N', 'V', '1', '2') /* Y plane and 2x2 subsampled [15:0] Cr:Cb 8:8 little endian plane */
> +#define DRM_FORMAT_nv21                fourcc_code('N', 'V', '2', '1') /* Y plane and 2x2 subsampled [15:0] Cb:Cr 8:8 little endian plane */
> +#define DRM_FORMAT_nv16                fourcc_code('N', 'V', '1', '6') /* Y plane and 2x1 subsampled [15:0] Cr:Cb 8:8 little endian plane */
> +#define DRM_FORMAT_nv61                fourcc_code('N', 'V', '6', '1') /* Y plane and 2x1 subsampled [15:0] Cb:Cr 8:8 little endian plane */
>
> -/* DRM specific codes */
> -#define DRM_INTEL_RGB30   fourcc_code('R','G','B','0') /* RGB x:10:10:10 */
> +/* 3 plane YCbCr */
> +#define DRM_FORMAT_yuv410      fourcc_code('Y', 'U', 'V', '9') /* Y plane and 4x4 subsampled Cb and Cr planes */
> +#define DRM_FORMAT_yvu410      fourcc_code('Y', 'V', 'U', '9') /* Y plane and 4x4 subsampled Cr and Cb planes */
> +#define DRM_FORMAT_yuv411      fourcc_code('Y', 'U', '1', '1') /* Y plane and 4x1 subsampled Cb and Cr planes */
> +#define DRM_FORMAT_yvu411      fourcc_code('Y', 'V', '1', '1') /* Y plane and 4x1 subsampled Cr and Cb planes */
> +#define DRM_FORMAT_yuv420      fourcc_code('Y', 'U', '1', '2') /* Y plane and 2x2 subsampled Cb and Cr planes */
> +#define DRM_FORMAT_yvu420      fourcc_code('Y', 'V', '1', '2') /* Y plane and 2x2 subsampled Cr and Cb planes */
> +#define DRM_FORMAT_yuv422      fourcc_code('Y', 'U', '1', '6') /* Y plane and 2x1 subsampled Cb and Cr planes */
> +#define DRM_FORMAT_yvu422      fourcc_code('Y', 'V', '1', '6') /* Y plane and 2x1 subsampled Cr and Cb planes */
>
>  #endif /* DRM_FOURCC_H */
> --
> 1.7.3.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 18:42 drm pixel formats update ville.syrjala
                   ` (2 preceding siblings ...)
  2011-11-16 19:13 ` drm pixel formats update James Simmons
@ 2011-11-16 19:54 ` Alan Cox
  2011-11-16 21:19   ` Ville Syrjälä
  2011-11-29 12:10   ` Laurent Pinchart
  4 siblings, 1 reply; 24+ messages in thread
From: Alan Cox @ 2011-11-16 19:54 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel

> If anyone has problems with the way the formats are defined, please
> speak up now! Since only Jesse has bothered to comment on my rantings
> I can only assume people are happy with my approach to things.

Umm .. no. I don't see why they are needed. Its just an extra layer of
gratuitious confusing indirection. The rest of the world speaks and
understands FourCC sp for all the formats covered by an existing FourCC
name we should just the existing name.

You might need to check one now and then but everyone doing video
processing is familiar with them including all the Windows folk.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 19:54 ` Alan Cox
@ 2011-11-16 21:19   ` Ville Syrjälä
  2011-11-16 21:23     ` Jesse Barnes
  2011-11-16 21:26     ` Alan Cox
  0 siblings, 2 replies; 24+ messages in thread
From: Ville Syrjälä @ 2011-11-16 21:19 UTC (permalink / raw)
  To: Alan Cox; +Cc: dri-devel

On Wed, Nov 16, 2011 at 07:54:12PM +0000, Alan Cox wrote:
> > If anyone has problems with the way the formats are defined, please
> > speak up now! Since only Jesse has bothered to comment on my rantings
> > I can only assume people are happy with my approach to things.
> 
> Umm .. no. I don't see why they are needed. Its just an extra layer of
> gratuitious confusing indirection. The rest of the world speaks and
> understands FourCC sp for all the formats covered by an existing FourCC
> name we should just the existing name.
> 
> You might need to check one now and then but everyone doing video
> processing is familiar with them including all the Windows folk.

I think the only format in my list where I didn't use an existing fourcc
is I420/IYUV. And BTW, for that one I used the same "fake" fourcc that
v4l2 uses (YU12). 

And that brings another matter to the table. How should we deal with
duplicate fourccs? I420/IYUV and YUY2/YUYV come to mind.

Also, if I now add these ad-hoc fourccs for the RGB formats, and some
time later someone comes in with a format with a conflicting official
fourcc, what should we do?

Oh and one extra detail just occured to me regarding the three plane
formats. Should we even define formats for both the YUV vs. YVU
variant. Seeing as we now have independent handles and offsets for
each plane, we can make do with just one format definition.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 21:19   ` Ville Syrjälä
@ 2011-11-16 21:23     ` Jesse Barnes
  2011-11-16 22:20       ` Ville Syrjälä
  2011-11-16 21:26     ` Alan Cox
  1 sibling, 1 reply; 24+ messages in thread
From: Jesse Barnes @ 2011-11-16 21:23 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: dri-devel


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

On Wed, 16 Nov 2011 23:19:38 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> Oh and one extra detail just occured to me regarding the three plane
> formats. Should we even define formats for both the YUV vs. YVU
> variant. Seeing as we now have independent handles and offsets for
> each plane, we can make do with just one format definition.

Don't you still need to know the order?  I.e. what's in handle[1], U or
V?

-- 
Jesse Barnes, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 21:19   ` Ville Syrjälä
  2011-11-16 21:23     ` Jesse Barnes
@ 2011-11-16 21:26     ` Alan Cox
  2011-11-16 22:16       ` Ville Syrjälä
  1 sibling, 1 reply; 24+ messages in thread
From: Alan Cox @ 2011-11-16 21:26 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: dri-devel

> I think the only format in my list where I didn't use an existing fourcc
> is I420/IYUV. And BTW, for that one I used the same "fake" fourcc that

Right but you redefine all sorts of stuff in the driver in your patch to
non FourCC names which is just confusing (and painful given the format
picked)

> v4l2 uses (YU12). 
> 
> And that brings another matter to the table. How should we deal with
> duplicate fourccs? I420/IYUV and YUY2/YUYV come to mind.

Just accept both. FourCC as with all API's is not perfect
 
> Also, if I now add these ad-hoc fourccs for the RGB formats, and some
> time later someone comes in with a format with a conflicting official
> fourcc, what should we do?

One possibility I suggested originally was to mix FourCC codes and native
formats which are numbered. That works fine in both endiannesses in
theory because you'll always have a \0 in it which is invalid FourCC

ie just number the Linux specific DRM formats 0, 1, 2, 3, 4, 5, ...

> Oh and one extra detail just occured to me regarding the three plane
> formats. Should we even define formats for both the YUV vs. YVU
> variant. Seeing as we now have independent handles and offsets for
> each plane, we can make do with just one format definition.

I think so - or the helper should do the translation and flip the planes.
We want the user to get flexibility and the driver to be as simple as
possible.

(and btw I've no problem at all with the idea that you can pass in a
FourCC *or* a format specifying structure, or with an internal API where
a fourCC is always internally turned into a struct of offsets and other
useful info before hitting the drivers)

Alan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 21:26     ` Alan Cox
@ 2011-11-16 22:16       ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2011-11-16 22:16 UTC (permalink / raw)
  To: Alan Cox; +Cc: dri-devel

On Wed, Nov 16, 2011 at 09:26:20PM +0000, Alan Cox wrote:
> > I think the only format in my list where I didn't use an existing fourcc
> > is I420/IYUV. And BTW, for that one I used the same "fake" fourcc that
> 
> Right but you redefine all sorts of stuff in the driver in your patch to
> non FourCC names which is just confusing (and painful given the format
> picked)

Sorry, now I lost you completely. Care to elaborate, or perhaps
point to a specific line or lines in the patch?

> > v4l2 uses (YU12). 
> > 
> > And that brings another matter to the table. How should we deal with
> > duplicate fourccs? I420/IYUV and YUY2/YUYV come to mind.
> 
> Just accept both. FourCC as with all API's is not perfect
>  
> > Also, if I now add these ad-hoc fourccs for the RGB formats, and some
> > time later someone comes in with a format with a conflicting official
> > fourcc, what should we do?
> 
> One possibility I suggested originally was to mix FourCC codes and native
> formats which are numbered. That works fine in both endiannesses in
> theory because you'll always have a \0 in it which is invalid FourCC
> 
> ie just number the Linux specific DRM formats 0, 1, 2, 3, 4, 5, ...

I suggested a running number too. But I'd rather leave the fourccs to
user space completely. But if people insist that the kernel should eat
them too, we could just convert them to the simple number format in
some helper function, to isolate the rest of the code from fourccs.
And then there'd be no point in even defining any fourcc stuff in the
headers, as everyone knows how to construct them.

> > Oh and one extra detail just occured to me regarding the three plane
> > formats. Should we even define formats for both the YUV vs. YVU
> > variant. Seeing as we now have independent handles and offsets for
> > each plane, we can make do with just one format definition.
> 
> I think so - or the helper should do the translation and flip the planes.
> We want the user to get flexibility and the driver to be as simple as
> possible.
> 
> (and btw I've no problem at all with the idea that you can pass in a
> FourCC *or* a format specifying structure, or with an internal API where
> a fourCC is always internally turned into a struct of offsets and other
> useful info before hitting the drivers)

Even if there's such a structure, I think it's still beneficial to have
a constant identifier for each format. It allows you utilize switch
statements, whereas otherwise you'd possibly need to look at multiple
bits of information inside the structure.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 21:23     ` Jesse Barnes
@ 2011-11-16 22:20       ` Ville Syrjälä
  2011-11-17 17:06         ` Jesse Barnes
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2011-11-16 22:20 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: dri-devel

On Wed, Nov 16, 2011 at 01:23:01PM -0800, Jesse Barnes wrote:
> On Wed, 16 Nov 2011 23:19:38 +0200
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > Oh and one extra detail just occured to me regarding the three plane
> > formats. Should we even define formats for both the YUV vs. YVU
> > variant. Seeing as we now have independent handles and offsets for
> > each plane, we can make do with just one format definition.
> 
> Don't you still need to know the order?  I.e. what's in handle[1], U or
> V?

We could define it so that handle[1] is always Cb and handle [2] is Cr,
for example. Then it's up to user space to set the handles orrectly,
which it has to do anyway.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm: Redefine pixel formats
  2011-11-16 19:16   ` Ilyes Gouta
@ 2011-11-16 22:28     ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2011-11-16 22:28 UTC (permalink / raw)
  To: Ilyes Gouta; +Cc: dri-devel

On Wed, Nov 16, 2011 at 08:16:31PM +0100, Ilyes Gouta wrote:
> Hi Ville,
> 
> Regarding 3 plane YCbCr, DRM_FORMAT_yuv444 (non sub-sampled YCbCr)
> would also be useful.

Yeah I was wondering whether to add that. So far I've not run into
hardware which can eat that, so I left it out for now.

Packed 4:4:4 is supported by the overlays in most ATI chips IIRC,
so adding both packed and planar 4:4:4 formats would likely make
sense.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm: Redefine pixel formats
  2011-11-16 18:42 ` [PATCH 2/2] drm: Redefine pixel formats ville.syrjala
  2011-11-16 19:16   ` Ilyes Gouta
@ 2011-11-17  7:52   ` Michel Dänzer
  2011-11-17 13:06     ` Ville Syrjälä
  1 sibling, 1 reply; 24+ messages in thread
From: Michel Dänzer @ 2011-11-17  7:52 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel

On Mit, 2011-11-16 at 20:42 +0200, ville.syrjala@linux.intel.com wrote: 
> 
> Name the formats as DRM_FORMAT_X instead of DRM_FOURCC_X. Use consistent
> names, especially for the RGB formats. Component order and byte order are
> now strictly specified for each format.
> 
> The RGB format naming follows a convention where the components names
> and sizes are listed from left to right, matching the order within a
> single pixel from most significant bit to least significant bit. Lower
> case letters are used when listing the components to improve
> readablility. I believe this convention matches the one used by pixman.

The RGB formats are all defined in the CPU native byte order. But e.g.
pre-R600 Radeons can only scan out little endian formats. For the
framebuffer device, we use GPU byte swapping facilities to make the
pixels appear to the CPU in its native byte order, so these format
definitions make sense for that. But I'm not sure they make sense for
the KMS APIs, e.g. the userspace drivers don't use these facilities but
handle byte swapping themselves.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm: Redefine pixel formats
  2011-11-17  7:52   ` Michel Dänzer
@ 2011-11-17 13:06     ` Ville Syrjälä
  2011-11-17 14:00       ` Michel Dänzer
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2011-11-17 13:06 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: dri-devel

On Thu, Nov 17, 2011 at 08:52:05AM +0100, Michel Dänzer wrote:
> On Mit, 2011-11-16 at 20:42 +0200, ville.syrjala@linux.intel.com wrote: 
> > 
> > Name the formats as DRM_FORMAT_X instead of DRM_FOURCC_X. Use consistent
> > names, especially for the RGB formats. Component order and byte order are
> > now strictly specified for each format.
> > 
> > The RGB format naming follows a convention where the components names
> > and sizes are listed from left to right, matching the order within a
> > single pixel from most significant bit to least significant bit. Lower
> > case letters are used when listing the components to improve
> > readablility. I believe this convention matches the one used by pixman.
> 
> The RGB formats are all defined in the CPU native byte order. But e.g.
> pre-R600 Radeons can only scan out little endian formats. For the
> framebuffer device, we use GPU byte swapping facilities to make the
> pixels appear to the CPU in its native byte order, so these format
> definitions make sense for that. But I'm not sure they make sense for
> the KMS APIs, e.g. the userspace drivers don't use these facilities but
> handle byte swapping themselves.

Hmm. So who decides whether GPU byte swapping is needed when you eg.
mmap() some buffer?

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm: Redefine pixel formats
  2011-11-17 13:06     ` Ville Syrjälä
@ 2011-11-17 14:00       ` Michel Dänzer
  2011-11-17 14:40         ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Michel Dänzer @ 2011-11-17 14:00 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: dri-devel

On Don, 2011-11-17 at 15:06 +0200, Ville Syrjälä wrote: 
> On Thu, Nov 17, 2011 at 08:52:05AM +0100, Michel Dänzer wrote:
> > On Mit, 2011-11-16 at 20:42 +0200, ville.syrjala@linux.intel.com wrote: 
> > > 
> > > Name the formats as DRM_FORMAT_X instead of DRM_FOURCC_X. Use consistent
> > > names, especially for the RGB formats. Component order and byte order are
> > > now strictly specified for each format.
> > > 
> > > The RGB format naming follows a convention where the components names
> > > and sizes are listed from left to right, matching the order within a
> > > single pixel from most significant bit to least significant bit. Lower
> > > case letters are used when listing the components to improve
> > > readablility. I believe this convention matches the one used by pixman.
> > 
> > The RGB formats are all defined in the CPU native byte order. But e.g.
> > pre-R600 Radeons can only scan out little endian formats. For the
> > framebuffer device, we use GPU byte swapping facilities to make the
> > pixels appear to the CPU in its native byte order, so these format
> > definitions make sense for that. But I'm not sure they make sense for
> > the KMS APIs, e.g. the userspace drivers don't use these facilities but
> > handle byte swapping themselves.
> 
> Hmm. So who decides whether GPU byte swapping is needed when you eg.
> mmap() some buffer?

The userspace drivers.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] drm: Redefine pixel formats
  2011-11-17 14:00       ` Michel Dänzer
@ 2011-11-17 14:40         ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2011-11-17 14:40 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: dri-devel

On Thu, Nov 17, 2011 at 03:00:17PM +0100, Michel Dänzer wrote:
> On Don, 2011-11-17 at 15:06 +0200, Ville Syrjälä wrote: 
> > On Thu, Nov 17, 2011 at 08:52:05AM +0100, Michel Dänzer wrote:
> > > On Mit, 2011-11-16 at 20:42 +0200, ville.syrjala@linux.intel.com wrote: 
> > > > 
> > > > Name the formats as DRM_FORMAT_X instead of DRM_FOURCC_X. Use consistent
> > > > names, especially for the RGB formats. Component order and byte order are
> > > > now strictly specified for each format.
> > > > 
> > > > The RGB format naming follows a convention where the components names
> > > > and sizes are listed from left to right, matching the order within a
> > > > single pixel from most significant bit to least significant bit. Lower
> > > > case letters are used when listing the components to improve
> > > > readablility. I believe this convention matches the one used by pixman.
> > > 
> > > The RGB formats are all defined in the CPU native byte order. But e.g.
> > > pre-R600 Radeons can only scan out little endian formats. For the
> > > framebuffer device, we use GPU byte swapping facilities to make the
> > > pixels appear to the CPU in its native byte order, so these format
> > > definitions make sense for that. But I'm not sure they make sense for
> > > the KMS APIs, e.g. the userspace drivers don't use these facilities but
> > > handle byte swapping themselves.
> > 
> > Hmm. So who decides whether GPU byte swapping is needed when you eg.
> > mmap() some buffer?
> 
> The userspace drivers.

Hmm. OK. So I guess we should define the formats as little endian then.
Supposing we also need big endian formats in the future, we could just
define an extra flag like so:

#define DRM_FORMAT_BIG_ENDIAN (1<<31)

which would be ORed with the fourcc to get a big endian version of the
format. Otherwise we have to duplicate the formats for big endian as
well.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 22:20       ` Ville Syrjälä
@ 2011-11-17 17:06         ` Jesse Barnes
  2011-11-17 21:05           ` Rob Clark
  0 siblings, 1 reply; 24+ messages in thread
From: Jesse Barnes @ 2011-11-17 17:06 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: dri-devel


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

On Thu, 17 Nov 2011 00:20:44 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Wed, Nov 16, 2011 at 01:23:01PM -0800, Jesse Barnes wrote:
> > On Wed, 16 Nov 2011 23:19:38 +0200
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > Oh and one extra detail just occured to me regarding the three plane
> > > formats. Should we even define formats for both the YUV vs. YVU
> > > variant. Seeing as we now have independent handles and offsets for
> > > each plane, we can make do with just one format definition.
> > 
> > Don't you still need to know the order?  I.e. what's in handle[1], U or
> > V?
> 
> We could define it so that handle[1] is always Cb and handle [2] is Cr,
> for example. Then it's up to user space to set the handles orrectly,
> which it has to do anyway.

Yeah see my other mail; I'm mainly worried about single bo cases.

-- 
Jesse Barnes, Intel Open Source Technology Center

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-17 17:06         ` Jesse Barnes
@ 2011-11-17 21:05           ` Rob Clark
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Clark @ 2011-11-17 21:05 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: dri-devel

On Thu, Nov 17, 2011 at 11:06 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Thu, 17 Nov 2011 00:20:44 +0200
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>
>> On Wed, Nov 16, 2011 at 01:23:01PM -0800, Jesse Barnes wrote:
>> > On Wed, 16 Nov 2011 23:19:38 +0200
>> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > > Oh and one extra detail just occured to me regarding the three plane
>> > > formats. Should we even define formats for both the YUV vs. YVU
>> > > variant. Seeing as we now have independent handles and offsets for
>> > > each plane, we can make do with just one format definition.
>> >
>> > Don't you still need to know the order?  I.e. what's in handle[1], U or
>> > V?
>>
>> We could define it so that handle[1] is always Cb and handle [2] is Cr,
>> for example. Then it's up to user space to set the handles orrectly,
>> which it has to do anyway.
>
> Yeah see my other mail; I'm mainly worried about single bo cases.


well, you do still have an offset per-plane..  so single bo case:

  cmd->handles[0] = bo;
  cmd->offsets[0] = 0;
  cmd->handles[1] = bo;
  cmd->offsets[1] = u_offset;
  cmd->handles[2] = bo;
  cmd->offsets[2] = v_offset;

nothing says that you can't have u_offset > v_offset

BR,
-R

> --
> Jesse Barnes, Intel Open Source Technology Center
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-16 18:42 drm pixel formats update ville.syrjala
@ 2011-11-29 12:10   ` Laurent Pinchart
  2011-11-16 18:42 ` [PATCH 2/2] drm: Redefine pixel formats ville.syrjala
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2011-11-29 12:10 UTC (permalink / raw)
  To: dri-devel; +Cc: ville.syrjala, Tomi Valkeinen, linux-fbdev, linux-media

Hi Ville,

Sorry for the late reply.

(Cross-posting to the linux-fbdev and linux-media mailing lists, as the topics 
I'm about to discuss are of interest to everybody)

On Wednesday 16 November 2011 19:42:23 ville.syrjala@linux.intel.com wrote:
> I decided to go all out with the pixel format definitions. Added pretty
> much all of the possible RGB/BGR variations. Just left out ones with
> 16bit components and floats. Also added a whole bunch of YUV formats,
> and 8 bit pseudocolor for good measure.
> 
> I'm sure some of the fourccs now clash with the ones used by v4l2,
> but that's life.

Disclaimer: I realize this is a somehow controversial topic, and I'm not 
trying to start a flame war :-)

What about defining a common set of fourccs for both subsystem (and using them 
in FBDEV, which currently uses the V4L2 fourccs) ?

There's more and more overlap between DRM, FBDEV and V4L2, which results in 
confusion for the user and mess in the kernel. I believe cleaning this up will 
become more and more important with time, and also probably more and more 
difficult if we don't act now.

There's no way we will all quickly agree on a big picture unified architecture 
(I don't even know what it should look like), so I'm thinking about starting 
small and see where it will lead us. Sharing fourccs would be such a first 
step, and would make it easier for drivers to implement several of the DRM, 
FBDEV and V4L2 APIs.

Another step I'd like to take would be working on sharing the video mode 
definitions between subsystems. DRM has struct drm_mode_modeinfo and FBDEV has 
struct fb_videomode. The former can't be modified as it's used in userspace 
APIs, but I believe we should create a common in-kernel structure to represent 
modes. This would also make it easier to share the EDID parser between DRM and 
FBDEV.

One issue here is names. I understand that using names from another subsystem 
in a driver that has nothing to do with it (like using V4L2_PIX_FMT_* in DRM, 
or drm_mode_modeinfo in FBDEV) can be frustrating for many developers, so I'd 
like to propose an alternative. We have a media-related kernel API that is 
cross-subsystem, that's the media controller. It uses the prefix media_. We 
could use more specific versions of that preview (such as media_video_ and 
media_display_) as a neutral ground.

Another issue is control. It's quite natural to be suspicious about subsystems 
we're not familiar with, and giving up control on things we consider as highly 
important to other subsystems feels dangerous and wrong. Once again, media_* 
could be an answer to this problem. Instead of trying to push other subsystems 
to use our APIs (which are, obviously, the best possible ever, as they're the 
ones we work on :-)) with the promise that we will extend them to fullfill 
their needs if necessary, we could design new shared structures and core 
functions together with a media_ prefix, and make sure they fullfill all the 
needs from the start. What I like about this approach is that each of the 3 
video-related subsystems would then benefit from the experience of the other 
2.

To make it perfectly clear, I want to emphasize that I'm not trying to replace 
DRM, FBDEV and V4L2 with a new shared subsystem. What I would like to see in 
the (near future) is collaboration and sharing of core features that make 
sense to share. I believe we should address this from a "pain point" point of 
view. The one that lead me to writing this e-mail is that developing a driver 
that implements both the DRM and FBDEV APIs for video output currently 
requires various similar structures, and code to translate between all of 
them. That code can't be shared between multiple drivers, is error-prone, and 
painful to maintin.

I can (and actually would like to) submit an RFC, but I would first like to 
hear your opinion on the topic.

> If anyone has problems with the way the formats are defined, please
> speak up now! Since only Jesse has bothered to comment on my rantings
> I can only assume people are happy with my approach to things.
> 
> These patches should apply on top of Jesse's v3+v5 set... I think.
> I sort of lost track of things when the patches started having
> different version numbers. At least we're not yet into two digits
> numbers ;)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
@ 2011-11-29 12:10   ` Laurent Pinchart
  0 siblings, 0 replies; 24+ messages in thread
From: Laurent Pinchart @ 2011-11-29 12:10 UTC (permalink / raw)
  To: dri-devel; +Cc: ville.syrjala, Tomi Valkeinen, linux-fbdev, linux-media

Hi Ville,

Sorry for the late reply.

(Cross-posting to the linux-fbdev and linux-media mailing lists, as the topics 
I'm about to discuss are of interest to everybody)

On Wednesday 16 November 2011 19:42:23 ville.syrjala@linux.intel.com wrote:
> I decided to go all out with the pixel format definitions. Added pretty
> much all of the possible RGB/BGR variations. Just left out ones with
> 16bit components and floats. Also added a whole bunch of YUV formats,
> and 8 bit pseudocolor for good measure.
> 
> I'm sure some of the fourccs now clash with the ones used by v4l2,
> but that's life.

Disclaimer: I realize this is a somehow controversial topic, and I'm not 
trying to start a flame war :-)

What about defining a common set of fourccs for both subsystem (and using them 
in FBDEV, which currently uses the V4L2 fourccs) ?

There's more and more overlap between DRM, FBDEV and V4L2, which results in 
confusion for the user and mess in the kernel. I believe cleaning this up will 
become more and more important with time, and also probably more and more 
difficult if we don't act now.

There's no way we will all quickly agree on a big picture unified architecture 
(I don't even know what it should look like), so I'm thinking about starting 
small and see where it will lead us. Sharing fourccs would be such a first 
step, and would make it easier for drivers to implement several of the DRM, 
FBDEV and V4L2 APIs.

Another step I'd like to take would be working on sharing the video mode 
definitions between subsystems. DRM has struct drm_mode_modeinfo and FBDEV has 
struct fb_videomode. The former can't be modified as it's used in userspace 
APIs, but I believe we should create a common in-kernel structure to represent 
modes. This would also make it easier to share the EDID parser between DRM and 
FBDEV.

One issue here is names. I understand that using names from another subsystem 
in a driver that has nothing to do with it (like using V4L2_PIX_FMT_* in DRM, 
or drm_mode_modeinfo in FBDEV) can be frustrating for many developers, so I'd 
like to propose an alternative. We have a media-related kernel API that is 
cross-subsystem, that's the media controller. It uses the prefix media_. We 
could use more specific versions of that preview (such as media_video_ and 
media_display_) as a neutral ground.

Another issue is control. It's quite natural to be suspicious about subsystems 
we're not familiar with, and giving up control on things we consider as highly 
important to other subsystems feels dangerous and wrong. Once again, media_* 
could be an answer to this problem. Instead of trying to push other subsystems 
to use our APIs (which are, obviously, the best possible ever, as they're the 
ones we work on :-)) with the promise that we will extend them to fullfill 
their needs if necessary, we could design new shared structures and core 
functions together with a media_ prefix, and make sure they fullfill all the 
needs from the start. What I like about this approach is that each of the 3 
video-related subsystems would then benefit from the experience of the other 
2.

To make it perfectly clear, I want to emphasize that I'm not trying to replace 
DRM, FBDEV and V4L2 with a new shared subsystem. What I would like to see in 
the (near future) is collaboration and sharing of core features that make 
sense to share. I believe we should address this from a "pain point" point of 
view. The one that lead me to writing this e-mail is that developing a driver 
that implements both the DRM and FBDEV APIs for video output currently 
requires various similar structures, and code to translate between all of 
them. That code can't be shared between multiple drivers, is error-prone, and 
painful to maintin.

I can (and actually would like to) submit an RFC, but I would first like to 
hear your opinion on the topic.

> If anyone has problems with the way the formats are defined, please
> speak up now! Since only Jesse has bothered to comment on my rantings
> I can only assume people are happy with my approach to things.
> 
> These patches should apply on top of Jesse's v3+v5 set... I think.
> I sort of lost track of things when the patches started having
> different version numbers. At least we're not yet into two digits
> numbers ;)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-29 12:10   ` Laurent Pinchart
@ 2011-11-29 13:30     ` Hans Verkuil
  -1 siblings, 0 replies; 24+ messages in thread
From: Hans Verkuil @ 2011-11-29 13:30 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, ville.syrjala, Tomi Valkeinen, linux-fbdev,
	linux-media, Jesse Barker

On Tuesday 29 November 2011 13:10:35 Laurent Pinchart wrote:
> Hi Ville,
> 
> Sorry for the late reply.
> 
> (Cross-posting to the linux-fbdev and linux-media mailing lists, as the
> topics I'm about to discuss are of interest to everybody)
> 
> On Wednesday 16 November 2011 19:42:23 ville.syrjala@linux.intel.com wrote:
> > I decided to go all out with the pixel format definitions. Added pretty
> > much all of the possible RGB/BGR variations. Just left out ones with
> > 16bit components and floats. Also added a whole bunch of YUV formats,
> > and 8 bit pseudocolor for good measure.
> > 
> > I'm sure some of the fourccs now clash with the ones used by v4l2,
> > but that's life.
> 
> Disclaimer: I realize this is a somehow controversial topic, and I'm not
> trying to start a flame war :-)
> 
> What about defining a common set of fourccs for both subsystem (and using
> them in FBDEV, which currently uses the V4L2 fourccs) ?
> 
> There's more and more overlap between DRM, FBDEV and V4L2, which results in
> confusion for the user and mess in the kernel. I believe cleaning this up
> will become more and more important with time, and also probably more and
> more difficult if we don't act now.
> 
> There's no way we will all quickly agree on a big picture unified
> architecture (I don't even know what it should look like), so I'm thinking
> about starting small and see where it will lead us. Sharing fourccs would
> be such a first step, and would make it easier for drivers to implement
> several of the DRM, FBDEV and V4L2 APIs.
> 
> Another step I'd like to take would be working on sharing the video mode
> definitions between subsystems. DRM has struct drm_mode_modeinfo and FBDEV
> has struct fb_videomode. The former can't be modified as it's used in
> userspace APIs, but I believe we should create a common in-kernel
> structure to represent modes. This would also make it easier to share the
> EDID parser between DRM and FBDEV.
> 
> One issue here is names. I understand that using names from another
> subsystem in a driver that has nothing to do with it (like using
> V4L2_PIX_FMT_* in DRM, or drm_mode_modeinfo in FBDEV) can be frustrating
> for many developers, so I'd like to propose an alternative. We have a
> media-related kernel API that is cross-subsystem, that's the media
> controller. It uses the prefix media_. We could use more specific versions
> of that preview (such as media_video_ and media_display_) as a neutral
> ground.

I agree.

> Another issue is control. It's quite natural to be suspicious about
> subsystems we're not familiar with, and giving up control on things we
> consider as highly important to other subsystems feels dangerous and
> wrong. Once again, media_* could be an answer to this problem. Instead of
> trying to push other subsystems to use our APIs (which are, obviously, the
> best possible ever, as they're the ones we work on :-)) with the promise
> that we will extend them to fullfill their needs if necessary, we could
> design new shared structures and core functions together with a media_
> prefix, and make sure they fullfill all the needs from the start. What I
> like about this approach is that each of the 3 video-related subsystems
> would then benefit from the experience of the other 2.
> 
> To make it perfectly clear, I want to emphasize that I'm not trying to
> replace DRM, FBDEV and V4L2 with a new shared subsystem. What I would like
> to see in the (near future) is collaboration and sharing of core features
> that make sense to share. I believe we should address this from a "pain
> point" point of view. The one that lead me to writing this e-mail is that
> developing a driver that implements both the DRM and FBDEV APIs for video
> output currently requires various similar structures, and code to
> translate between all of them. That code can't be shared between multiple
> drivers, is error-prone, and painful to maintin.
> 
> I can (and actually would like to) submit an RFC, but I would first like to
> hear your opinion on the topic.

I have added Jesse Barker to the CC list. I think that to achieve closer
cooperation between the various subsystems we need to organize a few days
of talks between the main developers to 1) understand each others subsystem,
2) to discover what functionality would be good to share, and 3) figure out
some sort of roadmap on how to do this.

Linaro might be a suitable organization that can assist with at least the
organizational part of setting up such a meeting. They did a good job in the
past (in Budapest and Cambourne) with the buffer sharing discussions.

One week of discussions/brainstorming between a small(ish) group of developers
can be very, very productive in my experience.

Regards,

	Hans

> 
> > If anyone has problems with the way the formats are defined, please
> > speak up now! Since only Jesse has bothered to comment on my rantings
> > I can only assume people are happy with my approach to things.
> > 
> > These patches should apply on top of Jesse's v3+v5 set... I think.
> > I sort of lost track of things when the patches started having
> > different version numbers. At least we're not yet into two digits
> > numbers ;)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
@ 2011-11-29 13:30     ` Hans Verkuil
  0 siblings, 0 replies; 24+ messages in thread
From: Hans Verkuil @ 2011-11-29 13:30 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, ville.syrjala, Tomi Valkeinen, linux-fbdev,
	linux-media, Jesse Barker

On Tuesday 29 November 2011 13:10:35 Laurent Pinchart wrote:
> Hi Ville,
> 
> Sorry for the late reply.
> 
> (Cross-posting to the linux-fbdev and linux-media mailing lists, as the
> topics I'm about to discuss are of interest to everybody)
> 
> On Wednesday 16 November 2011 19:42:23 ville.syrjala@linux.intel.com wrote:
> > I decided to go all out with the pixel format definitions. Added pretty
> > much all of the possible RGB/BGR variations. Just left out ones with
> > 16bit components and floats. Also added a whole bunch of YUV formats,
> > and 8 bit pseudocolor for good measure.
> > 
> > I'm sure some of the fourccs now clash with the ones used by v4l2,
> > but that's life.
> 
> Disclaimer: I realize this is a somehow controversial topic, and I'm not
> trying to start a flame war :-)
> 
> What about defining a common set of fourccs for both subsystem (and using
> them in FBDEV, which currently uses the V4L2 fourccs) ?
> 
> There's more and more overlap between DRM, FBDEV and V4L2, which results in
> confusion for the user and mess in the kernel. I believe cleaning this up
> will become more and more important with time, and also probably more and
> more difficult if we don't act now.
> 
> There's no way we will all quickly agree on a big picture unified
> architecture (I don't even know what it should look like), so I'm thinking
> about starting small and see where it will lead us. Sharing fourccs would
> be such a first step, and would make it easier for drivers to implement
> several of the DRM, FBDEV and V4L2 APIs.
> 
> Another step I'd like to take would be working on sharing the video mode
> definitions between subsystems. DRM has struct drm_mode_modeinfo and FBDEV
> has struct fb_videomode. The former can't be modified as it's used in
> userspace APIs, but I believe we should create a common in-kernel
> structure to represent modes. This would also make it easier to share the
> EDID parser between DRM and FBDEV.
> 
> One issue here is names. I understand that using names from another
> subsystem in a driver that has nothing to do with it (like using
> V4L2_PIX_FMT_* in DRM, or drm_mode_modeinfo in FBDEV) can be frustrating
> for many developers, so I'd like to propose an alternative. We have a
> media-related kernel API that is cross-subsystem, that's the media
> controller. It uses the prefix media_. We could use more specific versions
> of that preview (such as media_video_ and media_display_) as a neutral
> ground.

I agree.

> Another issue is control. It's quite natural to be suspicious about
> subsystems we're not familiar with, and giving up control on things we
> consider as highly important to other subsystems feels dangerous and
> wrong. Once again, media_* could be an answer to this problem. Instead of
> trying to push other subsystems to use our APIs (which are, obviously, the
> best possible ever, as they're the ones we work on :-)) with the promise
> that we will extend them to fullfill their needs if necessary, we could
> design new shared structures and core functions together with a media_
> prefix, and make sure they fullfill all the needs from the start. What I
> like about this approach is that each of the 3 video-related subsystems
> would then benefit from the experience of the other 2.
> 
> To make it perfectly clear, I want to emphasize that I'm not trying to
> replace DRM, FBDEV and V4L2 with a new shared subsystem. What I would like
> to see in the (near future) is collaboration and sharing of core features
> that make sense to share. I believe we should address this from a "pain
> point" point of view. The one that lead me to writing this e-mail is that
> developing a driver that implements both the DRM and FBDEV APIs for video
> output currently requires various similar structures, and code to
> translate between all of them. That code can't be shared between multiple
> drivers, is error-prone, and painful to maintin.
> 
> I can (and actually would like to) submit an RFC, but I would first like to
> hear your opinion on the topic.

I have added Jesse Barker to the CC list. I think that to achieve closer
cooperation between the various subsystems we need to organize a few days
of talks between the main developers to 1) understand each others subsystem,
2) to discover what functionality would be good to share, and 3) figure out
some sort of roadmap on how to do this.

Linaro might be a suitable organization that can assist with at least the
organizational part of setting up such a meeting. They did a good job in the
past (in Budapest and Cambourne) with the buffer sharing discussions.

One week of discussions/brainstorming between a small(ish) group of developers
can be very, very productive in my experience.

Regards,

	Hans

> 
> > If anyone has problems with the way the formats are defined, please
> > speak up now! Since only Jesse has bothered to comment on my rantings
> > I can only assume people are happy with my approach to things.
> > 
> > These patches should apply on top of Jesse's v3+v5 set... I think.
> > I sort of lost track of things when the patches started having
> > different version numbers. At least we're not yet into two digits
> > numbers ;)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
  2011-11-29 12:10   ` Laurent Pinchart
@ 2011-11-29 16:13     ` Tomi Valkeinen
  -1 siblings, 0 replies; 24+ messages in thread
From: Tomi Valkeinen @ 2011-11-29 16:13 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: dri-devel, ville.syrjala, linux-fbdev, linux-media

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

On Tue, 2011-11-29 at 13:10 +0100, Laurent Pinchart wrote:

> To make it perfectly clear, I want to emphasize that I'm not trying to replace 
> DRM, FBDEV and V4L2 with a new shared subsystem. What I would like to see in 
> the (near future) is collaboration and sharing of core features that make 
> sense to share. I believe we should address this from a "pain point" point of 
> view. The one that lead me to writing this e-mail is that developing a driver 
> that implements both the DRM and FBDEV APIs for video output currently 
> requires various similar structures, and code to translate between all of 
> them. That code can't be shared between multiple drivers, is error-prone, and 
> painful to maintin.

We've been in the same situation with OMAP display driver for years. The
same low level display driver is used by FB, V4L2 and now DRM drivers,
so I didn't have much choice but to implement omapdss versions for
things like video timings and color formats.

I've been planning to write code for this almost as long as omapdss has
had this problem, but there's always been
yet-another-important-thing-to-do. So good that somebody finally brought
this up =).

I'm happy to test any code related to this with omapdss.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: drm pixel formats update
@ 2011-11-29 16:13     ` Tomi Valkeinen
  0 siblings, 0 replies; 24+ messages in thread
From: Tomi Valkeinen @ 2011-11-29 16:13 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: dri-devel, ville.syrjala, linux-fbdev, linux-media

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

On Tue, 2011-11-29 at 13:10 +0100, Laurent Pinchart wrote:

> To make it perfectly clear, I want to emphasize that I'm not trying to replace 
> DRM, FBDEV and V4L2 with a new shared subsystem. What I would like to see in 
> the (near future) is collaboration and sharing of core features that make 
> sense to share. I believe we should address this from a "pain point" point of 
> view. The one that lead me to writing this e-mail is that developing a driver 
> that implements both the DRM and FBDEV APIs for video output currently 
> requires various similar structures, and code to translate between all of 
> them. That code can't be shared between multiple drivers, is error-prone, and 
> painful to maintin.

We've been in the same situation with OMAP display driver for years. The
same low level display driver is used by FB, V4L2 and now DRM drivers,
so I didn't have much choice but to implement omapdss versions for
things like video timings and color formats.

I've been planning to write code for this almost as long as omapdss has
had this problem, but there's always been
yet-another-important-thing-to-do. So good that somebody finally brought
this up =).

I'm happy to test any code related to this with omapdss.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2011-11-29 16:13 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-16 18:42 drm pixel formats update ville.syrjala
2011-11-16 18:42 ` [PATCH 1/2] drm: Add a missing ')' ville.syrjala
2011-11-16 18:42 ` [PATCH 2/2] drm: Redefine pixel formats ville.syrjala
2011-11-16 19:16   ` Ilyes Gouta
2011-11-16 22:28     ` Ville Syrjälä
2011-11-17  7:52   ` Michel Dänzer
2011-11-17 13:06     ` Ville Syrjälä
2011-11-17 14:00       ` Michel Dänzer
2011-11-17 14:40         ` Ville Syrjälä
2011-11-16 19:13 ` drm pixel formats update James Simmons
2011-11-16 19:54 ` Alan Cox
2011-11-16 21:19   ` Ville Syrjälä
2011-11-16 21:23     ` Jesse Barnes
2011-11-16 22:20       ` Ville Syrjälä
2011-11-17 17:06         ` Jesse Barnes
2011-11-17 21:05           ` Rob Clark
2011-11-16 21:26     ` Alan Cox
2011-11-16 22:16       ` Ville Syrjälä
2011-11-29 12:10 ` Laurent Pinchart
2011-11-29 12:10   ` Laurent Pinchart
2011-11-29 13:30   ` Hans Verkuil
2011-11-29 13:30     ` Hans Verkuil
2011-11-29 16:13   ` Tomi Valkeinen
2011-11-29 16:13     ` Tomi Valkeinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.