diff mbox

drm/gma500: remove the process of stolen page in page fault handler.

Message ID OF1369924B.BD4386B8-ON4825801F.000ACF25-4825801F.001C602B@zte.com.cn (mailing list archive)
State New, archived
Headers show

Commit Message

Jiang Biao Aug. 30, 2016, 5:10 a.m. UTC
Direct gtt range is used in the page fault scene in current driver,
instead of stolen page. So no need to keep relative process.

---
 drivers/gpu/drm/gma500/gem.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

--
2.1.0

Comments

Patrik Jakobsson Aug. 30, 2016, 10:21 a.m. UTC | #1
On Tue, Aug 30, 2016 at 7:10 AM,  <jiang.biao2@zte.com.cn> wrote:
>
> Direct gtt range is used in the page fault scene in current driver,
> instead of stolen page. So no need to keep relative process.

Hi

Are you saying that we don't use stolen memory? Afaik stolen memory
should be accessed through the stolen range so we do need this.

-Patrik

>
> ---
>  drivers/gpu/drm/gma500/gem.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
> index 6d1cb6b..53cb6704 100644
> --- a/drivers/gpu/drm/gma500/gem.c
> +++ b/drivers/gpu/drm/gma500/gem.c
> @@ -201,10 +201,7 @@ int psb_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>                                 >> PAGE_SHIFT;
>
>         /* CPU view of the page, don't go via the GART for CPU writes */
> -       if (r->stolen)
> -               pfn = (dev_priv->stolen_base + r->offset) >> PAGE_SHIFT;
> -       else
> -               pfn = page_to_pfn(r->pages[page_offset]);
> +       pfn = page_to_pfn(r->pages[page_offset]);
>         ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn);
>
>  fail:
> --
> 2.1.0
>
Jiang Biao Aug. 31, 2016, 2:27 a.m. UTC | #2
Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote on 2016/08/30 18:21:08:

> Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> 2016/08/30 18:21
>
> From
> jiang.biao2@zte.com.cn,
> cc
> dri-devel <dri-devel@lists.freedesktop.org>
> Re: [PATCH] drm/gma500: remove the process of stolen page in page fault handler.
>
> On Tue, Aug 30, 2016 at 7:10 AM,  <jiang.biao2@zte.com.cn> wrote:
> >
> > Direct gtt range is used in the page fault scene in current driver,
> > instead of stolen page. So no need to keep relative process.
>
> Hi
>
> Are you saying that we don't use stolen memory? Afaik stolen memory
> should be accessed through the stolen range so we do need this.
>
> -Patrik
>
As far as I can see, the stolen memory is only used by fbdev driver in gma500,
but the fbdev driver maps the stloen memory directly in psbfb_vm_fault, not
using psb_gem_fault to map the stolen memory.
The only scenario using the psb_gem_fault is the gtt range created by
psb_gem_create, which alloc the gtt range without stolen memory backed.

If I missed something, pls enlighten me.
Thanks a lot.
Jiang Biao Sept. 2, 2016, 9:31 a.m. UTC | #3
JiangBiao162664/user/zte_ltd Wrote 2016/08/31 10:27:34:

> JiangBiao162664/user/zte_ltd
> 2016/08/31 10:27
> 
> From
> Patrik Jakobsson <patrik.r.jakobsson@gmail.com>, 
> Re: [PATCH] drm/gma500: remove the process of stolen page in page 
> fault handler.
> 
> Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote on 2016/08/30 
18:21:08:
> 
> > Patrik Jakobsson <patrik.r.jakobsson@gmail.com> 
> > 2016/08/30 18:21
> > 
> > From
> > jiang.biao2@zte.com.cn, 
> > cc
> > dri-devel <dri-devel@lists.freedesktop.org>
> > Re: [PATCH] drm/gma500: remove the process of stolen page in page 
> fault handler.
> > 
> > On Tue, Aug 30, 2016 at 7:10 AM,  <jiang.biao2@zte.com.cn> wrote:
> > >
> > > Direct gtt range is used in the page fault scene in current driver,
> > > instead of stolen page. So no need to keep relative process.
> > 
> > Hi
> > 
> > Are you saying that we don't use stolen memory? Afaik stolen memory
> > should be accessed through the stolen range so we do need this.
> > 
> > -Patrik
> >
> As far as I can see, the stolen memory is only used by fbdev driver 
> in gma500, 
> but the fbdev driver maps the stloen memory directly in psbfb_vm_fault, 
not 
> using psb_gem_fault to map the stolen memory. 
> The only scenario using the psb_gem_fault is the gtt range created by 
> psb_gem_create, which alloc the gtt range without stolen memory backed.

> If I missed something, pls enlighten me.
> Thanks a lot. 

Hi Patrik,

Could you please help to confirm my question?
Thank you very much.
Patrik Jakobsson Sept. 2, 2016, 1:54 p.m. UTC | #4
On Fri, Sep 2, 2016 at 11:31 AM,  <jiang.biao2@zte.com.cn> wrote:
>
> JiangBiao162664/user/zte_ltd Wrote 2016/08/31 10:27:34:
>
>> JiangBiao162664/user/zte_ltd
>> 2016/08/31 10:27
>>
>> From
>> Patrik Jakobsson <patrik.r.jakobsson@gmail.com>,
>> Re: [PATCH] drm/gma500: remove the process of stolen page in page
>> fault handler.
>>
>> Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote on 2016/08/30
>> 18:21:08:
>>
>> > Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>> > 2016/08/30 18:21
>> >
>> > From
>> > jiang.biao2@zte.com.cn,
>> > cc
>> > dri-devel <dri-devel@lists.freedesktop.org>
>> > Re: [PATCH] drm/gma500: remove the process of stolen page in page
>> fault handler.
>> >
>> > On Tue, Aug 30, 2016 at 7:10 AM,  <jiang.biao2@zte.com.cn> wrote:
>> > >
>> > > Direct gtt range is used in the page fault scene in current driver,
>> > > instead of stolen page. So no need to keep relative process.
>> >
>> > Hi
>> >
>> > Are you saying that we don't use stolen memory? Afaik stolen memory
>> > should be accessed through the stolen range so we do need this.
>> >
>> > -Patrik
>> >
>> As far as I can see, the stolen memory is only used by fbdev driver
>> in gma500,
>> but the fbdev driver maps the stloen memory directly in psbfb_vm_fault,
>> not
>> using psb_gem_fault to map the stolen memory.
>> The only scenario using the psb_gem_fault is the gtt range created by
>> psb_gem_create, which alloc the gtt range without stolen memory backed.
>
>> If I missed something, pls enlighten me.
>> Thanks a lot.
>
> Hi Patrik,
>
> Could you please help to confirm my question?
> Thank you very much.

Hi,

The assumption that stolen memory will never be used with
psb_gem_create() might not hold true in the future and silently
breaking support for it ito save a few lines of code is not the right
way to do it. Actually, if we find use for stolen memory we would
basically get memory for free since it is already reserved for
graphics usage.

Cheers
Patrik
Jiang Biao Sept. 5, 2016, 1:04 a.m. UTC | #5
Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote 2016/09/02 21:54:41:

> Patrik Jakobsson <patrik.r.jakobsson@gmail.com> 

> 2016/09/02 21:54

> 

> Re: [PATCH] drm/gma500: remove the process of stolen page in page 

> fault handler.

> 

> On Fri, Sep 2, 2016 at 11:31 AM,  <jiang.biao2@zte.com.cn> wrote:

> >

> > JiangBiao162664/user/zte_ltd Wrote 2016/08/31 10:27:34:

> >

> >> JiangBiao162664/user/zte_ltd

> >> 2016/08/31 10:27

> >>

> >> From

> >> Patrik Jakobsson <patrik.r.jakobsson@gmail.com>,

> >> Re: [PATCH] drm/gma500: remove the process of stolen page in page

> >> fault handler.

> >>

> >> Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote on 2016/08/30

> >> 18:21:08:

> >>

> >> > Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

> >> > 2016/08/30 18:21

> >> >

> >> > From

> >> > jiang.biao2@zte.com.cn,

> >> > cc

> >> > dri-devel <dri-devel@lists.freedesktop.org>

> >> > Re: [PATCH] drm/gma500: remove the process of stolen page in page

> >> fault handler.

> >> >

> >> > On Tue, Aug 30, 2016 at 7:10 AM,  <jiang.biao2@zte.com.cn> wrote:

> >> > >

> >> > > Direct gtt range is used in the page fault scene in current 

driver,
> >> > > instead of stolen page. So no need to keep relative process.

> >> >

> >> > Hi

> >> >

> >> > Are you saying that we don't use stolen memory? Afaik stolen memory

> >> > should be accessed through the stolen range so we do need this.

> >> >

> >> > -Patrik

> >> >

> >> As far as I can see, the stolen memory is only used by fbdev driver

> >> in gma500,

> >> but the fbdev driver maps the stloen memory directly in 

psbfb_vm_fault,
> >> not

> >> using psb_gem_fault to map the stolen memory.

> >> The only scenario using the psb_gem_fault is the gtt range created by

> >> psb_gem_create, which alloc the gtt range without stolen memory 

backed.
> >

> >> If I missed something, pls enlighten me.

> >> Thanks a lot.

> >

> > Hi Patrik,

> >

> > Could you please help to confirm my question?

> > Thank you very much.

> 

> Hi,

> 

> The assumption that stolen memory will never be used with

> psb_gem_create() might not hold true in the future and silently

> breaking support for it ito save a few lines of code is not the right

> way to do it. Actually, if we find use for stolen memory we would

> basically get memory for free since it is already reserved for

> graphics usage.

> 

> Cheers

> Patrik


Understood, but maybe It's better to add it when it's actually used.
Indeed, that does not that matter.

Thanks for the reply.
diff mbox

Patch

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 6d1cb6b..53cb6704 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -201,10 +201,7 @@  int psb_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 				>> PAGE_SHIFT;

 	/* CPU view of the page, don't go via the GART for CPU writes */
-	if (r->stolen)
-		pfn = (dev_priv->stolen_base + r->offset) >> PAGE_SHIFT;
-	else
-		pfn = page_to_pfn(r->pages[page_offset]);
+	pfn = page_to_pfn(r->pages[page_offset]);
 	ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn);

 fail: