public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Hans Verkuil <hverkuil@xs4all.nl>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Sakari Ailus <sakari.ailus@iki.fi>
Subject: Re: [RFC PATCH] vb2: Stop allocating 'alloc_ctx', just set the device instead
Date: Tue, 15 Dec 2015 11:01:11 +0100	[thread overview]
Message-ID: <566FE4E7.5060001@samsung.com> (raw)
In-Reply-To: <28435978.1Ghf2YSlFk@avalon>

Hello,

On 2015-12-14 16:40, Laurent Pinchart wrote:
> Hi Hans,
>
> 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@cisco.com>
> 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.

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


  reply	other threads:[~2015-12-15 10:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-14 14:36 [RFC PATCH] vb2: Stop allocating 'alloc_ctx', just set the device instead Hans Verkuil
2015-12-14 15:40 ` Laurent Pinchart
2015-12-15 10:01   ` Marek Szyprowski [this message]
2015-12-15 10:14     ` Hans Verkuil
2015-12-15 10:16       ` Marek Szyprowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=566FE4E7.5060001@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=sakari.ailus@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox