[PATCH 01/12] drm/amdgpu: implement vm_operations_struct.access

[Felix Kuehling]

On 17-07-12 04:01 AM, Michel Dänzer wrote:
> On 12/07/17 02:37 PM, Felix Kuehling wrote:
>> Any comments on this one?
>>
>> This was requested by the HSA runtime team a long time ago as a
>> debugging feature. It allows gdb to access the content of CPU-mapped
>> BOs. I imagine this may be useful for user mode driver developers.
> Adding the dri-devel list.
>
>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 15148f1..3f927c2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1237,6 +1237,134 @@ void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size)
>>>  	man->size = size >> PAGE_SHIFT;
>>>  }
>>>  
>>> +static struct vm_operations_struct amdgpu_ttm_vm_ops;
>>> +static const struct vm_operations_struct *ttm_vm_ops /* = NULL;
>>> +						      * (appease checkpatch) */;
> How does this appease checkpatch?

Checkpatch doesn't like explicit initialization of global variables to
0. Uninitialized data is automatically 0, so no point wasting space in
the initialized data segment.

>
>
>>> +static int amdgpu_ttm_bo_access_kmap(struct amdgpu_bo *abo,
>>> +				     unsigned long offset,
>>> +				     void *buf, int len, int write)
>>> +{
>>> +	struct ttm_buffer_object *bo = &abo->tbo;
>>> +	struct ttm_bo_kmap_obj map;
>>> +	void *ptr;
>>> +	bool is_iomem;
>>> +	int r;
>>> +
>>> +	r = ttm_bo_kmap(bo, 0, bo->num_pages, &map);
>>> +	if (r)
>>> +		return r;
>>> +	ptr = (uint8_t *)ttm_kmap_obj_virtual(&map, &is_iomem) + offset;
>>> +	WARN_ON(is_iomem);
>>> +	if (write)
>>> +		memcpy(ptr, buf, len);
>>> +	else
>>> +		memcpy(buf, ptr, len);
>>> +	ttm_bo_kunmap(&map);
>>> +
>>> +	return len;
>>> +}
> This could be in TTM, right? As a helper function and/or a generic
> vm_operations_struct::access hook (at least for GTT/CPU domains).

I guess. If I were to get TTM involved, I'd also have TTM install its
own access hook in the vm_ops, so the driver doesn't need to override
it. Then I'd add driver-specific callbacks in ttm_bo_driver for
accessing VRAM and GTT/CPU memory. TTM could offer something like
amdgpu_ttm_bo_access_kmap as a helper for use by all drivers.

Regards,
  Felix

>
>