dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Enable R8A7790/1 DU VSPD compositor
@ 2017-01-25 21:33 Sergei Shtylyov
  2017-01-25 21:35 ` [PATCH v3 1/3] drm: Add live source object Sergei Shtylyov
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-25 21:33 UTC (permalink / raw)
  To: laurent.pinchart, airlied, dri-devel, daniel.vetter, jani.nikula,
	seanpaul
  Cc: linux-renesas-soc, linux-kernel

Hello.

   Here's the set of 3 patches against the 'drm-next' branch of David Airlie's
'linux.git' repo. This is a port of Laurent's DRM/DU live source patches to the
recent kernel, see [1] for the version 2 of the patchset (including a Laurent's
big blurb :-)). For the patch #3 to work one absolutely needs the VSP1 patch
[2] to be applied, else the kernel will just hang...

[1] https://lists.freedesktop.org/archives/dri-devel/2015-March/079319.html.
[2] https://patchwork.kernel.org/patch/9473265/

[1/3] drm: Add live source object
[2/3] drm: Connect live source to framebuffers
[3/3] drm: rcar-du: Add VSP1 live source support

MBR, Sergei

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

* [PATCH v3 1/3] drm: Add live source object
  2017-01-25 21:33 [PATCH v3 0/3] Enable R8A7790/1 DU VSPD compositor Sergei Shtylyov
@ 2017-01-25 21:35 ` Sergei Shtylyov
  2017-01-31 15:41   ` Brian Starkey
  2017-01-25 21:36 ` [PATCH v3 2/3] drm: Connect live source to framebuffers Sergei Shtylyov
  2017-01-25 21:38 ` [PATCH v3 3/3] drm: rcar-du: Add VSP1 live source support Sergei Shtylyov
  2 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-25 21:35 UTC (permalink / raw)
  To: laurent.pinchart, airlied, dri-devel, daniel.vetter, jani.nikula,
	seanpaul
  Cc: linux-renesas-soc, linux-kernel

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Live sources represent a hardware connection between a video stream
source and a CRTC, going through a plane. The kernel API lets driver
register live sources, and the userspace API lets applications enumerate
them.

[Sergei: ported to the modern kernel.]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes in version 3:
- moved drm_live_source_{init|cleanup}() from drm_crtc.c to drm_plane.c and
  their prototypes from <drm/drm_crtc.h> to <drm/drm_plane.h> (along with
  *struct* drm_live_source[_funcs]);
- moved drm_mode_getsource[_res]() prototypes from <drm/drm_crtc.h> to
  drm_crtc_internal.h;
- updated the ioctl() numbers;
- generally followed the upstream source code moves;
- added my signoff.

 drivers/gpu/drm/drm_crtc.c          |   93 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc_internal.h |    4 +
 drivers/gpu/drm/drm_ioctl.c         |    2 
 drivers/gpu/drm/drm_mode_config.c   |    7 ++
 drivers/gpu/drm/drm_plane.c         |   62 ++++++++++++++++++++++++
 include/drm/drm_mode_config.h       |    2 
 include/drm/drm_plane.h             |   29 +++++++++++
 include/uapi/drm/drm.h              |    3 +
 include/uapi/drm/drm_mode.h         |   17 ++++++
 9 files changed, 219 insertions(+)

Index: linux/drivers/gpu/drm/drm_crtc.c
===================================================================
--- linux.orig/drivers/gpu/drm/drm_crtc.c
+++ linux/drivers/gpu/drm/drm_crtc.c
@@ -415,6 +415,99 @@ int drm_mode_getcrtc(struct drm_device *
 }
 
 /**
+ * drm_mode_getsource_res - get live source info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return a live source and set of IDs.
+ */
+int drm_mode_getsource_res(struct drm_device *dev, void *data,
+			   struct drm_file *file_priv)
+{
+	struct drm_mode_get_live_source_res *src_resp = data;
+	struct drm_mode_config *config;
+	struct drm_live_source *src;
+	uint32_t __user *src_ptr;
+	int copied = 0, ret = 0;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	drm_modeset_lock_all(dev);
+	config = &dev->mode_config;
+
+	/*
+	 * This ioctl is called twice, once to determine how much space is
+	 * needed, and the 2nd time to fill it.
+	 */
+	if (config->num_live_source &&
+	    (src_resp->count_sources >= config->num_live_source)) {
+		src_ptr = (uint32_t __user *)(unsigned long)src_resp->source_id_ptr;
+
+		list_for_each_entry(src, &config->live_source_list, head) {
+			if (put_user(src->base.id, src_ptr + copied)) {
+				ret = -EFAULT;
+				goto out;
+			}
+			copied++;
+		}
+	}
+	src_resp->count_sources = config->num_live_source;
+
+out:
+	drm_modeset_unlock_all(dev);
+	return ret;
+}
+
+/**
+ * drm_mode_getsource - get live source info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return live source info, including formats supported, ...
+ */
+int drm_mode_getsource(struct drm_device *dev, void *data,
+		       struct drm_file *file_priv)
+{
+	struct drm_mode_get_live_source *src_resp = data;
+	struct drm_mode_object *obj;
+	struct drm_live_source *src;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	obj = drm_mode_object_find(dev, src_resp->source_id,
+				   DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (!obj)
+		return -ENOENT;
+	src = obj_to_live_source(obj);
+
+	src_resp->source_id = src->base.id;
+	strlcpy(src_resp->name, src->name, DRM_SOURCE_NAME_LEN);
+	src_resp->possible_planes = src->possible_planes;
+
+	/*
+	 * This ioctl is called twice, once to determine how much space is
+	 * needed, and the 2nd time to fill it.
+	 */
+	if (src->format_count &&
+	    (src_resp->count_format_types >= src->format_count)) {
+		uint32_t __user *format_ptr;
+
+		format_ptr = (uint32_t __user *)(unsigned long)src_resp->format_type_ptr;
+		if (copy_to_user(format_ptr, src->format_types,
+				 sizeof(uint32_t) * src->format_count)) {
+			return -EFAULT;
+		}
+	}
+	src_resp->count_format_types = src->format_count;
+
+	return 0;
+}
+
+/**
  * drm_mode_set_config_internal - helper to call ->set_config
  * @set: modeset config to set
  *
Index: linux/drivers/gpu/drm/drm_crtc_internal.h
===================================================================
--- linux.orig/drivers/gpu/drm/drm_crtc_internal.h
+++ linux/drivers/gpu/drm/drm_crtc_internal.h
@@ -50,6 +50,10 @@ int drm_mode_getcrtc(struct drm_device *
 		     void *data, struct drm_file *file_priv);
 int drm_mode_setcrtc(struct drm_device *dev,
 		     void *data, struct drm_file *file_priv);
+int drm_mode_getsource_res(struct drm_device *dev, void *data,
+			   struct drm_file *file_priv);
+int drm_mode_getsource(struct drm_device *dev, void *data,
+		       struct drm_file *file_priv);
 
 
 /* drm_mode_config.c */
Index: linux/drivers/gpu/drm/drm_ioctl.c
===================================================================
--- linux.orig/drivers/gpu/drm/drm_ioctl.c
+++ linux/drivers/gpu/drm/drm_ioctl.c
@@ -612,10 +612,12 @@ static const struct drm_ioctl_desc drm_i
 	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETSOURCERESOURCES, drm_mode_getsource_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETSOURCE, drm_mode_getsource, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETGAMMA, drm_mode_gamma_set_ioctl, DRM_MASTER|DRM_UNLOCKED),
Index: linux/drivers/gpu/drm/drm_mode_config.c
===================================================================
--- linux.orig/drivers/gpu/drm/drm_mode_config.c
+++ linux/drivers/gpu/drm/drm_mode_config.c
@@ -366,6 +366,7 @@ void drm_mode_config_init(struct drm_dev
 	INIT_LIST_HEAD(&dev->mode_config.property_list);
 	INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
 	INIT_LIST_HEAD(&dev->mode_config.plane_list);
+	INIT_LIST_HEAD(&dev->mode_config.live_source_list);
 	idr_init(&dev->mode_config.crtc_idr);
 	idr_init(&dev->mode_config.tile_idr);
 	ida_init(&dev->mode_config.connector_ida);
@@ -406,6 +407,7 @@ void drm_mode_config_cleanup(struct drm_
 	struct drm_property *property, *pt;
 	struct drm_property_blob *blob, *bt;
 	struct drm_plane *plane, *plt;
+	struct drm_live_source *src, *psrc;
 
 	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
 				 head) {
@@ -433,6 +435,11 @@ void drm_mode_config_cleanup(struct drm_
 		plane->funcs->destroy(plane);
 	}
 
+	list_for_each_entry_safe(src, psrc, &dev->mode_config.live_source_list,
+				 head) {
+		src->funcs->destroy(src);
+	}
+
 	list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) {
 		crtc->funcs->destroy(crtc);
 	}
Index: linux/drivers/gpu/drm/drm_plane.c
===================================================================
--- linux.orig/drivers/gpu/drm/drm_plane.c
+++ linux/drivers/gpu/drm/drm_plane.c
@@ -248,6 +248,68 @@ void drm_plane_cleanup(struct drm_plane
 }
 EXPORT_SYMBOL(drm_plane_cleanup);
 
+int drm_live_source_init(struct drm_device *dev, struct drm_live_source *src,
+			 const char *name, unsigned long possible_planes,
+			 const uint32_t *formats, uint32_t format_count,
+			 const struct drm_live_source_funcs *funcs)
+{
+	unsigned int i;
+	int ret;
+
+	/* Multi-planar live sources are not supported for now. */
+	for (i = 0; i < format_count; ++i) {
+		if (drm_format_num_planes(formats[i]) != 1) {
+			DRM_DEBUG_KMS("multiplanar live sources unsupported\n");
+			return -EINVAL;
+		}
+	}
+
+	drm_modeset_lock_all(dev);
+
+	ret = drm_mode_object_get(dev, &src->base, DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (ret)
+		goto out;
+
+	src->dev = dev;
+	src->funcs = funcs;
+	if (name)
+		strlcpy(src->name, name, DRM_SOURCE_NAME_LEN);
+	src->possible_planes = possible_planes;
+
+	src->format_types = kmalloc_array(format_count,
+					  sizeof(*src->format_types),
+					  GFP_KERNEL);
+	if (!src->format_types) {
+		DRM_DEBUG_KMS("out of memory when allocating source foramts\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	memcpy(src->format_types, formats,
+	       format_count * sizeof(*src->format_types));
+	src->format_count = format_count;
+
+	list_add_tail(&src->head, &dev->mode_config.live_source_list);
+	dev->mode_config.num_live_source++;
+
+ out:
+	drm_modeset_unlock_all(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_live_source_init);
+
+void drm_live_source_cleanup(struct drm_live_source *src)
+{
+	struct drm_device *dev = src->dev;
+
+	drm_modeset_lock_all(dev);
+	list_del(&src->head);
+	dev->mode_config.num_live_source--;
+	drm_modeset_unlock_all(dev);
+}
+EXPORT_SYMBOL(drm_live_source_cleanup);
+
 /**
  * drm_plane_from_index - find the registered plane at an index
  * @dev: DRM device
Index: linux/include/drm/drm_mode_config.h
===================================================================
--- linux.orig/include/drm/drm_mode_config.h
+++ linux/include/drm/drm_mode_config.h
@@ -396,6 +396,8 @@ struct drm_mode_config {
 	int num_overlay_plane;
 	int num_total_plane;
 	struct list_head plane_list;
+	int num_live_source;
+	struct list_head live_source_list;
 
 	int num_crtc;
 	struct list_head crtc_list;
Index: linux/include/drm/drm_plane.h
===================================================================
--- linux.orig/include/drm/drm_plane.h
+++ linux/include/drm/drm_plane.h
@@ -29,6 +29,7 @@
 
 struct drm_crtc;
 struct drm_printer;
+struct drm_live_source;
 
 /**
  * struct drm_plane_state - mutable plane state
@@ -527,6 +528,34 @@ extern int drm_plane_init(struct drm_dev
 			  bool is_primary);
 extern void drm_plane_cleanup(struct drm_plane *plane);
 
+struct drm_live_source_funcs {
+	void (*destroy)(struct drm_live_source *src);
+};
+
+struct drm_live_source {
+	struct drm_device *dev;
+	struct list_head head;
+
+	struct drm_mode_object base;
+
+	char name[DRM_SOURCE_NAME_LEN];
+
+	uint32_t possible_planes;
+	uint32_t *format_types;
+	uint32_t format_count;
+
+	const struct drm_live_source_funcs *funcs;
+};
+
+#define obj_to_live_source(x) container_of(x, struct drm_live_source, base)
+
+extern int drm_live_source_init(struct drm_device *dev,
+				struct drm_live_source *src, const char *name,
+				unsigned long possible_planes,
+				const uint32_t *formats, uint32_t format_count,
+				const struct drm_live_source_funcs *funcs);
+extern void drm_live_source_cleanup(struct drm_live_source *src);
+
 /**
  * drm_plane_index - find the index of a registered plane
  * @plane: plane to find index for
Index: linux/include/uapi/drm/drm.h
===================================================================
--- linux.orig/include/uapi/drm/drm.h
+++ linux/include/uapi/drm/drm.h
@@ -814,6 +814,9 @@ extern "C" {
 #define DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, struct drm_mode_create_blob)
 #define DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, struct drm_mode_destroy_blob)
 
+#define DRM_IOCTL_MODE_GETSOURCERESOURCES DRM_IOWR(0xBF, struct drm_mode_get_live_source_res)
+#define DRM_IOCTL_MODE_GETSOURCE	DRM_IOWR(0xC0, struct drm_mode_get_live_source)
+
 /**
  * Device specific ioctls should only be in their respective headers
  * The device specific ioctl range is from 0x40 to 0x9f.
Index: linux/include/uapi/drm/drm_mode.h
===================================================================
--- linux.orig/include/uapi/drm/drm_mode.h
+++ linux/include/uapi/drm/drm_mode.h
@@ -37,6 +37,7 @@ extern "C" {
 #define DRM_CONNECTOR_NAME_LEN	32
 #define DRM_DISPLAY_MODE_LEN	32
 #define DRM_PROP_NAME_LEN	32
+#define DRM_SOURCE_NAME_LEN	32
 
 #define DRM_MODE_TYPE_BUILTIN	(1<<0)
 #define DRM_MODE_TYPE_CLOCK_C	((1<<1) | DRM_MODE_TYPE_BUILTIN)
@@ -214,6 +215,21 @@ struct drm_mode_get_plane_res {
 	__u32 count_planes;
 };
 
+struct drm_mode_get_live_source {
+	__u32 source_id;
+	char name[DRM_SOURCE_NAME_LEN];
+
+	__u32 possible_planes;
+
+	__u32 count_format_types;
+	__u64 format_type_ptr;
+};
+
+struct drm_mode_get_live_source_res {
+	__u64 source_id_ptr;
+	__u32 count_sources;
+};
+
 #define DRM_MODE_ENCODER_NONE	0
 #define DRM_MODE_ENCODER_DAC	1
 #define DRM_MODE_ENCODER_TMDS	2
@@ -352,6 +368,7 @@ struct drm_mode_connector_set_property {
 #define DRM_MODE_OBJECT_FB 0xfbfbfbfb
 #define DRM_MODE_OBJECT_BLOB 0xbbbbbbbb
 #define DRM_MODE_OBJECT_PLANE 0xeeeeeeee
+#define DRM_MODE_OBJECT_LIVE_SOURCE 0xe1e1e1e1
 #define DRM_MODE_OBJECT_ANY 0
 
 struct drm_mode_obj_get_properties {

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

* [PATCH v3 2/3] drm: Connect live source to framebuffers
  2017-01-25 21:33 [PATCH v3 0/3] Enable R8A7790/1 DU VSPD compositor Sergei Shtylyov
  2017-01-25 21:35 ` [PATCH v3 1/3] drm: Add live source object Sergei Shtylyov
