Re: [PATCH 10/11] drm/i915: Support to create write combined type vmaps

Goel, Akash
2016-06-28T10:25:18Z

On 6/28/2016 3:22 PM, Chris Wilson wrote:
> On Mon, Jun 27, 2016 at 05:46:57PM +0530, akash.goel@intel.com wrote:
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 20c701c..3ef1ee5 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3197,6 +3197,7 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
>> /**
>> * i915_gem_object_pin_map - return a contiguous mapping of the entire object
>> * @obj - the object to map into kernel address space
>> + * &use_wc - whether the mapping should be using WC or WB pgprot_t
>
> s/&/@/ I think

Sorry my bad.

>
>> /* get, pin, and map the pages of the object into kernel space */
>> -void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj)
>> +void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, bool use_wc)
>> {
>> + void *ptr;
>> + bool has_wc;
>> + bool pinned;
>> int ret;
>>
>> lockdep_assert_held(&obj->base.dev->struct_mutex);
>> + GEM_BUG_ON((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0);
>>
>> ret = i915_gem_object_get_pages(obj);
>> if (ret)
>> return ERR_PTR(ret);
>>
>> + GEM_BUG_ON(obj->pages == NULL);
>> i915_gem_object_pin_pages(obj);
>>
>> - if (!obj->mapping) {
>> - obj->mapping = i915_gem_object_map(obj);
>> - if (!obj->mapping) {
>> - i915_gem_object_unpin_pages(obj);
>> - return ERR_PTR(-ENOMEM);
>> + pinned = (obj->pages_pin_count > 1);
>
> Too many ()

Sorry is the above condition not correct ?
If pin count is more than 1 then it implies that pages have been pinned
elsewhere also, so pages were already pinned before they were pinned
one more time, inside this function.
Please let me know, will fix it.

Best regards
Akash
>
> Hmm. It may look a bit dubious if I add my r-b here. But I didn't spot
> any rebasing errors.
> -Chris