Den 24.05.2018 10.42, skrev Daniel Vetter:
On Wed, May 23, 2018 at 04:34:06PM +0200, Noralf Trønnes wrote:
This the beginning of an API for in-kernel clients.
First out is a way to get a framebuffer backed by a dumb buffer.
Only GEM drivers are supported.
The original idea of using an exported dma-buf was dropped because it
also creates an anonomous file descriptor which doesn't work when the
buffer is created from a kernel thread. The easy way out is to use
drm_driver.gem_prime_vmap to get the virtual address, which requires a
GEM object. This excludes the vmwgfx driver which is the only non-GEM
driver apart from the legacy ones. A solution for vmwgfx will have to be
worked out later if it wants to support the client API which it probably
will when we have a bootsplash client.
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
A few small nits below, with those addressed:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
[...]
+/**
+ * drm_client_new - Create a DRM client
+ * @dev: DRM device
+ *
+ * Returns:
+ * Pointer to a client or an error pointer on failure.
+ */
+struct drm_client_dev *drm_client_new(struct drm_device *dev)
Api nitpick:
int drm_client_init(struct drm_device *dev,
struct drm_client_dev *client)
and dropping the kzalloc from this structure here. This allows users of
this to embed the client struct into their own thing, which means the
->private backpointer isn't necessary. Allowing embedding is the preferred
interface in the kernel (since it's strictly more powerful, you can always
just kzalloc + _init to get the _new behaviour).
+{
+ struct drm_client_dev *client;
+ int ret;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
+ !dev->driver->dumb_create || !dev->driver->gem_prime_vmap)
+ return ERR_PTR(-ENOTSUPP);
+
+ client = kzalloc(sizeof(*client), GFP_KERNEL);
+ if (!client)
+ return ERR_PTR(-ENOMEM);
+
+ client->dev = dev;
+
+ ret = drm_client_alloc_file(client);
+ if (ret) {
+ kfree(client);
+ return ERR_PTR(ret);
+ }
+
+ return client;
+}
+EXPORT_SYMBOL(drm_client_new);
+
[...]
+static struct drm_client_buffer *
+drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
+{
+ struct drm_mode_create_dumb dumb_args = { };
+ struct drm_device *dev = client->dev;
+ struct drm_client_buffer *buffer;
+ struct drm_gem_object *obj;
+ void *vaddr;
+ int ret;
+
+ buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
+ if (!buffer)
+ return ERR_PTR(-ENOMEM);
+
+ buffer->client = client;
+
+ dumb_args.width = width;
+ dumb_args.height = height;
+ dumb_args.bpp = drm_format_plane_cpp(format, 0) * 8;
+ ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
+ if (ret)
+ goto err_free;
+
+ buffer->handle = dumb_args.handle;
+ buffer->pitch = dumb_args.pitch;
+
+ obj = drm_gem_object_lookup(client->file, dumb_args.handle);
+ if (!obj) {
+ ret = -ENOENT;
+ goto err_delete;
+ }
+
+ buffer->gem = obj;
+
I'm paranoid, I think an
if (WARN_ON(!gem_prime_vmap))
return -EINVAL;
would be cool here.
This is already checked in drm_client_init().
Noralf.
Also perhaps the following comment:
/*
* FIXME: The dependency on GEM here isn't required, we could
* convert the driver handle to a dma-buf instead and use the
* backend-agnostic dma-buf vmap support instead. This would
* require that the handle2fd prime ioctl is reworked to pull the
* fd_install step out of the driver backend hooks, to make that
* final step optional for internal users.
*/
+ vaddr = dev->driver->gem_prime_vmap(obj);
+ if (!vaddr) {
+ ret = -ENOMEM;
+ goto err_delete;
+ }
+
+ buffer->vaddr = vaddr;
+
+ return buffer;
+
+err_delete:
+ drm_client_buffer_delete(buffer);
+err_free:
+ kfree(buffer);
+
+ return ERR_PTR(ret);
+}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
