lock will now return the vaddr of the buffer. map/umap are gone.

- make sure to return an error if a buffer is locked twice or unlocked while not locked.
- added registerBuffer() and unregisterBuffer() to the gralloc module so that we can do some cleanup when a buffer is no longer needed. this became necessary after we removed map/unmap so we have a place to unmap buffers without the need of a kernel module.
- change the constants for GRALLOC_USAGE_SW_{READ|WRITE}_NEVER to 0, so that NOT specifying them means "NEVER".

4 files changed, 151 insertions, 64 deletions

diff --git a/modules/gralloc/framebuffer.cpp b/modules/gralloc/framebuffer.cpp
index 05b9418..31adfca 100644
--- a/modules/gralloc/framebuffer.cpp
+++ b/modules/gralloc/framebuffer.cpp
@@ -97,7 +97,7 @@ static int fb_post(struct framebuffer_device_t* dev, buffer_handle_t buffer)
 
         m->base.lock(&m->base, buffer, 
                 private_module_t::PRIV_USAGE_LOCKED_FOR_POST, 
-                0, 0, m->info.xres, m->info.yres);
+                0, 0, m->info.xres, m->info.yres, NULL);
 
         const size_t offset = hnd->base - m->framebuffer->base;
         m->info.activate = FB_ACTIVATE_VBL;
@@ -113,14 +113,23 @@ static int fb_post(struct framebuffer_device_t* dev, buffer_handle_t buffer)
         // If we can't do the page_flip, just copy the buffer to the front 
         // FIXME: use copybit HAL instead of memcpy
         
-        m->base.lock(&m->base, buffer, 
-                private_module_t::PRIV_USAGE_LOCKED_FOR_POST, 
-                0, 0, m->info.xres, m->info.yres);
+        void* fb_vaddr;
+        void* buffer_vaddr;
         
-        memcpy((void*)m->framebuffer->base, (void*)hnd->base,
-                m->finfo.line_length * m->info.yres);
+        m->base.lock(&m->base, m->framebuffer, 
+                GRALLOC_USAGE_SW_WRITE_RARELY, 
+                0, 0, m->info.xres, m->info.yres,
+                &fb_vaddr);
+
+        m->base.lock(&m->base, buffer, 
+                GRALLOC_USAGE_SW_READ_RARELY, 
+                0, 0, m->info.xres, m->info.yres,
+                &buffer_vaddr);
+
+        memcpy(fb_vaddr, buffer_vaddr, m->finfo.line_length * m->info.yres);
         
         m->base.unlock(&m->base, buffer); 
+        m->base.unlock(&m->base, m->framebuffer); 
     }
     
     return 0;
@@ -283,9 +292,9 @@ int mapFrameBufferLocked(struct private_module_t* module)
 
     module->numBuffers = info.yres_virtual / info.yres;
     module->bufferMask = 0;
-    
+
     void* vaddr;
-    module->base.map(&module->base, module->framebuffer, &vaddr);
+    gralloc_map(&module->base, module->framebuffer, &vaddr);
     memset(vaddr, 0, fbSize);
 
     return 0;
diff --git a/modules/gralloc/gralloc.cpp b/modules/gralloc/gralloc.cpp
index abb8c96..5a38b33 100644
--- a/modules/gralloc/gralloc.cpp
+++ b/modules/gralloc/gralloc.cpp
@@ -53,19 +53,20 @@ int fb_device_open(const hw_module_t* module, const char* name,
 static int gralloc_device_open(const hw_module_t* module, const char* name,
         hw_device_t** device);
 
-extern int gralloc_map(gralloc_module_t const* module,
-        buffer_handle_t handle, void** vaddr);
-
-extern int gralloc_unmap(gralloc_module_t const* module, 
-        buffer_handle_t handle);
-
 extern int gralloc_lock(gralloc_module_t const* module,
         buffer_handle_t handle, int usage,
-        int l, int t, int w, int h);
+        int l, int t, int w, int h,
+        void** vaddr);
 
 extern int gralloc_unlock(gralloc_module_t const* module, 
         buffer_handle_t handle);
 
+extern int gralloc_register_buffer(gralloc_module_t const* module,
+        buffer_handle_t handle);
+
+extern int gralloc_unregister_buffer(gralloc_module_t const* module,
+        buffer_handle_t handle);
+
 /*****************************************************************************/
 
 static struct hw_module_methods_t gralloc_module_methods = {
@@ -83,8 +84,8 @@ struct private_module_t HAL_MODULE_INFO_SYM = {
             author: "The Android Open Source Project",
             methods: &gralloc_module_methods
         },
-        map: gralloc_map,
-        unmap: gralloc_unmap,
+        registerBuffer: gralloc_register_buffer,
+        unregisterBuffer: gralloc_unregister_buffer,
         lock: gralloc_lock,
         unlock: gralloc_unlock,
     },
@@ -98,12 +99,6 @@ struct private_module_t HAL_MODULE_INFO_SYM = {
 
 /*****************************************************************************/
 
-/*
- * This function creates a buffer_handle_t initialized with the given fd.
- * the offset passed in parameter is used to mmap() this fd later at this
- * offset.
- */
-
 static int gralloc_alloc_framebuffer_locked(alloc_device_t* dev,
         size_t size, int usage, buffer_handle_t* pHandle)
 {
@@ -112,18 +107,14 @@ static int gralloc_alloc_framebuffer_locked(alloc_device_t* dev,
 
     // allocate the framebuffer
     if (m->framebuffer == NULL) {
-        // initialize the framebuffer
+        // initialize the framebuffer, the framebuffer is mapped once
+        // and forever.
         int err = mapFrameBufferLocked(m);
         if (err < 0) {
             return err;
         }
     }
 
-    if (m->framebuffer->base == 0) {
-        void *vaddr;
-        m->base.map(&m->base, m->framebuffer, &vaddr);
-    }
-    
     const uint32_t bufferMask = m->bufferMask;
     const uint32_t numBuffers = m->numBuffers;
     const size_t bufferSize = m->finfo.line_length * m->info.yres;
@@ -258,11 +249,10 @@ static int gralloc_free(alloc_device_t* dev,
         int index = (hnd->base - m->framebuffer->base) / bufferSize;
         m->bufferMask &= ~(1<<index); 
     }
-    
-    if (hnd->base) {
-        LOGW("handle %p still mapped at %p", hnd, hnd->base);
-        //gralloc_unmap((gralloc_module_t*) dev->common.module, handle);
-    }
+
+    gralloc_module_t* m = reinterpret_cast<gralloc_module_t*>(
+            dev->common.module);
+    gralloc_unregister_buffer(m, handle);
     
     close(hnd->fd);
     delete hnd;
@@ -276,16 +266,8 @@ static int gralloc_close(struct hw_device_t *dev)
     gralloc_context_t* ctx = reinterpret_cast<gralloc_context_t*>(dev);
     if (ctx) {
         /* TODO: keep a list of all buffer_handle_t created, and free them
-         * all here
+         * all here.
          */
-
-        private_module_t* m = reinterpret_cast<private_module_t*>(dev->module);
-        pthread_mutex_lock(&m->lock);
-        if (m->framebuffer) {
-            m->base.unmap(&m->base, m->framebuffer);
-        }
-        pthread_mutex_unlock(&m->lock);
-
         free(ctx);
     }
     return 0;
diff --git a/modules/gralloc/gralloc_priv.h b/modules/gralloc/gralloc_priv.h
index a6fa69a..b0f2c6c 100644
--- a/modules/gralloc/gralloc_priv.h
+++ b/modules/gralloc/gralloc_priv.h
@@ -36,6 +36,13 @@ inline size_t roundUpToPageSize(size_t x) {
     return (x + (PAGESIZE-1)) & ~(PAGESIZE-1);
 }
 
+int gralloc_map(gralloc_module_t const* module,
+        buffer_handle_t handle, void** vaddr);
+
+int gralloc_unmap(gralloc_module_t const* module, 
+        buffer_handle_t handle);
+
+
 int mapFrameBufferLocked(struct private_module_t* module);
 
 /*****************************************************************************/
@@ -58,6 +65,7 @@ struct private_module_t {
     float fps;
     
     enum {
+        // flag to indicate we'll post this buffer
         PRIV_USAGE_LOCKED_FOR_POST = 0x80000000
     };
 };
@@ -68,16 +76,18 @@ struct private_handle_t : public native_handle
 {
     enum {
         PRIV_FLAGS_FRAMEBUFFER = 0x00000001,
-        PRIV_FLAGS_USES_PMEM   = 0x00000002
+        PRIV_FLAGS_USES_PMEM   = 0x00000002,
+        PRIV_FLAGS_MAPPED      = 0x00000004,    // FIXME: should be out-of-line
+        PRIV_FLAGS_LOCKED      = 0x00000008     // FIXME: should be out-of-line
     };
 
     int     fd;
     int     magic;
-    int     base;
+    int     base;   // FIXME: should be out-of-line (meaningless with ipc)
     int     flags;
     int     size;
 
-    static const int sNumInts = 5;
+    static const int sNumInts = 4;
     static const int sNumFds = 1;
     static const int sMagic = 0x3141592;
 
diff --git a/modules/gralloc/mapper.cpp b/modules/gralloc/mapper.cpp
index a5f52ed..94be43b 100644
--- a/modules/gralloc/mapper.cpp
+++ b/modules/gralloc/mapper.cpp
@@ -38,14 +38,9 @@ struct mapped_buffer_t {
     int base;
     int refCount;
 
-    int init(private_handle_t* hnd) {
-        size = hnd->size;
-        base = 0;
-        refCount = 0;
-        struct stat buf;
-        int result = 0;
-        key = intptr_t(hnd);
-        return result;
+    mapped_buffer_t() { /* no init */ };
+    mapped_buffer_t(private_handle_t* hnd) 
+        : key(intptr_t(hnd)), size(hnd->size), base(0), refCount(0) {
     }
 };
 
@@ -68,12 +63,9 @@ struct mapped_buffers_t {
             void** vaddr)
     {
         private_handle_t* hnd = (private_handle_t*)(handle);
-        mapped_buffer_t key;
-        int result = key.init(hnd);
+        mapped_buffer_t key(hnd);
         //printRecord(ANDROID_LOG_DEBUG, "map", &key);
-        if (result) {
-            return result;
-        }
+        int result = 0;
         mapped_buffer_t* record = 0;
         pthread_mutex_lock(&mutex);
         // From here to the end of the function we return by jumping to "exit"
@@ -116,11 +108,9 @@ struct mapped_buffers_t {
     int unmap(gralloc_module_t const* module,
             buffer_handle_t handle)
     {
-        mapped_buffer_t key;
-        key.init((private_handle_t*) handle);
+        mapped_buffer_t key((private_handle_t*) handle);
         //printRecord(ANDROID_LOG_DEBUG, "unmap", &key);
         int index = -1;
-
         int result = 0;
         mapped_buffer_t* record = 0;
         pthread_mutex_lock(&mutex);
@@ -214,18 +204,114 @@ int gralloc_unmap(gralloc_module_t const* module,
     return sMappedBuffers.unmap(module, handle);
 }
 
+int gralloc_register_buffer(gralloc_module_t const* module,
+        buffer_handle_t handle)
+{
+    if (private_handle_t::validate(handle) < 0)
+        return -EINVAL;
+    
+    // In this implementation, we don't need to do anything here
+
+    /* FIXME: we need to initialize the buffer as not mapped/not locked
+     * because it shouldn't when this function is called the first time
+     * in a new process. ideally these flags shouldn't be part of the
+     * handle, but instead maintained in the kernel or at least 
+     * out-of-line
+     */ 
+    private_handle_t* hnd = (private_handle_t*)(handle);
+    hnd->base = 0;
+    hnd->flags &= ~(private_handle_t::PRIV_FLAGS_LOCKED | 
+                    private_handle_t::PRIV_FLAGS_MAPPED);
+    
+    return 0;
+}
+
+int gralloc_unregister_buffer(gralloc_module_t const* module,
+        buffer_handle_t handle)
+{
+    if (private_handle_t::validate(handle) < 0)
+        return -EINVAL;
+
+    /*
+     * If the buffer has been mapped during a lock operation, it's time
+     * to unmap it. It's an error to be here with a locked buffer.
+     * NOTE: the framebuffer is handled differently and is never unmapped.
+     */
+
+    private_handle_t* hnd = (private_handle_t*)(handle);
+    
+    LOGE_IF(hnd->flags & private_handle_t::PRIV_FLAGS_LOCKED,
+            "handle %p still locked", hnd);
+
+    if (hnd->flags & private_handle_t::PRIV_FLAGS_MAPPED) {
+        if (!(hnd->flags & private_handle_t::PRIV_FLAGS_FRAMEBUFFER)) {
+            gralloc_unmap(module, handle);
+            LOGE_IF(hnd->base,
+                    "handle %p still mapped at %p",
+                    hnd, (void*)hnd->base);
+        }
+    }
+    
+    return 0;
+}
 
 int gralloc_lock(gralloc_module_t const* module,
         buffer_handle_t handle, int usage,
-        int l, int t, int w, int h)
+        int l, int t, int w, int h,
+        void** vaddr)
 {
     // FIXME: gralloc_lock() needs implementation
-    return 0;
+
+    if (private_handle_t::validate(handle) < 0)
+        return -EINVAL;
+
+    int err = 0;
+    private_handle_t* hnd = (private_handle_t*)(handle);
+
+    // already locked
+    if (hnd->flags & private_handle_t::PRIV_FLAGS_LOCKED) {
+        LOGE("handle %p already locked", handle);
+        return -EBUSY;
+    }
+    
+    uint32_t mask = GRALLOC_USAGE_SW_READ_MASK | 
+                    GRALLOC_USAGE_SW_WRITE_MASK;
+    if ((usage & mask) && vaddr){
+        if (hnd->flags & private_handle_t::PRIV_FLAGS_MAPPED) {
+            *vaddr = (void*)hnd->base;
+        } else {
+            hnd->flags |= private_handle_t::PRIV_FLAGS_MAPPED;
+            err = gralloc_map(module, handle, vaddr);
+        }
+    }
+    
+    hnd->flags |= private_handle_t::PRIV_FLAGS_LOCKED;
+    return err;
 }
 
 int gralloc_unlock(gralloc_module_t const* module, 
         buffer_handle_t handle)
 {
     // FIXME: gralloc_unlock() needs implementation
+    if (private_handle_t::validate(handle) < 0)
+        return -EINVAL;
+
+    private_handle_t* hnd = (private_handle_t*)(handle);
+
+    // not locked
+    if (!(hnd->flags & private_handle_t::PRIV_FLAGS_LOCKED)) {
+        LOGE("handle %p is not locked", handle);
+        return -EINVAL;
+    }
+
+    /* FOR DEBUGGING
+    if (hnd->flags & private_handle_t::PRIV_FLAGS_MAPPED) {
+        if (gralloc_unmap(module, handle) == 0) {
+            hnd->flags &= ~private_handle_t::PRIV_FLAGS_MAPPED;
+        }
+    }
+    */
+    
+    hnd->flags &= ~private_handle_t::PRIV_FLAGS_LOCKED;
     return 0;
 }