Hi Daniel/Hermann, > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel > Vetter > Sent: Thursday, October 16, 2014 6:29 PM > To: Cheng, Yao > Cc: David Herrmann; Vetter, Daniel; Intel Graphics Development; Jiang, Fei; > dri-devel@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [RFC PATCH 2/3] drm/ipvr: drm driver for vxd392 > > On Wed, Oct 15, 2014 at 02:14:33AM +0000, Cheng, Yao wrote: > > Hi Herrmann > > > > > -----Original Message----- > > > From: David Herrmann [mailto:dh.herrmann@xxxxxxxxx] > > > Sent: Monday, October 13, 2014 10:27 PM > > > To: Cheng, Yao > > > Cc: Intel Graphics Development; Jiang, Fei; > > > dri-devel@xxxxxxxxxxxxxxxxxxxxx; Vetter, Daniel > > > Subject: Re: [RFC PATCH 2/3] drm/ipvr: drm driver for vxd392 > > > > > > Hi > > > > > > > +static struct drm_ioctl_desc ipvr_gem_ioctls[] = { > > > > + DRM_IOCTL_DEF_DRV(IPVR_CONTEXT_CREATE, > > > > + ipvr_context_create_ioctl, DRM_UNLOCKED), > > > > + DRM_IOCTL_DEF_DRV(IPVR_CONTEXT_DESTROY, > > > > + ipvr_context_destroy_ioctl, DRM_UNLOCKED), > > > > + DRM_IOCTL_DEF_DRV(IPVR_MISC, > > > > + ipvr_misc_ioctl, DRM_AUTH), > > > > + DRM_IOCTL_DEF_DRV(IPVR_GEM_EXECBUFFER, > > > > + ipvr_gem_execbuffer, DRM_AUTH|DRM_UNLOCKED), > > > > + DRM_IOCTL_DEF_DRV(IPVR_GEM_BUSY, > > > > + ipvr_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED), > > > > + DRM_IOCTL_DEF_DRV(IPVR_GEM_CREATE, > > > > + ipvr_gem_create_ioctl, DRM_UNLOCKED), > > > > + DRM_IOCTL_DEF_DRV(IPVR_GEM_MMAP, > > > > + ipvr_gem_mmap_ioctl, DRM_UNLOCKED), > > > > > > Why do you need this ioctl? mmap() should work perfectly fine. I > > > don't see why you require people to use a ipvr specific ioctl to map > buffers. > > > > Many thanks to your comments, in our existing libdrm helper and > > userspace drivers, mmap_ioctl was the interface of mapping objects. We > > continued using the ioctl way for compatibility. Is it mandatory to > > implement mmap() to replace mmap_ioctl for GEM drivers? > > Yeah, the new way is to have a ioctl in the drm driver to create an mmap > offset (look at the gtt_mmap_offset ioctl for i915), and the use mmap on the > drm fd. Doing a direct driver ioctl to forward the mmap is deprecated. > > The reason for that is that a) this is the new standard way used by all drm > drivers b) if you use the standard mmap system call debug tools like valgrind > will understand it without any special actions. > > I'll do a patch to i915 so that people stop copy-pasting that old misdesign. > -Daniel Accepted :) I will update the patch to implement the mmap interface and remove the legacy MMAP_IOCTL. BTW I didn't see a field to get mmap_offset in struct drm_gem_open, I guess something like a new "GET_MMAP_OFFSET_IOCTL" need be added to support mapping flinked/primed BO, is it? > > > > > > > > > > + DRM_IOCTL_DEF_DRV(IPVR_SYNC_CPU, > > > > + ipvr_sync_cpu_ioctl, DRM_UNLOCKED), > > > > + DRM_IOCTL_DEF_DRV(IPVR_GEM_WAIT, > > > > + ipvr_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED), > > > > + DRM_IOCTL_DEF_DRV(IPVR_GEM_USERPTR, > > > > + ipvr_gem_userptr_ioctl, DRM_UNLOCKED), }; > > > > + > > > > +static void ipvr_gem_init(struct drm_device *dev) { > > > > + struct drm_ipvr_private *dev_priv = dev->dev_private; > > > > + > > > > + dev_priv->ipvr_bo_slab = > kmem_cache_create("ipvr_gem_object", > > > > + sizeof(union drm_ipvr_gem_objects), 0, > > > > + SLAB_HWCACHE_ALIGN, NULL); > > > > + > > > > + INIT_LIST_HEAD(&dev_priv->ipvr_mm.unbound_list); > > > > + INIT_LIST_HEAD(&dev_priv->ipvr_mm.bound_list); > > > > + spin_lock_init(&dev_priv->ipvr_mm.object_stat_lock); > > > > + > > > > + dev_priv->ipvr_mm.interruptible = true; } > > > > + > > > > +static void ipvr_gem_setup_mmu(struct drm_device *dev, > > > > + unsigned long linear_start, > > > > + unsigned long linear_end, > > > > + unsigned long tiling_start, > > > > + unsigned long tiling_end) { > > > > + /* Let GEM Manage all of the aperture. > > > > + * > > > > + * However, leave one page at the end still bound to the scratch > page. > > > > + * There are a number of places where hardware apparently > > > prefetches > > > > + * past the end of the object, and we've seen multiple hangs with > the > > > > + * GPU head pointer stuck in a batchbuffer bound at last page of > the > > > > + * aperture. One page should be enough to keep any > > > > + prefetching > > > inside > > > > + * of the aperture. > > > > + */ > > > > + struct drm_ipvr_private *dev_priv = dev->dev_private; > > > > + struct ipvr_address_space *addr_space = > > > > + &dev_priv->addr_space; > > > > + > > > > + /* todo: add sanity check */ > > > > + addr_space->dev = dev_priv->dev; > > > > + INIT_LIST_HEAD(&addr_space->active_list); > > > > + INIT_LIST_HEAD(&addr_space->inactive_list); > > > > + > > > > + /* Subtract the guard page ... */ > > > > + drm_mm_init(&addr_space->linear_mm, linear_start, > > > > + linear_end - linear_start - PAGE_SIZE); > > > > + dev_priv->addr_space.linear_start = linear_start; > > > > + dev_priv->addr_space.linear_total = linear_end - > > > > + linear_start; > > > > + > > > > + drm_mm_init(&addr_space->tiling_mm, tiling_start, > > > > + tiling_end - tiling_start - PAGE_SIZE); > > > > + dev_priv->addr_space.tiling_start = tiling_start; > > > > + dev_priv->addr_space.tiling_total = tiling_end - > > > > +tiling_start; } > > > > + > > > > +static void ipvr_do_takedown(struct drm_device *dev) { > > > > + /* todo: need check if need clean up mm here */ > > > > + ipvr_ved_uninit(dev); > > > > +} > > > > + > > > > +static int32_t ipvr_drm_unload(struct drm_device *dev) { > > > > + struct drm_ipvr_private *dev_priv = dev->dev_private; > > > > + > > > > + BUG_ON(!dev->platformdev); > > > > + BUG_ON(atomic_read(&dev_priv->ved_power_usage)); > > > > + > > > > + IPVR_DEBUG_ENTRY("entered."); > > > > + if (dev_priv) { > > > > + > > > > + WARN_ON(pm_runtime_get_sync(&dev->platformdev->dev) < 0); > > > > + > > > > + if (dev_priv->ipvr_bo_slab) > > > > + kmem_cache_destroy(dev_priv->ipvr_bo_slab); > > > > + ipvr_fence_driver_fini(dev_priv); > > > > + > > > > + ipvr_do_takedown(dev); > > > > + > > > > + > > > > + WARN_ON(pm_runtime_put_sync_suspend(&dev->platformdev- > >dev) > > > < 0); > > > > + > > > > + if (dev_priv->validate_ctx.buffers) > > > > + vfree(dev_priv->validate_ctx.buffers); > > > > + > > > > + if (dev_priv->pf_pd) { > > > > + ipvr_mmu_free_pagedir(dev_priv->pf_pd); > > > > + dev_priv->pf_pd = NULL; > > > > + } > > > > + if (dev_priv->mmu) { > > > > + ipvr_mmu_driver_takedown(dev_priv->mmu); > > > > + dev_priv->mmu = NULL; > > > > + } > > > > + > > > > + if (dev_priv->ved_reg_base) { > > > > + iounmap(dev_priv->ved_reg_base - dev_priv- > > > >ved_reg_offset); > > > > + dev_priv->ved_reg_base = NULL; > > > > + dev_priv->ved_reg_offset = 0; > > > > + } > > > > + > > > > + list_del(&dev_priv->default_ctx.head); > > > > + idr_remove(&dev_priv->ipvr_ctx_idr, dev_priv- > > > >default_ctx.ctx_id); > > > > + kfree(dev_priv); > > > > + > > > > + } > > > > + > > > > + pm_runtime_disable(&dev->platformdev->dev); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int32_t ipvr_drm_load(struct drm_device *dev, unsigned > > > > +long > > > > +flags) { > > > > + struct drm_ipvr_private *dev_priv; > > > > + int32_t ctx_id, ret = 0; > > > > + struct platform_device *platdev; > > > > + struct resource *res_mmio, *res_reg; > > > > + void __iomem* mmio_addr; > > > > + > > > > + dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); > > > > + if (dev_priv == NULL) > > > > + return -ENOMEM; > > > > + > > > > + dev->dev_private = dev_priv; > > > > + dev_priv->dev = dev; > > > > + > > > > + BUG_ON(!dev->platformdev); > > > > + platdev = dev->platformdev; > > > > + > > > > + mutex_init(&dev_priv->cmdbuf_mutex); > > > > + INIT_LIST_HEAD(&dev_priv->validate_ctx.validate_list); > > > > + > > > > + dev_priv->pci_root = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0)); > > > > + if (!dev_priv->pci_root) { > > > > + kfree(dev_priv); > > > > + return -ENODEV; > > > > + } > > > > + > > > > + dev->irq = platform_get_irq(platdev, 0); > > > > + if (dev->irq < 0) { > > > > + ret = -ENODEV; > > > > + goto out_err; > > > > + } > > > > + > > > > + res_mmio = platform_get_resource(platdev, IORESOURCE_MEM, > 0); > > > > + res_reg = platform_get_resource(platdev, IORESOURCE_REG, 0); > > > > + if (!res_mmio || !res_reg) { > > > > + ret = -ENXIO; > > > > + goto out_err; > > > > + } > > > > + > > > > + mmio_addr = ioremap_wc(res_mmio->start, > > > > + res_mmio->end - res_mmio->start); > > > > + if (IS_ERR(mmio_addr)) { > > > > + ret = -EACCES; > > > > + goto out_err; > > > > + } > > > > + > > > > + dev_priv->ved_reg_base = mmio_addr + res_reg->start; > > > > + dev_priv->ved_reg_offset = res_reg->start; > > > > + IPVR_DEBUG_VED("ved_reg_base is %p, range is 0x%llx - > 0x%llx.\n", > > > > + dev_priv->ved_reg_base, > > > > + res_reg->start, res_reg->end); > > > > + > > > > + platform_set_drvdata(dev->platformdev, dev); > > > > + pm_runtime_enable(&dev->platformdev->dev); > > > > + > > > > + if (pm_runtime_get_sync(&dev->platformdev->dev) < 0) { > > > > + ret = -EBUSY; > > > > + goto out_err; > > > > + } > > > > + > > > > + IPVR_DEBUG_INIT("MSVDX_CORE_REV_OFFSET by readl is > 0x%x.\n", > > > > + readl(dev_priv->ved_reg_base + 0x640)); > > > > + IPVR_DEBUG_INIT("MSVDX_CORE_REV_OFFSET by > > > VED_REG_READ32 is 0x%x.\n", > > > > + VED_REG_READ32(MSVDX_CORE_REV_OFFSET)); > > > > + > > > > + /* mmu init */ > > > > + dev_priv->mmu = ipvr_mmu_driver_init((void *)0, > > > > + drm_ipvr_trap_pagefaults, > > > > + 0, dev_priv); > > > > + if (!dev_priv->mmu) { > > > > + ret = -EBUSY; > > > > + goto out_err; > > > > + } > > > > + > > > > + dev_priv->pf_pd = ipvr_mmu_alloc_pd(dev_priv->mmu, 1, 0); > > > > + if (!dev_priv->pf_pd) { > > > > + ret = -ENOMEM; > > > > + goto out_err; > > > > + } > > > > + > > > > + ipvr_mmu_set_pd_context(ipvr_mmu_get_default_pd(dev_priv- > > > >mmu), 0); > > > > + ipvr_mmu_set_pd_context(dev_priv->pf_pd, 1); > > > > + > > > > + /* > > > > + * Initialize sequence numbers for the different command > > > > + * submission mechanisms. > > > > + */ > > > > + dev_priv->last_seq = 1; > > > > + > > > > + ipvr_gem_init(dev); > > > > + > > > > + ipvr_gem_setup_mmu(dev, > > > > + IPVR_MEM_MMU_LINEAR_START, > > > > + IPVR_MEM_MMU_LINEAR_END, > > > > + IPVR_MEM_MMU_TILING_START, > > > > + IPVR_MEM_MMU_TILING_END); > > > > + > > > > + ipvr_ved_init(dev); > > > > + > > > > + WARN_ON(pm_runtime_put_sync_suspend(&dev->platformdev- > > > >dev) < > > > > + 0); > > > > + > > > > + dev_priv->ved_private->ved_needs_reset = 1; > > > > + mutex_init(&dev_priv->ved_pm_mutex); > > > > + atomic_set(&dev_priv->ved_power_usage, 0); > > > > + > > > > + ipvr_fence_driver_init(dev_priv); > > > > + > > > > + dev_priv->validate_ctx.buffers = > > > > + vmalloc(IPVR_NUM_VALIDATE_BUFFERS * > > > > + sizeof(struct ipvr_validate_buffer)); > > > > + if (!dev_priv->validate_ctx.buffers) { > > > > + ret = -ENOMEM; > > > > + goto out_err; > > > > + } > > > > + > > > > + /* ipvr context initialization */ > > > > + INIT_LIST_HEAD(&dev_priv->ipvr_ctx_list); > > > > + spin_lock_init(&dev_priv->ipvr_ctx_lock); > > > > + idr_init(&dev_priv->ipvr_ctx_idr); > > > > + /* default ipvr context is used for scaling, rotation case */ > > > > + ctx_id = idr_alloc(&dev_priv->ipvr_ctx_idr, &dev_priv- > >default_ctx, > > > > + IPVR_MIN_CONTEXT_ID, IPVR_MAX_CONTEXT_ID, > > > > + GFP_KERNEL); > > > > + if (ctx_id < 0) { > > > > + return -ENOMEM; > > > > + goto out_err; > > > > + } > > > > + dev_priv->default_ctx.ctx_id = ctx_id; > > > > + INIT_LIST_HEAD(&dev_priv->default_ctx.head); > > > > + dev_priv->default_ctx.ctx_type = 0; > > > > + dev_priv->default_ctx.ipvr_fpriv = NULL; > > > > + > > > > + /* don't need protect with spinlock during module load stage */ > > > > + list_add(&dev_priv->default_ctx.head, &dev_priv->ipvr_ctx_list); > > > > + dev_priv->default_ctx.tiling_scheme = 0; > > > > + dev_priv->default_ctx.tiling_stride = 0; > > > > + > > > > + return 0; > > > > +out_err: > > > > + ipvr_drm_unload(dev); > > > > + return ret; > > > > + > > > > +} > > > > + > > > > +/* > > > > + * The .open() method is called every time the device is opened > > > > +by an > > > > + * application. Drivers can allocate per-file private data in > > > > +this method and > > > > + * store them in the struct drm_file::driver_priv field. Note > > > > +that the .open() > > > > + * method is called before .firstopen(). > > > > + */ > > > > +static int32_t > > > > +ipvr_drm_open(struct drm_device *dev, struct drm_file *file_priv) { > > > > + struct drm_ipvr_file_private *ipvr_fp; > > > > + IPVR_DEBUG_ENTRY("enter\n"); > > > > + > > > > + ipvr_fp = kzalloc(sizeof(*ipvr_fp), GFP_KERNEL); > > > > + if (unlikely(ipvr_fp == NULL)) > > > > + return -ENOMEM; > > > > + > > > > + file_priv->driver_priv = ipvr_fp; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +/* > > > > + * The close operation is split into .preclose() and .postclose() methods. > > > > + * Drivers must stop and cleanup all per-file operations in the > > > > +.preclose() > > > > + * method. For instance pending vertical blanking and page flip > > > > +events must be > > > > + * cancelled. No per-file operation is allowed on the file handle > > > > +after > > > > + * returning from the .preclose() method. > > > > + */ > > > > +static void > > > > +ipvr_drm_preclose(struct drm_device *dev, struct drm_file > > > > +*file_priv) { > > > > + /* if user didn't destory ctx explicitly, remove ctx here */ > > > > + struct drm_ipvr_private *dev_priv; > > > > + struct drm_ipvr_file_private *ipvr_fpriv; > > > > + struct ved_private *ved_priv; > > > > + struct ipvr_context *ipvr_ctx = NULL; > > > > + unsigned long irq_flags; > > > > + > > > > + IPVR_DEBUG_ENTRY("enter\n"); > > > > + dev_priv = dev->dev_private; > > > > + ipvr_fpriv = file_priv->driver_priv; > > > > + ved_priv = dev_priv->ved_private; > > > > + > > > > + if (ipvr_fpriv->ctx_id == IPVR_CONTEXT_INVALID_ID) > > > > + return; > > > > + ipvr_ctx = (struct ipvr_context *) > > > > + idr_find(&dev_priv->ipvr_ctx_idr, ipvr_fpriv->ctx_id); > > > > + if (!ipvr_ctx || (ipvr_ctx->ipvr_fpriv != ipvr_fpriv)) { > > > > + IPVR_DEBUG_GENERAL("ctx for id %d has already > destroyed\n", > > > > + ipvr_fpriv->ctx_id); > > > > + return; > > > > + } > > > > + IPVR_DEBUG_PM("Video:remove context profile %d, > > > entrypoint %d\n", > > > > + (ipvr_ctx->ctx_type >> 8) & 0xff, > > > > + (ipvr_ctx->ctx_type & 0xff)); > > > > + mutex_lock(&ved_priv->ved_mutex); > > > > + if (ved_priv->ipvr_ctx == ipvr_ctx ) > > > > + ved_priv->ipvr_ctx = NULL; > > > > + mutex_unlock(&ved_priv->ved_mutex); > > > > + > > > > + spin_lock_irqsave(&dev_priv->ipvr_ctx_lock, irq_flags); > > > > + list_del(&ipvr_ctx->head); > > > > + ipvr_fpriv->ctx_id = IPVR_CONTEXT_INVALID_ID; > > > > + spin_unlock_irqrestore(&dev_priv->ipvr_ctx_lock, > > > > + irq_flags); > > > > + > > > > + idr_remove(&dev_priv->ipvr_ctx_idr, ipvr_ctx->ctx_id); > > > > + > > > > + kfree(ipvr_ctx ); > > > > +} > > > > + > > > > +/* > > > > + * Finally the .postclose() method is called as the last step of > > > > +the close > > > > + * operation, right before calling the .lastclose() method if no > > > > +other open > > > > + * file handle exists for the device. Drivers that have allocated > > > > +per-file > > > > + * private data in the .open() method should free it here. > > > > + */ > > > > +static void > > > > +ipvr_drm_postclose(struct drm_device *dev, struct drm_file > > > > +*file_priv) { > > > > + struct drm_ipvr_file_private *ipvr_fpriv = file_priv->driver_priv; > > > > + IPVR_DEBUG_ENTRY("enter\n"); > > > > + kfree(ipvr_fpriv); > > > > +} > > > > > > postclose() is deprecated. You can safely move this into preclose(). > > > > Thx, will update it in next version. > > > > > > > > > + > > > > +static irqreturn_t ipvr_irq_handler(int32_t irq, void *arg) { > > > > + struct drm_device *dev = (struct drm_device *) arg; > > > > + WARN_ON(ved_irq_handler(dev)); > > > > + return IRQ_HANDLED; > > > > +} > > > > + > > > > +static const struct file_operations ipvr_fops = { > > > > + .owner = THIS_MODULE, > > > > + .open = drm_open, > > > > + .release = drm_release, > > > > + .unlocked_ioctl = drm_ioctl, #ifdef CONFIG_COMPAT > > > > + .compat_ioctl = drm_ioctl, #endif > > > > + /* no need to define mmap. User space maps bo with > > > > +DRM_IPVR_GEM_MMAP */ }; > > > > + > > > > +static int32_t ipvr_drm_freeze(struct drm_device *dev) { > > > > + int32_t ret; > > > > + int32_t power_usage; > > > > + struct drm_ipvr_private *dev_priv = dev->dev_private; > > > > + > > > > + IPVR_DEBUG_ENTRY("enter\n"); > > > > + power_usage = atomic_read(&dev_priv->ved_power_usage); > > > > + BUG_ON(power_usage < 0); > > > > + if (power_usage > 0) { > > > > + IPVR_DEBUG_PM("VED power usage is %d, skip > > > > + freezing\n", > > > power_usage); > > > > + return 0; > > > > + } > > > > + > > > > + ret = ved_check_idle(dev); > > > > + if (ret) { > > > > + IPVR_DEBUG_PM("VED check idle fail: %d, skip freezing\n", > ret); > > > > + return 0; > > > > + } > > > > + > > > > + if (dev->irq_enabled) { > > > > + ret = drm_irq_uninstall(dev); > > > > + if (unlikely(ret)) { > > > > + IPVR_ERROR("Error uninstalling IRQ handler: %d\n", ret); > > > > + return -EFAULT; > > > > + } > > > > + IPVR_DEBUG_PM("Successfully uninstalled IRQ\n"); > > > > + } > > > > + else > > > > + IPVR_DEBUG_PM("irq_enabled is %d\n", > > > > + dev->irq_enabled); > > > > + > > > > + if (is_ved_on(dev)) { > > > > + if (!ved_power_off(dev)) { > > > > + IPVR_ERROR("Failed to power off VED\n"); > > > > + return -EFAULT; > > > > + } > > > > + IPVR_DEBUG_PM("Successfully powered off\n"); > > > > + } else { > > > > + IPVR_DEBUG_PM("Skiped power-off since already > > > > + powered > > > off\n"); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int32_t ipvr_drm_thaw(struct drm_device *dev) { > > > > + int ret; > > > > + IPVR_DEBUG_ENTRY("enter\n"); > > > > + if (!is_ved_on(dev)) { > > > > + if (!ved_power_on(dev)) { > > > > + IPVR_ERROR("Failed to power on VED\n"); > > > > + return -EFAULT; > > > > + } > > > > + IPVR_DEBUG_PM("Successfully powered on\n"); > > > > + } else { > > > > + IPVR_DEBUG_PM("Skiped power-on since already > > > > +powered > > > on\n"); > > > > + } > > > > + > > > > + if (!dev->irq_enabled) { > > > > + ret = drm_irq_install(dev, dev->irq); > > > > + if (ret) { > > > > + IPVR_ERROR("Error installing IRQ handler: %d\n", ret); > > > > + return -EFAULT; > > > > + } > > > > + IPVR_DEBUG_PM("Successfully installed IRQ\n"); > > > > + } > > > > + else > > > > + IPVR_DEBUG_PM("irq_enabled is %d\n", > > > > + dev->irq_enabled); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int32_t ipvr_pm_suspend(struct device *dev) { > > > > + struct platform_device *platformdev = to_platform_device(dev); > > > > + struct drm_device *drm_dev = > platform_get_drvdata(platformdev); > > > > + IPVR_DEBUG_PM("PM suspend called\n"); > > > > + BUG_ON(!drm_dev); > > > > + return ipvr_drm_freeze(drm_dev); } static int32_t > > > > +ipvr_pm_resume(struct device *dev) { > > > > + struct platform_device *platformdev = to_platform_device(dev); > > > > + struct drm_device *drm_dev = > platform_get_drvdata(platformdev); > > > > + IPVR_DEBUG_PM("PM resume called\n"); > > > > + BUG_ON(!drm_dev); > > > > + return ipvr_drm_thaw(drm_dev); } > > > > + > > > > +/* > > > > + * dump GEM API is mainly for dumb buffers suitable for scanout, > > > > + * it is not needed for ipvr driver. > > > > + * gem_vm_ops is used for mmap case, also not needed for ipvr > > > > + * todo: prime support can be enabled later */ static struct > > > > +drm_driver ipvr_drm_driver = { > > > > + .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM, > > > > + .load = ipvr_drm_load, > > > > + .unload = ipvr_drm_unload, > > > > + .open = ipvr_drm_open, > > > > + .preclose = ipvr_drm_preclose, > > > > + .postclose = ipvr_drm_postclose, > > > > + .irq_handler = ipvr_irq_handler, > > > > + .set_busid = drm_platform_set_busid, > > > This should be NULL for new drivers. > > > > > > > + .gem_free_object = ipvr_gem_free_object, #ifdef > > > > +CONFIG_DEBUG_FS > > > > + .debugfs_init = ipvr_debugfs_init, > > > > + .debugfs_cleanup = ipvr_debugfs_cleanup, #endif > > > > + .ioctls = ipvr_gem_ioctls, > > > > + .num_ioctls = ARRAY_SIZE(ipvr_gem_ioctls), > > > > + .fops = &ipvr_fops, > > > > + .name = IPVR_DRIVER_NAME, > > > > + .desc = IPVR_DRIVER_DESC, > > > > + .date = IPVR_DRIVER_DATE, > > > > + .major = IPVR_DRIVER_MAJOR, > > > > + .minor = IPVR_DRIVER_MINOR, > > > > + .patchlevel = IPVR_DRIVER_PATCHLEVEL, }; > > > > + > > > > +static int32_t ipvr_plat_probe(struct platform_device *device) { > > > > + return drm_platform_init(&ipvr_drm_driver, device); > > > > > > drm_platform_init() should be used. Please use drm_dev_alloc(), > > > drm_dev_register() directly. udl and tegra should serve as example. > > > > > > > Thx, will update it in next version. > > > > > > +} > > > > + > > > > +static int32_t ipvr_plat_remove(struct platform_device *device) { > > > > + drm_put_dev(platform_get_drvdata(device)); > > > > > > Again, please use drm_dev_unregister() + drm_dev_unref() directly. > > > > Thx, will update it in next version. > > > > > > > > Thanks > > > David > > > > > > > + return 0; > > > > +} > > > > + > > > > +static struct dev_pm_ops ipvr_pm_ops = { > > > > + .suspend = ipvr_pm_suspend, > > > > + .resume = ipvr_pm_resume, > > > > + .freeze = ipvr_pm_suspend, > > > > + .thaw = ipvr_pm_resume, > > > > + .poweroff = ipvr_pm_suspend, > > > > + .restore = ipvr_pm_resume, #ifdef CONFIG_PM_RUNTIME > > > > + .runtime_suspend = ipvr_pm_suspend, > > > > + .runtime_resume = ipvr_pm_resume, #endif }; > > > > + > > > > +static struct platform_driver ipvr_plat_driver = { > > > > + .driver = { > > > > + .name = "ipvr-ved", > > > > + .owner = THIS_MODULE, #ifdef CONFIG_PM > > > > + .pm = &ipvr_pm_ops, #endif > > > > + }, > > > > + .probe = ipvr_plat_probe, > > > > + .remove = ipvr_plat_remove, }; > > > > + > > > > +module_platform_driver(ipvr_plat_driver); > > > > +MODULE_LICENSE("GPL"); > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
