[PATCH 1/2] Add DRM_IOCTL_MODE_GETPLANE2 ioctl wrapper

[PATCH 1/2] Add DRM_IOCTL_MODE_GETPLANE2 ioctl wrapper

[Kristian H. Kristensen]

From: "Kristian H. Kristensen" <hoegsberg@chromium.org>

This adds support for the new DRM_IOCTL_MODE_GETPLANE2 ioctl. For older
kernels drmModeGetPlane2() falls back to DRM_IOCTL_MODE_GETPLANE and
return the new, bigger drmModePlaneRec, reporting 0 modifiers.

BUG=chrome-os-partner:56407
TEST=modetest with next commit reports modifiers

Change-Id: I9cf9979c0b72933bad661fd03b9beebb36120dfd
---
 include/drm/drm.h      |  1 +
 include/drm/drm_mode.h | 27 +++++++++++++++++++++++++++
 xf86drmMode.c          | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
 xf86drmMode.h          |  4 ++++
 4 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index f6fd5c2..09d4262 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -799,6 +799,7 @@ extern "C" {
 #define DRM_IOCTL_MODE_DESTROY_DUMB    DRM_IOWR(0xB4, struct drm_mode_destroy_dumb)
 #define DRM_IOCTL_MODE_GETPLANERESOURCES DRM_IOWR(0xB5, struct drm_mode_get_plane_res)
 #define DRM_IOCTL_MODE_GETPLANE	DRM_IOWR(0xB6, struct drm_mode_get_plane)
+#define DRM_IOCTL_MODE_GETPLANE2	DRM_IOWR(0xB6, struct drm_mode_get_plane2)
 #define DRM_IOCTL_MODE_SETPLANE	DRM_IOWR(0xB7, struct drm_mode_set_plane)
 #define DRM_IOCTL_MODE_ADDFB2		DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
 #define DRM_IOCTL_MODE_OBJ_GETPROPERTIES	DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index df0e350..ce773fa 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -193,6 +193,33 @@ struct drm_mode_get_plane {
 	__u64 format_type_ptr;
 };
 
+struct drm_format_modifier {
+	/* Bitmask of formats in get_plane format list this info
+	 * applies to. */
+	__u64 formats;
+
+	/* This modifier can be used with the format for this plane. */
+	__u64 modifier;
+};
+
+struct drm_mode_get_plane2 {
+	__u32 plane_id;
+
+	__u32 crtc_id;
+	__u32 fb_id;
+
+	__u32 possible_crtcs;
+	__u32 gamma_size;
+
+	__u32 count_format_types;
+	__u64 format_type_ptr;
+
+	/* New in v2 */
+	__u32 count_format_modifiers;
+	__u32 flags;
+	__u64 format_modifier_ptr;
+};
+
 struct drm_mode_get_plane_res {
 	__u64 plane_id_ptr;
 	__u32 count_planes;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index fb22f68..298d502 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -990,15 +990,15 @@ int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
 	return DRM_IOCTL(fd, DRM_IOCTL_MODE_SETPLANE, &s);
 }
 
-drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id)
+static drmModePlanePtr get_plane(unsigned long cmd, int fd, uint32_t plane_id)
 {
-	struct drm_mode_get_plane ovr, counts;
+	struct drm_mode_get_plane2 ovr, counts;
 	drmModePlanePtr r = 0;
 
 retry:
 	memclear(ovr);
 	ovr.plane_id = plane_id;
-	if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANE, &ovr))
+	if (drmIoctl(fd, cmd, &ovr))
 		return 0;
 
 	counts = ovr;
@@ -1010,11 +1010,21 @@ retry:
 			goto err_allocs;
 	}
 
-	if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANE, &ovr))
+	if (ovr.count_format_modifiers) {
+		ovr.format_modifier_ptr =
+			VOID2U64(drmMalloc(ovr.count_format_modifiers *
+					   sizeof(struct drm_format_modifier)));
+		if (!ovr.format_modifier_ptr)
+			goto err_allocs;
+	}
+
+	if (drmIoctl(fd, cmd, &ovr))
 		goto err_allocs;
 
