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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxx>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
