Den 23.03.2016 18:37, skrev David Herrmann:
Hey
On Wed, Mar 16, 2016 at 2:34 PM, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote:
tinydrm provides a very simplified view of DRM for displays that has
onboard video memory and is connected through a slow bus like SPI/I2C.
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
---
[...]
+static struct drm_driver tinydrm_driver = {
+ .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME
+ | DRIVER_ATOMIC,
+ .load = tinydrm_load,
+ .lastclose = tinydrm_lastclose,
+// .unload = tinydrm_unload,
+ .get_vblank_counter = drm_vblank_count,
+// .enable_vblank = tinydrm_enable_vblank,
+// .disable_vblank = tinydrm_disable_vblank,
+ .gem_free_object = drm_gem_cma_free_object,
+ .gem_vm_ops = &drm_gem_cma_vm_ops,
+ .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
+ .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+ .gem_prime_import = drm_gem_prime_import,
+ .gem_prime_export = drm_gem_prime_export,
+ .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
+ .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+ .gem_prime_vmap = drm_gem_cma_prime_vmap,
+ .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
+ .gem_prime_mmap = drm_gem_cma_prime_mmap,
+ .dumb_create = drm_gem_cma_dumb_create,
+ .dumb_map_offset = drm_gem_cma_dumb_map_offset,
+ .dumb_destroy = drm_gem_dumb_destroy,
+ .fops = &tinydrm_fops,
+ .name = "tinydrm",
+ .desc = "tinydrm",
+ .date = "20150916",
Can we just drop "date" and "desc" from new drivers? It doesn't add any value.
+ .major = 1,
+ .minor = 0,
+};
+
+void tinydrm_release(struct tinydrm_device *tdev)
We usually prefer "unregister()" to stay consistent with "register()".
Sure.
+{
+ DRM_DEBUG_KMS("\n");
+
+ if (tdev->deferred)
+ cancel_delayed_work_sync(&tdev->deferred->dwork);
+
+ tinydrm_fbdev_fini(tdev);
+
+ drm_panel_detach(&tdev->panel);
+ drm_panel_remove(&tdev->panel);
+
+ drm_mode_config_cleanup(tdev->base);
+ drm_dev_unregister(tdev->base);
+ drm_dev_unref(tdev->base);
+}
+EXPORT_SYMBOL(tinydrm_release);
+
+int tinydrm_register(struct device *dev, struct tinydrm_device *tdev)
+{
+ struct drm_driver *driver = &tinydrm_driver;
+ struct drm_device *ddev;
+ int ret;
+
+ dev_info(dev, "%s\n", __func__);
+
+dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+ if (WARN_ON(!tdev->dirtyfb))
+ return -EINVAL;
+
+ ddev = drm_dev_alloc(driver, dev);
+ if (!ddev)
+ return -ENOMEM;
+
+ tdev->base = ddev;
+ ddev->dev_private = tdev;
+
+ ret = drm_dev_set_unique(ddev, dev_name(ddev->dev));
+ if (ret)
+ goto err_free;
+
+ ret = drm_dev_register(ddev, 0);
+ if (ret)
+ goto err_free;
Whatever your .load() callback does, do that here and drop it. It is
really not needed. Optionally do it before calling drm_dev_register(),
depending on which semantics you want.
Ok.
In general, this looks very similar to what I did with SimpleDRM.
SimpleDRM was the first drm code I studied and tinydrm started with chunks
of code from it :-)
However, I wonder whether we can make it more modular. Right now it
always adds code for fbdev, CMA, backlight, etc., but as Daniel
mentioned those better live in DRM-core helpers.
Yes I will make the next version more modular.
Instead of trying to guess which parts would fit as core helpers, I just
put everything into one module and posted an RFC hoping to get some
feedback.
I will try an implement the suggestions Daniel has made.
I'll try forward porting the SimpleDRM drivers to it, and let you know
whether it works out.
Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
