Re: [PATCH 1/2] drm: Implement vm_operations_struct.access

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15/07/17 04:47 AM, Felix Kuehling wrote:
> On 17-07-13 11:26 PM, Michel Dänzer wrote:
>> On 14/07/17 06:08 AM, Felix Kuehling wrote:
>>> Allows gdb to access contents of user mode mapped BOs. VRAM access
>>> requires the driver to implement a new callback in ttm_bo_driver.
>> Thanks for doing this. Looks mostly good, but I still have some comments.
>>
>> The shortlog prefix should be "drm/ttm:".
>>
>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>> index 9f53df9..0ef2eb9 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>> @@ -294,10 +294,74 @@ static void ttm_bo_vm_close(struct vm_area_struct *vma)
>>>  	vma->vm_private_data = NULL;
>>>  }
>>>  
>>> +static int ttm_bo_vm_access_kmap(struct ttm_buffer_object *bo,
>>> +				 unsigned long offset,
>>> +				 void *buf, int len, int write)
>>> +{
>>> +	unsigned long first_page = offset >> PAGE_SHIFT;
>>> +	unsigned long last_page = (offset + len - 1) >> PAGE_SHIFT;
>>> +	unsigned long num_pages = last_page - first_page + 1;
>>> +	struct ttm_bo_kmap_obj map;
>>> +	void *ptr;
>>> +	bool is_iomem;
>>> +	int ret;
>>> +
>>> +	ret = ttm_bo_kmap(bo, first_page, num_pages, &map);
>>> +	if (ret)
>>> +		return ret;
>> Could there be cases (e.g. on 32-bit) where mapping all pages at once
>> fails, but mapping one page at a time would work?
> 
> Maybe. I'm not really familiar with ttm_bo_kmap limitations on 32-bit. I
> guess the the access would have to be really big. I think in practice
> GDB doesn't do very big accesses. So I'm not sure it's worth the trouble.

Okay, I guess it can always be changed later if necessary.


>>> +	int (*access_vram)(struct ttm_buffer_object *bo, unsigned long offset,
>>> +			   void *buf, int len, int write);
>>>  };
>> I suggest making the write parameter a bool.
> 
> I made this as similar as possible to the vm_ops->access API, where
> write is also an integer.

I see, makes sense.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux