lkml.org 
[lkml]   [2020]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags
    On (20/04/28 16:47), Hans Verkuil wrote:
    > Hi Sergey,
    >
    > On 24/04/2020 11:29, Sergey Senozhatsky wrote:

    [..]

    > I missed that. What should happen is that q->allow_cache_hints is set by the
    > driver before vb2_queue_init is called. And the documentation should be updated
    > to say that the V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS flag is only valid when using the
    > MMAP streaming I/O model.
    >
    > Perhaps the flag should be renamed to V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS to
    > make this explicit? Other opinions are welcome.
    >
    > >
    > > Second. Even if the queue is setup, we still can report wrong cache
    > > hint values. Let's look at the following code
    > >
    > > fill_buf_caps(q, &p->capabilities);
    > > if (!vb2_queue_allows_cache_hints(q))
    > > p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
    >
    > The problem here is that vb2_queue_allows_cache_hints(q) uses stale information:
    > the current streaming mode instead of the requested streaming mode.
    >
    > This should read:
    >
    > if (!q->allow_cache_hints || p->memory != V4L2_MEMORY_MMAP)
    > p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
    >
    > And V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS is always set regardless of the
    > memory model. It just needs to be documented that this capability applies
    > to MMAP mode only.
    >
    > > ret = vb2_core_reqbufs(...);
    > > return ret;
    > >
    > > The thing here is that vb2_core_reqbufs() and vb2_core_create_bufs()
    > > can re-initialize the queue and invoke ->queue_setup(), possibly
    > > changing its memory model, etc. so cache hints cap which we set or
    > > clear before vb2_core_reqbufs() and vb2_core_create_bufs() can become
    > > invalid after we call those functions. It's the same with
    > > ``req->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT``, we cannot clear
    > > it before reqbufs()/create_bufs(). Therefore I added two simple
    > > functions which fixup cache hint cap and non_consistent flag after
    > > reqbufs()/create_bufs(). So the code looks like this now:
    > >
    > > fill_buf_caps(q, &p->capabilities);
    > > ret = vb2_core_reqbufs(...);
    > > fixup_consistency_attr(q, &p->flags);
    > > fixup_cache_hints_cap(q, &p->capabilities);
    >
    > These fixup functions are ugly, unless I missed something I think the
    > approach described above works just fine.
    >
    > With these changes I think it is ready to go in.

    ACK to all of these.
    Will send the updated patch set shortly.

    -ss

    \
     
     \ /
      Last update: 2020-05-14 17:36    [W:8.936 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site