On 17/06/15 13:02, Daniel Vetter wrote:
> On Wed, Jun 17, 2015 at 08:23:40AM +0100, Dave Gordon wrote:
>> On 15/06/15 21:09, Chris Wilson wrote:
>>> On Mon, Jun 15, 2015 at 07:36:19PM +0100, Dave Gordon wrote:
>>>> From: Alex Dai <yu.dai@xxxxxxxxx>
>>>>
>>>> i915_gem_object_write() is a generic function to copy data from a plain
>>>> linear buffer to a paged gem object.
>>>>
>>>> We will need this for the microcontroller firmware loading support code.
>>>>
>>>> Issue: VIZ-4884
>>>> Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx>
>>>> Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_drv.h | 2 ++
>>>> drivers/gpu/drm/i915/i915_gem.c | 28 ++++++++++++++++++++++++++++
>>>> 2 files changed, 30 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index 611fbd8..9094c06 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -2713,6 +2713,8 @@ void *i915_gem_object_alloc(struct drm_device *dev);
>>>> void i915_gem_object_free(struct drm_i915_gem_object *obj);
>>>> void i915_gem_object_init(struct drm_i915_gem_object *obj,
>>>> const struct drm_i915_gem_object_ops *ops);
>>>> +int i915_gem_object_write(struct drm_i915_gem_object *obj,
>>>> + const void *data, size_t size);
>>>> struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
>>>> size_t size);
>>>> void i915_init_vm(struct drm_i915_private *dev_priv,
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>>> index be35f04..75d63c2 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>>> @@ -5392,3 +5392,31 @@ 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, 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);
>>>
>>> You don't set the object into the CPU domain, or instead manually handle
>>> the domain flushing. You don't handle objects that cannot be written
>>> directly by the CPU, nor do you handle objects whose representation in
>>> memory is not linear.
>>> -Chris
>>
>> No we don't handle just any random gem object, but we do return an error
>> code for any types not supported. However, as we don't really need the
>> full generality of writing into a gem object of any type, I will replace
>> this function with one that combines the allocation of a new object
>> (which will therefore definitely be of the correct type, in the correct
>> domain, etc) and filling it with the data to be preserved.
The usage pattern for the particular case is going to be:
Once-only:
Allocate
Fill
Then each time GuC is (re-)initialised:
Map to GTT
DMA-read from buffer into GuC private memory
Unmap
Only on unload:
Dispose
So our object is write-once by the CPU (and that's always the first
operation), thereafter read-occasionally by the GuC's DMA engine.
> Domain handling is required for all gem objects, and the resulting bugs if
> you don't for one-off objects are absolutely no fun to track down.
> -Daniel
Is it not the case that the new object returned by
i915_gem_alloc_object() is
(a) of a type that can be mapped into the GTT, and
(b) initially in the CPU domain for both reading and writing?
So AFAICS the allocate-and-fill function I'm describing (to appear in
next patch series respin) doesn't need any further domain handling.
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