@ 2017-01-25 21:36 ` Sergei Shtylyov
  2017-01-31 15:42   ` Brian Starkey
  2017-01-25 21:38 ` [PATCH v3 3/3] drm: rcar-du: Add VSP1 live source support Sergei Shtylyov
  2 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-25 21:36 UTC (permalink / raw)
  To: laurent.pinchart, airlied, dri-devel, daniel.vetter, jani.nikula,
	seanpaul
  Cc: linux-renesas-soc, linux-kernel

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Introduce a new live source flag for framebuffers. When a framebuffer is
created with that flag set, a live source is associated with the
framebuffer instead of buffer objects. The framebuffer can then be used
with a plane to connect it with the live source.

[Sergei: ported to the modern kernel.]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes in version 3:
- ported the patch to the modern kernel;
- added my signoff.

 drivers/gpu/drm/drm_framebuffer.c |  134 ++++++++++++++++++++++++++++++--------
 include/uapi/drm/drm_mode.h       |    7 +
 2 files changed, 113 insertions(+), 28 deletions(-)

Index: linux/drivers/gpu/drm/drm_framebuffer.c
===================================================================
--- linux.orig/drivers/gpu/drm/drm_framebuffer.c
+++ linux/drivers/gpu/drm/drm_framebuffer.c
@@ -126,34 +126,18 @@ int drm_mode_addfb(struct drm_device *de
 	return 0;
 }
 
-static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
+static int framebuffer_check_buffers(const struct drm_mode_fb_cmd2 *r,
+				     int hsub, int vsub)
 {
-	const struct drm_format_info *info;
-	int i;
+	int num_planes, i;
+	unsigned int cpp;
 
-	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
-	if (!info) {
-		struct drm_format_name_buf format_name;
-		DRM_DEBUG_KMS("bad framebuffer format %s\n",
-		              drm_get_format_name(r->pixel_format,
-		                                  &format_name));
-		return -EINVAL;
-	}
-
-	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 % info->vsub) {
-		DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
-		return -EINVAL;
-	}
+ 	num_planes = drm_format_num_planes(r->pixel_format);
+	cpp = drm_format_plane_cpp(r->pixel_format, 0);
 
-	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];
+	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);
 
 		if (!r->handles[i]) {
 			DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i);
@@ -203,7 +187,7 @@ static int framebuffer_check(const struc
 		}
 	}
 
-	for (i = info->num_planes; i < 4; i++) {
+	for (i = num_planes; i < 4; i++) {
 		if (r->modifier[i]) {
 			DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i);
 			return -EINVAL;
@@ -232,6 +216,99 @@ static int framebuffer_check(const struc
 	return 0;
 }
 
+static int framebuffer_check_sources(struct drm_device *dev,
+				     const struct drm_mode_fb_cmd2 *r)
+{
+	struct drm_mode_object *obj;
+	struct drm_live_source *src;
+	unsigned int cpp;
+	unsigned int i;
+
+	/*
+	 * Ensure that userspace has zeroed unused handles, pitches, offsets and
+	 * modifiers to allow future API extensions.
+	 */
+	if (r->offsets[0] || r->modifier[0])
+		return -EINVAL;
+
+	for (i = 1; i < ARRAY_SIZE(r->handles); ++i) {
+		if (r->handles[i] || r->pitches[i] ||
+		    r->offsets[i] || r->modifier[i])
+			return -EINVAL;
+	}
+
+	/* Validate width, height and pitch. */
+	cpp = drm_format_plane_cpp(r->pixel_format, 0);
+
+	if ((uint64_t) r->width * cpp > UINT_MAX)
+		return -ERANGE;
+
+	if ((uint64_t) r->height * r->pitches[0] > UINT_MAX)
+		return -ERANGE;
+
+	if (r->pitches[0] != r->width * cpp) {
+		DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[0], i);
+		return -EINVAL;
+	}
+
+	/*
+	 * Find the live source and check whether it supports the requested
+	 * pixel format.
+	 */
+
+	obj = drm_mode_object_find(dev, r->handles[0],
+				   DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (!obj) {
+		DRM_DEBUG_KMS("bad framebuffer source ID %u\n", r->handles[0]);
+		return -EINVAL;
+	}
+
+	src = obj_to_live_source(obj);
+
+	for (i = 0; i < src->format_count; i++) {
+		if (r->pixel_format == src->format_types[i])
+			break;
+	}
+
+	if (i == src->format_count) {
+		DRM_DEBUG_KMS("bad framebuffer pixel format 0x%08x for source %u\n",
+			      r->pixel_format, r->handles[0]);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int framebuffer_check(struct drm_device *dev,
+			     const struct drm_mode_fb_cmd2 *r)
+{
+	const struct drm_format_info *info;
+
+	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
+	if (!info) {
+		struct drm_format_name_buf format_name;
+		DRM_DEBUG_KMS("bad framebuffer format %s\n",
+		              drm_get_format_name(r->pixel_format,
+		                                  &format_name));
+		return -EINVAL;
+	}
+
+	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 % info->vsub) {
+		DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
+		return -EINVAL;
+	}
+
+	if (r->flags & DRM_MODE_FB_LIVE_SOURCE)
+		return framebuffer_check_sources(dev, r);
+	else
+		return framebuffer_check_buffers(r, info->hsub, info->vsub);
+}
+
 struct drm_framebuffer *
 drm_internal_framebuffer_create(struct drm_device *dev,
 				const struct drm_mode_fb_cmd2 *r,
@@ -241,7 +318,8 @@ drm_internal_framebuffer_create(struct d
 	struct drm_framebuffer *fb;
 	int ret;
 
-	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
+	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS |
+			 DRM_MODE_FB_LIVE_SOURCE)) {
 		DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
 		return ERR_PTR(-EINVAL);
 	}
@@ -263,7 +341,7 @@ drm_internal_framebuffer_create(struct d
 		return ERR_PTR(-EINVAL);
 	}
 
-	ret = framebuffer_check(r);
+	ret = framebuffer_check(dev, r);
 	if (ret)
 		return ERR_PTR(ret);
 
Index: linux/include/uapi/drm/drm_mode.h
===================================================================
--- linux.orig/include/uapi/drm/drm_mode.h
+++ linux/include/uapi/drm/drm_mode.h
@@ -405,6 +405,7 @@ struct drm_mode_fb_cmd {
 
 #define DRM_MODE_FB_INTERLACED	(1<<0) /* for interlaced framebuffers */
 #define DRM_MODE_FB_MODIFIERS	(1<<1) /* enables ->modifer[] */
+#define DRM_MODE_FB_LIVE_SOURCE	(1<<2) /* connected to a live source */
 
 struct drm_mode_fb_cmd2 {
 	__u32 fb_id;
@@ -436,6 +437,12 @@ struct drm_mode_fb_cmd2 {
 	 * Thus all combinations of different data layouts for
 	 * multi plane formats must be enumerated as separate
 	 * modifiers.
+	 *
+	 * If the DRM_MODE_FB_LIVE_SOURCE flag is set the frame buffer input
+	 * comes from a live source instead of from memory. The handles[0]
+	 * field contains the ID of the connected live source object. All other
+	 * handles and all pitches, offsets and modifiers are then ignored by
+	 * the kernel and must be set to zero by applications.
 	 */
 	__u32 handles[4];
 	__u32 pitches[4]; /* pitch for each plane */

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

* [PATCH v3 3/3] drm: rcar-du: Add VSP1 live source support
  2017-01-25 21:33 [PATCH v3 0/3] Enable R8A7790/1 DU VSPD compositor Sergei Shtylyov
  2017-01-25 21:35 ` [PATCH v3 1/3] drm: Add live source object Sergei Shtylyov
  2017-01-25 21:36 ` [PATCH v3 2/3] drm: Connect live source to framebuffers Sergei Shtylyov
@ 2017-01-25 21:38 ` Sergei Shtylyov
  2017-01-31 15:42   ` Brian Starkey
  2 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2017-01-25 21:38 UTC (permalink / raw)
  To: laurent.pinchart, airlied, dri-devel; +Cc: linux-renesas-soc, linux-kernel

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Register live sources for VSPD0 and VSPD1 and configure the plane source
at plane setup time to source frames from memory or from the VSP1.

[Sergei: ported to the modern kernel.]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes in version 3:
- ported the patch to the modern kernel;
- reformatted rcar_du_live_fb_create_handle()/rcar_du_live_fb_create()'s
  parameter lists;
- added my signoff.

 drivers/gpu/drm/rcar-du/rcar_du_drv.c   |    6 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   |   77 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_kms.h   |    3 +
 drivers/gpu/drm/rcar-du/rcar_du_plane.c |   79 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |    3 +
 5 files changed, 164 insertions(+), 4 deletions(-)

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -60,7 +60,8 @@ static const struct rcar_du_device_info
 static const struct rcar_du_device_info rcar_du_r8a7790_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
 	.quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
 	.num_crtcs = 3,
 	.routes = {
@@ -90,7 +91,8 @@ static const struct rcar_du_device_info
 static const struct rcar_du_device_info rcar_du_r8a7791_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
 	.num_crtcs = 2,
 	.routes = {
 		/* R8A779[13] has one RGB output, one LVDS output and one
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -166,6 +166,74 @@ int rcar_du_dumb_create(struct drm_file
 	return drm_gem_cma_dumb_create_internal(file, dev, args);
 }
 
+struct rcar_du_live_framebuffer {
+	struct drm_framebuffer fb;
+	struct drm_live_source *src;
+};
+
+#define to_live_fb(fb) container_of(fb, struct rcar_du_live_framebuffer, fb)
+
+struct drm_live_source *rcar_du_live_fb_source(struct drm_framebuffer *fb)
+{
+	return fb->flags & DRM_MODE_FB_LIVE_SOURCE ? to_live_fb(fb)->src : NULL;
+}
+
+static void rcar_du_live_fb_destroy(struct drm_framebuffer *fb)
+{
+	struct rcar_du_live_framebuffer *live_fb = to_live_fb(fb);
+
+	drm_framebuffer_cleanup(fb);
+	kfree(live_fb);
+}
+
+static int rcar_du_live_fb_create_handle(struct drm_framebuffer *fb,
+					 struct drm_file *file_priv,
+					 unsigned int *handle)
+{
+	*handle = 0;
+	return 0;
+}
+
+static struct drm_framebuffer_funcs rcar_du_live_fb_funcs = {
+	.destroy	= rcar_du_live_fb_destroy,
+	.create_handle	= rcar_du_live_fb_create_handle,
+};
+
+static struct drm_framebuffer *
+rcar_du_live_fb_create(struct drm_device *dev, struct drm_file *file_priv,
+		       const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	struct rcar_du_live_framebuffer *fb;
+	struct drm_mode_object *obj;
+	struct drm_live_source *src;
+	int ret;
+
+	obj = drm_mode_object_find(dev, mode_cmd->handles[0],
+				   DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (!obj)
+		return ERR_PTR(-EINVAL);
+
+	src = obj_to_live_source(obj);
+
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+	if (!fb)
+		return ERR_PTR(-ENOMEM);
+
+	drm_helper_mode_fill_fb_struct(dev, &fb->fb, mode_cmd);
+
+	fb->src = src;
+
+	ret = drm_framebuffer_init(dev, &fb->fb, &rcar_du_live_fb_funcs);
+	if (ret) {
+		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n",
+			ret);
+		kfree(fb);
+		return ERR_PTR(ret);
+	}
+
+	return &fb->fb;
+}
+
 static struct drm_framebuffer *
 rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 		  const struct drm_mode_fb_cmd2 *mode_cmd)
@@ -177,6 +245,9 @@ rcar_du_fb_create(struct drm_device *dev
 	unsigned int bpp;
 	unsigned int i;
 
+	if (mode_cmd->flags & DRM_MODE_FB_LIVE_SOURCE)
+		return rcar_du_live_fb_create(dev, file_priv, mode_cmd);
+
 	format = rcar_du_format_info(mode_cmd->pixel_format);
 	if (format == NULL) {
 		dev_dbg(dev->dev, "unsupported pixel format %08x\n",
@@ -659,6 +730,12 @@ int rcar_du_modeset_init(struct rcar_du_
 
 	drm_mode_config_reset(dev);
 
+	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) {
+		ret = rcar_du_vsp1_sources_init(rcdu);
+		if (ret < 0)
+			return ret;
+	}
+
 	drm_kms_helper_poll_init(dev);
 
 	if (dev->mode_config.num_connector) {
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_kms.h
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_kms.h
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_kms.h
@@ -18,6 +18,7 @@
 
 struct drm_file;
 struct drm_device;
+struct drm_live_source;
 struct drm_mode_create_dumb;
 struct rcar_du_device;
 
@@ -36,4 +37,6 @@ int rcar_du_modeset_init(struct rcar_du_
 int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
 			struct drm_mode_create_dumb *args);
 
+struct drm_live_source *rcar_du_live_fb_source(struct drm_framebuffer *fb);
+
 #endif /* __RCAR_DU_KMS_H__ */
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_plane.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -27,6 +27,71 @@
 #include "rcar_du_regs.h"
 
 /* -----------------------------------------------------------------------------
+ * Live Sources
+ */
+
+struct rcar_du_vsp1_source {
+	struct drm_live_source base;
+
+	enum rcar_du_plane_source source;
+};
+
+static inline struct rcar_du_vsp1_source *
+to_rcar_vsp1_source(struct drm_live_source *src)
+{
+	return container_of(src, struct rcar_du_vsp1_source, base);
+}
+
+static const struct drm_live_source_funcs rcar_du_live_source_funcs = {
+	.destroy = drm_live_source_cleanup,
+};
+
+static const uint32_t source_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+int rcar_du_vsp1_sources_init(struct rcar_du_device *rcdu)
+{
+	static const struct {
+		enum rcar_du_plane_source source;
+		unsigned int planes;
+	} sources[] = {
+		{ RCAR_DU_PLANE_VSPD0, BIT(RCAR_DU_NUM_KMS_PLANES - 1) },
+		{ RCAR_DU_PLANE_VSPD1, BIT(RCAR_DU_NUM_KMS_PLANES - 2) |
+				       BIT(2 * RCAR_DU_NUM_KMS_PLANES - 1) },
+	};
+	unsigned int planes_mask;
+	unsigned int num_planes;
+	unsigned int i;
+
+	num_planes = RCAR_DU_NUM_KMS_PLANES * DIV_ROUND_UP(rcdu->num_crtcs, 2);
+	planes_mask = (1 << num_planes) - 1;
+
+	for (i = 0; i < ARRAY_SIZE(sources); ++i) {
+		struct rcar_du_vsp1_source *src;
+		char name[6];
+		int ret;
+
+		src = devm_kzalloc(rcdu->dev, sizeof(*src), GFP_KERNEL);
+		if (src == NULL)
+			return -ENOMEM;
+
+		src->source = sources[i].source;
+
+		sprintf(name, "vspd%u", i);
+		ret = drm_live_source_init(rcdu->ddev, &src->base, name,
+					   sources[i].planes & planes_mask,
+					   source_formats,
+					   ARRAY_SIZE(source_formats),
+					   &rcar_du_live_source_funcs);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+/* -----------------------------------------------------------------------------
  * Atomic hardware plane allocator
  *
  * The hardware plane allocator is solely based on the atomic plane states
@@ -555,6 +620,7 @@ static int rcar_du_plane_atomic_check(st
 	struct rcar_du_plane_state *rstate = to_rcar_plane_state(state);
 	struct rcar_du_plane *rplane = to_rcar_plane(plane);
 	struct rcar_du_device *rcdu = rplane->group->dev;
+	uint32_t pixel_format;
 
 	if (!state->fb || !state->crtc) {
 		rstate->format = NULL;
@@ -567,13 +633,22 @@ static int rcar_du_plane_atomic_check(st
 		return -EINVAL;
 	}
 
-	rstate->format = rcar_du_format_info(state->fb->format->format);
+	pixel_format = state->fb->format->format;
+	rstate->format = rcar_du_format_info(pixel_format);
 	if (rstate->format == NULL) {
 		dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__,
-			state->fb->format->format);
+			pixel_format);
 		return -EINVAL;
 	}
 
+	if (state->fb->flags & DRM_MODE_FB_LIVE_SOURCE) {
+		struct drm_live_source *src = rcar_du_live_fb_source(state->fb);
+
+		rstate->source = to_rcar_vsp1_source(src)->source;
+	} else {
+		rstate->source = RCAR_DU_PLANE_MEMORY;
+	}
+
 	return 0;
 }
 
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_plane.h
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -17,6 +17,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 
+struct rcar_du_device;
 struct rcar_du_format_info;
 struct rcar_du_group;
 
@@ -72,6 +73,8 @@ to_rcar_plane_state(struct drm_plane_sta
 int rcar_du_atomic_check_planes(struct drm_device *dev,
 				struct drm_atomic_state *state);
 
+int rcar_du_vsp1_sources_init(struct rcar_du_device *rcdu);
+
 int rcar_du_planes_init(struct rcar_du_group *rgrp);
 
 void __rcar_du_plane_setup(struct rcar_du_group *rgrp,

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

* Re: [PATCH v3 1/3] drm: Add live source object
  2017-01-25 21:35 ` [PATCH v3 1/3] drm: Add live source object Sergei Shtylyov
