On Tue, Jun 13, 2017 at 02:49:46PM -0400, Rob Clark wrote:
Pull some of the logic out into msm_gem_new() (since we don't need to care about the imported-bo case), and don't defer allocating pages. The latter is generally a good idea, since if we are using VRAM carveout to allocate contiguous buffers (ie. no IOMMU), the allocation is more likely to fail. So failing at allocation time is a more sane option. Plus this simplifies things in the next patch.
Signed-off-by: Rob Clark robdclark@gmail.com
drivers/gpu/drm/msm/msm_gem.c | 48 ++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 2f470a6..754432c 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -324,12 +324,8 @@ int msm_gem_get_iova_locked(struct drm_gem_object *obj, if (IS_ERR(pages)) return PTR_ERR(pages);
if (iommu_present(&platform_bus_type)) {ret = msm_gem_map_vma(priv->aspace[id], &msm_obj->domain[id],msm_obj->sgt, obj->size >> PAGE_SHIFT);} else {msm_obj->domain[id].iova = physaddr(obj);}
ret = msm_gem_map_vma(priv->aspace[id], &msm_obj->domain[id],msm_obj->sgt, obj->size >> PAGE_SHIFT);}
if (!ret)
@@ -765,7 +761,6 @@ static int msm_gem_new_impl(struct drm_device *dev, { struct msm_drm_private *priv = dev->dev_private; struct msm_gem_object *msm_obj;
bool use_vram = false;
switch (flags & MSM_BO_CACHE_MASK) { case MSM_BO_UNCACHED:
@@ -778,21 +773,10 @@ static int msm_gem_new_impl(struct drm_device *dev, return -EINVAL; }
if (!iommu_present(&platform_bus_type))
use_vram = true;else if ((flags & MSM_BO_STOLEN) && priv->vram.size)
use_vram = true;if (WARN_ON(use_vram && !priv->vram.size))
return -EINVAL;msm_obj = kzalloc(sizeof(*msm_obj), GFP_KERNEL); if (!msm_obj) return -ENOMEM;
if (use_vram)
msm_obj->vram_node = &msm_obj->domain[0].node;msm_obj->flags = flags; msm_obj->madv = MSM_MADV_WILLNEED;
@@ -814,13 +798,23 @@ static int msm_gem_new_impl(struct drm_device *dev, struct drm_gem_object *msm_gem_new(struct drm_device *dev, uint32_t size, uint32_t flags) {
struct msm_drm_private *priv = dev->dev_private; struct drm_gem_object *obj = NULL;
bool use_vram = false; int ret;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
size = PAGE_ALIGN(size);
if (!iommu_present(&platform_bus_type))
use_vram = true;else if ((flags & MSM_BO_STOLEN) && priv->vram.size)
use_vram = true;if (WARN_ON(use_vram && !priv->vram.size))
return ERR_PTR(-EINVAL);
I would say WARN_ONCE here but I guess if the first allocation fails we aren't making any more. This might be worth a nastygram to the log.
/* Disallow zero sized objects as they make the underlying * infrastructure grumpy */ @@ -831,12 +825,24 @@ struct drm_gem_object *msm_gem_new(struct drm_device *dev, if (ret) goto fail;
- if (use_pages(obj)) {
- if (use_vram) {
struct msm_gem_object *msm_obj = to_msm_bo(obj);struct page **pages;msm_obj->vram_node = &msm_obj->domain[0].node;drm_gem_private_object_init(dev, obj, size);msm_obj->pages = get_pages(obj);pages = get_pages(obj);if (IS_ERR(pages)) {ret = PTR_ERR(pages);goto fail;}msm_obj->domain[0].iova = physaddr(obj);- } else { ret = drm_gem_object_init(dev, obj, size); if (ret) goto fail;
} else {
drm_gem_private_object_init(dev, obj, size);}
return obj;
-- 2.9.4
Otherwise, Acked-by: Jordan Crouse jcrouse@codeaurora.org