Re: [RFC PATCH 2/3] drm/ipvr: drm driver for vxd392

David Herrmann
2014-10-16T09:46:17Z

Hi

On Wed, Oct 15, 2014 at 4:14 AM, Cheng, Yao <yao.cheng@intel.com> wrote:
> Hi Herrmann
>
>> -----Original Message-----
>> From: David Herrmann [mailto:dh.herrmann@gmail.com]
>> Sent: Monday, October 13, 2014 10:27 PM
>> To: Cheng, Yao
>> Cc: Intel Graphics Development; Jiang, Fei; dri-devel@lists.freedesktop.org;
>> 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?

Usually we don't care for existing user-space. We want APIs to be
cleanly designed when they go upstream. But I'm not the authority to
decide that. All I do is recommend hooking it up with mmap() like
everyone else does. If that's not what you want, you have to convince
Dave (and probably Daniel). The least you can do is hook up mmap() and
make it work. Whether there is a legacy ioctl to do the same, I don't
care..

Thanks
David