Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [RFC][PATCHv2 02/12] videobuf2: handle V4L2 buffer cache flags
To:     Sergey Senozhatsky <senozhatsky@chromium.org>
Cc:     Hans Verkuil <hans.verkuil@cisco.com>,
        Tomasz Figa <tfiga@chromium.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Kyungmin Park <kyungmin.park@samsung.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Sakari Ailus <sakari.ailus@iki.fi>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Pawel Osciak <posciak@chromium.org>,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20200204025641.218376-1-senozhatsky@chromium.org>
 <20200204025641.218376-3-senozhatsky@chromium.org>
 <be430540-4b8e-6b44-1eee-9d7291a365be@xs4all.nl>
 <20200219081340.GA122464@google.com>
From:   Hans Verkuil <hverkuil@xs4all.nl>
Message-ID: <2b1d9855-48dd-ecfe-7a7c-5998f76c3cd8@xs4all.nl>
Date:   Wed, 19 Feb 2020 09:25:29 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: <20200219081340.GA122464@google.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2/19/20 9:13 AM, Sergey Senozhatsky wrote:
> On (20/02/19 09:07), Hans Verkuil wrote:
> [..]
>>> +static void set_buffer_cache_hints(struct vb2_queue *q,
>>> +				   struct vb2_buffer *vb,
>>> +				   struct v4l2_buffer *b)
>>> +{
>>> +	/*
>>> +	 * DMA exporter should take care of cache syncs, so we can avoid
>>> +	 * explicit ->prepare()/->finish() syncs. For other ->memory types
>>> +	 * we always need ->prepare() or/and ->finish() cache sync.
>>> +	 */
>>> +	if (q->memory == VB2_MEMORY_DMABUF) {
>>> +		vb->need_cache_sync_on_finish = 0;
>>> +		vb->need_cache_sync_on_prepare = 0;
>>> +		return;
>>> +	}
>>> +
>>> +	if (!q->allow_cache_hints)
>>> +		return;
>>> +
>>> +	vb->need_cache_sync_on_prepare = 1;
>>
>> This needs a comment explaining why prepare is set to 1 by default. I remember
>> we discussed this earlier, and the conclusion of that discussion needs to be
>> documented here in a comment.
> 
> Please ignore this patch. There is a follow up which sets _both_
> flags by default. The purpose is to preserve the existing behaviour,
> we can do all sorts of incremental changes (clear flags in more cases,
> etc.) later on. Do you want me to document this in the code?

Yes please!

Regards,

	Hans

> 
> 	-ss
>