Re: [PATCH 0/5] media: Access videobuf2 buffers via an accessor

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 06 Jun 2019 15:13:00 -0300
Ezequiel Garcia <ezequiel@collabora.com> wrote:

> On Thu, 2019-06-06 at 19:43 +0200, Boris Brezillon wrote:
> > On Thu,  6 Jun 2019 12:44:21 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> wrote:
> >   
> > > Hi,
> > > 
> > > This patchset introduces a new vb2_get_buffer accessor and then
> > > uses it on all the drivers that are accessing videobuf2
> > > private buffer array directly.  
> > 
> > Just curious, how did you find all occurrences of direct q->bufs[]
> > accesses? If you used a cocci script it might be worth submitting it so
> > we don't end up with new offenders of the "don't access q->bufs[]
> > directly" rule.
> >   
> 
> No, I just inspected the code and tried a few grep variants.

Okay.

> 
> Hopefully, I haven't missed any!
> 
> > > I'm skipping Intel IPU3 driver here, since the code goes beyond
> > > just accessing the buffer. It also modifies the buffer queue
> > > directly. I believe this driver would need some more cleanup
> > > and love from its maintainers.
> > > 
> > > Note that OMAP2/OMAP3 display driver is videobuf1 and so not
> > > affected by this change.
> > > 
> > > Lastly, note that I'm doing the minimum changes to drivers I can't test,
> > > only using the new accessor and avoiding any further changes.  
> > 
> > Can you also add a patch to remove the private buf pointers array in the
> > cedrus driver?
> >   
> 
> You mean removing the dst_bufs field?

Yes.

> 
> I can but it's not part of this series, is it?

Fair enough.

> 
> And I'd rather someone else test it, as my cedrus boards
> are not wired at the moment.

I guess we can ask Jernej, Jonas, Paul or Maxime if they can test.