@ 2017-01-31 15:41   ` Brian Starkey
  0 siblings, 0 replies; 7+ messages in thread
From: Brian Starkey @ 2017-01-31 15:41 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: dri-devel, linux-kernel, linux-renesas-soc, laurent.pinchart,
	daniel.vetter

Hi Sergei,

I wasn't around to see v1/v2 but I've taken a quick look in the
archives.

As context for my comments below, I'm keen to push for something which
can be reused for live-sinks, so that we can hook a writeback
connector (when I land that) into a v4l2 device for video capture.

In that case, it's not a direct hardware connection, as the writeback
connector is still hitting memory - but it is a direct streaming link
between two different kernel devices. I don't think live-sources (and
sinks) should be limited to cases where no memory buffer is involved.
There could be cases where a video encoder/decoder is tightly coupled
with a display engine, and is able to stream without userspace
involvement but still go via a memory buffer.

With writeback in mind (and I think I'm echoing one of Daniel's
comments from v2), some simple renaming to replace "source" with
something that suits both sources and sinks would be good IMO.

I also think it's pretty hard to review this properly without seeing
the v4l2 side as-well. Does that exist somewhere? Definitely as a
minimum there needs to be an established way for userspace to map a
DRM live-source object to a v4l2 device. I guess there's also some
intercommunication needed to agree on sizes, formats etc. I don't know
how much of that can be generic, but leaving it entirely up to drivers
without any examples/helpers sounds like a recipe for lots of
similar-but-incompatible interfaces.

What's the expected sequence of setting up a v4l2 device + live
source? Something like...?

1) VIDIOC_S_FMT etc
2) DRM_IOCTL_MODE_ADDFB2
3) VIDIOC_STREAMON
4) DRM_IOCTL_MODE_ATOMIC

The sequence above would imply that the v4l2 device gets configured
first, and then ADDFB2 must use settings which agree. An alternative
would be to call ADDFB2 first to "lock" the parameters of the v4l2
device, so subsequent calls to VIDIOC_G_FMT, VIDIOC_ENUM_FRAMSIZES etc
would only return the same values as used to create the live fb - that
sounds neater to me. Anyway I think this is something that needs to be
figured out and documented as part of this implementation.

Some more comments inline.

On Thu, Jan 26, 2017 at 12:35:30AM +0300, Sergei Shtylyov wrote:
>From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
>Live sources represent a hardware connection between a video stream
>source and a CRTC, going through a plane. The kernel API lets driver
>register live sources, and the userspace API lets applications enumerate
>them.
>
>[Sergei: ported to the modern kernel.]
>
>Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>---
>Changes in version 3:
>- moved drm_live_source_{init|cleanup}() from drm_crtc.c to drm_plane.c and
>  their prototypes from <drm/drm_crtc.h> to <drm/drm_plane.h> (along with
>  *struct* drm_live_source[_funcs]);
>- moved drm_mode_getsource[_res]() prototypes from <drm/drm_crtc.h> to
>  drm_crtc_internal.h;
>- updated the ioctl() numbers;
>- generally followed the upstream source code moves;
>- added my signoff.
>
> drivers/gpu/drm/drm_crtc.c          |   93 ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_crtc_internal.h |    4 +
> drivers/gpu/drm/drm_ioctl.c         |    2
> drivers/gpu/drm/drm_mode_config.c   |    7 ++
> drivers/gpu/drm/drm_plane.c         |   62 ++++++++++++++++++++++++
> include/drm/drm_mode_config.h       |    2
> include/drm/drm_plane.h             |   29 +++++++++++
> include/uapi/drm/drm.h              |    3 +
> include/uapi/drm/drm_mode.h         |   17 ++++++
> 9 files changed, 219 insertions(+)
>
>Index: linux/drivers/gpu/drm/drm_crtc.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/drm_crtc.c
>+++ linux/drivers/gpu/drm/drm_crtc.c
>@@ -415,6 +415,99 @@ int drm_mode_getcrtc(struct drm_device *
> }
>
> /**
>+ * drm_mode_getsource_res - get live source info
>+ * @dev: DRM device
>+ * @data: ioctl data
>+ * @file_priv: DRM file info
>+ *
>+ * Return a live source and set of IDs.
>+ */
>+int drm_mode_getsource_res(struct drm_device *dev, void *data,
>+			   struct drm_file *file_priv)
>+{
>+	struct drm_mode_get_live_source_res *src_resp = data;
>+	struct drm_mode_config *config;
>+	struct drm_live_source *src;
>+	uint32_t __user *src_ptr;
>+	int copied = 0, ret = 0;
>+
>+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>+		return -EINVAL;
>+
>+	drm_modeset_lock_all(dev);
>+	config = &dev->mode_config;
>+
>+	/*
>+	 * This ioctl is called twice, once to determine how much space is
>+	 * needed, and the 2nd time to fill it.
>+	 */
>+	if (config->num_live_source &&
>+	    (src_resp->count_sources >= config->num_live_source)) {
>+		src_ptr = (uint32_t __user *)(unsigned long)src_resp->source_id_ptr;
>+
>+		list_for_each_entry(src, &config->live_source_list, head) {
>+			if (put_user(src->base.id, src_ptr + copied)) {
>+				ret = -EFAULT;
>+				goto out;
>+			}
>+			copied++;
>+		}
>+	}
>+	src_resp->count_sources = config->num_live_source;
>+
>+out:
>+	drm_modeset_unlock_all(dev);
>+	return ret;
>+}
>+
>+/**
>+ * drm_mode_getsource - get live source info
>+ * @dev: DRM device
>+ * @data: ioctl data
>+ * @file_priv: DRM file info
>+ *
>+ * Return live source info, including formats supported, ...
>+ */
>+int drm_mode_getsource(struct drm_device *dev, void *data,
>+		       struct drm_file *file_priv)
>+{
>+	struct drm_mode_get_live_source *src_resp = data;
>+	struct drm_mode_object *obj;
>+	struct drm_live_source *src;
>+
>+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>+		return -EINVAL;
>+
>+	obj = drm_mode_object_find(dev, src_resp->source_id,
>+				   DRM_MODE_OBJECT_LIVE_SOURCE);
>+	if (!obj)
>+		return -ENOENT;
>+	src = obj_to_live_source(obj);
>+
>+	src_resp->source_id = src->base.id;
>+	strlcpy(src_resp->name, src->name, DRM_SOURCE_NAME_LEN);
>+	src_resp->possible_planes = src->possible_planes;
>+
>+	/*
>+	 * This ioctl is called twice, once to determine how much space is
>+	 * needed, and the 2nd time to fill it.
>+	 */
>+	if (src->format_count &&
>+	    (src_resp->count_format_types >= src->format_count)) {
>+		uint32_t __user *format_ptr;
>+
>+		format_ptr = (uint32_t __user *)(unsigned long)src_resp->format_type_ptr;
>+		if (copy_to_user(format_ptr, src->format_types,
>+				 sizeof(uint32_t) * src->format_count)) {
>+			return -EFAULT;
>+		}
>+	}
>+	src_resp->count_format_types = src->format_count;
>+
>+	return 0;
>+}
>+
>+/**
>  * drm_mode_set_config_internal - helper to call ->set_config
>  * @set: modeset config to set
>  *
>Index: linux/drivers/gpu/drm/drm_crtc_internal.h
>===================================================================
>--- linux.orig/drivers/gpu/drm/drm_crtc_internal.h
>+++ linux/drivers/gpu/drm/drm_crtc_internal.h
>@@ -50,6 +50,10 @@ int drm_mode_getcrtc(struct drm_device *
> 		     void *data, struct drm_file *file_priv);
> int drm_mode_setcrtc(struct drm_device *dev,
> 		     void *data, struct drm_file *file_priv);
>+int drm_mode_getsource_res(struct drm_device *dev, void *data,
>+			   struct drm_file *file_priv);
>+int drm_mode_getsource(struct drm_device *dev, void *data,
>+		       struct drm_file *file_priv);
>
>
> /* drm_mode_config.c */
>Index: linux/drivers/gpu/drm/drm_ioctl.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/drm_ioctl.c
>+++ linux/drivers/gpu/drm/drm_ioctl.c
>@@ -612,10 +612,12 @@ static const struct drm_ioctl_desc drm_i
> 	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
>
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETSOURCERESOURCES, drm_mode_getsource_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETSOURCE, drm_mode_getsource, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, DRM_UNLOCKED),
> 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETGAMMA, drm_mode_gamma_set_ioctl, DRM_MASTER|DRM_UNLOCKED),
>Index: linux/drivers/gpu/drm/drm_mode_config.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/drm_mode_config.c
>+++ linux/drivers/gpu/drm/drm_mode_config.c
>@@ -366,6 +366,7 @@ void drm_mode_config_init(struct drm_dev
> 	INIT_LIST_HEAD(&dev->mode_config.property_list);
> 	INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
> 	INIT_LIST_HEAD(&dev->mode_config.plane_list);
>+	INIT_LIST_HEAD(&dev->mode_config.live_source_list);
> 	idr_init(&dev->mode_config.crtc_idr);
> 	idr_init(&dev->mode_config.tile_idr);
> 	ida_init(&dev->mode_config.connector_ida);
>@@ -406,6 +407,7 @@ void drm_mode_config_cleanup(struct drm_
> 	struct drm_property *property, *pt;
> 	struct drm_property_blob *blob, *bt;
> 	struct drm_plane *plane, *plt;
>+	struct drm_live_source *src, *psrc;
>
> 	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
> 				 head) {
>@@ -433,6 +435,11 @@ void drm_mode_config_cleanup(struct drm_
> 		plane->funcs->destroy(plane);
> 	}
>
>+	list_for_each_entry_safe(src, psrc, &dev->mode_config.live_source_list,
>+				 head) {
>+		src->funcs->destroy(src);
>+	}
>+
> 	list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) {
> 		crtc->funcs->destroy(crtc);
> 	}
>Index: linux/drivers/gpu/drm/drm_plane.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/drm_plane.c
>+++ linux/drivers/gpu/drm/drm_plane.c
>@@ -248,6 +248,68 @@ void drm_plane_cleanup(struct drm_plane
> }
> EXPORT_SYMBOL(drm_plane_cleanup);
>
>+int drm_live_source_init(struct drm_device *dev, struct drm_live_source *src,
>+			 const char *name, unsigned long possible_planes,
>+			 const uint32_t *formats, uint32_t format_count,
>+			 const struct drm_live_source_funcs *funcs)
>+{
>+	unsigned int i;
>+	int ret;
>+
>+	/* Multi-planar live sources are not supported for now. */
>+	for (i = 0; i < format_count; ++i) {
>+		if (drm_format_num_planes(formats[i]) != 1) {
>+			DRM_DEBUG_KMS("multiplanar live sources unsupported\n");
>+			return -EINVAL;
>+		}
>+	}
>+
>+	drm_modeset_lock_all(dev);
>+
>+	ret = drm_mode_object_get(dev, &src->base, DRM_MODE_OBJECT_LIVE_SOURCE);
>+	if (ret)
>+		goto out;
>+
>+	src->dev = dev;
>+	src->funcs = funcs;
>+	if (name)
>+		strlcpy(src->name, name, DRM_SOURCE_NAME_LEN);
>+	src->possible_planes = possible_planes;
>+
>+	src->format_types = kmalloc_array(format_count,
>+					  sizeof(*src->format_types),
>+					  GFP_KERNEL);
>+	if (!src->format_types) {
>+		DRM_DEBUG_KMS("out of memory when allocating source foramts\n");
>+		ret = -ENOMEM;
>+		goto out;
>+	}
>+
>+	memcpy(src->format_types, formats,
>+	       format_count * sizeof(*src->format_types));
>+	src->format_count = format_count;
>+
>+	list_add_tail(&src->head, &dev->mode_config.live_source_list);
>+	dev->mode_config.num_live_source++;
>+
>+ out:
>+	drm_modeset_unlock_all(dev);
>+
>+	return ret;
>+}
>+EXPORT_SYMBOL(drm_live_source_init);
>+
>+void drm_live_source_cleanup(struct drm_live_source *src)
>+{
>+	struct drm_device *dev = src->dev;
>+
>+	drm_modeset_lock_all(dev);
>+	list_del(&src->head);
>+	dev->mode_config.num_live_source--;
>+	drm_modeset_unlock_all(dev);
>+}
>+EXPORT_SYMBOL(drm_live_source_cleanup);
>+
> /**
>  * drm_plane_from_index - find the registered plane at an index
>  * @dev: DRM device
>Index: linux/include/drm/drm_mode_config.h
>===================================================================
>--- linux.orig/include/drm/drm_mode_config.h
>+++ linux/include/drm/drm_mode_config.h
>@@ -396,6 +396,8 @@ struct drm_mode_config {
> 	int num_overlay_plane;
> 	int num_total_plane;
> 	struct list_head plane_list;
>+	int num_live_source;
>+	struct list_head live_source_list;
>
> 	int num_crtc;
> 	struct list_head crtc_list;
>Index: linux/include/drm/drm_plane.h
>===================================================================
>--- linux.orig/include/drm/drm_plane.h
>+++ linux/include/drm/drm_plane.h
>@@ -29,6 +29,7 @@
>
> struct drm_crtc;
> struct drm_printer;
>+struct drm_live_source;
>
> /**
>  * struct drm_plane_state - mutable plane state
>@@ -527,6 +528,34 @@ extern int drm_plane_init(struct drm_dev
> 			  bool is_primary);
> extern void drm_plane_cleanup(struct drm_plane *plane);
>
>+struct drm_live_source_funcs {
>+	void (*destroy)(struct drm_live_source *src);
>+};
>+
>+struct drm_live_source {
>+	struct drm_device *dev;
>+	struct list_head head;
>+
>+	struct drm_mode_object base;
>+
>+	char name[DRM_SOURCE_NAME_LEN];
>+
>+	uint32_t possible_planes;
>+	uint32_t *format_types;
>+	uint32_t format_count;
>+
>+	const struct drm_live_source_funcs *funcs;
>+};
>+
>+#define obj_to_live_source(x) container_of(x, struct drm_live_source, base)
>+
>+extern int drm_live_source_init(struct drm_device *dev,
>+				struct drm_live_source *src, const char *name,
>+				unsigned long possible_planes,
>+				const uint32_t *formats, uint32_t format_count,
>+				const struct drm_live_source_funcs *funcs);
>+extern void drm_live_source_cleanup(struct drm_live_source *src);
>+
> /**
>  * drm_plane_index - find the index of a registered plane
>  * @plane: plane to find index for
>Index: linux/include/uapi/drm/drm.h
>===================================================================
>--- linux.orig/include/uapi/drm/drm.h
>+++ linux/include/uapi/drm/drm.h
>@@ -814,6 +814,9 @@ extern "C" {
> #define DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, struct drm_mode_create_blob)
> #define DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, struct drm_mode_destroy_blob)
>
>+#define DRM_IOCTL_MODE_GETSOURCERESOURCES DRM_IOWR(0xBF, struct drm_mode_get_live_source_res)
>+#define DRM_IOCTL_MODE_GETSOURCE	DRM_IOWR(0xC0, struct drm_mode_get_live_source)
>+
> /**
>  * Device specific ioctls should only be in their respective headers
>  * The device specific ioctl range is from 0x40 to 0x9f.
>Index: linux/include/uapi/drm/drm_mode.h
>===================================================================
>--- linux.orig/include/uapi/drm/drm_mode.h
>+++ linux/include/uapi/drm/drm_mode.h
>@@ -37,6 +37,7 @@ extern "C" {
> #define DRM_CONNECTOR_NAME_LEN	32
> #define DRM_DISPLAY_MODE_LEN	32
> #define DRM_PROP_NAME_LEN	32
>+#define DRM_SOURCE_NAME_LEN	32
>
> #define DRM_MODE_TYPE_BUILTIN	(1<<0)
> #define DRM_MODE_TYPE_CLOCK_C	((1<<1) | DRM_MODE_TYPE_BUILTIN)
>@@ -214,6 +215,21 @@ struct drm_mode_get_plane_res {
> 	__u32 count_planes;
> };
>
>+struct drm_mode_get_live_source {
>+	__u32 source_id;
>+	char name[DRM_SOURCE_NAME_LEN];
>+
>+	__u32 possible_planes;

Limiting this to planes would prevent live sinks coming off a
connector. Same question as Daniel from before - is the
possible_planes intended for userspace to figure out routing, or only
for internal validation?

>+
>+	__u32 count_format_types;
>+	__u64 format_type_ptr;

As v4l2 also has its own get/set format ioctls, how do you manage the
two together? If possible, only having to validate it in one place is
probably better than two.

Cheers,
-Brian

>+};
>+
>+struct drm_mode_get_live_source_res {
>+	__u64 source_id_ptr;
>+	__u32 count_sources;
>+};
>+
> #define DRM_MODE_ENCODER_NONE	0
> #define DRM_MODE_ENCODER_DAC	1
> #define DRM_MODE_ENCODER_TMDS	2
>@@ -352,6 +368,7 @@ struct drm_mode_connector_set_property {
> #define DRM_MODE_OBJECT_FB 0xfbfbfbfb
> #define DRM_MODE_OBJECT_BLOB 0xbbbbbbbb
> #define DRM_MODE_OBJECT_PLANE 0xeeeeeeee
>+#define DRM_MODE_OBJECT_LIVE_SOURCE 0xe1e1e1e1
> #define DRM_MODE_OBJECT_ANY 0
>
> struct drm_mode_obj_get_properties {
>
>_______________________________________________
>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

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

* Re: [PATCH v3 2/3] drm: Connect live source to framebuffers
  2017-01-25 21:36 ` [PATCH v3 2/3] drm: Connect live source to framebuffers Sergei Shtylyov
