[PATCH] drm/etnaviv: switch to PFN mappings

[PATCH] drm/etnaviv: switch to PFN mappings

[Lucas Stach]

There is no reason to use page based mappings, as the established
mappings are special driver mappings anyways and should not be
handled like normal pages.

Be consistent with what other drivers do and use raw PFN based
mappings.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index d45bf0368339..68e4446a94ad 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -130,7 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
 {
 	pgprot_t vm_page_prot;
 
-	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
+	vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
 
 	vm_page_prot = vm_get_page_prot(vma->vm_flags);
 
@@ -165,7 +165,8 @@ static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
 	struct vm_area_struct *vma = vmf->vma;
 	struct drm_gem_object *obj = vma->vm_private_data;
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
-	struct page **pages, *page;
+	struct page **pages;
+	unsigned long pfn;
 	pgoff_t pgoff;
 	int err;
 
@@ -189,12 +190,12 @@ static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
 	/* We don't use vmf->pgoff since that has the fake offset: */
 	pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
 
-	page = pages[pgoff];
+	pfn = page_to_pfn(pages[pgoff]);
 
 	VERB("Inserting %p pfn %lx, pa %lx", (void *)vmf->address,
-	     page_to_pfn(page), page_to_pfn(page) << PAGE_SHIFT);
+	     pfn, pfn << PAGE_SHIFT);
 
-	return vmf_insert_page(vma, vmf->address, page);
+	return vmf_insert_pfn(vma, vmf->address, pfn);
 }
 
 int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset)
-- 
2.30.2

Re: [PATCH] drm/etnaviv: switch to PFN mappings

[Lucas Stach]

Am Freitag, dem 09.09.2022 um 11:20 +0200 schrieb Lucas Stach:
> There is no reason to use page based mappings, as the established
> mappings are special driver mappings anyways and should not be
> handled like normal pages.
> 
> Be consistent with what other drivers do and use raw PFN based
> mappings.

Anyone up for reviewing this? Basically it moves etnaviv closer to what
all the other DRM drivers are doing.

Regards,
Lucas

> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index d45bf0368339..68e4446a94ad 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -130,7 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>  {
>  	pgprot_t vm_page_prot;
>  
> -	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
> +	vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
>  
>  	vm_page_prot = vm_get_page_prot(vma->vm_flags);
>  
> @@ -165,7 +165,8 @@ static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
>  	struct vm_area_struct *vma = vmf->vma;
>  	struct drm_gem_object *obj = vma->vm_private_data;
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> -	struct page **pages, *page;
> +	struct page **pages;
> +	unsigned long pfn;
>  	pgoff_t pgoff;
>  	int err;
>  
> @@ -189,12 +190,12 @@ static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
>  	/* We don't use vmf->pgoff since that has the fake offset: */
>  	pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>  
> -	page = pages[pgoff];
> +	pfn = page_to_pfn(pages[pgoff]);
>  
>  	VERB("Inserting %p pfn %lx, pa %lx", (void *)vmf->address,
> -	     page_to_pfn(page), page_to_pfn(page) << PAGE_SHIFT);
> +	     pfn, pfn << PAGE_SHIFT);
>  
> -	return vmf_insert_page(vma, vmf->address, page);
> +	return vmf_insert_pfn(vma, vmf->address, pfn);
>  }
>  
>  int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset)

Re: [PATCH] drm/etnaviv: switch to PFN mappings

[Daniel Vetter]

On Wed, Nov 16, 2022 at 10:17:02AM +0100, Lucas Stach wrote:
> Am Freitag, dem 09.09.2022 um 11:20 +0200 schrieb Lucas Stach:
> > There is no reason to use page based mappings, as the established
> > mappings are special driver mappings anyways and should not be
> > handled like normal pages.
> > 
> > Be consistent with what other drivers do and use raw PFN based
> > mappings.
> 
> Anyone up for reviewing this? Basically it moves etnaviv closer to what
> all the other DRM drivers are doing.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Note that since you've had VM_IO before it was already VM_SPECIAL, so
outside of etnaviv (for stuff like get_user_pages/userptr) there's not
really a difference. But it's nice to be a tad more consistent, since page
mapping + VM_IO really doesn't make that much sense :-)
-Daniel

> 
> Regards,
> Lucas
> 
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > index d45bf0368339..68e4446a94ad 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > @@ -130,7 +130,7 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
> >  {
> >  	pgprot_t vm_page_prot;
> >  
> > -	vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
> > +	vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
> >  
> >  	vm_page_prot = vm_get_page_prot(vma->vm_flags);
> >  
> > @@ -165,7 +165,8 @@ static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
> >  	struct vm_area_struct *vma = vmf->vma;
> >  	struct drm_gem_object *obj = vma->vm_private_data;
> >  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> > -	struct page **pages, *page;
> > +	struct page **pages;
> > +	unsigned long pfn;
> >  	pgoff_t pgoff;
> >  	int err;
> >  
> > @@ -189,12 +190,12 @@ static vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
> >  	/* We don't use vmf->pgoff since that has the fake offset: */
> >  	pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
> >  
> > -	page = pages[pgoff];
> > +	pfn = page_to_pfn(pages[pgoff]);
> >  
> >  	VERB("Inserting %p pfn %lx, pa %lx", (void *)vmf->address,
> > -	     page_to_pfn(page), page_to_pfn(page) << PAGE_SHIFT);
> > +	     pfn, pfn << PAGE_SHIFT);
> >  
> > -	return vmf_insert_page(vma, vmf->address, page);
> > +	return vmf_insert_pfn(vma, vmf->address, pfn);
> >  }
> >  
> >  int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset)
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch