Hello,
On 2015-12-15 11:14, Hans Verkuil wrote:
On 12/15/15 11:01, Marek Szyprowski wrote:
On 2015-12-14 16:40, Laurent Pinchart wrote:
On Monday 14 December 2015 15:36:04 Hans Verkuil wrote:
(Before I post this as the 'final' patch and CC all the driver developers
that are affected, I'd like to do an RFC post first. I always hated the
alloc context for obfuscating what is really going on, but let's see what
others think).
Instead of allocating a struct that contains just a single device pointer,
just pass that device pointer around. This avoids having to check for
memory allocation errors and is much easier to understand since it makes
explicit what was hidden in an opaque handle before.
Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
As most devices use the same allocation context for all planes, wouldn't it
make sense to just store the struct device pointer in the queue structure ?
The oddball driver that requires different allocation contexts (I'm thinking
about s5p-mfc here, there might be a couple more) would have to set the
allocation contexts properly in the queue_setup handler, but for all other
devices you could just remove that code completely.
This seams a reasonable approach. vb2 was written with special (or strange ;)
requirements in mind to align it with Exynos HW. However after some time it
turned out that most device drivers are simple and don't need fancy handling
of allocator context, so this definitely can be simplified. It also turned out
also that there is no real 'context' for vb2 memory allocators, although some
out-of-tree code (used in Android kernels) use some more advanced structures
there. Maybe it will be enough to let drivers to change defaults in queue_setup
and ensure that there is a 'void *alloc_ctx_priv' placeholder for allocator
specific data.
There is one more advantage of moving struct device * to vb2_queue. One can
then change all debugs to use dev_info/err/dbg infrastructure, so the logs will
be significantly easier to read, especially when more than one media device is
used.
I hadn't thought of that. That's nice indeed. However, it would require that
vb2-vmalloc drivers also set the device pointer.
They have some kind of virtual struct device (or fake platform device)
registered
somewhere anyway, so this should not be a problem. In case of usb
devices, they can
use struct device from that usb device. In both cases this will make the
logs much
easier to read.
I think I'll work on this a bit more, clearly it is the right direction to go.
Thanks!
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
