Skip to contentSkip to conversation

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

DISCUSSION Hans Verkuil 3 participants 3 messages Sep 21, 2014 – Sep 22, 2014 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

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

Quoted 24 lines · Hans Verkuil

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

Signature · 2 lines
Best regards,
Pawel Osciak
#2 lore raw
In reply to Hans Verkuil

Hello,

Quoted 13 lines · Hans Verkuil

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];
Quoted 7 lines

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

Signature · 2 lines
Marek Szyprowski, PhD
Samsung R&D Institute Poland