-	if (counts.count_format_types < ovr.count_format_types) {
+	if (counts.count_format_types < ovr.count_format_types ||
+	    counts.count_format_modifiers < ovr.count_format_modifiers) {
 		drmFree(U642VOID(ovr.format_type_ptr));
+		drmFree(U642VOID(ovr.format_modifier_ptr));
 		goto retry;
 	}
 
@@ -1022,6 +1032,7 @@ retry:
 		goto err_allocs;
 
 	r->count_formats = ovr.count_format_types;
+	r->count_format_modifiers = ovr.count_format_modifiers;
 	r->plane_id = ovr.plane_id;
 	r->crtc_id = ovr.crtc_id;
 	r->fb_id = ovr.fb_id;
@@ -1033,20 +1044,48 @@ retry:
 		drmFree(r->formats);
 		drmFree(r);
 		r = 0;
+		goto err_allocs;
+	}
+
+	r->format_modifiers =
+		drmAllocCpy(U642VOID(ovr.format_modifier_ptr),
+			    ovr.count_format_modifiers,
+			    sizeof(struct drm_format_modifier));
+	if (ovr.count_format_modifiers && !r->format_modifiers) {
+		drmFree(r->formats);
+		drmFree(r);
+		r = 0;
 	}
 
 err_allocs:
 	drmFree(U642VOID(ovr.format_type_ptr));
+	drmFree(U642VOID(ovr.format_modifier_ptr));
 
 	return r;
 }
 
+drmModePlanePtr drmModeGetPlane2(int fd, uint32_t plane_id)
+{
+	drmModePlanePtr r = get_plane(DRM_IOCTL_MODE_GETPLANE2, fd, plane_id);
+
+	if (r || errno != EINVAL)
+		return r;
+
+	return get_plane(DRM_IOCTL_MODE_GETPLANE, fd, plane_id);
+}
+
+drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id)
+{
+	return get_plane(DRM_IOCTL_MODE_GETPLANE, fd, plane_id);
+}
+
 void drmModeFreePlane(drmModePlanePtr ptr)
 {
 	if (!ptr)
 		return;
 
 	drmFree(ptr->formats);
+	drmFree(ptr->format_modifiers);
 	drmFree(ptr);
 }
 
diff --git a/xf86drmMode.h b/xf86drmMode.h
index b684967..044f38e 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -328,6 +328,9 @@ typedef struct _drmModePlane {
 
 	uint32_t possible_crtcs;
 	uint32_t gamma_size;
+
+	uint32_t count_format_modifiers;
+	struct drm_format_modifier *format_modifiers;
 } drmModePlane, *drmModePlanePtr;
 
 typedef struct _drmModePlaneRes {
@@ -479,6 +482,7 @@ extern int drmModePageFlipTarget(int fd, uint32_t crtc_id, uint32_t fb_id,
 
 extern drmModePlaneResPtr drmModeGetPlaneResources(int fd);
 extern drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id);
+extern drmModePlanePtr drmModeGetPlane2(int fd, uint32_t plane_id);
 extern int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
 			   uint32_t fb_id, uint32_t flags,
 			   int32_t crtc_x, int32_t crtc_y,
-- 
2.9.3

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

[PATCH 2/2] modetest: Teach modetest about format info

[Kristian H. Kristensen]

From: "Kristian H. Kristensen" <hoegsberg@chromium.org>

BUG=chrome-os-partner:56407
TEST=modetest on a KMS driver that exposes modifiers should print those

Change-Id: I91b2a408b1c8f112d7ba5d0998119b3c800b199c
---
 tests/modetest/modetest.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index dedd286..091bcba 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -417,9 +417,30 @@ static void dump_framebuffers(struct device *dev)
 	printf("\n");
 }
 
