On Tue, Jul 9, 2013 at 1:07 AM, Rob Clark <robdclark@xxxxxxxxx> wrote:
> On Mon, Jul 8, 2013 at 4:18 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>> On Mon, Jul 08, 2013 at 02:56:31PM -0400, Rob Clark wrote:
>>> On Mon, Jul 8, 2013 at 4:45 AM, Patrik Jakobsson
>>> <patrik.r.jakobsson@xxxxxxxxx> wrote:
>>> > On Sun, Jul 7, 2013 at 8:58 PM, Rob Clark <robdclark@xxxxxxxxx> wrote:
>>> >> Basically just extracting some code duplicated in gma500, omapdrm, udl,
>>> >> and upcoming msm driver.
>>> >>
>>> >> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
>>> >> ---
>>> >> drivers/gpu/drm/drm_gem.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++
>>> >> include/drm/drmP.h | 4 +++
>>> >> 2 files changed, 95 insertions(+)
>>> >>
>>> >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>>> >> index 443eeff..853dea6 100644
>>> >> --- a/drivers/gpu/drm/drm_gem.c
>>> >> +++ b/drivers/gpu/drm/drm_gem.c
>>> >> @@ -406,6 +406,97 @@ int drm_gem_create_mmap_offset(struct drm_gem_object *obj)
>>> >> }
>>> >> EXPORT_SYMBOL(drm_gem_create_mmap_offset);
>>> >>
>>> >> +/**
>>> >> + * drm_gem_get_pages - helper to allocate backing pages for a GEM object
>>> >> + * from shmem
>>> >> + * @obj: obj in question
>>> >> + * @gfpmask: gfp mask of requested pages
>>> >> + */
>>> >> +struct page **drm_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask)
>>> >> +{
>>> >> + struct inode *inode;
>>> >> + struct address_space *mapping;
>>> >> + struct page *p, **pages;
>>> >> + int i, npages;
>>> >> +
>>> >> + /* This is the shared memory object that backs the GEM resource */
>>> >> + inode = file_inode(obj->filp);
>>> >> + mapping = inode->i_mapping;
>>> >> +
>>> >> + npages = obj->size >> PAGE_SHIFT;
>>> >
>>> > Theoretical issue, but what if obj->size is not page aligned? Perhaps put a
>>> > roundup(obj->size, PAGE_SIZE) here?
>>>
>>> so, drm_gem_object_init() does have:
>>>
>>> BUG_ON((size & (PAGE_SIZE - 1)) != 0);
>>>
>>> so I was kinda assuming that we can count on the size already being
>>> aligned. But I guess in case someone somehow bypasses
>>> drm_gem_object_init() it wouldn't hurt to round up the size..
>>
>> Would look funny to me to allow it in one place and not in another one.
>> Maybe just throw a new WARN_ON in here (WARN since it's not fatal)?
>> -Daniel
>
> sounds good, I'll toss in a WARN_ON()
Yes, sounds good.
Patrik
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
