Re: [PATCH v3 02/15] drm/i915: Add i915_gem_object_write() to i915_gem.c

[Dave Gordon <david.s.gordon@intel.com>]

On 21/04/15 19:41, Dave Gordon wrote:
> On 17/04/15 22:21, yu.dai@intel.com wrote:
>> From: "Michael H. Nguyen" <michael.h.nguyen@intel.com>
>>
>> i915_gem_object_write() is a generic function to copy data from
>> user memory to gem object.
>>
>> Issue: VIZ-4884
>> Signed-off-by: Alex Dai <yu.dai@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h |  3 +++
>>  drivers/gpu/drm/i915/i915_gem.c | 30 ++++++++++++++++++++++++++++++
>>  2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 40ef672..6e8d106 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2645,6 +2645,9 @@ void i915_init_vm(struct drm_i915_private *dev_priv,
>>  void i915_gem_free_object(struct drm_gem_object *obj);
>>  void i915_gem_vma_destroy(struct i915_vma *vma);
>>  
>> +int i915_gem_object_write(struct drm_i915_gem_object *obj,
>> +			const void *data, const size_t size);
>> +
>>  #define PIN_MAPPABLE 0x1
>>  #define PIN_NONBLOCK 0x2
>>  #define PIN_GLOBAL 0x4
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index f7b8766..44154fe 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -5260,3 +5260,33 @@ bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj)
>>  	return false;
>>  }
>>  
>> +/* Fill the @obj with the @size amount of @data */
>> +int i915_gem_object_write(struct drm_i915_gem_object *obj,
>> +			const void *data, const size_t size)
>> +{
>> +	struct sg_table *sg;
>> +	size_t bytes;
>> +	int ret;
>> +
>> +	ret = i915_gem_object_get_pages(obj);
>> +	if (ret)
>> +		return ret;
>> +
>> +	i915_gem_object_pin_pages(obj);
>> +
>> +	sg = obj->pages;
>> +
>> +	bytes = sg_copy_from_buffer(sg->sgl, sg->nents,
>> +				    (void *)data, (size_t)size);
> 
> The second cast (size_t) is not required; better still, without
> that cast the code fits on one line :)
> 
>> +	i915_gem_object_unpin_pages(obj);
>> +
>> +	if (WARN(bytes != size,
>> +		 "Failed to upload all data (completed %zu bytes out of %zu total",
> 
> There's a mismatched parenthesis in the warning message above.
> Also the line is just a bit too long, according to checkpatch :(
> Perhaps just "Incomplete transfer, wrote %zu of %zu bytes" ?

Also a WARNing should have a trailing newline, so that would be
"Incomplete transfer, wrote %zu of %zu bytes\n" !

> The caller is going to issue a message too, if we ever reach
> this case, so this code doesn't have to explain too much.
> 
> With these fixed,
> 
> Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
> 
>> +		 bytes, size)) {
>> +		i915_gem_object_put_pages(obj);
>> +		return -EIO;
>> +	}
>> +
>> +	return 0;
>> +}
>>
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx