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

Submitted by yu.dai@intel.com on April 17, 2015, 9:21 p.m.

Details

Message ID 1429305680-4990-3-git-send-email-yu.dai@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

yu.dai@intel.com April 17, 2015, 9:21 p.m.
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(+)

Patch hide | download patch | download mbox

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);
+
+	i915_gem_object_unpin_pages(obj);
+
+	if (WARN(bytes != size,
+		 "Failed to upload all data (completed %zu bytes out of %zu total",
+		 bytes, size)) {
+		i915_gem_object_put_pages(obj);
+		return -EIO;
+	}
+
+	return 0;
+}

Comments

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" ?

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;
> +}
>
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