Den 23.03.2016 18:37, skrev David Herrmann:
Hey
On Wed, Mar 16, 2016 at 2:34 PM, Noralf Trønnes noralf@tronnes.org 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@tronnes.org
[...]
+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