@ 2017-01-31 15:42   ` Brian Starkey
  0 siblings, 0 replies; 7+ messages in thread
From: Brian Starkey @ 2017-01-31 15:42 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: dri-devel, linux-kernel, linux-renesas-soc, laurent.pinchart,
	daniel.vetter

On Thu, Jan 26, 2017 at 12:36:55AM +0300, Sergei Shtylyov wrote:
>From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
>Introduce a new live source flag for framebuffers. When a framebuffer is
>created with that flag set, a live source is associated with the
>framebuffer instead of buffer objects. The framebuffer can then be used
>with a plane to connect it with the live source.
>
>[Sergei: ported to the modern kernel.]
>
>Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>---
>Changes in version 3:
>- ported the patch to the modern kernel;
>- added my signoff.
>
> drivers/gpu/drm/drm_framebuffer.c |  134 ++++++++++++++++++++++++++++++--------
> include/uapi/drm/drm_mode.h       |    7 +
> 2 files changed, 113 insertions(+), 28 deletions(-)
>
>Index: linux/drivers/gpu/drm/drm_framebuffer.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/drm_framebuffer.c
>+++ linux/drivers/gpu/drm/drm_framebuffer.c
>@@ -126,34 +126,18 @@ int drm_mode_addfb(struct drm_device *de
> 	return 0;
> }
>
>-static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
>+static int framebuffer_check_buffers(const struct drm_mode_fb_cmd2 *r,
>+				     int hsub, int vsub)
> {
>-	const struct drm_format_info *info;
>-	int i;
>+	int num_planes, i;
>+	unsigned int cpp;
>
>-	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
>-	if (!info) {
>-		struct drm_format_name_buf format_name;
>-		DRM_DEBUG_KMS("bad framebuffer format %s\n",
>-		              drm_get_format_name(r->pixel_format,
>-		                                  &format_name));
>-		return -EINVAL;
>-	}
>-
>-	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 % info->vsub) {
>-		DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
>-		return -EINVAL;
>-	}
>+ 	num_planes = drm_format_num_planes(r->pixel_format);
>+	cpp = drm_format_plane_cpp(r->pixel_format, 0);

You can pass in the drm_format_info and use it instead of looking it
up again here. That also saves the hsub/vsub arguments.

>
>-	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];
>+	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);
>
> 		if (!r->handles[i]) {
> 			DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i);
>@@ -203,7 +187,7 @@ static int framebuffer_check(const struc
> 		}
> 	}
>
>-	for (i = info->num_planes; i < 4; i++) {
>+	for (i = num_planes; i < 4; i++) {
> 		if (r->modifier[i]) {
> 			DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i);
> 			return -EINVAL;
>@@ -232,6 +216,99 @@ static int framebuffer_check(const struc
> 	return 0;
> }
>
>+static int framebuffer_check_sources(struct drm_device *dev,
>+				     const struct drm_mode_fb_cmd2 *r)
>+{
>+	struct drm_mode_object *obj;
>+	struct drm_live_source *src;
>+	unsigned int cpp;
>+	unsigned int i;
>+
>+	/*
>+	 * Ensure that userspace has zeroed unused handles, pitches, offsets and
>+	 * modifiers to allow future API extensions.
>+	 */
>+	if (r->offsets[0] || r->modifier[0])
>+		return -EINVAL;
>+
>+	for (i = 1; i < ARRAY_SIZE(r->handles); ++i) {
>+		if (r->handles[i] || r->pitches[i] ||
>+		    r->offsets[i] || r->modifier[i])
>+			return -EINVAL;
>+	}
>+
>+	/* Validate width, height and pitch. */
>+	cpp = drm_format_plane_cpp(r->pixel_format, 0);

Same comment about passing in the drm_format_info instead of looking
it up again.

>+
>+	if ((uint64_t) r->width * cpp > UINT_MAX)
>+		return -ERANGE;
>+
>+	if ((uint64_t) r->height * r->pitches[0] > UINT_MAX)
>+		return -ERANGE;
>+
>+	if (r->pitches[0] != r->width * cpp) {
>+		DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[0], i);
>+		return -EINVAL;
>+	}
>+
>+	/*
>+	 * Find the live source and check whether it supports the requested
>+	 * pixel format.
>+	 */

Is it out-of-scope to check with the source subsystem (v4l2) that its
format is configured the same way? And/or inform it of the live-source
format so that it can validate such things.

-Brian
>+
>+	obj = drm_mode_object_find(dev, r->handles[0],
>+				   DRM_MODE_OBJECT_LIVE_SOURCE);
>+	if (!obj) {
>+		DRM_DEBUG_KMS("bad framebuffer source ID %u\n", r->handles[0]);
>+		return -EINVAL;
>+	}
>+
>+	src = obj_to_live_source(obj);
>+
>+	for (i = 0; i < src->format_count; i++) {
>+		if (r->pixel_format == src->format_types[i])
>+			break;
>+	}
>+
>+	if (i == src->format_count) {
>+		DRM_DEBUG_KMS("bad framebuffer pixel format 0x%08x for source %u\n",
>+			      r->pixel_format, r->handles[0]);
>+		return -EINVAL;
>+	}
>+
>+	return 0;
>+}
>+
>+static int framebuffer_check(struct drm_device *dev,
>+			     const struct drm_mode_fb_cmd2 *r)
>+{
>+	const struct drm_format_info *info;
>+
>+	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
>+	if (!info) {
>+		struct drm_format_name_buf format_name;
>+		DRM_DEBUG_KMS("bad framebuffer format %s\n",
>+		              drm_get_format_name(r->pixel_format,
>+		                                  &format_name));
>+		return -EINVAL;
>+	}
>+
>+	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 % info->vsub) {
>+		DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
>+		return -EINVAL;
>+	}
>+
>+	if (r->flags & DRM_MODE_FB_LIVE_SOURCE)
>+		return framebuffer_check_sources(dev, r);
>+	else
>+		return framebuffer_check_buffers(r, info->hsub, info->vsub);
>+}
>+
> struct drm_framebuffer *
> drm_internal_framebuffer_create(struct drm_device *dev,
> 				const struct drm_mode_fb_cmd2 *r,
>@@ -241,7 +318,8 @@ drm_internal_framebuffer_create(struct d
> 	struct drm_framebuffer *fb;
> 	int ret;
>
>-	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
>+	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS |
>+			 DRM_MODE_FB_LIVE_SOURCE)) {
> 		DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
> 		return ERR_PTR(-EINVAL);
> 	}
>@@ -263,7 +341,7 @@ drm_internal_framebuffer_create(struct d
> 		return ERR_PTR(-EINVAL);
> 	}
>
>-	ret = framebuffer_check(r);
>+	ret = framebuffer_check(dev, r);
> 	if (ret)
> 		return ERR_PTR(ret);
>
>Index: linux/include/uapi/drm/drm_mode.h
>===================================================================
>--- linux.orig/include/uapi/drm/drm_mode.h
>+++ linux/include/uapi/drm/drm_mode.h
>@@ -405,6 +405,7 @@ struct drm_mode_fb_cmd {
>
> #define DRM_MODE_FB_INTERLACED	(1<<0) /* for interlaced framebuffers */
> #define DRM_MODE_FB_MODIFIERS	(1<<1) /* enables ->modifer[] */
>+#define DRM_MODE_FB_LIVE_SOURCE	(1<<2) /* connected to a live source */
>
> struct drm_mode_fb_cmd2 {
> 	__u32 fb_id;
>@@ -436,6 +437,12 @@ struct drm_mode_fb_cmd2 {
> 	 * Thus all combinations of different data layouts for
> 	 * multi plane formats must be enumerated as separate
> 	 * modifiers.
>+	 *
>+	 * If the DRM_MODE_FB_LIVE_SOURCE flag is set the frame buffer input
>+	 * comes from a live source instead of from memory. The handles[0]
>+	 * field contains the ID of the connected live source object. All other
>+	 * handles and all pitches, offsets and modifiers are then ignored by
>+	 * the kernel and must be set to zero by applications.
> 	 */
> 	__u32 handles[4];
> 	__u32 pitches[4]; /* pitch for each plane */
>
>_______________________________________________
>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

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

* Re: [PATCH v3 3/3] drm: rcar-du: Add VSP1 live source support
  2017-01-25 21:38 ` [PATCH v3 3/3] drm: rcar-du: Add VSP1 live source support Sergei Shtylyov
