[PATCH libdrm] omap: add omapdrm support

[Luca Tettamanti]

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