[PATCH v5 00/13] Centralize format information

[PATCH v5 00/13] Centralize format information

[Laurent Pinchart]

Hello,

Various pieces of information about DRM formats (number of planes, color
depth, chroma subsampling, ...) are scattered across different helper
functions in the DRM core. Callers of those functions often need to access
more than a single parameter of the format, leading to inefficiencies due to
multiple lookups.

This patch series addresses this issue by centralizing all format information
in a single data structure (01/13). It reimplements the existing format helper
functions based on that structure (02/13) and converts the DRM core code to
use the new structure (03/13). The DRM core now WARNs when a driver tries to
query information about an unsupported format (04/13).

The second part of the patch series removes the drm_fb_get_bpp_depth() legacy
function that shouldn't be used directly by drivers. It modifies all its users
to use the appropriate API instead (05/13 to 12/13) and finally merges the
function into its only caller in the DRM core (13/13).

The new API is also useful for drivers as shown by the "[PATCH v2 00/20] OMAP
DRM fixes and improvements" patch series previously posted.

All patches have been acked, the series is ready to be merged for v4.10.

Changes since v4:

- Rebased on top of latest drm/master branch
- Collected acks
- Fixed depth value of DRM_FORMAT_[AXRGB]{4}4444 formats to match current code
- Added support for DRM_FORMAT_BGR565, DRM_FORMAT_XBGR8888 and
  DRM_FORMAT_BGR888 to tilcdc
- Documented the depth field as legacy

Changes since v3:

- Rebased on top of latest drm/master branch
- Collected acks
- Dropped "drm: Move format-related helpers to drm_fourcc.c" and
  "drm/msm: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()" that
  have been merged already
- Added new "drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with
  drm_format_plane_cpp()" patch
- Coding style fixes and variable renames

Changes since v2:

- Remove bpp field from drm_format_info structure
- Replace all users of drm_fb_get_bpp_depth() with the appropriate API
- Merge drm_fb_get_bpp_depth() into its only caller

Changes since v1:

- Move format-related helpers to drm_fourcc.c
- Use named initializers for the formats array
- WARN when calling drm_format_info() for an unsupported format
- Don't drop the drm_format_plane_width() and drm_format_plane_height()
  helpers

Laurent Pinchart (13):
  drm: Centralize format information
  drm: Implement the drm_format_*() helpers as drm_format_info()
    wrappers
  drm: Use drm_format_info() in DRM core code
  drm: WARN when calling drm_format_info() for an unsupported format
  drm: hdlcd: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()
  drm: tilcdc: Replace drm_fb_get_bpp_depth() with
    drm_format_plane_cpp()
  drm: cirrus: Replace drm_fb_get_bpp_depth() with
    drm_format_plane_cpp()
  drm: gma500: Replace drm_fb_get_bpp_depth() with drm_format_info()
  drm: amdgpu: Replace drm_fb_get_bpp_depth() with
    drm_format_plane_cpp()
  drm: radeon: Replace drm_fb_get_bpp_depth() with
    drm_format_plane_cpp()
  drm: vmwgfx: Replace drm_fb_get_bpp_depth() with drm_format_info()
  drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with
    drm_format_plane_cpp()
  drm: Don't export the drm_fb_get_bpp_depth() function

 Documentation/gpu/drm-kms.rst           |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  |  14 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |   3 +-
 drivers/gpu/drm/arm/hdlcd_crtc.c        |   5 +-
 drivers/gpu/drm/arm/malidp_hw.c         |   7 +-
 drivers/gpu/drm/cirrus/cirrus_fbdev.c   |   6 +-
 drivers/gpu/drm/cirrus/cirrus_main.c    |   4 +-
 drivers/gpu/drm/drm_fb_cma_helper.c     |  23 +--
 drivers/gpu/drm/drm_fourcc.c            | 281 ++++++++++++++------------------
 drivers/gpu/drm/drm_framebuffer.c       | 102 ++----------
 drivers/gpu/drm/drm_modeset_helper.c    |  17 +-
 drivers/gpu/drm/gma500/framebuffer.c    |  20 +--
 drivers/gpu/drm/radeon/radeon_fb.c      |  20 +--
 drivers/gpu/drm/radeon/radeon_gem.c     |   3 +-
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c    |  18 +-
 drivers/gpu/drm/tilcdc/tilcdc_plane.c   |   7 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     |  12 +-
 include/drm/drm_fourcc.h                |  23 ++-
 18 files changed, 244 insertions(+), 324 deletions(-)

-- 
Regards,

Laurent Pinchart

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

[PATCH v5 01/13] drm: Centralize format information

[Laurent Pinchart]

Various pieces of information about DRM formats (number of planes, color
depth, chroma subsampling, ...) are scattered across different helper
functions in the DRM core. Callers of those functions often need to
access more than a single parameter of the format, leading to
inefficiencies due to multiple lookups.

Centralize all format information in a data structure and create a
function to look up information based on the format 4CC.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
Changes since v4:

- Fixed depth value of DRM_FORMAT_[AXRGB]{4}4444 formats to match
  current code
- Documented the depth field as legacy
---
 Documentation/gpu/drm-kms.rst |  3 ++
 drivers/gpu/drm/drm_fourcc.c  | 84 +++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_fourcc.h      | 21 +++++++++++
 3 files changed, 108 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 53b872c105d2..cb0d3537b705 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -63,6 +63,9 @@ Frame Buffer Functions Reference
 DRM Format Handling
 ===================
 
+.. kernel-doc:: include/drm/drm_fourcc.h
+   :internal:
+
 .. kernel-doc:: drivers/gpu/drm/drm_fourcc.c
    :export:
 
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 29c56b4331e0..39f09c564111 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -103,6 +103,90 @@ char *drm_get_format_name(uint32_t format)
 EXPORT_SYMBOL(drm_get_format_name);
 
 /**
+ * drm_format_info - query information for a given format
+ * @format: pixel format (DRM_FORMAT_*)
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *drm_format_info(u32 format)
+{
+	static const struct drm_format_info formats[] = {
+		{ .format = DRM_FORMAT_C8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGB332,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGR233,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XRGB4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XBGR4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBX4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRX4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ARGB4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ABGR4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBA4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRA4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XRGB1555,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XBGR1555,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBX5551,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRX5551,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ARGB1555,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ABGR1555,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBA5551,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRA5551,	.depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGB565,		.depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGR565,		.depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGB888,		.depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGR888,		.depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XRGB8888,	.depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XBGR8888,	.depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBX8888,	.depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRX8888,	.depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XRGB2101010,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_XBGR2101010,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBX1010102,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRX1010102,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ARGB2101010,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ABGR2101010,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBA1010102,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRA1010102,	.depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ARGB8888,	.depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_ABGR8888,	.depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_RGBA8888,	.depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_BGRA8888,	.depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_YUV410,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
+		{ .format = DRM_FORMAT_YVU410,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
+		{ .format = DRM_FORMAT_YUV411,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
+		{ .format = DRM_FORMAT_YVU411,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
+		{ .format = DRM_FORMAT_YUV420,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
+		{ .format = DRM_FORMAT_YVU420,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
+		{ .format = DRM_FORMAT_YUV422,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_YVU422,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_YUV444,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_YVU444,		.depth = 0,  .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_NV12,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
+		{ .format = DRM_FORMAT_NV21,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
+		{ .format = DRM_FORMAT_NV16,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_NV61,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_NV24,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_NV42,		.depth = 0,  .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_YUYV,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_YVYU,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_UYVY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_VYUY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_AYUV,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+	};
+
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(formats); ++i) {
+		if (formats[i].format == format)
+			return &formats[i];
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL(drm_format_info);
+
+/**
  * drm_fb_get_bpp_depth - get the bpp/depth values for format
  * @format: pixel format (DRM_FORMAT_*)
  * @depth: storage for the depth value
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 30c30fa87ee8..135fef050ee6 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -25,6 +25,27 @@
 #include <linux/types.h>
 #include <uapi/drm/drm_fourcc.h>
 
+/**
+ * struct drm_format_info - information about a DRM format
+ * @format: 4CC format identifier (DRM_FORMAT_*)
+ * @depth: Color depth (number of bits per pixel excluding padding bits),
+ *	valid for a subset of RGB formats only. This is a legacy field, do not
+ *	use in new code and set to 0 for new formats.
+ * @num_planes: Number of color planes (1 to 3)
+ * @cpp: Number of bytes per pixel (per plane)
+ * @hsub: Horizontal chroma subsampling factor
+ * @vsub: Vertical chroma subsampling factor
+ */
+struct drm_format_info {
+	u32 format;
+	u8 depth;
+	u8 num_planes;
+	u8 cpp[3];
+	u8 hsub;
+	u8 vsub;
+};
+
+const struct drm_format_info *drm_format_info(u32 format);
 uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
 int drm_format_num_planes(uint32_t format);
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 02/13] drm: Implement the drm_format_*() helpers as drm_format_info() wrappers

