On 11.03.2013 09:18, Thierry Reding wrote: > This sound a bit over-engineered at this point in time. DRM is currently > the only user. Is anybody working on any non-DRM drivers that would use > this? Well, this contains beginning of that: http://nv-tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=blob;f=drivers/media/video/tegra_v4l2_camera.c;h=644d0be5380367aca4c826c49724c03aad08387c;hb=l4t/l4t-r16-r2 I don't want to give these guys any excuse not to port it over to host1x code base. :-) > Even that aside, I don't think host1x_mem_handle is a good choice of > name here. The objects are much more than handles. They are in fact > buffer objects, which can optionally be attached to a handle. I also > think that using a void * to store the handle specific data isn't such a > good idea. Naming if not an issue for me - we can easily agree on using _bo. > So how about the following proposal, which I think might satisfy both of > us: > > struct host1x_bo; > > struct host1x_bo_ops { > struct host1x_bo *(*get)(struct host1x_bo *bo); > void (*put)(struct host1x_bo *bo); > dma_addr_t (*pin)(struct host1x_bo *bo, struct sg_table **sgt); > ... > }; > > struct host1x_bo *host1x_bo_get(struct host1x_bo *bo); > void host1x_bo_put(struct host1x_bo *bo); > dma_addr_t host1x_bo_pin(struct host1x_bo *bo, struct sg_table **sgt); > ... > > struct host1x_bo { > const struct host1x_bo_ops *ops; > ... > }; > > struct tegra_drm_bo { > struct host1x_bo base; > ... > }; > > That way you can get rid of the host1x_memmgr_create_handle() helper and > instead embed host1x_bo into driver-/framework-specific structures with > the necessary initialization. This would make sense. We'll get back when we have enough of implementation done to understand it all. One consequence is that we cannot use drm_gem_cma_create() anymore. We'll have to introduce a function that does the same as drm_gem_cma_create(), but it takes a pre-allocated drm_gem_cma_object pointer. That way we can allocate the struct, and use DRM CMA just to initialize the drm_gem_cma_object. Other way would be just taking a copy of DRM CMA helper, but I'd like to defer that to the next step when we implement IOMMU aware allocator. > It also allows you to interact directly with the objects instead of > having to go through the memmgr API. The memory manager doesn't really > exist anymore so keeping the name in the API is only confusing. Your > current proposal deals with memory handles directly already so it's > really just making the naming more consistent. The memmgr APIs are currently just a shortcut wrapper to the ops, so in that sense the memmgr does not really exist. I think it might still make sense to keep static inline wrappers for calling the ops within, but we could rename them to host1x_bo_somethingandother. Then it'd follow the pattern we are using for the hw ops in the latest set. Terje -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
