Quoting Thierry Reding (2017-08-15 14:42:40)
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> The mapping of PRIME buffers can reuse much of the GEM mapping code, so
> extract the common bits into a new tegra_gem_mmap() helper.
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> drivers/gpu/drm/tegra/gem.c | 48 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 7a39a355678a..88b0250ec6d0 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -481,30 +481,27 @@ const struct vm_operations_struct tegra_bo_vm_ops = {
> .close = drm_gem_vm_close,
> };
>
> -int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
> +int tegra_gem_mmap(struct drm_gem_object *gem, struct vm_area_struct *vma)
Now static.
> {
> - struct drm_gem_object *gem;
> - struct tegra_bo *bo;
> - int ret;
> -
> - ret = drm_gem_mmap(file, vma);
> - if (ret)
> - return ret;
> -
> - gem = vma->vm_private_data;
> - bo = to_tegra_bo(gem);
> + struct tegra_bo *bo = to_tegra_bo(gem);
>
> if (!bo->pages) {
> unsigned long vm_pgoff = vma->vm_pgoff;
> + int err;
>
> + /*
> + * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(),
> + * and set the vm_pgoff (used as a fake buffer offset by DRM)
> + * to 0 as we want to map the whole buffer.
> + */
> vma->vm_flags &= ~VM_PFNMAP;
> vma->vm_pgoff = 0;
>
> - ret = dma_mmap_wc(gem->dev->dev, vma, bo->vaddr, bo->paddr,
> + err = dma_mmap_wc(gem->dev->dev, vma, bo->vaddr, bo->paddr,
> gem->size);
> - if (ret) {
> + if (err < 0) {
> drm_gem_vm_close(vma);
> - return ret;
> + return err;
> }
>
> vma->vm_pgoff = vm_pgoff;
> @@ -520,6 +517,20 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
> return 0;
> }
>
> +int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct drm_gem_object *gem;
> + int err;
> +
> + err = drm_gem_mmap(file, vma);
> + if (err < 0)
> + return err;
> +
> + gem = vma->vm_private_data;
> +
> + return tegra_gem_mmap(gem, vma);
Ok, simple mechanical code motion.
> +}
> +
> static struct sg_table *
> tegra_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
> enum dma_data_direction dir)
> @@ -603,7 +614,14 @@ static void tegra_gem_prime_kunmap(struct dma_buf *buf, unsigned long page,
>
> static int tegra_gem_prime_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
> {
> - return -EINVAL;
> + struct drm_gem_object *gem = buf->priv;
> + int err;
> +
> + err = drm_gem_mmap_obj(gem, gem->size, vma);
> + if (err < 0)
> + return err;
> +
> + return tegra_gem_mmap(gem, vma);
> }
>
Lgtm, with the one sparse fixup
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
So !bo->pages you have a WC pointer into dma space, but for bo->pages
you have physical ram that you create a new WC mapping into. Are all
users of bo->pages cache coherent (you are happy that the cache is on
physical tags and would be flushed by the WC access), or do you need the
dma_buf_begin_cpu_access to ensure that memory is cache coherent?
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