@ 2017-01-31 15:42   ` Brian Starkey
  0 siblings, 0 replies; 7+ messages in thread
From: Brian Starkey @ 2017-01-31 15:42 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-renesas-soc, laurent.pinchart, dri-devel, linux-kernel

On Thu, Jan 26, 2017 at 12:38:26AM +0300, Sergei Shtylyov wrote:
>From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
>Register live sources for VSPD0 and VSPD1 and configure the plane source
>at plane setup time to source frames from memory or from the VSP1.
>
>[Sergei: ported to the modern kernel.]
>
>Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>---
>Changes in version 3:
>- ported the patch to the modern kernel;
>- reformatted rcar_du_live_fb_create_handle()/rcar_du_live_fb_create()'s
>  parameter lists;
>- added my signoff.
>
> drivers/gpu/drm/rcar-du/rcar_du_drv.c   |    6 +-
> drivers/gpu/drm/rcar-du/rcar_du_kms.c   |   77 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/rcar-du/rcar_du_kms.h   |    3 +
> drivers/gpu/drm/rcar-du/rcar_du_plane.c |   79 +++++++++++++++++++++++++++++++-
> drivers/gpu/drm/rcar-du/rcar_du_plane.h |    3 +
> 5 files changed, 164 insertions(+), 4 deletions(-)
>
>Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>+++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>@@ -60,7 +60,8 @@ static const struct rcar_du_device_info
> static const struct rcar_du_device_info rcar_du_r8a7790_info = {
> 	.gen = 2,
> 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
> 	.quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
> 	.num_crtcs = 3,
> 	.routes = {
>@@ -90,7 +91,8 @@ static const struct rcar_du_device_info
> static const struct rcar_du_device_info rcar_du_r8a7791_info = {
> 	.gen = 2,
> 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
> 	.num_crtcs = 2,
> 	.routes = {
> 		/* R8A779[13] has one RGB output, one LVDS output and one
>Index: linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>+++ linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>@@ -166,6 +166,74 @@ int rcar_du_dumb_create(struct drm_file
> 	return drm_gem_cma_dumb_create_internal(file, dev, args);
> }
>
>+struct rcar_du_live_framebuffer {
>+	struct drm_framebuffer fb;
>+	struct drm_live_source *src;

I feel like this src member should live in the generic
drm_framebuffer, and core/helpers should take care of setting it up
based on the passed-in live-source handle.

>+};
>+
>+#define to_live_fb(fb) container_of(fb, struct rcar_du_live_framebuffer, fb)
>+
>+struct drm_live_source *rcar_du_live_fb_source(struct drm_framebuffer *fb)
>+{
>+	return fb->flags & DRM_MODE_FB_LIVE_SOURCE ? to_live_fb(fb)->src : NULL;
>+}
>+
>+static void rcar_du_live_fb_destroy(struct drm_framebuffer *fb)
>+{
>+	struct rcar_du_live_framebuffer *live_fb = to_live_fb(fb);
>+
>+	drm_framebuffer_cleanup(fb);
>+	kfree(live_fb);
>+}
>+
>+static int rcar_du_live_fb_create_handle(struct drm_framebuffer *fb,
>+					 struct drm_file *file_priv,
>+					 unsigned int *handle)
>+{
>+	*handle = 0;
>+	return 0;
>+}
>+
>+static struct drm_framebuffer_funcs rcar_du_live_fb_funcs = {
>+	.destroy	= rcar_du_live_fb_destroy,
>+	.create_handle	= rcar_du_live_fb_create_handle,
>+};
>+
>+static struct drm_framebuffer *
>+rcar_du_live_fb_create(struct drm_device *dev, struct drm_file *file_priv,
>+		       const struct drm_mode_fb_cmd2 *mode_cmd)
>+{
>+	struct rcar_du_live_framebuffer *fb;
>+	struct drm_mode_object *obj;
>+	struct drm_live_source *src;
>+	int ret;
>+
>+	obj = drm_mode_object_find(dev, mode_cmd->handles[0],
>+				   DRM_MODE_OBJECT_LIVE_SOURCE);
>+	if (!obj)
>+		return ERR_PTR(-EINVAL);
>+
>+	src = obj_to_live_source(obj);
>+
>+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
>+	if (!fb)
>+		return ERR_PTR(-ENOMEM);
>+
>+	drm_helper_mode_fill_fb_struct(dev, &fb->fb, mode_cmd);
>+
>+	fb->src = src;
>+
>+	ret = drm_framebuffer_init(dev, &fb->fb, &rcar_du_live_fb_funcs);
>+	if (ret) {
>+		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n",
>+			ret);
>+		kfree(fb);
>+		return ERR_PTR(ret);
>+	}
>+
>+	return &fb->fb;
>+}

This looks pretty generic, should it live somewhere outside rcar-du?
Something like drm_live_fb_create_with_funcs, similar to
drm_fb_cma_create_with_funcs.

>+
> static struct drm_framebuffer *
> rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv,
> 		  const struct drm_mode_fb_cmd2 *mode_cmd)
>@@ -177,6 +245,9 @@ rcar_du_fb_create(struct drm_device *dev
> 	unsigned int bpp;
> 	unsigned int i;
>
>+	if (mode_cmd->flags & DRM_MODE_FB_LIVE_SOURCE)
>+		return rcar_du_live_fb_create(dev, file_priv, mode_cmd);
>+

Maybe even the core can offload this to a ->live_fb_create function in
the drm_mode_config_funcs.

> 	format = rcar_du_format_info(mode_cmd->pixel_format);
> 	if (format == NULL) {
> 		dev_dbg(dev->dev, "unsupported pixel format %08x\n",
>@@ -659,6 +730,12 @@ int rcar_du_modeset_init(struct rcar_du_
>
> 	drm_mode_config_reset(dev);
>
>+	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) {
>+		ret = rcar_du_vsp1_sources_init(rcdu);
>+		if (ret < 0)
>+			return ret;
>+	}
>+
> 	drm_kms_helper_poll_init(dev);
>
> 	if (dev->mode_config.num_connector) {
>Index: linux/drivers/gpu/drm/rcar-du/rcar_du_kms.h
>===================================================================
>--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_kms.h
>+++ linux/drivers/gpu/drm/rcar-du/rcar_du_kms.h
>@@ -18,6 +18,7 @@
>
> struct drm_file;
> struct drm_device;
>+struct drm_live_source;
> struct drm_mode_create_dumb;
> struct rcar_du_device;
>
>@@ -36,4 +37,6 @@ int rcar_du_modeset_init(struct rcar_du_
> int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> 			struct drm_mode_create_dumb *args);
>
>+struct drm_live_source *rcar_du_live_fb_source(struct drm_framebuffer *fb);
>+
> #endif /* __RCAR_DU_KMS_H__ */
>Index: linux/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>===================================================================
>--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>+++ linux/drivers/gpu/drm/rcar-du/rcar_du_plane.c
>@@ -27,6 +27,71 @@
> #include "rcar_du_regs.h"
>
> /* -----------------------------------------------------------------------------
>+ * Live Sources
>+ */
>+
>+struct rcar_du_vsp1_source {
>+	struct drm_live_source base;
>+
>+	enum rcar_du_plane_source source;
>+};
>+
>+static inline struct rcar_du_vsp1_source *
>+to_rcar_vsp1_source(struct drm_live_source *src)
>+{
>+	return container_of(src, struct rcar_du_vsp1_source, base);
>+}
>+
>+static const struct drm_live_source_funcs rcar_du_live_source_funcs = {
>+	.destroy = drm_live_source_cleanup,
>+};
>+
>+static const uint32_t source_formats[] = {
>+	DRM_FORMAT_XRGB8888,
>+};
>+
>+int rcar_du_vsp1_sources_init(struct rcar_du_device *rcdu)
>+{
>+	static const struct {
>+		enum rcar_du_plane_source source;
>+		unsigned int planes;
>+	} sources[] = {
>+		{ RCAR_DU_PLANE_VSPD0, BIT(RCAR_DU_NUM_KMS_PLANES - 1) },
>+		{ RCAR_DU_PLANE_VSPD1, BIT(RCAR_DU_NUM_KMS_PLANES - 2) |
>+				       BIT(2 * RCAR_DU_NUM_KMS_PLANES - 1) },
>+	};
>+	unsigned int planes_mask;
>+	unsigned int num_planes;
>+	unsigned int i;
>+
>+	num_planes = RCAR_DU_NUM_KMS_PLANES * DIV_ROUND_UP(rcdu->num_crtcs, 2);
>+	planes_mask = (1 << num_planes) - 1;
>+
>+	for (i = 0; i < ARRAY_SIZE(sources); ++i) {
>+		struct rcar_du_vsp1_source *src;
>+		char name[6];
>+		int ret;
>+
>+		src = devm_kzalloc(rcdu->dev, sizeof(*src), GFP_KERNEL);
>+		if (src == NULL)
>+			return -ENOMEM;
>+
>+		src->source = sources[i].source;
>+
>+		sprintf(name, "vspd%u", i);
>+		ret = drm_live_source_init(rcdu->ddev, &src->base, name,
>+					   sources[i].planes & planes_mask,
>+					   source_formats,
>+					   ARRAY_SIZE(source_formats),
>+					   &rcar_du_live_source_funcs);
>+		if (ret < 0)
>+			return ret;
>+	}
>+
>+	return 0;
>+}
>+
>+/* -----------------------------------------------------------------------------
>  * Atomic hardware plane allocator
>  *
>  * The hardware plane allocator is solely based on the atomic plane states
>@@ -555,6 +620,7 @@ static int rcar_du_plane_atomic_check(st
> 	struct rcar_du_plane_state *rstate = to_rcar_plane_state(state);
> 	struct rcar_du_plane *rplane = to_rcar_plane(plane);
> 	struct rcar_du_device *rcdu = rplane->group->dev;
>+	uint32_t pixel_format;
>
> 	if (!state->fb || !state->crtc) {
> 		rstate->format = NULL;
>@@ -567,13 +633,22 @@ static int rcar_du_plane_atomic_check(st
> 		return -EINVAL;
> 	}
>
>-	rstate->format = rcar_du_format_info(state->fb->format->format);
>+	pixel_format = state->fb->format->format;
>+	rstate->format = rcar_du_format_info(pixel_format);
> 	if (rstate->format == NULL) {
> 		dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__,
>-			state->fb->format->format);
>+			pixel_format);
> 		return -EINVAL;
> 	}
>
>+	if (state->fb->flags & DRM_MODE_FB_LIVE_SOURCE) {
>+		struct drm_live_source *src = rcar_du_live_fb_source(state->fb);

state->fb->src seems a bit neater and more in-line with the rest of
the atomic_state (taking userspace-provided flags etc. and turning it
into actual objects for the kernel to use), if you move the src
pointer to drm_framebuffer.

-Brian

>+
>+		rstate->source = to_rcar_vsp1_source(src)->source;
>+	} else {
>+		rstate->source = RCAR_DU_PLANE_MEMORY;
>+	}
>+
> 	return 0;
> }
>
>Index: linux/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>===================================================================
>--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>+++ linux/drivers/gpu/drm/rcar-du/rcar_du_plane.h
>@@ -17,6 +17,7 @@
> #include <drm/drmP.h>
> #include <drm/drm_crtc.h>
>
>+struct rcar_du_device;
> struct rcar_du_format_info;
> struct rcar_du_group;
>
>@@ -72,6 +73,8 @@ to_rcar_plane_state(struct drm_plane_sta
> int rcar_du_atomic_check_planes(struct drm_device *dev,
> 				struct drm_atomic_state *state);
>
>+int rcar_du_vsp1_sources_init(struct rcar_du_device *rcdu);
>+
> int rcar_du_planes_init(struct rcar_du_group *rgrp);
>
> void __rcar_du_plane_setup(struct rcar_du_group *rgrp,
>
>_______________________________________________
>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

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

end of thread, other threads:[~2017-01-31 15:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-25 21:33 [PATCH v3 0/3] Enable R8A7790/1 DU VSPD compositor Sergei Shtylyov
2017-01-25 21:35 ` [PATCH v3 1/3] drm: Add live source object Sergei Shtylyov
2017-01-31 15:41   ` Brian Starkey
2017-01-25 21:36 ` [PATCH v3 2/3] drm: Connect live source to framebuffers Sergei Shtylyov
2017-01-31 15:42   ` Brian Starkey
2017-01-25 21:38 ` [PATCH v3 3/3] drm: rcar-du: Add VSP1 live source support Sergei Shtylyov
2017-01-31 15:42   ` Brian Starkey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).