[Intel-gfx] [RFC PATCH 2/3] drm/ipvr: drm driver for vxd392

[David Herrmann]

Hi

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