[Laurent Pinchart]

Turn the drm_format_*() helpers into wrappers around the drm_format_info
lookup function to centralize all format information in a single place.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
 drivers/gpu/drm/drm_fourcc.c | 186 +++++++++----------------------------------
 1 file changed, 37 insertions(+), 149 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 39f09c564111..23d4b82ec17c 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -198,69 +198,22 @@ EXPORT_SYMBOL(drm_format_info);
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
 			  int *bpp)
 {
-	char *format_name;
-
-	switch (format) {
-	case DRM_FORMAT_C8:
-	case DRM_FORMAT_RGB332:
-	case DRM_FORMAT_BGR233:
-		*depth = 8;
-		*bpp = 8;
-		break;
-	case DRM_FORMAT_XRGB1555:
-	case DRM_FORMAT_XBGR1555:
-	case DRM_FORMAT_RGBX5551:
-	case DRM_FORMAT_BGRX5551:
-	case DRM_FORMAT_ARGB1555:
-	case DRM_FORMAT_ABGR1555:
-	case DRM_FORMAT_RGBA5551:
-	case DRM_FORMAT_BGRA5551:
-		*depth = 15;
-		*bpp = 16;
-		break;
-	case DRM_FORMAT_RGB565:
-	case DRM_FORMAT_BGR565:
-		*depth = 16;
-		*bpp = 16;
-		break;
-	case DRM_FORMAT_RGB888:
-	case DRM_FORMAT_BGR888:
-		*depth = 24;
-		*bpp = 24;
-		break;
-	case DRM_FORMAT_XRGB8888:
-	case DRM_FORMAT_XBGR8888:
-	case DRM_FORMAT_RGBX8888:
-	case DRM_FORMAT_BGRX8888:
-		*depth = 24;
-		*bpp = 32;
-		break;
-	case DRM_FORMAT_XRGB2101010:
-	case DRM_FORMAT_XBGR2101010:
-	case DRM_FORMAT_RGBX1010102:
-	case DRM_FORMAT_BGRX1010102:
-	case DRM_FORMAT_ARGB2101010:
-	case DRM_FORMAT_ABGR2101010:
-	case DRM_FORMAT_RGBA1010102:
-	case DRM_FORMAT_BGRA1010102:
-		*depth = 30;
-		*bpp = 32;
-		break;
-	case DRM_FORMAT_ARGB8888:
-	case DRM_FORMAT_ABGR8888:
-	case DRM_FORMAT_RGBA8888:
-	case DRM_FORMAT_BGRA8888:
-		*depth = 32;
-		*bpp = 32;
-		break;
-	default:
-		format_name = drm_get_format_name(format);
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info || !info->depth) {
+		char *format_name = drm_get_format_name(format);
+
 		DRM_DEBUG_KMS("unsupported pixel format %s\n", format_name);
 		kfree(format_name);
+
 		*depth = 0;
 		*bpp = 0;
-		break;
+		return;
 	}
+
+	*depth = info->depth;
+	*bpp = info->cpp[0] * 8;
 }
 EXPORT_SYMBOL(drm_fb_get_bpp_depth);
 
@@ -273,28 +226,10 @@ EXPORT_SYMBOL(drm_fb_get_bpp_depth);
  */
 int drm_format_num_planes(uint32_t format)
 {
-	switch (format) {
-	case DRM_FORMAT_YUV410:
-	case DRM_FORMAT_YVU410:
-	case DRM_FORMAT_YUV411:
-	case DRM_FORMAT_YVU411:
-	case DRM_FORMAT_YUV420:
-	case DRM_FORMAT_YVU420:
-	case DRM_FORMAT_YUV422:
-	case DRM_FORMAT_YVU422:
-	case DRM_FORMAT_YUV444:
-	case DRM_FORMAT_YVU444:
-		return 3;
-	case DRM_FORMAT_NV12:
-	case DRM_FORMAT_NV21:
-	case DRM_FORMAT_NV16:
-	case DRM_FORMAT_NV61:
-	case DRM_FORMAT_NV24:
-	case DRM_FORMAT_NV42:
-		return 2;
-	default:
-		return 1;
-	}
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	return info ? info->num_planes : 1;
 }
 EXPORT_SYMBOL(drm_format_num_planes);
 
@@ -308,40 +243,13 @@ EXPORT_SYMBOL(drm_format_num_planes);
  */
 int drm_format_plane_cpp(uint32_t format, int plane)
 {
-	unsigned int depth;
-	int bpp;
+	const struct drm_format_info *info;
 
-	if (plane >= drm_format_num_planes(format))
+	info = drm_format_info(format);
+	if (!info || plane >= info->num_planes)
 		return 0;
 
-	switch (format) {
-	case DRM_FORMAT_YUYV:
-	case DRM_FORMAT_YVYU:
-	case DRM_FORMAT_UYVY:
-	case DRM_FORMAT_VYUY:
-		return 2;
-	case DRM_FORMAT_NV12:
-	case DRM_FORMAT_NV21:
-	case DRM_FORMAT_NV16:
-	case DRM_FORMAT_NV61:
-	case DRM_FORMAT_NV24:
-	case DRM_FORMAT_NV42:
-		return plane ? 2 : 1;
-	case DRM_FORMAT_YUV410:
-	case DRM_FORMAT_YVU410:
-	case DRM_FORMAT_YUV411:
-	case DRM_FORMAT_YVU411:
-	case DRM_FORMAT_YUV420:
-	case DRM_FORMAT_YVU420:
-	case DRM_FORMAT_YUV422:
-	case DRM_FORMAT_YVU422:
-	case DRM_FORMAT_YUV444:
-	case DRM_FORMAT_YVU444:
-		return 1;
-	default:
-		drm_fb_get_bpp_depth(format, &depth, &bpp);
-		return bpp >> 3;
-	}
+	return info->cpp[plane];
 }
 EXPORT_SYMBOL(drm_format_plane_cpp);
 
@@ -355,28 +263,10 @@ EXPORT_SYMBOL(drm_format_plane_cpp);
  */
 int drm_format_horz_chroma_subsampling(uint32_t format)
 {
-	switch (format) {
-	case DRM_FORMAT_YUV411:
-	case DRM_FORMAT_YVU411:
-	case DRM_FORMAT_YUV410:
-	case DRM_FORMAT_YVU410:
-		return 4;
-	case DRM_FORMAT_YUYV:
-	case DRM_FORMAT_YVYU:
-	case DRM_FORMAT_UYVY:
-	case DRM_FORMAT_VYUY:
-	case DRM_FORMAT_NV12:
-	case DRM_FORMAT_NV21:
-	case DRM_FORMAT_NV16:
-	case DRM_FORMAT_NV61:
-	case DRM_FORMAT_YUV422:
-	case DRM_FORMAT_YVU422:
-	case DRM_FORMAT_YUV420:
-	case DRM_FORMAT_YVU420:
-		return 2;
-	default:
-		return 1;
-	}
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	return info ? info->hsub : 1;
 }
 EXPORT_SYMBOL(drm_format_horz_chroma_subsampling);
 
@@ -390,18 +280,10 @@ EXPORT_SYMBOL(drm_format_horz_chroma_subsampling);
  */
 int drm_format_vert_chroma_subsampling(uint32_t format)
 {
-	switch (format) {
-	case DRM_FORMAT_YUV410:
-	case DRM_FORMAT_YVU410:
-		return 4;
-	case DRM_FORMAT_YUV420:
-	case DRM_FORMAT_YVU420:
-	case DRM_FORMAT_NV12:
-	case DRM_FORMAT_NV21:
-		return 2;
-	default:
-		return 1;
-	}
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	return info ? info->vsub : 1;
 }
 EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
 
