Re: RFC: vb2: replace alloc_ctx by struct device * in vb2_queue

2014-09-21 Thread Marek Szyprowski

Hello,

On 2014-09-21 15:05, Hans Verkuil wrote:

Hi Marek, Pawel,

Currently for dma_config (and the dma_sg code that I posted before) drivers have
to allocate a alloc_ctx context, but in practice that just contains a device 
pointer.

Is there any reason why we can't just change in struct vb2_queue:

void*alloc_ctx[VIDEO_MAX_PLANES];

to:

struct device   *alloc_ctx[VIDEO_MAX_PLANES];

or possibly even just:

struct device   *alloc_ctx;

That simplifies the code quite a bit and I don't see and need for anything
else. The last option would make it impossible to have different allocation
contexts for different planes, but that might be something that Samsumg needs.


The last option won't work for for s5p-mfc driver, so better please keep
separate context per each plane.

If we are going to change the structures and their names, then maybe we 
should

get rid of 'context' name are simply replace it by the following entry in
vb2_queue:

struct device  *alloc_dev[VIDEO_MAX_PLANES];


and change respective parameter names in memory allocators.

The true context was useful when we were using custom, non-mainline memory
allocators.

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: vb2: replace alloc_ctx by struct device * in vb2_queue

2014-09-21 Thread Pawel Osciak
Hi Hans,
I'm not very much against it, but I'm not sure how option 2 is
significantly simpler than option 1. It's just one cast away from
being the same, unless you have some more rework in mind, e.g. making
vb2 aware of devices somehow and skip driver's involvement for
example?
Thanks,
Pawel

On Sun, Sep 21, 2014 at 10:05 PM, Hans Verkuil  wrote:
> Hi Marek, Pawel,
>
> Currently for dma_config (and the dma_sg code that I posted before) drivers 
> have
> to allocate a alloc_ctx context, but in practice that just contains a device 
> pointer.
>
> Is there any reason why we can't just change in struct vb2_queue:
>
> void*alloc_ctx[VIDEO_MAX_PLANES];
>
> to:
>
> struct device   *alloc_ctx[VIDEO_MAX_PLANES];
>
> or possibly even just:
>
> struct device   *alloc_ctx;
>
> That simplifies the code quite a bit and I don't see and need for anything
> else. The last option would make it impossible to have different allocation
> contexts for different planes, but that might be something that Samsumg needs.
>
> Regards,
>
> Hans



-- 
Best regards,
Pawel Osciak
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html