+static const char *
+mod_to_string(uint64_t mod, char *buf, int len)
+{
+	switch (mod) {
+	case DRM_FORMAT_MOD_NONE:
+		return "LINEAR";
+	case I915_FORMAT_MOD_X_TILED:
+		return "X_TILED";
+	case I915_FORMAT_MOD_Y_TILED:
+		return "Y_TILED";
+	case I915_FORMAT_MOD_Yf_TILED:
+		return "Yf_TILED";
+	case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
+		return "SAMSUNG_64_32_TILE";
+	default:
+		snprintf(buf, len, "%016x", mod);
+		return buf;
+	}
+}
+
 static void dump_planes(struct device *dev)
 {
-	unsigned int i, j;
+	unsigned int i, j, k;
+	char buf[17];
 
 	printf("Planes:\n");
 	printf("id\tcrtc\tfb\tCRTC x,y\tx,y\tgamma size\tpossible crtcs\n");
@@ -442,8 +463,19 @@ static void dump_planes(struct device *dev)
 			continue;
 
 		printf("  formats:");
-		for (j = 0; j < ovr->count_formats; j++)
-			printf(" %4.4s", (char *)&ovr->formats[j]);
+		for (j = 0; j < ovr->count_formats; j++) {
+			if (ovr->count_format_modifiers == 0) {
+				printf(" %4.4s", (char *)&ovr->formats[j]);
+				continue;
+			}
+			struct drm_format_modifier *fm;
+			for (k = 0; k < ovr->count_format_modifiers; k++) {
+				fm = &ovr->format_modifiers[k];
+				if (fm->formats & (1 << j))
+					printf(" %4.4s:%s", (char *)&ovr->formats[j],
+					       mod_to_string(fm->modifier, buf, sizeof(buf)));
+			}
+		}
 		printf("\n");
 
 		if (plane->props) {
@@ -609,7 +641,7 @@ static struct resources *get_resources(struct device *dev)
 	if (!res->planes)
 		goto error;
 
-	get_resource(res, plane_res, plane, Plane);
+	get_resource(res, plane_res, plane, Plane2);
 	get_properties(res, plane_res, plane, PLANE);
 
 	return res;
-- 
2.9.3

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

Re: [PATCH 1/2] Add DRM_IOCTL_MODE_GETPLANE2 ioctl wrapper

[Ben Widawsky]

On 16-12-20 16:13:32, Kristian H. Kristensen wrote:
>From: "Kristian H. Kristensen" <hoegsberg@chromium.org>
>
>This adds support for the new DRM_IOCTL_MODE_GETPLANE2 ioctl. For older
>kernels drmModeGetPlane2() falls back to DRM_IOCTL_MODE_GETPLANE and
>return the new, bigger drmModePlaneRec, reporting 0 modifiers.
>
>BUG=chrome-os-partner:56407
>TEST=modetest with next commit reports modifiers
>
>Change-Id: I9cf9979c0b72933bad661fd03b9beebb36120dfd
>---
> include/drm/drm.h      |  1 +
> include/drm/drm_mode.h | 27 +++++++++++++++++++++++++++
> xf86drmMode.c          | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
> xf86drmMode.h          |  4 ++++
> 4 files changed, 76 insertions(+), 5 deletions(-)
>
>diff --git a/include/drm/drm.h b/include/drm/drm.h
>index f6fd5c2..09d4262 100644
>--- a/include/drm/drm.h
>+++ b/include/drm/drm.h
>@@ -799,6 +799,7 @@ extern "C" {
> #define DRM_IOCTL_MODE_DESTROY_DUMB    DRM_IOWR(0xB4, struct drm_mode_destroy_dumb)
> #define DRM_IOCTL_MODE_GETPLANERESOURCES DRM_IOWR(0xB5, struct drm_mode_get_plane_res)
> #define DRM_IOCTL_MODE_GETPLANE	DRM_IOWR(0xB6, struct drm_mode_get_plane)
>+#define DRM_IOCTL_MODE_GETPLANE2	DRM_IOWR(0xB6, struct drm_mode_get_plane2)
> #define DRM_IOCTL_MODE_SETPLANE	DRM_IOWR(0xB7, struct drm_mode_set_plane)
> #define DRM_IOCTL_MODE_ADDFB2		DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
> #define DRM_IOCTL_MODE_OBJ_GETPROPERTIES	DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
>diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
>index df0e350..ce773fa 100644
>--- a/include/drm/drm_mode.h
>+++ b/include/drm/drm_mode.h
>@@ -193,6 +193,33 @@ struct drm_mode_get_plane {
> 	__u64 format_type_ptr;
> };
>
>+struct drm_format_modifier {
>+	/* Bitmask of formats in get_plane format list this info
>+	 * applies to. */
>+	__u64 formats;
>+
>+	/* This modifier can be used with the format for this plane. */
>+	__u64 modifier;
>+};
>+
>+struct drm_mode_get_plane2 {
>+	__u32 plane_id;
>+
>+	__u32 crtc_id;
>+	__u32 fb_id;
>+
>+	__u32 possible_crtcs;
>+	__u32 gamma_size;
>+
>+	__u32 count_format_types;
>+	__u64 format_type_ptr;
>+
>+	/* New in v2 */
>+	__u32 count_format_modifiers;
>+	__u32 flags;
>+	__u64 format_modifier_ptr;
>+};
>+
> struct drm_mode_get_plane_res {
> 	__u64 plane_id_ptr;
> 	__u32 count_planes;
>diff --git a/xf86drmMode.c b/xf86drmMode.c
>index fb22f68..298d502 100644
>--- a/xf86drmMode.c
>+++ b/xf86drmMode.c
>@@ -990,15 +990,15 @@ int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
> 	return DRM_IOCTL(fd, DRM_IOCTL_MODE_SETPLANE, &s);
> }
>
>-drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id)
>+static drmModePlanePtr get_plane(unsigned long cmd, int fd, uint32_t plane_id)
> {
>-	struct drm_mode_get_plane ovr, counts;
>+	struct drm_mode_get_plane2 ovr, counts;
> 	drmModePlanePtr r = 0;
>
> retry:
> 	memclear(ovr);
> 	ovr.plane_id = plane_id;
>-	if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANE, &ovr))
>+	if (drmIoctl(fd, cmd, &ovr))
> 		return 0;
>
> 	counts = ovr;
>@@ -1010,11 +1010,21 @@ retry:
> 			goto err_allocs;
> 	}
>
>-	if (drmIoctl(fd, DRM_IOCTL_MODE_GETPLANE, &ovr))
>+	if (ovr.count_format_modifiers) {
>+		ovr.format_modifier_ptr =
>+			VOID2U64(drmMalloc(ovr.count_format_modifiers *
>+					   sizeof(struct drm_format_modifier)));
>+		if (!ovr.format_modifier_ptr)
>+			goto err_allocs;
>+	}
>+
>+	if (drmIoctl(fd, cmd, &ovr))
> 		goto err_allocs;
>
>-	if (counts.count_format_types < ovr.count_format_types) {
>+	if (counts.count_format_types < ovr.count_format_types ||
>+	    counts.count_format_modifiers < ovr.count_format_modifiers) {
> 		drmFree(U642VOID(ovr.format_type_ptr));
>+		drmFree(U642VOID(ovr.format_modifier_ptr));
> 		goto retry;
> 	}
>
>@@ -1022,6 +1032,7 @@ retry:
> 		goto err_allocs;
>
> 	r->count_formats = ovr.count_format_types;
>+	r->count_format_modifiers = ovr.count_format_modifiers;
> 	r->plane_id = ovr.plane_id;
> 	r->crtc_id = ovr.crtc_id;
> 	r->fb_id = ovr.fb_id;
>@@ -1033,20 +1044,48 @@ retry:
> 		drmFree(r->formats);
> 		drmFree(r);
> 		r = 0;
>+		goto err_allocs;
>+	}
>+
>+	r->format_modifiers =
>+		drmAllocCpy(U642VOID(ovr.format_modifier_ptr),
>+			    ovr.count_format_modifiers,
>+			    sizeof(struct drm_format_modifier));
>+	if (ovr.count_format_modifiers && !r->format_modifiers) {
>+		drmFree(r->formats);
>+		drmFree(r);
>+		r = 0;
> 	}
>
> err_allocs:
> 	drmFree(U642VOID(ovr.format_type_ptr));
>+	drmFree(U642VOID(ovr.format_modifier_ptr));
>
> 	return r;
> }
>
>+drmModePlanePtr drmModeGetPlane2(int fd, uint32_t plane_id)
>+{
>+	drmModePlanePtr r = get_plane(DRM_IOCTL_MODE_GETPLANE2, fd, plane_id);
>+
>+	if (r || errno != EINVAL)
>+		return r;
>+
>+	return get_plane(DRM_IOCTL_MODE_GETPLANE, fd, plane_id);
>+}
>+

