* [PATCH v5 1/8] drm: Begin an API for in-kernel clients
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 2/8] drm/fb-helper: Add generic fbdev emulation .fb_probe function Noralf Trønnes
` (13 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Noralf Trønnes, laurent.pinchart, david
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.
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Documentation/gpu/drm-client.rst | 12 ++
Documentation/gpu/index.rst | 1 +
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_client.c | 387 +++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_drv.c | 8 +
drivers/gpu/drm/drm_file.c | 3 +
drivers/gpu/drm/drm_probe_helper.c | 3 +
include/drm/drm_client.h | 136 +++++++++++++
include/drm/drm_device.h | 21 ++
9 files changed, 572 insertions(+), 1 deletion(-)
create mode 100644 Documentation/gpu/drm-client.rst
create mode 100644 drivers/gpu/drm/drm_client.c
create mode 100644 include/drm/drm_client.h
diff --git a/Documentation/gpu/drm-client.rst b/Documentation/gpu/drm-client.rst
new file mode 100644
index 000000000000..7e672063e7eb
--- /dev/null
+++ b/Documentation/gpu/drm-client.rst
@@ -0,0 +1,12 @@
+=================
+Kernel clients
+=================
+
+.. kernel-doc:: drivers/gpu/drm/drm_client.c
+ :doc: overview
+
+.. kernel-doc:: include/drm/drm_client.h
+ :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_client.c
+ :export:
diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
index 00288f34c5a6..1fcf8e851e15 100644
--- a/Documentation/gpu/index.rst
+++ b/Documentation/gpu/index.rst
@@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide
drm-kms
drm-kms-helpers
drm-uapi
+ drm-client
drivers
vga-switcheroo
vgaarbiter
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 69c13517ea3a..d6657a61d037 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,7 +18,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
- drm_syncobj.o drm_lease.o drm_writeback.o
+ drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o
drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
new file mode 100644
index 000000000000..743495f7f833
--- /dev/null
+++ b/drivers/gpu/drm/drm_client.c
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2018 Noralf Trønnes
+ */
+
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+
+#include <drm/drm_client.h>
+#include <drm/drm_debugfs.h>
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_file.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_mode.h>
+#include <drm/drm_print.h>
+#include <drm/drmP.h>
+
+#include "drm_crtc_internal.h"
+#include "drm_internal.h"
+
+/**
+ * DOC: overview
+ *
+ * This library provides support for clients running in the kernel like fbdev and bootsplash.
+ * Currently it's only partially implemented, just enough to support fbdev.
+ *
+ * GEM drivers which provide a GEM based dumb buffer with a virtual address are supported.
+ */
+
+static int drm_client_open(struct drm_client_dev *client)
+{
+ struct drm_device *dev = client->dev;
+ struct drm_file *file;
+
+ file = drm_file_alloc(dev->primary);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ mutex_lock(&dev->filelist_mutex);
+ list_add(&file->lhead, &dev->filelist_internal);
+ mutex_unlock(&dev->filelist_mutex);
+
+ client->file = file;
+
+ return 0;
+}
+
+static void drm_client_close(struct drm_client_dev *client)
+{
+ struct drm_device *dev = client->dev;
+
+ mutex_lock(&dev->filelist_mutex);
+ list_del(&client->file->lhead);
+ mutex_unlock(&dev->filelist_mutex);
+
+ drm_file_free(client->file);
+}
+EXPORT_SYMBOL(drm_client_close);
+
+/**
+ * drm_client_new - Create a DRM client
+ * @dev: DRM device
+ * @client: DRM client
+ * @name: Client name
+ * @funcs: DRM client functions (optional)
+ *
+ * The caller needs to hold a reference on @dev before calling this function.
+ * The client is freed when the &drm_device is unregistered. See drm_client_release().
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
+ const char *name, const struct drm_client_funcs *funcs)
+{
+ bool registered;
+ int ret;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
+ !dev->driver->dumb_create || !dev->driver->gem_prime_vmap)
+ return -ENOTSUPP;
+
+ if (funcs && !try_module_get(funcs->owner))
+ return -ENODEV;
+
+ client->dev = dev;
+ client->name = name;
+ client->funcs = funcs;
+
+ ret = drm_client_open(client);
+ if (ret)
+ goto err_put_module;
+
+ mutex_lock(&dev->clientlist_mutex);
+ registered = dev->registered;
+ if (registered)
+ list_add(&client->list, &dev->clientlist);
+ mutex_unlock(&dev->clientlist_mutex);
+ if (!registered) {
+ ret = -ENODEV;
+ goto err_close;
+ }
+
+ drm_dev_get(dev);
+
+ return 0;
+
+err_close:
+ drm_client_close(client);
+err_put_module:
+ if (funcs)
+ module_put(funcs->owner);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_client_new);
+
+/**
+ * drm_client_release - Release DRM client resources
+ * @client: DRM client
+ *
+ * Releases resources by closing the &drm_file that was opened by drm_client_new().
+ * It is called automatically if the &drm_client_funcs.unregister callback is _not_ set.
+ *
+ * This function should only be called from the unregister callback. An exception
+ * is fbdev which cannot free the buffer if userspace has open file descriptors.
+ *
+ * Note:
+ * Clients cannot initiate a release by themselves. This is done to keep the code simple.
+ * The driver has to be unloaded before the client can be unloaded.
+ */
+void drm_client_release(struct drm_client_dev *client)
+{
+ struct drm_device *dev = client->dev;
+
+ DRM_DEV_DEBUG_KMS(dev->dev, "%s\n", client->name);
+
+ drm_client_close(client);
+ drm_dev_put(dev);
+ if (client->funcs)
+ module_put(client->funcs->owner);
+}
+EXPORT_SYMBOL(drm_client_release);
+
+void drm_client_dev_unregister(struct drm_device *dev)
+{
+ struct drm_client_dev *client, *tmp;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return;
+
+ mutex_lock(&dev->clientlist_mutex);
+ list_for_each_entry_safe(client, tmp, &dev->clientlist, list) {
+ list_del(&client->list);
+ if (client->funcs && client->funcs->unregister) {
+ client->funcs->unregister(client);
+ } else {
+ drm_client_release(client);
+ kfree(client);
+ }
+ }
+ mutex_unlock(&dev->clientlist_mutex);
+}
+
+/**
+ * drm_client_dev_hotplug - Send hotplug event to clients
+ * @dev: DRM device
+ *
+ * This function calls the &drm_client_funcs.hotplug callback on the attached clients.
+ *
+ * drm_kms_helper_hotplug_event() calls this function, so drivers that use it
+ * don't need to call this function themselves.
+ */
+void drm_client_dev_hotplug(struct drm_device *dev)
+{
+ struct drm_client_dev *client;
+ int ret;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return;
+
+ mutex_lock(&dev->clientlist_mutex);
+ list_for_each_entry(client, &dev->clientlist, list) {
+ if (!client->funcs || !client->funcs->hotplug)
+ continue;
+
+ ret = client->funcs->hotplug(client);
+ DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret);
+ }
+ mutex_unlock(&dev->clientlist_mutex);
+}
+EXPORT_SYMBOL(drm_client_dev_hotplug);
+
+void drm_client_dev_restore(struct drm_device *dev)
+{
+ struct drm_client_dev *client;
+ int ret;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return;
+
+ mutex_lock(&dev->clientlist_mutex);
+ list_for_each_entry(client, &dev->clientlist, list) {
+ if (!client->funcs || !client->funcs->restore)
+ continue;
+
+ ret = client->funcs->restore(client);
+ DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret);
+ if (!ret) /* The first one to return zero gets the privilege to restore */
+ break;
+ }
+ mutex_unlock(&dev->clientlist_mutex);
+}
+
+static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
+{
+ struct drm_device *dev = buffer->client->dev;
+
+ if (buffer->vaddr && dev->driver->gem_prime_vunmap)
+ dev->driver->gem_prime_vunmap(buffer->gem, buffer->vaddr);
+
+ if (buffer->gem)
+ drm_gem_object_put_unlocked(buffer->gem);
+
+ drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);
+ kfree(buffer);
+}
+
+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;
+
+ /*
+ * 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);
+}
+
+static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
+{
+ int ret;
+
+ if (!buffer->fb)
+ return;
+
+ ret = drm_mode_rmfb(buffer->client->dev, buffer->fb->base.id, buffer->client->file);
+ if (ret)
+ DRM_DEV_ERROR(buffer->client->dev->dev,
+ "Error removing FB:%u (%d)\n", buffer->fb->base.id, ret);
+
+ buffer->fb = NULL;
+}
+
+static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
+ u32 width, u32 height, u32 format)
+{
+ struct drm_client_dev *client = buffer->client;
+ struct drm_mode_fb_cmd fb_req = { };
+ const struct drm_format_info *info;
+ int ret;
+
+ info = drm_format_info(format);
+ fb_req.bpp = info->cpp[0] * 8;
+ fb_req.depth = info->depth;
+ fb_req.width = width;
+ fb_req.height = height;
+ fb_req.handle = buffer->handle;
+ fb_req.pitch = buffer->pitch;
+
+ ret = drm_mode_addfb(client->dev, &fb_req, client->file);
+ if (ret)
+ return ret;
+
+ buffer->fb = drm_framebuffer_lookup(client->dev, buffer->client->file, fb_req.fb_id);
+ if (WARN_ON(!buffer->fb))
+ return -ENOENT;
+
+ /* drop the reference we picked up in framebuffer lookup */
+ drm_framebuffer_put(buffer->fb);
+
+ strscpy(buffer->fb->comm, client->name, TASK_COMM_LEN);
+
+ return 0;
+}
+
+/**
+ * drm_client_framebuffer_create - Create a client framebuffer
+ * @client: DRM client
+ * @width: Framebuffer width
+ * @height: Framebuffer height
+ * @format: Buffer format
+ *
+ * This function creates a &drm_client_buffer which consists of a
+ * &drm_framebuffer backed by a dumb buffer.
+ * Call drm_client_framebuffer_delete() to free the buffer.
+ *
+ * Returns:
+ * Pointer to a client buffer or an error pointer on failure.
+ */
+struct drm_client_buffer *
+drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
+{
+ struct drm_client_buffer *buffer;
+ int ret;
+
+ buffer = drm_client_buffer_create(client, width, height, format);
+ if (IS_ERR(buffer))
+ return buffer;
+
+ ret = drm_client_buffer_addfb(buffer, width, height, format);
+ if (ret) {
+ drm_client_buffer_delete(buffer);
+ return ERR_PTR(ret);
+ }
+
+ return buffer;
+}
+EXPORT_SYMBOL(drm_client_framebuffer_create);
+
+/**
+ * drm_client_framebuffer_delete - Delete a client framebuffer
+ * @buffer: DRM client buffer (can be NULL)
+ */
+void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
+{
+ if (!buffer)
+ return;
+
+ drm_client_buffer_rmfb(buffer);
+ drm_client_buffer_delete(buffer);
+}
+EXPORT_SYMBOL(drm_client_framebuffer_delete);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7af748ed1c58..6eb935bb2f92 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -34,6 +34,7 @@
#include <linux/slab.h>
#include <linux/srcu.h>
+#include <drm/drm_client.h>
#include <drm/drm_drv.h>
#include <drm/drmP.h>
@@ -505,6 +506,8 @@ int drm_dev_init(struct drm_device *dev,
dev->driver = driver;
INIT_LIST_HEAD(&dev->filelist);
+ INIT_LIST_HEAD(&dev->filelist_internal);
+ INIT_LIST_HEAD(&dev->clientlist);
INIT_LIST_HEAD(&dev->ctxlist);
INIT_LIST_HEAD(&dev->vmalist);
INIT_LIST_HEAD(&dev->maplist);
@@ -514,6 +517,7 @@ int drm_dev_init(struct drm_device *dev,
spin_lock_init(&dev->event_lock);
mutex_init(&dev->struct_mutex);
mutex_init(&dev->filelist_mutex);
+ mutex_init(&dev->clientlist_mutex);
mutex_init(&dev->ctxlist_mutex);
mutex_init(&dev->master_mutex);
@@ -569,6 +573,7 @@ int drm_dev_init(struct drm_device *dev,
err_free:
mutex_destroy(&dev->master_mutex);
mutex_destroy(&dev->ctxlist_mutex);
+ mutex_destroy(&dev->clientlist_mutex);
mutex_destroy(&dev->filelist_mutex);
mutex_destroy(&dev->struct_mutex);
return ret;
@@ -603,6 +608,7 @@ void drm_dev_fini(struct drm_device *dev)
mutex_destroy(&dev->master_mutex);
mutex_destroy(&dev->ctxlist_mutex);
+ mutex_destroy(&dev->clientlist_mutex);
mutex_destroy(&dev->filelist_mutex);
mutex_destroy(&dev->struct_mutex);
kfree(dev->unique);
@@ -858,6 +864,8 @@ void drm_dev_unregister(struct drm_device *dev)
dev->registered = false;
+ drm_client_dev_unregister(dev);
+
if (drm_core_check_feature(dev, DRIVER_MODESET))
drm_modeset_unregister_all(dev);
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 66bb403dc8ab..ffa8dc35515f 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -35,6 +35,7 @@
#include <linux/slab.h>
#include <linux/module.h>
+#include <drm/drm_client.h>
#include <drm/drm_file.h>
#include <drm/drmP.h>
@@ -444,6 +445,8 @@ void drm_lastclose(struct drm_device * dev)
if (drm_core_check_feature(dev, DRIVER_LEGACY))
drm_legacy_dev_reinit(dev);
+
+ drm_client_dev_restore(dev);
}
/**
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 527743394150..26be57e28a9d 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -33,6 +33,7 @@
#include <linux/moduleparam.h>
#include <drm/drmP.h>
+#include <drm/drm_client.h>
#include <drm/drm_crtc.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_crtc_helper.h>
@@ -563,6 +564,8 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev)
drm_sysfs_hotplug_event(dev);
if (dev->mode_config.funcs->output_poll_changed)
dev->mode_config.funcs->output_poll_changed(dev);
+
+ drm_client_dev_hotplug(dev);
}
EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
new file mode 100644
index 000000000000..671052d80e38
--- /dev/null
+++ b/include/drm/drm_client.h
@@ -0,0 +1,136 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DRM_CLIENT_H_
+#define _DRM_CLIENT_H_
+
+#include <linux/types.h>
+
+struct drm_client_dev;
+struct drm_device;
+struct drm_file;
+struct drm_framebuffer;
+struct drm_gem_object;
+struct module;
+
+/**
+ * struct drm_client_funcs - DRM client callbacks
+ */
+struct drm_client_funcs {
+ /**
+ * @owner: The module owner
+ */
+ struct module *owner;
+
+ /**
+ * @unregister:
+ *
+ * Called when &drm_device is unregistered. The client should respond by
+ * releasing it's resources using drm_client_release().
+ *
+ * This callback is optional.
+ */
+ void (*unregister)(struct drm_client_dev *client);
+
+ /**
+ * @restore:
+ *
+ * Called on drm_lastclose(). The first client instance in the list that
+ * returns zero gets the privilege to restore and no more clients are
+ * called. This callback is not called after @unregister has been called.
+ *
+ * This callback is optional.
+ */
+ int (*restore)(struct drm_client_dev *client);
+
+ /**
+ * @hotplug:
+ *
+ * Called on drm_kms_helper_hotplug_event().
+ * This callback is not called after @unregister has been called.
+ *
+ * This callback is optional.
+ */
+ int (*hotplug)(struct drm_client_dev *client);
+};
+
+/**
+ * struct drm_client_dev - DRM client instance
+ */
+struct drm_client_dev {
+ /**
+ * @dev: DRM device
+ */
+ struct drm_device *dev;
+
+ /**
+ * @name: Name of the client.
+ */
+ const char *name;
+
+ /**
+ * @list:
+ *
+ * List of all clients of a DRM device, linked into
+ * &drm_device.clientlist. Protected by &drm_device.clientlist_mutex.
+ */
+ struct list_head list;
+
+ /**
+ * @funcs: DRM client functions (optional)
+ */
+ const struct drm_client_funcs *funcs;
+
+ /**
+ * @file: DRM file
+ */
+ struct drm_file *file;
+};
+
+int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
+ const char *name, const struct drm_client_funcs *funcs);
+void drm_client_release(struct drm_client_dev *client);
+
+void drm_client_dev_unregister(struct drm_device *dev);
+void drm_client_dev_hotplug(struct drm_device *dev);
+void drm_client_dev_restore(struct drm_device *dev);
+
+/**
+ * struct drm_client_buffer - DRM client buffer
+ */
+struct drm_client_buffer {
+ /**
+ * @client: DRM client
+ */
+ struct drm_client_dev *client;
+
+ /**
+ * @handle: Buffer handle
+ */
+ u32 handle;
+
+ /**
+ * @pitch: Buffer pitch
+ */
+ u32 pitch;
+
+ /**
+ * @gem: GEM object backing this buffer
+ */
+ struct drm_gem_object *gem;
+
+ /**
+ * @vaddr: Virtual address for the buffer
+ */
+ void *vaddr;
+
+ /**
+ * @fb: DRM framebuffer
+ */
+ struct drm_framebuffer *fb;
+};
+
+struct drm_client_buffer *
+drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
+void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
+
+#endif
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 858ba19a3e29..f9c6e0e3aec7 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -74,6 +74,27 @@ struct drm_device {
struct mutex filelist_mutex;
struct list_head filelist;
+ /**
+ * @filelist_internal:
+ *
+ * List of open DRM files for in-kernel clients. Protected by @filelist_mutex.
+ */
+ struct list_head filelist_internal;
+
+ /**
+ * @clientlist_mutex:
+ *
+ * Protects @clientlist access.
+ */
+ struct mutex clientlist_mutex;
+
+ /**
+ * @clientlist:
+ *
+ * List of in-kernel clients. Protected by @clientlist_mutex.
+ */
+ struct list_head clientlist;
+
/** \name Memory management */
/*@{ */
struct list_head maplist; /**< Linked list of regions */
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v5 2/8] drm/fb-helper: Add generic fbdev emulation .fb_probe function
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 1/8] drm: Begin an API for in-kernel clients Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 3/8] drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap Noralf Trønnes
` (12 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, laurent.pinchart, david
This is the first step in getting generic fbdev emulation.
A drm_fb_helper_funcs.fb_probe function is added which uses the
DRM client API to get a framebuffer backed by a dumb buffer.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_fb_helper.c | 199 +++++++++++++++++++++++++++++++++++++++-
include/drm/drm_fb_helper.h | 31 +++++++
2 files changed, 229 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index cab14f253384..bea3a0cb324a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -30,6 +30,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/console.h>
+#include <linux/dma-buf.h>
#include <linux/kernel.h>
#include <linux/sysrq.h>
#include <linux/slab.h>
@@ -738,6 +739,24 @@ static void drm_fb_helper_resume_worker(struct work_struct *work)
console_unlock();
}
+static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
+ struct drm_clip_rect *clip)
+{
+ struct drm_framebuffer *fb = fb_helper->fb;
+ unsigned int cpp = drm_format_plane_cpp(fb->format->format, 0);
+ size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
+ void *src = fb_helper->fbdev->screen_buffer + offset;
+ void *dst = fb_helper->buffer->vaddr + offset;
+ size_t len = (clip->x2 - clip->x1) * cpp;
+ unsigned int y;
+
+ for (y = clip->y1; y < clip->y2; y++) {
+ memcpy(dst, src, len);
+ src += fb->pitches[0];
+ dst += fb->pitches[0];
+ }
+}
+
static void drm_fb_helper_dirty_work(struct work_struct *work)
{
struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
@@ -753,8 +772,12 @@ static void drm_fb_helper_dirty_work(struct work_struct *work)
spin_unlock_irqrestore(&helper->dirty_lock, flags);
/* call dirty callback only when it has been really touched */
- if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2)
+ if (clip_copy.x1 < clip_copy.x2 && clip_copy.y1 < clip_copy.y2) {
+ /* Generic fbdev uses a shadow buffer */
+ if (helper->buffer)
+ drm_fb_helper_dirty_blit_real(helper, &clip_copy);
helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1);
+ }
}
/**
@@ -2921,6 +2944,180 @@ void drm_fb_helper_output_poll_changed(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
+/* @user: 1=userspace, 0=fbcon */
+static int drm_fbdev_fb_open(struct fb_info *info, int user)
+{
+ struct drm_fb_helper *fb_helper = info->par;
+
+ if (!try_module_get(fb_helper->dev->driver->fops->owner))
+ return -ENODEV;
+
+ return 0;
+}
+
+static int drm_fbdev_fb_release(struct fb_info *info, int user)
+{
+ struct drm_fb_helper *fb_helper = info->par;
+
+ module_put(fb_helper->dev->driver->fops->owner);
+
+ return 0;
+}
+
+/*
+ * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of
+ * unregister_framebuffer() or fb_release().
+ */
+static void drm_fbdev_fb_destroy(struct fb_info *info)
+{
+ struct drm_fb_helper *fb_helper = info->par;
+ struct fb_info *fbi = fb_helper->fbdev;
+ struct fb_ops *fbops = NULL;
+ void *shadow = NULL;
+
+ if (fbi->fbdefio) {
+ fb_deferred_io_cleanup(fbi);
+ shadow = fbi->screen_buffer;
+ fbops = fbi->fbops;
+ }
+
+ drm_fb_helper_fini(fb_helper);
+
+ if (shadow) {
+ vfree(shadow);
+ kfree(fbops);
+ }
+
+ drm_client_framebuffer_delete(fb_helper->buffer);
+ /*
+ * FIXME:
+ * Remove conditional when all CMA drivers have been moved over to using
+ * drm_fbdev_generic_setup().
+ */
+ if (fb_helper->client.funcs) {
+ drm_client_release(&fb_helper->client);
+ kfree(fb_helper);
+ }
+}
+
+static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+ struct drm_fb_helper *fb_helper = info->par;
+
+ if (fb_helper->dev->driver->gem_prime_mmap)
+ return fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma);
+ else
+ return -ENODEV;
+}
+
+static struct fb_ops drm_fbdev_fb_ops = {
+ .owner = THIS_MODULE,
+ DRM_FB_HELPER_DEFAULT_OPS,
+ .fb_open = drm_fbdev_fb_open,
+ .fb_release = drm_fbdev_fb_release,
+ .fb_destroy = drm_fbdev_fb_destroy,
+ .fb_mmap = drm_fbdev_fb_mmap,
+ .fb_read = drm_fb_helper_sys_read,
+ .fb_write = drm_fb_helper_sys_write,
+ .fb_fillrect = drm_fb_helper_sys_fillrect,
+ .fb_copyarea = drm_fb_helper_sys_copyarea,
+ .fb_imageblit = drm_fb_helper_sys_imageblit,
+};
+
+static struct fb_deferred_io drm_fbdev_defio = {
+ .delay = HZ / 20,
+ .deferred_io = drm_fb_helper_deferred_io,
+};
+
+/**
+ * drm_fb_helper_generic_probe - Generic fbdev emulation probe helper
+ * @fb_helper: fbdev helper structure
+ * @sizes: describes fbdev size and scanout surface size
+ *
+ * This function uses the client API to crate a framebuffer backed by a dumb buffer.
+ *
+ * The _sys_ versions are used for &fb_ops.fb_read, fb_write, fb_fillrect,
+ * fb_copyarea, fb_imageblit.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
+ struct drm_fb_helper_surface_size *sizes)
+{
+ struct drm_client_dev *client = &fb_helper->client;
+ struct drm_client_buffer *buffer;
+ struct drm_framebuffer *fb;
+ struct fb_info *fbi;
+ u32 format;
+ int ret;
+
+ DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
+ sizes->surface_width, sizes->surface_height,
+ sizes->surface_bpp);
+
+ format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
+ buffer = drm_client_framebuffer_create(client, sizes->surface_width,
+ sizes->surface_height, format);
+ if (IS_ERR(buffer))
+ return PTR_ERR(buffer);
+
+ fb_helper->buffer = buffer;
+ fb_helper->fb = buffer->fb;
+ fb = buffer->fb;
+
+ fbi = drm_fb_helper_alloc_fbi(fb_helper);
+ if (IS_ERR(fbi)) {
+ ret = PTR_ERR(fbi);
+ goto err_free_buffer;
+ }
+
+ fbi->par = fb_helper;
+ fbi->fbops = &drm_fbdev_fb_ops;
+ fbi->screen_size = fb->height * fb->pitches[0];
+ fbi->fix.smem_len = fbi->screen_size;
+ fbi->screen_buffer = buffer->vaddr;
+ strcpy(fbi->fix.id, "DRM emulated");
+
+ drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
+ drm_fb_helper_fill_var(fbi, fb_helper, sizes->fb_width, sizes->fb_height);
+
+ if (fb->funcs->dirty) {
+ struct fb_ops *fbops;
+ void *shadow;
+
+ /*
+ * fb_deferred_io_cleanup() clears &fbops->fb_mmap so a per
+ * instance version is necessary.
+ */
+ fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
+ shadow = vzalloc(fbi->screen_size);
+ if (!fbops || !shadow) {
+ kfree(fbops);
+ vfree(shadow);
+ ret = -ENOMEM;
+ goto err_fb_info_destroy;
+ }
+
+ *fbops = *fbi->fbops;
+ fbi->fbops = fbops;
+ fbi->screen_buffer = shadow;
+ fbi->fbdefio = &drm_fbdev_defio;
+
+ fb_deferred_io_init(fbi);
+ }
+
+ return 0;
+
+err_fb_info_destroy:
+ drm_fb_helper_fini(fb_helper);
+err_free_buffer:
+ drm_client_framebuffer_delete(buffer);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_fb_helper_generic_probe);
+
/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
* but the module doesn't depend on any fb console symbols. At least
* attempt to load fbcon to avoid leaving the system without a usable console.
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index b069433e7fc1..c134bbcfd2e9 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -32,6 +32,7 @@
struct drm_fb_helper;
+#include <drm/drm_client.h>
#include <drm/drm_crtc.h>
#include <drm/drm_device.h>
#include <linux/kgdb.h>
@@ -154,6 +155,20 @@ struct drm_fb_helper_connector {
* operations.
*/
struct drm_fb_helper {
+ /**
+ * @client:
+ *
+ * DRM client used by the generic fbdev emulation.
+ */
+ struct drm_client_dev client;
+
+ /**
+ * @buffer:
+ *
+ * Framebuffer used by the generic fbdev emulation.
+ */
+ struct drm_client_buffer *buffer;
+
struct drm_framebuffer *fb;
struct drm_device *dev;
int crtc_count;
@@ -234,6 +249,12 @@ struct drm_fb_helper {
int preferred_bpp;
};
+static inline struct drm_fb_helper *
+drm_fb_helper_from_client(struct drm_client_dev *client)
+{
+ return container_of(client, struct drm_fb_helper, client);
+}
+
/**
* define DRM_FB_HELPER_DEFAULT_OPS - helper define for drm drivers
*
@@ -330,6 +351,9 @@ void drm_fb_helper_fbdev_teardown(struct drm_device *dev);
void drm_fb_helper_lastclose(struct drm_device *dev);
void drm_fb_helper_output_poll_changed(struct drm_device *dev);
+
+int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
+ struct drm_fb_helper_surface_size *sizes);
#else
static inline void drm_fb_helper_prepare(struct drm_device *dev,
struct drm_fb_helper *helper,
@@ -564,6 +588,13 @@ static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
{
}
+static inline int
+drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
+ struct drm_fb_helper_surface_size *sizes)
+{
+ return 0;
+}
+
#endif
static inline int
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v5 3/8] drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 1/8] drm: Begin an API for in-kernel clients Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 2/8] drm/fb-helper: Add generic fbdev emulation .fb_probe function Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation Noralf Trønnes
` (11 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Noralf Trønnes, laurent.pinchart, david
These are needed for pl111 to use the generic fbdev emulation.
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
drivers/gpu/drm/pl111/pl111_drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 054b93689d94..17a38e85ba7d 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -250,6 +250,8 @@ static struct drm_driver pl111_drm_driver = {
.gem_prime_import_sg_table = pl111_gem_import_sg_table,
.gem_prime_export = drm_gem_prime_export,
.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
+ .gem_prime_mmap = drm_gem_cma_prime_mmap,
+ .gem_prime_vmap = drm_gem_cma_prime_vmap,
#if defined(CONFIG_DEBUG_FS)
.debugfs_init = pl111_debugfs_init,
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (2 preceding siblings ...)
2018-07-03 16:03 ` [PATCH v5 3/8] drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-08-21 6:44 ` John Stultz
2018-07-03 16:03 ` [PATCH v5 5/8] drm/debugfs: Add internal client debugfs file Noralf Trønnes
` (10 subsequent siblings)
14 siblings, 1 reply; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Noralf Trønnes, laurent.pinchart, david
This switches the CMA helper drivers that use its fbdev emulation over
to the generic fbdev emulation. It's the first phase of using generic
fbdev. A later phase will use DRM client callbacks for the
lastclose/hotplug/remove callbacks.
There are currently 2 fbdev init/fini functions:
- drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
- drm_fbdev_cma_init/drm_fbdev_cma_fini
This is because the work on generic fbdev came up during a fbdev
refactoring and thus wasn't completed. No point in completing that
refactoring when drivers will soon move to drm_fb_helper_generic_probe().
tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
include/drm/drm_fb_cma_helper.h | 3 -
2 files changed, 42 insertions(+), 321 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 186d00adfb5f..718c7f961d8a 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -18,6 +18,7 @@
*/
#include <drm/drmP.h>
+#include <drm/drm_client.h>
#include <drm/drm_fb_helper.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_gem_cma_helper.h>
@@ -26,11 +27,8 @@
#include <drm/drm_print.h>
#include <linux/module.h>
-#define DEFAULT_FBDEFIO_DELAY_MS 50
-
struct drm_fbdev_cma {
struct drm_fb_helper fb_helper;
- const struct drm_framebuffer_funcs *fb_funcs;
};
/**
@@ -44,36 +42,6 @@ struct drm_fbdev_cma {
*
* An fbdev framebuffer backed by cma is also available by calling
* drm_fb_cma_fbdev_init(). drm_fb_cma_fbdev_fini() tears it down.
- * If the &drm_framebuffer_funcs.dirty callback is set, fb_deferred_io will be
- * set up automatically. &drm_framebuffer_funcs.dirty is called by
- * drm_fb_helper_deferred_io() in process context (&struct delayed_work).
- *
- * Example fbdev deferred io code::
- *
- * static int driver_fb_dirty(struct drm_framebuffer *fb,
- * struct drm_file *file_priv,
- * unsigned flags, unsigned color,
- * struct drm_clip_rect *clips,
- * unsigned num_clips)
- * {
- * struct drm_gem_cma_object *cma = drm_fb_cma_get_gem_obj(fb, 0);
- * ... push changes ...
- * return 0;
- * }
- *
- * static struct drm_framebuffer_funcs driver_fb_funcs = {
- * .destroy = drm_gem_fb_destroy,
- * .create_handle = drm_gem_fb_create_handle,
- * .dirty = driver_fb_dirty,
- * };
- *
- * Initialize::
- *
- * fbdev = drm_fb_cma_fbdev_init_with_funcs(dev, 16,
- * dev->mode_config.num_crtc,
- * dev->mode_config.num_connector,
- * &driver_fb_funcs);
- *
*/
static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
@@ -131,153 +99,6 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
}
EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
-static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma)
-{
- return dma_mmap_writecombine(info->device, vma, info->screen_base,
- info->fix.smem_start, info->fix.smem_len);
-}
-
-static struct fb_ops drm_fbdev_cma_ops = {
- .owner = THIS_MODULE,
- DRM_FB_HELPER_DEFAULT_OPS,
- .fb_fillrect = drm_fb_helper_sys_fillrect,
- .fb_copyarea = drm_fb_helper_sys_copyarea,
- .fb_imageblit = drm_fb_helper_sys_imageblit,
- .fb_mmap = drm_fb_cma_mmap,
-};
-
-static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
- struct vm_area_struct *vma)
-{
- fb_deferred_io_mmap(info, vma);
- vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
-
- return 0;
-}
-
-static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
- struct drm_gem_cma_object *cma_obj)
-{
- struct fb_deferred_io *fbdefio;
- struct fb_ops *fbops;
-
- /*
- * Per device structures are needed because:
- * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
- * fbdefio: individual delays
- */
- fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
- fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
- if (!fbdefio || !fbops) {
- kfree(fbdefio);
- kfree(fbops);
- return -ENOMEM;
- }
-
- /* can't be offset from vaddr since dirty() uses cma_obj */
- fbi->screen_buffer = cma_obj->vaddr;
- /* fb_deferred_io_fault() needs a physical address */
- fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
-
- *fbops = *fbi->fbops;
- fbi->fbops = fbops;
-
- fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
- fbdefio->deferred_io = drm_fb_helper_deferred_io;
- fbi->fbdefio = fbdefio;
- fb_deferred_io_init(fbi);
- fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
-
- return 0;
-}
-
-static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
-{
- if (!fbi->fbdefio)
- return;
-
- fb_deferred_io_cleanup(fbi);
- kfree(fbi->fbdefio);
- kfree(fbi->fbops);
-}
-
-static int
-drm_fbdev_cma_create(struct drm_fb_helper *helper,
- struct drm_fb_helper_surface_size *sizes)
-{
- struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
- struct drm_device *dev = helper->dev;
- struct drm_gem_cma_object *obj;
- struct drm_framebuffer *fb;
- unsigned int bytes_per_pixel;
- unsigned long offset;
- struct fb_info *fbi;
- size_t size;
- int ret;
-
- DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
- sizes->surface_width, sizes->surface_height,
- sizes->surface_bpp);
-
- bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
- size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
- obj = drm_gem_cma_create(dev, size);
- if (IS_ERR(obj))
- return -ENOMEM;
-
- fbi = drm_fb_helper_alloc_fbi(helper);
- if (IS_ERR(fbi)) {
- ret = PTR_ERR(fbi);
- goto err_gem_free_object;
- }
-
- fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
- fbdev_cma->fb_funcs);
- if (IS_ERR(fb)) {
- dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
- ret = PTR_ERR(fb);
- goto err_fb_info_destroy;
- }
-
- helper->fb = fb;
-
- fbi->par = helper;
- fbi->flags = FBINFO_FLAG_DEFAULT;
- fbi->fbops = &drm_fbdev_cma_ops;
-
- drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
- drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
-
- offset = fbi->var.xoffset * bytes_per_pixel;
- offset += fbi->var.yoffset * fb->pitches[0];
-
- dev->mode_config.fb_base = (resource_size_t)obj->paddr;
- fbi->screen_base = obj->vaddr + offset;
- fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
- fbi->screen_size = size;
- fbi->fix.smem_len = size;
-
- if (fb->funcs->dirty) {
- ret = drm_fbdev_cma_defio_init(fbi, obj);
- if (ret)
- goto err_cma_destroy;
- }
-
- return 0;
-
-err_cma_destroy:
- drm_framebuffer_remove(fb);
-err_fb_info_destroy:
- drm_fb_helper_fini(helper);
-err_gem_free_object:
- drm_gem_object_put_unlocked(&obj->base);
- return ret;
-}
-
-static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
- .fb_probe = drm_fbdev_cma_create,
-};
-
/**
* drm_fb_cma_fbdev_init_with_funcs() - Allocate and initialize fbdev emulation
* @dev: DRM device
@@ -295,53 +116,7 @@ int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
unsigned int preferred_bpp, unsigned int max_conn_count,
const struct drm_framebuffer_funcs *funcs)
{
- struct drm_fbdev_cma *fbdev_cma;
- struct drm_fb_helper *fb_helper;
- int ret;
-
- if (!preferred_bpp)
- preferred_bpp = dev->mode_config.preferred_depth;
- if (!preferred_bpp)
- preferred_bpp = 32;
-
- if (!max_conn_count)
- max_conn_count = dev->mode_config.num_connector;
-
- fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
- if (!fbdev_cma)
- return -ENOMEM;
-
- fbdev_cma->fb_funcs = funcs;
- fb_helper = &fbdev_cma->fb_helper;
-
- drm_fb_helper_prepare(dev, fb_helper, &drm_fb_cma_helper_funcs);
-
- ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
- if (ret < 0) {
- DRM_DEV_ERROR(dev->dev, "Failed to initialize fbdev helper.\n");
- goto err_free;
- }
-
- ret = drm_fb_helper_single_add_all_connectors(fb_helper);
- if (ret < 0) {
- DRM_DEV_ERROR(dev->dev, "Failed to add connectors.\n");
- goto err_drm_fb_helper_fini;
- }
-
- ret = drm_fb_helper_initial_config(fb_helper, preferred_bpp);
- if (ret < 0) {
- DRM_DEV_ERROR(dev->dev, "Failed to set fbdev configuration.\n");
- goto err_drm_fb_helper_fini;
- }
-
- return 0;
-
-err_drm_fb_helper_fini:
- drm_fb_helper_fini(fb_helper);
-err_free:
- kfree(fbdev_cma);
-
- return ret;
+ return drm_fb_cma_fbdev_init(dev, preferred_bpp, max_conn_count);
}
EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init_with_funcs);
@@ -359,8 +134,14 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init_with_funcs);
int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
unsigned int max_conn_count)
{
- return drm_fb_cma_fbdev_init_with_funcs(dev, preferred_bpp,
- max_conn_count, NULL);
+ struct drm_fbdev_cma *fbdev_cma;
+
+ /* dev->fb_helper will indirectly point to fbdev_cma after this call */
+ fbdev_cma = drm_fbdev_cma_init(dev, preferred_bpp, max_conn_count);
+ if (IS_ERR(fbdev_cma))
+ return PTR_ERR(fbdev_cma);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init);
@@ -370,87 +151,13 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init);
*/
void drm_fb_cma_fbdev_fini(struct drm_device *dev)
{
- struct drm_fb_helper *fb_helper = dev->fb_helper;
-
- if (!fb_helper)
- return;
-
- /* Unregister if it hasn't been done already */
- if (fb_helper->fbdev && fb_helper->fbdev->dev)
- drm_fb_helper_unregister_fbi(fb_helper);
-
- if (fb_helper->fbdev)
- drm_fbdev_cma_defio_fini(fb_helper->fbdev);
-
- if (fb_helper->fb)
- drm_framebuffer_remove(fb_helper->fb);
-
- drm_fb_helper_fini(fb_helper);
- kfree(to_fbdev_cma(fb_helper));
+ if (dev->fb_helper)
+ drm_fbdev_cma_fini(to_fbdev_cma(dev->fb_helper));
}
EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_fini);
-/**
- * drm_fbdev_cma_init_with_funcs() - Allocate and initializes a drm_fbdev_cma struct
- * @dev: DRM device
- * @preferred_bpp: Preferred bits per pixel for the device
- * @max_conn_count: Maximum number of connectors
- * @funcs: fb helper functions, in particular a custom dirty() callback
- *
- * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
- */
-struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
- unsigned int preferred_bpp, unsigned int max_conn_count,
- const struct drm_framebuffer_funcs *funcs)
-{
- struct drm_fbdev_cma *fbdev_cma;
- struct drm_fb_helper *helper;
- int ret;
-
- fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
- if (!fbdev_cma) {
- dev_err(dev->dev, "Failed to allocate drm fbdev.\n");
- return ERR_PTR(-ENOMEM);
- }
- fbdev_cma->fb_funcs = funcs;
-
- helper = &fbdev_cma->fb_helper;
-
- drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs);
-
- ret = drm_fb_helper_init(dev, helper, max_conn_count);
- if (ret < 0) {
- dev_err(dev->dev, "Failed to initialize drm fb helper.\n");
- goto err_free;
- }
-
- ret = drm_fb_helper_single_add_all_connectors(helper);
- if (ret < 0) {
- dev_err(dev->dev, "Failed to add connectors.\n");
- goto err_drm_fb_helper_fini;
-
- }
-
- ret = drm_fb_helper_initial_config(helper, preferred_bpp);
- if (ret < 0) {
- dev_err(dev->dev, "Failed to set initial hw configuration.\n");
- goto err_drm_fb_helper_fini;
- }
-
- return fbdev_cma;
-
-err_drm_fb_helper_fini:
- drm_fb_helper_fini(helper);
-err_free:
- kfree(fbdev_cma);
-
- return ERR_PTR(ret);
-}
-EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
-
-static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
- .destroy = drm_gem_fb_destroy,
- .create_handle = drm_gem_fb_create_handle,
+static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
+ .fb_probe = drm_fb_helper_generic_probe,
};
/**
@@ -464,9 +171,33 @@ static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
unsigned int preferred_bpp, unsigned int max_conn_count)
{
- return drm_fbdev_cma_init_with_funcs(dev, preferred_bpp,
- max_conn_count,
- &drm_fb_cma_funcs);
+ struct drm_fbdev_cma *fbdev_cma;
+ struct drm_fb_helper *fb_helper;
+ int ret;
+
+ fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
+ if (!fbdev_cma)
+ return ERR_PTR(-ENOMEM);
+
+ fb_helper = &fbdev_cma->fb_helper;
+
+ ret = drm_client_new(dev, &fb_helper->client, "fbdev", NULL);
+ if (ret)
+ goto err_free;
+
+ ret = drm_fb_helper_fbdev_setup(dev, fb_helper, &drm_fb_cma_helper_funcs,
+ preferred_bpp, max_conn_count);
+ if (ret)
+ goto err_client_put;
+
+ return fbdev_cma;
+
+err_client_put:
+ drm_client_release(&fb_helper->client);
+err_free:
+ kfree(fbdev_cma);
+
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
@@ -477,14 +208,7 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
{
drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
- if (fbdev_cma->fb_helper.fbdev)
- drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
-
- if (fbdev_cma->fb_helper.fb)
- drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
-
- drm_fb_helper_fini(&fbdev_cma->fb_helper);
- kfree(fbdev_cma);
+ /* All resources have now been freed by drm_fbdev_fb_destroy() */
}
EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini);
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index d532f88a8d55..a0546c3451f9 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -23,9 +23,6 @@ int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
unsigned int max_conn_count);
void drm_fb_cma_fbdev_fini(struct drm_device *dev);
-struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
- unsigned int preferred_bpp, unsigned int max_conn_count,
- const struct drm_framebuffer_funcs *funcs);
struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
unsigned int preferred_bpp, unsigned int max_conn_count);
void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-07-03 16:03 ` [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation Noralf Trønnes
@ 2018-08-21 6:44 ` John Stultz
2018-08-21 8:44 ` Daniel Vetter
` (2 more replies)
0 siblings, 3 replies; 40+ messages in thread
From: John Stultz @ 2018-08-21 6:44 UTC (permalink / raw)
To: Noralf Trønnes
Cc: Intel Graphics Development, Laurent Pinchart, dri-devel,
David Lechner
On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
> This switches the CMA helper drivers that use its fbdev emulation over
> to the generic fbdev emulation. It's the first phase of using generic
> fbdev. A later phase will use DRM client callbacks for the
> lastclose/hotplug/remove callbacks.
>
> There are currently 2 fbdev init/fini functions:
> - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
> - drm_fbdev_cma_init/drm_fbdev_cma_fini
>
> This is because the work on generic fbdev came up during a fbdev
> refactoring and thus wasn't completed. No point in completing that
> refactoring when drivers will soon move to drm_fb_helper_generic_probe().
>
> tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
> include/drm/drm_fb_cma_helper.h | 3 -
> 2 files changed, 42 insertions(+), 321 deletions(-)
...
> -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
> - struct drm_gem_cma_object *cma_obj)
> -{
> - struct fb_deferred_io *fbdefio;
> - struct fb_ops *fbops;
> -
> - /*
> - * Per device structures are needed because:
> - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
> - * fbdefio: individual delays
> - */
> - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
> - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
> - if (!fbdefio || !fbops) {
> - kfree(fbdefio);
> - kfree(fbops);
> - return -ENOMEM;
> - }
> -
> - /* can't be offset from vaddr since dirty() uses cma_obj */
> - fbi->screen_buffer = cma_obj->vaddr;
> - /* fb_deferred_io_fault() needs a physical address */
> - fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
> -
> - *fbops = *fbi->fbops;
> - fbi->fbops = fbops;
> -
> - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
> - fbdefio->deferred_io = drm_fb_helper_deferred_io;
> - fbi->fbdefio = fbdefio;
> - fb_deferred_io_init(fbi);
> - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
> -
> - return 0;
> -}
> -
> -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
> -{
> - if (!fbi->fbdefio)
> - return;
> -
> - fb_deferred_io_cleanup(fbi);
> - kfree(fbi->fbdefio);
> - kfree(fbi->fbops);
> -}
> -
> -static int
> -drm_fbdev_cma_create(struct drm_fb_helper *helper,
> - struct drm_fb_helper_surface_size *sizes)
> -{
> - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
> - struct drm_device *dev = helper->dev;
> - struct drm_gem_cma_object *obj;
> - struct drm_framebuffer *fb;
> - unsigned int bytes_per_pixel;
> - unsigned long offset;
> - struct fb_info *fbi;
> - size_t size;
> - int ret;
> -
> - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> - sizes->surface_width, sizes->surface_height,
> - sizes->surface_bpp);
> -
> - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> - size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
> - obj = drm_gem_cma_create(dev, size);
> - if (IS_ERR(obj))
> - return -ENOMEM;
> -
> - fbi = drm_fb_helper_alloc_fbi(helper);
> - if (IS_ERR(fbi)) {
> - ret = PTR_ERR(fbi);
> - goto err_gem_free_object;
> - }
> -
> - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
> - fbdev_cma->fb_funcs);
> - if (IS_ERR(fb)) {
> - dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
> - ret = PTR_ERR(fb);
> - goto err_fb_info_destroy;
> - }
> -
> - helper->fb = fb;
> -
> - fbi->par = helper;
> - fbi->flags = FBINFO_FLAG_DEFAULT;
> - fbi->fbops = &drm_fbdev_cma_ops;
> -
> - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
> - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
> -
> - offset = fbi->var.xoffset * bytes_per_pixel;
> - offset += fbi->var.yoffset * fb->pitches[0];
> -
> - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
> - fbi->screen_base = obj->vaddr + offset;
> - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
Hey Noralf, all,
I've been digging for a bit on the regression that this patch has
tripped on the HiKey board as reported here:
https://lkml.org/lkml/2018/8/16/81
The first issue was that the kirin driver was setting
mode_config.max_width/height = 2048, which was causing errors as the
the requested resolution was 1920x2160 (due to surfaceflinger
requesting y*2 for page flipping). This was relatively simple enough
to figure out and fix, but bumping the values up on its own didn't
seem to resolve the issue entirely, as I started to see hard hangs on
bootup when userspace started using the emulated fbdev device.
After confirming reverting the patch here worked around it, I started
digging through what might be different, and I think I've found it.
In the drm_fbdev_cma_create() function above, we set the
fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
which now replaces that, we don't set smem_start value at all.
Since we don't have a drm_gem_cma_object reference in
drm_fb_helper_generic_probe(), I instead added:
fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
And that got things working!
So yea, I wanted to figure out if we are just missing the line above
in drm_fb_helper_generic_probe()? Or if the kirin driver should be
setting the fix.smem_start in some other fashion?
thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 6:44 ` John Stultz
@ 2018-08-21 8:44 ` Daniel Vetter
2018-08-21 14:59 ` Noralf Trønnes
2018-08-21 14:15 ` Noralf Trønnes
2018-08-23 4:14 ` John Stultz
2 siblings, 1 reply; 40+ messages in thread
From: Daniel Vetter @ 2018-08-21 8:44 UTC (permalink / raw)
To: John Stultz
Cc: David Lechner, Intel Graphics Development, dri-devel,
Noralf Trønnes, Laurent Pinchart
On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
> On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
> > This switches the CMA helper drivers that use its fbdev emulation over
> > to the generic fbdev emulation. It's the first phase of using generic
> > fbdev. A later phase will use DRM client callbacks for the
> > lastclose/hotplug/remove callbacks.
> >
> > There are currently 2 fbdev init/fini functions:
> > - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
> > - drm_fbdev_cma_init/drm_fbdev_cma_fini
> >
> > This is because the work on generic fbdev came up during a fbdev
> > refactoring and thus wasn't completed. No point in completing that
> > refactoring when drivers will soon move to drm_fb_helper_generic_probe().
> >
> > tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
> >
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> > drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
> > include/drm/drm_fb_cma_helper.h | 3 -
> > 2 files changed, 42 insertions(+), 321 deletions(-)
> ...
> > -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
> > - struct drm_gem_cma_object *cma_obj)
> > -{
> > - struct fb_deferred_io *fbdefio;
> > - struct fb_ops *fbops;
> > -
> > - /*
> > - * Per device structures are needed because:
> > - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
> > - * fbdefio: individual delays
> > - */
> > - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
> > - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
> > - if (!fbdefio || !fbops) {
> > - kfree(fbdefio);
> > - kfree(fbops);
> > - return -ENOMEM;
> > - }
> > -
> > - /* can't be offset from vaddr since dirty() uses cma_obj */
> > - fbi->screen_buffer = cma_obj->vaddr;
> > - /* fb_deferred_io_fault() needs a physical address */
> > - fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
> > -
> > - *fbops = *fbi->fbops;
> > - fbi->fbops = fbops;
> > -
> > - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
> > - fbdefio->deferred_io = drm_fb_helper_deferred_io;
> > - fbi->fbdefio = fbdefio;
> > - fb_deferred_io_init(fbi);
> > - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
> > -
> > - return 0;
> > -}
> > -
> > -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
> > -{
> > - if (!fbi->fbdefio)
> > - return;
> > -
> > - fb_deferred_io_cleanup(fbi);
> > - kfree(fbi->fbdefio);
> > - kfree(fbi->fbops);
> > -}
> > -
> > -static int
> > -drm_fbdev_cma_create(struct drm_fb_helper *helper,
> > - struct drm_fb_helper_surface_size *sizes)
> > -{
> > - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
> > - struct drm_device *dev = helper->dev;
> > - struct drm_gem_cma_object *obj;
> > - struct drm_framebuffer *fb;
> > - unsigned int bytes_per_pixel;
> > - unsigned long offset;
> > - struct fb_info *fbi;
> > - size_t size;
> > - int ret;
> > -
> > - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> > - sizes->surface_width, sizes->surface_height,
> > - sizes->surface_bpp);
> > -
> > - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> > - size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
> > - obj = drm_gem_cma_create(dev, size);
> > - if (IS_ERR(obj))
> > - return -ENOMEM;
> > -
> > - fbi = drm_fb_helper_alloc_fbi(helper);
> > - if (IS_ERR(fbi)) {
> > - ret = PTR_ERR(fbi);
> > - goto err_gem_free_object;
> > - }
> > -
> > - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
> > - fbdev_cma->fb_funcs);
> > - if (IS_ERR(fb)) {
> > - dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
> > - ret = PTR_ERR(fb);
> > - goto err_fb_info_destroy;
> > - }
> > -
> > - helper->fb = fb;
> > -
> > - fbi->par = helper;
> > - fbi->flags = FBINFO_FLAG_DEFAULT;
> > - fbi->fbops = &drm_fbdev_cma_ops;
> > -
> > - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
> > - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
> > -
> > - offset = fbi->var.xoffset * bytes_per_pixel;
> > - offset += fbi->var.yoffset * fb->pitches[0];
> > -
> > - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
> > - fbi->screen_base = obj->vaddr + offset;
> > - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
>
> Hey Noralf, all,
> I've been digging for a bit on the regression that this patch has
> tripped on the HiKey board as reported here:
> https://lkml.org/lkml/2018/8/16/81
>
> The first issue was that the kirin driver was setting
> mode_config.max_width/height = 2048, which was causing errors as the
> the requested resolution was 1920x2160 (due to surfaceflinger
> requesting y*2 for page flipping). This was relatively simple enough
> to figure out and fix, but bumping the values up on its own didn't
> seem to resolve the issue entirely, as I started to see hard hangs on
> bootup when userspace started using the emulated fbdev device.
>
> After confirming reverting the patch here worked around it, I started
> digging through what might be different, and I think I've found it.
> In the drm_fbdev_cma_create() function above, we set the
> fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
> which now replaces that, we don't set smem_start value at all.
>
> Since we don't have a drm_gem_cma_object reference in
> drm_fb_helper_generic_probe(), I instead added:
> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>
> And that got things working!
>
> So yea, I wanted to figure out if we are just missing the line above
> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
> setting the fix.smem_start in some other fashion?
With the generic helpers we can't use the generic fb_mmap() implementation
in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
you pls double-check that this is wired up correctly for kirin?
At least I don't see any other place where we use smem_start in the fbdev
code. It does get copied to userspace, but userspace should never use
that.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 8:44 ` Daniel Vetter
@ 2018-08-21 14:59 ` Noralf Trønnes
2018-08-21 15:41 ` Daniel Vetter
2018-08-21 18:43 ` John Stultz
0 siblings, 2 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-08-21 14:59 UTC (permalink / raw)
To: Daniel Vetter, John Stultz
Cc: Intel Graphics Development, Laurent Pinchart, dri-devel,
David Lechner
Den 21.08.2018 10.44, skrev Daniel Vetter:
> On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
>> On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
>>> This switches the CMA helper drivers that use its fbdev emulation over
>>> to the generic fbdev emulation. It's the first phase of using generic
>>> fbdev. A later phase will use DRM client callbacks for the
>>> lastclose/hotplug/remove callbacks.
>>>
>>> There are currently 2 fbdev init/fini functions:
>>> - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
>>> - drm_fbdev_cma_init/drm_fbdev_cma_fini
>>>
>>> This is because the work on generic fbdev came up during a fbdev
>>> refactoring and thus wasn't completed. No point in completing that
>>> refactoring when drivers will soon move to drm_fb_helper_generic_probe().
>>>
>>> tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
>>>
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> ---
>>> drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
>>> include/drm/drm_fb_cma_helper.h | 3 -
>>> 2 files changed, 42 insertions(+), 321 deletions(-)
>> ...
>>> -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
>>> - struct drm_gem_cma_object *cma_obj)
>>> -{
>>> - struct fb_deferred_io *fbdefio;
>>> - struct fb_ops *fbops;
>>> -
>>> - /*
>>> - * Per device structures are needed because:
>>> - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
>>> - * fbdefio: individual delays
>>> - */
>>> - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>>> - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>>> - if (!fbdefio || !fbops) {
>>> - kfree(fbdefio);
>>> - kfree(fbops);
>>> - return -ENOMEM;
>>> - }
>>> -
>>> - /* can't be offset from vaddr since dirty() uses cma_obj */
>>> - fbi->screen_buffer = cma_obj->vaddr;
>>> - /* fb_deferred_io_fault() needs a physical address */
>>> - fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>> -
>>> - *fbops = *fbi->fbops;
>>> - fbi->fbops = fbops;
>>> -
>>> - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
>>> - fbdefio->deferred_io = drm_fb_helper_deferred_io;
>>> - fbi->fbdefio = fbdefio;
>>> - fb_deferred_io_init(fbi);
>>> - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
>>> -{
>>> - if (!fbi->fbdefio)
>>> - return;
>>> -
>>> - fb_deferred_io_cleanup(fbi);
>>> - kfree(fbi->fbdefio);
>>> - kfree(fbi->fbops);
>>> -}
>>> -
>>> -static int
>>> -drm_fbdev_cma_create(struct drm_fb_helper *helper,
>>> - struct drm_fb_helper_surface_size *sizes)
>>> -{
>>> - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
>>> - struct drm_device *dev = helper->dev;
>>> - struct drm_gem_cma_object *obj;
>>> - struct drm_framebuffer *fb;
>>> - unsigned int bytes_per_pixel;
>>> - unsigned long offset;
>>> - struct fb_info *fbi;
>>> - size_t size;
>>> - int ret;
>>> -
>>> - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
>>> - sizes->surface_width, sizes->surface_height,
>>> - sizes->surface_bpp);
>>> -
>>> - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>>> - size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
>>> - obj = drm_gem_cma_create(dev, size);
>>> - if (IS_ERR(obj))
>>> - return -ENOMEM;
>>> -
>>> - fbi = drm_fb_helper_alloc_fbi(helper);
>>> - if (IS_ERR(fbi)) {
>>> - ret = PTR_ERR(fbi);
>>> - goto err_gem_free_object;
>>> - }
>>> -
>>> - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
>>> - fbdev_cma->fb_funcs);
>>> - if (IS_ERR(fb)) {
>>> - dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
>>> - ret = PTR_ERR(fb);
>>> - goto err_fb_info_destroy;
>>> - }
>>> -
>>> - helper->fb = fb;
>>> -
>>> - fbi->par = helper;
>>> - fbi->flags = FBINFO_FLAG_DEFAULT;
>>> - fbi->fbops = &drm_fbdev_cma_ops;
>>> -
>>> - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
>>> - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
>>> -
>>> - offset = fbi->var.xoffset * bytes_per_pixel;
>>> - offset += fbi->var.yoffset * fb->pitches[0];
>>> -
>>> - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
>>> - fbi->screen_base = obj->vaddr + offset;
>>> - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
>> Hey Noralf, all,
>> I've been digging for a bit on the regression that this patch has
>> tripped on the HiKey board as reported here:
>> https://lkml.org/lkml/2018/8/16/81
>>
>> The first issue was that the kirin driver was setting
>> mode_config.max_width/height = 2048, which was causing errors as the
>> the requested resolution was 1920x2160 (due to surfaceflinger
>> requesting y*2 for page flipping). This was relatively simple enough
>> to figure out and fix, but bumping the values up on its own didn't
>> seem to resolve the issue entirely, as I started to see hard hangs on
>> bootup when userspace started using the emulated fbdev device.
>>
>> After confirming reverting the patch here worked around it, I started
>> digging through what might be different, and I think I've found it.
>> In the drm_fbdev_cma_create() function above, we set the
>> fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
>> which now replaces that, we don't set smem_start value at all.
>>
>> Since we don't have a drm_gem_cma_object reference in
>> drm_fb_helper_generic_probe(), I instead added:
>> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>
>> And that got things working!
>>
>> So yea, I wanted to figure out if we are just missing the line above
>> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
>> setting the fix.smem_start in some other fashion?
> With the generic helpers we can't use the generic fb_mmap() implementation
> in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
> fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
> you pls double-check that this is wired up correctly for kirin?
>
> At least I don't see any other place where we use smem_start in the fbdev
> code. It does get copied to userspace, but userspace should never use
> that.
I googled 'FBIOGET_FSCREENINFO smem_start' and came across info[1]
about the Mali blob using it and HiKey seems to have a Mali GPU:
00:17 <ssvb> BorgCuba:
the mali blob is supposed to open the framebuffer device,
get "smem_start" and try to map this memory via MALI_IOC_MEM_MAP_EXT ioctl
Could this be the problem here?
Noralf.
[1] https://irclog.whitequark.org/linux-rockchip/2015-12-22
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 14:59 ` Noralf Trønnes
@ 2018-08-21 15:41 ` Daniel Vetter
2018-08-21 16:03 ` Noralf Trønnes
2018-08-21 18:43 ` John Stultz
1 sibling, 1 reply; 40+ messages in thread
From: Daniel Vetter @ 2018-08-21 15:41 UTC (permalink / raw)
To: Noralf Trønnes
Cc: David Lechner, Intel Graphics Development, dri-devel,
Laurent Pinchart
On Tue, Aug 21, 2018 at 04:59:46PM +0200, Noralf Trønnes wrote:
>
> Den 21.08.2018 10.44, skrev Daniel Vetter:
> > On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
> > > On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
> > > > This switches the CMA helper drivers that use its fbdev emulation over
> > > > to the generic fbdev emulation. It's the first phase of using generic
> > > > fbdev. A later phase will use DRM client callbacks for the
> > > > lastclose/hotplug/remove callbacks.
> > > >
> > > > There are currently 2 fbdev init/fini functions:
> > > > - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
> > > > - drm_fbdev_cma_init/drm_fbdev_cma_fini
> > > >
> > > > This is because the work on generic fbdev came up during a fbdev
> > > > refactoring and thus wasn't completed. No point in completing that
> > > > refactoring when drivers will soon move to drm_fb_helper_generic_probe().
> > > >
> > > > tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
> > > >
> > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > ---
> > > > drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
> > > > include/drm/drm_fb_cma_helper.h | 3 -
> > > > 2 files changed, 42 insertions(+), 321 deletions(-)
> > > ...
> > > > -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
> > > > - struct drm_gem_cma_object *cma_obj)
> > > > -{
> > > > - struct fb_deferred_io *fbdefio;
> > > > - struct fb_ops *fbops;
> > > > -
> > > > - /*
> > > > - * Per device structures are needed because:
> > > > - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
> > > > - * fbdefio: individual delays
> > > > - */
> > > > - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
> > > > - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
> > > > - if (!fbdefio || !fbops) {
> > > > - kfree(fbdefio);
> > > > - kfree(fbops);
> > > > - return -ENOMEM;
> > > > - }
> > > > -
> > > > - /* can't be offset from vaddr since dirty() uses cma_obj */
> > > > - fbi->screen_buffer = cma_obj->vaddr;
> > > > - /* fb_deferred_io_fault() needs a physical address */
> > > > - fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
> > > > -
> > > > - *fbops = *fbi->fbops;
> > > > - fbi->fbops = fbops;
> > > > -
> > > > - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
> > > > - fbdefio->deferred_io = drm_fb_helper_deferred_io;
> > > > - fbi->fbdefio = fbdefio;
> > > > - fb_deferred_io_init(fbi);
> > > > - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
> > > > -{
> > > > - if (!fbi->fbdefio)
> > > > - return;
> > > > -
> > > > - fb_deferred_io_cleanup(fbi);
> > > > - kfree(fbi->fbdefio);
> > > > - kfree(fbi->fbops);
> > > > -}
> > > > -
> > > > -static int
> > > > -drm_fbdev_cma_create(struct drm_fb_helper *helper,
> > > > - struct drm_fb_helper_surface_size *sizes)
> > > > -{
> > > > - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
> > > > - struct drm_device *dev = helper->dev;
> > > > - struct drm_gem_cma_object *obj;
> > > > - struct drm_framebuffer *fb;
> > > > - unsigned int bytes_per_pixel;
> > > > - unsigned long offset;
> > > > - struct fb_info *fbi;
> > > > - size_t size;
> > > > - int ret;
> > > > -
> > > > - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> > > > - sizes->surface_width, sizes->surface_height,
> > > > - sizes->surface_bpp);
> > > > -
> > > > - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
> > > > - size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
> > > > - obj = drm_gem_cma_create(dev, size);
> > > > - if (IS_ERR(obj))
> > > > - return -ENOMEM;
> > > > -
> > > > - fbi = drm_fb_helper_alloc_fbi(helper);
> > > > - if (IS_ERR(fbi)) {
> > > > - ret = PTR_ERR(fbi);
> > > > - goto err_gem_free_object;
> > > > - }
> > > > -
> > > > - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
> > > > - fbdev_cma->fb_funcs);
> > > > - if (IS_ERR(fb)) {
> > > > - dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
> > > > - ret = PTR_ERR(fb);
> > > > - goto err_fb_info_destroy;
> > > > - }
> > > > -
> > > > - helper->fb = fb;
> > > > -
> > > > - fbi->par = helper;
> > > > - fbi->flags = FBINFO_FLAG_DEFAULT;
> > > > - fbi->fbops = &drm_fbdev_cma_ops;
> > > > -
> > > > - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
> > > > - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
> > > > -
> > > > - offset = fbi->var.xoffset * bytes_per_pixel;
> > > > - offset += fbi->var.yoffset * fb->pitches[0];
> > > > -
> > > > - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
> > > > - fbi->screen_base = obj->vaddr + offset;
> > > > - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
> > > Hey Noralf, all,
> > > I've been digging for a bit on the regression that this patch has
> > > tripped on the HiKey board as reported here:
> > > https://lkml.org/lkml/2018/8/16/81
> > >
> > > The first issue was that the kirin driver was setting
> > > mode_config.max_width/height = 2048, which was causing errors as the
> > > the requested resolution was 1920x2160 (due to surfaceflinger
> > > requesting y*2 for page flipping). This was relatively simple enough
> > > to figure out and fix, but bumping the values up on its own didn't
> > > seem to resolve the issue entirely, as I started to see hard hangs on
> > > bootup when userspace started using the emulated fbdev device.
> > >
> > > After confirming reverting the patch here worked around it, I started
> > > digging through what might be different, and I think I've found it.
> > > In the drm_fbdev_cma_create() function above, we set the
> > > fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
> > > which now replaces that, we don't set smem_start value at all.
> > >
> > > Since we don't have a drm_gem_cma_object reference in
> > > drm_fb_helper_generic_probe(), I instead added:
> > > fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
> > >
> > > And that got things working!
> > >
> > > So yea, I wanted to figure out if we are just missing the line above
> > > in drm_fb_helper_generic_probe()? Or if the kirin driver should be
> > > setting the fix.smem_start in some other fashion?
> > With the generic helpers we can't use the generic fb_mmap() implementation
> > in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
> > fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
> > you pls double-check that this is wired up correctly for kirin?
> >
> > At least I don't see any other place where we use smem_start in the fbdev
> > code. It does get copied to userspace, but userspace should never use
> > that.
>
> I googled 'FBIOGET_FSCREENINFO smem_start' and came across info[1]
> about the Mali blob using it and HiKey seems to have a Mali GPU:
>
> 00:17 <ssvb> BorgCuba:
> the mali blob is supposed to open the framebuffer device,
> get "smem_start" and try to map this memory via MALI_IOC_MEM_MAP_EXT ioctl
>
> Could this be the problem here?
Yup, most certainly is. As you analyzed in your other reply, smem_start
shouldn't matter at all for fbdev mmap support.
And the only other place it's used is the ioctl.
I'm pretty sure John owes us one, because the Oops he's seeing is in the
mali kernel driver for the mali userspace blob :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 15:41 ` Daniel Vetter
@ 2018-08-21 16:03 ` Noralf Trønnes
2018-08-21 16:46 ` Daniel Vetter
0 siblings, 1 reply; 40+ messages in thread
From: Noralf Trønnes @ 2018-08-21 16:03 UTC (permalink / raw)
To: Daniel Vetter
Cc: Intel Graphics Development, John Stultz, Laurent Pinchart,
dri-devel, David Lechner
Den 21.08.2018 17.41, skrev Daniel Vetter:
> On Tue, Aug 21, 2018 at 04:59:46PM +0200, Noralf Trønnes wrote:
>> Den 21.08.2018 10.44, skrev Daniel Vetter:
>>> On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
>>>> On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
>>>>> This switches the CMA helper drivers that use its fbdev emulation over
>>>>> to the generic fbdev emulation. It's the first phase of using generic
>>>>> fbdev. A later phase will use DRM client callbacks for the
>>>>> lastclose/hotplug/remove callbacks.
>>>>>
>>>>> There are currently 2 fbdev init/fini functions:
>>>>> - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
>>>>> - drm_fbdev_cma_init/drm_fbdev_cma_fini
>>>>>
>>>>> This is because the work on generic fbdev came up during a fbdev
>>>>> refactoring and thus wasn't completed. No point in completing that
>>>>> refactoring when drivers will soon move to drm_fb_helper_generic_probe().
>>>>>
>>>>> tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
>>>>>
>>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> ---
>>>>> drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
>>>>> include/drm/drm_fb_cma_helper.h | 3 -
>>>>> 2 files changed, 42 insertions(+), 321 deletions(-)
>>>> ...
>>>>> -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
>>>>> - struct drm_gem_cma_object *cma_obj)
>>>>> -{
>>>>> - struct fb_deferred_io *fbdefio;
>>>>> - struct fb_ops *fbops;
>>>>> -
>>>>> - /*
>>>>> - * Per device structures are needed because:
>>>>> - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
>>>>> - * fbdefio: individual delays
>>>>> - */
>>>>> - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>>>>> - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>>>>> - if (!fbdefio || !fbops) {
>>>>> - kfree(fbdefio);
>>>>> - kfree(fbops);
>>>>> - return -ENOMEM;
>>>>> - }
>>>>> -
>>>>> - /* can't be offset from vaddr since dirty() uses cma_obj */
>>>>> - fbi->screen_buffer = cma_obj->vaddr;
>>>>> - /* fb_deferred_io_fault() needs a physical address */
>>>>> - fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>>>> -
>>>>> - *fbops = *fbi->fbops;
>>>>> - fbi->fbops = fbops;
>>>>> -
>>>>> - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
>>>>> - fbdefio->deferred_io = drm_fb_helper_deferred_io;
>>>>> - fbi->fbdefio = fbdefio;
>>>>> - fb_deferred_io_init(fbi);
>>>>> - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
>>>>> -
>>>>> - return 0;
>>>>> -}
>>>>> -
>>>>> -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
>>>>> -{
>>>>> - if (!fbi->fbdefio)
>>>>> - return;
>>>>> -
>>>>> - fb_deferred_io_cleanup(fbi);
>>>>> - kfree(fbi->fbdefio);
>>>>> - kfree(fbi->fbops);
>>>>> -}
>>>>> -
>>>>> -static int
>>>>> -drm_fbdev_cma_create(struct drm_fb_helper *helper,
>>>>> - struct drm_fb_helper_surface_size *sizes)
>>>>> -{
>>>>> - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
>>>>> - struct drm_device *dev = helper->dev;
>>>>> - struct drm_gem_cma_object *obj;
>>>>> - struct drm_framebuffer *fb;
>>>>> - unsigned int bytes_per_pixel;
>>>>> - unsigned long offset;
>>>>> - struct fb_info *fbi;
>>>>> - size_t size;
>>>>> - int ret;
>>>>> -
>>>>> - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
>>>>> - sizes->surface_width, sizes->surface_height,
>>>>> - sizes->surface_bpp);
>>>>> -
>>>>> - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>>>>> - size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
>>>>> - obj = drm_gem_cma_create(dev, size);
>>>>> - if (IS_ERR(obj))
>>>>> - return -ENOMEM;
>>>>> -
>>>>> - fbi = drm_fb_helper_alloc_fbi(helper);
>>>>> - if (IS_ERR(fbi)) {
>>>>> - ret = PTR_ERR(fbi);
>>>>> - goto err_gem_free_object;
>>>>> - }
>>>>> -
>>>>> - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
>>>>> - fbdev_cma->fb_funcs);
>>>>> - if (IS_ERR(fb)) {
>>>>> - dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
>>>>> - ret = PTR_ERR(fb);
>>>>> - goto err_fb_info_destroy;
>>>>> - }
>>>>> -
>>>>> - helper->fb = fb;
>>>>> -
>>>>> - fbi->par = helper;
>>>>> - fbi->flags = FBINFO_FLAG_DEFAULT;
>>>>> - fbi->fbops = &drm_fbdev_cma_ops;
>>>>> -
>>>>> - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
>>>>> - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
>>>>> -
>>>>> - offset = fbi->var.xoffset * bytes_per_pixel;
>>>>> - offset += fbi->var.yoffset * fb->pitches[0];
>>>>> -
>>>>> - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
>>>>> - fbi->screen_base = obj->vaddr + offset;
>>>>> - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
>>>> Hey Noralf, all,
>>>> I've been digging for a bit on the regression that this patch has
>>>> tripped on the HiKey board as reported here:
>>>> https://lkml.org/lkml/2018/8/16/81
>>>>
>>>> The first issue was that the kirin driver was setting
>>>> mode_config.max_width/height = 2048, which was causing errors as the
>>>> the requested resolution was 1920x2160 (due to surfaceflinger
>>>> requesting y*2 for page flipping). This was relatively simple enough
>>>> to figure out and fix, but bumping the values up on its own didn't
>>>> seem to resolve the issue entirely, as I started to see hard hangs on
>>>> bootup when userspace started using the emulated fbdev device.
>>>>
>>>> After confirming reverting the patch here worked around it, I started
>>>> digging through what might be different, and I think I've found it.
>>>> In the drm_fbdev_cma_create() function above, we set the
>>>> fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
>>>> which now replaces that, we don't set smem_start value at all.
>>>>
>>>> Since we don't have a drm_gem_cma_object reference in
>>>> drm_fb_helper_generic_probe(), I instead added:
>>>> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>>>
>>>> And that got things working!
>>>>
>>>> So yea, I wanted to figure out if we are just missing the line above
>>>> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
>>>> setting the fix.smem_start in some other fashion?
>>> With the generic helpers we can't use the generic fb_mmap() implementation
>>> in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
>>> fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
>>> you pls double-check that this is wired up correctly for kirin?
>>>
>>> At least I don't see any other place where we use smem_start in the fbdev
>>> code. It does get copied to userspace, but userspace should never use
>>> that.
>> I googled 'FBIOGET_FSCREENINFO smem_start' and came across info[1]
>> about the Mali blob using it and HiKey seems to have a Mali GPU:
>>
>> 00:17 <ssvb> BorgCuba:
>> the mali blob is supposed to open the framebuffer device,
>> get "smem_start" and try to map this memory via MALI_IOC_MEM_MAP_EXT ioctl
>>
>> Could this be the problem here?
> Yup, most certainly is. As you analyzed in your other reply, smem_start
> shouldn't matter at all for fbdev mmap support.
>
> And the only other place it's used is the ioctl.
>
> I'm pretty sure John owes us one, because the Oops he's seeing is in the
> mali kernel driver for the mali userspace blob :-)
Ah, so it makes sense after all.
The change John did:
fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
is the conversions legal on all platforms? It doesn't have to be a legal
value, but it can't be oops'ing or fail to build for some other user.
If it's ok we can just add that line and be done with it.
Noralf.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 16:03 ` Noralf Trønnes
@ 2018-08-21 16:46 ` Daniel Vetter
0 siblings, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2018-08-21 16:46 UTC (permalink / raw)
To: Noralf Trønnes
Cc: Intel Graphics Development, John Stultz, Laurent Pinchart,
dri-devel, David Lechner
On Tue, Aug 21, 2018 at 6:03 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Den 21.08.2018 17.41, skrev Daniel Vetter:
>>
>> On Tue, Aug 21, 2018 at 04:59:46PM +0200, Noralf Trønnes wrote:
>>>
>>> Den 21.08.2018 10.44, skrev Daniel Vetter:
>>>>
>>>> On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
>>>>>
>>>>> On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf@tronnes.org>
>>>>> wrote:
>>>>>>
>>>>>> This switches the CMA helper drivers that use its fbdev emulation over
>>>>>> to the generic fbdev emulation. It's the first phase of using generic
>>>>>> fbdev. A later phase will use DRM client callbacks for the
>>>>>> lastclose/hotplug/remove callbacks.
>>>>>>
>>>>>> There are currently 2 fbdev init/fini functions:
>>>>>> - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
>>>>>> - drm_fbdev_cma_init/drm_fbdev_cma_fini
>>>>>>
>>>>>> This is because the work on generic fbdev came up during a fbdev
>>>>>> refactoring and thus wasn't completed. No point in completing that
>>>>>> refactoring when drivers will soon move to
>>>>>> drm_fb_helper_generic_probe().
>>>>>>
>>>>>> tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
>>>>>>
>>>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>> ---
>>>>>> drivers/gpu/drm/drm_fb_cma_helper.c | 360
>>>>>> +++++-------------------------------
>>>>>> include/drm/drm_fb_cma_helper.h | 3 -
>>>>>> 2 files changed, 42 insertions(+), 321 deletions(-)
>>>>>
>>>>> ...
>>>>>>
>>>>>> -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
>>>>>> - struct drm_gem_cma_object
>>>>>> *cma_obj)
>>>>>> -{
>>>>>> - struct fb_deferred_io *fbdefio;
>>>>>> - struct fb_ops *fbops;
>>>>>> -
>>>>>> - /*
>>>>>> - * Per device structures are needed because:
>>>>>> - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
>>>>>> - * fbdefio: individual delays
>>>>>> - */
>>>>>> - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>>>>>> - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>>>>>> - if (!fbdefio || !fbops) {
>>>>>> - kfree(fbdefio);
>>>>>> - kfree(fbops);
>>>>>> - return -ENOMEM;
>>>>>> - }
>>>>>> -
>>>>>> - /* can't be offset from vaddr since dirty() uses cma_obj */
>>>>>> - fbi->screen_buffer = cma_obj->vaddr;
>>>>>> - /* fb_deferred_io_fault() needs a physical address */
>>>>>> - fbi->fix.smem_start =
>>>>>> page_to_phys(virt_to_page(fbi->screen_buffer));
>>>>>> -
>>>>>> - *fbops = *fbi->fbops;
>>>>>> - fbi->fbops = fbops;
>>>>>> -
>>>>>> - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
>>>>>> - fbdefio->deferred_io = drm_fb_helper_deferred_io;
>>>>>> - fbi->fbdefio = fbdefio;
>>>>>> - fb_deferred_io_init(fbi);
>>>>>> - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
>>>>>> -
>>>>>> - return 0;
>>>>>> -}
>>>>>> -
>>>>>> -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
>>>>>> -{
>>>>>> - if (!fbi->fbdefio)
>>>>>> - return;
>>>>>> -
>>>>>> - fb_deferred_io_cleanup(fbi);
>>>>>> - kfree(fbi->fbdefio);
>>>>>> - kfree(fbi->fbops);
>>>>>> -}
>>>>>> -
>>>>>> -static int
>>>>>> -drm_fbdev_cma_create(struct drm_fb_helper *helper,
>>>>>> - struct drm_fb_helper_surface_size *sizes)
>>>>>> -{
>>>>>> - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
>>>>>> - struct drm_device *dev = helper->dev;
>>>>>> - struct drm_gem_cma_object *obj;
>>>>>> - struct drm_framebuffer *fb;
>>>>>> - unsigned int bytes_per_pixel;
>>>>>> - unsigned long offset;
>>>>>> - struct fb_info *fbi;
>>>>>> - size_t size;
>>>>>> - int ret;
>>>>>> -
>>>>>> - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
>>>>>> - sizes->surface_width, sizes->surface_height,
>>>>>> - sizes->surface_bpp);
>>>>>> -
>>>>>> - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>>>>>> - size = sizes->surface_width * sizes->surface_height *
>>>>>> bytes_per_pixel;
>>>>>> - obj = drm_gem_cma_create(dev, size);
>>>>>> - if (IS_ERR(obj))
>>>>>> - return -ENOMEM;
>>>>>> -
>>>>>> - fbi = drm_fb_helper_alloc_fbi(helper);
>>>>>> - if (IS_ERR(fbi)) {
>>>>>> - ret = PTR_ERR(fbi);
>>>>>> - goto err_gem_free_object;
>>>>>> - }
>>>>>> -
>>>>>> - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
>>>>>> - fbdev_cma->fb_funcs);
>>>>>> - if (IS_ERR(fb)) {
>>>>>> - dev_err(dev->dev, "Failed to allocate DRM
>>>>>> framebuffer.\n");
>>>>>> - ret = PTR_ERR(fb);
>>>>>> - goto err_fb_info_destroy;
>>>>>> - }
>>>>>> -
>>>>>> - helper->fb = fb;
>>>>>> -
>>>>>> - fbi->par = helper;
>>>>>> - fbi->flags = FBINFO_FLAG_DEFAULT;
>>>>>> - fbi->fbops = &drm_fbdev_cma_ops;
>>>>>> -
>>>>>> - drm_fb_helper_fill_fix(fbi, fb->pitches[0],
>>>>>> fb->format->depth);
>>>>>> - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width,
>>>>>> sizes->fb_height);
>>>>>> -
>>>>>> - offset = fbi->var.xoffset * bytes_per_pixel;
>>>>>> - offset += fbi->var.yoffset * fb->pitches[0];
>>>>>> -
>>>>>> - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
>>>>>> - fbi->screen_base = obj->vaddr + offset;
>>>>>> - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
>>>>>
>>>>> Hey Noralf, all,
>>>>> I've been digging for a bit on the regression that this patch has
>>>>> tripped on the HiKey board as reported here:
>>>>> https://lkml.org/lkml/2018/8/16/81
>>>>>
>>>>> The first issue was that the kirin driver was setting
>>>>> mode_config.max_width/height = 2048, which was causing errors as the
>>>>> the requested resolution was 1920x2160 (due to surfaceflinger
>>>>> requesting y*2 for page flipping). This was relatively simple enough
>>>>> to figure out and fix, but bumping the values up on its own didn't
>>>>> seem to resolve the issue entirely, as I started to see hard hangs on
>>>>> bootup when userspace started using the emulated fbdev device.
>>>>>
>>>>> After confirming reverting the patch here worked around it, I started
>>>>> digging through what might be different, and I think I've found it.
>>>>> In the drm_fbdev_cma_create() function above, we set the
>>>>> fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
>>>>> which now replaces that, we don't set smem_start value at all.
>>>>>
>>>>> Since we don't have a drm_gem_cma_object reference in
>>>>> drm_fb_helper_generic_probe(), I instead added:
>>>>> fbi->fix.smem_start =
>>>>> page_to_phys(virt_to_page(fbi->screen_buffer));
>>>>>
>>>>> And that got things working!
>>>>>
>>>>> So yea, I wanted to figure out if we are just missing the line above
>>>>> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
>>>>> setting the fix.smem_start in some other fashion?
>>>>
>>>> With the generic helpers we can't use the generic fb_mmap()
>>>> implementation
>>>> in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
>>>> fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
>>>> you pls double-check that this is wired up correctly for kirin?
>>>>
>>>> At least I don't see any other place where we use smem_start in the
>>>> fbdev
>>>> code. It does get copied to userspace, but userspace should never use
>>>> that.
>>>
>>> I googled 'FBIOGET_FSCREENINFO smem_start' and came across info[1]
>>> about the Mali blob using it and HiKey seems to have a Mali GPU:
>>>
>>> 00:17 <ssvb> BorgCuba:
>>> the mali blob is supposed to open the framebuffer device,
>>> get "smem_start" and try to map this memory via MALI_IOC_MEM_MAP_EXT
>>> ioctl
>>>
>>> Could this be the problem here?
>>
>> Yup, most certainly is. As you analyzed in your other reply, smem_start
>> shouldn't matter at all for fbdev mmap support.
>>
>> And the only other place it's used is the ioctl.
>>
>> I'm pretty sure John owes us one, because the Oops he's seeing is in the
>> mali kernel driver for the mali userspace blob :-)
>
>
> Ah, so it makes sense after all.
>
> The change John did:
>
> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>
> is the conversions legal on all platforms? It doesn't have to be a legal
> value, but it can't be oops'ing or fail to build for some other user.
>
> If it's ok we can just add that line and be done with it.
Nope, very much not ok for anything using discontinuous buffers. For
that you need to point it at some remapping unit to make fbdev happy,
and only the driver knows about that.
Also, dri-devel officially doesn't care about blob userspace and
non-upstream kernel drivers :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 14:59 ` Noralf Trønnes
2018-08-21 15:41 ` Daniel Vetter
@ 2018-08-21 18:43 ` John Stultz
2018-08-22 7:56 ` Daniel Vetter
1 sibling, 1 reply; 40+ messages in thread
From: John Stultz @ 2018-08-21 18:43 UTC (permalink / raw)
To: Noralf Trønnes
Cc: David Lechner, Intel Graphics Development, dri-devel,
Laurent Pinchart
On Tue, Aug 21, 2018 at 7:59 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 21.08.2018 10.44, skrev Daniel Vetter:
>> On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
>>>
>>> Since we don't have a drm_gem_cma_object reference in
>>> drm_fb_helper_generic_probe(), I instead added:
>>> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>>
>>> And that got things working!
>>>
>>> So yea, I wanted to figure out if we are just missing the line above
>>> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
>>> setting the fix.smem_start in some other fashion?
>>
>> With the generic helpers we can't use the generic fb_mmap() implementation
>> in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
>> fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
>> you pls double-check that this is wired up correctly for kirin?
>>
>> At least I don't see any other place where we use smem_start in the fbdev
>> code. It does get copied to userspace, but userspace should never use
>> that.
>
>
> I googled 'FBIOGET_FSCREENINFO smem_start' and came across info[1]
> about the Mali blob using it and HiKey seems to have a Mali GPU:
>
> 00:17 <ssvb> BorgCuba:
> the mali blob is supposed to open the framebuffer device,
> get "smem_start" and try to map this memory via MALI_IOC_MEM_MAP_EXT ioctl
>
> Could this be the problem here?
It does look like that's the case. Looking for smem_start usage, for
Android its the gralloc code that fetches it:
https://android.googlesource.com/device/linaro/hikey/+/master/gralloc/framebuffer_device.cpp#353
and then puts it into the private_handle_t:
https://android.googlesource.com/device/linaro/hikey/+/master/gralloc/framebuffer_device.cpp#384
Which I assume then gets used later in the opaque mali blob (which
then results in a similarly opaque hang).
While I'm perfectly understanding that folks are not interested as its
related to mali (which is fine, I can carry a patch - working to move
away from fbdev emulation anyway), I am wondering if it might cause
trouble for other users, as smem_start was previously set and now its
not, so it seems likely to break userspace examples like:
https://www.linuxtv.org/downloads/v4l-dvb-apis-old/osd.html
Or are those such old legacy they don't really count?
thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 18:43 ` John Stultz
@ 2018-08-22 7:56 ` Daniel Vetter
0 siblings, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2018-08-22 7:56 UTC (permalink / raw)
To: John Stultz
Cc: Intel Graphics Development, Noralf Trønnes, Laurent Pinchart,
dri-devel, David Lechner
On Tue, Aug 21, 2018 at 8:43 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Aug 21, 2018 at 7:59 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
>> Den 21.08.2018 10.44, skrev Daniel Vetter:
>>> On Mon, Aug 20, 2018 at 11:44:56PM -0700, John Stultz wrote:
>>>>
>>>> Since we don't have a drm_gem_cma_object reference in
>>>> drm_fb_helper_generic_probe(), I instead added:
>>>> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>>>>
>>>> And that got things working!
>>>>
>>>> So yea, I wanted to figure out if we are just missing the line above
>>>> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
>>>> setting the fix.smem_start in some other fashion?
>>>
>>> With the generic helpers we can't use the generic fb_mmap() implementation
>>> in fbdev/core/fbmem.c anymore. Instead Noralf added a generic fb_mmap
>>> fb_ops callback in drm_fbdev_fb_mmap, which you're supposed to use. Can
>>> you pls double-check that this is wired up correctly for kirin?
>>>
>>> At least I don't see any other place where we use smem_start in the fbdev
>>> code. It does get copied to userspace, but userspace should never use
>>> that.
>>
>>
>> I googled 'FBIOGET_FSCREENINFO smem_start' and came across info[1]
>> about the Mali blob using it and HiKey seems to have a Mali GPU:
>>
>> 00:17 <ssvb> BorgCuba:
>> the mali blob is supposed to open the framebuffer device,
>> get "smem_start" and try to map this memory via MALI_IOC_MEM_MAP_EXT ioctl
>>
>> Could this be the problem here?
>
> It does look like that's the case. Looking for smem_start usage, for
> Android its the gralloc code that fetches it:
> https://android.googlesource.com/device/linaro/hikey/+/master/gralloc/framebuffer_device.cpp#353
> and then puts it into the private_handle_t:
> https://android.googlesource.com/device/linaro/hikey/+/master/gralloc/framebuffer_device.cpp#384
>
> Which I assume then gets used later in the opaque mali blob (which
> then results in a similarly opaque hang).
>
> While I'm perfectly understanding that folks are not interested as its
> related to mali (which is fine, I can carry a patch - working to move
> away from fbdev emulation anyway), I am wondering if it might cause
> trouble for other users, as smem_start was previously set and now its
> not, so it seems likely to break userspace examples like:
> https://www.linuxtv.org/downloads/v4l-dvb-apis-old/osd.html
>
> Or are those such old legacy they don't really count?
We've been pretty strict in drm about never ever exposing any physical
offsets to userspace. I'm mildly tempted to plug this even more if
possible, to stop more abuse. If you want mmap, fbdev has that. If you
want buffer sharing, use drm. Don't do buffer sharing by passing
physical addresses around in userspace, that idea is dead since about
8 years (back when we've done the initial dma-buf discussions).
So yeah, not going to care one bit here :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 6:44 ` John Stultz
2018-08-21 8:44 ` Daniel Vetter
@ 2018-08-21 14:15 ` Noralf Trønnes
2018-08-23 4:14 ` John Stultz
2 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-08-21 14:15 UTC (permalink / raw)
To: John Stultz
Cc: Intel Graphics Development, Laurent Pinchart, dri-devel,
David Lechner
Den 21.08.2018 08.44, skrev John Stultz:
> On Tue, Jul 3, 2018 at 9:03 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
>> This switches the CMA helper drivers that use its fbdev emulation over
>> to the generic fbdev emulation. It's the first phase of using generic
>> fbdev. A later phase will use DRM client callbacks for the
>> lastclose/hotplug/remove callbacks.
>>
>> There are currently 2 fbdev init/fini functions:
>> - drm_fb_cma_fbdev_init/drm_fb_cma_fbdev_fini
>> - drm_fbdev_cma_init/drm_fbdev_cma_fini
>>
>> This is because the work on generic fbdev came up during a fbdev
>> refactoring and thus wasn't completed. No point in completing that
>> refactoring when drivers will soon move to drm_fb_helper_generic_probe().
>>
>> tinydrm uses drm_fb_cma_fbdev_init_with_funcs().
>>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>> drivers/gpu/drm/drm_fb_cma_helper.c | 360 +++++-------------------------------
>> include/drm/drm_fb_cma_helper.h | 3 -
>> 2 files changed, 42 insertions(+), 321 deletions(-)
> ...
>> -static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
>> - struct drm_gem_cma_object *cma_obj)
>> -{
>> - struct fb_deferred_io *fbdefio;
>> - struct fb_ops *fbops;
>> -
>> - /*
>> - * Per device structures are needed because:
>> - * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
>> - * fbdefio: individual delays
>> - */
>> - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>> - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>> - if (!fbdefio || !fbops) {
>> - kfree(fbdefio);
>> - kfree(fbops);
>> - return -ENOMEM;
>> - }
>> -
>> - /* can't be offset from vaddr since dirty() uses cma_obj */
>> - fbi->screen_buffer = cma_obj->vaddr;
>> - /* fb_deferred_io_fault() needs a physical address */
>> - fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>> -
>> - *fbops = *fbi->fbops;
>> - fbi->fbops = fbops;
>> -
>> - fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
>> - fbdefio->deferred_io = drm_fb_helper_deferred_io;
>> - fbi->fbdefio = fbdefio;
>> - fb_deferred_io_init(fbi);
>> - fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
>> -
>> - return 0;
>> -}
>> -
>> -static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
>> -{
>> - if (!fbi->fbdefio)
>> - return;
>> -
>> - fb_deferred_io_cleanup(fbi);
>> - kfree(fbi->fbdefio);
>> - kfree(fbi->fbops);
>> -}
>> -
>> -static int
>> -drm_fbdev_cma_create(struct drm_fb_helper *helper,
>> - struct drm_fb_helper_surface_size *sizes)
>> -{
>> - struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper);
>> - struct drm_device *dev = helper->dev;
>> - struct drm_gem_cma_object *obj;
>> - struct drm_framebuffer *fb;
>> - unsigned int bytes_per_pixel;
>> - unsigned long offset;
>> - struct fb_info *fbi;
>> - size_t size;
>> - int ret;
>> -
>> - DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
>> - sizes->surface_width, sizes->surface_height,
>> - sizes->surface_bpp);
>> -
>> - bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>> - size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
>> - obj = drm_gem_cma_create(dev, size);
>> - if (IS_ERR(obj))
>> - return -ENOMEM;
>> -
>> - fbi = drm_fb_helper_alloc_fbi(helper);
>> - if (IS_ERR(fbi)) {
>> - ret = PTR_ERR(fbi);
>> - goto err_gem_free_object;
>> - }
>> -
>> - fb = drm_gem_fbdev_fb_create(dev, sizes, 0, &obj->base,
>> - fbdev_cma->fb_funcs);
>> - if (IS_ERR(fb)) {
>> - dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
>> - ret = PTR_ERR(fb);
>> - goto err_fb_info_destroy;
>> - }
>> -
>> - helper->fb = fb;
>> -
>> - fbi->par = helper;
>> - fbi->flags = FBINFO_FLAG_DEFAULT;
>> - fbi->fbops = &drm_fbdev_cma_ops;
>> -
>> - drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->format->depth);
>> - drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
>> -
>> - offset = fbi->var.xoffset * bytes_per_pixel;
>> - offset += fbi->var.yoffset * fb->pitches[0];
>> -
>> - dev->mode_config.fb_base = (resource_size_t)obj->paddr;
>> - fbi->screen_base = obj->vaddr + offset;
>> - fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
> Hey Noralf, all,
> I've been digging for a bit on the regression that this patch has
> tripped on the HiKey board as reported here:
> https://lkml.org/lkml/2018/8/16/81
>
> The first issue was that the kirin driver was setting
> mode_config.max_width/height = 2048, which was causing errors as the
> the requested resolution was 1920x2160 (due to surfaceflinger
> requesting y*2 for page flipping). This was relatively simple enough
> to figure out and fix, but bumping the values up on its own didn't
> seem to resolve the issue entirely, as I started to see hard hangs on
> bootup when userspace started using the emulated fbdev device.
>
> After confirming reverting the patch here worked around it, I started
> digging through what might be different, and I think I've found it.
> In the drm_fbdev_cma_create() function above, we set the
> fbi->fix.smem_start value, where as in drm_fb_helper_generic_probe(),
> which now replaces that, we don't set smem_start value at all.
>
> Since we don't have a drm_gem_cma_object reference in
> drm_fb_helper_generic_probe(), I instead added:
> fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
>
> And that got things working!
I don't understand how setting smem_start can fix anything.
Is that line the only thing you have added/changed?
AFAICT smem_start is only used as a fall back in fb_mmap() if
fb_ops->fb_mmap is not set (and for defio).
Have I understood it correctly that it is fbdev mmap that is broken?
fbcon is working correctly? If so, have you checked that returning
-ENODEV unconditionally in drm_fbdev_fb_mmap() to block it's usage
prevents the kernel from hanging?
This is fbdev mmap before the change:
static int
drm_fbdev_cma_create(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
{
bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
size = sizes->surface_width * sizes->surface_height * bytes_per_pixel;
[ offset is always zero because of drm_fb_helper_fill_var() ]
dev->mode_config.fb_base = (resource_size_t)obj->paddr;
fbi->screen_base = obj->vaddr + offset;
fbi->fix.smem_start = (unsigned long)(obj->paddr + offset);
fbi->screen_size = size;
fbi->fix.smem_len = size;
static struct fb_ops drm_fbdev_cma_ops = {
.fb_mmap = drm_fb_cma_mmap,
static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma)
{
[ info->device is drm_device->dev ]
return dma_mmap_writecombine(info->device, vma, info->screen_base,
info->fix.smem_start, info->fix.smem_len);
}
#define dma_mmap_writecombine dma_mmap_wc
This is the current:
int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
struct drm_fb_helper_surface_size *sizes)
{
format = drm_mode_legacy_fb_format(sizes->surface_bpp,
sizes->surface_depth);
buffer = drm_client_framebuffer_create(client, sizes->surface_width,
sizes->surface_height, format);
fb = buffer->fb;
fbi->screen_size = fb->height * fb->pitches[0];
fbi->fix.smem_len = fbi->screen_size;
fbi->screen_buffer = buffer->vaddr;
static struct fb_ops drm_fbdev_fb_ops = {
.fb_mmap = drm_fbdev_fb_mmap,
static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct
*vma)
{
struct drm_fb_helper *fb_helper = info->par;
if (fb_helper->dev->driver->gem_prime_mmap)
return
fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma);
else
return -ENODEV;
}
static struct drm_driver kirin_drm_driver = {
.gem_prime_mmap = drm_gem_cma_prime_mmap,
int drm_gem_cma_prime_mmap(struct drm_gem_object *obj,
struct vm_area_struct *vma)
{
struct drm_gem_cma_object *cma_obj;
int ret;
ret = drm_gem_mmap_obj(obj, obj->size, vma);
if (ret < 0)
return ret;
cma_obj = to_drm_gem_cma_obj(obj);
return drm_gem_cma_mmap_obj(cma_obj, vma);
}
static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj,
struct vm_area_struct *vma)
{
int ret;
/*
* Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
* vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want
to map
* the whole buffer.
*/
vma->vm_flags &= ~VM_PFNMAP;
vma->vm_pgoff = 0;
ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr,
cma_obj->paddr, vma->vm_end - vma->vm_start);
if (ret)
drm_gem_vm_close(vma);
return ret;
}
I see one difference and that is: vma->vm_pgoff = 0;
I don't know if that can affect the outcome.
Noralf.
> So yea, I wanted to figure out if we are just missing the line above
> in drm_fb_helper_generic_probe()? Or if the kirin driver should be
> setting the fix.smem_start in some other fashion?
>
> thanks
> -john
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-21 6:44 ` John Stultz
2018-08-21 8:44 ` Daniel Vetter
2018-08-21 14:15 ` Noralf Trønnes
@ 2018-08-23 4:14 ` John Stultz
2018-08-23 5:51 ` Daniel Vetter
` (2 more replies)
2 siblings, 3 replies; 40+ messages in thread
From: John Stultz @ 2018-08-23 4:14 UTC (permalink / raw)
To: Noralf Trønnes
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Laurent Pinchart
On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org> wrote:
> Hey Noralf, all,
> I've been digging for a bit on the regression that this patch has
> tripped on the HiKey board as reported here:
> https://lkml.org/lkml/2018/8/16/81
>
> The first issue was that the kirin driver was setting
> mode_config.max_width/height = 2048, which was causing errors as the
> the requested resolution was 1920x2160 (due to surfaceflinger
> requesting y*2 for page flipping).
Hey Noralf,
Sorry, I know your probably sick of me. But I just wanted to circle
around on this little bit. So part of the issue I found earlier, was
that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
Surfaceflinger's request for page flipping. This is what makes the Y
resolution 2160, which runs afoul of the new max_height check of 2048
in the generic code.
I was checking with Xinliang, who know the kirin display hardware,
about the max_height being set to 2048 to ensure bumping it up wasn't
a problem, but he said 2048x2048 was unfortunately not arbitrary, and
that was the hard limit of the display hardware. However, with
overalloc, the 1920x2160 res fbdev should still be ok, as only
1920x1080 is actually displayed at one time.
So it seems like we might need to multiply the max_height by the
overalloc factor when we are checking it in
drm_internal_framebuffer_create?
Does that approach sound sane, or would folks prefer something different?
thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 4:14 ` John Stultz
@ 2018-08-23 5:51 ` Daniel Vetter
2018-08-23 6:21 ` John Stultz
2018-08-23 7:46 ` Laurent Pinchart
2018-08-23 17:24 ` Ville Syrjälä
2 siblings, 1 reply; 40+ messages in thread
From: Daniel Vetter @ 2018-08-23 5:51 UTC (permalink / raw)
To: John Stultz
Cc: David Lechner, Intel Graphics Development, dri-devel,
Laurent Pinchart
On Thu, Aug 23, 2018 at 6:14 AM, John Stultz <john.stultz@linaro.org> wrote:
> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org> wrote:
>> Hey Noralf, all,
>> I've been digging for a bit on the regression that this patch has
>> tripped on the HiKey board as reported here:
>> https://lkml.org/lkml/2018/8/16/81
>>
>> The first issue was that the kirin driver was setting
>> mode_config.max_width/height = 2048, which was causing errors as the
>> the requested resolution was 1920x2160 (due to surfaceflinger
>> requesting y*2 for page flipping).
>
> Hey Noralf,
> Sorry, I know your probably sick of me. But I just wanted to circle
> around on this little bit. So part of the issue I found earlier, was
> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
> Surfaceflinger's request for page flipping. This is what makes the Y
> resolution 2160, which runs afoul of the new max_height check of 2048
> in the generic code.
>
> I was checking with Xinliang, who know the kirin display hardware,
> about the max_height being set to 2048 to ensure bumping it up wasn't
> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
> that was the hard limit of the display hardware. However, with
> overalloc, the 1920x2160 res fbdev should still be ok, as only
> 1920x1080 is actually displayed at one time.
>
> So it seems like we might need to multiply the max_height by the
> overalloc factor when we are checking it in
> drm_internal_framebuffer_create?
>
> Does that approach sound sane, or would folks prefer something different?
I guess we could simply not check against the height limit when
allocating framebuffers. But we've done that for userspace buffers
since forever (they just allocate 2 buffers for page-flipping), so I
have no idea what would all break if we'd suddenly lift this
restriction. And whether we'd lift it for fbdev only or for everyone
doesn't really make much of a difference, since either this works, or
it doesn't (across all chips).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 5:51 ` Daniel Vetter
@ 2018-08-23 6:21 ` John Stultz
2018-08-23 7:37 ` Daniel Vetter
0 siblings, 1 reply; 40+ messages in thread
From: John Stultz @ 2018-08-23 6:21 UTC (permalink / raw)
To: Daniel Vetter
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes, Laurent Pinchart
On Wed, Aug 22, 2018 at 10:51 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Aug 23, 2018 at 6:14 AM, John Stultz <john.stultz@linaro.org> wrote:
>> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org> wrote:
>>> Hey Noralf, all,
>>> I've been digging for a bit on the regression that this patch has
>>> tripped on the HiKey board as reported here:
>>> https://lkml.org/lkml/2018/8/16/81
>>>
>>> The first issue was that the kirin driver was setting
>>> mode_config.max_width/height = 2048, which was causing errors as the
>>> the requested resolution was 1920x2160 (due to surfaceflinger
>>> requesting y*2 for page flipping).
>>
>> Hey Noralf,
>> Sorry, I know your probably sick of me. But I just wanted to circle
>> around on this little bit. So part of the issue I found earlier, was
>> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
>> Surfaceflinger's request for page flipping. This is what makes the Y
>> resolution 2160, which runs afoul of the new max_height check of 2048
>> in the generic code.
>>
>> I was checking with Xinliang, who know the kirin display hardware,
>> about the max_height being set to 2048 to ensure bumping it up wasn't
>> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
>> that was the hard limit of the display hardware. However, with
>> overalloc, the 1920x2160 res fbdev should still be ok, as only
>> 1920x1080 is actually displayed at one time.
>>
>> So it seems like we might need to multiply the max_height by the
>> overalloc factor when we are checking it in
>> drm_internal_framebuffer_create?
>>
>> Does that approach sound sane, or would folks prefer something different?
>
> I guess we could simply not check against the height limit when
> allocating framebuffers. But we've done that for userspace buffers
> since forever (they just allocate 2 buffers for page-flipping), so I
> have no idea what would all break if we'd suddenly lift this
> restriction. And whether we'd lift it for fbdev only or for everyone
> doesn't really make much of a difference, since either this works, or
> it doesn't (across all chips).
That feels a bit more risky then what I was thinking. What about
something like (apologies, whitespace corrupted)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index fe7e545..0424a71 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1810,6 +1810,7 @@ static int drm_fb_helper_single_fb_probe(struct
drm_fb_helper *fb_helper,
int i;
struct drm_fb_helper_surface_size sizes;
int gamma_size = 0;
+ struct drm_mode_config *config;
memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
sizes.surface_depth = 24;
@@ -1910,6 +1911,11 @@ static int drm_fb_helper_single_fb_probe(struct
drm_fb_helper *fb_helper,
sizes.surface_height *= drm_fbdev_overalloc;
sizes.surface_height /= 100;
+ config = &fb_helper->client.dev->mode_config;
+ config->max_height *= drm_fbdev_overalloc;
+ config->max_height /= 100;
+
+
/* push down into drivers */
ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
if (ret < 0)
That way it only effects the fbdev + overalloc case?
thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 6:21 ` John Stultz
@ 2018-08-23 7:37 ` Daniel Vetter
2018-08-23 16:49 ` Noralf Trønnes
0 siblings, 1 reply; 40+ messages in thread
From: Daniel Vetter @ 2018-08-23 7:37 UTC (permalink / raw)
To: John Stultz
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes, Laurent Pinchart
On Wed, Aug 22, 2018 at 11:21:11PM -0700, John Stultz wrote:
> On Wed, Aug 22, 2018 at 10:51 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Aug 23, 2018 at 6:14 AM, John Stultz <john.stultz@linaro.org> wrote:
> >> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org> wrote:
> >>> Hey Noralf, all,
> >>> I've been digging for a bit on the regression that this patch has
> >>> tripped on the HiKey board as reported here:
> >>> https://lkml.org/lkml/2018/8/16/81
> >>>
> >>> The first issue was that the kirin driver was setting
> >>> mode_config.max_width/height = 2048, which was causing errors as the
> >>> the requested resolution was 1920x2160 (due to surfaceflinger
> >>> requesting y*2 for page flipping).
> >>
> >> Hey Noralf,
> >> Sorry, I know your probably sick of me. But I just wanted to circle
> >> around on this little bit. So part of the issue I found earlier, was
> >> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
> >> Surfaceflinger's request for page flipping. This is what makes the Y
> >> resolution 2160, which runs afoul of the new max_height check of 2048
> >> in the generic code.
> >>
> >> I was checking with Xinliang, who know the kirin display hardware,
> >> about the max_height being set to 2048 to ensure bumping it up wasn't
> >> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
> >> that was the hard limit of the display hardware. However, with
> >> overalloc, the 1920x2160 res fbdev should still be ok, as only
> >> 1920x1080 is actually displayed at one time.
> >>
> >> So it seems like we might need to multiply the max_height by the
> >> overalloc factor when we are checking it in
> >> drm_internal_framebuffer_create?
> >>
> >> Does that approach sound sane, or would folks prefer something different?
> >
> > I guess we could simply not check against the height limit when
> > allocating framebuffers. But we've done that for userspace buffers
> > since forever (they just allocate 2 buffers for page-flipping), so I
> > have no idea what would all break if we'd suddenly lift this
> > restriction. And whether we'd lift it for fbdev only or for everyone
> > doesn't really make much of a difference, since either this works, or
> > it doesn't (across all chips).
>
> That feels a bit more risky then what I was thinking. What about
> something like (apologies, whitespace corrupted)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index fe7e545..0424a71 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1810,6 +1810,7 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> int i;
> struct drm_fb_helper_surface_size sizes;
> int gamma_size = 0;
> + struct drm_mode_config *config;
>
> memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
> sizes.surface_depth = 24;
> @@ -1910,6 +1911,11 @@ static int drm_fb_helper_single_fb_probe(struct
> drm_fb_helper *fb_helper,
> sizes.surface_height *= drm_fbdev_overalloc;
> sizes.surface_height /= 100;
>
> + config = &fb_helper->client.dev->mode_config;
> + config->max_height *= drm_fbdev_overalloc;
> + config->max_height /= 100;
> +
> +
> /* push down into drivers */
> ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
> if (ret < 0)
>
>
> That way it only effects the fbdev + overalloc case?
Still changes it for everyone, not just fbdev, if you enable overalloc.
You'd need to reset.
Another, cleaner way to fix this would be to overallocate the buffer, but
have the drm_framebuffer limited. But that means we need to change the
fbdev scrolling logic. And the entire interface between fbdev helpers and
drivers needs a rework, since atm the driver allocates the drm_framebuffer
for you. That's what userspace can/will do in this case I guess. Has all
the issues of scrolling by moving the drm_fb without hw knowledge.
I guess maybe just dropping the max_height check in fbdev is ok, if we put
a really big comment/FIXME there. Or maybe make it conditional on
fbdev_overalloc being at the default value, that'd probably be better
even.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 7:37 ` Daniel Vetter
@ 2018-08-23 16:49 ` Noralf Trønnes
2018-08-23 17:15 ` Daniel Vetter
0 siblings, 1 reply; 40+ messages in thread
From: Noralf Trønnes @ 2018-08-23 16:49 UTC (permalink / raw)
To: Daniel Vetter, John Stultz
Cc: Xinliang Liu, Intel Graphics Development, Laurent Pinchart,
dri-devel, David Lechner
Den 23.08.2018 09.37, skrev Daniel Vetter:
> On Wed, Aug 22, 2018 at 11:21:11PM -0700, John Stultz wrote:
>> On Wed, Aug 22, 2018 at 10:51 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> On Thu, Aug 23, 2018 at 6:14 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>>> Hey Noralf, all,
>>>>> I've been digging for a bit on the regression that this patch has
>>>>> tripped on the HiKey board as reported here:
>>>>> https://lkml.org/lkml/2018/8/16/81
>>>>>
>>>>> The first issue was that the kirin driver was setting
>>>>> mode_config.max_width/height = 2048, which was causing errors as the
>>>>> the requested resolution was 1920x2160 (due to surfaceflinger
>>>>> requesting y*2 for page flipping).
>>>> Hey Noralf,
>>>> Sorry, I know your probably sick of me. But I just wanted to circle
>>>> around on this little bit. So part of the issue I found earlier, was
>>>> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
>>>> Surfaceflinger's request for page flipping. This is what makes the Y
>>>> resolution 2160, which runs afoul of the new max_height check of 2048
>>>> in the generic code.
>>>>
>>>> I was checking with Xinliang, who know the kirin display hardware,
>>>> about the max_height being set to 2048 to ensure bumping it up wasn't
>>>> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
>>>> that was the hard limit of the display hardware. However, with
>>>> overalloc, the 1920x2160 res fbdev should still be ok, as only
>>>> 1920x1080 is actually displayed at one time.
>>>>
>>>> So it seems like we might need to multiply the max_height by the
>>>> overalloc factor when we are checking it in
>>>> drm_internal_framebuffer_create?
>>>>
>>>> Does that approach sound sane, or would folks prefer something different?
>>> I guess we could simply not check against the height limit when
>>> allocating framebuffers. But we've done that for userspace buffers
>>> since forever (they just allocate 2 buffers for page-flipping), so I
>>> have no idea what would all break if we'd suddenly lift this
>>> restriction. And whether we'd lift it for fbdev only or for everyone
>>> doesn't really make much of a difference, since either this works, or
>>> it doesn't (across all chips).
>> That feels a bit more risky then what I was thinking. What about
>> something like (apologies, whitespace corrupted)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index fe7e545..0424a71 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1810,6 +1810,7 @@ static int drm_fb_helper_single_fb_probe(struct
>> drm_fb_helper *fb_helper,
>> int i;
>> struct drm_fb_helper_surface_size sizes;
>> int gamma_size = 0;
>> + struct drm_mode_config *config;
>>
>> memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
>> sizes.surface_depth = 24;
>> @@ -1910,6 +1911,11 @@ static int drm_fb_helper_single_fb_probe(struct
>> drm_fb_helper *fb_helper,
>> sizes.surface_height *= drm_fbdev_overalloc;
>> sizes.surface_height /= 100;
>>
>> + config = &fb_helper->client.dev->mode_config;
>> + config->max_height *= drm_fbdev_overalloc;
>> + config->max_height /= 100;
>> +
>> +
>> /* push down into drivers */
>> ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
>> if (ret < 0)
>>
>>
>> That way it only effects the fbdev + overalloc case?
> Still changes it for everyone, not just fbdev, if you enable overalloc.
> You'd need to reset.
>
> Another, cleaner way to fix this would be to overallocate the buffer, but
> have the drm_framebuffer limited. But that means we need to change the
> fbdev scrolling logic. And the entire interface between fbdev helpers and
> drivers needs a rework, since atm the driver allocates the drm_framebuffer
> for you. That's what userspace can/will do in this case I guess. Has all
> the issues of scrolling by moving the drm_fb without hw knowledge.
>
> I guess maybe just dropping the max_height check in fbdev is ok, if we put
> a really big comment/FIXME there. Or maybe make it conditional on
> fbdev_overalloc being at the default value, that'd probably be better
> even.
Maybe something like this could work:
int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
struct drm_fb_helper_surface_size *sizes)
{
DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
sizes->surface_width, sizes->surface_height,
sizes->surface_bpp);
format = drm_mode_legacy_fb_format(sizes->surface_bpp,
sizes->surface_depth);
if (drm_fbdev_overalloc > 100 &&
sizes->surface_height >
fb_helper->client.dev->mode_config.max_height) {
u32 divisor = drm_fbdev_overalloc / 100;
buffer = drm_client_buffer_create(client, sizes->surface_width,
sizes->surface_height, format);
if (IS_ERR(buffer))
return PTR_ERR(buffer);
ret = drm_client_buffer_addfb(buffer, sizes->surface_width,
sizes->surface_height / divisor, format);
if (ret) {
drm_client_buffer_delete(buffer);
return ret;
}
buffer->fb->height = sizes->surface_height;
} else {
buffer = drm_client_framebuffer_create(client,
sizes->surface_width,
sizes->surface_height, format);
if (IS_ERR(buffer))
return PTR_ERR(buffer);
}
Noralf.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 16:49 ` Noralf Trønnes
@ 2018-08-23 17:15 ` Daniel Vetter
0 siblings, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2018-08-23 17:15 UTC (permalink / raw)
To: Noralf Trønnes
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, John Stultz, Laurent Pinchart
On Thu, Aug 23, 2018 at 6:49 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Den 23.08.2018 09.37, skrev Daniel Vetter:
>>
>> On Wed, Aug 22, 2018 at 11:21:11PM -0700, John Stultz wrote:
>>>
>>> On Wed, Aug 22, 2018 at 10:51 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>>>
>>>> On Thu, Aug 23, 2018 at 6:14 AM, John Stultz <john.stultz@linaro.org>
>>>> wrote:
>>>>>
>>>>> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org>
>>>>> wrote:
>>>>>>
>>>>>> Hey Noralf, all,
>>>>>> I've been digging for a bit on the regression that this patch has
>>>>>> tripped on the HiKey board as reported here:
>>>>>> https://lkml.org/lkml/2018/8/16/81
>>>>>>
>>>>>> The first issue was that the kirin driver was setting
>>>>>> mode_config.max_width/height = 2048, which was causing errors as the
>>>>>> the requested resolution was 1920x2160 (due to surfaceflinger
>>>>>> requesting y*2 for page flipping).
>>>>>
>>>>> Hey Noralf,
>>>>> Sorry, I know your probably sick of me. But I just wanted to circle
>>>>> around on this little bit. So part of the issue I found earlier, was
>>>>> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
>>>>> Surfaceflinger's request for page flipping. This is what makes the Y
>>>>> resolution 2160, which runs afoul of the new max_height check of 2048
>>>>> in the generic code.
>>>>>
>>>>> I was checking with Xinliang, who know the kirin display hardware,
>>>>> about the max_height being set to 2048 to ensure bumping it up wasn't
>>>>> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
>>>>> that was the hard limit of the display hardware. However, with
>>>>> overalloc, the 1920x2160 res fbdev should still be ok, as only
>>>>> 1920x1080 is actually displayed at one time.
>>>>>
>>>>> So it seems like we might need to multiply the max_height by the
>>>>> overalloc factor when we are checking it in
>>>>> drm_internal_framebuffer_create?
>>>>>
>>>>> Does that approach sound sane, or would folks prefer something
>>>>> different?
>>>>
>>>> I guess we could simply not check against the height limit when
>>>> allocating framebuffers. But we've done that for userspace buffers
>>>> since forever (they just allocate 2 buffers for page-flipping), so I
>>>> have no idea what would all break if we'd suddenly lift this
>>>> restriction. And whether we'd lift it for fbdev only or for everyone
>>>> doesn't really make much of a difference, since either this works, or
>>>> it doesn't (across all chips).
>>>
>>> That feels a bit more risky then what I was thinking. What about
>>> something like (apologies, whitespace corrupted)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>>> b/drivers/gpu/drm/drm_fb_helper.c
>>> index fe7e545..0424a71 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -1810,6 +1810,7 @@ static int drm_fb_helper_single_fb_probe(struct
>>> drm_fb_helper *fb_helper,
>>> int i;
>>> struct drm_fb_helper_surface_size sizes;
>>> int gamma_size = 0;
>>> + struct drm_mode_config *config;
>>>
>>> memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
>>> sizes.surface_depth = 24;
>>> @@ -1910,6 +1911,11 @@ static int drm_fb_helper_single_fb_probe(struct
>>> drm_fb_helper *fb_helper,
>>> sizes.surface_height *= drm_fbdev_overalloc;
>>> sizes.surface_height /= 100;
>>>
>>> + config = &fb_helper->client.dev->mode_config;
>>> + config->max_height *= drm_fbdev_overalloc;
>>> + config->max_height /= 100;
>>> +
>>> +
>>> /* push down into drivers */
>>> ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);
>>> if (ret < 0)
>>>
>>>
>>> That way it only effects the fbdev + overalloc case?
>>
>> Still changes it for everyone, not just fbdev, if you enable overalloc.
>> You'd need to reset.
>>
>> Another, cleaner way to fix this would be to overallocate the buffer, but
>> have the drm_framebuffer limited. But that means we need to change the
>> fbdev scrolling logic. And the entire interface between fbdev helpers and
>> drivers needs a rework, since atm the driver allocates the drm_framebuffer
>> for you. That's what userspace can/will do in this case I guess. Has all
>> the issues of scrolling by moving the drm_fb without hw knowledge.
>>
>> I guess maybe just dropping the max_height check in fbdev is ok, if we put
>> a really big comment/FIXME there. Or maybe make it conditional on
>> fbdev_overalloc being at the default value, that'd probably be better
>> even.
>
>
> Maybe something like this could work:
>
> int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
> struct drm_fb_helper_surface_size *sizes)
> {
> DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
> sizes->surface_width, sizes->surface_height,
> sizes->surface_bpp);
>
> format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> sizes->surface_depth);
>
> if (drm_fbdev_overalloc > 100 &&
> sizes->surface_height >
> fb_helper->client.dev->mode_config.max_height) {
> u32 divisor = drm_fbdev_overalloc / 100;
>
> buffer = drm_client_buffer_create(client, sizes->surface_width,
> sizes->surface_height, format);
> if (IS_ERR(buffer))
> return PTR_ERR(buffer);
>
> ret = drm_client_buffer_addfb(buffer, sizes->surface_width,
> sizes->surface_height / divisor, format);
Doesn't work, because the fbdev code checks against the framebuffer
size when we "flip", i.e. when the viewport gets moved. And atm
there's no support in the fbdev emulation code to re-create the
drm_framebuffer for overallocated buffers. Hence why the proper
solution is a pile more work than just this.
-Daniel
> if (ret) {
> drm_client_buffer_delete(buffer);
> return ret;
> }
>
> buffer->fb->height = sizes->surface_height;
> } else {
> buffer = drm_client_framebuffer_create(client, sizes->surface_width,
> sizes->surface_height, format);
> if (IS_ERR(buffer))
> return PTR_ERR(buffer);
> }
>
>
> Noralf.
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 4:14 ` John Stultz
2018-08-23 5:51 ` Daniel Vetter
@ 2018-08-23 7:46 ` Laurent Pinchart
2018-08-23 8:09 ` Daniel Vetter
2018-08-23 17:38 ` John Stultz
2018-08-23 17:24 ` Ville Syrjälä
2 siblings, 2 replies; 40+ messages in thread
From: Laurent Pinchart @ 2018-08-23 7:46 UTC (permalink / raw)
To: John Stultz; +Cc: David Lechner, Intel Graphics Development, dri-devel
Hi John,
On Thursday, 23 August 2018 07:14:08 EEST John Stultz wrote:
> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz wrote:
> > Hey Noralf, all,
> >
> > I've been digging for a bit on the regression that this patch has
> >
> > tripped on the HiKey board as reported here:
> > https://lkml.org/lkml/2018/8/16/81
> >
> > The first issue was that the kirin driver was setting
> > mode_config.max_width/height = 2048, which was causing errors as the
> > the requested resolution was 1920x2160 (due to surfaceflinger
> > requesting y*2 for page flipping).
>
> Hey Noralf,
> Sorry, I know your probably sick of me. But I just wanted to circle
> around on this little bit. So part of the issue I found earlier, was
> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
> Surfaceflinger's request for page flipping.
Possibly slightly out of topic, but we're in 2018, is there any plan to make
SurfaceFlinger move away from FBDEV ?
> This is what makes the Y resolution 2160, which runs afoul of the new
> max_height check of 2048 in the generic code.
>
> I was checking with Xinliang, who know the kirin display hardware,
> about the max_height being set to 2048 to ensure bumping it up wasn't
> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
> that was the hard limit of the display hardware. However, with
> overalloc, the 1920x2160 res fbdev should still be ok, as only
> 1920x1080 is actually displayed at one time.
>
> So it seems like we might need to multiply the max_height by the
> overalloc factor when we are checking it in
> drm_internal_framebuffer_create?
>
> Does that approach sound sane, or would folks prefer something different?
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 7:46 ` Laurent Pinchart
@ 2018-08-23 8:09 ` Daniel Vetter
2018-08-23 17:48 ` John Stultz
2018-08-23 17:38 ` John Stultz
1 sibling, 1 reply; 40+ messages in thread
From: Daniel Vetter @ 2018-08-23 8:09 UTC (permalink / raw)
To: Laurent Pinchart
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes, John Stultz
On Thu, Aug 23, 2018 at 10:46:15AM +0300, Laurent Pinchart wrote:
> Hi John,
>
> On Thursday, 23 August 2018 07:14:08 EEST John Stultz wrote:
> > On Mon, Aug 20, 2018 at 11:44 PM, John Stultz wrote:
> > > Hey Noralf, all,
> > >
> > > I've been digging for a bit on the regression that this patch has
> > >
> > > tripped on the HiKey board as reported here:
> > > https://lkml.org/lkml/2018/8/16/81
> > >
> > > The first issue was that the kirin driver was setting
> > > mode_config.max_width/height = 2048, which was causing errors as the
> > > the requested resolution was 1920x2160 (due to surfaceflinger
> > > requesting y*2 for page flipping).
> >
> > Hey Noralf,
> > Sorry, I know your probably sick of me. But I just wanted to circle
> > around on this little bit. So part of the issue I found earlier, was
> > that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
> > Surfaceflinger's request for page flipping.
>
> Possibly slightly out of topic, but we're in 2018, is there any plan to make
> SurfaceFlinger move away from FBDEV ?
Is surfaceflinger really using direct fbdev still (maybe for boot-up)? Or
is this just an artifact of the mali blob hwcomposer backend?
-Daniel
>
> > This is what makes the Y resolution 2160, which runs afoul of the new
> > max_height check of 2048 in the generic code.
> >
> > I was checking with Xinliang, who know the kirin display hardware,
> > about the max_height being set to 2048 to ensure bumping it up wasn't
> > a problem, but he said 2048x2048 was unfortunately not arbitrary, and
> > that was the hard limit of the display hardware. However, with
> > overalloc, the 1920x2160 res fbdev should still be ok, as only
> > 1920x1080 is actually displayed at one time.
> >
> > So it seems like we might need to multiply the max_height by the
> > overalloc factor when we are checking it in
> > drm_internal_framebuffer_create?
> >
> > Does that approach sound sane, or would folks prefer something different?
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 8:09 ` Daniel Vetter
@ 2018-08-23 17:48 ` John Stultz
2018-08-23 20:49 ` Laurent Pinchart
0 siblings, 1 reply; 40+ messages in thread
From: John Stultz @ 2018-08-23 17:48 UTC (permalink / raw)
To: Daniel Vetter
Cc: David Lechner, Intel Graphics Development, dri-devel,
Laurent Pinchart
On Thu, Aug 23, 2018 at 1:09 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Aug 23, 2018 at 10:46:15AM +0300, Laurent Pinchart wrote:
>> Hi John,
>>
>> On Thursday, 23 August 2018 07:14:08 EEST John Stultz wrote:
>> > On Mon, Aug 20, 2018 at 11:44 PM, John Stultz wrote:
>> > > Hey Noralf, all,
>> > >
>> > > I've been digging for a bit on the regression that this patch has
>> > >
>> > > tripped on the HiKey board as reported here:
>> > > https://lkml.org/lkml/2018/8/16/81
>> > >
>> > > The first issue was that the kirin driver was setting
>> > > mode_config.max_width/height = 2048, which was causing errors as the
>> > > the requested resolution was 1920x2160 (due to surfaceflinger
>> > > requesting y*2 for page flipping).
>> >
>> > Hey Noralf,
>> > Sorry, I know your probably sick of me. But I just wanted to circle
>> > around on this little bit. So part of the issue I found earlier, was
>> > that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
>> > Surfaceflinger's request for page flipping.
>>
>> Possibly slightly out of topic, but we're in 2018, is there any plan to make
>> SurfaceFlinger move away from FBDEV ?
>
> Is surfaceflinger really using direct fbdev still (maybe for boot-up)? Or
> is this just an artifact of the mali blob hwcomposer backend?
Mostly its due to the simple fbdev being a legacy solution on android
that works out of the box.
I do suspect the android devs hope to retire it, which is why I'm
working on getting things going w/ the drm_hwcomposer right now so we
can get away from the fbdev. But in the meantime, keeping the fbdev
method running is important so we have a functioning device for
testing AOSP w/ mainline.
thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 17:48 ` John Stultz
@ 2018-08-23 20:49 ` Laurent Pinchart
2018-08-23 21:12 ` John Stultz
0 siblings, 1 reply; 40+ messages in thread
From: Laurent Pinchart @ 2018-08-23 20:49 UTC (permalink / raw)
To: John Stultz
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes
Hi John,
On Thursday, 23 August 2018 20:48:40 EEST John Stultz wrote:
> On Thu, Aug 23, 2018 at 1:09 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Aug 23, 2018 at 10:46:15AM +0300, Laurent Pinchart wrote:
> >> On Thursday, 23 August 2018 07:14:08 EEST John Stultz wrote:
> >>> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz wrote:
> >>>> Hey Noralf, all,
> >>>>
> >>>> I've been digging for a bit on the regression that this patch has
> >>>>
> >>>> tripped on the HiKey board as reported here:
> >>>> https://lkml.org/lkml/2018/8/16/81
> >>>>
> >>>> The first issue was that the kirin driver was setting
> >>>> mode_config.max_width/height = 2048, which was causing errors as the
> >>>> the requested resolution was 1920x2160 (due to surfaceflinger
> >>>> requesting y*2 for page flipping).
> >>>
> >>> Hey Noralf,
> >>>
> >>> Sorry, I know your probably sick of me. But I just wanted to circle
> >>>
> >>> around on this little bit. So part of the issue I found earlier, was
> >>> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
> >>> Surfaceflinger's request for page flipping.
> >>
> >> Possibly slightly out of topic, but we're in 2018, is there any plan to
> >> make SurfaceFlinger move away from FBDEV ?
> >
> > Is surfaceflinger really using direct fbdev still (maybe for boot-up)? Or
> > is this just an artifact of the mali blob hwcomposer backend?
>
> Mostly its due to the simple fbdev being a legacy solution on android
> that works out of the box.
> I do suspect the android devs hope to retire it, which is why I'm
> working on getting things going w/ the drm_hwcomposer right now so we
> can get away from the fbdev.
That would be good news. Are there many Android components other than vendor-
specific hwcomposer implementations that still use fbdev ?
> But in the meantime, keeping the fbdev method running is important so we
> have a functioning device for testing AOSP w/ mainline.
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 20:49 ` Laurent Pinchart
@ 2018-08-23 21:12 ` John Stultz
2018-08-24 8:57 ` Laurent Pinchart
0 siblings, 1 reply; 40+ messages in thread
From: John Stultz @ 2018-08-23 21:12 UTC (permalink / raw)
To: Laurent Pinchart
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes
On Thu, Aug 23, 2018 at 1:49 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday, 23 August 2018 20:48:40 EEST John Stultz wrote:
>> On Thu, Aug 23, 2018 at 1:09 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Thu, Aug 23, 2018 at 10:46:15AM +0300, Laurent Pinchart wrote:
>> >> Possibly slightly out of topic, but we're in 2018, is there any plan to
>> >> make SurfaceFlinger move away from FBDEV ?
>> >
>> > Is surfaceflinger really using direct fbdev still (maybe for boot-up)? Or
>> > is this just an artifact of the mali blob hwcomposer backend?
>>
>> Mostly its due to the simple fbdev being a legacy solution on android
>> that works out of the box.
>> I do suspect the android devs hope to retire it, which is why I'm
>> working on getting things going w/ the drm_hwcomposer right now so we
>> can get away from the fbdev.
>
> That would be good news. Are there many Android components other than vendor-
> specific hwcomposer implementations that still use fbdev ?
So yea, I can't really speak about what the various vendors are doing,
as I don't really know, but I'm aware there are still a few (in some
cases major) vendors who still use fbdev on their shipping devices
with their custom hwcomposer code.
Other then that, to my knowledge AOSP only has a default fallback
hwcomposer that uses fbdev, which is what we've used here as we didn't
want to take the vendor's proprietary hwcomposer blob. But again,
moving to the drm_hwcomposer is the shiny bright future, as soon as a
few remaining issues are sorted upstream.
thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 21:12 ` John Stultz
@ 2018-08-24 8:57 ` Laurent Pinchart
0 siblings, 0 replies; 40+ messages in thread
From: Laurent Pinchart @ 2018-08-24 8:57 UTC (permalink / raw)
To: John Stultz
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes
Hi John,
On Friday, 24 August 2018 00:12:46 EEST John Stultz wrote:
> On Thu, Aug 23, 2018 at 1:49 PM, Laurent Pinchart wrote:
> > On Thursday, 23 August 2018 20:48:40 EEST John Stultz wrote:
> >> On Thu, Aug 23, 2018 at 1:09 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >>> On Thu, Aug 23, 2018 at 10:46:15AM +0300, Laurent Pinchart wrote:
> >>>> Possibly slightly out of topic, but we're in 2018, is there any plan
> >>>> to make SurfaceFlinger move away from FBDEV ?
> >>>
> >>> Is surfaceflinger really using direct fbdev still (maybe for boot-up)?
> >>> Or is this just an artifact of the mali blob hwcomposer backend?
> >>
> >> Mostly its due to the simple fbdev being a legacy solution on android
> >> that works out of the box.
> >> I do suspect the android devs hope to retire it, which is why I'm
> >> working on getting things going w/ the drm_hwcomposer right now so we
> >> can get away from the fbdev.
> >
> > That would be good news. Are there many Android components other than
> > vendor- specific hwcomposer implementations that still use fbdev ?
>
> So yea, I can't really speak about what the various vendors are doing,
> as I don't really know, but I'm aware there are still a few (in some
> cases major) vendors who still use fbdev on their shipping devices
> with their custom hwcomposer code.
>
> Other then that, to my knowledge AOSP only has a default fallback
> hwcomposer that uses fbdev, which is what we've used here as we didn't
> want to take the vendor's proprietary hwcomposer blob. But again,
> moving to the drm_hwcomposer is the shiny bright future, as soon as a
> few remaining issues are sorted upstream.
Last time I looked (and that was years ago) the init process also used fbdev
to render the boot splash screen. Is it still the case ? If so is there any
chance you could add a fix for that to your todo list ? :-)
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 7:46 ` Laurent Pinchart
2018-08-23 8:09 ` Daniel Vetter
@ 2018-08-23 17:38 ` John Stultz
1 sibling, 0 replies; 40+ messages in thread
From: John Stultz @ 2018-08-23 17:38 UTC (permalink / raw)
To: Laurent Pinchart
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes
On Thu, Aug 23, 2018 at 12:46 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi John,
>
> On Thursday, 23 August 2018 07:14:08 EEST John Stultz wrote:
>> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz wrote:
>> > Hey Noralf, all,
>> >
>> > I've been digging for a bit on the regression that this patch has
>> >
>> > tripped on the HiKey board as reported here:
>> > https://lkml.org/lkml/2018/8/16/81
>> >
>> > The first issue was that the kirin driver was setting
>> > mode_config.max_width/height = 2048, which was causing errors as the
>> > the requested resolution was 1920x2160 (due to surfaceflinger
>> > requesting y*2 for page flipping).
>>
>> Hey Noralf,
>> Sorry, I know your probably sick of me. But I just wanted to circle
>> around on this little bit. So part of the issue I found earlier, was
>> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
>> Surfaceflinger's request for page flipping.
>
> Possibly slightly out of topic, but we're in 2018, is there any plan to make
> SurfaceFlinger move away from FBDEV ?
Yes. That is actually work-in-progress for both HiKey and HiKey960,
switching to use the drm_hwcomposer once we can get upstream project
into a sane state.
thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 4:14 ` John Stultz
2018-08-23 5:51 ` Daniel Vetter
2018-08-23 7:46 ` Laurent Pinchart
@ 2018-08-23 17:24 ` Ville Syrjälä
2018-08-23 17:42 ` John Stultz
2 siblings, 1 reply; 40+ messages in thread
From: Ville Syrjälä @ 2018-08-23 17:24 UTC (permalink / raw)
To: John Stultz
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes, Laurent Pinchart
On Wed, Aug 22, 2018 at 09:14:08PM -0700, John Stultz wrote:
> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org> wrote:
> > Hey Noralf, all,
> > I've been digging for a bit on the regression that this patch has
> > tripped on the HiKey board as reported here:
> > https://lkml.org/lkml/2018/8/16/81
> >
> > The first issue was that the kirin driver was setting
> > mode_config.max_width/height = 2048, which was causing errors as the
> > the requested resolution was 1920x2160 (due to surfaceflinger
> > requesting y*2 for page flipping).
>
> Hey Noralf,
> Sorry, I know your probably sick of me. But I just wanted to circle
> around on this little bit. So part of the issue I found earlier, was
> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
> Surfaceflinger's request for page flipping. This is what makes the Y
> resolution 2160, which runs afoul of the new max_height check of 2048
> in the generic code.
>
> I was checking with Xinliang, who know the kirin display hardware,
> about the max_height being set to 2048 to ensure bumping it up wasn't
> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
> that was the hard limit of the display hardware. However, with
> overalloc, the 1920x2160 res fbdev should still be ok, as only
> 1920x1080 is actually displayed at one time.
I recently tried to clarify that max_width/height are simply the max
framebuffer dimensions supported by the driver. So it's perfectly legal
for a driver to declare max_height as something big that can't be
scanned out in its entirety by a single plane. For i915 I'm currently
working on bumping these limits to 32k-1 regardless of the hardware
scanout limitations.
So if you're already running with a framebuffer height >2048 and it
works then it would seem to me that you could just bump this limit in
the driver.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation
2018-08-23 17:24 ` Ville Syrjälä
@ 2018-08-23 17:42 ` John Stultz
0 siblings, 0 replies; 40+ messages in thread
From: John Stultz @ 2018-08-23 17:42 UTC (permalink / raw)
To: Ville Syrjälä
Cc: David Lechner, Xinliang Liu, Intel Graphics Development,
dri-devel, Noralf Trønnes, Laurent Pinchart
On Thu, Aug 23, 2018 at 10:24 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Wed, Aug 22, 2018 at 09:14:08PM -0700, John Stultz wrote:
>> On Mon, Aug 20, 2018 at 11:44 PM, John Stultz <john.stultz@linaro.org> wrote:
>> > Hey Noralf, all,
>> > I've been digging for a bit on the regression that this patch has
>> > tripped on the HiKey board as reported here:
>> > https://lkml.org/lkml/2018/8/16/81
>> >
>> > The first issue was that the kirin driver was setting
>> > mode_config.max_width/height = 2048, which was causing errors as the
>> > the requested resolution was 1920x2160 (due to surfaceflinger
>> > requesting y*2 for page flipping).
>>
>> Hey Noralf,
>> Sorry, I know your probably sick of me. But I just wanted to circle
>> around on this little bit. So part of the issue I found earlier, was
>> that I'm running w/ CONFIG_DRM_FBDEV_OVERALLOC=200, to support
>> Surfaceflinger's request for page flipping. This is what makes the Y
>> resolution 2160, which runs afoul of the new max_height check of 2048
>> in the generic code.
>>
>> I was checking with Xinliang, who know the kirin display hardware,
>> about the max_height being set to 2048 to ensure bumping it up wasn't
>> a problem, but he said 2048x2048 was unfortunately not arbitrary, and
>> that was the hard limit of the display hardware. However, with
>> overalloc, the 1920x2160 res fbdev should still be ok, as only
>> 1920x1080 is actually displayed at one time.
>
> I recently tried to clarify that max_width/height are simply the max
> framebuffer dimensions supported by the driver. So it's perfectly legal
> for a driver to declare max_height as something big that can't be
> scanned out in its entirety by a single plane. For i915 I'm currently
> working on bumping these limits to 32k-1 regardless of the hardware
> scanout limitations.
>
> So if you're already running with a framebuffer height >2048 and it
> works then it would seem to me that you could just bump this limit in
> the driver.
Ok. I'm fine with this as long as its not going to cause further trouble.
thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v5 5/8] drm/debugfs: Add internal client debugfs file
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (3 preceding siblings ...)
2018-07-03 16:03 ` [PATCH v5 4/8] drm/cma-helper: Use the generic fbdev emulation Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 6/8] drm/fb-helper: Finish the generic fbdev emulation Noralf Trønnes
` (9 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, laurent.pinchart, david
Print the names of the internal clients currently attached.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/drm_client.c | 28 ++++++++++++++++++++++++++++
drivers/gpu/drm/drm_debugfs.c | 7 +++++++
include/drm/drm_client.h | 3 +++
3 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 743495f7f833..4039a4d103a8 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -385,3 +385,31 @@ void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
drm_client_buffer_delete(buffer);
}
EXPORT_SYMBOL(drm_client_framebuffer_delete);
+
+#ifdef CONFIG_DEBUG_FS
+static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = m->private;
+ struct drm_device *dev = node->minor->dev;
+ struct drm_printer p = drm_seq_file_printer(m);
+ struct drm_client_dev *client;
+
+ mutex_lock(&dev->clientlist_mutex);
+ list_for_each_entry(client, &dev->clientlist, list)
+ drm_printf(&p, "%s\n", client->name);
+ mutex_unlock(&dev->clientlist_mutex);
+
+ return 0;
+}
+
+static const struct drm_info_list drm_client_debugfs_list[] = {
+ { "internal_clients", drm_client_debugfs_internal_clients, 0 },
+};
+
+int drm_client_debugfs_init(struct drm_minor *minor)
+{
+ return drm_debugfs_create_files(drm_client_debugfs_list,
+ ARRAY_SIZE(drm_client_debugfs_list),
+ minor->debugfs_root, minor);
+}
+#endif
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index b2482818fee8..50a20bfc07ea 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -28,6 +28,7 @@
#include <linux/slab.h>
#include <linux/export.h>
+#include <drm/drm_client.h>
#include <drm/drm_debugfs.h>
#include <drm/drm_edid.h>
#include <drm/drm_atomic.h>
@@ -164,6 +165,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
DRM_ERROR("Failed to create framebuffer debugfs file\n");
return ret;
}
+
+ ret = drm_client_debugfs_init(minor);
+ if (ret) {
+ DRM_ERROR("Failed to create client debugfs file\n");
+ return ret;
+ }
}
if (dev->driver->debugfs_init) {
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 671052d80e38..989f8e52864d 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -10,6 +10,7 @@ struct drm_device;
struct drm_file;
struct drm_framebuffer;
struct drm_gem_object;
+struct drm_minor;
struct module;
/**
@@ -133,4 +134,6 @@ struct drm_client_buffer *
drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
+int drm_client_debugfs_init(struct drm_minor *minor);
+
#endif
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v5 6/8] drm/fb-helper: Finish the generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (4 preceding siblings ...)
2018-07-03 16:03 ` [PATCH v5 5/8] drm/debugfs: Add internal client debugfs file Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 7/8] drm/tinydrm: Use drm_fbdev_generic_setup() Noralf Trønnes
` (8 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, laurent.pinchart, david
This adds a drm_fbdev_generic_setup() function that sets up generic
fbdev emulation with client callbacks for restore, hotplug and
unregister.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_fb_helper.c | 117 ++++++++++++++++++++++++++++++++++++++++
include/drm/drm_fb_helper.h | 7 +++
2 files changed, 124 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bea3a0cb324a..e2f0db1432aa 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -67,6 +67,9 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
* helper functions used by many drivers to implement the kernel mode setting
* interfaces.
*
+ * Drivers that support a dumb buffer with a virtual address and mmap support,
+ * should try out the generic fbdev emulation using drm_fbdev_generic_setup().
+ *
* Setup fbdev emulation by calling drm_fb_helper_fbdev_setup() and tear it
* down by calling drm_fb_helper_fbdev_teardown().
*
@@ -3118,6 +3121,120 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
}
EXPORT_SYMBOL(drm_fb_helper_generic_probe);
+static const struct drm_fb_helper_funcs drm_fb_helper_generic_funcs = {
+ .fb_probe = drm_fb_helper_generic_probe,
+};
+
+static void drm_fbdev_client_unregister(struct drm_client_dev *client)
+{
+ struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+
+ if (fb_helper->fbdev) {
+ drm_fb_helper_unregister_fbi(fb_helper);
+ /* drm_fbdev_fb_destroy() takes care of cleanup */
+ return;
+ }
+
+ /* Did drm_fb_helper_fbdev_setup() run? */
+ if (fb_helper->dev)
+ drm_fb_helper_fini(fb_helper);
+
+ drm_client_release(client);
+ kfree(fb_helper);
+}
+
+static int drm_fbdev_client_restore(struct drm_client_dev *client)
+{
+ struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+
+ drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
+
+ return 0;
+}
+
+static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
+{
+ struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+ struct drm_device *dev = client->dev;
+ int ret;
+
+ /* If drm_fb_helper_fbdev_setup() failed, we only try once */
+ if (!fb_helper->dev && fb_helper->funcs)
+ return 0;
+
+ if (dev->fb_helper)
+ return drm_fb_helper_hotplug_event(dev->fb_helper);
+
+ if (!dev->mode_config.num_connector)
+ return 0;
+
+ ret = drm_fb_helper_fbdev_setup(dev, fb_helper, &drm_fb_helper_generic_funcs,
+ fb_helper->preferred_bpp, 0);
+ if (ret) {
+ fb_helper->dev = NULL;
+ fb_helper->fbdev = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct drm_client_funcs drm_fbdev_client_funcs = {
+ .owner = THIS_MODULE,
+ .unregister = drm_fbdev_client_unregister,
+ .restore = drm_fbdev_client_restore,
+ .hotplug = drm_fbdev_client_hotplug,
+};
+
+/**
+ * drm_fb_helper_generic_fbdev_setup() - Setup generic fbdev emulation
+ * @dev: DRM device
+ * @preferred_bpp: Preferred bits per pixel for the device.
+ * @dev->mode_config.preferred_depth is used if this is zero.
+ *
+ * This function sets up generic fbdev emulation for drivers that supports
+ * dumb buffers with a virtual address and that can be mmap'ed.
+ *
+ * Restore, hotplug events and teardown are all taken care of. Drivers that do
+ * suspend/resume need to call drm_fb_helper_set_suspend_unlocked() themselves.
+ * Simple drivers might use drm_mode_config_helper_suspend().
+ *
+ * Drivers that set the dirty callback on their framebuffer will get a shadow
+ * fbdev buffer that is blitted onto the real buffer. This is done in order to
+ * make deferred I/O work with all kinds of buffers.
+ *
+ * This function is safe to call even when there are no connectors present.
+ * Setup will be retried on the next hotplug event.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp)
+{
+ struct drm_fb_helper *fb_helper;
+ int ret;
+
+ if (!drm_fbdev_emulation)
+ return 0;
+
+ fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
+ if (!fb_helper)
+ return -ENOMEM;
+
+ ret = drm_client_new(dev, &fb_helper->client, "fbdev", &drm_fbdev_client_funcs);
+ if (ret) {
+ kfree(fb_helper);
+ return ret;
+ }
+
+ fb_helper->preferred_bpp = preferred_bpp;
+
+ drm_fbdev_client_hotplug(&fb_helper->client);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_fbdev_generic_setup);
+
/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
* but the module doesn't depend on any fb console symbols. At least
* attempt to load fbcon to avoid leaving the system without a usable console.
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index c134bbcfd2e9..5db08c8f1d25 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -354,6 +354,7 @@ void drm_fb_helper_output_poll_changed(struct drm_device *dev);
int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
struct drm_fb_helper_surface_size *sizes);
+int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp);
#else
static inline void drm_fb_helper_prepare(struct drm_device *dev,
struct drm_fb_helper *helper,
@@ -595,6 +596,12 @@ drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
return 0;
}
+static inline int
+drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp)
+{
+ return 0;
+}
+
#endif
static inline int
--
2.15.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v5 7/8] drm/tinydrm: Use drm_fbdev_generic_setup()
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (5 preceding siblings ...)
2018-07-03 16:03 ` [PATCH v5 6/8] drm/fb-helper: Finish the generic fbdev emulation Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-07-03 16:03 ` [PATCH v5 8/8] drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs() Noralf Trønnes
` (7 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Noralf Trønnes, laurent.pinchart, david
Make full use of the generic fbdev client.
Cc: David Lechner <david@lechnology.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 3 +--
drivers/gpu/drm/tinydrm/ili9225.c | 1 -
drivers/gpu/drm/tinydrm/ili9341.c | 1 -
drivers/gpu/drm/tinydrm/mi0283qt.c | 1 -
drivers/gpu/drm/tinydrm/st7586.c | 1 -
drivers/gpu/drm/tinydrm/st7735r.c | 1 -
6 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 24a33bf862fa..19c7f70adfa5 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -204,7 +204,7 @@ static int tinydrm_register(struct tinydrm_device *tdev)
if (ret)
return ret;
- ret = drm_fb_cma_fbdev_init_with_funcs(drm, 0, 0, tdev->fb_funcs);
+ ret = drm_fbdev_generic_setup(drm, 0);
if (ret)
DRM_ERROR("Failed to initialize fbdev: %d\n", ret);
@@ -214,7 +214,6 @@ static int tinydrm_register(struct tinydrm_device *tdev)
static void tinydrm_unregister(struct tinydrm_device *tdev)
{
drm_atomic_helper_shutdown(tdev->drm);
- drm_fb_cma_fbdev_fini(tdev->drm);
drm_dev_unregister(tdev->drm);
}
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index 841c69aba059..455fefe012f5 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -368,7 +368,6 @@ static struct drm_driver ili9225_driver = {
DRIVER_ATOMIC,
.fops = &ili9225_fops,
TINYDRM_GEM_DRIVER_OPS,
- .lastclose = drm_fb_helper_lastclose,
.name = "ili9225",
.desc = "Ilitek ILI9225",
.date = "20171106",
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index 8864dcde6edc..6701037749a7 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -145,7 +145,6 @@ static struct drm_driver ili9341_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
.fops = &ili9341_fops,
TINYDRM_GEM_DRIVER_OPS,
- .lastclose = drm_fb_helper_lastclose,
.debugfs_init = mipi_dbi_debugfs_init,
.name = "ili9341",
.desc = "Ilitek ILI9341",
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 015d03f2acba..d7bb4c5e6657 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -154,7 +154,6 @@ static struct drm_driver mi0283qt_driver = {
DRIVER_ATOMIC,
.fops = &mi0283qt_fops,
TINYDRM_GEM_DRIVER_OPS,
- .lastclose = drm_fb_helper_lastclose,
.debugfs_init = mipi_dbi_debugfs_init,
.name = "mi0283qt",
.desc = "Multi-Inno MI0283QT",
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 5c29e3803ecb..2fcbc3067d71 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -304,7 +304,6 @@ static struct drm_driver st7586_driver = {
DRIVER_ATOMIC,
.fops = &st7586_fops,
TINYDRM_GEM_DRIVER_OPS,
- .lastclose = drm_fb_helper_lastclose,
.debugfs_init = mipi_dbi_debugfs_init,
.name = "st7586",
.desc = "Sitronix ST7586",
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 6c7b15c9da4f..3081bc57c116 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -120,7 +120,6 @@ static struct drm_driver st7735r_driver = {
DRIVER_ATOMIC,
.fops = &st7735r_fops,
TINYDRM_GEM_DRIVER_OPS,
- .lastclose = drm_fb_helper_lastclose,
.debugfs_init = mipi_dbi_debugfs_init,
.name = "st7735r",
.desc = "Sitronix ST7735R",
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v5 8/8] drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (6 preceding siblings ...)
2018-07-03 16:03 ` [PATCH v5 7/8] drm/tinydrm: Use drm_fbdev_generic_setup() Noralf Trønnes
@ 2018-07-03 16:03 ` Noralf Trønnes
2018-07-03 16:15 ` ✗ Fi.CI.CHECKPATCH: warning for drm: Add generic fbdev emulation Patchwork
` (6 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-03 16:03 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Noralf Trønnes, laurent.pinchart, david
Remove drm_fb_cma_fbdev_init_with_funcs(), its only user tinydrm has
moved to drm_fbdev_generic_setup().
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/drm_fb_cma_helper.c | 21 ---------------------
include/drm/drm_fb_cma_helper.h | 3 ---
2 files changed, 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 718c7f961d8a..9da36a6271d3 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -99,27 +99,6 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
}
EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
-/**
- * drm_fb_cma_fbdev_init_with_funcs() - Allocate and initialize fbdev emulation
- * @dev: DRM device
- * @preferred_bpp: Preferred bits per pixel for the device.
- * @dev->mode_config.preferred_depth is used if this is zero.
- * @max_conn_count: Maximum number of connectors.
- * @dev->mode_config.num_connector is used if this is zero.
- * @funcs: Framebuffer functions, in particular a custom dirty() callback.
- * Can be NULL.
- *
- * Returns:
- * Zero on success or negative error code on failure.
- */
-int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
- unsigned int preferred_bpp, unsigned int max_conn_count,
- const struct drm_framebuffer_funcs *funcs)
-{
- return drm_fb_cma_fbdev_init(dev, preferred_bpp, max_conn_count);
-}
-EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init_with_funcs);
-
/**
* drm_fb_cma_fbdev_init() - Allocate and initialize fbdev emulation
* @dev: DRM device
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index a0546c3451f9..96e26e3b9a0c 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -16,9 +16,6 @@ struct drm_mode_fb_cmd2;
struct drm_plane;
struct drm_plane_state;
-int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
- unsigned int preferred_bpp, unsigned int max_conn_count,
- const struct drm_framebuffer_funcs *funcs);
int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
unsigned int max_conn_count);
void drm_fb_cma_fbdev_fini(struct drm_device *dev);
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 40+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for drm: Add generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (7 preceding siblings ...)
2018-07-03 16:03 ` [PATCH v5 8/8] drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs() Noralf Trønnes
@ 2018-07-03 16:15 ` Patchwork
2018-07-03 16:18 ` ✗ Fi.CI.SPARSE: " Patchwork
` (5 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2018-07-03 16:15 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx
== Series Details ==
Series: drm: Add generic fbdev emulation
URL : https://patchwork.freedesktop.org/series/45848/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
ad434a5a0dae drm: Begin an API for in-kernel clients
-:27: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#27:
new file mode 100644
-:32: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#32: FILE: Documentation/gpu/drm-client.rst:1:
+=================
total: 0 errors, 2 warnings, 0 checks, 651 lines checked
781f36c686a2 drm/fb-helper: Add generic fbdev emulation .fb_probe function
05745c28977b drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
da8e3c4bb5c4 drm/cma-helper: Use the generic fbdev emulation
-:359: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#359: FILE: drivers/gpu/drm/drm_fb_cma_helper.c:172:
+struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
+ unsigned int preferred_bpp, unsigned int max_conn_count)
total: 0 errors, 0 warnings, 1 checks, 425 lines checked
a5580869f41f drm/debugfs: Add internal client debugfs file
c62fda127952 drm/fb-helper: Finish the generic fbdev emulation
41a2c1ca0b18 drm/tinydrm: Use drm_fbdev_generic_setup()
5b9c5070474f drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* ✗ Fi.CI.SPARSE: warning for drm: Add generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (8 preceding siblings ...)
2018-07-03 16:15 ` ✗ Fi.CI.CHECKPATCH: warning for drm: Add generic fbdev emulation Patchwork
@ 2018-07-03 16:18 ` Patchwork
2018-07-03 16:32 ` ✓ Fi.CI.BAT: success " Patchwork
` (4 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2018-07-03 16:18 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx
== Series Details ==
Series: drm: Add generic fbdev emulation
URL : https://patchwork.freedesktop.org/series/45848/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Commit: drm: Begin an API for in-kernel clients
Okay!
Commit: drm/fb-helper: Add generic fbdev emulation .fb_probe function
+drivers/gpu/drm/drm_fb_helper.c:1006:20: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_fb_helper.c:1006:20: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_fb_helper.c:1007:20: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_fb_helper.c:1007:20: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_fb_helper.c:1008:20: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_fb_helper.c:1008:20: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_fb_helper.c:1009:20: warning: expression using sizeof(void)
+drivers/gpu/drm/drm_fb_helper.c:1009:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1006:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1006:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1007:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1007:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1008:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1008:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1009:20: warning: expression using sizeof(void)
-drivers/gpu/drm/drm_fb_helper.c:1009:20: warning: expression using sizeof(void)
Commit: drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
Okay!
Commit: drm/cma-helper: Use the generic fbdev emulation
Okay!
Commit: drm/debugfs: Add internal client debugfs file
Okay!
Commit: drm/fb-helper: Finish the generic fbdev emulation
Okay!
Commit: drm/tinydrm: Use drm_fbdev_generic_setup()
Okay!
Commit: drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* ✓ Fi.CI.BAT: success for drm: Add generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (9 preceding siblings ...)
2018-07-03 16:18 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-07-03 16:32 ` Patchwork
2018-07-04 2:38 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2018-07-03 16:32 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx
== Series Details ==
Series: drm: Add generic fbdev emulation
URL : https://patchwork.freedesktop.org/series/45848/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4423 -> Patchwork_9513 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/45848/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9513:
=== IGT changes ===
==== Possible regressions ====
igt@kms_flip@basic-flip-vs-dpms:
{fi-cfl-8109u}: NOTRUN -> DMESG-WARN
igt@kms_flip@basic-flip-vs-modeset:
{fi-cfl-8109u}: NOTRUN -> INCOMPLETE
== Known issues ==
Here are the changes found in Patchwork_9513 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@gem_exec_suspend@basic-s3:
{fi-cfl-8109u}: INCOMPLETE -> PASS
igt@kms_chamelium@dp-edid-read:
fi-kbl-7500u: FAIL (fdo#103841) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
== Participating hosts (45 -> 41) ==
Additional (1): fi-bxt-dsi
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4423 -> Patchwork_9513
CI_DRM_4423: 9b9b45349fe3a36d41586992426d03a238396531 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4533: 199220052af977598033d3810ffb4cc32d377522 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9513: 5b9c5070474f9f74644659bd4a73bb358ce5ecc2 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
5b9c5070474f drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()
41a2c1ca0b18 drm/tinydrm: Use drm_fbdev_generic_setup()
c62fda127952 drm/fb-helper: Finish the generic fbdev emulation
a5580869f41f drm/debugfs: Add internal client debugfs file
da8e3c4bb5c4 drm/cma-helper: Use the generic fbdev emulation
05745c28977b drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
781f36c686a2 drm/fb-helper: Add generic fbdev emulation .fb_probe function
ad434a5a0dae drm: Begin an API for in-kernel clients
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9513/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* ✓ Fi.CI.IGT: success for drm: Add generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (10 preceding siblings ...)
2018-07-03 16:32 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-07-04 2:38 ` Patchwork
2018-07-04 8:19 ` [PATCH v5 0/8] " Daniel Vetter
` (2 subsequent siblings)
14 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2018-07-04 2:38 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx
== Series Details ==
Series: drm: Add generic fbdev emulation
URL : https://patchwork.freedesktop.org/series/45848/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4423_full -> Patchwork_9513_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9513_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9513_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9513_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-blt:
shard-kbl: SKIP -> PASS +1
igt@gem_exec_schedule@deep-vebox:
shard-kbl: PASS -> SKIP +4
== Known issues ==
Here are the changes found in Patchwork_9513_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_eio@reset-stress:
shard-glk: PASS -> FAIL (fdo#105957)
igt@kms_flip_tiling@flip-to-x-tiled:
shard-glk: PASS -> FAIL (fdo#103822, fdo#104724)
igt@kms_flip_tiling@flip-y-tiled:
shard-glk: PASS -> FAIL (fdo#104724)
igt@kms_setmode@basic:
shard-apl: PASS -> FAIL (fdo#99912)
shard-kbl: PASS -> FAIL (fdo#99912)
igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
shard-kbl: PASS -> INCOMPLETE (fdo#103665)
==== Possible fixes ====
igt@gem_ppgtt@blt-vs-render-ctxn:
shard-kbl: INCOMPLETE (fdo#103665, fdo#106023) -> PASS
igt@kms_flip@plain-flip-fb-recreate:
shard-glk: FAIL (fdo#100368) -> PASS
igt@kms_flip_tiling@flip-to-y-tiled:
shard-glk: FAIL (fdo#103822, fdo#104724) -> PASS
igt@kms_vblank@pipe-a-ts-continuation-suspend:
shard-hsw: FAIL (fdo#104894) -> PASS
igt@perf@polling:
shard-hsw: FAIL (fdo#102252) -> PASS
==== Warnings ====
igt@drv_selftest@live_gtt:
shard-glk: INCOMPLETE (fdo#103359, k.org#198133) -> FAIL (fdo#105347)
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
fdo#104894 https://bugs.freedesktop.org/show_bug.cgi?id=104894
fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4423 -> Patchwork_9513
CI_DRM_4423: 9b9b45349fe3a36d41586992426d03a238396531 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4533: 199220052af977598033d3810ffb4cc32d377522 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9513: 5b9c5070474f9f74644659bd4a73bb358ce5ecc2 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9513/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 0/8] drm: Add generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (11 preceding siblings ...)
2018-07-04 2:38 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-07-04 8:19 ` Daniel Vetter
2018-07-10 13:10 ` Noralf Trønnes
2018-08-23 6:25 ` ✗ Fi.CI.BAT: failure for drm: Add generic fbdev emulation (rev2) Patchwork
14 siblings, 0 replies; 40+ messages in thread
From: Daniel Vetter @ 2018-07-04 8:19 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx, laurent.pinchart, dri-devel, david
On Tue, Jul 03, 2018 at 06:03:46PM +0200, Noralf Trønnes wrote:
> This patchset adds generic fbdev emulation for drivers that supports GEM
> based dumb buffers which support .gem_prime_vmap and gem_prime_mmap. An
> API is begun to support in-kernel clients in general.
>
> I've squashed the client patches to ease review.
> All patches have ack's and rb's so I'll apply this next week unless
> something more comes up. It's taken me 6 months to get this done so I
> look forward to getting it applied.
>
> Thanks a lot Daniel for helping me make this happen!
>
> Noralf.
>
> Changes since version 4:
> - Squash the two client patches to ease review.
> - Remove drm_client_put() doc references.
> - Remove drm_client_funcs->release, it's use went away in version 3.
> - Add drm_client_dev_hotplug() doc
>
> Changes since version 3:
> - drm/cma-helper: Use the generic fbdev emulation: Fix error path
> - Remove setting .lastclose in new tinydrm driver ili9341
>
> Changes since version 2:
> - Applied first 3 patches to drm-misc-next
> - Drop client reference counting and only allow the driver to release
> clients.
Quick aside: I like changelogs also in each patch (for the specific
patch), avoids having to jump back&forth to see what's changed ...
-Daniel
>
> Noralf Trønnes (8):
> drm: Begin an API for in-kernel clients
> drm/fb-helper: Add generic fbdev emulation .fb_probe function
> drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
> drm/cma-helper: Use the generic fbdev emulation
> drm/debugfs: Add internal client debugfs file
> drm/fb-helper: Finish the generic fbdev emulation
> drm/tinydrm: Use drm_fbdev_generic_setup()
> drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()
>
> Documentation/gpu/drm-client.rst | 12 +
> Documentation/gpu/index.rst | 1 +
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_client.c | 415 ++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_debugfs.c | 7 +
> drivers/gpu/drm/drm_drv.c | 8 +
> drivers/gpu/drm/drm_fb_cma_helper.c | 379 +++----------------------
> drivers/gpu/drm/drm_fb_helper.c | 316 ++++++++++++++++++++-
> drivers/gpu/drm/drm_file.c | 3 +
> drivers/gpu/drm/drm_probe_helper.c | 3 +
> drivers/gpu/drm/pl111/pl111_drv.c | 2 +
> drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 3 +-
> drivers/gpu/drm/tinydrm/ili9225.c | 1 -
> drivers/gpu/drm/tinydrm/ili9341.c | 1 -
> drivers/gpu/drm/tinydrm/mi0283qt.c | 1 -
> drivers/gpu/drm/tinydrm/st7586.c | 1 -
> drivers/gpu/drm/tinydrm/st7735r.c | 1 -
> include/drm/drm_client.h | 139 ++++++++++
> include/drm/drm_device.h | 21 ++
> include/drm/drm_fb_cma_helper.h | 6 -
> include/drm/drm_fb_helper.h | 38 +++
> 21 files changed, 1007 insertions(+), 353 deletions(-)
> create mode 100644 Documentation/gpu/drm-client.rst
> create mode 100644 drivers/gpu/drm/drm_client.c
> create mode 100644 include/drm/drm_client.h
>
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v5 0/8] drm: Add generic fbdev emulation
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (12 preceding siblings ...)
2018-07-04 8:19 ` [PATCH v5 0/8] " Daniel Vetter
@ 2018-07-10 13:10 ` Noralf Trønnes
2018-08-23 6:25 ` ✗ Fi.CI.BAT: failure for drm: Add generic fbdev emulation (rev2) Patchwork
14 siblings, 0 replies; 40+ messages in thread
From: Noralf Trønnes @ 2018-07-10 13:10 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, laurent.pinchart, david
Den 03.07.2018 18.03, skrev Noralf Trønnes:
> This patchset adds generic fbdev emulation for drivers that supports GEM
> based dumb buffers which support .gem_prime_vmap and gem_prime_mmap. An
> API is begun to support in-kernel clients in general.
>
> I've squashed the client patches to ease review.
> All patches have ack's and rb's so I'll apply this next week unless
> something more comes up. It's taken me 6 months to get this done so I
> look forward to getting it applied.
>
> Thanks a lot Daniel for helping me make this happen!
>
> Noralf.
>
> Changes since version 4:
> - Squash the two client patches to ease review.
> - Remove drm_client_put() doc references.
> - Remove drm_client_funcs->release, it's use went away in version 3.
> - Add drm_client_dev_hotplug() doc
>
> Changes since version 3:
> - drm/cma-helper: Use the generic fbdev emulation: Fix error path
> - Remove setting .lastclose in new tinydrm driver ili9341
>
> Changes since version 2:
> - Applied first 3 patches to drm-misc-next
> - Drop client reference counting and only allow the driver to release
> clients.
>
> Noralf Trønnes (8):
> drm: Begin an API for in-kernel clients
> drm/fb-helper: Add generic fbdev emulation .fb_probe function
> drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
> drm/cma-helper: Use the generic fbdev emulation
> drm/debugfs: Add internal client debugfs file
> drm/fb-helper: Finish the generic fbdev emulation
> drm/tinydrm: Use drm_fbdev_generic_setup()
> drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()
Series applied to drm-misc, thanks for reviewing.
Noralf.
> Documentation/gpu/drm-client.rst | 12 +
> Documentation/gpu/index.rst | 1 +
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_client.c | 415 ++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_debugfs.c | 7 +
> drivers/gpu/drm/drm_drv.c | 8 +
> drivers/gpu/drm/drm_fb_cma_helper.c | 379 +++----------------------
> drivers/gpu/drm/drm_fb_helper.c | 316 ++++++++++++++++++++-
> drivers/gpu/drm/drm_file.c | 3 +
> drivers/gpu/drm/drm_probe_helper.c | 3 +
> drivers/gpu/drm/pl111/pl111_drv.c | 2 +
> drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 3 +-
> drivers/gpu/drm/tinydrm/ili9225.c | 1 -
> drivers/gpu/drm/tinydrm/ili9341.c | 1 -
> drivers/gpu/drm/tinydrm/mi0283qt.c | 1 -
> drivers/gpu/drm/tinydrm/st7586.c | 1 -
> drivers/gpu/drm/tinydrm/st7735r.c | 1 -
> include/drm/drm_client.h | 139 ++++++++++
> include/drm/drm_device.h | 21 ++
> include/drm/drm_fb_cma_helper.h | 6 -
> include/drm/drm_fb_helper.h | 38 +++
> 21 files changed, 1007 insertions(+), 353 deletions(-)
> create mode 100644 Documentation/gpu/drm-client.rst
> create mode 100644 drivers/gpu/drm/drm_client.c
> create mode 100644 include/drm/drm_client.h
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread* ✗ Fi.CI.BAT: failure for drm: Add generic fbdev emulation (rev2)
2018-07-03 16:03 [PATCH v5 0/8] drm: Add generic fbdev emulation Noralf Trønnes
` (13 preceding siblings ...)
2018-07-10 13:10 ` Noralf Trønnes
@ 2018-08-23 6:25 ` Patchwork
14 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2018-08-23 6:25 UTC (permalink / raw)
To: John Stultz; +Cc: intel-gfx
== Series Details ==
Series: drm: Add generic fbdev emulation (rev2)
URL : https://patchwork.freedesktop.org/series/45848/
State : failure
== Summary ==
Applying: drm: Begin an API for in-kernel clients
Using index info to reconstruct a base tree...
M Documentation/gpu/index.rst
M drivers/gpu/drm/Makefile
M drivers/gpu/drm/drm_drv.c
M drivers/gpu/drm/drm_file.c
M drivers/gpu/drm/drm_probe_helper.c
M include/drm/drm_device.h
Falling back to patching base and 3-way merge...
Auto-merging include/drm/drm_client.h
CONFLICT (add/add): Merge conflict in include/drm/drm_client.h
Auto-merging drivers/gpu/drm/drm_client.c
CONFLICT (add/add): Merge conflict in drivers/gpu/drm/drm_client.c
error: Failed to merge in the changes.
Patch failed at 0001 drm: Begin an API for in-kernel clients
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 40+ messages in thread