[PATCH 6/6] drm/gma500: use common functions for get/put pages

[Rob Clark]

On Mon, Sep 12, 2011 at 3:31 PM, Alan Cox <alan at lxorguk.ukuu.org.uk> wrote:
> On Mon, 12 Sep 2011 14:21:26 -0500
> Rob Clark <rob.clark at linaro.org> wrote:
>
>> From: Rob Clark <rob at ti.com>
>>
>> Signed-off-by: Rob Clark <rob at ti.com>
>
> Generally looks sensible but:
>
> 1. This is a staging driver, so good practise is to cc the staging
> maintainer and preferably the author (though I'm on dri-devel so its ok).
> It needs to be co-ordinated with existing staging changes and the
> maintainer needs to know what is going on

ok, will do that in the future

> 2. It needs a changelog. Your 0/6 won't be in the git tree and someone
> chasing regressions may only see the individual patch changelog and not
> be sure what it relates to. From/Signed off by alone is not helpful.

ok.. in this case the changelog only applied to the first patches
(initial patchset didn't have the drm_gem_get/put_pages()) but I will
do this in the future as needed

> 3. GMA500 used the old way of doing things because last mail conversation
> I had with Hugh the cleaned up interfaces could not guarantee the page is
> mapped in the low 32bits and for any of the GMA500/600 series devices.
>
> Has that changed ? I think I'd also prefer it if the methods had a
> BUG_ON() getting a high page when they asked for DMA32.

Hmm, ok, I found this thread, which is I guess what you are referring to:

  https://lkml.org/lkml/2011/7/11/269

I'm not really sure if anything has changed.. But it doesn't look like
the gma500 driver ever unpins anything.. so I guess it isn't a problem
(yet).  (Well, either that or I am overlooking something.)

btw, I could be missing something, but it seems like as long as you
remove the pages from any userspace mmap'ing before you unpin the
pages, that you shouldn't hit the case of page getting swapped back in
>4gb.. if you are always in control of bringing the page back into
memory, you can ensure that DMA32 is always specified.  Not sure if
that helps at all.  But at some point in the not too distant future,
I'll be in the same boat so I am interested in a good way to handle
this.

re: BUG_ON():
I wonder if a check would better belong in
shmem_read_mapping_page_gfp() or shmem_getpage_gfp()?

BR,
-R

>
> Alan
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>