On Mon, Sep 12, 2011 at 3:31 PM, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote: > On Mon, 12 Sep 2011 14:21:26 -0500 > Rob Clark <rob.clark@xxxxxxxxxx> wrote: > >> From: Rob Clark <rob@xxxxxx> >> >> Signed-off-by: Rob Clark <rob@xxxxxx> > > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
