Den 15.08.2016 08:59, skrev Daniel Vetter: > On Sun, Aug 14, 2016 at 06:52:04PM +0200, Noralf Trønnes wrote: >> The SimpleDRM driver binds to simple-framebuffer devices and provides a >> DRM/KMS API. It provides only a single CRTC+encoder+connector combination >> plus one initial mode. >> >> Userspace can create dumb-buffers which can be blit into the real >> framebuffer similar to UDL. No access to the real framebuffer is allowed >> (compared to earlier version of this driver) to avoid security issues. >> Furthermore, this way we can support arbitrary modes as long as we have a >> conversion-helper. >> >> The driver was originally written by David Herrmann in 2014. >> My main contribution is to make use of drm_simple_kms_helper and >> rework the probe path to avoid use of the deprecated drm_platform_init() >> and drm_driver.{load,unload}(). >> Additions have also been made for later changes to the Device Tree binding >> document, like support for clocks, regulators and having the node under >> /chosen. This code was lifted verbatim from simplefb.c. >> >> Cc: dh.herrmann at gmail.com >> Cc: libv at skynet.be >> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
<snip> >> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_gem.c >> b/drivers/gpu/drm/simpledrm/simpledrm_gem.c >> new file mode 100644 >> index 0000000..81bd7c5 >> --- /dev/null >> +++ b/drivers/gpu/drm/simpledrm/simpledrm_gem.c <snip> >> +struct sdrm_gem_object *sdrm_gem_alloc_object(struct drm_device *ddev, >> + size_t size) >> +{ >> + struct sdrm_gem_object *obj; >> + >> + WARN_ON(!size || (size & ~PAGE_MASK) != 0); >> + >> + obj = kzalloc(sizeof(*obj), GFP_KERNEL); >> + if (!obj) >> + return NULL; >> + >> + drm_gem_private_object_init(ddev, &obj->base, size); >> + return obj; >> +} >> + >> +void sdrm_gem_free_object(struct drm_gem_object *gobj) >> +{ >> + struct sdrm_gem_object *obj = to_sdrm_bo(gobj); >> + struct drm_device *ddev = gobj->dev; >> + >> + if (obj->pages) { >> + /* kill all user-space mappings */ >> + drm_vma_node_unmap(&gobj->vma_node, >> + ddev->anon_inode->i_mapping); >> + sdrm_gem_put_pages(obj); >> + } >> + >> + if (gobj->import_attach) >> + drm_prime_gem_destroy(gobj, obj->sg); >> + >> + drm_gem_free_mmap_offset(gobj); >> + drm_gem_object_release(gobj); >> + kfree(obj); >> +} >> + >> +int sdrm_dumb_create(struct drm_file *dfile, struct drm_device *ddev, >> + struct drm_mode_create_dumb *args) >> +{ >> + struct sdrm_gem_object *obj; >> + int r; >> + >> + if (args->flags) >> + return -EINVAL; >> + >> + /* overflow checks are done by DRM core */ >> + args->pitch = (args->bpp + 7) / 8 * args->width; >> + args->size = PAGE_ALIGN(args->pitch * args->height); >> + >> + obj = sdrm_gem_alloc_object(ddev, args->size); >> + if (!obj) >> + return -ENOMEM; >> + >> + r = drm_gem_handle_create(dfile, &obj->base, &args->handle); >> + if (r) { >> + drm_gem_object_unreference_unlocked(&obj->base); >> + return r; >> + } >> + >> + /* handle owns a reference */ >> + drm_gem_object_unreference_unlocked(&obj->base); >> + return 0; >> +} >> + >> +int sdrm_dumb_destroy(struct drm_file *dfile, struct drm_device *ddev, >> + uint32_t handle) >> +{ >> + return drm_gem_handle_delete(dfile, handle); >> +} > I wonder whether some dumb+gem helpers wouldn't be useful to roll out. At > least drm_dumb_gem_destroy.c would be pretty simple. There's already a drm_gem_dumb_destroy() in drm_gem.c The drm_driver->gem_create_object callback makes it possible to do a generic dumb create: int drm_gem_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args) { struct drm_gem_object *obj; int ret; if (!dev->driver->gem_create_object) return -EINVAL; args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8); args->size = args->pitch * args->height; obj = dev->driver->gem_create_object(dev, args->size); if (!obj) return -ENOMEM; ret = drm_gem_handle_create(file, obj, &args->handle); drm_gem_object_unreference_unlocked(obj); return ret; } EXPORT_SYMBOL(drm_gem_dumb_create); struct drm_gem_object *sdrm_gem_create_object(struct drm_device *ddev, size_t size) { struct sdrm_gem_object *sobj; sobj = sdrm_gem_alloc_object(ddev, size); if (!sobj) return ERR_PTR(-ENOMEM); return &sobj->base; } >> + >> +int sdrm_dumb_map_offset(struct drm_file *dfile, struct drm_device *ddev, >> + uint32_t handle, uint64_t *offset) >> +{ >> + struct drm_gem_object *gobj; >> + int r; >> + >> + mutex_lock(&ddev->struct_mutex); > There's still some struct mutex here. > >> + >> + gobj = drm_gem_object_lookup(dfile, handle); >> + if (!gobj) { >> + r = -ENOENT; >> + goto out_unlock; >> + } >> + >> + r = drm_gem_create_mmap_offset(gobj); >> + if (r) >> + goto out_unref; >> + >> + *offset = drm_vma_node_offset_addr(&gobj->vma_node); >> + >> +out_unref: >> + drm_gem_object_unreference(gobj); >> +out_unlock: >> + mutex_unlock(&ddev->struct_mutex); >> + return r; >> +} >> + Maybe this addition to drm_gem.c as well: int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, uint32_t handle, uint64_t *offset) { struct drm_gem_object *obj; int ret; obj = drm_gem_object_lookup(file, handle); if (!obj) return -ENOENT; ret = drm_gem_create_mmap_offset(obj); if (ret) goto out_unref; *offset = drm_vma_node_offset_addr(&obj->vma_node); out_unref: drm_gem_object_unreference_unlocked(obj); return ret; } >> +int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma) >> +{ >> + struct drm_file *priv = filp->private_data; >> + struct drm_device *dev = priv->minor->dev; >> + struct drm_vma_offset_node *node; >> + struct drm_gem_object *gobj; >> + struct sdrm_gem_object *obj; >> + size_t size, i, num; >> + int r; >> + >> + if (drm_device_is_unplugged(dev)) >> + return -ENODEV; >> + >> + drm_vma_offset_lock_lookup(dev->vma_offset_manager); >> + node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager, >> + vma->vm_pgoff, >> + vma_pages(vma)); >> + drm_vma_offset_unlock_lookup(dev->vma_offset_manager); >> + >> + if (!drm_vma_node_is_allowed(node, filp)) >> + return -EACCES; >> + >> + gobj = container_of(node, struct drm_gem_object, vma_node); >> + obj = to_sdrm_bo(gobj); >> + size = drm_vma_node_size(node) << PAGE_SHIFT; >> + if (size < vma->vm_end - vma->vm_start) >> + return r; >> + >> + r = sdrm_gem_get_pages(obj); >> + if (r < 0) >> + return r; >> + >> + /* prevent dmabuf-imported mmap to user-space */ >> + if (!obj->pages) >> + return -EACCES; >> + >> + vma->vm_flags |= VM_DONTEXPAND; >> + vma->vm_page_prot = >> pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); >> + >> + num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT; >> + for (i = 0; i < num; ++i) { >> + r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE, >> + obj->pages[i]); >> + if (r < 0) { >> + if (i > 0) >> + zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE); >> + return r; >> + } >> + } >> + >> + return 0; >> +} > Why can't we just redirect to the underlying shmem mmap here (after > argument checking)? I don't know what that means, but looking at vc4 it seems I can use drm_gem_mmap() to remove some boilerplate. int sdrm_drm_mmap(struct file *filp, struct vm_area_struct *vma) { unsigned long vm_flags = vma->vm_flags; struct sdrm_gem_object *sobj; struct drm_gem_object *obj; size_t i, num; int r; r = drm_gem_mmap(filp, vma); if (r) return r; obj = vma->vm_private_data; sobj = to_sdrm_bo(obj); r = sdrm_gem_get_pages(obj); if (r < 0) return r; /* prevent dmabuf-imported mmap to user-space */ if (!obj->pages) return -EACCES; /* drm_gem_mmap() sets flags that we don't want */ vma->vm_flags = vm_flags | VM_DONTEXPAND; vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT; for (i = 0; i < num; ++i) { r = vm_insert_page(vma, vma->vm_start + i * PAGE_SIZE, obj->pages[i]); if (r < 0) { if (i > 0) zap_vma_ptes(vma, vma->vm_start, i * PAGE_SIZE); return r; } } return 0; } drm_gem_mmap() requires drm_driver->gem_vm_ops to be set: const struct vm_operations_struct sdrm_gem_vm_ops = { .open = drm_gem_vm_open, .close = drm_gem_vm_close, }; And browsing the code a bit more shows that tegra, udl, etnaviv and vgem does the vm_insert_page() thing in the vm_operations_struct->fault() handler. So maybe this makes sense for simpledrm as well: static int sdrm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) { struct drm_gem_object *obj = vma->vm_private_data; struct sdrm_gem_object *sobj = to_sdrm_bo(obj); loff_t num_pages; pgoff_t offset; int r; if (!sobj->pages) return VM_FAULT_SIGBUS; offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >> PAGE_SHIFT; num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE); if (page_offset > num_pages) return VM_FAULT_SIGBUS; r = vm_insert_page(vma, (unsigned long)vmf->virtual_address, sobj->pages[offset]); switch (r) { case -EAGAIN: case 0: case -ERESTARTSYS: case -EINTR: case -EBUSY: return VM_FAULT_NOPAGE; case -ENOMEM: return VM_FAULT_OOM; } return VM_FAULT_SIGBUS; } Noralf. >> + >> +struct drm_gem_object *sdrm_gem_prime_import(struct drm_device *ddev, >> + struct dma_buf *dma_buf) >> +{ >> + struct dma_buf_attachment *attach; >> + struct sdrm_gem_object *obj; >> + struct sg_table *sg; >> + int ret; >> + >> + /* need to attach */ >> + attach = dma_buf_attach(dma_buf, ddev->dev); >> + if (IS_ERR(attach)) >> + return ERR_CAST(attach); >> + >> + get_dma_buf(dma_buf); >> + >> + sg = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL); >> + if (IS_ERR(sg)) { >> + ret = PTR_ERR(sg); >> + goto fail_detach; >> + } >> + >> + /* >> + * dma_buf_vmap() gives us a page-aligned mapping, so lets bump the >> + * size of the dma-buf to the next page-boundary >> + */ > dma_buf should be page-aligned already. Not sure we enforce that, but it's > in practice a given. > >> + obj = sdrm_gem_alloc_object(ddev, PAGE_ALIGN(dma_buf->size)); >> + if (!obj) { >> + ret = -ENOMEM; >> + goto fail_unmap; >> + } >> + >> + obj->sg = sg; >> + obj->base.import_attach = attach; >> + >> + return &obj->base; >> + >> +fail_unmap: >> + dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL); >> +fail_detach: >> + dma_buf_detach(dma_buf, attach); >> + dma_buf_put(dma_buf); >> + return ERR_PTR(ret); >> +} >> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_kms.c >> b/drivers/gpu/drm/simpledrm/simpledrm_kms.c >> new file mode 100644 >> index 0000000..00e50d9 >> --- /dev/null >> +++ b/drivers/gpu/drm/simpledrm/simpledrm_kms.c >> @@ -0,0 +1,258 @@ >> +/* >> + * SimpleDRM firmware framebuffer driver >> + * Copyright (c) 2012-2014 David Herrmann <dh.herrmann at gmail.com> >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License as published by the >> Free >> + * Software Foundation; either version 2 of the License, or (at your option) >> + * any later version. >> + */ >> + >> +#include <drm/drmP.h> >> +#include <drm/drm_atomic_helper.h> >> +#include <drm/drm_crtc.h> >> +#include <drm/drm_crtc_helper.h> >> +#include <drm/drm_gem.h> >> +#include <drm/drm_simple_kms_helper.h> >> +#include <linux/slab.h> >> + >> +#include "simpledrm.h" >> + >> +static const uint32_t sdrm_formats[] = { >> + DRM_FORMAT_RGB565, >> + DRM_FORMAT_ARGB8888, >> + DRM_FORMAT_XRGB8888, >> +}; >> + >> +static int sdrm_conn_get_modes(struct drm_connector *conn) >> +{ >> + struct sdrm_device *sdrm = conn->dev->dev_private; >> + struct drm_display_mode *mode; >> + >> + mode = drm_cvt_mode(sdrm->ddev, sdrm->fb_width, sdrm->fb_height, >> + 60, false, false, false); >> + if (!mode) >> + return 0; >> + >> + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; >> + drm_mode_set_name(mode); >> + drm_mode_probed_add(conn, mode); >> + >> + return 1; >> +} >> + >> +static const struct drm_connector_helper_funcs sdrm_conn_hfuncs = { >> + .get_modes = sdrm_conn_get_modes, >> + .best_encoder = drm_atomic_helper_best_encoder, >> +}; >> + >> +static enum drm_connector_status sdrm_conn_detect(struct drm_connector >> *conn, >> + bool force) >> +{ >> + /* >> + * We simulate an always connected monitor. simple-fb doesn't >> + * provide any way to detect whether the connector is active. Hence, >> + * signal DRM core that it is always connected. >> + */ >> + >> + return connector_status_connected; >> +} >> + >> +static const struct drm_connector_funcs sdrm_conn_ops = { >> + .dpms = drm_atomic_helper_connector_dpms, >> + .reset = drm_atomic_helper_connector_reset, >> + .detect = sdrm_conn_detect, >> + .fill_modes = drm_helper_probe_single_connector_modes, >> + .destroy = drm_connector_cleanup, >> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, >> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, >> +}; >> + >> +static inline struct sdrm_device * >> +pipe_to_sdrm(struct drm_simple_display_pipe *pipe) >> +{ >> + return container_of(pipe, struct sdrm_device, pipe); >> +} >> + >> +static void sdrm_crtc_send_vblank_event(struct drm_crtc *crtc) >> +{ >> + if (crtc->state && crtc->state->event) { >> + spin_lock_irq(&crtc->dev->event_lock); >> + drm_crtc_send_vblank_event(crtc, crtc->state->event); >> + spin_unlock_irq(&crtc->dev->event_lock); >> + crtc->state->event = NULL; >> + } >> +} >> + >> +void sdrm_display_pipe_update(struct drm_simple_display_pipe *pipe, >> + struct drm_plane_state *plane_state) >> +{ >> + struct drm_framebuffer *fb = pipe->plane.state->fb; >> + struct sdrm_device *sdrm = pipe_to_sdrm(pipe); >> + >> + sdrm_crtc_send_vblank_event(&pipe->crtc); >> + >> + if (fb) { >> + pipe->plane.fb = fb; >> + sdrm_dirty_all_locked(sdrm); >> + } >> +} >> + >> +static void sdrm_display_pipe_enable(struct drm_simple_display_pipe *pipe, >> + struct drm_crtc_state *crtc_state) >> +{ >> + sdrm_crtc_send_vblank_event(&pipe->crtc); >> +} >> + >> +static void sdrm_display_pipe_disable(struct drm_simple_display_pipe *pipe) >> +{ >> + sdrm_crtc_send_vblank_event(&pipe->crtc); >> +} >> + >> +static const struct drm_simple_display_pipe_funcs sdrm_pipe_funcs = { >> + .update = sdrm_display_pipe_update, >> + .enable = sdrm_display_pipe_enable, >> + .disable = sdrm_display_pipe_disable, >> +}; >> + >> +static int sdrm_fb_create_handle(struct drm_framebuffer *fb, >> + struct drm_file *dfile, >> + unsigned int *handle) >> +{ >> + struct sdrm_framebuffer *sfb = to_sdrm_fb(fb); >> + >> + return drm_gem_handle_create(dfile, &sfb->obj->base, handle); >> +} >> + >> +static void sdrm_fb_destroy(struct drm_framebuffer *fb) >> +{ >> + struct sdrm_framebuffer *sfb = to_sdrm_fb(fb); >> + >> + drm_framebuffer_cleanup(fb); >> + drm_gem_object_unreference_unlocked(&sfb->obj->base); >> + kfree(sfb); >> +} >> + >> +static const struct drm_framebuffer_funcs sdrm_fb_ops = { >> + .create_handle = sdrm_fb_create_handle, >> + .dirty = sdrm_dirty, >> + .destroy = sdrm_fb_destroy, >> +}; >> + >> +static struct drm_framebuffer *sdrm_fb_create(struct drm_device *ddev, >> + struct drm_file *dfile, >> + const struct drm_mode_fb_cmd2 >> *cmd) >> +{ >> + struct sdrm_framebuffer *fb; >> + struct drm_gem_object *gobj; >> + u32 bpp, size; >> + int ret; >> + void *err; >> + >> + if (cmd->flags) >> + return ERR_PTR(-EINVAL); >> + >> + gobj = drm_gem_object_lookup(dfile, cmd->handles[0]); >> + if (!gobj) >> + return ERR_PTR(-EINVAL); >> + >> + fb = kzalloc(sizeof(*fb), GFP_KERNEL); >> + if (!fb) { >> + err = ERR_PTR(-ENOMEM); >> + goto err_unref; >> + } >> + fb->obj = to_sdrm_bo(gobj); >> + >> + fb->base.pitches[0] = cmd->pitches[0]; >> + fb->base.offsets[0] = cmd->offsets[0]; >> + fb->base.width = cmd->width; >> + fb->base.height = cmd->height; >> + fb->base.pixel_format = cmd->pixel_format; >> + drm_fb_get_bpp_depth(cmd->pixel_format, &fb->base.depth, >> + &fb->base.bits_per_pixel); >> + >> + /* >> + * width/height are already clamped into min/max_width/height range, >> + * so overflows are not possible >> + */ >> + >> + bpp = (fb->base.bits_per_pixel + 7) / 8; >> + size = cmd->pitches[0] * cmd->height; >> + if (!bpp || >> + bpp > 4 || >> + cmd->pitches[0] < bpp * fb->base.width || >> + cmd->pitches[0] > 0xffffU || >> + size + fb->base.offsets[0] < size || >> + size + fb->base.offsets[0] > fb->obj->base.size) { >> + err = ERR_PTR(-EINVAL); >> + goto err_free; >> + } >> + >> + ret = drm_framebuffer_init(ddev, &fb->base, &sdrm_fb_ops); >> + if (ret < 0) { >> + err = ERR_PTR(ret); >> + goto err_free; >> + } >> + >> + DRM_DEBUG_KMS("[FB:%d] pixel_format: %s\n", fb->base.base.id, >> + drm_get_format_name(fb->base.pixel_format)); >> + >> + return &fb->base; >> + >> +err_free: >> + kfree(fb); >> +err_unref: >> + drm_gem_object_unreference_unlocked(gobj); >> + >> + return err; >> +} >> + >> +static const struct drm_mode_config_funcs sdrm_mode_config_ops = { >> + .fb_create = sdrm_fb_create, >> + .atomic_check = drm_atomic_helper_check, >> + .atomic_commit = drm_atomic_helper_commit, >> +}; >> + >> +int sdrm_drm_modeset_init(struct sdrm_device *sdrm) >> +{ >> + struct drm_connector *conn = &sdrm->conn; >> + struct drm_device *ddev = sdrm->ddev; >> + int ret; >> + >> + drm_mode_config_init(ddev); >> + ddev->mode_config.min_width = sdrm->fb_width; >> + ddev->mode_config.max_width = sdrm->fb_width; >> + ddev->mode_config.min_height = sdrm->fb_height; >> + ddev->mode_config.max_height = sdrm->fb_height; >> + ddev->mode_config.preferred_depth = sdrm->fb_bpp; >> + ddev->mode_config.funcs = &sdrm_mode_config_ops; >> + >> + drm_connector_helper_add(conn, &sdrm_conn_hfuncs); >> + ret = drm_connector_init(ddev, conn, &sdrm_conn_ops, >> + DRM_MODE_CONNECTOR_VIRTUAL); >> + if (ret) >> + goto err_cleanup; >> + >> + ret = drm_mode_create_dirty_info_property(ddev); >> + if (ret) >> + goto err_cleanup; >> + >> + drm_object_attach_property(&conn->base, >> + ddev->mode_config.dirty_info_property, >> + DRM_MODE_DIRTY_ON); > dirty_info_property is gone. > >> + >> + ret = drm_simple_display_pipe_init(ddev, &sdrm->pipe, &sdrm_pipe_funcs, >> + sdrm_formats, >> + ARRAY_SIZE(sdrm_formats), conn); >> + if (ret) >> + goto err_cleanup; >> + >> + drm_mode_config_reset(ddev); >> + >> + return 0; >> + >> +err_cleanup: >> + drm_mode_config_cleanup(ddev); >> + >> + return ret; >> +} >> -- >> 2.8.2 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel