Hi Tomi,
Thank you for the patch.
On Friday 19 February 2016 11:47:52 Tomi Valkeinen wrote:
> We no longer have the omapdrm plugin system for SGX, and we can thus
> remove the support for external memory and sync objects from omap_gem.c.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> ---
> drivers/gpu/drm/omapdrm/omap_gem.c | 91 ++++++-----------------------------
> 1 file changed, 16 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
> b/drivers/gpu/drm/omapdrm/omap_gem.c index db5dbdb8cd0a..52e4a6c95058
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -33,9 +33,7 @@
> /* note: we use upper 8 bits of flags for driver-internal flags: */
> #define OMAP_BO_MEM_DMA_API 0x01000000 /* memory allocated with the
> dma_alloc_* API */
> #define OMAP_BO_MEM_SHMEM 0x02000000 /* memory allocated through
> shmem backing */
> -#define OMAP_BO_MEM_EXT 0x04000000 /* memory allocated
> externally */
> #define OMAP_BO_MEM_DMABUF 0x08000000 /* memory imported from a dmabuf
> */
> -#define OMAP_BO_EXT_SYNC 0x10000000 /* externally allocated sync
> object
> */
>
> struct omap_gem_object {
> struct drm_gem_object base;
> @@ -1177,20 +1175,6 @@ unlock:
> return ret;
> }
>
> -/* it is a bit lame to handle updates in this sort of polling way, but
> - * in case of PVR, the GPU can directly update read/write complete
> - * values, and not really tell us which ones it updated.. this also
> - * means that sync_lock is not quite sufficient. So we'll need to
> - * do something a bit better when it comes time to add support for
> - * separate 2d hw..
> - */
> -void omap_gem_op_update(void)
The function is still referenced from a comment above omap_gem_op_async().
> -{
> - spin_lock(&sync_lock);
> - sync_op_update();
> - spin_unlock(&sync_lock);
> -}
> -
> /* mark the start of read and/or write operation */
> int omap_gem_op_start(struct drm_gem_object *obj, enum omap_gem_op op)
> {
> @@ -1300,43 +1284,6 @@ int omap_gem_op_async(struct drm_gem_object *obj,
> enum omap_gem_op op, return 0;
> }
>
> -/* special API so PVR can update the buffer to use a sync-object allocated
> - * from it's sync-obj heap. Only used for a newly allocated (from PVR's
> - * perspective) sync-object, so we overwrite the new syncobj w/ values
> - * from the already allocated syncobj (if there is one)
> - */
> -int omap_gem_set_sync_object(struct drm_gem_object *obj, void *syncobj)
And this one from a comment in the omap_gem_object() structure.
The rest looks good to me, after updating the comments,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> -{
> - struct omap_gem_object *omap_obj = to_omap_bo(obj);
> - int ret = 0;
> -
> - spin_lock(&sync_lock);
> -
> - if ((omap_obj->flags & OMAP_BO_EXT_SYNC) && !syncobj) {
> - /* clearing a previously set syncobj */
> - syncobj = kmemdup(omap_obj->sync, sizeof(*omap_obj->sync),
> - GFP_ATOMIC);
> - if (!syncobj) {
> - ret = -ENOMEM;
> - goto unlock;
> - }
> - omap_obj->flags &= ~OMAP_BO_EXT_SYNC;
> - omap_obj->sync = syncobj;
> - } else if (syncobj && !(omap_obj->flags & OMAP_BO_EXT_SYNC)) {
> - /* replacing an existing syncobj */
> - if (omap_obj->sync) {
> - memcpy(syncobj, omap_obj->sync,
> sizeof(*omap_obj->sync));
> - kfree(omap_obj->sync);
> - }
> - omap_obj->flags |= OMAP_BO_EXT_SYNC;
> - omap_obj->sync = syncobj;
> - }
> -
> -unlock:
> - spin_unlock(&sync_lock);
> - return ret;
> -}
> -
> /*
> ---------------------------------------------------------------------------
> -- * Constructor & Destructor
> */
> @@ -1360,28 +1307,23 @@ void omap_gem_free_object(struct drm_gem_object
> *obj) */
> WARN_ON(omap_obj->paddr_cnt > 0);
>
> - /* don't free externally allocated backing memory */
> - if (!(omap_obj->flags & OMAP_BO_MEM_EXT)) {
> - if (omap_obj->pages) {
> - if (omap_obj->flags & OMAP_BO_MEM_DMABUF)
> - kfree(omap_obj->pages);
> - else
> - omap_gem_detach_pages(obj);
> - }
> + if (omap_obj->pages) {
> + if (omap_obj->flags & OMAP_BO_MEM_DMABUF)
> + kfree(omap_obj->pages);
> + else
> + omap_gem_detach_pages(obj);
> + }
>
> - if (omap_obj->flags & OMAP_BO_MEM_DMA_API) {
> - dma_free_writecombine(dev->dev, obj->size,
> - omap_obj->vaddr, omap_obj->paddr);
> - } else if (omap_obj->vaddr) {
> - vunmap(omap_obj->vaddr);
> - } else if (obj->import_attach) {
> - drm_prime_gem_destroy(obj, omap_obj->sgt);
> - }
> + if (omap_obj->flags & OMAP_BO_MEM_DMA_API) {
> + dma_free_writecombine(dev->dev, obj->size,
> + omap_obj->vaddr, omap_obj->paddr);
> + } else if (omap_obj->vaddr) {
> + vunmap(omap_obj->vaddr);
> + } else if (obj->import_attach) {
> + drm_prime_gem_destroy(obj, omap_obj->sgt);
> }
>
> - /* don't free externally allocated syncobj */
> - if (!(omap_obj->flags & OMAP_BO_EXT_SYNC))
> - kfree(omap_obj->sync);
> + kfree(omap_obj->sync);
>
> drm_gem_object_release(obj);
>
> @@ -1426,10 +1368,9 @@ struct drm_gem_object *omap_gem_new(struct drm_device
> *dev, * use contiguous memory only if no TILER is available.
> */
> flags |= OMAP_BO_MEM_DMA_API;
> - } else if (!(flags & (OMAP_BO_MEM_EXT | OMAP_BO_MEM_DMABUF))) {
> + } else if (!(flags & OMAP_BO_MEM_DMABUF)) {
> /*
> - * All other buffers not backed by external memory or dma_buf
> - * are shmem-backed.
> + * All other buffers not backed by dma_buf are shmem-backed.
> */
> flags |= OMAP_BO_MEM_SHMEM;
> }
--
Regards,
Laurent Pinchart