IMO, it should be up to the client to fallback to drmModeGetPlane if
DRM_IOCTL_MODE_GETPLANE2 doesn't exist. However, as long as the modifiers are
returned as 0, it should work fine.

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

>+drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id)
>+{
>+	return get_plane(DRM_IOCTL_MODE_GETPLANE, fd, plane_id);
>+}
>+
> void drmModeFreePlane(drmModePlanePtr ptr)
> {
> 	if (!ptr)
> 		return;
>
> 	drmFree(ptr->formats);
>+	drmFree(ptr->format_modifiers);
> 	drmFree(ptr);
> }
>
>diff --git a/xf86drmMode.h b/xf86drmMode.h
>index b684967..044f38e 100644
>--- a/xf86drmMode.h
>+++ b/xf86drmMode.h
>@@ -328,6 +328,9 @@ typedef struct _drmModePlane {
>
> 	uint32_t possible_crtcs;
> 	uint32_t gamma_size;
>+
>+	uint32_t count_format_modifiers;
>+	struct drm_format_modifier *format_modifiers;
> } drmModePlane, *drmModePlanePtr;
>
> typedef struct _drmModePlaneRes {
>@@ -479,6 +482,7 @@ extern int drmModePageFlipTarget(int fd, uint32_t crtc_id, uint32_t fb_id,
>
> extern drmModePlaneResPtr drmModeGetPlaneResources(int fd);
> extern drmModePlanePtr drmModeGetPlane(int fd, uint32_t plane_id);
>+extern drmModePlanePtr drmModeGetPlane2(int fd, uint32_t plane_id);
> extern int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
> 			   uint32_t fb_id, uint32_t flags,
> 			   int32_t crtc_x, int32_t crtc_y,
>-- 
>2.9.3
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] modetest: Teach modetest about format info

[Ben Widawsky]

On 16-12-20 16:13:33, Kristian H. Kristensen wrote:
>From: "Kristian H. Kristensen" <hoegsberg@chromium.org>
>
>BUG=chrome-os-partner:56407
>TEST=modetest on a KMS driver that exposes modifiers should print those
>
>Change-Id: I91b2a408b1c8f112d7ba5d0998119b3c800b199c
>---
> tests/modetest/modetest.c | 40 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
>diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
>index dedd286..091bcba 100644
>--- a/tests/modetest/modetest.c
>+++ b/tests/modetest/modetest.c
>@@ -417,9 +417,30 @@ static void dump_framebuffers(struct device *dev)
> 	printf("\n");
> }
>
>+static const char *
>+mod_to_string(uint64_t mod, char *buf, int len)
>+{
>+	switch (mod) {
>+	case DRM_FORMAT_MOD_NONE:
>+		return "LINEAR";

Just being pedantic here but a lack of modifier doesn't only mean that it's
linear, it means that there is no compression, and whatever else the future
holds. I'd just maybe return something like "unmodified".

>+	case I915_FORMAT_MOD_X_TILED:
>+		return "X_TILED";
>+	case I915_FORMAT_MOD_Y_TILED:
>+		return "Y_TILED";
>+	case I915_FORMAT_MOD_Yf_TILED:
>+		return "Yf_TILED";
>+	case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
>+		return "SAMSUNG_64_32_TILE";
>+	default:
>+		snprintf(buf, len, "%016x", mod);
>+		return buf;
>+	}
>+}
>+
> static void dump_planes(struct device *dev)
> {
>-	unsigned int i, j;
>+	unsigned int i, j, k;
>+	char buf[17];
>
> 	printf("Planes:\n");
> 	printf("id\tcrtc\tfb\tCRTC x,y\tx,y\tgamma size\tpossible crtcs\n");
>@@ -442,8 +463,19 @@ static void dump_planes(struct device *dev)
> 			continue;
>
> 		printf("  formats:");
>-		for (j = 0; j < ovr->count_formats; j++)
>-			printf(" %4.4s", (char *)&ovr->formats[j]);
>+		for (j = 0; j < ovr->count_formats; j++) {
>+			if (ovr->count_format_modifiers == 0) {
>+				printf(" %4.4s", (char *)&ovr->formats[j]);
>+				continue;
>+			}
>+			struct drm_format_modifier *fm;
>+			for (k = 0; k < ovr->count_format_modifiers; k++) {
>+				fm = &ovr->format_modifiers[k];
>+				if (fm->formats & (1 << j))
>+					printf(" %4.4s:%s", (char *)&ovr->formats[j],
>+					       mod_to_string(fm->modifier, buf, sizeof(buf)));
>+			}
>+		}

Wasn't the plan to have only 1 modifier per plane? Did that change? The GBM
interface only allows 1 modifier per plane.

> 		printf("\n");
>
> 		if (plane->props) {
>@@ -609,7 +641,7 @@ static struct resources *get_resources(struct device *dev)
> 	if (!res->planes)
> 		goto error;
>
>-	get_resource(res, plane_res, plane, Plane);
>+	get_resource(res, plane_res, plane, Plane2);
> 	get_properties(res, plane_res, plane, PLANE);
>
> 	return res;
>-- 
>2.9.3
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel