Re: External v4l / camera bo interface

[Thomas H. <th...@tu...>]

Austin Yuan wrote:
> On Wed, Sep 17, 2008 at 5:04 PM, Thomas Hellström
> <th...@tu...> wrote:
>   
>>> Is there a way to get drm_device in a separate kernel module, which is
>>> the first parameter of drm_buffer_object_create, if taking TTM as an
>>> example?
>>>
>>>
>>>       
>> No, currently not, but if we were to export buffer object functionality to
>> other kernel modules
>> one would have to create a separate interface for that with data types that
>> may be exported, and
>> with functionality that is relevant to other kernel modules.
>>
>> A v4l / camera capture module would probably want to have an interface
>> similar to
>>
>> struct drm_bo_external;
>> struct drm_bo_external_sync;
>>
>> /* Create an external BO from scratch. @minor is the minor number of the drm
>> device. Returns a user-space handle. */
>> struct drm_bo_external *drm_bo_external_create(int minor, size_t size,
>> uint32_t *user_space_handle);'
>>
>> /* Create an external BO from an already existing BO */
>> struct drm_bo_external* drm_bo_external_create_by_reference(int minor,
>> uint32_t user_space_handle);
>>     
> Maybe pass struct "ttm_bo_create_req" directly.
We should find the most suitable input parameters and use them. Since 
this is a kernel API
we must not depend on a user-space interface structure.

> Such V4L client may
> only care translated table based BO, and I don't thing other devices
> can DMA data into VRAM.
>
>   
Yes, I was thinking of that too, but I know of at least some UMA devices 
which have an integrated
V4L device that operates directly on VRAM, in which case we'd really 
want to hand back a
VRAM offset instead. The physical address is the most general since it 
can easily be translated to
a page:  (Here pfn is physical_address >> PAGE_SHIFT, which I think we 
should return).

if (pfn_valid(pfn))
    return pfn_to_page(pfn)

or in the vram case:
vram_offs = pfn - first_vram_pfn

>> /* Lock pages for external device DMA.
>> * (Is it possible for an external device to DMA into VRAM using a PCI
>> address??)
>> * Here we might want to hint where to lock pages. For fast GPU access or
>> fast CPU access.
>> */
>> int drm_bo_external_get_pages(struct drm_bo_external *ext_bo, unsigned long
>> dma_page_addr[],  size_t phys_page_addr_size);
>>     
> Return back an array of struct *page, just like get_user_pages?
>   
See above.
>> /* Release DMA pages. The sync object will have suitable methods for the
>> buffer manager to
>> * determine when the external device has finished DMA.
>> */
>> int drm_bo_external_put_pages(struct drm_bo_external *ext_bo, struct
>> drm_bo_external_sync *sync_obj);
>>     
> Why need sync_obj? I think client should make sure its DMA engine
> release those pages before call this function
>   
Yes. You're right. No need for a sync object AFAICT.

>> /* Unreference the exernal BO */
>> drm_bo_external_unref(**ext_bo);
>>
>> The user-space handles can then be used in the normal execbuf IOCTLS for the
>> GPU, and for user space data access if allowed by the creating module.
>>
>> Such an interface can probably made sufficiently generic to suit any
>> underlying implementation, be it GEM / TTM or a mix, and it would certainly
>> be very easy to write such an interface on top of TTM.
>>
>> /Thomas
>>
>>     
> Thomas, thank you for this valuable advice. I am not sure whether such
> interface can enter into DRM code base. If yes. maybe I can do some
> further implementation.
>   
 From my point of view, I think it's a good thing to have, but it would 
be good to hear from other
DRM developers as well.
> Austin
>   
/Thomas