Hi Rob, I've a couple of (minor) comments: On Wed, Mar 21, 2012 at 01:00:36PM -0500, Rob Clark wrote: > --- /dev/null > +++ b/omap/omap_drm.c [...] > +/* allocate a new (un-tiled) buffer object */ > +struct omap_bo * omap_bo_new(struct omap_device *dev, > + uint32_t size, uint32_t flags) > +{ > + if (flags & OMAP_BO_TILED) { > + return NULL; > + } > + return omap_bo_new_impl(dev, (union omap_gem_size){ > + .bytes = size, > + }, flags); Hum, the indentation of the anonymous union looks weird (but maybe it's just me...) > +} > + > +/* allocate a new buffer object */ > +struct omap_bo * omap_bo_new_tiled(struct omap_device *dev, > + uint32_t width, uint32_t height, uint32_t flags) > +{ > + if (!(flags & OMAP_BO_TILED)) { > + return NULL; > + } > + return omap_bo_new_impl(dev, (union omap_gem_size){ > + .tiled = { > + .width = width, > + .height = height, > + }, > + }, flags); > +} Here too :-) What about this: return omap_bo_new_impl(dev, (union omap_gem_size) { .stuff = blah, }); Or just use a temp var? > + > +/* get buffer info */ > +static int get_buffer_info(struct omap_bo *bo) > +{ > + struct drm_omap_gem_info req = { > + .handle = bo->handle, > + }; > + int ret = drmCommandWriteRead(bo->dev->fd, DRM_OMAP_GEM_INFO, > + &req, sizeof(req)); > + if (ret) { > + return ret; > + } > + > + /* really all we need for now is mmap offset */ > + bo->offset = req.offset; > + bo->size = req.size; > + > + return 0; > +} > + > +/* import a buffer object from DRI2 name */ > +struct omap_bo * omap_bo_from_name(struct omap_device *dev, uint32_t name) > +{ > + struct omap_bo *bo = calloc(sizeof(*bo), 1); > + struct drm_gem_open req = { > + .name = name, > + }; > + > + if (drmIoctl(dev->fd, DRM_IOCTL_GEM_OPEN, &req)) { > + goto fail; > + } bo may be NULL here: > + > + bo->dev = dev; > + bo->name = name; > + bo->handle = req.handle; I also woundn't use the calloc in the initialization block, I prefer to keep the allocation and the check together: bo = alloc_stuff(); if (!bo) oh_crap(); I find it more easy to check visually. Luca _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel