Review request to DRM Driver for Samsung SoC Exynos4210.
rob at ti.com
Tue Aug 30 15:16:12 PDT 2011
On Tue, Aug 30, 2011 at 7:38 AM, Inki Dae <inki.dae at samsung.com> wrote:
>> > Basically gem_dumb_* and gem_* are same operation. The difference
>> > them is that gem_dumb_ needs framebuffer information such width, height
>> > bpp to allocate buffer and gem_ needs only size. in case of gem_dumb_,
>> > would be calculated at kernel side(at samsung_drm_gem_dumb_create). do
>> > think it's better using only gem_dumb_? if so, I will remove
>> > gem_create_ioctl, gem_map_offset_ioctl and gem_mmap functions.
>> I think using the dumb ioctls initially is a good plan, esp as you
>> have no tiling or acceleration support, the idea
>> behind the dumb ioctls is to give splash screens and maybe write a
>> generic X.org driver in the future that can just do sw rendering.
>> Like at some point I forsee you needing driver specific ioctls for
>> alloc/mapping, I'd just rather wait until you have some userspace
>> available to use them that we can validate them with.
> Thank you for your pointing out. I will remove SAMSUNG_GEM_MAP_OFFSET and
> SAMSUNG_GEM_MAP ioctls except SAMSUNG_GEM_CREATE and SAMSUNG_GEM_MMAP ioctls
> because these are duplicated with dumb_*. and for alloc/mapping you
> mentioned above, we have already tested them through user application.
> This is example code in user level:
> /* allocation. */
> gem.size = 1024 * 600 * 4;
> ioctl(fd, DRM_IOCTL_SAMSUNG_GEM_CREATE, &gem);
> /* user space mapping. */
> map.handle = gem.handle
> map.offset = 0;
> map.size = gem.size;
> ioctl(fd, DRM_IOCTL_SAMSUNG_GEM_MMAP, &map);
Inki, any reason not to just pass the offset (which looks like you get
from _MMAP ioctl) back to mmap() syscall? Rather than having an ioctl
that calls do_mmap()?
Actually, it may even be enough to combine GEM_CREATE and GEM_MMAP
ioctls.. (currently in OMAP DRM driver I have them combined). I
*think* (and someone correct me if I am wrong), the only reason to
have separate ioctl to get mmap offset is to allow for separate
coherent and non-coherent mappings in same process. And this would
only work on ARM if you had a proper GART that you were mapping
through, otherwise it would be aliased virtual mappings of same
I think that it would be possible to add another ioctl later to
generate additional offsets if we ever had hw where this made sense.
Until then a single GEM_CREATE that returns a handle and offset (plus
GEM_INFO that gives you a way to get the offset in a different
process) would be sufficient.
> /* clear buffer. */
> memset(map.mapped, 0x0, map.size);
> drmModeAddFB(fd, width, height, 32, 32, stride, map.handle, &fb_id);
> drmModeSetCrtc(fd, ....);
> /* release. */
> munmap(map.mmaped, map.size);
> gem_close.handle = gem.handle;
> ioctl(fd, DRM_IOCTL_GEM_CLOSE, &gem_close);
More information about the dri-devel