@@ -416,13 +298,16 @@ EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
  */
 int drm_format_plane_width(int width, uint32_t format, int plane)
 {
-	if (plane >= drm_format_num_planes(format))
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info || plane >= info->num_planes)
 		return 0;
 
 	if (plane == 0)
 		return width;
 
-	return width / drm_format_horz_chroma_subsampling(format);
+	return width / info->hsub;
 }
 EXPORT_SYMBOL(drm_format_plane_width);
 
@@ -437,12 +322,15 @@ EXPORT_SYMBOL(drm_format_plane_width);
  */
 int drm_format_plane_height(int height, uint32_t format, int plane)
 {
-	if (plane >= drm_format_num_planes(format))
+	const struct drm_format_info *info;
+
+	info = drm_format_info(format);
+	if (!info || plane >= info->num_planes)
 		return 0;
 
 	if (plane == 0)
 		return height;
 
-	return height / drm_format_vert_chroma_subsampling(format);
+	return height / info->vsub;
 }
 EXPORT_SYMBOL(drm_format_plane_height);
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 03/13] drm: Use drm_format_info() in DRM core code

[Laurent Pinchart]

Replace calls to the drm_format_*() helper functions with direct use of
the drm_format_info structure. This improves efficiency by removing
duplicate lookups.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
 drivers/gpu/drm/drm_fb_cma_helper.c |  23 ++++----
 drivers/gpu/drm/drm_framebuffer.c   | 102 +++++-------------------------------
 2 files changed, 25 insertions(+), 100 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 1fd6eac1400c..fac4f06f8485 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -176,20 +176,20 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
 	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
 	const struct drm_framebuffer_funcs *funcs)
 {
+	const struct drm_format_info *info;
 	struct drm_fb_cma *fb_cma;
 	struct drm_gem_cma_object *objs[4];
 	struct drm_gem_object *obj;
-	unsigned int hsub;
-	unsigned int vsub;
 	int ret;
 	int i;
 
-	hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format);
-	vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format);
+	info = drm_format_info(mode_cmd->pixel_format);
+	if (!info)
+		return ERR_PTR(-EINVAL);
 
-	for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
-		unsigned int width = mode_cmd->width / (i ? hsub : 1);
-		unsigned int height = mode_cmd->height / (i ? vsub : 1);
+	for (i = 0; i < info->num_planes; i++) {
+		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
+		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
 		unsigned int min_size;
 
 		obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
@@ -200,7 +200,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
 		}
 
 		min_size = (height - 1) * mode_cmd->pitches[i]
-			 + width * drm_format_plane_cpp(mode_cmd->pixel_format, i)
+			 + width * info->cpp[i]
 			 + mode_cmd->offsets[i];
 
 		if (obj->size < min_size) {
@@ -269,12 +269,15 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
 static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
 {
 	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
-	int i, n = drm_format_num_planes(fb->pixel_format);
+	const struct drm_format_info *info;
+	int i;
 
 	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
 			(char *)&fb->pixel_format);
 
-	for (i = 0; i < n; i++) {
+	info = drm_format_info(fb->pixel_format);
+
+	for (i = 0; i < info->num_planes; i++) {
 		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
 				i, fb->offsets[i], fb->pitches[i]);
 		drm_gem_cma_describe(fb_cma->obj[i], m);
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 398efd67cb93..386977df72ce 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -126,111 +126,33 @@ int drm_mode_addfb(struct drm_device *dev,
 	return 0;
 }
 
-static int format_check(const struct drm_mode_fb_cmd2 *r)
-{
-	uint32_t format = r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN;
-	char *format_name;
-
-	switch (format) {
-	case DRM_FORMAT_C8:
-	case DRM_FORMAT_RGB332:
-	case DRM_FORMAT_BGR233:
-	case DRM_FORMAT_XRGB4444:
-	case DRM_FORMAT_XBGR4444:
-	case DRM_FORMAT_RGBX4444:
-	case DRM_FORMAT_BGRX4444:
-	case DRM_FORMAT_ARGB4444:
-	case DRM_FORMAT_ABGR4444:
-	case DRM_FORMAT_RGBA4444:
-	case DRM_FORMAT_BGRA4444:
-	case DRM_FORMAT_XRGB1555:
-	case DRM_FORMAT_XBGR1555:
-	case DRM_FORMAT_RGBX5551:
-	case DRM_FORMAT_BGRX5551:
-	case DRM_FORMAT_ARGB1555:
-	case DRM_FORMAT_ABGR1555:
-	case DRM_FORMAT_RGBA5551:
-	case DRM_FORMAT_BGRA5551:
-	case DRM_FORMAT_RGB565:
-	case DRM_FORMAT_BGR565:
-	case DRM_FORMAT_RGB888:
-	case DRM_FORMAT_BGR888:
-	case DRM_FORMAT_XRGB8888:
-	case DRM_FORMAT_XBGR8888:
-	case DRM_FORMAT_RGBX8888:
-	case DRM_FORMAT_BGRX8888:
-	case DRM_FORMAT_ARGB8888:
-	case DRM_FORMAT_ABGR8888:
-	case DRM_FORMAT_RGBA8888:
-	case DRM_FORMAT_BGRA8888:
-	case DRM_FORMAT_XRGB2101010:
-	case DRM_FORMAT_XBGR2101010:
-	case DRM_FORMAT_RGBX1010102:
-	case DRM_FORMAT_BGRX1010102:
-	case DRM_FORMAT_ARGB2101010:
-	case DRM_FORMAT_ABGR2101010:
-	case DRM_FORMAT_RGBA1010102:
-	case DRM_FORMAT_BGRA1010102:
-	case DRM_FORMAT_YUYV:
-	case DRM_FORMAT_YVYU:
-	case DRM_FORMAT_UYVY:
-	case DRM_FORMAT_VYUY:
-	case DRM_FORMAT_AYUV:
-	case DRM_FORMAT_NV12:
-	case DRM_FORMAT_NV21:
-	case DRM_FORMAT_NV16:
-	case DRM_FORMAT_NV61:
-	case DRM_FORMAT_NV24:
-	case DRM_FORMAT_NV42:
-	case DRM_FORMAT_YUV410:
-	case DRM_FORMAT_YVU410:
-	case DRM_FORMAT_YUV411:
-	case DRM_FORMAT_YVU411:
-	case DRM_FORMAT_YUV420:
-	case DRM_FORMAT_YVU420:
-	case DRM_FORMAT_YUV422:
-	case DRM_FORMAT_YVU422:
-	case DRM_FORMAT_YUV444:
-	case DRM_FORMAT_YVU444:
-		return 0;
-	default:
-		format_name = drm_get_format_name(r->pixel_format);
-		DRM_DEBUG_KMS("invalid pixel format %s\n", format_name);
-		kfree(format_name);
-		return -EINVAL;
-	}
-}
-
 static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
 {
-	int ret, hsub, vsub, num_planes, i;
+	const struct drm_format_info *info;
+	int i;
 
-	ret = format_check(r);
-	if (ret) {
+	info = drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
+	if (!info) {
 		char *format_name = drm_get_format_name(r->pixel_format);
 		DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
 		kfree(format_name);
-		return ret;
+		return -EINVAL;
 	}
 
-	hsub = drm_format_horz_chroma_subsampling(r->pixel_format);
-	vsub = drm_format_vert_chroma_subsampling(r->pixel_format);
-	num_planes = drm_format_num_planes(r->pixel_format);
-
-	if (r->width == 0 || r->width % hsub) {
+	if (r->width == 0 || r->width % info->hsub) {
 		DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
 		return -EINVAL;
 	}
 
-	if (r->height == 0 || r->height % vsub) {
+	if (r->height == 0 || r->height % info->vsub) {
 		DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
 		return -EINVAL;
 	}
 
-	for (i = 0; i < num_planes; i++) {
-		unsigned int width = r->width / (i != 0 ? hsub : 1);
-		unsigned int height = r->height / (i != 0 ? vsub : 1);
-		unsigned int cpp = drm_format_plane_cpp(r->pixel_format, i);
+	for (i = 0; i < info->num_planes; i++) {
+		unsigned int width = r->width / (i != 0 ? info->hsub : 1);
+		unsigned int height = r->height / (i != 0 ? info->vsub : 1);
+		unsigned int cpp = info->cpp[i];
 
 		if (!r->handles[i]) {
 			DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i);
@@ -273,7 +195,7 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
 		}
 	}
 
-	for (i = num_planes; i < 4; i++) {
+	for (i = info->num_planes; i < 4; i++) {
 		if (r->modifier[i]) {
 			DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i);
 			return -EINVAL;
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 04/13] drm: WARN when calling drm_format_info() for an unsupported format

[Laurent Pinchart]

The format helpers have historically treated unsupported formats as part
of the default case, returning values that are likely wrong. We can't
change this behaviour now without risking breaking drivers in difficult
to detect ways, but we can WARN on unsupported formats to catch faulty
callers.

The only exception is the framebuffer_check() function that calls
drm_format_info() to validate the format passed from userspace. This is
a valid use case that shouldn't generate a warning.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/drm_fourcc.c      | 32 ++++++++++++++++++++++++--------
 drivers/gpu/drm/drm_framebuffer.c |  2 +-
 include/drm/drm_fourcc.h          |  1 +
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 23d4b82ec17c..523ed916a1c0 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -102,15 +102,11 @@ char *drm_get_format_name(uint32_t format)
 }
 EXPORT_SYMBOL(drm_get_format_name);
 
-/**
- * drm_format_info - query information for a given format
- * @format: pixel format (DRM_FORMAT_*)
- *
- * Returns:
- * The instance of struct drm_format_info that describes the pixel format, or
- * NULL if the format is unsupported.
+/*
+ * Internal function to query information for a given format. See
+ * drm_format_info() for the public API.
  */
-const struct drm_format_info *drm_format_info(u32 format)
+const struct drm_format_info *__drm_format_info(u32 format)
 {
 	static const struct drm_format_info formats[] = {
 		{ .format = DRM_FORMAT_C8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
@@ -184,6 +180,26 @@ const struct drm_format_info *drm_format_info(u32 format)
 
 	return NULL;
 }
+
+/**
+ * drm_format_info - query information for a given format
+ * @format: pixel format (DRM_FORMAT_*)
+ *
+ * The caller should only pass a supported pixel format to this function.
+ * Unsupported pixel formats will generate a warning in the kernel log.
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *drm_format_info(u32 format)
+{
+	const struct drm_format_info *info;
+
+	info = __drm_format_info(format);
+	WARN_ON(!info);
+	return info;
+}
 EXPORT_SYMBOL(drm_format_info);
 
 /**
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 386977df72ce..49fd7db758e0 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -131,7 +131,7 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
 	const struct drm_format_info *info;
 	int i;
 
-	info = drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
+	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
 	if (!info) {
 		char *format_name = drm_get_format_name(r->pixel_format);
 		DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 135fef050ee6..f73f97afd1e2 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -45,6 +45,7 @@ struct drm_format_info {
 	u8 vsub;
 };
 
+const struct drm_format_info *__drm_format_info(u32 format);
 const struct drm_format_info *drm_format_info(u32 format);
 uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v5 04/13] drm: WARN when calling drm_format_info() for an unsupported format

[Eric Engestrom]

On Tuesday, 2016-10-18 01:41:12 +0300, Laurent Pinchart wrote:
> The format helpers have historically treated unsupported formats as part
> of the default case, returning values that are likely wrong. We can't
> change this behaviour now without risking breaking drivers in difficult
> to detect ways, but we can WARN on unsupported formats to catch faulty
> callers.
> 
> The only exception is the framebuffer_check() function that calls
> drm_format_info() to validate the format passed from userspace. This is
> a valid use case that shouldn't generate a warning.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>

> ---
>  drivers/gpu/drm/drm_fourcc.c      | 32 ++++++++++++++++++++++++--------
>  drivers/gpu/drm/drm_framebuffer.c |  2 +-
>  include/drm/drm_fourcc.h          |  1 +
>  3 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 23d4b82ec17c..523ed916a1c0 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -102,15 +102,11 @@ char *drm_get_format_name(uint32_t format)
>  }
>  EXPORT_SYMBOL(drm_get_format_name);
>  
> -/**
> - * drm_format_info - query information for a given format
> - * @format: pixel format (DRM_FORMAT_*)
> - *
> - * Returns:
> - * The instance of struct drm_format_info that describes the pixel format, or
> - * NULL if the format is unsupported.
> +/*
> + * Internal function to query information for a given format. See
> + * drm_format_info() for the public API.
>   */
> -const struct drm_format_info *drm_format_info(u32 format)
> +const struct drm_format_info *__drm_format_info(u32 format)
>  {
>  	static const struct drm_format_info formats[] = {
>  		{ .format = DRM_FORMAT_C8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> @@ -184,6 +180,26 @@ const struct drm_format_info *drm_format_info(u32 format)
>  
>  	return NULL;
>  }
> +
> +/**
> + * drm_format_info - query information for a given format
> + * @format: pixel format (DRM_FORMAT_*)
> + *
> + * The caller should only pass a supported pixel format to this function.
> + * Unsupported pixel formats will generate a warning in the kernel log.
> + *
> + * Returns:
> + * The instance of struct drm_format_info that describes the pixel format, or
> + * NULL if the format is unsupported.
> + */
> +const struct drm_format_info *drm_format_info(u32 format)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = __drm_format_info(format);
> +	WARN_ON(!info);
> +	return info;
> +}
>  EXPORT_SYMBOL(drm_format_info);
>  
>  /**
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 386977df72ce..49fd7db758e0 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -131,7 +131,7 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
>  	const struct drm_format_info *info;
>  	int i;
>  
> -	info = drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
> +	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
>  	if (!info) {
>  		char *format_name = drm_get_format_name(r->pixel_format);
>  		DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 135fef050ee6..f73f97afd1e2 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,7 @@ struct drm_format_info {
>  	u8 vsub;
>  };
>  
> +const struct drm_format_info *__drm_format_info(u32 format);
>  const struct drm_format_info *drm_format_info(u32 format);
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
>  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 05/13] drm: hdlcd: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

[Laurent Pinchart]

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 48019ae22ddb..bbaa55add2d2 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -223,14 +223,12 @@ static void hdlcd_plane_atomic_update(struct drm_plane *plane,
 {
 	struct hdlcd_drm_private *hdlcd;
 	struct drm_gem_cma_object *gem;
-	unsigned int depth, bpp;
 	u32 src_w, src_h, dest_w, dest_h;
 	dma_addr_t scanout_start;
 
 	if (!plane->state->fb)
 		return;
 
-	drm_fb_get_bpp_depth(plane->state->fb->pixel_format, &depth, &bpp);
 	src_w = plane->state->src_w >> 16;
 	src_h = plane->state->src_h >> 16;
 	dest_w = plane->state->crtc_w;
@@ -238,7 +236,8 @@ static void hdlcd_plane_atomic_update(struct drm_plane *plane,
 	gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
 	scanout_start = gem->paddr + plane->state->fb->offsets[0] +
 		plane->state->crtc_y * plane->state->fb->pitches[0] +
-		plane->state->crtc_x * bpp / 8;
+		plane->state->crtc_x *
+		drm_format_plane_cpp(plane->state->fb->pixel_format, 0);
 
 	hdlcd = plane->dev->dev_private;
 	hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, plane->state->fb->pitches[0]);
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 06/13] drm: tilcdc: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

[Laurent Pinchart]

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

In the tilcdc_crtc_mode_set() function compute the hardware register
value directly from the pixel format instead of computing the number of
bits per pixels first.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
Changes since v4:

- Added support for DRM_FORMAT_BGR565, DRM_FORMAT_XBGR8888 and
  DRM_FORMAT_BGR888

Changes since v3:

- Removed DRM_FORMAT_ARGB8888 support
- Fixed coding style
- Renamed min_pitch to pitch
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c  | 18 ++++++++----------
 drivers/gpu/drm/tilcdc/tilcdc_plane.c |  7 ++++---
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 52ebe8fc1784..822531ebd4b0 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -72,16 +72,14 @@ static void set_scanout(struct drm_crtc *crtc, struct drm_framebuffer *fb)
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct drm_gem_cma_object *gem;
-	unsigned int depth, bpp;
 	dma_addr_t start, end;
 	u64 dma_base_and_ceiling;
 
-	drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
 	gem = drm_fb_cma_get_gem_obj(fb, 0);
 
 	start = gem->paddr + fb->offsets[0] +
 		crtc->y * fb->pitches[0] +
-		crtc->x * bpp / 8;
+		crtc->x * drm_format_plane_cpp(fb->pixel_format, 0);
 
 	end = start + (crtc->mode.vdisplay * fb->pitches[0]);
 
@@ -461,16 +459,16 @@ static void tilcdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	if (info->tft_alt_mode)
 		reg |= LCDC_TFT_ALT_ENABLE;
 	if (priv->rev == 2) {
-		unsigned int depth, bpp;
-
-		drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
-		switch (bpp) {
-		case 16:
+		switch (fb->pixel_format) {
+		case DRM_FORMAT_BGR565:
+		case DRM_FORMAT_RGB565:
 			break;
-		case 32:
+		case DRM_FORMAT_XBGR8888:
+		case DRM_FORMAT_XRGB8888:
 			reg |= LCDC_V2_TFT_24BPP_UNPACK;
 			/* fallthrough */
-		case 24:
+		case DRM_FORMAT_BGR888:
+		case DRM_FORMAT_RGB888:
 			reg |= LCDC_V2_TFT_24BPP_MODE;
 			break;
 		default:
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
index 74c65fa859b2..8a6a50d74aff 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
@@ -39,7 +39,7 @@ static int tilcdc_plane_atomic_check(struct drm_plane *plane,
 {
 	struct drm_crtc_state *crtc_state;
 	struct drm_plane_state *old_state = plane->state;
-	unsigned int depth, bpp;
+	unsigned int pitch;
 
 	if (!state->crtc)
 		return 0;
@@ -68,8 +68,9 @@ static int tilcdc_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
-	drm_fb_get_bpp_depth(state->fb->pixel_format, &depth, &bpp);
-	if (state->fb->pitches[0] != crtc_state->mode.hdisplay * bpp / 8) {
+	pitch = crtc_state->mode.hdisplay *
+		drm_format_plane_cpp(state->fb->pixel_format, 0);
+	if (state->fb->pitches[0] != pitch) {
 		dev_err(plane->dev->dev,
 			"Invalid pitch: fb and crtc widths must be the same");
 		return -EINVAL;
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 07/13] drm: cirrus: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

[Laurent Pinchart]

The driver doesn't need the color depth, only the number of bits per
pixel. Use the right API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/cirrus/cirrus_fbdev.c | 6 +++---
 drivers/gpu/drm/cirrus/cirrus_main.c  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index daecf1ad76a4..3a6309d7d8e4 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -138,12 +138,12 @@ static int cirrusfb_create_object(struct cirrus_fbdev *afbdev,
 {
 	struct drm_device *dev = afbdev->helper.dev;
 	struct cirrus_device *cdev = dev->dev_private;
-	u32 bpp, depth;
+	u32 bpp;
 	u32 size;
 	struct drm_gem_object *gobj;
-
 	int ret = 0;
-	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+
+	bpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0) * 8;
 
 	if (!cirrus_check_framebuffer(cdev, mode_cmd->width, mode_cmd->height,
 				      bpp, mode_cmd->pitches[0]))
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c
index 76bcb43e7c06..2c3c0d4072ce 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -52,10 +52,10 @@ cirrus_user_framebuffer_create(struct drm_device *dev,
 	struct cirrus_device *cdev = dev->dev_private;
 	struct drm_gem_object *obj;
 	struct cirrus_framebuffer *cirrus_fb;
+	u32 bpp;
 	int ret;
-	u32 bpp, depth;
 
-	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+	bpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0) * 8;
 
 	if (!cirrus_check_framebuffer(cdev, mode_cmd->width, mode_cmd->height,
 				      bpp, mode_cmd->pitches[0]))
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 08/13] drm: gma500: Replace drm_fb_get_bpp_depth() with drm_format_info()

[Laurent Pinchart]

The driver uses drm_fb_get_bpp_depth() to check whether it can support
the format requested by userspace when creating a framebuffer. This
isn't the right API, as it doesn't differentiate between RGB formats
other than on a depth and bpp basis.

Fixing this requires non trivial changes to the drivers internals. As a
first step, replace usage of the drm_fb_get_bpp_depth() function with an
equivalent check based on drm_format_info(). This is part of a wider
effort to remove usage of the drm_fb_get_bpp_depth() function in
drivers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/gma500/framebuffer.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 3a44e705db53..6cb92cc0bef8 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -236,22 +236,20 @@ static int psb_framebuffer_init(struct drm_device *dev,
 					const struct drm_mode_fb_cmd2 *mode_cmd,
 					struct gtt_range *gt)
 {
-	u32 bpp, depth;
+	const struct drm_format_info *info;
 	int ret;
 
-	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+	/*
+	 * Reject unknown formats, YUV formats, and formats with more than
+	 * 4 bytes per pixel.
+	 */
+	info = drm_format_info(mode_cmd->pixel_format);
+	if (!info || !info->depth || info->cpp[0] > 4)
+		return -EINVAL;
 
 	if (mode_cmd->pitches[0] & 63)
 		return -EINVAL;
-	switch (bpp) {
-	case 8:
-	case 16:
-	case 24:
-	case 32:
-		break;
-	default:
-		return -EINVAL;
-	}
+
 	drm_helper_mode_fill_fb_struct(&fb->base, mode_cmd);
 	fb->gtt = gt;
 	ret = drm_framebuffer_init(dev, &fb->base, &psb_fb_funcs);
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 09/13] drm: amdgpu: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

[Laurent Pinchart]

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Changes since v4:

- Added Daniel Vetter's Reviewed-by received off-line

Changes since v3:

- Renamed bpp to cpp
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  | 14 +++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  3 ++-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 9fb8aa4d6bae..8d01aa24d68a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -90,12 +90,12 @@ static struct fb_ops amdgpufb_ops = {
 };
 
 
-int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int bpp, bool tiled)
+int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int cpp, bool tiled)
 {
 	int aligned = width;
 	int pitch_mask = 0;
 
-	switch (bpp / 8) {
+	switch (cpp) {
 	case 1:
 		pitch_mask = 255;
 		break;
@@ -110,7 +110,7 @@ int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int bpp, bool tile
 
 	aligned += pitch_mask;
 	aligned &= ~pitch_mask;
-	return aligned;
+	return aligned * cpp;
 }
 
 static void amdgpufb_destroy_pinned_object(struct drm_gem_object *gobj)
@@ -139,13 +139,13 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
 	int ret;
 	int aligned_size, size;
 	int height = mode_cmd->height;
-	u32 bpp, depth;
+	u32 cpp;
 
-	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+	cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
 
 	/* need to align pitch with crtc limits */
-	mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, bpp,
-						  fb_tiled) * ((bpp + 1) / 8);
+	mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
+						  fb_tiled);
 
 	height = ALIGN(mode_cmd->height, 8);
 	size = mode_cmd->pitches[0] * height;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index a7ea9a3b454e..63d89f38d680 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -704,7 +704,8 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
 	uint32_t handle;
 	int r;
 
-	args->pitch = amdgpu_align_pitch(adev, args->width, args->bpp, 0) * ((args->bpp + 1) / 8);
+	args->pitch = amdgpu_align_pitch(adev, args->width,
+					 DIV_ROUND_UP(args->bpp, 8), 0);
 	args->size = (u64)args->pitch * args->height;
 	args->size = ALIGN(args->size, PAGE_SIZE);
 
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 10/13] drm: radeon: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

[Laurent Pinchart]

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Changes since v4:

- Added Daniel Vetter's Reviewed-by received off-line

Changes since v3:

- Renamed bpp to cpp
---
 drivers/gpu/drm/radeon/radeon_fb.c  | 20 ++++++++++----------
 drivers/gpu/drm/radeon/radeon_gem.c |  3 ++-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 0daad446d2c7..f65f29911dca 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -89,13 +89,13 @@ static struct fb_ops radeonfb_ops = {
 };
 
 
-int radeon_align_pitch(struct radeon_device *rdev, int width, int bpp, bool tiled)
+int radeon_align_pitch(struct radeon_device *rdev, int width, int cpp, bool tiled)
 {
 	int aligned = width;
 	int align_large = (ASIC_IS_AVIVO(rdev)) || tiled;
 	int pitch_mask = 0;
 
-	switch (bpp / 8) {
+	switch (cpp) {
 	case 1:
 		pitch_mask = align_large ? 255 : 127;
 		break;
@@ -110,7 +110,7 @@ int radeon_align_pitch(struct radeon_device *rdev, int width, int bpp, bool tile
 
 	aligned += pitch_mask;
 	aligned &= ~pitch_mask;
-	return aligned;
+	return aligned * cpp;
 }
 
 static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
@@ -139,13 +139,13 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
 	int ret;
 	int aligned_size, size;
 	int height = mode_cmd->height;
-	u32 bpp, depth;
+	u32 cpp;
 
-	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+	cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
 
 	/* need to align pitch with crtc limits */
-	mode_cmd->pitches[0] = radeon_align_pitch(rdev, mode_cmd->width, bpp,
-						  fb_tiled) * ((bpp + 1) / 8);
+	mode_cmd->pitches[0] = radeon_align_pitch(rdev, mode_cmd->width, cpp,
+						  fb_tiled);
 
 	if (rdev->family >= CHIP_R600)
 		height = ALIGN(mode_cmd->height, 8);
@@ -165,11 +165,11 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
 		tiling_flags = RADEON_TILING_MACRO;
 
 #ifdef __BIG_ENDIAN
-	switch (bpp) {
-	case 32:
+	switch (cpp) {
+	case 4:
 		tiling_flags |= RADEON_TILING_SWAP_32BIT;
 		break;
-	case 16:
+	case 2:
 		tiling_flags |= RADEON_TILING_SWAP_16BIT;
 	default:
 		break;
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index deb9511725c9..0bcffd8a7bd3 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -745,7 +745,8 @@ int radeon_mode_dumb_create(struct drm_file *file_priv,
 	uint32_t handle;
 	int r;
 
-	args->pitch = radeon_align_pitch(rdev, args->width, args->bpp, 0) * ((args->bpp + 1) / 8);
+	args->pitch = radeon_align_pitch(rdev, args->width,
+					 DIV_ROUND_UP(args->bpp, 8), 0);
 	args->size = args->pitch * args->height;
 	args->size = ALIGN(args->size, PAGE_SIZE);
 
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 11/13] drm: vmwgfx: Replace drm_fb_get_bpp_depth() with drm_format_info()

[Laurent Pinchart]

The driver is the last users of the drm_fb_get_bpp_depth() function. It
should ideally be converted to use struct drm_mode_fb_cmd2 instead of
the legacy struct drm_mode_fb_cmd internally, but that will require
broad changes across the code base. As a first step, replace
drm_fb_get_bpp_depth() with drm_format_info() in order to stop exporting
the function to drivers.

The new DRM_ERROR() message comes from the vmw_create_dmabuf_proxy(),
vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_dmabuf()
functions that currently print an error if the pixel format is
unsupported.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index bf28ccc150df..c965514b82be 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -980,14 +980,22 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
 	struct vmw_dma_buffer *bo = NULL;
 	struct ttm_base_object *user_obj;
 	struct drm_mode_fb_cmd mode_cmd;
+	const struct drm_format_info *info;
 	int ret;
 
+	info = drm_format_info(mode_cmd2->pixel_format);
+	if (!info || !info->depth) {
+		DRM_ERROR("Unsupported framebuffer format %s\n",
+			  drm_get_format_name(mode_cmd2->pixel_format));
+		return ERR_PTR(-EINVAL);
+	}
+
 	mode_cmd.width = mode_cmd2->width;
 	mode_cmd.height = mode_cmd2->height;
 	mode_cmd.pitch = mode_cmd2->pitches[0];
 	mode_cmd.handle = mode_cmd2->handles[0];
-	drm_fb_get_bpp_depth(mode_cmd2->pixel_format, &mode_cmd.depth,
-				    &mode_cmd.bpp);
+	mode_cmd.depth = info->depth;
+	mode_cmd.bpp = info->cpp[0] * 8;
 
 	/**
 	 * This code should be conditioned on Screen Objects not being used.
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 12/13] drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

[Laurent Pinchart]

The driver doesn't need the color depth, only the number of bits per
pixel. Use the right API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/arm/malidp_hw.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index a6132f1d58c1..be815d0cc772 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -198,9 +198,6 @@ static void malidp500_modeset(struct malidp_hw_device *hwdev, struct videomode *
 
 static int malidp500_rotmem_required(struct malidp_hw_device *hwdev, u16 w, u16 h, u32 fmt)
 {
-	unsigned int depth;
-	int bpp;
-
 	/* RGB888 or BGR888 can't be rotated */
 	if ((fmt == DRM_FORMAT_RGB888) || (fmt == DRM_FORMAT_BGR888))
 		return -EINVAL;
@@ -210,9 +207,7 @@ static int malidp500_rotmem_required(struct malidp_hw_device *hwdev, u16 w, u16
 	 * worth of pixel data. Required size is then:
 	 *    size = rotated_width * (bpp / 8) * 8;
 	 */
-	drm_fb_get_bpp_depth(fmt, &depth, &bpp);
-
-	return w * bpp;
+	return w * drm_format_plane_cpp(fmt, 0) * 8;
 }
 
 static int malidp550_query_hw(struct malidp_hw_device *hwdev)
-- 
Regards,

Laurent Pinchart

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

[PATCH v5 13/13] drm: Don't export the drm_fb_get_bpp_depth() function

[Laurent Pinchart]

The function is only used by the drm_helper_mode_fill_fb_struct() core
function to fill the drm_framebuffer bpp and depth fields, used by
drivers that haven't been converted to use pixel formats directly yet.
It should not be used by new drivers, so inline it in its only caller.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/drm_fourcc.c         | 31 -------------------------------
 drivers/gpu/drm/drm_modeset_helper.c | 17 +++++++++++++++--
 include/drm/drm_fourcc.h             |  1 -
 3 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 523ed916a1c0..cbb8b77c363c 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -203,37 +203,6 @@ const struct drm_format_info *drm_format_info(u32 format)
 EXPORT_SYMBOL(drm_format_info);
 
 /**
- * drm_fb_get_bpp_depth - get the bpp/depth values for format
- * @format: pixel format (DRM_FORMAT_*)
- * @depth: storage for the depth value
- * @bpp: storage for the bpp value
- *
- * This only supports RGB formats here for compat with code that doesn't use
- * pixel formats directly yet.
- */
-void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
-			  int *bpp)
-{
-	const struct drm_format_info *info;
-
-	info = drm_format_info(format);
-	if (!info || !info->depth) {
-		char *format_name = drm_get_format_name(format);
-
-		DRM_DEBUG_KMS("unsupported pixel format %s\n", format_name);
-		kfree(format_name);
-
-		*depth = 0;
-		*bpp = 0;
-		return;
-	}
-
-	*depth = info->depth;
-	*bpp = info->cpp[0] * 8;
-}
-EXPORT_SYMBOL(drm_fb_get_bpp_depth);
-
-/**
  * drm_format_num_planes - get the number of planes for format
  * @format: pixel format (DRM_FORMAT_*)
  *
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 1d45738f8f98..2544dfe7354c 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -70,8 +70,23 @@ EXPORT_SYMBOL(drm_helper_move_panel_connectors_to_head);
 void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
 				    const struct drm_mode_fb_cmd2 *mode_cmd)
 {
+	const struct drm_format_info *info;
 	int i;
 
+	info = drm_format_info(mode_cmd->pixel_format);
+	if (!info || !info->depth) {
+		char *format_name = drm_get_format_name(mode_cmd->pixel_format);
+
+		DRM_DEBUG_KMS("non-RGB pixel format %s\n", format_name);
+		kfree(format_name);
+
+		fb->depth = 0;
+		fb->bits_per_pixel = 0;
+	} else {
+		fb->depth = info->depth;
+		fb->bits_per_pixel = info->cpp[0] * 8;
+	}
+
 	fb->width = mode_cmd->width;
 	fb->height = mode_cmd->height;
 	for (i = 0; i < 4; i++) {
@@ -79,8 +94,6 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
 		fb->offsets[i] = mode_cmd->offsets[i];
 		fb->modifier[i] = mode_cmd->modifier[i];
 	}
-	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &fb->depth,
-				    &fb->bits_per_pixel);
 	fb->pixel_format = mode_cmd->pixel_format;
 	fb->flags = mode_cmd->flags;
 }
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index f73f97afd1e2..dc0aafab9ffd 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -48,7 +48,6 @@ struct drm_format_info {
 const struct drm_format_info *__drm_format_info(u32 format);
 const struct drm_format_info *drm_format_info(u32 format);
 uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
-void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
 int drm_format_num_planes(uint32_t format);
 int drm_format_plane_cpp(uint32_t format, int plane);
 int drm_format_horz_chroma_subsampling(uint32_t format);
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v5 13/13] drm: Don't export the drm_fb_get_bpp_depth() function

[Eric Engestrom]

On Tuesday, 2016-10-18 01:41:21 +0300, Laurent Pinchart wrote:
> The function is only used by the drm_helper_mode_fill_fb_struct() core
> function to fill the drm_framebuffer bpp and depth fields, used by
> drivers that haven't been converted to use pixel formats directly yet.
> It should not be used by new drivers, so inline it in its only caller.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>

> ---
>  drivers/gpu/drm/drm_fourcc.c         | 31 -------------------------------
>  drivers/gpu/drm/drm_modeset_helper.c | 17 +++++++++++++++--
>  include/drm/drm_fourcc.h             |  1 -
>  3 files changed, 15 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 523ed916a1c0..cbb8b77c363c 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -203,37 +203,6 @@ const struct drm_format_info *drm_format_info(u32 format)
>  EXPORT_SYMBOL(drm_format_info);
>  
>  /**
> - * drm_fb_get_bpp_depth - get the bpp/depth values for format
> - * @format: pixel format (DRM_FORMAT_*)
> - * @depth: storage for the depth value
> - * @bpp: storage for the bpp value
> - *
> - * This only supports RGB formats here for compat with code that doesn't use
> - * pixel formats directly yet.
> - */
> -void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> -			  int *bpp)
> -{
> -	const struct drm_format_info *info;
> -
> -	info = drm_format_info(format);
> -	if (!info || !info->depth) {
> -		char *format_name = drm_get_format_name(format);
> -
> -		DRM_DEBUG_KMS("unsupported pixel format %s\n", format_name);
> -		kfree(format_name);
> -
> -		*depth = 0;
> -		*bpp = 0;
> -		return;
> -	}
> -
> -	*depth = info->depth;
> -	*bpp = info->cpp[0] * 8;
> -}
> -EXPORT_SYMBOL(drm_fb_get_bpp_depth);
> -
> -/**
>   * drm_format_num_planes - get the number of planes for format
>   * @format: pixel format (DRM_FORMAT_*)
>   *
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 1d45738f8f98..2544dfe7354c 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -70,8 +70,23 @@ EXPORT_SYMBOL(drm_helper_move_panel_connectors_to_head);
>  void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
>  				    const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> +	const struct drm_format_info *info;
>  	int i;
>  
> +	info = drm_format_info(mode_cmd->pixel_format);
> +	if (!info || !info->depth) {
> +		char *format_name = drm_get_format_name(mode_cmd->pixel_format);
> +
> +		DRM_DEBUG_KMS("non-RGB pixel format %s\n", format_name);
> +		kfree(format_name);
> +
> +		fb->depth = 0;
> +		fb->bits_per_pixel = 0;
> +	} else {
> +		fb->depth = info->depth;
> +		fb->bits_per_pixel = info->cpp[0] * 8;
> +	}
> +
>  	fb->width = mode_cmd->width;
>  	fb->height = mode_cmd->height;
>  	for (i = 0; i < 4; i++) {
> @@ -79,8 +94,6 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
>  		fb->offsets[i] = mode_cmd->offsets[i];
>  		fb->modifier[i] = mode_cmd->modifier[i];
>  	}
> -	drm_fb_get_bpp_depth(mode_cmd->pixel_format, &fb->depth,
> -				    &fb->bits_per_pixel);
>  	fb->pixel_format = mode_cmd->pixel_format;
>  	fb->flags = mode_cmd->flags;
>  }
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index f73f97afd1e2..dc0aafab9ffd 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -48,7 +48,6 @@ struct drm_format_info {
>  const struct drm_format_info *__drm_format_info(u32 format);
>  const struct drm_format_info *drm_format_info(u32 format);
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> -void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
>  int drm_format_num_planes(uint32_t format);
>  int drm_format_plane_cpp(uint32_t format, int plane);
>  int drm_format_horz_chroma_subsampling(uint32_t format);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 00/13] Centralize format information

[Archit Taneja]

Hi,

On 10/18/2016 04:11 AM, Laurent Pinchart wrote:
> Hello,
>
> Various pieces of information about DRM formats (number of planes, color
> depth, chroma subsampling, ...) are scattered across different helper
> functions in the DRM core. Callers of those functions often need to access
> more than a single parameter of the format, leading to inefficiencies due to
> multiple lookups.
>
> This patch series addresses this issue by centralizing all format information
> in a single data structure (01/13). It reimplements the existing format helper
> functions based on that structure (02/13) and converts the DRM core code to
> use the new structure (03/13). The DRM core now WARNs when a driver tries to
> query information about an unsupported format (04/13).
>
> The second part of the patch series removes the drm_fb_get_bpp_depth() legacy
> function that shouldn't be used directly by drivers. It modifies all its users
> to use the appropriate API instead (05/13 to 12/13) and finally merges the
> function into its only caller in the DRM core (13/13).
>
> The new API is also useful for drivers as shown by the "[PATCH v2 00/20] OMAP
> DRM fixes and improvements" patch series previously posted.
>
> All patches have been acked, the series is ready to be merged for v4.10.

Queued all patches to drm-misc.

Thanks,
Archit

>
> Changes since v4:
>
> - Rebased on top of latest drm/master branch
> - Collected acks
> - Fixed depth value of DRM_FORMAT_[AXRGB]{4}4444 formats to match current code
> - Added support for DRM_FORMAT_BGR565, DRM_FORMAT_XBGR8888 and
>   DRM_FORMAT_BGR888 to tilcdc
> - Documented the depth field as legacy
>
> Changes since v3:
>
> - Rebased on top of latest drm/master branch
> - Collected acks
> - Dropped "drm: Move format-related helpers to drm_fourcc.c" and
>   "drm/msm: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()" that
>   have been merged already
> - Added new "drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with
>   drm_format_plane_cpp()" patch
> - Coding style fixes and variable renames
>
> Changes since v2:
>
> - Remove bpp field from drm_format_info structure
> - Replace all users of drm_fb_get_bpp_depth() with the appropriate API
> - Merge drm_fb_get_bpp_depth() into its only caller
>
> Changes since v1:
>
> - Move format-related helpers to drm_fourcc.c
> - Use named initializers for the formats array
> - WARN when calling drm_format_info() for an unsupported format
> - Don't drop the drm_format_plane_width() and drm_format_plane_height()
>   helpers
>
> Laurent Pinchart (13):
>   drm: Centralize format information
>   drm: Implement the drm_format_*() helpers as drm_format_info()
>     wrappers
>   drm: Use drm_format_info() in DRM core code
>   drm: WARN when calling drm_format_info() for an unsupported format
>   drm: hdlcd: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()
>   drm: tilcdc: Replace drm_fb_get_bpp_depth() with
>     drm_format_plane_cpp()
>   drm: cirrus: Replace drm_fb_get_bpp_depth() with
>     drm_format_plane_cpp()
>   drm: gma500: Replace drm_fb_get_bpp_depth() with drm_format_info()
>   drm: amdgpu: Replace drm_fb_get_bpp_depth() with
>     drm_format_plane_cpp()
>   drm: radeon: Replace drm_fb_get_bpp_depth() with
>     drm_format_plane_cpp()
>   drm: vmwgfx: Replace drm_fb_get_bpp_depth() with drm_format_info()
>   drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with
>     drm_format_plane_cpp()
>   drm: Don't export the drm_fb_get_bpp_depth() function
>
>  Documentation/gpu/drm-kms.rst           |   3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  |  14 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |   3 +-
>  drivers/gpu/drm/arm/hdlcd_crtc.c        |   5 +-
>  drivers/gpu/drm/arm/malidp_hw.c         |   7 +-
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c   |   6 +-
>  drivers/gpu/drm/cirrus/cirrus_main.c    |   4 +-
>  drivers/gpu/drm/drm_fb_cma_helper.c     |  23 +--
>  drivers/gpu/drm/drm_fourcc.c            | 281 ++++++++++++++------------------
>  drivers/gpu/drm/drm_framebuffer.c       | 102 ++----------
>  drivers/gpu/drm/drm_modeset_helper.c    |  17 +-
>  drivers/gpu/drm/gma500/framebuffer.c    |  20 +--
>  drivers/gpu/drm/radeon/radeon_fb.c      |  20 +--
>  drivers/gpu/drm/radeon/radeon_gem.c     |   3 +-
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c    |  18 +-
>  drivers/gpu/drm/tilcdc/tilcdc_plane.c   |   7 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     |  12 +-
>  include/drm/drm_fourcc.h                |  23 ++-
>  18 files changed, 244 insertions(+), 324 deletions(-)
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 00/13] Centralize format information

[Ville Syrjälä]

On Tue, Oct 18, 2016 at 01:41:08AM +0300, Laurent Pinchart wrote:
> Hello,
> 
> Various pieces of information about DRM formats (number of planes, color
> depth, chroma subsampling, ...) are scattered across different helper
> functions in the DRM core. Callers of those functions often need to access
> more than a single parameter of the format, leading to inefficiencies due to
> multiple lookups.
> 
> This patch series addresses this issue by centralizing all format information
> in a single data structure (01/13). It reimplements the existing format helper
> functions based on that structure (02/13) and converts the DRM core code to
> use the new structure (03/13). The DRM core now WARNs when a driver tries to
> query information about an unsupported format (04/13).
> 
> The second part of the patch series removes the drm_fb_get_bpp_depth() legacy
> function that shouldn't be used directly by drivers. It modifies all its users
> to use the appropriate API instead (05/13 to 12/13) and finally merges the
> function into its only caller in the DRM core (13/13).
> 
> The new API is also useful for drivers as shown by the "[PATCH v2 00/20] OMAP
> DRM fixes and improvements" patch series previously posted.
> 
> All patches have been acked, the series is ready to be merged for v4.10.

BTW and idea I had recently is that we could store the pointer to the
format info into drm_framebuffer, and then a lot of the
drm_format_...() calls could just vanish, or if the information we
need isn't directly stored in the info structure we'd just need a small
function that takes the entire drm_framebuffer or just the format info
struct and computes what we need. Would avoid having to sprinkle
drm_format_info() calls into the drivers.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 00/13] Centralize format information

[Laurent Pinchart]

Hi Ville,

On Tuesday 18 Oct 2016 14:43:18 Ville Syrjälä wrote:
> On Tue, Oct 18, 2016 at 01:41:08AM +0300, Laurent Pinchart wrote:
> > Hello,
> > 
> > Various pieces of information about DRM formats (number of planes, color
> > depth, chroma subsampling, ...) are scattered across different helper
> > functions in the DRM core. Callers of those functions often need to access
> > more than a single parameter of the format, leading to inefficiencies due
> > to multiple lookups.
> > 
> > This patch series addresses this issue by centralizing all format
> > information in a single data structure (01/13). It reimplements the
> > existing format helper functions based on that structure (02/13) and
> > converts the DRM core code to use the new structure (03/13). The DRM core
> > now WARNs when a driver tries to query information about an unsupported
> > format (04/13).
> > 
> > The second part of the patch series removes the drm_fb_get_bpp_depth()
> > legacy function that shouldn't be used directly by drivers. It modifies
> > all its users to use the appropriate API instead (05/13 to 12/13) and
> > finally merges the function into its only caller in the DRM core (13/13).
> > 
> > The new API is also useful for drivers as shown by the "[PATCH v2 00/20]
> > OMAP DRM fixes and improvements" patch series previously posted.
> > 
> > All patches have been acked, the series is ready to be merged for v4.10.
> 
> BTW and idea I had recently is that we could store the pointer to the
> format info into drm_framebuffer, and then a lot of the
> drm_format_...() calls could just vanish, or if the information we
> need isn't directly stored in the info structure we'd just need a small
> function that takes the entire drm_framebuffer or just the format info
> struct and computes what we need. Would avoid having to sprinkle
> drm_format_info() calls into the drivers.

That's a good idea. I didn't want to add that to my patch series in order to 
avoid merge delays, but patches are certainly welcome :-)

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v5 00/13] Centralize format information

[Ville Syrjälä]

On Tue, Oct 18, 2016 at 03:33:29PM +0300, Laurent Pinchart wrote:
> Hi Ville,
> 
> On Tuesday 18 Oct 2016 14:43:18 Ville Syrjälä wrote:
> > On Tue, Oct 18, 2016 at 01:41:08AM +0300, Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > Various pieces of information about DRM formats (number of planes, color
> > > depth, chroma subsampling, ...) are scattered across different helper
> > > functions in the DRM core. Callers of those functions often need to access
> > > more than a single parameter of the format, leading to inefficiencies due
> > > to multiple lookups.
> > > 
> > > This patch series addresses this issue by centralizing all format
> > > information in a single data structure (01/13). It reimplements the
> > > existing format helper functions based on that structure (02/13) and
> > > converts the DRM core code to use the new structure (03/13). The DRM core
> > > now WARNs when a driver tries to query information about an unsupported
> > > format (04/13).
> > > 
> > > The second part of the patch series removes the drm_fb_get_bpp_depth()
> > > legacy function that shouldn't be used directly by drivers. It modifies
> > > all its users to use the appropriate API instead (05/13 to 12/13) and
> > > finally merges the function into its only caller in the DRM core (13/13).
> > > 
> > > The new API is also useful for drivers as shown by the "[PATCH v2 00/20]
> > > OMAP DRM fixes and improvements" patch series previously posted.
> > > 
> > > All patches have been acked, the series is ready to be merged for v4.10.
> > 
> > BTW and idea I had recently is that we could store the pointer to the
> > format info into drm_framebuffer, and then a lot of the
> > drm_format_...() calls could just vanish, or if the information we
> > need isn't directly stored in the info structure we'd just need a small
> > function that takes the entire drm_framebuffer or just the format info
> > struct and computes what we need. Would avoid having to sprinkle
> > drm_format_info() calls into the drivers.
> 
> That's a good idea. I didn't want to add that to my patch series in order to 
> avoid merge delays, but patches are certainly welcome :-)

Sure. I'll preserve the idea for a rainy day, unless someone else beats
me to it that is.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 00/13] Centralize format information

[Laurent Pinchart]

On Tuesday 18 Oct 2016 15:45:14 Ville Syrjälä wrote:
> On Tue, Oct 18, 2016 at 03:33:29PM +0300, Laurent Pinchart wrote:
> > On Tuesday 18 Oct 2016 14:43:18 Ville Syrjälä wrote:
> >> On Tue, Oct 18, 2016 at 01:41:08AM +0300, Laurent Pinchart wrote:
> >>> Hello,
> >>> 
> >>> Various pieces of information about DRM formats (number of planes,
> >>> color depth, chroma subsampling, ...) are scattered across different
> >>> helper functions in the DRM core. Callers of those functions often
> >>> need to access more than a single parameter of the format, leading to
> >>> inefficiencies due to multiple lookups.
> >>> 
> >>> This patch series addresses this issue by centralizing all format
> >>> information in a single data structure (01/13). It reimplements the
> >>> existing format helper functions based on that structure (02/13) and
> >>> converts the DRM core code to use the new structure (03/13). The DRM
> >>> core now WARNs when a driver tries to query information about an
> >>> unsupported format (04/13).
> >>> 
> >>> The second part of the patch series removes the drm_fb_get_bpp_depth()
> >>> legacy function that shouldn't be used directly by drivers. It
> >>> modifies all its users to use the appropriate API instead (05/13 to
> >>> 12/13) and finally merges the function into its only caller in the DRM
> >>> core (13/13).
> >>> 
> >>> The new API is also useful for drivers as shown by the "[PATCH v2
> >>> 00/20] OMAP DRM fixes and improvements" patch series previously
> >>> posted.
> >>> 
> >>> All patches have been acked, the series is ready to be merged for
> >>> v4.10.
> >> 
> >> BTW and idea I had recently is that we could store the pointer to the
> >> format info into drm_framebuffer, and then a lot of the
> >> drm_format_...() calls could just vanish, or if the information we
> >> need isn't directly stored in the info structure we'd just need a small
> >> function that takes the entire drm_framebuffer or just the format info
> >> struct and computes what we need. Would avoid having to sprinkle
> >> drm_format_info() calls into the drivers.
> > 
> > That's a good idea. I didn't want to add that to my patch series in order
> > to avoid merge delays, but patches are certainly welcome :-)
> 
> Sure. I'll preserve the idea for a rainy day, unless someone else beats
> me to it that is.

I suppose I should expect lots of similar patch series during the Finnish 
winter then ;-)

-- 
Regards,

Laurent Pinchart

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