Re: [PATCH v2 1/9] mm: Introduce new vm_insert_range API

2018-12-03 Thread Souptick Joarder
On Mon, Dec 3, 2018 at 11:52 AM Mike Rapoport  wrote:
>
> On Mon, Dec 03, 2018 at 09:51:45AM +0530, Souptick Joarder wrote:
> > Hi Mike,
> >
> > On Sun, Dec 2, 2018 at 4:43 PM Mike Rapoport  wrote:
> > >
> > > On Sun, Dec 02, 2018 at 11:49:44AM +0530, Souptick Joarder wrote:
> > > > Previouly drivers have their own way of mapping range of
> > > > kernel pages/memory into user vma and this was done by
> > > > invoking vm_insert_page() within a loop.
> > > >
> > > > As this pattern is common across different drivers, it can
> > > > be generalized by creating a new function and use it across
> > > > the drivers.
> > > >
> > > > vm_insert_range is the new API which will be used to map a
> > > > range of kernel memory/pages to user vma.
> > > >
> > > > This API is tested by Heiko for Rockchip drm driver, on rk3188,
> > > > rk3288, rk3328 and rk3399 with graphics.
> > > >
> > > > Signed-off-by: Souptick Joarder 
> > > > Reviewed-by: Matthew Wilcox 
> > > > Tested-by: Heiko Stuebner 
> > > > ---
> > > >  include/linux/mm_types.h |  3 +++
> > > >  mm/memory.c  | 38 ++
> > > >  mm/nommu.c   |  7 +++
> > > >  3 files changed, 48 insertions(+)
> > > >
> > > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > > > index 5ed8f62..15ae24f 100644
> > > > --- a/include/linux/mm_types.h
> > > > +++ b/include/linux/mm_types.h
> > > > @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, 
> > > > struct mm_struct *mm,
> > > >  extern void tlb_finish_mmu(struct mmu_gather *tlb,
> > > >   unsigned long start, unsigned long end);
> > > >
> > > > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > > > + struct page **pages, unsigned long page_count);
> > > > +
> > >
> > > This seem to belong to include/linux/mm.h, near vm_insert_page()
> >
> > Ok, I will change it. Apart from this change does it looks good ?
>
> With this change you can add
>
> Reviewed-by: Mike Rapoport 

Thanks Mike.

>
> > >
> > > >  static inline void init_tlb_flush_pending(struct mm_struct *mm)
> > > >  {
> > > >   atomic_set(&mm->tlb_flush_pending, 0);
> > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > index 15c417e..84ea46c 100644
> > > > --- a/mm/memory.c
> > > > +++ b/mm/memory.c
> > > > @@ -1478,6 +1478,44 @@ static int insert_page(struct vm_area_struct 
> > > > *vma, unsigned long addr,
> > > >  }
> > > >
> > > >  /**
> > > > + * vm_insert_range - insert range of kernel pages into user vma
> > > > + * @vma: user vma to map to
> > > > + * @addr: target user address of this page
> > > > + * @pages: pointer to array of source kernel pages
> > > > + * @page_count: number of pages need to insert into user vma
> > > > + *
> > > > + * This allows drivers to insert range of kernel pages they've 
> > > > allocated
> > > > + * into a user vma. This is a generic function which drivers can use
> > > > + * rather than using their own way of mapping range of kernel pages 
> > > > into
> > > > + * user vma.
> > > > + *
> > > > + * If we fail to insert any page into the vma, the function will return
> > > > + * immediately leaving any previously-inserted pages present.  Callers
> > > > + * from the mmap handler may immediately return the error as their 
> > > > caller
> > > > + * will destroy the vma, removing any successfully-inserted pages. 
> > > > Other
> > > > + * callers should make their own arrangements for calling 
> > > > unmap_region().
> > > > + *
> > > > + * Context: Process context. Called by mmap handlers.
> > > > + * Return: 0 on success and error code otherwise
> > > > + */
> > > > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > > > + struct page **pages, unsigned long page_count)
> > > > +{
> > > > + unsigned long uaddr = addr;
> > > > + int ret = 0, i;
> > > > +
> > > > + for (i = 0; i < page_count; i++) {
> > > > + ret = vm_insert_page(vma, uaddr, pages[i]);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + uaddr += PAGE_SIZE;
> > > > + }
> > > > +
> > > > + return ret;
> > > > +}
> > > > +EXPORT_SYMBOL(vm_insert_range);
> > > > +
> > > > +/**
> > > >   * vm_insert_page - insert single page into user vma
> > > >   * @vma: user vma to map to
> > > >   * @addr: target user address of this page
> > > > diff --git a/mm/nommu.c b/mm/nommu.c
> > > > index 749276b..d6ef5c7 100644
> > > > --- a/mm/nommu.c
> > > > +++ b/mm/nommu.c
> > > > @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, 
> > > > unsigned long addr,
> > > >  }
> > > >  EXPORT_SYMBOL(vm_insert_page);
> > > >
> > > > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > > > + struct page **pages, unsigned long page_count)
> > > > +{
> > > > + return -EINVAL;
> > > > +}
> > > > +EXPORT_SYMBOL(vm_insert_range);
> > > > +
> > > >  /*
> > 

Re: [PATCH v2 4/9] phy: dphy: Add configuration helpers

2018-12-03 Thread Kishon Vijay Abraham I
Hi Maxime,

On 21/11/18 3:03 PM, Maxime Ripard wrote:
> Hi Sakari,
> 
> Thanks for your review.
> 
> On Mon, Nov 19, 2018 at 03:43:57PM +0200, Sakari Ailus wrote:
>>> +/*
>>> + * Minimum D-PHY timings based on MIPI D-PHY specification. Derived
>>> + * from the valid ranges specified in Section 6.9, Table 14, Page 41
>>> + * of the D-PHY specification (v2.1).
>>
>> I assume these values are compliant with the earlier spec releases.
> 
> I have access to the versions 1.2 and 2.1 of the spec and as far as I
> can tell, they match here. I can't really say for other releases, but
> I wouldn't expect any changes (and it can always be adjusted later on
> if needed).
> 
>>> + */
>>> +int phy_mipi_dphy_get_default_config(unsigned long pixel_clock,
>>
>> How about using the bus frequency instead of the pixel clock? Chances are
>> that the caller already has that information, instead of calculating it
>> here?
> 
> I went for the pixel clock since it's something that all drivers will
> have access too without any computation. The bus frequency can be
> available as well in v4l2, but won't be in DRM, and that would require
> for all drivers to duplicate that computation, which doesn't seem like
> a good choice.
> 
>>> +unsigned int bpp,
>>> +unsigned int lanes,
>>> +struct phy_configure_opts_mipi_dphy *cfg)
>>> +{
>>> +   unsigned long hs_clk_rate;
>>> +   unsigned long ui;
>>> +
>>> +   if (!cfg)
>>> +   return -EINVAL;
>>> +
>>> +   hs_clk_rate = pixel_clock * bpp / lanes;
>>> +   ui = DIV_ROUND_UP(NSEC_PER_SEC, hs_clk_rate);
>>
>> Nanoseconds may not be precise enough for practical computations on these
>> values. At 1 GHz, this ends up being precisely 1. At least Intel hardware
>> has some more precision, I presume others do, too. How about using
>> picoseconds instead?
> 
> Sounds like a good idea.

Would you be fixing this? Or this can be a later patch?

Thanks
Kishon
> 
>>> +
>>> +   cfg->clk_miss = 0;
>>> +   cfg->clk_post = 60 + 52 * ui;
>>> +   cfg->clk_pre = 8;
>>> +   cfg->clk_prepare = 38;
>>> +   cfg->clk_settle = 95;
>>> +   cfg->clk_term_en = 0;
>>> +   cfg->clk_trail = 60;
>>> +   cfg->clk_zero = 262;
>>> +   cfg->d_term_en = 0;
>>> +   cfg->eot = 0;
>>> +   cfg->hs_exit = 100;
>>> +   cfg->hs_prepare = 40 + 4 * ui;
>>> +   cfg->hs_zero = 105 + 6 * ui;
>>> +   cfg->hs_settle = 85 + 6 * ui;
>>> +   cfg->hs_skip = 40;
>>> +
>>> +   /*
>>> +* The MIPI D-PHY specification (Section 6.9, v1.2, Table 14, Page 40)
>>> +* contains this formula as:
>>> +*
>>> +* T_HS-TRAIL = max(n * 8 * ui, 60 + n * 4 * ui)
>>> +*
>>> +* where n = 1 for forward-direction HS mode and n = 4 for reverse-
>>> +* direction HS mode. There's only one setting and this function does
>>> +* not parameterize on anything other that ui, so this code will
>>> +* assumes that reverse-direction HS mode is supported and uses n = 4.
>>> +*/
>>> +   cfg->hs_trail = max(4 * 8 * ui, 60 + 4 * 4 * ui);
>>> +
>>> +   cfg->init = 10;
>>> +   cfg->lpx = 60;
>>> +   cfg->ta_get = 5 * cfg->lpx;
>>> +   cfg->ta_go = 4 * cfg->lpx;
>>> +   cfg->ta_sure = 2 * cfg->lpx;
>>> +   cfg->wakeup = 100;
>>> +
>>> +   cfg->hs_clk_rate = hs_clk_rate;
>>
>> How about the LP clock?
>>
>> Frankly, I have worked with MIPI CSI-2 hardware soon a decade, and the very
>> few cases where software has needed to deal with these values has been in
>> form of defaults for a receiver, mostly limiting to clk_settle,
>> clk_term_en, d_term_en as well as hs_settle. On some hardware, the data
>> lane specific values can be at least in theory configured separately on
>> different lanes (but perhaps we could ignore that now).
>>
>> That doesn't say that it'd be useless to convey these values to the PHY
>> though. What I'm a little worried about though is what could be the effect
>> of adding support for this for existing drivers? If you have a new driver,
>> then there is no chance of regressions.
>>
>> I can't help noticing that many of the above values end up being unused in
>> the rest of the patches in the set. I guess that's ok, they come from the
>> standard anyway and some hardware may need them to be configured.
> 
> In order to get these parameters, I went through all the MIPI-DSI and
> MIPI-CSI drivers currently in the tree that could be converted, and
> looked at which parameters they needed to exchange with their PHY.
> 
> I made a summary to Kishon in the previous iteration here:
> https://lwn.net/ml/linux-media/20180919121436.ztjnxofe66quddeq@flea/
> 
> So it looks like the set of parameters on the MIPI-CSI side is indeed
> pretty limited, it really isn't for MIPI-DSI, and the whole point here
> is to support both :/
> 
>> Then there's the question of where should these values originate from.
>> Some drivers appear to have a need to obtain one of these values via
>> firmware, see Documentation/devicetree/bindings/media/sa

cron job: media_tree daily build: ERRORS

2018-12-03 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Dec  4 05:00:13 CET 2018
media-tree git hash:9b90dc85c718443a3e573a0ccf55900ff4fa73ae
media_build git hash:   47bf46ff21f75d1fe4ae3275a8692cb6ff77b6e8
v4l-utils git hash: cff58fcfbdf75381d5351f5ea8e7846f59cb7905
edid-decode git hash:   5eeb151a748788666534d6ea3da07f90400d24c2
gcc version:i686-linux-gcc (GCC) 8.2.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.18.0-2-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-3.10.108-i686: ERRORS
linux-3.10.108-x86_64: ERRORS
linux-3.11.10-i686: ERRORS
linux-3.11.10-x86_64: ERRORS
linux-3.12.74-i686: ERRORS
linux-3.12.74-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.79-i686: ERRORS
linux-3.14.79-x86_64: ERRORS
linux-3.15.10-i686: ERRORS
linux-3.15.10-x86_64: ERRORS
linux-3.16.57-i686: ERRORS
linux-3.16.57-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.123-i686: ERRORS
linux-3.18.123-x86_64: ERRORS
linux-3.19.8-i686: ERRORS
linux-3.19.8-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.52-i686: ERRORS
linux-4.1.52-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.159-i686: ERRORS
linux-4.4.159-x86_64: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.10-i686: ERRORS
linux-4.7.10-x86_64: ERRORS
linux-4.8.17-i686: ERRORS
linux-4.8.17-x86_64: ERRORS
linux-4.9.131-i686: ERRORS
linux-4.9.131-x86_64: ERRORS
linux-4.10.17-i686: ERRORS
linux-4.10.17-x86_64: ERRORS
linux-4.11.12-i686: ERRORS
linux-4.11.12-x86_64: ERRORS
linux-4.12.14-i686: ERRORS
linux-4.12.14-x86_64: ERRORS
linux-4.13.16-i686: ERRORS
linux-4.13.16-x86_64: ERRORS
linux-4.14.74-i686: ERRORS
linux-4.14.74-x86_64: ERRORS
linux-4.15.18-i686: OK
linux-4.15.18-x86_64: OK
linux-4.16.18-i686: OK
linux-4.16.18-x86_64: OK
linux-4.17.19-i686: OK
linux-4.17.19-x86_64: OK
linux-4.18.12-i686: OK
linux-4.18.12-x86_64: OK
linux-4.19.1-i686: OK
linux-4.19.1-x86_64: OK
linux-4.20-rc1-i686: OK
linux-4.20-rc1-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


Re: [PATCH v2 00/30] v4l: add support for multiplexed streams

2018-12-03 Thread Sakari Ailus
Hi Niklas,

On Fri, Nov 02, 2018 at 12:31:14AM +0100, Niklas Söderlund wrote:
> Hi all,
> 
> This series adds support for multiplexed streams within a media device
> link. The use-case addressed in this series covers CSI-2 Virtual
> Channels on the Renesas R-Car Gen3 platforms. The v4l2 changes have been
> a joint effort between Sakari and Laurent and floating around for some
> time [1].
> 
> I have added driver support for the devices used on the Renesas Gen3
> platforms, a ADV7482 connected to the R-Car CSI-2 receiver. With these
> changes I can control which of the analog inputs of the ADV7482 the
> video source is captured from and on which CSI-2 virtual channel the
> video is transmitted on to the R-Car CSI-2 receiver.
> 
> The series adds two new subdev IOCTLs [GS]_ROUTING which allows
> user-space to get and set routes inside a subdevice. I have added RFC
> support for these to v4l-utils [2] which can be used to test this
> series, example:
> 
> Check the internal routing of the adv748x csi-2 transmitter:
> v4l2-ctl -d /dev/v4l-subdev24 --get-routing
> 0/0 -> 1/0 [ENABLED]
> 0/0 -> 1/1 []
> 0/0 -> 1/2 []
> 0/0 -> 1/3 []
> 
> 
> Select that video should be outputed on VC 2 and check the result:
> $ v4l2-ctl -d /dev/v4l-subdev24 --set-routing '0/0 -> 1/2 [1]'
> 
> $ v4l2-ctl -d /dev/v4l-subdev24 --get-routing
> 0/0 -> 1/0 []
> 0/0 -> 1/1 []
> 0/0 -> 1/2 [ENABLED]
> 0/0 -> 1/3 []
> 
> This series is tested on R-Car M3-N and for your testing needs this
> series is available at
> 
> git://git.ragnatech.se/linux v4l2/mux
> 
> * Changes since v1
> - Rebased on latest media-tree.
> - Incorporated changes to patch 'v4l: subdev: compat: Implement handling 
>   for VIDIOC_SUBDEV_[GS]_ROUTING' by Sakari.
> - Added review tags.

I was looking at the patches and they seem very nice to me. It's not that
I've written most of them, but I still. X-)

I noticed that the new [GS]_ROUTING interface has no documentation
currently. Could you write it?

Also what I'd like to see is the media graph of a device that is driven by
these drivers. That'd help to better understand the use case also for those
who haven't worked with the patches.

Thanks.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver

2018-12-03 Thread Hans Verkuil
On 12/03/2018 09:37 PM, Eddie James wrote:



> +static int aspeed_video_start(struct aspeed_video *video)
> +{
> + int rc;
> +
> + aspeed_video_on(video);
> +
> + aspeed_video_init_regs(video);
> +
> + rc = aspeed_video_get_resolution(video);
> + if (rc)
> + return rc;
> +
> + /*
> +  * Set the timings here since the device was just opened for the first
> +  * time.
> +  */
> + video->active_timings = video->detected_timings;
 What happens if no valid signal was detected?

 My recommendation is to fallback to some default timings (VGA?) if no valid
 initial timings were found.

 The expectation is that applications will always call QUERY_DV_TIMINGS 
 first,
 so it is really not all that important what the initial active_timings are,
 as long as they are valid timings (valid as in: something that the hardware
 can support).
>>> See just above, this call returns with a failure if no signal is
>>> detected, meaning the device cannot be opened. The only valid timings
>>> are the detected timings.
>> That's wrong. You must ALWAYS be able to open the device. If not valid
>> resolution is detected, just fallback to some default.
> 
> Why must open always succeed? What use is a video device that cannot 
> provide any video?
You always must be able to open the video device so applications can call
QUERYCAP. In fact, any ioctl that returns state information (G_FMT, G_CTRL,
G_INPUT, ENUM_*, etc) can always be called, regardless of whether there is
a video signal or if video streaming is in progress.

With this restriction I cannot even run an application that waits for the
SOURCE_CHANGE event to start streaming, such as 'v4l2-ctl --stream-mmap'
does because the open() will fail immediately.

Sorry, this is really wrong.

Regards,

Hans


Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver

2018-12-03 Thread Eddie James




On 12/03/2018 02:14 PM, Hans Verkuil wrote:

On 12/03/2018 05:39 PM, Eddie James wrote:


On 12/03/2018 05:04 AM, Hans Verkuil wrote:

On 11/27/2018 08:37 PM, Eddie James wrote:

The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
can capture and compress video data from digital or analog sources. With
the Aspeed chip acting a service processor, the Video Engine can capture
the host processor graphics output.

Add a V4L2 driver to capture video data and compress it to JPEG images.
Make the video frames available through the V4L2 streaming interface.

Signed-off-by: Eddie James 
---




+static void aspeed_video_bufs_done(struct aspeed_video *video,
+  enum vb2_buffer_state state)
+{
+   unsigned long flags;
+   struct aspeed_video_buffer *buf;
+
+   spin_lock_irqsave(&video->lock, flags);
+   list_for_each_entry(buf, &video->buffers, link) {
+   if (list_is_last(&buf->link, &video->buffers))
+   buf->vb.flags |= V4L2_BUF_FLAG_LAST;

This really makes no sense. This flag is for codecs, not for receivers.

You say in an earlier reply about this:

"I mentioned before that dequeue calls hang in an error condition unless
this flag is specified. For example if resolution change is detected and
application is in the middle of trying to dequeue..."

What error condition are you referring to? Isn't your application using
the select() or poll() calls to wait for events or new buffers to dequeue?
If you just call VIDIOC_DQBUF to wait in blocking mode for a new buffer,
then it will indeed block in that call.

No other video receiver needs this flag, so there is something else that is
the cause.

Probably no one else uses it in blocking mode, but the thing should
still work. Why wouldn't it stop blocking if there is an error? Isn't
that normal?

As I said, the error condition I've tested this with is resolution
change. All the buffers are placed in error state, but dequeue does not
return.

If VIDIOC_DQBUF is waiting for a buffer, and the driver calls vb2_buffer_done,
then the ioctl will return. If not, then something else is wrong.

Is your application just requeueing the dequeued buffers? Does it work when
you use v4l2-ctl --stream-mmap?


I will try some tests.




I much prefer using blocking mode in applications because it reduces
complexity.

You say that the flag is for codecs, not receivers, but I don't see why
that has to be the case.

Because there is no concept of 'last' buffer for receivers. If the source
comes back with the same timings, then receiver will just pick it up again
(see also my other email on how video receivers behave when a source 
disappears).


+   vb2_buffer_done(&buf->vb.vb2_buf, state);
+   }
+   INIT_LIST_HEAD(&video->buffers);
+   spin_unlock_irqrestore(&video->lock, flags);
+}
+
+static irqreturn_t aspeed_video_irq(int irq, void *arg)
+{
+   struct aspeed_video *video = arg;
+   u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS);
+
+   if (atomic_read(&video->clients) == 0) {
+   dev_info(video->dev, "irq with no client; disabling irqs\n");
+
+   aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
+   aspeed_video_write(video, VE_INTERRUPT_STATUS, 0x);
+   return IRQ_HANDLED;
+   }
+
+   /* Resolution changed; reset entire engine and reinitialize */
+   if (sts & VE_INTERRUPT_MODE_DETECT_WD) {
+   dev_info(video->dev, "resolution changed; resetting\n");
+   set_bit(VIDEO_RES_CHANGE, &video->flags);
+   clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+   clear_bit(VIDEO_STREAMING, &video->flags);
+
+   aspeed_video_off(video);
+   aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
+
+   schedule_delayed_work(&video->res_work,
+ RESOLUTION_CHANGE_DELAY);
+   return IRQ_HANDLED;
+   }
+
+   if (sts & VE_INTERRUPT_MODE_DETECT) {
+   aspeed_video_update(video, VE_INTERRUPT_CTRL,
+   VE_INTERRUPT_MODE_DETECT, 0);
+   aspeed_video_write(video, VE_INTERRUPT_STATUS,
+  VE_INTERRUPT_MODE_DETECT);
+
+   set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
+   wake_up_interruptible_all(&video->wait);
+   }
+
+   if ((sts & VE_INTERRUPT_COMP_COMPLETE) &&
+   (sts & VE_INTERRUPT_CAPTURE_COMPLETE)) {
+   struct aspeed_video_buffer *buf;
+   u32 frame_size = aspeed_video_read(video,
+  VE_OFFSET_COMP_STREAM);
+
+   spin_lock(&video->lock);
+   clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+   buf = list_first_entry_or_null(&video->buffers,
+  struct aspeed_video_buffer,
+

Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver

2018-12-03 Thread Hans Verkuil
On 12/03/2018 05:39 PM, Eddie James wrote:
> 
> 
> On 12/03/2018 05:04 AM, Hans Verkuil wrote:
>> On 11/27/2018 08:37 PM, Eddie James wrote:
>>> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
>>> can capture and compress video data from digital or analog sources. With
>>> the Aspeed chip acting a service processor, the Video Engine can capture
>>> the host processor graphics output.
>>>
>>> Add a V4L2 driver to capture video data and compress it to JPEG images.
>>> Make the video frames available through the V4L2 streaming interface.
>>>
>>> Signed-off-by: Eddie James 
>>> ---



>>> +static void aspeed_video_bufs_done(struct aspeed_video *video,
>>> +  enum vb2_buffer_state state)
>>> +{
>>> +   unsigned long flags;
>>> +   struct aspeed_video_buffer *buf;
>>> +
>>> +   spin_lock_irqsave(&video->lock, flags);
>>> +   list_for_each_entry(buf, &video->buffers, link) {
>>> +   if (list_is_last(&buf->link, &video->buffers))
>>> +   buf->vb.flags |= V4L2_BUF_FLAG_LAST;
>> This really makes no sense. This flag is for codecs, not for receivers.
>>
>> You say in an earlier reply about this:
>>
>> "I mentioned before that dequeue calls hang in an error condition unless
>> this flag is specified. For example if resolution change is detected and
>> application is in the middle of trying to dequeue..."
>>
>> What error condition are you referring to? Isn't your application using
>> the select() or poll() calls to wait for events or new buffers to dequeue?
>> If you just call VIDIOC_DQBUF to wait in blocking mode for a new buffer,
>> then it will indeed block in that call.
>>
>> No other video receiver needs this flag, so there is something else that is
>> the cause.
> 
> Probably no one else uses it in blocking mode, but the thing should 
> still work. Why wouldn't it stop blocking if there is an error? Isn't 
> that normal?
> 
> As I said, the error condition I've tested this with is resolution 
> change. All the buffers are placed in error state, but dequeue does not 
> return.

If VIDIOC_DQBUF is waiting for a buffer, and the driver calls vb2_buffer_done,
then the ioctl will return. If not, then something else is wrong.

Is your application just requeueing the dequeued buffers? Does it work when
you use v4l2-ctl --stream-mmap?

> 
> I much prefer using blocking mode in applications because it reduces 
> complexity.
> 
> You say that the flag is for codecs, not receivers, but I don't see why 
> that has to be the case.

Because there is no concept of 'last' buffer for receivers. If the source
comes back with the same timings, then receiver will just pick it up again
(see also my other email on how video receivers behave when a source 
disappears).

> 
>>
>>> +   vb2_buffer_done(&buf->vb.vb2_buf, state);
>>> +   }
>>> +   INIT_LIST_HEAD(&video->buffers);
>>> +   spin_unlock_irqrestore(&video->lock, flags);
>>> +}
>>> +
>>> +static irqreturn_t aspeed_video_irq(int irq, void *arg)
>>> +{
>>> +   struct aspeed_video *video = arg;
>>> +   u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS);
>>> +
>>> +   if (atomic_read(&video->clients) == 0) {
>>> +   dev_info(video->dev, "irq with no client; disabling irqs\n");
>>> +
>>> +   aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
>>> +   aspeed_video_write(video, VE_INTERRUPT_STATUS, 0x);
>>> +   return IRQ_HANDLED;
>>> +   }
>>> +
>>> +   /* Resolution changed; reset entire engine and reinitialize */
>>> +   if (sts & VE_INTERRUPT_MODE_DETECT_WD) {
>>> +   dev_info(video->dev, "resolution changed; resetting\n");
>>> +   set_bit(VIDEO_RES_CHANGE, &video->flags);
>>> +   clear_bit(VIDEO_FRAME_INPRG, &video->flags);
>>> +   clear_bit(VIDEO_STREAMING, &video->flags);
>>> +
>>> +   aspeed_video_off(video);
>>> +   aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
>>> +
>>> +   schedule_delayed_work(&video->res_work,
>>> + RESOLUTION_CHANGE_DELAY);
>>> +   return IRQ_HANDLED;
>>> +   }
>>> +
>>> +   if (sts & VE_INTERRUPT_MODE_DETECT) {
>>> +   aspeed_video_update(video, VE_INTERRUPT_CTRL,
>>> +   VE_INTERRUPT_MODE_DETECT, 0);
>>> +   aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>> +  VE_INTERRUPT_MODE_DETECT);
>>> +
>>> +   set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
>>> +   wake_up_interruptible_all(&video->wait);
>>> +   }
>>> +
>>> +   if ((sts & VE_INTERRUPT_COMP_COMPLETE) &&
>>> +   (sts & VE_INTERRUPT_CAPTURE_COMPLETE)) {
>>> +   struct aspeed_video_buffer *buf;
>>> +   u32 frame_size = aspeed_video_read(video,
>>> +  VE_OFFSET_COMP_STREAM);
>>> +
>>> +   spin_lock(&video->lock);
>>> +   clear_bit(VIDEO_FRAME_INPRG, &video->flags);
>>> +   buf = list_first_entry_or_null(&v

Re: [GIT PULL for v4.20-rc6] media fixes

2018-12-03 Thread pr-tracker-bot
The pull request you sent on Mon, 3 Dec 2018 16:09:11 -0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
> tags/media/v4.20-4

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/0072a0c14d5b7cb72c611d396f143f5dcd73ebe2

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker


Re: [PATCH v6 01/12] media: ov5640: Fix set format regression

2018-12-03 Thread Adam Ford
On Mon, Dec 3, 2018 at 2:45 AM Maxime Ripard  wrote:
>
> From: Jacopo Mondi 
>
> The set_fmt operations updates the sensor format only when the image format
> is changed. When only the image sizes gets changed, the format do not get
> updated causing the sensor to always report the one that was previously in
> use.
>
> Without this patch, updating frame size only fails:
>   [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> With this patch applied:
>   [fmt:UYVY8_2X8/1024x768@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> Fixes: 6949d864776e ("media: ov5640: do not change mode if format or frame
> interval is unchanged")
> Signed-off-by: Jacopo Mondi 
> Signed-off-by: Maxime Ripard 

Tested-by: Adam Ford  #imx6 w/ CSI2 interface on
4.19.6 and 4.20-RC5
> ---
>  drivers/media/i2c/ov5640.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index a91d91715d00..807bd0e386a4 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2021,6 +2021,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> struct ov5640_dev *sensor = to_ov5640_dev(sd);
> const struct ov5640_mode_info *new_mode;
> struct v4l2_mbus_framefmt *mbus_fmt = &format->format;
> +   struct v4l2_mbus_framefmt *fmt;
> int ret;
>
> if (format->pad != 0)
> @@ -2038,22 +2039,20 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> if (ret)
> goto out;
>
> -   if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -   struct v4l2_mbus_framefmt *fmt =
> -   v4l2_subdev_get_try_format(sd, cfg, 0);
> +   if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> +   fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> +   else
> +   fmt = &sensor->fmt;
>
> -   *fmt = *mbus_fmt;
> -   goto out;
> -   }
> +   *fmt = *mbus_fmt;
>
> if (new_mode != sensor->current_mode) {
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
> }
> -   if (mbus_fmt->code != sensor->fmt.code) {
> -   sensor->fmt = *mbus_fmt;
> +   if (mbus_fmt->code != sensor->fmt.code)
> sensor->pending_fmt_change = true;
> -   }
> +
>  out:
> mutex_unlock(&sensor->lock);
> return ret;
> --
> git-series 0.9.1


[GIT FIXES FOR v4.20] seco-cec: add missing header

2018-12-03 Thread Hans Verkuil
The following changes since commit 708d75fe1c7c6e9abc5381b6fcc32b49830383d0:

  media: dvb-pll: don't re-validate tuner frequencies (2018-11-23 12:27:18 
-0500)

are available in the Git repository at:

  git://linuxtv.org/hverkuil/media_tree.git tags/br-v4.20p

for you to fetch changes up to 783f306b4e480b964daa5ac45d13edd1579f42e4:

  media: seco-cec: add missing header file to fix build (2018-12-03 20:26:37 
+0100)


Tag branch


Randy Dunlap (1):
  media: seco-cec: add missing header file to fix build

 drivers/media/platform/seco-cec/seco-cec.c | 1 +
 1 file changed, 1 insertion(+)


Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE

2018-12-03 Thread Doug Ledford
On Mon, 2018-11-26 at 17:56 +0530, Anshuman Khandual wrote:
> At present there are multiple places where invalid node number is encoded
> as -1. Even though implicitly understood it is always better to have macros
> in there. Replace these open encodings for an invalid node number with the
> global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
> 'invalid node' from various places redirecting them to a common definition.
> 
> Signed-off-by: Anshuman Khandual 
> ---
> Changes in V2:
> 
> - Added inclusion of 'numa.h' header at various places per Andrew
> - Updated 'dev_to_node' to use NUMA_NO_NODE instead per Vinod
> 
> Changes in V1: (https://lkml.org/lkml/2018/11/23/485)
> 
> - Dropped OCFS2 changes per Joseph
> - Dropped media/video drivers changes per Hans
> 
> RFC - https://patchwork.kernel.org/patch/10678035/
> 
> Build tested this with multiple cross compiler options like alpha, sparc,
> arm64, x86, powerpc, powerpc64le etc with their default config which might
> not have compiled tested all driver related changes. I will appreciate
> folks giving this a test in their respective build environment.
> 
> All these places for replacement were found by running the following grep
> patterns on the entire kernel code. Please let me know if this might have
> missed some instances. This might also have replaced some false positives.
> I will appreciate suggestions, inputs and review.
> 
> 1. git grep "nid == -1"
> 2. git grep "node == -1"
> 3. git grep "nid = -1"
> 4. git grep "node = -1"
> 
>  drivers/infiniband/hw/hfi1/affinity.c |  3 ++-
>  drivers/infiniband/hw/hfi1/init.c |  3 ++-

For the drivers/infiniband changes,

Acked-by: Doug Ledford 

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part


Re: [GIT PULL FOR v4.21] dvb fixes

2018-12-03 Thread Mauro Carvalho Chehab
Em Wed, 28 Nov 2018 19:40:30 +
Sean Young  escreveu:

> Hi Mauro,
> 
> So I've gone through the outstanding DVB patches and picked up the easier
> ones to deal with first. Please scrutinise.

Thanks! Just one note. See below.


> 
> Thanks,
> 
> Sean
> 
> The following changes since commit 708d75fe1c7c6e9abc5381b6fcc32b49830383d0:
> 
>   media: dvb-pll: don't re-validate tuner frequencies (2018-11-23 12:27:18 
> -0500)
> 
> are available in the Git repository at:
> 
>   git://linuxtv.org/syoung/media_tree.git for-v4.21b
> 
> for you to fetch changes up to b2d148f755fc60840fbaec52388152896f8339be:
> 
>   media: sony-cxd2880: add optional vcc regulator to bindings (2018-11-27 
> 16:04:16 +)
> 
> 
> Colin Ian King (1):
>   media: dib0700: fix spelling mistake "Amplifyer" -> "Amplifier"
> 
> Hans Verkuil (1):
>   media: dib0900: fix smatch warnings
> 
> Julia Lawall (1):
>   media: mxl5xx: constify dvb_frontend_ops structure
> 
> Malcolm Priestley (1):
>   media: dvb-usb-v2: Fix incorrect use of transfer_flags URB_FREE_BUFFER
> 


> Neil Armstrong (3):
>   media: cxd2880-spi: fix probe when dvb_attach fails
>   media: cxd2880-spi: Add optional vcc regulator

Looks fine. You only forgot to add a:
Acked-by: Yasunari Takiguchi 

To the above patches. The Acked was written to patch 0/2. Patchwork
doesn't automatically handle replies to patch 0/x, so you need to
add the ack yourself ;-)

I added it myself and merged.

>   media: sony-cxd2880: add optional vcc regulator to bindings
> 
> Sean Young (1):
>   media: saa7134: rc-core maintains users count, no need to duplicate
> 
> Victor Toso (2):
>   media: af9033: Remove duplicated switch statement
>   media: dvb: Use WARM definition from identify_state()
> 
> zhong jiang (2):
>   media: usb: Use kmemdup instead of duplicating its function.
>   media: dvb-frontends: Use kmemdup instead of duplicating its function
> 
>  .../devicetree/bindings/media/spi/sony-cxd2880.txt |  4 ++
>  drivers/media/dvb-frontends/af9033.c   | 12 +---
>  drivers/media/dvb-frontends/dib0090.c  | 32 +-
>  drivers/media/dvb-frontends/lgdt3306a.c|  6 +-
>  drivers/media/dvb-frontends/mxl5xx.c   |  2 +-
>  drivers/media/pci/saa7134/saa7134-core.c   |  8 +--
>  drivers/media/pci/saa7134/saa7134-input.c  | 68 
> --
>  drivers/media/pci/saa7134/saa7134.h|  9 ++-
>  drivers/media/spi/cxd2880-spi.c| 17 ++
>  drivers/media/usb/dvb-usb-v2/dvb_usb_core.c|  6 +-
>  drivers/media/usb/dvb-usb-v2/gl861.c   |  3 +-
>  drivers/media/usb/dvb-usb-v2/usb_urb.c |  5 +-
>  drivers/media/usb/dvb-usb/dib0700_devices.c|  2 +-
>  13 files changed, 66 insertions(+), 108 deletions(-)



Thanks,
Mauro


[GIT PULL for v4.20-rc6] media fixes

2018-12-03 Thread Mauro Carvalho Chehab
Hi Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.20-4

For a couple of fixes:

  - Revert a dt-bindings patch whose driver didn't make for 4.20;
  - fix a kernel oops at vicodec driver;
  - fix a frame overflow at gspca with was causing regressions on some
cameras, making them to not work;
  - use the proper type for wait_queue head;
  - make media request API compatible with 32 bits userspace on 64 bits
Kernel;
  - fix a regression on Kernel 4.19 at dvb-pll;
  - don't use SPDX headers yet for GFDL.

Thanks!
Mauro

The following changes since commit 4e26f692e2e2aa4d7d6ddb3c4d3dec17f45d6495:

  media: ipu3-cio2: Use cio2_queues_exit (2018-11-06 07:11:59 -0500)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.20-4

for you to fetch changes up to a7c3a0d5f8d8cd5cdb32c06d4d68f5b4e4d2104b:

  media: mediactl docs: Fix licensing message (2018-11-27 13:52:46 -0500)


media fixes for v4.20-rc4


Ezequiel Garcia (1):
  media: Revert "media: dt-bindings: Document the Rockchip VPU bindings"

Hans Verkuil (3):
  media: cedrus: add action item to the TODO
  media: vicodec: fix memchr() kernel oops
  media: gspca: fix frame overflow error

Jasmin Jessich (1):
  media: Use wait_queue_head_t for media_request

Jernej Skrabec (1):
  media: media-request: Add compat ioctl

Mauro Carvalho Chehab (3):
  media: dvb-pll: fix tuner frequency ranges
  media: dvb-pll: don't re-validate tuner frequencies
  media: mediactl docs: Fix licensing message

 .../devicetree/bindings/media/rockchip-vpu.txt |  29 --
 .../uapi/mediactl/media-ioc-request-alloc.rst  |  26 -
 .../uapi/mediactl/media-request-ioc-queue.rst  |  26 -
 .../uapi/mediactl/media-request-ioc-reinit.rst |  26 -
 Documentation/media/uapi/mediactl/request-api.rst  |  26 -
 .../media/uapi/mediactl/request-func-close.rst |  26 -
 .../media/uapi/mediactl/request-func-ioctl.rst |  26 -
 .../media/uapi/mediactl/request-func-poll.rst  |  26 -
 drivers/media/dvb-frontends/dvb-pll.c  | 106 ++---
 drivers/media/media-request.c  |   3 +
 drivers/media/platform/vicodec/vicodec-core.c  |   3 +-
 drivers/media/usb/gspca/gspca.c|  11 ++-
 drivers/staging/media/sunxi/cedrus/TODO|   5 +
 include/media/media-request.h  |   2 +-
 14 files changed, 240 insertions(+), 101 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/rockchip-vpu.txt



Re: [PATCH 2/2] media: ov5640: make MIPI clock depend on mode

2018-12-03 Thread jacopo mondi
Hi Adam,
thanks for testing

On Mon, Dec 03, 2018 at 10:28:04AM -0600, Adam Ford wrote:
> On Thu, Nov 29, 2018 at 8:49 AM Jacopo Mondi  
> wrote:
> >
> > The MIPI clock generation tree uses the MIPI_DIV divider to generate both 
> > the
> > MIPI SCALER CLOCK and the PIXEL_CLOCK. It seems the MIPI BIT CLK frequency 
> > is
> > generated by either the MIPI SCALER or the PIXEL CLOCK, depending if the
> > currently applied image mode uses the scaler or not.
> >
> > Make the MIPI_DIV selection value depend on the subsampling mode used by the
> > currently applied image mode.
> >
> > Tested with:
> > 172x144 320x240, 640x480, 1024x756, 1024x768, 1280x720, 1920x1080 in YUYV 
> > mode
> > at 10, 15, 20, and 30 FPS with MIPI CSI-2 2 lanes mode.
> >
> > Signed-off-by: Jacopo Mondi 
>
> I am not able to apply this patch to 4.19.6, 4.19-RC5, or Linux-media master 
> [1]
>
> Is there a specific branch/repo somewhere I can pull?  I was able to
> apply patch 1/2 just fine, but 2/2 wouldn't apply

Reading the cover letter:
"these two patches should be applied on top of Maxime's clock tree rework v5"

Maxime has included those 2 in his v6. You may want to test that one
:)

Thanks
   j

>
> [1] - git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git
>
> adam
> > ---
> > Maxime,
> >this patch is not just a cosmetic updates to the 'set_mipi_pclk()'
> > comment block as I anticipated, but it actually fix the framerate handling
> > compared for CSI-2, which in you v5 results halved for most modes. I have
> > not included any "Fixes:" tag, as I hope this patch will get in with your 
> > v5.
> >
> > That's my bad, as in the patches I sent to be applied on top of your v4 I
> > forgot to add a change, and the requested SCLK rate was always half of what
> > it was actually required.
> >
> > Also, I have left out from this patches most of Sam's improvements (better 
> > SCLK
> > selection policies, and programming of register OV5640_REG_PCLK_PERIOD as 
> > for
> > from testing they're not required, and I don't want to pile more patches 
> > than
> > necessary for the next merge window, not to slow down the clock tree rework
> > inclusion. We can get back to it after it got merged.
> >
> > This are the test result obtained by capturing 100 frames with yavta and
> > inspecting the reported fps results.
> >
> > Results are pretty neat, 2592x1944 mode apart, which runs a bit slow.
> >
> > capturing 176x144 @ 10FPS
> > Captured 100 frames in 10.019398 seconds (9.980640 fps, 505898.657683 B/s).
> > capturing 176x144 @ 15FPS
> > Captured 100 frames in 6.681860 seconds (14.965892 fps, 758591.132803 B/s).
> > capturing 176x144 @ 20FPS
> > Captured 100 frames in 5.056204 seconds (19.777683 fps, 1002491.196755 B/s).
> > capturing 176x144 @ 30FPS
> > Captured 100 frames in 3.357204 seconds (29.786686 fps, 1509827.521040 B/s).
> >
> > capturing 320x240 @ 10FPS
> > Captured 100 frames in 10.015432 seconds (9.984591 fps, 1533633.245951 B/s).
> > capturing 320x240 @ 15FPS
> > Captured 100 frames in 6.684035 seconds (14.961021 fps, 2298012.872049 B/s).
> > capturing 320x240 @ 20FPS
> > Captured 100 frames in 5.019164 seconds (19.923635 fps, 3060270.391218 B/s).
> > capturing 320x240 @ 30FPS
> > Captured 100 frames in 3.352991 seconds (29.824115 fps, 4580984.103432 B/s).
> >
> > capturing 640x480 @ 10FPS
> > Captured 100 frames in 9.990389 seconds (10.009620 fps, 6149910.678538 B/s).
> > capturing 640x480 @ 15FPS
> > Captured 100 frames in 6.856242 seconds (14.585249 fps, 8961176.838123 B/s).
> > capturing 640x480 @ 20FPS
> > Captured 100 frames in 5.030264 seconds (19.879670 fps, 12214069.053476 
> > B/s).
> > capturing 640x480 @ 30FPS
> > Captured 100 frames in 3.364612 seconds (29.721103 fps, 18260645.750580 
> > B/s).
> >
> > capturing 720x480 @ 10FPS
> > Captured 100 frames in 10.022488 seconds (9.977562 fps, 6896491.169279 B/s).
> > capturing 720x480 @ 15FPS
> > Captured 100 frames in 6.891968 seconds (14.509643 fps, 10029065.232208 
> > B/s).
> > capturing 720x480 @ 20FPS
> > Captured 100 frames in 5.234133 seconds (19.105360 fps, 13205624.616211 
> > B/s).
> > capturing 720x480 @ 30FPS
> > Captured 100 frames in 3.602298 seconds (27.760055 fps, 19187750.044688 
> > B/s).
> >
> > capturing 720x576 @ 10FPS
> > Captured 100 frames in 10.197244 seconds (9.806571 fps, 8133961.937805 B/s).
> > capturing 720x576 @ 15FPS
> > Captured 100 frames in 6.925244 seconds (14.439924 fps, 11977050.339261 
> > B/s).
> > capturing 720x576 @ 20FPS
> > Captured 100 frames in 4.68 seconds (20.000127 fps, 16588905.060854 
> > B/s).
> > capturing 720x576 @ 30FPS
> > Captured 100 frames in 3.487568 seconds (28.673276 fps, 23782762.085212 
> > B/s).
> >
> > capturing 1024x768 @ 10FPS
> > Captured 100 frames in 10.107128 seconds (9.894007 fps, 15561928.174298 
> > B/s).
> > capturing 1024x768 @ 15FPS
> > Captured 100 frames in 6.810568 seconds (14.683062 fps, 23094459.169337 
> > B/s).
> > capturing 1024x768 @ 20FPS
> > Captured 100 frames in

Re: [PATCH 2/2] media: ov5640: make MIPI clock depend on mode

2018-12-03 Thread Adam Ford
On Mon, Dec 3, 2018 at 10:28 AM Adam Ford  wrote:
>
> On Thu, Nov 29, 2018 at 8:49 AM Jacopo Mondi  
> wrote:
> >
> > The MIPI clock generation tree uses the MIPI_DIV divider to generate both 
> > the
> > MIPI SCALER CLOCK and the PIXEL_CLOCK. It seems the MIPI BIT CLK frequency 
> > is
> > generated by either the MIPI SCALER or the PIXEL CLOCK, depending if the
> > currently applied image mode uses the scaler or not.
> >
> > Make the MIPI_DIV selection value depend on the subsampling mode used by the
> > currently applied image mode.
> >
> > Tested with:
> > 172x144 320x240, 640x480, 1024x756, 1024x768, 1280x720, 1920x1080 in YUYV 
> > mode
> > at 10, 15, 20, and 30 FPS with MIPI CSI-2 2 lanes mode.
> >
> > Signed-off-by: Jacopo Mondi 
>
> I am not able to apply this patch to 4.19.6, 4.19-RC5, or Linux-media master 
> [1]
>
> Is there a specific branch/repo somewhere I can pull?  I was able to
> apply patch 1/2 just fine, but 2/2 wouldn't apply
>

I also attempted to apply to [2] without success.

> [1] - git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git

[2] - git://linuxtv.org/media_tree.git

>
> adam
> > ---
> > Maxime,
> >this patch is not just a cosmetic updates to the 'set_mipi_pclk()'
> > comment block as I anticipated, but it actually fix the framerate handling
> > compared for CSI-2, which in you v5 results halved for most modes. I have
> > not included any "Fixes:" tag, as I hope this patch will get in with your 
> > v5.
> >
> > That's my bad, as in the patches I sent to be applied on top of your v4 I
> > forgot to add a change, and the requested SCLK rate was always half of what
> > it was actually required.
> >
> > Also, I have left out from this patches most of Sam's improvements (better 
> > SCLK
> > selection policies, and programming of register OV5640_REG_PCLK_PERIOD as 
> > for
> > from testing they're not required, and I don't want to pile more patches 
> > than
> > necessary for the next merge window, not to slow down the clock tree rework
> > inclusion. We can get back to it after it got merged.
> >
> > This are the test result obtained by capturing 100 frames with yavta and
> > inspecting the reported fps results.
> >
> > Results are pretty neat, 2592x1944 mode apart, which runs a bit slow.
> >
> > capturing 176x144 @ 10FPS
> > Captured 100 frames in 10.019398 seconds (9.980640 fps, 505898.657683 B/s).
> > capturing 176x144 @ 15FPS
> > Captured 100 frames in 6.681860 seconds (14.965892 fps, 758591.132803 B/s).
> > capturing 176x144 @ 20FPS
> > Captured 100 frames in 5.056204 seconds (19.777683 fps, 1002491.196755 B/s).
> > capturing 176x144 @ 30FPS
> > Captured 100 frames in 3.357204 seconds (29.786686 fps, 1509827.521040 B/s).
> >
> > capturing 320x240 @ 10FPS
> > Captured 100 frames in 10.015432 seconds (9.984591 fps, 1533633.245951 B/s).
> > capturing 320x240 @ 15FPS
> > Captured 100 frames in 6.684035 seconds (14.961021 fps, 2298012.872049 B/s).
> > capturing 320x240 @ 20FPS
> > Captured 100 frames in 5.019164 seconds (19.923635 fps, 3060270.391218 B/s).
> > capturing 320x240 @ 30FPS
> > Captured 100 frames in 3.352991 seconds (29.824115 fps, 4580984.103432 B/s).
> >
> > capturing 640x480 @ 10FPS
> > Captured 100 frames in 9.990389 seconds (10.009620 fps, 6149910.678538 B/s).
> > capturing 640x480 @ 15FPS
> > Captured 100 frames in 6.856242 seconds (14.585249 fps, 8961176.838123 B/s).
> > capturing 640x480 @ 20FPS
> > Captured 100 frames in 5.030264 seconds (19.879670 fps, 12214069.053476 
> > B/s).
> > capturing 640x480 @ 30FPS
> > Captured 100 frames in 3.364612 seconds (29.721103 fps, 18260645.750580 
> > B/s).
> >
> > capturing 720x480 @ 10FPS
> > Captured 100 frames in 10.022488 seconds (9.977562 fps, 6896491.169279 B/s).
> > capturing 720x480 @ 15FPS
> > Captured 100 frames in 6.891968 seconds (14.509643 fps, 10029065.232208 
> > B/s).
> > capturing 720x480 @ 20FPS
> > Captured 100 frames in 5.234133 seconds (19.105360 fps, 13205624.616211 
> > B/s).
> > capturing 720x480 @ 30FPS
> > Captured 100 frames in 3.602298 seconds (27.760055 fps, 19187750.044688 
> > B/s).
> >
> > capturing 720x576 @ 10FPS
> > Captured 100 frames in 10.197244 seconds (9.806571 fps, 8133961.937805 B/s).
> > capturing 720x576 @ 15FPS
> > Captured 100 frames in 6.925244 seconds (14.439924 fps, 11977050.339261 
> > B/s).
> > capturing 720x576 @ 20FPS
> > Captured 100 frames in 4.68 seconds (20.000127 fps, 16588905.060854 
> > B/s).
> > capturing 720x576 @ 30FPS
> > Captured 100 frames in 3.487568 seconds (28.673276 fps, 23782762.085212 
> > B/s).
> >
> > capturing 1024x768 @ 10FPS
> > Captured 100 frames in 10.107128 seconds (9.894007 fps, 15561928.174298 
> > B/s).
> > capturing 1024x768 @ 15FPS
> > Captured 100 frames in 6.810568 seconds (14.683062 fps, 23094459.169337 
> > B/s).
> > capturing 1024x768 @ 20FPS
> > Captured 100 frames in 5.012039 seconds (19.951960 fps, 31381719.096759 
> > B/s).
> > capturing 1024x768 @ 30FPS
> > Captured 100 frames in 3.346338 seconds (29

Re: [PATCH 1/2] media: ov5640: Fix set format regression

2018-12-03 Thread Adam Ford
On Thu, Nov 29, 2018 at 8:48 AM Jacopo Mondi  wrote:
>
> The set_fmt operations updates the sensor format only when the image format
> is changed. When only the image sizes gets changed, the format do not get
> updated causing the sensor to always report the one that was previously in
> use.
>
> Without this patch, updating frame size only fails:
>   [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> With this patch applied:
>   [fmt:UYVY8_2X8/1024x768@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> Fixes: 6949d864776e ("media: ov5640: do not change mode if format or frame
> interval is unchanged")
> Signed-off-by: Jacopo Mondi 

For patch 1 of 2 only,

Tested-by: Adam Ford #imx6d with CSI2
interface on 4.19.6 and 4.20-RC5

It would be great if this could be applied to 4.19+

adam
> ---
>  drivers/media/i2c/ov5640.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 03a031a42b3e..c659efe918a4 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2160,6 +2160,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> struct ov5640_dev *sensor = to_ov5640_dev(sd);
> const struct ov5640_mode_info *new_mode;
> struct v4l2_mbus_framefmt *mbus_fmt = &format->format;
> +   struct v4l2_mbus_framefmt *fmt;
> int ret;
>
> if (format->pad != 0)
> @@ -2177,22 +2178,20 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> if (ret)
> goto out;
>
> -   if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -   struct v4l2_mbus_framefmt *fmt =
> -   v4l2_subdev_get_try_format(sd, cfg, 0);
> +   if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> +   fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> +   else
> +   fmt = &sensor->fmt;
>
> -   *fmt = *mbus_fmt;
> -   goto out;
> -   }
> +   *fmt = *mbus_fmt;
>
> if (new_mode != sensor->current_mode) {
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
> }
> -   if (mbus_fmt->code != sensor->fmt.code) {
> -   sensor->fmt = *mbus_fmt;
> +   if (mbus_fmt->code != sensor->fmt.code)
> sensor->pending_fmt_change = true;
> -   }
> +
>  out:
> mutex_unlock(&sensor->lock);
> return ret;
> --
> 2.7.4
>


Re: [PATCH v4] media: vivid: Improve timestamping

2018-12-03 Thread Arnd Bergmann
On Mon, Dec 3, 2018 at 10:15 AM Hans Verkuil  wrote:
>
> On 12/02/2018 09:43 PM, Arnd Bergmann wrote:
> > On Sun, Dec 2, 2018 at 2:47 PM Gabriel Francisco Mandaji
> >  wrote:
> >
> >> @@ -667,10 +653,28 @@ static void vivid_overlay(struct vivid_dev *dev, 
> >> struct vivid_buffer *buf)
> >> }
> >>  }
> >>
> >> +static void vivid_cap_update_frame_period(struct vivid_dev *dev)
> >> +{
> >> +   u64 f_period;
> >> +
> >> +   f_period = (u64)dev->timeperframe_vid_cap.numerator * 10;
> >> +   do_div(f_period, dev->timeperframe_vid_cap.denominator);
> >> +   if (dev->field_cap == V4L2_FIELD_ALTERNATE)
> >> +   do_div(f_period, 2);
> >> +   /*
> >> +* If "End of Frame", then offset the exposure time by 0.9
> >> +* of the frame period.
> >> +*/
> >> +   dev->cap_frame_eof_offset = f_period * 9;
> >> +   do_div(dev->cap_frame_eof_offset, 10);
> >> +   dev->cap_frame_period = f_period;
> >> +}
> >
> > Doing two or three do_div() operations is going to make this rather
> > expensive on 32-bit architectures, and it looks like this happens for
> > each frame?
> >
> > Since each one is a multiplication followed by a division, could this
> > be changed to using a different factor followed by a bit shift?
>
> The division by 2 can obviously be replaced by a shift, and the
> 'End of Frame' calculation can be simplified as well by multiplying by
> 7 and dividing by 8 (again a simple shift): this equals 0.875 which is
> close enough to 0.9 (so update the comment as well).

The first division

  f_period = (u64)dev->timeperframe_vid_cap.numerator * 10;
  do_div(f_period, dev->timeperframe_vid_cap.denominator);

looks like it could be replaced with a fixed multiplication with a
precomputed 'u64 factor = div_u64(numerator * 1, denominator)

> It's all a bit overkill since this function isn't called very often,
> but these are easy changes to make.

Ah, I assumed it was called once per frame or more. If this is only done
during initalization, it doesn't matter.

Arnd


Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver

2018-12-03 Thread Eddie James




On 12/03/2018 05:04 AM, Hans Verkuil wrote:

On 11/27/2018 08:37 PM, Eddie James wrote:

The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
can capture and compress video data from digital or analog sources. With
the Aspeed chip acting a service processor, the Video Engine can capture
the host processor graphics output.

Add a V4L2 driver to capture video data and compress it to JPEG images.
Make the video frames available through the V4L2 streaming interface.

Signed-off-by: Eddie James 
---
  MAINTAINERS   |8 +
  drivers/media/platform/Kconfig|9 +
  drivers/media/platform/Makefile   |1 +
  drivers/media/platform/aspeed-video.c | 1719 +
  4 files changed, 1737 insertions(+)
  create mode 100644 drivers/media/platform/aspeed-video.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 602142c..51f513f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2423,6 +2423,14 @@ S:   Maintained
  F:Documentation/hwmon/asc7621
  F:drivers/hwmon/asc7621.c
  
+ASPEED VIDEO ENGINE DRIVER

+M: Eddie James 
+L: linux-media@vger.kernel.org
+L: open...@lists.ozlabs.org (moderated for non-subscribers)
+S: Maintained
+F: drivers/media/platform/aspeed-video.c
+F: Documentation/devicetree/bindings/media/aspeed-video.txt
+
  ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
  M:Corentin Chary 
  L:acpi4asus-u...@lists.sourceforge.net
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index ea33063..a505e9f 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig"
  
  source "drivers/media/platform/omap/Kconfig"
  
+config VIDEO_ASPEED

+   tristate "Aspeed AST2400 and AST2500 Video Engine driver"
+   depends on VIDEO_V4L2
+   select VIDEOBUF2_DMA_CONTIG
+   help
+ Support for the Aspeed Video Engine (VE) embedded in the Aspeed
+ AST2400 and AST2500 SOCs. The VE can capture and compress video data
+ from digital or analog sources.
+
  config VIDEO_SH_VOU
tristate "SuperH VOU video output driver"
depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index d347a55..e6deb25 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -3,6 +3,7 @@
  # Makefile for the video capture/playback device drivers.
  #
  
+obj-$(CONFIG_VIDEO_ASPEED)		+= aspeed-video.o

  obj-$(CONFIG_VIDEO_CADENCE)   += cadence/
  obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o
  obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/
diff --git a/drivers/media/platform/aspeed-video.c 
b/drivers/media/platform/aspeed-video.c
new file mode 100644
index 000..200f4d82
--- /dev/null
+++ b/drivers/media/platform/aspeed-video.c
@@ -0,0 +1,1719 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEVICE_NAME"aspeed-video"
+
+#define ASPEED_VIDEO_JPEG_NUM_QUALITIES12
+#define ASPEED_VIDEO_JPEG_HEADER_SIZE  10
+#define ASPEED_VIDEO_JPEG_QUANT_SIZE   116
+#define ASPEED_VIDEO_JPEG_DCT_SIZE 34
+
+#define MAX_FRAME_RATE 60
+#define MAX_HEIGHT 1200
+#define MAX_WIDTH  1920
+#define MIN_HEIGHT 480
+#define MIN_WIDTH  640
+
+#define NUM_POLARITY_CHECKS10
+#define INVALID_RESOLUTION_RETRIES 2
+#define INVALID_RESOLUTION_DELAY   msecs_to_jiffies(250)
+#define RESOLUTION_CHANGE_DELAYmsecs_to_jiffies(500)
+#define MODE_DETECT_TIMEOUTmsecs_to_jiffies(500)
+#define STOP_TIMEOUT   msecs_to_jiffies(250)
+#define DIRECT_FETCH_THRESHOLD 0x0c /* 1024 * 768 */
+
+#define VE_MAX_SRC_BUFFER_SIZE 0x8ca000 /* 1920 * 1200, 32bpp */
+#define VE_JPEG_HEADER_SIZE0x006000 /* 512 * 12 * 4 */
+
+#define VE_PROTECTION_KEY  0x000
+#define  VE_PROTECTION_KEY_UNLOCK  0x1a038aa8
+
+#define VE_SEQ_CTRL0x004
+#define  VE_SEQ_CTRL_TRIG_MODE_DET BIT(0)
+#define  VE_SEQ_CTRL_TRIG_CAPTURE  BIT(1)
+#define  VE_SEQ_CTRL_FORCE_IDLEBIT(2)
+#define  VE_SEQ_CTRL_MULT_FRAMEBIT(3)
+#define  VE_SEQ_CTRL_TRIG_COMP BIT(4)
+#define  VE_SEQ_CTRL_AUTO_COMP BIT(5)
+#define  VE_SEQ_CTRL_EN_WATCHDOG   BIT(7)
+#define  VE_SEQ_CTRL_YUV420BIT(10)
+#define  VE_SEQ_CTRL_COMP_FMT  GENMASK(11, 10)
+#define  VE_SEQ_CTRL_HALT  BIT(12)
+#define  VE_SEQ_CTRL_EN_WATCHDOG_COMP  BIT(14)
+#define  VE_SEQ_

Re: [PATCH 2/2] media: ov5640: make MIPI clock depend on mode

2018-12-03 Thread Adam Ford
On Thu, Nov 29, 2018 at 8:49 AM Jacopo Mondi  wrote:
>
> The MIPI clock generation tree uses the MIPI_DIV divider to generate both the
> MIPI SCALER CLOCK and the PIXEL_CLOCK. It seems the MIPI BIT CLK frequency is
> generated by either the MIPI SCALER or the PIXEL CLOCK, depending if the
> currently applied image mode uses the scaler or not.
>
> Make the MIPI_DIV selection value depend on the subsampling mode used by the
> currently applied image mode.
>
> Tested with:
> 172x144 320x240, 640x480, 1024x756, 1024x768, 1280x720, 1920x1080 in YUYV mode
> at 10, 15, 20, and 30 FPS with MIPI CSI-2 2 lanes mode.
>
> Signed-off-by: Jacopo Mondi 

I am not able to apply this patch to 4.19.6, 4.19-RC5, or Linux-media master [1]

Is there a specific branch/repo somewhere I can pull?  I was able to
apply patch 1/2 just fine, but 2/2 wouldn't apply

[1] - git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git

adam
> ---
> Maxime,
>this patch is not just a cosmetic updates to the 'set_mipi_pclk()'
> comment block as I anticipated, but it actually fix the framerate handling
> compared for CSI-2, which in you v5 results halved for most modes. I have
> not included any "Fixes:" tag, as I hope this patch will get in with your v5.
>
> That's my bad, as in the patches I sent to be applied on top of your v4 I
> forgot to add a change, and the requested SCLK rate was always half of what
> it was actually required.
>
> Also, I have left out from this patches most of Sam's improvements (better 
> SCLK
> selection policies, and programming of register OV5640_REG_PCLK_PERIOD as for
> from testing they're not required, and I don't want to pile more patches than
> necessary for the next merge window, not to slow down the clock tree rework
> inclusion. We can get back to it after it got merged.
>
> This are the test result obtained by capturing 100 frames with yavta and
> inspecting the reported fps results.
>
> Results are pretty neat, 2592x1944 mode apart, which runs a bit slow.
>
> capturing 176x144 @ 10FPS
> Captured 100 frames in 10.019398 seconds (9.980640 fps, 505898.657683 B/s).
> capturing 176x144 @ 15FPS
> Captured 100 frames in 6.681860 seconds (14.965892 fps, 758591.132803 B/s).
> capturing 176x144 @ 20FPS
> Captured 100 frames in 5.056204 seconds (19.777683 fps, 1002491.196755 B/s).
> capturing 176x144 @ 30FPS
> Captured 100 frames in 3.357204 seconds (29.786686 fps, 1509827.521040 B/s).
>
> capturing 320x240 @ 10FPS
> Captured 100 frames in 10.015432 seconds (9.984591 fps, 1533633.245951 B/s).
> capturing 320x240 @ 15FPS
> Captured 100 frames in 6.684035 seconds (14.961021 fps, 2298012.872049 B/s).
> capturing 320x240 @ 20FPS
> Captured 100 frames in 5.019164 seconds (19.923635 fps, 3060270.391218 B/s).
> capturing 320x240 @ 30FPS
> Captured 100 frames in 3.352991 seconds (29.824115 fps, 4580984.103432 B/s).
>
> capturing 640x480 @ 10FPS
> Captured 100 frames in 9.990389 seconds (10.009620 fps, 6149910.678538 B/s).
> capturing 640x480 @ 15FPS
> Captured 100 frames in 6.856242 seconds (14.585249 fps, 8961176.838123 B/s).
> capturing 640x480 @ 20FPS
> Captured 100 frames in 5.030264 seconds (19.879670 fps, 12214069.053476 B/s).
> capturing 640x480 @ 30FPS
> Captured 100 frames in 3.364612 seconds (29.721103 fps, 18260645.750580 B/s).
>
> capturing 720x480 @ 10FPS
> Captured 100 frames in 10.022488 seconds (9.977562 fps, 6896491.169279 B/s).
> capturing 720x480 @ 15FPS
> Captured 100 frames in 6.891968 seconds (14.509643 fps, 10029065.232208 B/s).
> capturing 720x480 @ 20FPS
> Captured 100 frames in 5.234133 seconds (19.105360 fps, 13205624.616211 B/s).
> capturing 720x480 @ 30FPS
> Captured 100 frames in 3.602298 seconds (27.760055 fps, 19187750.044688 B/s).
>
> capturing 720x576 @ 10FPS
> Captured 100 frames in 10.197244 seconds (9.806571 fps, 8133961.937805 B/s).
> capturing 720x576 @ 15FPS
> Captured 100 frames in 6.925244 seconds (14.439924 fps, 11977050.339261 B/s).
> capturing 720x576 @ 20FPS
> Captured 100 frames in 4.68 seconds (20.000127 fps, 16588905.060854 B/s).
> capturing 720x576 @ 30FPS
> Captured 100 frames in 3.487568 seconds (28.673276 fps, 23782762.085212 B/s).
>
> capturing 1024x768 @ 10FPS
> Captured 100 frames in 10.107128 seconds (9.894007 fps, 15561928.174298 B/s).
> capturing 1024x768 @ 15FPS
> Captured 100 frames in 6.810568 seconds (14.683062 fps, 23094459.169337 B/s).
> capturing 1024x768 @ 20FPS
> Captured 100 frames in 5.012039 seconds (19.951960 fps, 31381719.096759 B/s).
> capturing 1024x768 @ 30FPS
> Captured 100 frames in 3.346338 seconds (29.883407 fps, 47002534.905114 B/s).
>
> capturing 1280x720 @ 10FPS
> Captured 100 frames in 9.957613 seconds (10.042567 fps, 18510459.665283 B/s).
> capturing 1280x720 @ 15FPS
> Captured 100 frames in 6.597751 seconds (15.156679 fps, 27936790.986673 B/s).
> capturing 1280x720 @ 20FPS
> Captured 100 frames in 4.946115 seconds (20.217888 fps, 37265611.495083 B/s).
> capturing 1280x720 @ 30FPS
> Captured 100 frames in 3.301329 secon

Re: [PATCHv3 9/9] cedrus: add tag support

2018-12-03 Thread Paul Kocialkowski
Hi,

On Mon, 2018-12-03 at 14:51 +0100, hverkuil-ci...@xs4all.nl wrote:
> From: Hans Verkuil 
> 
> Replace old reference frame indices by new tag method.

> Signed-off-by: Hans Verkuil 
> Reviewed-by: Paul Kocialkowski 

I missed it earlier, but we should remember to update the MPEG-2
controls documentation to mention the use of tags instead of buffer
indices.

Cheers,

Paul

> Reviewed-by: Alexandre Courbot 
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c  |  9 
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  9 +---
>  .../staging/media/sunxi/cedrus/cedrus_dec.c   |  2 ++
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ---
>  .../staging/media/sunxi/cedrus/cedrus_video.c |  2 ++
>  include/uapi/linux/v4l2-controls.h| 14 +
>  6 files changed, 24 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 5f2b033a7a42..b854cceb19dc 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1660,15 +1660,6 @@ static int std_validate(const struct v4l2_ctrl *ctrl, 
> u32 idx,
>   return -EINVAL;
>   }
>  
> - if (p_mpeg2_slice_params->backward_ref_index >= VIDEO_MAX_FRAME 
> ||
> - p_mpeg2_slice_params->forward_ref_index >= VIDEO_MAX_FRAME)
> - return -EINVAL;
> -
> - if (p_mpeg2_slice_params->pad ||
> - p_mpeg2_slice_params->picture.pad ||
> - p_mpeg2_slice_params->sequence.pad)
> - return -EINVAL;
> -
>   return 0;
>  
>   case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h 
> b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 3f61248c57ac..781676b55a1b 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -142,11 +142,14 @@ static inline dma_addr_t cedrus_buf_addr(struct 
> vb2_buffer *buf,
>  }
>  
>  static inline dma_addr_t cedrus_dst_buf_addr(struct cedrus_ctx *ctx,
> -  unsigned int index,
> -  unsigned int plane)
> +  int index, unsigned int plane)
>  {
> - struct vb2_buffer *buf = ctx->dst_bufs[index];
> + struct vb2_buffer *buf;
>  
> + if (index < 0)
> + return 0;
> +
> + buf = ctx->dst_bufs[index];
>   return buf ? cedrus_buf_addr(buf, &ctx->dst_fmt, plane) : 0;
>  }
>  
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c 
> b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index e40180a33951..0cfd6036d0cd 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -53,6 +53,8 @@ void cedrus_device_run(void *priv)
>   break;
>   }
>  
> + v4l2_m2m_buf_copy_data(run.src, run.dst, true);
> +
>   dev->dec_ops[ctx->current_codec]->setup(ctx, &run);
>  
>   spin_unlock_irqrestore(&ctx->dev->irq_lock, flags);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c 
> b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> index 9abd39cae38c..fdde9a099153 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> @@ -82,7 +82,10 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
> struct cedrus_run *run)
>   dma_addr_t fwd_luma_addr, fwd_chroma_addr;
>   dma_addr_t bwd_luma_addr, bwd_chroma_addr;
>   struct cedrus_dev *dev = ctx->dev;
> + struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
>   const u8 *matrix;
> + int forward_idx;
> + int backward_idx;
>   unsigned int i;
>   u32 reg;
>  
> @@ -156,23 +159,17 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
> struct cedrus_run *run)
>   cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
>  
>   /* Forward and backward prediction reference buffers. */
> + forward_idx = vb2_find_tag(cap_q, slice_params->forward_ref_tag, 0);
>  
> - fwd_luma_addr = cedrus_dst_buf_addr(ctx,
> - slice_params->forward_ref_index,
> - 0);
> - fwd_chroma_addr = cedrus_dst_buf_addr(ctx,
> -   slice_params->forward_ref_index,
> -   1);
> + fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0);
> + fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1);
>  
>   cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr);
>   cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, fwd_chroma_addr);
>  
> - bwd_luma_addr = cedrus_dst_buf_addr(ctx,
> - slice_params->backward_ref_index,
> -   

Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver

2018-12-03 Thread Hans Verkuil
On 12/03/2018 12:04 PM, Hans Verkuil wrote:
> On 11/27/2018 08:37 PM, Eddie James wrote:
>> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
>> can capture and compress video data from digital or analog sources. With
>> the Aspeed chip acting a service processor, the Video Engine can capture
>> the host processor graphics output.
>>
>> Add a V4L2 driver to capture video data and compress it to JPEG images.
>> Make the video frames available through the V4L2 streaming interface.
>>
>> Signed-off-by: Eddie James 
>> ---



>> +static void aspeed_video_bufs_done(struct aspeed_video *video,
>> +   enum vb2_buffer_state state)
>> +{
>> +unsigned long flags;
>> +struct aspeed_video_buffer *buf;
>> +
>> +spin_lock_irqsave(&video->lock, flags);
>> +list_for_each_entry(buf, &video->buffers, link) {
>> +if (list_is_last(&buf->link, &video->buffers))
>> +buf->vb.flags |= V4L2_BUF_FLAG_LAST;
> 
> This really makes no sense. This flag is for codecs, not for receivers.
> 
> You say in an earlier reply about this:
> 
> "I mentioned before that dequeue calls hang in an error condition unless
> this flag is specified. For example if resolution change is detected and
> application is in the middle of trying to dequeue..."
> 
> What error condition are you referring to? Isn't your application using
> the select() or poll() calls to wait for events or new buffers to dequeue?
> If you just call VIDIOC_DQBUF to wait in blocking mode for a new buffer,
> then it will indeed block in that call.
> 
> No other video receiver needs this flag, so there is something else that is
> the cause.

Let me give a bit more information on how video receivers behave when the
signal disappears:

They will all send the SOURCE_CHANGE event, but what they do with respect
to streaming buffers is hardware dependent:

1) Some receivers have a freewheeling mode where the hardware generates
   an image when there is no signal (usually this is just a fixed color).
   In that case the application will just keep receiving buffers.

2) VIDIOC_DQBUF blocks until a new signal appears with the same timings,
   then the driver will just keep going as if nothing changed. DQBUF
   remains blocked as long as there is no signal, or the timings are
   different from the currently active timings.

3) The hardware requires a hard stop and cannot continue streaming. In
   that case it can call vb2_queue_error().

That last option should be avoided if possible as it's not very polite.
>From what I can tell from this hardware it seems option 2 is the
appropriate choice.

Regards,

Hans

> 
>> +vb2_buffer_done(&buf->vb.vb2_buf, state);
>> +}
>> +INIT_LIST_HEAD(&video->buffers);
>> +spin_unlock_irqrestore(&video->lock, flags);
>> +}


Re: [PATCH v5] media: imx: add mem2mem device

2018-12-03 Thread Hans Verkuil
On 12/03/2018 12:48 PM, Philipp Zabel wrote:
> Add a single imx-media mem2mem video device that uses the IPU IC PP
> (image converter post processing) task for scaling and colorspace
> conversion.
> On i.MX6Q/DL SoCs with two IPUs currently only the first IPU is used.
> 
> The hardware only supports writing to destination buffers up to
> 1024x1024 pixels in a single pass, arbitrary sizes can be achieved
> by rendering multiple tiles per frame.
> 
> Signed-off-by: Philipp Zabel 
> [slongerb...@gmail.com: use ipu_image_convert_adjust(), fix
>  device_run() error handling]
> Signed-off-by: Steve Longerbeam 
> ---
> Changes since v4:
>  - No functional changes.
>  - Dropped deprecated TODO comment. This driver has no interaction with
>the IC task v4l2 subdevices.
>  - Dropped ipu-v3 patches, those are merged independently via imx-drm.

These land in kernel 4.21? Or are they already in 4.20?

> ---
>  drivers/staging/media/imx/Kconfig |   1 +
>  drivers/staging/media/imx/Makefile|   1 +
>  drivers/staging/media/imx/imx-media-dev.c |  10 +
>  drivers/staging/media/imx/imx-media-mem2mem.c | 873 ++

Please give this file a better name! imx-media-csc-scaler.c or something is
much more descriptive. Calling it mem2mem doesn't tell me what it actually does!

>  drivers/staging/media/imx/imx-media.h |  10 +
>  5 files changed, 895 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx-media-mem2mem.c
> 
> diff --git a/drivers/staging/media/imx/Kconfig 
> b/drivers/staging/media/imx/Kconfig
> index bfc17de56b17..07013cb3cb66 100644
> --- a/drivers/staging/media/imx/Kconfig
> +++ b/drivers/staging/media/imx/Kconfig
> @@ -6,6 +6,7 @@ config VIDEO_IMX_MEDIA
>   depends on HAS_DMA
>   select VIDEOBUF2_DMA_CONTIG
>   select V4L2_FWNODE
> + select V4L2_MEM2MEM_DEV
>   ---help---
> Say yes here to enable support for video4linux media controller
> driver for the i.MX5/6 SOC.
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index 698a4210316e..f2e722d0fa19 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -6,6 +6,7 @@ imx-media-ic-objs := imx-ic-common.o imx-ic-prp.o 
> imx-ic-prpencvf.o
>  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media.o
>  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-common.o
>  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-capture.o
> +obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-mem2mem.o
>  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-vdic.o
>  obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o
>  
> diff --git a/drivers/staging/media/imx/imx-media-dev.c 
> b/drivers/staging/media/imx/imx-media-dev.c
> index 4b344a4a3706..0376b52cb784 100644
> --- a/drivers/staging/media/imx/imx-media-dev.c
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -318,6 +318,16 @@ static int imx_media_probe_complete(struct 
> v4l2_async_notifier *notifier)
>   goto unlock;
>  
>   ret = v4l2_device_register_subdev_nodes(&imxmd->v4l2_dev);
> + if (ret)
> + goto unlock;
> +
> + imxmd->m2m_vdev = imx_media_mem2mem_device_init(imxmd);
> + if (IS_ERR(imxmd->m2m_vdev)) {
> + ret = PTR_ERR(imxmd->m2m_vdev);
> + goto unlock;
> + }
> +
> + ret = imx_media_mem2mem_device_register(imxmd->m2m_vdev);
>  unlock:
>   mutex_unlock(&imxmd->mutex);
>   if (ret)
> diff --git a/drivers/staging/media/imx/imx-media-mem2mem.c 
> b/drivers/staging/media/imx/imx-media-mem2mem.c
> new file mode 100644
> index ..a2a4dca017ce
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-mem2mem.c
> @@ -0,0 +1,873 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * i.MX IPUv3 mem2mem Scaler/CSC driver
> + *
> + * Copyright (C) 2011 Pengutronix, Sascha Hauer
> + * Copyright (C) 2018 Pengutronix, Philipp Zabel
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "imx-media.h"
> +
> +#define fh_to_ctx(__fh)  container_of(__fh, struct mem2mem_ctx, fh)
> +
> +enum {
> + V4L2_M2M_SRC = 0,
> + V4L2_M2M_DST = 1,
> +};
> +
> +struct mem2mem_priv {
> + struct imx_media_video_dev vdev;
> +
> + struct v4l2_m2m_dev   *m2m_dev;
> + struct device *dev;
> +
> + struct imx_media_dev  *md;
> +
> + struct mutex  mutex;   /* mem2mem device mutex */
> +
> + atomic_t  num_inst;
> +};
> +
> +#define to_mem2mem_priv(v) container_of(v, struct mem2mem_priv, vdev)
> +
> +/* Per-queue, driver-specific private data */
> +struct mem2mem_q_data {
> + struct v4l2_pix_format  cur_fmt;
> + struct v4l2_rectrect;
> +};
> +
> +struct mem2mem_ctx {
> + struct mem2mem_priv *priv;
> +
> + struct v4l2_fh  fh;
> + struct mem2mem_q_data   q_data[2];
> + int   

[PATCHv3 3/9] v4l2-ioctl.c: log v4l2_buffer tag

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

When debugging is on, log the new tag field of struct v4l2_buffer
as well.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index e384142d2826..653497f31104 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -498,9 +498,12 @@ static void v4l_print_buffer(const void *arg, bool 
write_only)
p->bytesused, p->m.userptr, p->length);
}
 
-   printk(KERN_DEBUG "timecode=%02d:%02d:%02d type=%d, flags=0x%08x, 
frames=%d, userbits=0x%08x\n",
-   tc->hours, tc->minutes, tc->seconds,
-   tc->type, tc->flags, tc->frames, *(__u32 
*)tc->userbits);
+   if (p->flags & V4L2_BUF_FLAG_TAG)
+   printk(KERN_DEBUG "tag=%x\n", p->tag);
+   else if (p->flags & V4L2_BUF_FLAG_TIMECODE)
+   printk(KERN_DEBUG "timecode=%02d:%02d:%02d type=%d, 
flags=0x%08x, frames=%d, userbits=0x%08x\n",
+  tc->hours, tc->minutes, tc->seconds,
+  tc->type, tc->flags, tc->frames, *(__u32 *)tc->userbits);
 }
 
 static void v4l_print_exportbuffer(const void *arg, bool write_only)
-- 
2.19.1



[PATCHv3 0/9] vb2/cedrus: add tag support

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

As was discussed here (among other places):

https://lkml.org/lkml/2018/10/19/440

using capture queue buffer indices to refer to reference frames is
not a good idea. A better idea is to use a 'tag' where the
application can assign a u32 tag to an output buffer, which is then 
copied to the capture buffer(s) derived from the output buffer.

It has been suggested that the timestamp can be used for this. But
there are a number of reasons why this is a bad idea:

1) the struct timeval is converted to a u64 in vb2. So there can be
   all sorts of unexpected conversion issues. In particular, the
   output of ns_to_timeval(timeval_to_ns(tv)) does not necessarily
   match the input.

2) it gets worse with the y2038 code where userspace either deals
   with a 32 bit tv_sec value or a 64 bit value.

In other words, using timestamp for this is not a good idea.

This implementation adds a new tag field in a union with the timecode
field. Right now timecode is not actually used in any driver, except
that m2m drivers are supposed to copy it from the OUTPUT queue to the
CAPTURE queue (it can be set by userspace, so it should be copied by
the driver). An alternative would be to use the reserved2 field in
struct v4l2_buffer for the tag. But then we have no more room for
a fence fd. But if we decide on a struct v4l2_ext_buffer API going
forward, then we might just use that for fences as well and we can
just take the reserved2 field for the tag instead.

I have no strong opinion on this.

The first three patches add core tag support, the next patch document
the tag support, then a new helper function is added to v4l2-mem2mem.c
to easily copy data from a source to a destination buffer that drivers
can use.

Next a new supports_tags vb2_queue flag is added to indicate that
the driver supports tags. Ideally this should not be necessary, but
that would require that all m2m drivers are converted to using the
new helper function introduced in the previous patch. That takes more
time then I have now.

Finally the vim2m, vicodec and cedrus drivers are converted to support
tags.

I also removed the 'pad' fields from the mpeg2 control structs (it
should never been added in the first place) and aligned the structs
to a u32 boundary.

Note that this might change further (Paul suggested using bitfields).

Also note that the cedrus code doesn't set the sequence counter, that's
something that should still be added before this driver can be moved
out of staging.

Note: if no buffer is found for a certain tag, then the dma address
is just set to 0. That happened before as well with invalid buffer
indices. This should be checked in the driver!

Regards,

Hans

Changes since v2:

- rebased
- added Reviewed-by tags
- fixed a few remaining references in the documentation to the old
  v4l2_buffer_tag struct that was used in early versions of this
  series.

Changes since v1:

- changed to a u32 tag. Using a 64 bit tag was overly complicated due
  to the bad layout of the v4l2_buffer struct, and there is no real
  need for it by applications.

Main changes since the RFC:

- Added new buffer capability flag
- Added m2m helper to copy data between buffers
- Added documentation
- Added tag logging in v4l2-ioctl.c

Hans Verkuil (9):
  videodev2.h: add tag support
  vb2: add tag support
  v4l2-ioctl.c: log v4l2_buffer tag
  buffer.rst: document the new buffer tag feature.
  v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
  vb2: add new supports_tags queue flag
  vim2m: add tag support
  vicodec: add tag support
  cedrus: add tag support

 Documentation/media/uapi/v4l/buffer.rst   | 32 +
 .../media/uapi/v4l/vidioc-reqbufs.rst |  4 ++
 .../media/common/videobuf2/videobuf2-v4l2.c   | 45 ---
 drivers/media/platform/vicodec/vicodec-core.c | 14 ++
 drivers/media/platform/vim2m.c| 14 ++
 drivers/media/v4l2-core/v4l2-ctrls.c  |  9 
 drivers/media/v4l2-core/v4l2-ioctl.c  |  9 ++--
 drivers/media/v4l2-core/v4l2-mem2mem.c| 23 ++
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  9 ++--
 .../staging/media/sunxi/cedrus/cedrus_dec.c   |  2 +
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 -
 .../staging/media/sunxi/cedrus/cedrus_video.c |  2 +
 include/media/v4l2-mem2mem.h  | 21 +
 include/media/videobuf2-core.h|  2 +
 include/media/videobuf2-v4l2.h| 21 -
 include/uapi/linux/v4l2-controls.h| 14 +++---
 include/uapi/linux/videodev2.h|  9 +++-
 17 files changed, 178 insertions(+), 73 deletions(-)

-- 
2.19.1



[PATCHv3 1/9] videodev2.h: add tag support

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Add support for 'tags' to struct v4l2_buffer. These can be used
by m2m devices so userspace can set a tag for an output buffer and
this value will then be copied to the capture buffer(s).

This tag can be used to refer to capture buffers, something that
is needed by stateless HW codecs.

The new V4L2_BUF_CAP_SUPPORTS_TAGS capability indicates whether
or not tags are supported.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 include/uapi/linux/videodev2.h | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 2a223835214c..99b2b13a5757 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -880,6 +880,7 @@ struct v4l2_requestbuffers {
 #define V4L2_BUF_CAP_SUPPORTS_DMABUF   (1 << 2)
 #define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3)
 #define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS (1 << 4)
+#define V4L2_BUF_CAP_SUPPORTS_TAGS (1 << 5)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
@@ -924,6 +925,7 @@ struct v4l2_plane {
  * @field: enum v4l2_field; field order of the image in the buffer
  * @timestamp: frame timestamp
  * @timecode:  frame timecode
+ * @tag:   buffer tag
  * @sequence:  sequence count of this frame
  * @memory:enum v4l2_memory; the method, in which the actual video data is
  * passed
@@ -951,7 +953,10 @@ struct v4l2_buffer {
__u32   flags;
__u32   field;
struct timeval  timestamp;
-   struct v4l2_timecodetimecode;
+   union {
+   struct v4l2_timecodetimecode;
+   __u32   tag;
+   };
__u32   sequence;
 
/* memory location */
@@ -989,6 +994,8 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_IN_REQUEST   0x0080
 /* timecode field is valid */
 #define V4L2_BUF_FLAG_TIMECODE 0x0100
+/* tag field is valid */
+#define V4L2_BUF_FLAG_TAG  0x0200
 /* Buffer is prepared for queuing */
 #define V4L2_BUF_FLAG_PREPARED 0x0400
 /* Cache handling flags */
-- 
2.19.1



[PATCHv3 7/9] vim2m: add tag support

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Copy tags in vim2m.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 drivers/media/platform/vim2m.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index d01821a6906a..be328483a53a 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -241,17 +241,7 @@ static int device_process(struct vim2m_ctx *ctx,
out_vb->sequence =
get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE)->sequence++;
in_vb->sequence = q_data->sequence++;
-   out_vb->vb2_buf.timestamp = in_vb->vb2_buf.timestamp;
-
-   if (in_vb->flags & V4L2_BUF_FLAG_TIMECODE)
-   out_vb->timecode = in_vb->timecode;
-   out_vb->field = in_vb->field;
-   out_vb->flags = in_vb->flags &
-   (V4L2_BUF_FLAG_TIMECODE |
-V4L2_BUF_FLAG_KEYFRAME |
-V4L2_BUF_FLAG_PFRAME |
-V4L2_BUF_FLAG_BFRAME |
-V4L2_BUF_FLAG_TSTAMP_SRC_MASK);
+   v4l2_m2m_buf_copy_data(out_vb, in_vb, true);
 
switch (ctx->mode) {
case MEM2MEM_HFLIP | MEM2MEM_VFLIP:
@@ -855,6 +845,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, 
struct vb2_queue *ds
src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
src_vq->lock = &ctx->dev->dev_mutex;
src_vq->supports_requests = true;
+   src_vq->supports_tags = true;
 
ret = vb2_queue_init(src_vq);
if (ret)
@@ -868,6 +859,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, 
struct vb2_queue *ds
dst_vq->mem_ops = &vb2_vmalloc_memops;
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = &ctx->dev->dev_mutex;
+   dst_vq->supports_tags = true;
 
return vb2_queue_init(dst_vq);
 }
-- 
2.19.1



[PATCHv3 5/9] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Memory-to-memory devices should copy various parts of
struct v4l2_buffer from the output buffer to the capture buffer.

Add a helper function that does that to simplify the driver code.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 23 +++
 include/media/v4l2-mem2mem.h   | 21 +
 2 files changed, 44 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 5bbdec55b7d7..a9cb1ac33dc0 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -975,6 +975,29 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx,
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
 
+void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb,
+   struct vb2_v4l2_buffer *cap_vb,
+   bool copy_frame_flags)
+{
+   u32 mask = V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG |
+  V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
+
+   if (copy_frame_flags)
+   mask |= V4L2_BUF_FLAG_KEYFRAME | V4L2_BUF_FLAG_PFRAME |
+   V4L2_BUF_FLAG_BFRAME;
+
+   cap_vb->vb2_buf.timestamp = out_vb->vb2_buf.timestamp;
+
+   if (out_vb->flags & V4L2_BUF_FLAG_TAG)
+   cap_vb->tag = out_vb->tag;
+   if (out_vb->flags & V4L2_BUF_FLAG_TIMECODE)
+   cap_vb->timecode = out_vb->timecode;
+   cap_vb->field = out_vb->field;
+   cap_vb->flags &= ~mask;
+   cap_vb->flags |= out_vb->flags & mask;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_buf_copy_data);
+
 void v4l2_m2m_request_queue(struct media_request *req)
 {
struct media_request_object *obj, *obj_safe;
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 5467264771ec..bb4feb6969d2 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -622,6 +622,27 @@ v4l2_m2m_dst_buf_remove_by_idx(struct v4l2_m2m_ctx 
*m2m_ctx, unsigned int idx)
return v4l2_m2m_buf_remove_by_idx(&m2m_ctx->cap_q_ctx, idx);
 }
 
+/**
+ * v4l2_m2m_buf_copy_data() - copy buffer data from the output buffer to the
+ * capture buffer
+ *
+ * @out_vb: the output buffer that is the source of the data.
+ * @cap_vb: the capture buffer that will receive the data.
+ * @copy_frame_flags: copy the KEY/B/PFRAME flags as well.
+ *
+ * This helper function copies the timestamp, timecode (if the TIMECODE
+ * buffer flag was set), tag (if the TAG buffer flag was set), field
+ * and the TIMECODE, TAG, KEYFRAME, BFRAME, PFRAME and TSTAMP_SRC_MASK
+ * flags from @out_vb to @cap_vb.
+ *
+ * If @copy_frame_flags is false, then the KEYFRAME, BFRAME and PFRAME
+ * flags are not copied. This is typically needed for encoders that
+ * set this bits explicitly.
+ */
+void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb,
+   struct vb2_v4l2_buffer *cap_vb,
+   bool copy_frame_flags);
+
 /* v4l2 request helper */
 
 void v4l2_m2m_request_queue(struct media_request *req);
-- 
2.19.1



[PATCHv3 2/9] vb2: add tag support

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Add support for tags to vb2. Besides just storing and setting
the tag this patch also adds the vb2_find_tag() function that
can be used to find a buffer with the given tag.

This function will only look at DEQUEUED and DONE buffers, i.e.
buffers that are already processed.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 .../media/common/videobuf2/videobuf2-v4l2.c   | 43 ---
 include/media/videobuf2-v4l2.h| 21 -
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c 
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 1244c246d0c4..ecbf4f0755cb 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -50,7 +50,8 @@ module_param(debug, int, 0644);
 V4L2_BUF_FLAG_TIMESTAMP_MASK)
 /* Output buffer flags that should be passed on to the driver */
 #define V4L2_BUFFER_OUT_FLAGS  (V4L2_BUF_FLAG_PFRAME | V4L2_BUF_FLAG_BFRAME | \
-V4L2_BUF_FLAG_KEYFRAME | 
V4L2_BUF_FLAG_TIMECODE)
+V4L2_BUF_FLAG_KEYFRAME | \
+V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG)
 
 /*
  * __verify_planes_array() - verify that the planes array passed in struct
@@ -144,8 +145,11 @@ static void __copy_timestamp(struct vb2_buffer *vb, const 
void *pb)
 */
if (q->copy_timestamp)
vb->timestamp = timeval_to_ns(&b->timestamp);
-   vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE;
-   if (b->flags & V4L2_BUF_FLAG_TIMECODE)
+   vbuf->flags |= b->flags &
+   (V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG);
+   if (b->flags & V4L2_BUF_FLAG_TAG)
+   vbuf->tag = b->tag;
+   else if (b->flags & V4L2_BUF_FLAG_TIMECODE)
vbuf->timecode = b->timecode;
}
 };
@@ -194,6 +198,7 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, 
struct v4l2_buffer *b
}
vbuf->sequence = 0;
vbuf->request_fd = -1;
+   vbuf->tag = 0;
 
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
switch (b->memory) {
@@ -313,13 +318,19 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer 
*vb, struct v4l2_buffer *b
}
 
if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+   if ((b->flags & V4L2_BUF_FLAG_TIMECODE) &&
+   (b->flags & V4L2_BUF_FLAG_TAG)) {
+   dprintk(1, "buffer flag TIMECODE cannot be combined 
with flag TAG\n");
+   return -EINVAL;
+   }
+
/*
 * For output buffers mask out the timecode flag:
 * this will be handled later in vb2_qbuf().
 * The 'field' is valid metadata for this output buffer
 * and so that needs to be copied here.
 */
-   vbuf->flags &= ~V4L2_BUF_FLAG_TIMECODE;
+   vbuf->flags &= ~(V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG);
vbuf->field = b->field;
} else {
/* Zero any output buffer flags as this is a capture buffer */
@@ -460,7 +471,10 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void 
*pb)
b->flags = vbuf->flags;
b->field = vbuf->field;
b->timestamp = ns_to_timeval(vb->timestamp);
-   b->timecode = vbuf->timecode;
+   if (b->flags & V4L2_BUF_FLAG_TAG)
+   b->tag = vbuf->tag;
+   else if (b->flags & V4L2_BUF_FLAG_TIMECODE)
+   b->timecode = vbuf->timecode;
b->sequence = vbuf->sequence;
b->reserved2 = 0;
b->request_fd = 0;
@@ -586,6 +600,25 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
.copy_timestamp = __copy_timestamp,
 };
 
+int vb2_find_tag(const struct vb2_queue *q, u32 tag,
+unsigned int start_idx)
+{
+   unsigned int i;
+
+   for (i = start_idx; i < q->num_buffers; i++) {
+   struct vb2_buffer *vb = q->bufs[i];
+   struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+   if ((vb->state == VB2_BUF_STATE_DEQUEUED ||
+vb->state == VB2_BUF_STATE_DONE) &&
+   (vbuf->flags & V4L2_BUF_FLAG_TAG) &&
+   vbuf->tag == tag)
+   return i;
+   }
+   return -1;
+}
+EXPORT_SYMBOL_GPL(vb2_find_tag);
+
 /*
  * vb2_querybuf() - query video buffer information
  * @q: videobuf queue
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 727855463838..c2a541af6b2c 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -31,8 +31,9 @@
  * @field: field order of the image in the buffer, as defined by
  * &enum v4l2_fi

[PATCHv3 4/9] buffer.rst: document the new buffer tag feature.

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Document V4L2_BUF_FLAG_TAG.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 Documentation/media/uapi/v4l/buffer.rst   | 32 ++-
 .../media/uapi/v4l/vidioc-reqbufs.rst |  4 +++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index 2e266d32470a..3c09a94c5a10 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -220,17 +220,25 @@ struct v4l2_buffer
use ``V4L2_BUF_FLAG_TIMESTAMP_COPY`` the application has to fill
in the timestamp which will be copied by the driver to the capture
stream.
-* - struct :c:type:`v4l2_timecode`
+* - union
+* -
+  - struct :c:type:`v4l2_timecode`
   - ``timecode``
-  -
-  - When ``type`` is ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` and the
-   ``V4L2_BUF_FLAG_TIMECODE`` flag is set in ``flags``, this
+  - When the ``V4L2_BUF_FLAG_TIMECODE`` flag is set in ``flags``, this
structure contains a frame timecode. In
:c:type:`V4L2_FIELD_ALTERNATE ` mode the top and
bottom field contain the same timecode. Timecodes are intended to
help video editing and are typically recorded on video tapes, but
also embedded in compressed formats like MPEG. This field is
independent of the ``timestamp`` and ``sequence`` fields.
+* -
+  - __u32
+  - ``tag``
+  - When the ``V4L2_BUF_FLAG_TAG`` flag is set in ``flags``, this
+   field contains a user-specified tag value.
+
+   It is used by stateless codecs where this tag can be used to
+   refer to buffers that contain reference frames.
 * - __u32
   - ``sequence``
   -
@@ -567,6 +575,14 @@ Buffer Flags
when the ``VIDIOC_DQBUF`` ioctl is called. Applications can set
this bit and the corresponding ``timecode`` structure when
``type`` refers to an output stream.
+* .. _`V4L2-BUF-FLAG-TAG`:
+
+  - ``V4L2_BUF_FLAG_TAG``
+  - 0x0200
+  - The ``tag`` field is valid. Applications can set
+   this bit and the corresponding ``tag`` field. If tags are
+   supported then the ``V4L2_BUF_CAP_SUPPORTS_TAGS`` capability
+   is also set.
 * .. _`V4L2-BUF-FLAG-PREPARED`:
 
   - ``V4L2_BUF_FLAG_PREPARED``
@@ -704,10 +720,10 @@ enum v4l2_memory
 Timecodes
 =
 
-The struct :c:type:`v4l2_timecode` structure is designed to hold a
-:ref:`smpte12m` or similar timecode. (struct
-struct :c:type:`timeval` timestamps are stored in struct
-:c:type:`v4l2_buffer` field ``timestamp``.)
+The :c:type:`v4l2_buffer_timecode` structure is designed to hold a
+:ref:`smpte12m` or similar timecode.
+(struct :c:type:`timeval` timestamps are stored in the struct
+:c:type:`v4l2_buffer` ``timestamp`` field.)
 
 
 .. c:type:: v4l2_timecode
diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst 
b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index e62a15782790..38a7d0aee483 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -118,6 +118,7 @@ aborting or finishing any DMA in progress, an implicit
 .. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
 .. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
 .. _V4L2-BUF-CAP-SUPPORTS-ORPHANED-BUFS:
+.. _V4L2-BUF-CAP-SUPPORTS-TAGS:
 
 .. cssclass:: longtable
 
@@ -143,6 +144,9 @@ aborting or finishing any DMA in progress, an implicit
   - The kernel allows calling :ref:`VIDIOC_REQBUFS` while buffers are still
 mapped or exported via DMABUF. These orphaned buffers will be freed
 when they are unmapped or when the exported DMABUF fds are closed.
+* - ``V4L2_BUF_CAP_SUPPORTS_TAGS``
+  - 0x0020
+  - This buffer type supports ``V4L2_BUF_FLAG_TAG``.
 
 Return Value
 
-- 
2.19.1



[PATCHv3 6/9] vb2: add new supports_tags queue flag

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Add new flag to indicate that buffer tags are supported.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 ++
 include/media/videobuf2-core.h  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c 
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index ecbf4f0755cb..189dd7fb12c0 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -665,6 +665,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
*caps |= V4L2_BUF_CAP_SUPPORTS_DMABUF;
if (q->supports_requests)
*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
+   if (q->supports_tags)
+   *caps |= V4L2_BUF_CAP_SUPPORTS_TAGS;
 }
 
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index e86981d615ae..81f2dbfd0094 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -473,6 +473,7 @@ struct vb2_buf_ops {
  *  has not been called. This is a vb1 idiom that has been adopted
  *  also by vb2.
  * @supports_requests: this queue supports the Request API.
+ * @supports_tags: this queue supports tags in struct v4l2_buffer.
  * @uses_qbuf: qbuf was used directly for this queue. Set to 1 the first
  * time this is called. Set to 0 when the queue is canceled.
  * If this is 1, then you cannot queue buffers from a request.
@@ -547,6 +548,7 @@ struct vb2_queue {
unsignedallow_zero_bytesused:1;
unsigned   quirk_poll_must_check_waiting_for_buffers:1;
unsignedsupports_requests:1;
+   unsignedsupports_tags:1;
unsigneduses_qbuf:1;
unsigneduses_requests:1;
 
-- 
2.19.1



[PATCHv3 8/9] vicodec: add tag support

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Copy tags in vicodec.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 drivers/media/platform/vicodec/vicodec-core.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c 
b/drivers/media/platform/vicodec/vicodec-core.c
index b7bdfe97215b..4d39ea033653 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -190,18 +190,8 @@ static int device_process(struct vicodec_ctx *ctx,
}
 
out_vb->sequence = q_cap->sequence++;
-   out_vb->vb2_buf.timestamp = in_vb->vb2_buf.timestamp;
-
-   if (in_vb->flags & V4L2_BUF_FLAG_TIMECODE)
-   out_vb->timecode = in_vb->timecode;
-   out_vb->field = in_vb->field;
out_vb->flags &= ~V4L2_BUF_FLAG_LAST;
-   out_vb->flags |= in_vb->flags &
-   (V4L2_BUF_FLAG_TIMECODE |
-V4L2_BUF_FLAG_KEYFRAME |
-V4L2_BUF_FLAG_PFRAME |
-V4L2_BUF_FLAG_BFRAME |
-V4L2_BUF_FLAG_TSTAMP_SRC_MASK);
+   v4l2_m2m_buf_copy_data(in_vb, out_vb, !ctx->is_enc);
 
return 0;
 }
@@ -1083,6 +1073,7 @@ static int queue_init(void *priv, struct vb2_queue 
*src_vq,
src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
src_vq->lock = ctx->is_enc ? &ctx->dev->enc_mutex :
&ctx->dev->dec_mutex;
+   src_vq->supports_tags = true;
 
ret = vb2_queue_init(src_vq);
if (ret)
@@ -1098,6 +1089,7 @@ static int queue_init(void *priv, struct vb2_queue 
*src_vq,
dst_vq->mem_ops = &vb2_vmalloc_memops;
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = src_vq->lock;
+   dst_vq->supports_tags = true;
 
return vb2_queue_init(dst_vq);
 }
-- 
2.19.1



[PATCHv3 9/9] cedrus: add tag support

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Replace old reference frame indices by new tag method.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/v4l2-ctrls.c  |  9 
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  9 +---
 .../staging/media/sunxi/cedrus/cedrus_dec.c   |  2 ++
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ---
 .../staging/media/sunxi/cedrus/cedrus_video.c |  2 ++
 include/uapi/linux/v4l2-controls.h| 14 +
 6 files changed, 24 insertions(+), 33 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 5f2b033a7a42..b854cceb19dc 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1660,15 +1660,6 @@ static int std_validate(const struct v4l2_ctrl *ctrl, 
u32 idx,
return -EINVAL;
}
 
-   if (p_mpeg2_slice_params->backward_ref_index >= VIDEO_MAX_FRAME 
||
-   p_mpeg2_slice_params->forward_ref_index >= VIDEO_MAX_FRAME)
-   return -EINVAL;
-
-   if (p_mpeg2_slice_params->pad ||
-   p_mpeg2_slice_params->picture.pad ||
-   p_mpeg2_slice_params->sequence.pad)
-   return -EINVAL;
-
return 0;
 
case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h 
b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 3f61248c57ac..781676b55a1b 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -142,11 +142,14 @@ static inline dma_addr_t cedrus_buf_addr(struct 
vb2_buffer *buf,
 }
 
 static inline dma_addr_t cedrus_dst_buf_addr(struct cedrus_ctx *ctx,
-unsigned int index,
-unsigned int plane)
+int index, unsigned int plane)
 {
-   struct vb2_buffer *buf = ctx->dst_bufs[index];
+   struct vb2_buffer *buf;
 
+   if (index < 0)
+   return 0;
+
+   buf = ctx->dst_bufs[index];
return buf ? cedrus_buf_addr(buf, &ctx->dst_fmt, plane) : 0;
 }
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c 
b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index e40180a33951..0cfd6036d0cd 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -53,6 +53,8 @@ void cedrus_device_run(void *priv)
break;
}
 
+   v4l2_m2m_buf_copy_data(run.src, run.dst, true);
+
dev->dec_ops[ctx->current_codec]->setup(ctx, &run);
 
spin_unlock_irqrestore(&ctx->dev->irq_lock, flags);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c 
b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 9abd39cae38c..fdde9a099153 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -82,7 +82,10 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
struct cedrus_run *run)
dma_addr_t fwd_luma_addr, fwd_chroma_addr;
dma_addr_t bwd_luma_addr, bwd_chroma_addr;
struct cedrus_dev *dev = ctx->dev;
+   struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
const u8 *matrix;
+   int forward_idx;
+   int backward_idx;
unsigned int i;
u32 reg;
 
@@ -156,23 +159,17 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
struct cedrus_run *run)
cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
 
/* Forward and backward prediction reference buffers. */
+   forward_idx = vb2_find_tag(cap_q, slice_params->forward_ref_tag, 0);
 
-   fwd_luma_addr = cedrus_dst_buf_addr(ctx,
-   slice_params->forward_ref_index,
-   0);
-   fwd_chroma_addr = cedrus_dst_buf_addr(ctx,
- slice_params->forward_ref_index,
- 1);
+   fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0);
+   fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1);
 
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr);
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, fwd_chroma_addr);
 
-   bwd_luma_addr = cedrus_dst_buf_addr(ctx,
-   slice_params->backward_ref_index,
-   0);
-   bwd_chroma_addr = cedrus_dst_buf_addr(ctx,
- slice_params->backward_ref_index,
- 1);
+   backward_idx = vb2_find_tag(cap_q, slice_params->backward_ref_tag, 0);
+   bwd_luma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 0);
+   bwd_

Re: [PATCH 1/6] media: v4l2-subdev: stub v4l2_subdev_get_try_format() 

2018-12-03 Thread jacopo mondi
Hi Lubomir,

  thanks for the patches

On Wed, Nov 28, 2018 at 06:19:13PM +0100, Lubomir Rintel wrote:
> Provide a dummy implementation when configured without
> CONFIG_VIDEO_V4L2_SUBDEV_API to avoid ifdef dance in the drivers that can
> be built either with or without the option.
>
> Suggested-by: Jacopo Mondi 
> Signed-off-by: Lubomir Rintel 
> ---
>  include/media/v4l2-subdev.h | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 9102d6ca566e..906e28011bb4 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -967,6 +967,17 @@ static inline struct v4l2_rect
>   pad = 0;
>   return &cfg[pad].try_compose;
>  }
> +
> +#else /* !defined(CONFIG_VIDEO_V4L2_SUBDEV_API) */
> +
> +static inline struct v4l2_mbus_framefmt
> +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + unsigned int pad)
> +{
> + return ERR_PTR(-ENOTTY);
> +}
> +
>  #endif

While at there, what about doing the same for get_try_crop and
get_try_compose? At lease provide stubs, I let you figure out if
you're willing to fix callers too, it seems there are quite a few of
them though

$ git grep v4l2_subdev_get_try* drivers/media/ | grep -v '_format' | wc -l
44

>
>  extern const struct v4l2_file_operations v4l2_subdev_fops;
> --
> 2.19.1
>


signature.asc
Description: PGP signature


Re: [PATCHv2] videodev2.h: add V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF/CREATE_BUFS

2018-12-03 Thread Hans Verkuil
On 11/29/2018 10:30 AM, Hans Verkuil wrote:
> Add new buffer capability flags to indicate if the VIDIOC_PREPARE_BUF or
> VIDIOC_CREATE_BUFS ioctls are supported.
> 
> The reason for this is that there is currently no way for an application
> to detect if VIDIOC_PREPARE_BUF is implemented other than trying it, but
> then the buffer is already prepared. You would like to know this before
> taking an irreversible action.
> 
> Since we need V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF it makes sense to add
> V4L2_BUF_CAP_SUPPORTS_CREATE_BUFS as well because not all drivers support
> this ioctl.
> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Sakari Ailus 
> ---
> Changes since v1:
> 
> - rebased
> - improved commit msg
> - added missing include for media/v4l2-ioctl.h
> ---
>  Documentation/media/uapi/v4l/vidioc-reqbufs.rst |  8 
>  drivers/media/common/videobuf2/videobuf2-v4l2.c | 15 +--
>  include/uapi/linux/videodev2.h  | 12 +++-
>  3 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst 
> b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> index e62a15782790..092d6373380a 100644
> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> @@ -118,6 +118,8 @@ aborting or finishing any DMA in progress, an implicit
>  .. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
>  .. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
>  .. _V4L2-BUF-CAP-SUPPORTS-ORPHANED-BUFS:
> +.. _V4L2-BUF-CAP-SUPPORTS-PREPARE-BUF:
> +.. _V4L2-BUF-CAP-SUPPORTS-CREATE-BUFS:
> 
>  .. cssclass:: longtable
> 
> @@ -143,6 +145,12 @@ aborting or finishing any DMA in progress, an implicit
>- The kernel allows calling :ref:`VIDIOC_REQBUFS` while buffers are 
> still
>  mapped or exported via DMABUF. These orphaned buffers will be freed
>  when they are unmapped or when the exported DMABUF fds are closed.
> +* - ``V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF``
> +  - 0x0020
> +  - This buffer type supports :ref:`VIDIOC_PREPARE_BUF`.
> +* - ``V4L2_BUF_CAP_SUPPORTS_CREATE_BUFS``
> +  - 0x0040
> +  - This buffer type supports :ref:`VIDIOC_CREATE_BUFS`.
> 
>  Return Value
>  
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c 
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 1244c246d0c4..5273f574fb7a 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> 
>  #include 
> @@ -870,6 +871,16 @@ static inline bool vb2_queue_is_busy(struct video_device 
> *vdev, struct file *fil
>   return vdev->queue->owner && vdev->queue->owner != file->private_data;
>  }
> 
> +static void fill_buf_caps_vdev(struct video_device *vdev, u32 *caps)
> +{
> + *caps = 0;
> + fill_buf_caps(vdev->queue, caps);
> + if (vdev->ioctl_ops->vidioc_prepare_buf)
> + *caps |= V4L2_BUF_CAP_SUPPORTS_PREPARE_BUF;
> + if (vdev->ioctl_ops->vidioc_create_bufs)
> + *caps |= V4L2_BUF_CAP_SUPPORTS_CREATE_BUFS;
> +}
> +
>  /* vb2 ioctl helpers */
> 
>  int vb2_ioctl_reqbufs(struct file *file, void *priv,
> @@ -878,7 +889,7 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
>   struct video_device *vdev = video_devdata(file);
>   int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type);
> 
> - fill_buf_caps(vdev->queue, &p->capabilities);
> + fill_buf_caps_vdev(vdev, &p->capabilities);
>   if (res)
>   return res;
>   if (vb2_queue_is_busy(vdev, file))
> @@ -900,7 +911,7 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
>   p->format.type);
> 
>   p->index = vdev->queue->num_buffers;
> - fill_buf_caps(vdev->queue, &p->capabilities);
> + fill_buf_caps_vdev(vdev, &p->capabilities);
>   /*
>* If count == 0, then just check if memory and type are valid.
>* Any -EBUSY result from vb2_verify_memory_type can be mapped to 0.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 2a223835214c..8ebc66e311e0 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -875,11 +875,13 @@ struct v4l2_requestbuffers {
>  };
> 
>  /* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
> -#define V4L2_BUF_CAP_SUPPORTS_MMAP   (1 << 0)
> -#define V4L2_BUF_CAP_SUPPORTS_USERPTR(1 << 1)
> -#define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2)
> -#define V4L2_BUF_CAP_SUPPORTS_REQUESTS   (1 << 3)
> -#define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS (1 << 4)
> +#define V4L2_BUF_CAP_SUPPORTS_MMAP   (1 << 0)
> +#define V4L2_BUF_CAP_SUPPORTS_USERPTR(1 << 1)
> +#define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2)
> +#define V4L2_BUF_CAP_SUPPORTS_REQUESTS   (1 << 3)
> +#define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS  (1 << 4)

[PATCH 3/3] vivid: add buf_validate callback

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Split off the field validation from buf_prepare into a new
buf_validate function. Field validation for output buffers should
be done there since buf_prepare is not guaranteed to be called at
QBUF time.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/vivid/vivid-vid-out.c | 23 ++--
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-vid-out.c 
b/drivers/media/platform/vivid/vivid-vid-out.c
index 7642cbdb0e14..3e93dbbb4ffe 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -81,10 +81,24 @@ static int vid_out_queue_setup(struct vb2_queue *vq,
return 0;
 }
 
-static int vid_out_buf_prepare(struct vb2_buffer *vb)
+static int vid_out_buf_validate(struct vb2_buffer *vb)
 {
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct vivid_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
+
+   dprintk(dev, 1, "%s\n", __func__);
+
+   if (dev->field_out != V4L2_FIELD_ALTERNATE)
+   vbuf->field = dev->field_out;
+   else if (vbuf->field != V4L2_FIELD_TOP &&
+vbuf->field != V4L2_FIELD_BOTTOM)
+   return -EINVAL;
+   return 0;
+}
+
+static int vid_out_buf_prepare(struct vb2_buffer *vb)
+{
+   struct vivid_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
unsigned long size;
unsigned planes;
unsigned p;
@@ -105,12 +119,6 @@ static int vid_out_buf_prepare(struct vb2_buffer *vb)
return -EINVAL;
}
 
-   if (dev->field_out != V4L2_FIELD_ALTERNATE)
-   vbuf->field = dev->field_out;
-   else if (vbuf->field != V4L2_FIELD_TOP &&
-vbuf->field != V4L2_FIELD_BOTTOM)
-   return -EINVAL;
-
for (p = 0; p < planes; p++) {
size = dev->bytesperline_out[p] * dev->fmt_out_rect.height +
vb->planes[p].data_offset;
@@ -190,6 +198,7 @@ static void vid_out_buf_request_complete(struct vb2_buffer 
*vb)
 
 const struct vb2_ops vivid_vid_out_qops = {
.queue_setup= vid_out_queue_setup,
+   .buf_validate   = vid_out_buf_validate,
.buf_prepare= vid_out_buf_prepare,
.buf_queue  = vid_out_buf_queue,
.start_streaming= vid_out_start_streaming,
-- 
2.19.1



[PATCH 1/3] vb2: add buf_validate callback

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Adding the request API uncovered a pre-existing problem with
validating output buffers.

The problem is that for output buffers the driver has to validate
the 'field' field of struct v4l2_buffer. This is critical when
encoding or deinterlacing interlaced video.

Drivers always did this in the buf_prepare callback, but that is
not called from VIDIOC_QBUF in two situations: when queueing a
buffer to a request and if VIDIOC_PREPARE_BUF has been called
earlier for that buffer.

As a result of this the 'field' value is not validated.

This patch adds a new buf_validate callback to validate the
output buffer at QBUF time.

Note that PREPARE_BUF doesn't need to validate this: it just
locks the buffer memory and doesn't need nor want to know about
how this buffer is actually going to be used. It's the QBUF ioctl
that determines this.

This issue was found by v4l2-compliance since it failed to replace
V4L2_FIELD_ANY by a proper field value when testing the vivid video
output in combination with requests.

There never was a test before for the PREPARE_BUF/QBUF case, so even
though this bug has been present for quite some time, it was never
noticed.

Signed-off-by: Hans Verkuil 
---
 drivers/media/common/videobuf2/videobuf2-core.c | 12 +---
 include/media/videobuf2-core.h  |  5 +
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 0ca81d495bda..42eb7716f8a9 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -499,9 +499,9 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned 
int buffers)
pr_info(" buf_init: %u buf_cleanup: %u buf_prepare: 
%u buf_finish: %u\n",
vb->cnt_buf_init, vb->cnt_buf_cleanup,
vb->cnt_buf_prepare, vb->cnt_buf_finish);
-   pr_info(" buf_queue: %u buf_done: %u 
buf_request_complete: %u\n",
-   vb->cnt_buf_queue, vb->cnt_buf_done,
-   vb->cnt_buf_request_complete);
+   pr_info(" buf_validate: %u buf_queue: %u buf_done: 
%u buf_request_complete: %u\n",
+   vb->cnt_buf_validate, vb->cnt_buf_queue,
+   vb->cnt_buf_done, vb->cnt_buf_request_complete);
pr_info(" alloc: %u put: %u prepare: %u finish: %u 
mmap: %u\n",
vb->cnt_mem_alloc, vb->cnt_mem_put,
vb->cnt_mem_prepare, vb->cnt_mem_finish,
@@ -1506,6 +1506,12 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb,
return -EBUSY;
}
 
+   ret = call_vb_qop(vb, buf_validate, vb);
+   if (ret) {
+   dprintk(1, "buffer validation failed\n");
+   return ret;
+   }
+
if (req) {
int ret;
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index e86981d615ae..c9f0f3f4ef9a 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -294,6 +294,7 @@ struct vb2_buffer {
u32 cnt_mem_num_users;
u32 cnt_mem_mmap;
 
+   u32 cnt_buf_validate;
u32 cnt_buf_init;
u32 cnt_buf_prepare;
u32 cnt_buf_finish;
@@ -340,6 +341,9 @@ struct vb2_buffer {
  * @wait_finish:   reacquire all locks released in the previous callback;
  * required to continue operation after sleeping while
  * waiting for a new buffer to arrive.
+ * @buf_validate:  called every time the buffer is queued from userspace;
+ * drivers can use this to validate userspace-provided
+ * information; optional.
  * @buf_init:  called once after allocating a buffer (in MMAP case)
  * or after acquiring a new USERPTR buffer; drivers may
  * perform additional buffer-related initialization;
@@ -407,6 +411,7 @@ struct vb2_ops {
void (*wait_prepare)(struct vb2_queue *q);
void (*wait_finish)(struct vb2_queue *q);
 
+   int (*buf_validate)(struct vb2_buffer *vb);
int (*buf_init)(struct vb2_buffer *vb);
int (*buf_prepare)(struct vb2_buffer *vb);
void (*buf_finish)(struct vb2_buffer *vb);
-- 
2.19.1



[PATCH 2/3] vim2m: add buf_validate callback

2018-12-03 Thread hverkuil-cisco
From: Hans Verkuil 

Split off the field validation from buf_prepare into a new
buf_validate function. Field validation for output buffers should
be done there since buf_prepare is not guaranteed to be called at
QBUF time.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/vim2m.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index d01821a6906a..9559be91daca 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -753,15 +753,13 @@ static int vim2m_queue_setup(struct vb2_queue *vq,
return 0;
 }
 
-static int vim2m_buf_prepare(struct vb2_buffer *vb)
+static int vim2m_buf_validate(struct vb2_buffer *vb)
 {
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct vim2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
-   struct vim2m_q_data *q_data;
 
dprintk(ctx->dev, "type: %d\n", vb->vb2_queue->type);
 
-   q_data = get_q_data(ctx, vb->vb2_queue->type);
if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
if (vbuf->field == V4L2_FIELD_ANY)
vbuf->field = V4L2_FIELD_NONE;
@@ -772,6 +770,17 @@ static int vim2m_buf_prepare(struct vb2_buffer *vb)
}
}
 
+   return 0;
+}
+
+static int vim2m_buf_prepare(struct vb2_buffer *vb)
+{
+   struct vim2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+   struct vim2m_q_data *q_data;
+
+   dprintk(ctx->dev, "type: %d\n", vb->vb2_queue->type);
+
+   q_data = get_q_data(ctx, vb->vb2_queue->type);
if (vb2_plane_size(vb, 0) < q_data->sizeimage) {
dprintk(ctx->dev, "%s data will not fit into plane (%lu < 
%lu)\n",
__func__, vb2_plane_size(vb, 0), 
(long)q_data->sizeimage);
@@ -832,6 +841,7 @@ static void vim2m_buf_request_complete(struct vb2_buffer 
*vb)
 
 static const struct vb2_ops vim2m_qops = {
.queue_setup = vim2m_queue_setup,
+   .buf_validate= vim2m_buf_validate,
.buf_prepare = vim2m_buf_prepare,
.buf_queue   = vim2m_buf_queue,
.start_streaming = vim2m_start_streaming,
-- 
2.19.1



Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset

2018-12-03 Thread Laurent Pinchart
Hi Sakari,

On Monday, 3 December 2018 11:51:01 EET Sakari Ailus wrote:
> On Fri, Nov 30, 2018 at 01:07:53AM +0200, Laurent Pinchart wrote:
> > On Wednesday, 7 November 2018 06:16:47 EET Bing Bu Cao wrote:
> >> On 11/01/2018 08:03 PM, Sakari Ailus wrote:
> >>> On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote:
> > 
> > [snip]
> > 
>  ImgU media topology print:
>  
>  # media-ctl -d /dev/media0 -p
>  Media controller API version 4.19.0
>  
>  Media device information
>  
>  driver  ipu3-imgu
>  model   ipu3-imgu
>  serial
>  bus infoPCI::00:05.0
>  hw revision 0x80862015
>  driver version  4.19.0
>  
>  Device topology
>  - entity 1: ipu3-imgu 0 (5 pads, 5 links)
>  type V4L2 subdev subtype Unknown flags 0
>  device node name /dev/v4l-subdev0
>   pad0: Sink
>   [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown
> >>> 
> >>> This doesn't seem right. Which formats can be enumerated from the pad?
> >>> 
>    crop:(0,0)/1920x1080
>    compose:(0,0)/1920x1080]
> >>> 
> >>> Does the compose rectangle affect the scaling on all outputs?
> >> 
> >> Sakari, driver use crop and compose targets to help set input-feeder and
> >> BDS output resolutions which are 2 key block of whole imaging pipeline,
> >> not the actual ending output, but they will impact the final output.
> >> 
>   <- "ipu3-imgu 0 input":0 []
> >>> 
> >>> Are there links that have no useful link configuration? If so, you
> >>> should set them enabled and immutable in the driver.
> >> 
> >> The enabled status of input pads is used to get which pipe that user is
> >> trying to enable (ipu3_link_setup()), so it could not been set as
> >> immutable.
> > 
> > Each pipe needs an input in order to operate, so from that point of view
> > the input is mandatory. Why can't we make this link immutable, and use
> > the stream state (VIDIOC_STREAMON/VIDIOC_STREAMOFF) to enable/disable the
> > pipes ?
> 
> There are only two options (AFAIK) in choosing the firmware, and by
> configuring the links this is better visible to the user: the links the
> state of which can be changed are not immutable. The driver can also obtain
> the explicit pipeline configuration, which makes the implementation more
> simple.

Do you mean that different firmwares are loaded based on link configuration ? 
Does it also mean that once we start using the first pipeline the 
configuration of the second pipeline can't be changed anymore ? If so, what's 
the reason for such a limitation ?

-- 
Regards,

Laurent Pinchart





Re: [PATCH v9 04/13] media: staging/imx7: add MIPI CSI-2 receiver subdev for i.MX7

2018-12-03 Thread Sakari Ailus
Hi Rui,

On Thu, Nov 22, 2018 at 03:18:25PM +, Rui Miguel Silva wrote:
> Adds MIPI CSI-2 subdev for i.MX7 to connect with sensors with a MIPI
> CSI-2 interface.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  drivers/staging/media/imx/Makefile |1 +
>  drivers/staging/media/imx/imx7-mipi-csis.c | 1135 
>  2 files changed, 1136 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx7-mipi-csis.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index 074f016d3519..d2d909a36239 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
>  
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> new file mode 100644
> index ..56963d0c2043
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -0,0 +1,1135 @@
> +// SPDX-License-Identifier: GPL
> +/*
> + * Freescale i.MX7 SoC series MIPI-CSI V3.3 receiver driver
> + *
> + * Copyright (C) 2018 Linaro Ltd
> + * Copyright (C) 2015-2016 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "imx-media.h"
> +
> +static int debug;
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "Debug level (0-2)");

Could you rely on dynamic debug instead?

> +
> +#define CSIS_DRIVER_NAME "imx7-mipi-csis"
> +#define CSIS_SUBDEV_NAME CSIS_DRIVER_NAME
> +
> +#define CSIS_PAD_SINK0
> +#define CSIS_PAD_SOURCE  1
> +#define CSIS_PADS_NUM2
> +
> +#define MIPI_CSIS_DEF_PIX_WIDTH  640
> +#define MIPI_CSIS_DEF_PIX_HEIGHT 480
> +
> +/* Register map definition */
> +
> +/* CSIS common control */
> +#define MIPI_CSIS_CMN_CTRL   0x04
> +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW BIT(16)
> +#define MIPI_CSIS_CMN_CTRL_INTER_MODEBIT(10)
> +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRLBIT(2)
> +#define MIPI_CSIS_CMN_CTRL_RESET BIT(1)
> +#define MIPI_CSIS_CMN_CTRL_ENABLEBIT(0)
> +
> +#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET8
> +#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK  (3 << 8)
> +
> +/* CSIS clock control */
> +#define MIPI_CSIS_CLK_CTRL   0x08
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)  ((x) << 28)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)  ((x) << 24)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)  ((x) << 20)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)  ((x) << 16)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK(0xf << 4)
> +#define MIPI_CSIS_CLK_CTRL_WCLK_SRC  BIT(0)
> +
> +/* CSIS Interrupt mask */
> +#define MIPI_CSIS_INTMSK 0x10
> +#define MIPI_CSIS_INTMSK_EVEN_BEFORE BIT(31)
> +#define MIPI_CSIS_INTMSK_EVEN_AFTER  BIT(30)
> +#define MIPI_CSIS_INTMSK_ODD_BEFORE  BIT(29)
> +#define MIPI_CSIS_INTMSK_ODD_AFTER   BIT(28)
> +#define MIPI_CSIS_INTMSK_FRAME_START BIT(24)
> +#define MIPI_CSIS_INTMSK_FRAME_END   BIT(20)
> +#define MIPI_CSIS_INTMSK_ERR_SOT_HS  BIT(16)
> +#define MIPI_CSIS_INTMSK_ERR_LOST_FS BIT(12)
> +#define MIPI_CSIS_INTMSK_ERR_LOST_FE BIT(8)
> +#define MIPI_CSIS_INTMSK_ERR_OVERBIT(4)
> +#define MIPI_CSIS_INTMSK_ERR_WRONG_CFG   BIT(3)
> +#define MIPI_CSIS_INTMSK_ERR_ECC BIT(2)
> +#define MIPI_CSIS_INTMSK_ERR_CRC BIT(1)
> +#define MIPI_CSIS_INTMSK_ERR_UNKNOWN BIT(0)
> +
> +/* CSIS Interrupt source */
> +#define MIPI_CSIS_INTSRC 0x14
> +#define MIPI_CSIS_INTSRC_EVEN_BEFORE BIT(31)
> +#define MIPI_CSIS_INTSRC_EVEN_AFTER  BIT(30)
> +#define MIPI_CSIS_INTSRC_EVENBIT(30)
> +#define MIPI_CSIS_INTSRC_ODD_BEFORE  BIT(29)
> +#define MIPI_CSIS_INTSRC_ODD_AFTER   BIT(28)
> +#define MIPI_CSIS_INTSRC_ODD (0x3 << 28)
> +#define MIPI_CSIS_INTSRC_NON_IMAGE_DATA  (0xf << 28)
> +#define MIPI_CSIS_INTSRC_FRAME_START BIT(24)
> +#define MIPI_CSIS_INTSRC_FRAME_END   BIT(20)
> +#define MIPI_CSIS_INTSRC_ERR_SOT_HS  BIT(16)
> +#define MIPI_CSIS_INTSRC_ERR_LOST_FS BIT(12)
> +#define MIPI_CSIS_INTSRC_ERR_LOST_FE BIT(8)
> +#define MIPI_CSIS_INTSRC_ERR_OVERBIT(4)
> +#define MIPI_CSIS_INTSRC_ERR_WRONG_CFG   BIT(3)
> +#define MIPI_CSIS_INTSRC_ERR_ECC BIT(2)
> +#define MIPI_CSIS_INTSRC_ERR_CRC BIT(1)
> +#define MIPI_CSIS_INTSRC_ERR_UNKNOWN BIT(0)
> +#define MIPI_CSIS_INTSRC_ERRORS  0xf
> +
> +/* D-PHY status control */
> +#define MIPI_CSIS_DPHYSTATU

[PATCH] Add support to reset device controls

2018-12-03 Thread Kieran Bingham
Provide a new option '--reset-controls' which will enumerate the
available controls on a device or sub-device, and re-initialise them to
defaults.

Signed-off-by: Kieran Bingham 

---

This 'extends' the video_list_controls() function with an extra bool
flag for 'reset' to prevent duplicating the iteration functionality.

The patch could also pass the same flag into 'video_print_control()'
rather than implementing 'video_reset_control()' which necessitates
calling query_control() a second time.

I have chosen to add the extra call, as I feel it makes the code
cleaner, and pollutes the existing implementation less. The cost of the
extra query, while a little redundant should be minimal and produces a
simple function to reset the control independently.
---
 yavta.c | 41 +++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/yavta.c b/yavta.c
index c7986bd9e031..30022c45ed5b 100644
--- a/yavta.c
+++ b/yavta.c
@@ -1186,7 +1186,28 @@ static int video_print_control(struct device *dev, 
unsigned int id, bool full)
return query.id;
 }
 
-static void video_list_controls(struct device *dev)
+static int video_reset_control(struct device *dev, unsigned int id)
+{
+   struct v4l2_queryctrl query;
+   int ret;
+
+   ret = query_control(dev, id, &query);
+   if (ret < 0)
+   return ret;
+
+   if (query.flags & V4L2_CTRL_FLAG_DISABLED)
+   return query.id;
+
+   if (query.type == V4L2_CTRL_TYPE_CTRL_CLASS)
+   return query.id;
+
+   printf("Reset control 0x%08x to %d:\n", query.id, query.default_value);
+   set_control(dev, query.id, query.default_value);
+
+   return query.id;
+}
+
+static void video_list_controls(struct device *dev, bool reset)
 {
unsigned int nctrls = 0;
unsigned int id;
@@ -1207,6 +1228,12 @@ static void video_list_controls(struct device *dev)
if (ret < 0)
break;
 
+   if (reset) {
+   ret = video_reset_control(dev, id);
+   if (ret < 0)
+   break;
+   }
+
id = ret;
nctrls++;
}
@@ -1837,6 +1864,7 @@ static void usage(const char *argv0)
printf("--offsetUser pointer buffer offset from 
page start\n");
printf("--premultiplied Color components are 
premultiplied by alpha value\n");
printf("--queue-lateQueue buffers after streamon, 
not before\n");
+   printf("--reset-controlsEnumerate available controls 
and reset to defaults\n");
printf("--requeue-last  Requeue the last buffers before 
streamoff\n");
printf("--timestamp-source  Set timestamp source on output 
buffers [eof, soe]\n");
printf("--skip nSkip the first n frames\n");
@@ -1860,6 +1888,7 @@ static void usage(const char *argv0)
 #define OPT_PREMULTIPLIED  269
 #define OPT_QUEUE_LATE 270
 #define OPT_DATA_PREFIX271
+#define OPT_RESET_CONTROLS 272
 
 static struct option opts[] = {
{"buffer-size", 1, 0, OPT_BUFFER_SIZE},
@@ -1888,6 +1917,7 @@ static struct option opts[] = {
{"queue-late", 0, 0, OPT_QUEUE_LATE},
{"get-control", 1, 0, 'r'},
{"requeue-last", 0, 0, OPT_REQUEUE_LAST},
+   {"reset-controls", 0, 0, OPT_RESET_CONTROLS},
{"realtime", 2, 0, 'R'},
{"size", 1, 0, 's'},
{"set-control", 1, 0, 'w'},
@@ -1915,6 +1945,7 @@ int main(int argc, char *argv[])
int do_enum_formats = 0, do_set_format = 0;
int do_enum_inputs = 0, do_set_input = 0;
int do_list_controls = 0, do_get_control = 0, do_set_control = 0;
+   int do_reset_controls = 0;
int do_sleep_forever = 0, do_requeue_last = 0;
int do_rt = 0, do_log_status = 0;
int no_query = 0, do_queue_late = 0;
@@ -2107,6 +2138,9 @@ int main(int argc, char *argv[])
case OPT_QUEUE_LATE:
do_queue_late = 1;
break;
+   case OPT_RESET_CONTROLS:
+   do_reset_controls = 1;
+   break;
case OPT_REQUEUE_LAST:
do_requeue_last = 1;
break;
@@ -2185,7 +2219,10 @@ int main(int argc, char *argv[])
set_control(&dev, ctrl_name, ctrl_value);
 
if (do_list_controls)
-   video_list_controls(&dev);
+   video_list_controls(&dev, false);
+
+   if (do_reset_controls)
+   video_list_controls(&dev, true);
 
if (do_enum_formats) {
printf("- Available formats:\n");
-- 
2.17.1



[PATCH v5] media: imx: add mem2mem device

2018-12-03 Thread Philipp Zabel
Add a single imx-media mem2mem video device that uses the IPU IC PP
(image converter post processing) task for scaling and colorspace
conversion.
On i.MX6Q/DL SoCs with two IPUs currently only the first IPU is used.

The hardware only supports writing to destination buffers up to
1024x1024 pixels in a single pass, arbitrary sizes can be achieved
by rendering multiple tiles per frame.

Signed-off-by: Philipp Zabel 
[slongerb...@gmail.com: use ipu_image_convert_adjust(), fix
 device_run() error handling]
Signed-off-by: Steve Longerbeam 
---
Changes since v4:
 - No functional changes.
 - Dropped deprecated TODO comment. This driver has no interaction with
   the IC task v4l2 subdevices.
 - Dropped ipu-v3 patches, those are merged independently via imx-drm.
---
 drivers/staging/media/imx/Kconfig |   1 +
 drivers/staging/media/imx/Makefile|   1 +
 drivers/staging/media/imx/imx-media-dev.c |  10 +
 drivers/staging/media/imx/imx-media-mem2mem.c | 873 ++
 drivers/staging/media/imx/imx-media.h |  10 +
 5 files changed, 895 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx-media-mem2mem.c

diff --git a/drivers/staging/media/imx/Kconfig 
b/drivers/staging/media/imx/Kconfig
index bfc17de56b17..07013cb3cb66 100644
--- a/drivers/staging/media/imx/Kconfig
+++ b/drivers/staging/media/imx/Kconfig
@@ -6,6 +6,7 @@ config VIDEO_IMX_MEDIA
depends on HAS_DMA
select VIDEOBUF2_DMA_CONTIG
select V4L2_FWNODE
+   select V4L2_MEM2MEM_DEV
---help---
  Say yes here to enable support for video4linux media controller
  driver for the i.MX5/6 SOC.
diff --git a/drivers/staging/media/imx/Makefile 
b/drivers/staging/media/imx/Makefile
index 698a4210316e..f2e722d0fa19 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -6,6 +6,7 @@ imx-media-ic-objs := imx-ic-common.o imx-ic-prp.o 
imx-ic-prpencvf.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-common.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-capture.o
+obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-mem2mem.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-vdic.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o
 
diff --git a/drivers/staging/media/imx/imx-media-dev.c 
b/drivers/staging/media/imx/imx-media-dev.c
index 4b344a4a3706..0376b52cb784 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -318,6 +318,16 @@ static int imx_media_probe_complete(struct 
v4l2_async_notifier *notifier)
goto unlock;
 
ret = v4l2_device_register_subdev_nodes(&imxmd->v4l2_dev);
+   if (ret)
+   goto unlock;
+
+   imxmd->m2m_vdev = imx_media_mem2mem_device_init(imxmd);
+   if (IS_ERR(imxmd->m2m_vdev)) {
+   ret = PTR_ERR(imxmd->m2m_vdev);
+   goto unlock;
+   }
+
+   ret = imx_media_mem2mem_device_register(imxmd->m2m_vdev);
 unlock:
mutex_unlock(&imxmd->mutex);
if (ret)
diff --git a/drivers/staging/media/imx/imx-media-mem2mem.c 
b/drivers/staging/media/imx/imx-media-mem2mem.c
new file mode 100644
index ..a2a4dca017ce
--- /dev/null
+++ b/drivers/staging/media/imx/imx-media-mem2mem.c
@@ -0,0 +1,873 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * i.MX IPUv3 mem2mem Scaler/CSC driver
+ *
+ * Copyright (C) 2011 Pengutronix, Sascha Hauer
+ * Copyright (C) 2018 Pengutronix, Philipp Zabel
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "imx-media.h"
+
+#define fh_to_ctx(__fh)container_of(__fh, struct mem2mem_ctx, fh)
+
+enum {
+   V4L2_M2M_SRC = 0,
+   V4L2_M2M_DST = 1,
+};
+
+struct mem2mem_priv {
+   struct imx_media_video_dev vdev;
+
+   struct v4l2_m2m_dev   *m2m_dev;
+   struct device *dev;
+
+   struct imx_media_dev  *md;
+
+   struct mutex  mutex;   /* mem2mem device mutex */
+
+   atomic_t  num_inst;
+};
+
+#define to_mem2mem_priv(v) container_of(v, struct mem2mem_priv, vdev)
+
+/* Per-queue, driver-specific private data */
+struct mem2mem_q_data {
+   struct v4l2_pix_format  cur_fmt;
+   struct v4l2_rectrect;
+};
+
+struct mem2mem_ctx {
+   struct mem2mem_priv *priv;
+
+   struct v4l2_fh  fh;
+   struct mem2mem_q_data   q_data[2];
+   int error;
+   struct ipu_image_convert_ctx *icc;
+
+   struct v4l2_ctrl_handler ctrl_hdlr;
+   int rotate;
+   bool hflip;
+   bool vflip;
+   enum ipu_rotate_moderot_mode;
+};
+
+static struct mem2mem_q_data *get_q_data(struct mem2mem_ctx *ctx,
+enum v4l2_buf_type type)
+{
+   if (V4L2_TYPE_IS_OUTPUT(type))
+   return &ctx->q_data[V4L2_M2M_SRC];
+   else
+   retur

[PATCH] [media] marvell-ccic: trivial fix to the datasheet URL

2018-12-03 Thread Lubomir Rintel
Signed-off-by: Lubomir Rintel 
---
 drivers/media/platform/marvell-ccic/cafe-driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/marvell-ccic/cafe-driver.c 
b/drivers/media/platform/marvell-ccic/cafe-driver.c
index 2986cb4b88d0..8d00d9d8adff 100644
--- a/drivers/media/platform/marvell-ccic/cafe-driver.c
+++ b/drivers/media/platform/marvell-ccic/cafe-driver.c
@@ -4,7 +4,7 @@
  * sensor.
  *
  * The data sheet for this device can be found at:
- *http://www.marvell.com/products/pc_connectivity/88alp01/
+ *http://wiki.laptop.org/images/5/5c/88ALP01_Datasheet_July_2007.pdf
  *
  * Copyright 2006-11 One Laptop Per Child Association, Inc.
  * Copyright 2006-11 Jonathan Corbet 
-- 
2.19.1



Re: v4l controls API

2018-12-03 Thread Sebastian Süsens
Ok, nevertheless thank your for your reply.



   Sebastian Süsens   Tel.   +49 4321 559 56-27
   mycable GmbH   Fax+49 4321 559 56-10
   Gartenstrasse 10
   24534 Neumuenster, Germany Email  s...@mycable.de

   mycable GmbH, Managing Director: Michael Carstens-Behrens
   USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM

   This e-mail and any files transmitted with it are confidential and
   intended solely for the use of the individual or entity to whom
   they are addressed. If you have received this e-mail in error,
   please notify the sender and delete all copies from your system.


- Ursprüngliche Mail -
Von: "Hans Verkuil" 
An: "Sebastian Süsens" 
CC: "linux-media" 
Gesendet: Montag, 3. Dezember 2018 11:06:29
Betreff: Re: v4l controls API

On 12/03/2018 11:03 AM, Sebastian Süsens wrote:
> Hey,
> 
> I use the driver mx6s_capture kernel 4.9.88 
> On the device tree it is registered with following name "fsl,imx6s-csi".

Ah, that's probably the freescale driver. We don't support that. It's known
to be quite buggy.

Sorry, you're on your own here.

Regards,

Hans

> 
> Hint:
> I have no sub-devices on my systems only /dev/video0
> 
> 
>Sebastian Süsens   Tel.   +49 4321 559 56-27
>mycable GmbH   Fax+49 4321 559 56-10
>Gartenstrasse 10
>24534 Neumuenster, Germany Email  s...@mycable.de
> 
>mycable GmbH, Managing Director: Michael Carstens-Behrens
>USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM
> 
>This e-mail and any files transmitted with it are confidential and
>intended solely for the use of the individual or entity to whom
>they are addressed. If you have received this e-mail in error,
>please notify the sender and delete all copies from your system.
> 
> 
> - Ursprüngliche Mail -
> Von: "Hans Verkuil" 
> An: "Sebastian Süsens" , "linux-media" 
> 
> Gesendet: Montag, 3. Dezember 2018 09:29:14
> Betreff: Re: v4l controls API
> 
> On 12/03/2018 09:02 AM, Sebastian Süsens wrote:
>> Hello,
>>
>> I don't know how to get access to the v4l controls on a I2C camera sensor.
>>
>> My driver structure looks following:
>>
>> bridge driver-> csi-driver   
>>-> sensor driver (includes controls)
>> register-async-notifer for csi driverregister-async-notifer for 
>> sensor driver
>> register video device
>>
>> The v4l2 API say:
>> When a sub-device is registered with a V4L2 driver by calling 
>> v4l2_device_register_subdev() and the ctrl_handler fields of both 
>> v4l2_subdev and v4l2_device are set, then the controls of the subdev will 
>> become automatically available in the V4L2 driver as well. If the subdev 
>> driver contains controls that already exist in the V4L2 driver, then those 
>> will be skipped (so a V4L2 driver can always override a subdev control).
>>
>> But how can I get access to the controls by asynchronous registration, 
>> because the controls are not added to the video device automatically?
> 
> Yes, they are via v4l2_device_register_subdev(), which is called by the async 
> code
> when the subdev driver arrives.
> 
> Note that this assumes that the bridge driver has a control handler that 
> struct
> v4l2_device points to (the ctrl_handler field).
> 
> Also note that certain types of drivers (media controller-based) such as the 
> imx
> driver do not 'inherit' controls since each subdev has its own v4l-subdevX 
> device node
> through which its controls can be set. You do not mention which bridge driver 
> you are
> using, so I can't tell whether or not it falls in this category.
> 
> Regards,
> 
>   Hans
> 
>>
>> Normally I can use:
>>
>> v4l2-ctl -l -d /dev/video0
>>
>> I don't know if this forum is the right place for this question, so please 
>> answer with a private e-mail s...@mycable.de
>>
>> 
>>Sebastian Süsens   Tel.   +49 4321 559 56-27
>>mycable GmbH   Fax+49 4321 559 56-10
>>Gartenstrasse 10
>>24534 Neumuenster, Germany Email  s...@mycable.de
>> 
>>mycable GmbH, Managing Director: Michael Carstens-Behrens
>>USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM
>> ---

Re: [PATCH v6 2/2] media: platform: Add Aspeed Video Engine driver

2018-12-03 Thread Hans Verkuil
On 11/27/2018 08:37 PM, Eddie James wrote:
> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
> can capture and compress video data from digital or analog sources. With
> the Aspeed chip acting a service processor, the Video Engine can capture
> the host processor graphics output.
> 
> Add a V4L2 driver to capture video data and compress it to JPEG images.
> Make the video frames available through the V4L2 streaming interface.
> 
> Signed-off-by: Eddie James 
> ---
>  MAINTAINERS   |8 +
>  drivers/media/platform/Kconfig|9 +
>  drivers/media/platform/Makefile   |1 +
>  drivers/media/platform/aspeed-video.c | 1719 
> +
>  4 files changed, 1737 insertions(+)
>  create mode 100644 drivers/media/platform/aspeed-video.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 602142c..51f513f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2423,6 +2423,14 @@ S: Maintained
>  F:   Documentation/hwmon/asc7621
>  F:   drivers/hwmon/asc7621.c
>  
> +ASPEED VIDEO ENGINE DRIVER
> +M:   Eddie James 
> +L:   linux-media@vger.kernel.org
> +L:   open...@lists.ozlabs.org (moderated for non-subscribers)
> +S:   Maintained
> +F:   drivers/media/platform/aspeed-video.c
> +F:   Documentation/devicetree/bindings/media/aspeed-video.txt
> +
>  ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
>  M:   Corentin Chary 
>  L:   acpi4asus-u...@lists.sourceforge.net
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index ea33063..a505e9f 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -32,6 +32,15 @@ source "drivers/media/platform/davinci/Kconfig"
>  
>  source "drivers/media/platform/omap/Kconfig"
>  
> +config VIDEO_ASPEED
> + tristate "Aspeed AST2400 and AST2500 Video Engine driver"
> + depends on VIDEO_V4L2
> + select VIDEOBUF2_DMA_CONTIG
> + help
> +   Support for the Aspeed Video Engine (VE) embedded in the Aspeed
> +   AST2400 and AST2500 SOCs. The VE can capture and compress video data
> +   from digital or analog sources.
> +
>  config VIDEO_SH_VOU
>   tristate "SuperH VOU video output driver"
>   depends on MEDIA_CAMERA_SUPPORT
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index d347a55..e6deb25 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for the video capture/playback device drivers.
>  #
>  
> +obj-$(CONFIG_VIDEO_ASPEED)   += aspeed-video.o
>  obj-$(CONFIG_VIDEO_CADENCE)  += cadence/
>  obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o
>  obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/
> diff --git a/drivers/media/platform/aspeed-video.c 
> b/drivers/media/platform/aspeed-video.c
> new file mode 100644
> index 000..200f4d82
> --- /dev/null
> +++ b/drivers/media/platform/aspeed-video.c
> @@ -0,0 +1,1719 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DEVICE_NAME  "aspeed-video"
> +
> +#define ASPEED_VIDEO_JPEG_NUM_QUALITIES  12
> +#define ASPEED_VIDEO_JPEG_HEADER_SIZE10
> +#define ASPEED_VIDEO_JPEG_QUANT_SIZE 116
> +#define ASPEED_VIDEO_JPEG_DCT_SIZE   34
> +
> +#define MAX_FRAME_RATE   60
> +#define MAX_HEIGHT   1200
> +#define MAX_WIDTH1920
> +#define MIN_HEIGHT   480
> +#define MIN_WIDTH640
> +
> +#define NUM_POLARITY_CHECKS  10
> +#define INVALID_RESOLUTION_RETRIES   2
> +#define INVALID_RESOLUTION_DELAY msecs_to_jiffies(250)
> +#define RESOLUTION_CHANGE_DELAY  msecs_to_jiffies(500)
> +#define MODE_DETECT_TIMEOUT  msecs_to_jiffies(500)
> +#define STOP_TIMEOUT msecs_to_jiffies(250)
> +#define DIRECT_FETCH_THRESHOLD   0x0c /* 1024 * 768 */
> +
> +#define VE_MAX_SRC_BUFFER_SIZE   0x8ca000 /* 1920 * 1200, 32bpp 
> */
> +#define VE_JPEG_HEADER_SIZE  0x006000 /* 512 * 12 * 4 */
> +
> +#define VE_PROTECTION_KEY0x000
> +#define  VE_PROTECTION_KEY_UNLOCK0x1a038aa8
> +
> +#define VE_SEQ_CTRL  0x004
> +#define  VE_SEQ_CTRL_TRIG_MODE_DET   BIT(0)
> +#define  VE_SEQ_CTRL_TRIG_CAPTUREBIT(1)
> +#define  VE_SEQ_CTRL_FORCE_IDLE  BIT(2)
> +#define  VE_SEQ_CTRL_MULT_FRAME  BIT(3)
> +#define  VE_SEQ_CTRL_TRIG_COMP   BIT(4)
> +#define  VE_SEQ_CTRL_AUTO_COMP   BIT(5)
> +#define  VE_SEQ_CTRL_EN_WATCHDOG BIT(7)
> +#define  VE_SEQ_CTRL_YUV420  BI

Re: [PATCH 4/6] media: ov2680: get rid of extra ifdefs

2018-12-03 Thread Dan Carpenter
Hi Lubomir,

url:
https://github.com/0day-ci/linux/commits/Lubomir-Rintel/media-don-t-ifdef-v4l2_subdev_get_try_format-any-more/20181129-205631
base:   git://linuxtv.org/media_tree.git master

smatch warnings:
drivers/media/i2c/ov2680.c:687 ov2680_get_fmt() warn: inconsistent returns 
'mutex:&sensor->lock'.
  Locked on:   line 677
  Unlocked on: line 670

# 
https://github.com/0day-ci/linux/commit/45699a2f04294ea9ca96a3d178232ecae7f607ed
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 45699a2f04294ea9ca96a3d178232ecae7f607ed
vim +687 drivers/media/i2c/ov2680.c

3ee47cad Rui Miguel Silva 2018-07-03  660  
3ee47cad Rui Miguel Silva 2018-07-03  661  static int ov2680_get_fmt(struct 
v4l2_subdev *sd,
3ee47cad Rui Miguel Silva 2018-07-03  662 struct 
v4l2_subdev_pad_config *cfg,
3ee47cad Rui Miguel Silva 2018-07-03  663 struct 
v4l2_subdev_format *format)
3ee47cad Rui Miguel Silva 2018-07-03  664  {
3ee47cad Rui Miguel Silva 2018-07-03  665   struct ov2680_dev *sensor = 
to_ov2680_dev(sd);
3ee47cad Rui Miguel Silva 2018-07-03  666   struct v4l2_mbus_framefmt *fmt 
= NULL;
3ee47cad Rui Miguel Silva 2018-07-03  667   int ret = 0;
3ee47cad Rui Miguel Silva 2018-07-03  668  
3ee47cad Rui Miguel Silva 2018-07-03  669   if (format->pad != 0)
3ee47cad Rui Miguel Silva 2018-07-03  670   return -EINVAL;
3ee47cad Rui Miguel Silva 2018-07-03  671  
3ee47cad Rui Miguel Silva 2018-07-03  672   mutex_lock(&sensor->lock);
3ee47cad Rui Miguel Silva 2018-07-03  673  
3ee47cad Rui Miguel Silva 2018-07-03  674   if (format->which == 
V4L2_SUBDEV_FORMAT_TRY) {
3ee47cad Rui Miguel Silva 2018-07-03  675   fmt = 
v4l2_subdev_get_try_format(&sensor->sd, cfg, format->pad);
45699a2f Lubomir Rintel   2018-11-28  676   if (IS_ERR(fmt))
45699a2f Lubomir Rintel   2018-11-28  677   return 
PTR_ERR(fmt);

^^^
goto unlock;

3ee47cad Rui Miguel Silva 2018-07-03  678   } else {
3ee47cad Rui Miguel Silva 2018-07-03  679   fmt = &sensor->fmt;
3ee47cad Rui Miguel Silva 2018-07-03  680   }
3ee47cad Rui Miguel Silva 2018-07-03  681  
3ee47cad Rui Miguel Silva 2018-07-03  682   if (fmt)
3ee47cad Rui Miguel Silva 2018-07-03  683   format->format = *fmt;
3ee47cad Rui Miguel Silva 2018-07-03  684  
3ee47cad Rui Miguel Silva 2018-07-03  685   mutex_unlock(&sensor->lock);
3ee47cad Rui Miguel Silva 2018-07-03  686  
3ee47cad Rui Miguel Silva 2018-07-03 @687   return ret;
3ee47cad Rui Miguel Silva 2018-07-03  688  }
3ee47cad Rui Miguel Silva 2018-07-03  689  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


Re: [PATCH 3/6] media: ov2659: get rid of extra ifdefs

2018-12-03 Thread Dan Carpenter
Hi Lubomir,

url:
https://github.com/0day-ci/linux/commits/Lubomir-Rintel/media-don-t-ifdef-v4l2_subdev_get_try_format-any-more/20181129-205631
base:   git://linuxtv.org/media_tree.git master

smatch warnings:
drivers/media/i2c/ov2659.c:1157 ov2659_set_fmt() warn: inconsistent returns 
'mutex:&ov2659->lock'.
  Locked on:   line 1129
  Unlocked on: line 1119

# 
https://github.com/0day-ci/linux/commit/ceed6707bbb8d34fa04448a9eaf77a574dae59a8
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout ceed6707bbb8d34fa04448a9eaf77a574dae59a8
vim +1157 drivers/media/i2c/ov2659.c

c4c0283a Benoit Parrot  2015-03-20  1098  
c4c0283a Benoit Parrot  2015-03-20  1099  static int ov2659_set_fmt(struct 
v4l2_subdev *sd,
c4c0283a Benoit Parrot  2015-03-20  1100  struct 
v4l2_subdev_pad_config *cfg,
c4c0283a Benoit Parrot  2015-03-20  1101  struct 
v4l2_subdev_format *fmt)
c4c0283a Benoit Parrot  2015-03-20  1102  {
c4c0283a Benoit Parrot  2015-03-20  1103struct i2c_client *client = 
v4l2_get_subdevdata(sd);
5f5859d1 Dan Carpenter  2015-04-15  1104int index = 
ARRAY_SIZE(ov2659_formats);
c4c0283a Benoit Parrot  2015-03-20  1105struct v4l2_mbus_framefmt *mf = 
&fmt->format;
c4c0283a Benoit Parrot  2015-03-20  1106const struct ov2659_framesize 
*size = NULL;
c4c0283a Benoit Parrot  2015-03-20  1107struct ov2659 *ov2659 = 
to_ov2659(sd);
c4c0283a Benoit Parrot  2015-03-20  1108int ret = 0;
c4c0283a Benoit Parrot  2015-03-20  1109  
c4c0283a Benoit Parrot  2015-03-20  1110dev_dbg(&client->dev, 
"ov2659_set_fmt\n");
c4c0283a Benoit Parrot  2015-03-20    
c4c0283a Benoit Parrot  2015-03-20  1112__ov2659_try_frame_size(mf, 
&size);
c4c0283a Benoit Parrot  2015-03-20  1113  
c4c0283a Benoit Parrot  2015-03-20  1114while (--index >= 0)
c4c0283a Benoit Parrot  2015-03-20  1115if 
(ov2659_formats[index].code == mf->code)
c4c0283a Benoit Parrot  2015-03-20  1116break;
c4c0283a Benoit Parrot  2015-03-20  1117  
c4c0283a Benoit Parrot  2015-03-20  1118if (index < 0)
c4c0283a Benoit Parrot  2015-03-20  1119return -EINVAL;
c4c0283a Benoit Parrot  2015-03-20  1120  
c4c0283a Benoit Parrot  2015-03-20  1121mf->colorspace = 
V4L2_COLORSPACE_SRGB;
c4c0283a Benoit Parrot  2015-03-20  1122mf->field = V4L2_FIELD_NONE;
c4c0283a Benoit Parrot  2015-03-20  1123  
c4c0283a Benoit Parrot  2015-03-20  1124mutex_lock(&ov2659->lock);
c4c0283a Benoit Parrot  2015-03-20  1125  
c4c0283a Benoit Parrot  2015-03-20  1126if (fmt->which == 
V4L2_SUBDEV_FORMAT_TRY) {
c4c0283a Benoit Parrot  2015-03-20  1127mf = 
v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
ceed6707 Lubomir Rintel 2018-11-28  1128if (IS_ERR(mf))
ceed6707 Lubomir Rintel 2018-11-28  1129return 
PTR_ERR(mf);

^^
goto unlock;

c4c0283a Benoit Parrot  2015-03-20  1130*mf = fmt->format;
c4c0283a Benoit Parrot  2015-03-20  1131} else {
c4c0283a Benoit Parrot  2015-03-20  1132s64 val;
c4c0283a Benoit Parrot  2015-03-20  1133  
c4c0283a Benoit Parrot  2015-03-20  1134if (ov2659->streaming) {
c4c0283a Benoit Parrot  2015-03-20  1135
mutex_unlock(&ov2659->lock);
c4c0283a Benoit Parrot  2015-03-20  1136return -EBUSY;
c4c0283a Benoit Parrot  2015-03-20  1137}
c4c0283a Benoit Parrot  2015-03-20  1138  
c4c0283a Benoit Parrot  2015-03-20  1139ov2659->frame_size = 
size;
c4c0283a Benoit Parrot  2015-03-20  1140ov2659->format = 
fmt->format;
c4c0283a Benoit Parrot  2015-03-20  1141
ov2659->format_ctrl_regs =
c4c0283a Benoit Parrot  2015-03-20  1142
ov2659_formats[index].format_ctrl_regs;
c4c0283a Benoit Parrot  2015-03-20  1143  
c4c0283a Benoit Parrot  2015-03-20  1144if (ov2659->format.code 
!= MEDIA_BUS_FMT_SBGGR8_1X8)
c4c0283a Benoit Parrot  2015-03-20  1145val = 
ov2659->pdata->link_frequency / 2;
c4c0283a Benoit Parrot  2015-03-20  1146else
c4c0283a Benoit Parrot  2015-03-20  1147val = 
ov2659->pdata->link_frequency;
c4c0283a Benoit Parrot  2015-03-20  1148  
c4c0283a Benoit Parrot  2015-03-20  1149ret = 
v4l2_ctrl_s_ctrl_int64(ov2659->link_frequency, val);
c4c0283a Benoit Parrot  2015-03-20  1150if (ret < 0)
c4c0283a Benoit Parrot  2015-03-20  1151
dev_warn(&client->dev,
c4c0283a Benoit Parrot  2015-03-20  1152 
"failed to set link_frequency rate (%d)\n",
c4c0283a Benoit Parrot  2015-03-20  1153 ret);
c4c0283a Benoit

Re: [PATCH] media: unify some sony camera sensors pattern naming

2018-12-03 Thread Sakari Ailus
Hi Bing Bu, Tomasz,

On Mon, Dec 03, 2018 at 10:53:34AM +0800, Bingbu Cao wrote:
> 
> 
> On 12/01/2018 02:08 AM, Tomasz Figa wrote:
> > Hi Bingbu,
> > 
> > On Mon, Nov 26, 2018 at 7:56 PM  wrote:
> > > From: Bingbu Cao 
> > > 
> > > Some Sony camera sensors have same test pattern
> > > definitions, this patch unify the pattern naming
> > > to make it more clear to the userspace.
> > > 
> > > Suggested-by: Sakari Ailus 
> > > Signed-off-by: Bingbu Cao 
> > > ---
> > >   drivers/media/i2c/imx258.c | 8 
> > >   drivers/media/i2c/imx319.c | 8 
> > >   drivers/media/i2c/imx355.c | 8 
> > >   3 files changed, 12 insertions(+), 12 deletions(-)
> > > 
> > Thanks for the patch! One comment inline.
> > 
> > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> > > index 31a1e2294843..a8a2880c6b4e 100644
> > > --- a/drivers/media/i2c/imx258.c
> > > +++ b/drivers/media/i2c/imx258.c
> > > @@ -504,10 +504,10 @@ struct imx258_mode {
> > > 
> > >   static const char * const imx258_test_pattern_menu[] = {
> > >  "Disabled",
> > > -   "Color Bars",
> > > -   "Solid Color",
> > > -   "Grey Color Bars",
> > > -   "PN9"
> > > +   "Solid Colour",
> > > +   "Eight Vertical Colour Bars",
> > Is it just me or "solid color" and "color bars" are being swapped
> > here? Did the driver had the names mixed up before or the order of
> > modes is different between these sensors?
> The test pattern value order of the 3 camera sensors should be same.
> All are:
> 0 - Disabled
> 1 - Solid Colour
> 2 - Eight Vertical Colour Bars
> ...
> 
> This patch swapped the first 2 item for imx258 (wrong order before) and use 
> unified
> name for all 3 sensors.

I guess this isn't based on Jason's patch (now merged) that fixed the
issue. I'll rebase this; it's trivial.

commit 53f6f81da7db96557fe2bff9b15bd6b83d301f9f
Author: Chen, JasonX Z 
Date:   Wed Nov 7 21:47:34 2018 -0500

media: imx258: remove test pattern map from driver

change bayer order when using test pattern mode.
remove test pattern mapping method

[Sakari Ailus: Drop extra added newline]

Signed-off-by: Chen, JasonX Z 
Signed-off-by: Sakari Ailus 
Signed-off-by: Mauro Carvalho Chehab 

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH 5/5] arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

2018-12-03 Thread Chen-Yu Tsai
On Mon, Dec 3, 2018 at 6:08 PM Jagan Teki  wrote:
>
> Amarula A64-Relic board by default bound with OV5640 camera,
> so add support for it with below pin information.
>
> - PE13, PE12 via i2c-gpio bitbanging
> - CLK_CSI_MCLK as external clock
> - PE1 as external clock pin muxing
> - DLDO3 as vcc-csi supply
> - DLDO3 as AVDD supply
> - ALDO1 as DOVDD supply
> - ELDO3 as DVDD supply
> - PE14 gpio for reset pin
> - PE15 gpio for powerdown pin
>
> Signed-off-by: Jagan Teki 
> ---
>  .../allwinner/sun50i-a64-amarula-relic.dts| 54 +++
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  5 ++
>  2 files changed, 59 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts 
> b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> index 6cb2b7f0c817..9ac6d773188b 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> @@ -22,6 +22,41 @@
> stdout-path = "serial0:115200n8";
> };
>
> +   i2c-csi {
> +   compatible = "i2c-gpio";
> +   sda-gpios = <&pio 4 13 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +   scl-gpios = <&pio 4 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

FYI our hardware doesn't do open drain.

> +   i2c-gpio,delay-us = <5>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   ov5640: camera@3c {
> +   compatible = "ovti,ov5640";
> +   reg = <0x3c>;
> +   pinctrl-names = "default";
> +   pinctrl-0 = <&csi_mclk_pin>;
> +   clocks = <&ccu CLK_CSI_MCLK>;
> +   clock-names = "xclk";
> +
> +   AVDD-supply = <®_dldo3>;
> +   DOVDD-supply = <®_aldo1>;

DOVDD is the supply for I/O. You say it is ALDO1 here.

> +   DVDD-supply = <®_eldo3>;
> +   reset-gpios = <&pio 4 14 GPIO_ACTIVE_LOW>; /* 
> CSI-RST-R: PE14 */
> +   powerdown-gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* 
> CSI-STBY-R: PE15 */
> +
> +   port {
> +   ov5640_ep: endpoint {
> +   remote-endpoint = <&csi_ep>;
> +   bus-width = <8>;
> +   hsync-active = <1>; /* Active high */
> +   vsync-active = <0>; /* Active low */
> +   data-active = <1>;  /* Active high */
> +   pclk-sample = <1>;  /* Rising */
> +   };
> +   };
> +   };
> +   };
> +
> wifi_pwrseq: wifi-pwrseq {
> compatible = "mmc-pwrseq-simple";
> clocks = <&rtc 1>;
> @@ -30,6 +65,25 @@
> };
>  };
>
> +&csi {
> +   vcc-csi-supply = <®_dldo3>;

But here you say the SoC-side pins are driven from DLDO3.

This is a somewhat odd mismatch.

Regardless, the ov5640 driver enables all three regulators at probe time.
Shouldn't that be enough to get the I2C bus working? The pin voltage
supply does not belong here.

> +   status = "okay";
> +
> +   port {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   csi_ep: endpoint {
> +   remote-endpoint = <&ov5640_ep>;
> +   bus-width = <8>;
> +   hsync-active = <1>; /* Active high */
> +   vsync-active = <0>; /* Active low */
> +   data-active = <1>;  /* Active high */
> +   pclk-sample = <1>;  /* Rising */
> +   };
> +   };
> +};
> +
>  &ehci0 {
> status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index d32ff694ac5c..844bb44a78af 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -538,6 +538,11 @@
> function = "csi0";
> };
>
> +   csi_mclk_pin: csi-mclk {
> +   pins = "PE1";
> +   function = "csi0";
> +   };
> +

This should be a separate patch.

ChenYu

> i2c0_pins: i2c0_pins {
> pins = "PH0", "PH1";
> function = "i2c0";
> --
> 2.18.0.321.gffc6fa0e3
>


Re: [PATCH v6 0/2] media: platform: Add Aspeed Video Engine driver

2018-12-03 Thread Hans Verkuil
On 11/27/2018 08:37 PM, Eddie James wrote:
> The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
> can capture and compress video data from digital or analog sources. With
> the Aspeed chip acting as a service processor, the Video Engine can
> capture the host processor graphics output.
> 
> This series adds a V4L2 driver for the VE, providing the usual V4L2 streaming
> interface by way of videobuf2. Each frame, the driver triggers the hardware to
> capture the host graphics output and compress it to JPEG format.
> 
> v4l2-compliance SHA: ac9596560fedb3cd29148ef90b03778d76f1cd9a, 32 bits
> 
> Compliance test for device /dev/video0:
> 
> Driver Info:
>   Driver name  : aspeed-video
>   Card type: Aspeed Video Engine
>   Bus info : platform:aspeed-video
>   Driver version   : 4.18.19
>   Capabilities : 0x8521
>   Video Capture
>   Read/Write
>   Streaming
>   Extended Pix Format
>   Device Capabilities
>   Device Caps  : 0x0521
>   Video Capture
>   Read/Write
>   Streaming
>   Extended Pix Format
> 
> Required ioctls:
>   test VIDIOC_QUERYCAP: OK
> 
> Allow for multiple opens:
>   test second /dev/video0 open: OK
>   test VIDIOC_QUERYCAP: OK
>   test VIDIOC_G/S_PRIORITY: OK
>   test for unlimited opens: OK
> 
> Debug ioctls:
>   test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>   test VIDIOC_LOG_STATUS: OK (Not Supported)
> 
> Input ioctls:
>   test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>   test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>   test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>   test VIDIOC_ENUMAUDIO: OK (Not Supported)
>   test VIDIOC_G/S/ENUMINPUT: OK
>   test VIDIOC_G/S_AUDIO: OK (Not Supported)
>   Inputs: 1 Audio Inputs: 0 Tuners: 0
> 
> Output ioctls:
>   test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>   test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>   test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>   test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>   test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>   Outputs: 0 Audio Outputs: 0 Modulators: 0
> 
> Input/Output configuration ioctls:
>   test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>   test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK
>   test VIDIOC_DV_TIMINGS_CAP: OK
>   test VIDIOC_G/S_EDID: OK (Not Supported)
> 
> Control ioctls (Input 0):
>   test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
>   test VIDIOC_QUERYCTRL: OK
>   test VIDIOC_G/S_CTRL: OK
>   test VIDIOC_G/S/TRY_EXT_CTRLS: OK
>   fail: v4l2-test-controls.cpp(831): failed to find event for 
> control 'Chroma Subsampling'

This is fixed in our master branch. Event handling was broken for a while.

I recommend that you upgrade, since this was a nasty little bug.

Regards,

Hans

>   test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
>   test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>   Standard Controls: 3 Private Controls: 0
> 
> Format ioctls (Input 0):
>   test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>   test VIDIOC_G/S_PARM: OK
>   test VIDIOC_G_FBUF: OK (Not Supported)
>   test VIDIOC_G_FMT: OK
>   test VIDIOC_TRY_FMT: OK
>   test VIDIOC_S_FMT: OK
>   test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>   test Cropping: OK (Not Supported)
>   test Composing: OK (Not Supported)
>   test Scaling: OK (Not Supported)
> 
> Codec ioctls (Input 0):
>   test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>   test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>   test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> 
> Buffer ioctls (Input 0):
>   test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>   test VIDIOC_EXPBUF: OK
>   test Requests: OK (Not Supported)
> 
> Test input 0:
> 
> Streaming ioctls:
>   test read/write: OK
>   test blocking wait: OK
>   test MMAP: OK 
>   test USERPTR: OK (Not Supported)
>   test DMABUF: Cannot test, specify --expbuf-device
> 
> Total: 48, Succeeded: 47, Failed: 1, Warnings: 0
> 
> Changes since v5:
>  - Rework resolution change and v4l2 timings functions again with active and
>detected timings.
>  - Renamed a few things.
>  - Fixed polarities in the timings.
> 
> Changes since v4:
>  - Set real min and max resolution in enum_dv_timings
>  - Add check for vb2_is_busy before settings the timings
>  - Set max frame rate to the actual max rather than max + 1
>  - Correct the input status to 0.
>  - Rework resolution change to only set the relevant h/w regs during startup 
> or
>when set_timings is called.
> 
> Changes since v3:
>  - Switch update reg function to use u32 clear rather than unsigned long mask
>  - Add timing information from host VGA signal
>  - Fix binding documentation mispelling
>  - Fix upper case hex v

Re: [PATCH 0/5] media/sun6i: Allwinner A64 CSI support

2018-12-03 Thread Chen-Yu Tsai
On Mon, Dec 3, 2018 at 6:07 PM Jagan Teki  wrote:
>
> This series support CSI on Allwinner A64.
>
> The CSI controller seems similar to that of in H3, so fallback
> compatible is used to load the driver.
>
> Unlike other SoC's A64 has set of GPIO Pin gropus SDA, SCK intead
> of dedicated I2C controller, so this series used i2c-gpio bitbanging.
>
> Right now the camera is able to detect, but capture images shows
> sequence of red, blue line. any suggestion please help.

The CSI controller doesn't seem to work properly at the default
clock rate of 600 MHz. Dropping it down to 300 MHz, the default
rate used by the BSP, fixes things.

The BSP also tries to use different clock rates (multiples of 108 MHz)
according to the captured image size. I've not tried this since the
driver no longer exports sub-device controls, and I currently don't
know how to handle that to change the resolution.

Regards
ChenYu


Re: [PATCH 2/5] dt-bindings: media: sun6i: Add vcc-csi supply property

2018-12-03 Thread Chen-Yu Tsai
On Mon, Dec 3, 2018 at 6:08 PM Jagan Teki  wrote:
>
> Most of the Allwinner A64 CSI controllers are supply with
> VCC-PE pin. which need to supply for some of the boards to
> trigger the power.
>
> So, document the supply property as vcc-csi so-that the required
> board can eable it via device tree.
>
> Used vcc-csi instead of vcc-pe to have better naming convention
> wrt other controller pin supplies.

This is not related to the CSI controller. It belongs in the pin
controller, but that has its own set of problems like possible
circular dependencies.

ChenYu


[PATCH 1/5] dt-bindings: media: sun6i: Add A64 CSI compatible (w/ H3 fallback)

2018-12-03 Thread Jagan Teki
Allwinner A64 CSI has single channel time-multiplexed BT.656
CMOS sensor interface like H3.

Add a compatible string for it with H3 fallback compatible string,
in this case the H3 driver can be used.

Signed-off-by: Jagan Teki 
---
 Documentation/devicetree/bindings/media/sun6i-csi.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
b/Documentation/devicetree/bindings/media/sun6i-csi.txt
index cc37cf7fd051..e78cf4f9bc8c 100644
--- a/Documentation/devicetree/bindings/media/sun6i-csi.txt
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -7,6 +7,7 @@ Required properties:
   - compatible: value must be one of:
 * "allwinner,sun6i-a31-csi"
 * "allwinner,sun8i-h3-csi"
+* "allwinner,sun50i-a64-csi", "allwinner,sun8i-h3-csi"
 * "allwinner,sun8i-v3s-csi"
   - reg: base address and size of the memory-mapped region.
   - interrupts: interrupt associated to this IP
-- 
2.18.0.321.gffc6fa0e3



[PATCH 2/5] dt-bindings: media: sun6i: Add vcc-csi supply property

2018-12-03 Thread Jagan Teki
Most of the Allwinner A64 CSI controllers are supply with
VCC-PE pin. which need to supply for some of the boards to
trigger the power.

So, document the supply property as vcc-csi so-that the required
board can eable it via device tree.

Used vcc-csi instead of vcc-pe to have better naming convention
wrt other controller pin supplies.

Signed-off-by: Jagan Teki 
---
 Documentation/devicetree/bindings/media/sun6i-csi.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
b/Documentation/devicetree/bindings/media/sun6i-csi.txt
index e78cf4f9bc8c..5fb6fd4e2c7d 100644
--- a/Documentation/devicetree/bindings/media/sun6i-csi.txt
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -18,6 +18,9 @@ Required properties:
   - clock-names: the clock names mentioned above
   - resets: phandles to the reset line driving the CSI
 
+Optional properties:
+  - vcc-csi-supply: the VCC-CSI power supply of the CSI PE group
+
 The CSI node should contain one 'port' child node with one child 'endpoint'
 node, according to the bindings defined in
 Documentation/devicetree/bindings/media/video-interfaces.txt.
-- 
2.18.0.321.gffc6fa0e3



[PATCH 3/5] media: sun6i: Add vcc-csi supply regulator

2018-12-03 Thread Jagan Teki
Most of the Allwinner A64 CSI controllers are supply with
VCC-PE pin, which may not be turned on by default.

Add support for such boards by adding voltage regulator handling
code to sun6i csi driver.

Used vcc-csi instead of vcc-pe to have better naming convention
wrt other controller pin supplies.

Signed-off-by: Jagan Teki 
---
 .../media/platform/sunxi/sun6i-csi/sun6i_csi.c| 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c 
b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index 6950585edb5a..5836fa5e6b01 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -36,6 +37,7 @@ struct sun6i_csi_dev {
struct clk  *clk_mod;
struct clk  *clk_ram;
struct reset_control*rstc_bus;
+   struct regulator*regulator;
 
int planar_offset[3];
 };
@@ -163,9 +165,16 @@ int sun6i_csi_set_power(struct sun6i_csi *csi, bool enable)
clk_disable_unprepare(sdev->clk_ram);
clk_disable_unprepare(sdev->clk_mod);
reset_control_assert(sdev->rstc_bus);
+   regulator_disable(sdev->regulator);
return 0;
}
 
+   ret = regulator_enable(sdev->regulator);
+   if (ret) {
+   dev_err(sdev->dev, "Enable vcc csi supply err %d\n", ret);
+   return ret;
+   }
+
ret = clk_prepare_enable(sdev->clk_mod);
if (ret) {
dev_err(sdev->dev, "Enable csi clk err %d\n", ret);
@@ -809,6 +818,12 @@ static int sun6i_csi_resource_request(struct sun6i_csi_dev 
*sdev,
if (IS_ERR(io_base))
return PTR_ERR(io_base);
 
+   sdev->regulator = devm_regulator_get(&pdev->dev, "vcc-csi");
+   if (IS_ERR(sdev->regulator)) {
+   dev_err(&pdev->dev, "Unable to acquire csi vcc supply\n");
+   return PTR_ERR(sdev->regulator);
+   }
+
sdev->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "bus", io_base,
 &sun6i_csi_regmap_config);
if (IS_ERR(sdev->regmap)) {
-- 
2.18.0.321.gffc6fa0e3



[PATCH 5/5] arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

2018-12-03 Thread Jagan Teki
Amarula A64-Relic board by default bound with OV5640 camera,
so add support for it with below pin information.

- PE13, PE12 via i2c-gpio bitbanging
- CLK_CSI_MCLK as external clock
- PE1 as external clock pin muxing
- DLDO3 as vcc-csi supply
- DLDO3 as AVDD supply
- ALDO1 as DOVDD supply
- ELDO3 as DVDD supply
- PE14 gpio for reset pin
- PE15 gpio for powerdown pin

Signed-off-by: Jagan Teki 
---
 .../allwinner/sun50i-a64-amarula-relic.dts| 54 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  5 ++
 2 files changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 6cb2b7f0c817..9ac6d773188b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -22,6 +22,41 @@
stdout-path = "serial0:115200n8";
};
 
+   i2c-csi {
+   compatible = "i2c-gpio";
+   sda-gpios = <&pio 4 13 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+   scl-gpios = <&pio 4 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+   i2c-gpio,delay-us = <5>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ov5640: camera@3c {
+   compatible = "ovti,ov5640";
+   reg = <0x3c>;
+   pinctrl-names = "default";
+   pinctrl-0 = <&csi_mclk_pin>;
+   clocks = <&ccu CLK_CSI_MCLK>;
+   clock-names = "xclk";
+
+   AVDD-supply = <®_dldo3>;
+   DOVDD-supply = <®_aldo1>;
+   DVDD-supply = <®_eldo3>;
+   reset-gpios = <&pio 4 14 GPIO_ACTIVE_LOW>; /* 
CSI-RST-R: PE14 */
+   powerdown-gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* 
CSI-STBY-R: PE15 */
+
+   port {
+   ov5640_ep: endpoint {
+   remote-endpoint = <&csi_ep>;
+   bus-width = <8>;
+   hsync-active = <1>; /* Active high */
+   vsync-active = <0>; /* Active low */
+   data-active = <1>;  /* Active high */
+   pclk-sample = <1>;  /* Rising */
+   };
+   };
+   };
+   };
+
wifi_pwrseq: wifi-pwrseq {
compatible = "mmc-pwrseq-simple";
clocks = <&rtc 1>;
@@ -30,6 +65,25 @@
};
 };
 
+&csi {
+   vcc-csi-supply = <®_dldo3>;
+   status = "okay";
+
+   port {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   csi_ep: endpoint {
+   remote-endpoint = <&ov5640_ep>;
+   bus-width = <8>;
+   hsync-active = <1>; /* Active high */
+   vsync-active = <0>; /* Active low */
+   data-active = <1>;  /* Active high */
+   pclk-sample = <1>;  /* Rising */
+   };
+   };
+};
+
 &ehci0 {
status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index d32ff694ac5c..844bb44a78af 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -538,6 +538,11 @@
function = "csi0";
};
 
+   csi_mclk_pin: csi-mclk {
+   pins = "PE1";
+   function = "csi0";
+   };
+
i2c0_pins: i2c0_pins {
pins = "PH0", "PH1";
function = "i2c0";
-- 
2.18.0.321.gffc6fa0e3



[PATCH 4/5] arm64: dts: allwinner: a64: Add A64 CSI controller

2018-12-03 Thread Jagan Teki
Allwinner A64 CSI controller has similar features as like in
H3, So add support for A64 via H3 fallback.

Signed-off-by: Jagan Teki 
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 21 +++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 384c417cb7a2..d32ff694ac5c 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -532,6 +532,12 @@
interrupt-controller;
#interrupt-cells = <3>;
 
+   csi_pins: csi-pins {
+   pins = "PE0", "PE2", "PE3", "PE4", "PE5", "PE6",
+  "PE7", "PE8", "PE9", "PE10", "PE11";
+   function = "csi0";
+   };
+
i2c0_pins: i2c0_pins {
pins = "PH0", "PH1";
function = "i2c0";
@@ -899,6 +905,21 @@
status = "disabled";
};
 
+   csi: csi@1cb {
+   compatible = "allwinner,sun50i-a64-csi",
+"allwinner,sun8i-h3-csi";
+   reg = <0x01cb 0x1000>;
+   interrupts = ;
+   clocks = <&ccu CLK_BUS_CSI>,
+<&ccu CLK_CSI_SCLK>,
+<&ccu CLK_DRAM_CSI>;
+   clock-names = "bus", "mod", "ram";
+   resets = <&ccu RST_BUS_CSI>;
+   pinctrl-names = "default";
+   pinctrl-0 = <&csi_pins>;
+   status = "disabled";
+   };
+
hdmi: hdmi@1ee {
compatible = "allwinner,sun50i-a64-dw-hdmi",
 "allwinner,sun8i-a83t-dw-hdmi";
-- 
2.18.0.321.gffc6fa0e3



[PATCH 0/5] media/sun6i: Allwinner A64 CSI support

2018-12-03 Thread Jagan Teki
This series support CSI on Allwinner A64.

The CSI controller seems similar to that of in H3, so fallback
compatible is used to load the driver.

Unlike other SoC's A64 has set of GPIO Pin gropus SDA, SCK intead
of dedicated I2C controller, so this series used i2c-gpio bitbanging.

Right now the camera is able to detect, but capture images shows 
sequence of red, blue line. any suggestion please help.

Any inputs,
Jagan.

Jagan Teki (5):
  dt-bindings: media: sun6i: Add A64 CSI compatible (w/ H3 fallback)
  dt-bindings: media: sun6i: Add vcc-csi supply property
  media: sun6i: Add vcc-csi supply regulator
  arm64: dts: allwinner: a64: Add A64 CSI controller
  arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

 .../devicetree/bindings/media/sun6i-csi.txt   |  4 ++
 .../allwinner/sun50i-a64-amarula-relic.dts| 54 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 26 +
 .../platform/sunxi/sun6i-csi/sun6i_csi.c  | 15 ++
 4 files changed, 99 insertions(+)

-- 
2.18.0.321.gffc6fa0e3



Re: v4l controls API

2018-12-03 Thread Hans Verkuil
On 12/03/2018 11:03 AM, Sebastian Süsens wrote:
> Hey,
> 
> I use the driver mx6s_capture kernel 4.9.88 
> On the device tree it is registered with following name "fsl,imx6s-csi".

Ah, that's probably the freescale driver. We don't support that. It's known
to be quite buggy.

Sorry, you're on your own here.

Regards,

Hans

> 
> Hint:
> I have no sub-devices on my systems only /dev/video0
> 
> 
>Sebastian Süsens   Tel.   +49 4321 559 56-27
>mycable GmbH   Fax+49 4321 559 56-10
>Gartenstrasse 10
>24534 Neumuenster, Germany Email  s...@mycable.de
> 
>mycable GmbH, Managing Director: Michael Carstens-Behrens
>USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM
> 
>This e-mail and any files transmitted with it are confidential and
>intended solely for the use of the individual or entity to whom
>they are addressed. If you have received this e-mail in error,
>please notify the sender and delete all copies from your system.
> 
> 
> - Ursprüngliche Mail -
> Von: "Hans Verkuil" 
> An: "Sebastian Süsens" , "linux-media" 
> 
> Gesendet: Montag, 3. Dezember 2018 09:29:14
> Betreff: Re: v4l controls API
> 
> On 12/03/2018 09:02 AM, Sebastian Süsens wrote:
>> Hello,
>>
>> I don't know how to get access to the v4l controls on a I2C camera sensor.
>>
>> My driver structure looks following:
>>
>> bridge driver-> csi-driver   
>>-> sensor driver (includes controls)
>> register-async-notifer for csi driverregister-async-notifer for 
>> sensor driver
>> register video device
>>
>> The v4l2 API say:
>> When a sub-device is registered with a V4L2 driver by calling 
>> v4l2_device_register_subdev() and the ctrl_handler fields of both 
>> v4l2_subdev and v4l2_device are set, then the controls of the subdev will 
>> become automatically available in the V4L2 driver as well. If the subdev 
>> driver contains controls that already exist in the V4L2 driver, then those 
>> will be skipped (so a V4L2 driver can always override a subdev control).
>>
>> But how can I get access to the controls by asynchronous registration, 
>> because the controls are not added to the video device automatically?
> 
> Yes, they are via v4l2_device_register_subdev(), which is called by the async 
> code
> when the subdev driver arrives.
> 
> Note that this assumes that the bridge driver has a control handler that 
> struct
> v4l2_device points to (the ctrl_handler field).
> 
> Also note that certain types of drivers (media controller-based) such as the 
> imx
> driver do not 'inherit' controls since each subdev has its own v4l-subdevX 
> device node
> through which its controls can be set. You do not mention which bridge driver 
> you are
> using, so I can't tell whether or not it falls in this category.
> 
> Regards,
> 
>   Hans
> 
>>
>> Normally I can use:
>>
>> v4l2-ctl -l -d /dev/video0
>>
>> I don't know if this forum is the right place for this question, so please 
>> answer with a private e-mail s...@mycable.de
>>
>> 
>>Sebastian Süsens   Tel.   +49 4321 559 56-27
>>mycable GmbH   Fax+49 4321 559 56-10
>>Gartenstrasse 10
>>24534 Neumuenster, Germany Email  s...@mycable.de
>> 
>>mycable GmbH, Managing Director: Michael Carstens-Behrens
>>USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM
>> 
>>This e-mail and any files transmitted with it are confidential and
>>intended solely for the use of the individual or entity to whom
>>they are addressed. If you have received this e-mail in error,
>>please notify the sender and delete all copies from your system.
>> 
>>



Re: v4l controls API

2018-12-03 Thread Sebastian Süsens
Hey,

I use the driver mx6s_capture kernel 4.9.88 
On the device tree it is registered with following name "fsl,imx6s-csi".

Hint:
I have no sub-devices on my systems only /dev/video0


   Sebastian Süsens   Tel.   +49 4321 559 56-27
   mycable GmbH   Fax+49 4321 559 56-10
   Gartenstrasse 10
   24534 Neumuenster, Germany Email  s...@mycable.de

   mycable GmbH, Managing Director: Michael Carstens-Behrens
   USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM

   This e-mail and any files transmitted with it are confidential and
   intended solely for the use of the individual or entity to whom
   they are addressed. If you have received this e-mail in error,
   please notify the sender and delete all copies from your system.


- Ursprüngliche Mail -
Von: "Hans Verkuil" 
An: "Sebastian Süsens" , "linux-media" 

Gesendet: Montag, 3. Dezember 2018 09:29:14
Betreff: Re: v4l controls API

On 12/03/2018 09:02 AM, Sebastian Süsens wrote:
> Hello,
> 
> I don't know how to get access to the v4l controls on a I2C camera sensor.
> 
> My driver structure looks following:
> 
> bridge driver-> csi-driver
>   -> sensor driver (includes controls)
> register-async-notifer for csi driverregister-async-notifer for 
> sensor driver
> register video device
> 
> The v4l2 API say:
> When a sub-device is registered with a V4L2 driver by calling 
> v4l2_device_register_subdev() and the ctrl_handler fields of both v4l2_subdev 
> and v4l2_device are set, then the controls of the subdev will become 
> automatically available in the V4L2 driver as well. If the subdev driver 
> contains controls that already exist in the V4L2 driver, then those will be 
> skipped (so a V4L2 driver can always override a subdev control).
> 
> But how can I get access to the controls by asynchronous registration, 
> because the controls are not added to the video device automatically?

Yes, they are via v4l2_device_register_subdev(), which is called by the async 
code
when the subdev driver arrives.

Note that this assumes that the bridge driver has a control handler that struct
v4l2_device points to (the ctrl_handler field).

Also note that certain types of drivers (media controller-based) such as the imx
driver do not 'inherit' controls since each subdev has its own v4l-subdevX 
device node
through which its controls can be set. You do not mention which bridge driver 
you are
using, so I can't tell whether or not it falls in this category.

Regards,

Hans

> 
> Normally I can use:
> 
> v4l2-ctl -l -d /dev/video0
> 
> I don't know if this forum is the right place for this question, so please 
> answer with a private e-mail s...@mycable.de
> 
> 
>Sebastian Süsens   Tel.   +49 4321 559 56-27
>mycable GmbH   Fax+49 4321 559 56-10
>Gartenstrasse 10
>24534 Neumuenster, Germany Email  s...@mycable.de
> 
>mycable GmbH, Managing Director: Michael Carstens-Behrens
>USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM
> 
>This e-mail and any files transmitted with it are confidential and
>intended solely for the use of the individual or entity to whom
>they are addressed. If you have received this e-mail in error,
>please notify the sender and delete all copies from your system.
> 
>


Re: [PATCH 5/6] [DO NOT MERGE] ARM: dts: sunxi: bananapi-m2p: Add HDF5640 camera module

2018-12-03 Thread Chen-Yu Tsai
On Mon, Dec 3, 2018 at 5:52 PM Jagan Teki  wrote:
>
> On Fri, Nov 30, 2018 at 1:29 PM Chen-Yu Tsai  wrote:
> >
> > The Bananapi M2+ comes with an optional sensor based on the ov5640 from
> > Omnivision. Enable the support for it in the DT.
> >
> > Signed-off-by: Chen-Yu Tsai 
> > ---
> >  arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi | 87 +++
> >  1 file changed, 87 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi 
> > b/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi
> > index b3283aeb5b7d..d97a98acf378 100644
> > --- a/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi
> > +++ b/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi
> > @@ -89,6 +89,42 @@
> > };
> > };
> >
> > +   reg_cam_avdd: cam-avdd {
> > +   compatible = "regulator-fixed";
> > +   regulator-name = "csi-avdd";
> > +   regulator-min-microvolt = <280>;
> > +   regulator-max-microvolt = <280>;
> > +   startup-delay-us = <200>; /* 50 us + board delays */
> > +   enable-active-high;
> > +   gpio = <&pio 3 14 GPIO_ACTIVE_HIGH>; /* PD14 */
> > +   };
> > +
> > +   reg_cam_dovdd: cam-dovdd {
> > +   compatible = "regulator-fixed";
> > +   regulator-name = "csi-dovdd";
> > +   regulator-min-microvolt = <280>;
> > +   regulator-max-microvolt = <280>;
> > +   /*
> > +* This regulator also powers the pull-ups for the I2C bus.
> > +* For some reason, if this is turned off, subsequent use
> > +* of the I2C bus, even when turned on, does not work.
> > +*/
> > +   startup-delay-us = <200>; /* 50 us + board delays */
> > +   regulator-always-on;
> > +   enable-active-high;
> > +   gpio = <&pio 3 14 GPIO_ACTIVE_HIGH>; /* PD14 */
> > +   };
> > +
> > +   reg_cam_dvdd: cam-dvdd {
> > +   compatible = "regulator-fixed";
> > +   regulator-name = "csi-dvdd";
> > +   regulator-min-microvolt = <150>;
> > +   regulator-max-microvolt = <150>;
> > +   startup-delay-us = <200>; /* 50 us + board delays */
> > +   enable-active-high;
> > +   gpio = <&pio 3 14 GPIO_ACTIVE_HIGH>; /* PD14 */
> > +   };
> > +
> > reg_gmac_3v3: gmac-3v3 {
> >   compatible = "regulator-fixed";
> >   regulator-name = "gmac-3v3";
> > @@ -106,6 +142,26 @@
> > };
> >  };
> >
> > +&csi {
> > +   status = "okay";
> > +
> > +   port {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   /* Parallel bus endpoint */
> > +   csi_from_ov5640: endpoint {
> > +   remote-endpoint = <&ov5640_to_csi>;
> > +   bus-width = <8>;
> > +   data-shift = <2>;
>
> If I'm not wrong, the data-shift is not available in sun6i at-least in
> conf register.

Indeed. Seems only a few drivers actually take this into account. Since
this is just an example, I'm not going to respin it.

ChenYu


Re: [PATCH 5/6] [DO NOT MERGE] ARM: dts: sunxi: bananapi-m2p: Add HDF5640 camera module

2018-12-03 Thread Jagan Teki
On Fri, Nov 30, 2018 at 1:29 PM Chen-Yu Tsai  wrote:
>
> The Bananapi M2+ comes with an optional sensor based on the ov5640 from
> Omnivision. Enable the support for it in the DT.
>
> Signed-off-by: Chen-Yu Tsai 
> ---
>  arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi | 87 +++
>  1 file changed, 87 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi 
> b/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi
> index b3283aeb5b7d..d97a98acf378 100644
> --- a/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi
> +++ b/arch/arm/boot/dts/sunxi-bananapi-m2-plus.dtsi
> @@ -89,6 +89,42 @@
> };
> };
>
> +   reg_cam_avdd: cam-avdd {
> +   compatible = "regulator-fixed";
> +   regulator-name = "csi-avdd";
> +   regulator-min-microvolt = <280>;
> +   regulator-max-microvolt = <280>;
> +   startup-delay-us = <200>; /* 50 us + board delays */
> +   enable-active-high;
> +   gpio = <&pio 3 14 GPIO_ACTIVE_HIGH>; /* PD14 */
> +   };
> +
> +   reg_cam_dovdd: cam-dovdd {
> +   compatible = "regulator-fixed";
> +   regulator-name = "csi-dovdd";
> +   regulator-min-microvolt = <280>;
> +   regulator-max-microvolt = <280>;
> +   /*
> +* This regulator also powers the pull-ups for the I2C bus.
> +* For some reason, if this is turned off, subsequent use
> +* of the I2C bus, even when turned on, does not work.
> +*/
> +   startup-delay-us = <200>; /* 50 us + board delays */
> +   regulator-always-on;
> +   enable-active-high;
> +   gpio = <&pio 3 14 GPIO_ACTIVE_HIGH>; /* PD14 */
> +   };
> +
> +   reg_cam_dvdd: cam-dvdd {
> +   compatible = "regulator-fixed";
> +   regulator-name = "csi-dvdd";
> +   regulator-min-microvolt = <150>;
> +   regulator-max-microvolt = <150>;
> +   startup-delay-us = <200>; /* 50 us + board delays */
> +   enable-active-high;
> +   gpio = <&pio 3 14 GPIO_ACTIVE_HIGH>; /* PD14 */
> +   };
> +
> reg_gmac_3v3: gmac-3v3 {
>   compatible = "regulator-fixed";
>   regulator-name = "gmac-3v3";
> @@ -106,6 +142,26 @@
> };
>  };
>
> +&csi {
> +   status = "okay";
> +
> +   port {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   /* Parallel bus endpoint */
> +   csi_from_ov5640: endpoint {
> +   remote-endpoint = <&ov5640_to_csi>;
> +   bus-width = <8>;
> +   data-shift = <2>;

If I'm not wrong, the data-shift is not available in sun6i at-least in
conf register.


Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset

2018-12-03 Thread Sakari Ailus
Hi Laurent,

On Fri, Nov 30, 2018 at 01:07:53AM +0200, Laurent Pinchart wrote:
> Hello Bing,
> 
> On Wednesday, 7 November 2018 06:16:47 EET Bing Bu Cao wrote:
> > On 11/01/2018 08:03 PM, Sakari Ailus wrote:
> > > On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote:
> 
> [snip]
> 
> > >> ImgU media topology print:
> > >> 
> > >> # media-ctl -d /dev/media0 -p
> > >> Media controller API version 4.19.0
> > >> 
> > >> Media device information
> > >> 
> > >> driver  ipu3-imgu
> > >> model   ipu3-imgu
> > >> serial
> > >> bus infoPCI::00:05.0
> > >> hw revision 0x80862015
> > >> driver version  4.19.0
> > >> 
> > >> Device topology
> > >> - entity 1: ipu3-imgu 0 (5 pads, 5 links)
> > >> type V4L2 subdev subtype Unknown flags 0
> > >> device node name /dev/v4l-subdev0
> > >>  pad0: Sink
> > >>  [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown
> > > 
> > > This doesn't seem right. Which formats can be enumerated from the pad?
> > > 
> > >>   crop:(0,0)/1920x1080
> > >>   compose:(0,0)/1920x1080]
> > > 
> > > Does the compose rectangle affect the scaling on all outputs?
> > 
> > Sakari, driver use crop and compose targets to help set input-feeder and BDS
> > output resolutions which are 2 key block of whole imaging pipeline, not the
> > actual ending output, but they will impact the final output.
> > 
> > >>  <- "ipu3-imgu 0 input":0 []
> > > 
> > > Are there links that have no useful link configuration? If so, you should
> > > set them enabled and immutable in the driver.
> > 
> > The enabled status of input pads is used to get which pipe that user is
> > trying to enable (ipu3_link_setup()), so it could not been set as immutable.
> 
> Each pipe needs an input in order to operate, so from that point of view the 
> input is mandatory. Why can't we make this link immutable, and use the stream 
> state (VIDIOC_STREAMON/VIDIOC_STREAMOFF) to enable/disable the pipes ?

There are only two options (AFAIK) in choosing the firmware, and by
configuring the links this is better visible to the user: the links the
state of which can be changed are not immutable. The driver can also obtain
the explicit pipeline configuration, which makes the implementation more
simple.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH 1/6] media: dt-bindings: media: sun6i: Separate H3 compatible from A31

2018-12-03 Thread Chen-Yu Tsai
On Mon, Dec 3, 2018 at 5:42 PM Jagan Teki  wrote:
>
> On Fri, Nov 30, 2018 at 1:29 PM Chen-Yu Tsai  wrote:
> >
> > The CSI controller found on the H3 (and H5) is a reduced version of the
> > one found on the A31. It only has 1 channel, instead of 4 channels for
> > time-multiplexed BT.656. Since the H3 is a reduced version, it cannot
> > "fallback" to a compatible that implements more features than it
> > supports.
> >
> > Split out the H3 compatible as a separate entry, with no fallback.
> >
> > Fixes: b7eadaa3a02a ("media: dt-bindings: media: sun6i: Add A31 and H3
> >   compatibles")
> > Signed-off-by: Chen-Yu Tsai 
>
> "media" text appear two times on commit head.

Just following what previous commits did. :)

> Reviewed-by: Jagan Teki 

BTW, I know you've been trying to get CSI to work on the A64.
I have it working for the Bananapi-M64. The CSI SCLK needs to
be lowered to 300 MHz or the image gets corrupted.

ChenYu


Re: [PATCH 4/6] ARM: dts: sunxi: h3-h5: Add pinmux setting for CSI MCLK on PE1

2018-12-03 Thread Jagan Teki
On Fri, Nov 30, 2018 at 1:28 PM Chen-Yu Tsai  wrote:
>
> Some camera modules have the SoC feeding a master clock to the sensor
> instead of having a standalone crystal. This clock signal is generated
> from the clock control unit and output from the CSI MCLK function of
> pin PE1.
>
> Add a pinmux setting for it for camera sensors to reference.
>
> Signed-off-by: Chen-Yu Tsai 
> ---

On Fri, Nov 30, 2018 at 1:29 PM Chen-Yu Tsai  wrote:
>
> The CSI controller found on the H3 (and H5) is a reduced version of the
> one found on the A31. It only has 1 channel, instead of 4 channels for
> time-multiplexed BT.656. Since the H3 is a reduced version, it cannot
> "fallback" to a compatible that implements more features than it
> supports.
>
> Drop the A31 fallback compatible.
>
> Fixes: f89120b6f554 ("ARM: dts: sun8i: Add the H3/H5 CSI controller")
> Signed-off-by: Chen-Yu Tsai 
> ---

Reviewed-by: Jagan Teki 


Re: [PATCH 3/6] ARM: dts: sunxi: h3/h5: Drop A31 fallback compatible for CSI controller

2018-12-03 Thread Jagan Teki
On Fri, Nov 30, 2018 at 1:29 PM Chen-Yu Tsai  wrote:
>
> The CSI controller found on the H3 (and H5) is a reduced version of the
> one found on the A31. It only has 1 channel, instead of 4 channels for
> time-multiplexed BT.656. Since the H3 is a reduced version, it cannot
> "fallback" to a compatible that implements more features than it
> supports.
>
> Drop the A31 fallback compatible.
>
> Fixes: f89120b6f554 ("ARM: dts: sun8i: Add the H3/H5 CSI controller")
> Signed-off-by: Chen-Yu Tsai 
> ---

Reviewed-by: Jagan Teki 


Re: [PATCH 2/6] media: sun6i: Add H3 compatible

2018-12-03 Thread Jagan Teki
On Fri, Nov 30, 2018 at 1:29 PM Chen-Yu Tsai  wrote:
>
> The CSI controller found on the H3 (and H5) is a reduced version of the
> one found on the A31. It only has 1 channel, instead of 4 channels for
> time-multiplexed BT.656. Since the H3 is a reduced version, it cannot
> "fallback" to a compatible that implements more features than it
> supports.
>
> Add a compatible string entry for the H3.
>
> Signed-off-by: Chen-Yu Tsai 
> ---

Reviewed-by: Jagan Teki 


Re: [PATCH 1/6] media: dt-bindings: media: sun6i: Separate H3 compatible from A31

2018-12-03 Thread Jagan Teki
On Fri, Nov 30, 2018 at 1:29 PM Chen-Yu Tsai  wrote:
>
> The CSI controller found on the H3 (and H5) is a reduced version of the
> one found on the A31. It only has 1 channel, instead of 4 channels for
> time-multiplexed BT.656. Since the H3 is a reduced version, it cannot
> "fallback" to a compatible that implements more features than it
> supports.
>
> Split out the H3 compatible as a separate entry, with no fallback.
>
> Fixes: b7eadaa3a02a ("media: dt-bindings: media: sun6i: Add A31 and H3
>   compatibles")
> Signed-off-by: Chen-Yu Tsai 

"media" text appear two times on commit head.

Reviewed-by: Jagan Teki 


Re: [PATCH v4] media: vivid: Improve timestamping

2018-12-03 Thread Hans Verkuil
On 12/02/2018 09:43 PM, Arnd Bergmann wrote:
> On Sun, Dec 2, 2018 at 2:47 PM Gabriel Francisco Mandaji
>  wrote:
> 
>> @@ -667,10 +653,28 @@ static void vivid_overlay(struct vivid_dev *dev, 
>> struct vivid_buffer *buf)
>> }
>>  }
>>
>> +static void vivid_cap_update_frame_period(struct vivid_dev *dev)
>> +{
>> +   u64 f_period;
>> +
>> +   f_period = (u64)dev->timeperframe_vid_cap.numerator * 10;
>> +   do_div(f_period, dev->timeperframe_vid_cap.denominator);
>> +   if (dev->field_cap == V4L2_FIELD_ALTERNATE)
>> +   do_div(f_period, 2);
>> +   /*
>> +* If "End of Frame", then offset the exposure time by 0.9
>> +* of the frame period.
>> +*/
>> +   dev->cap_frame_eof_offset = f_period * 9;
>> +   do_div(dev->cap_frame_eof_offset, 10);
>> +   dev->cap_frame_period = f_period;
>> +}
> 
> Doing two or three do_div() operations is going to make this rather
> expensive on 32-bit architectures, and it looks like this happens for
> each frame?
> 
> Since each one is a multiplication followed by a division, could this
> be changed to using a different factor followed by a bit shift?

The division by 2 can obviously be replaced by a shift, and the
'End of Frame' calculation can be simplified as well by multiplying by
7 and dividing by 8 (again a simple shift): this equals 0.875 which is
close enough to 0.9 (so update the comment as well).

It's all a bit overkill since this function isn't called very often,
but these are easy changes to make.

Regards,

Hans


[PATCH v6 07/12] media: ov5640: Remove pixel clock rates

2018-12-03 Thread Maxime Ripard
The pixel clock rates were introduced to report the initially static clock
rate.

Since this is now handled dynamically, we can remove them entirely.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 536261618d0d..06afe7572f0e 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -173,7 +173,6 @@ struct ov5640_mode_info {
u32 htot;
u32 vact;
u32 vtot;
-   u32 pixel_clock;
const struct reg_value *reg_data;
u32 reg_data_size;
 };
@@ -697,7 +696,6 @@ static const struct reg_value 
ov5640_setting_15fps_QSXGA_2592_1944[] = {
 /* power-on sensor init reg table */
 static const struct ov5640_mode_info ov5640_mode_init_data = {
0, SUBSAMPLING, 640, 1896, 480, 984,
-   5600,
ov5640_init_setting_30fps_VGA,
ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
 };
@@ -707,91 +705,74 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] 
= {
{
{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 176, 1896, 144, 984,
-2800,
 ov5640_setting_15fps_QCIF_176_144,
 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 320, 1896, 240, 984,
-2800,
 ov5640_setting_15fps_QVGA_320_240,
 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 640, 1896, 480, 1080,
-2800,
 ov5640_setting_15fps_VGA_640_480,
 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 720, 1896, 480, 984,
-2800,
 ov5640_setting_15fps_NTSC_720_480,
 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
{OV5640_MODE_PAL_720_576, SUBSAMPLING,
 720, 1896, 576, 984,
-2800,
 ov5640_setting_15fps_PAL_720_576,
 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
 1024, 1896, 768, 1080,
-2800,
 ov5640_setting_15fps_XGA_1024_768,
 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
{OV5640_MODE_720P_1280_720, SUBSAMPLING,
 1280, 1892, 720, 740,
-2100,
 ov5640_setting_15fps_720P_1280_720,
 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
{OV5640_MODE_1080P_1920_1080, SCALING,
 1920, 2500, 1080, 1120,
-4200,
 ov5640_setting_15fps_1080P_1920_1080,
 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
{OV5640_MODE_QSXGA_2592_1944, SCALING,
 2592, 2844, 1944, 1968,
-8400,
 ov5640_setting_15fps_QSXGA_2592_1944,
 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
}, {
{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 176, 1896, 144, 984,
-5600,
 ov5640_setting_30fps_QCIF_176_144,
 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 320, 1896, 240, 984,
-5600,
 ov5640_setting_30fps_QVGA_320_240,
 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 640, 1896, 480, 1080,
-5600,
 ov5640_setting_30fps_VGA_640_480,
 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 720, 1896, 480, 984,
-5600,
 ov5640_setting_30fps_NTSC_720_480,
 ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
{OV5640_MODE_PAL_720_576, SUBSAMPLING,
 720, 1896, 576, 984,
-5600,
 ov5640_setting_30fps_PAL_720_576,
 ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
 1024, 1896, 768, 1080,
-5600,
 ov5640_setting_30fps_XGA_1024_768,
 ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
{OV5640_MODE_720P_1280_720, SUBSAMPLING,
 1280, 1892, 720, 740,
-4200,
 ov5640_setting_30fps_720P_1280_720,
 ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
{OV

[PATCH v6 11/12] media: ov5640: Add 60 fps support

2018-12-03 Thread Maxime Ripard
Now that we have everything in place to compute the clock rate at runtime,
we can enable the 60fps framerate for the mode we tested it with.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 93fa072135ad..6391ea4f712c 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -111,6 +111,7 @@ enum ov5640_mode_id {
 enum ov5640_frame_rate {
OV5640_15_FPS = 0,
OV5640_30_FPS,
+   OV5640_60_FPS,
OV5640_NUM_FRAMERATES,
 };
 
@@ -139,6 +140,7 @@ MODULE_PARM_DESC(virtual_channel,
 static const int ov5640_framerates[] = {
[OV5640_15_FPS] = 15,
[OV5640_30_FPS] = 30,
+   [OV5640_60_FPS] = 60,
 };
 
 /* regulator supplies */
@@ -1562,6 +1564,11 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum 
ov5640_frame_rate fr,
(!nearest && (mode->hact != width || mode->vact != height)))
return NULL;
 
+   /* Only 640x480 can operate at 60fps (for now) */
+   if (fr == OV5640_60_FPS &&
+   !(mode->hact == 640 && mode->vact == 480))
+   return NULL;
+
return mode;
 }
 
@@ -2057,12 +2064,13 @@ static int ov5640_try_frame_interval(struct ov5640_dev 
*sensor,
int i;
 
minfps = ov5640_framerates[OV5640_15_FPS];
-   maxfps = ov5640_framerates[OV5640_30_FPS];
+   maxfps = ov5640_framerates[OV5640_60_FPS];
 
if (fi->numerator == 0) {
fi->denominator = maxfps;
fi->numerator = 1;
-   return OV5640_30_FPS;
+   rate = OV5640_60_FPS;
+   goto find_mode;
}
 
fps = clamp_val(DIV_ROUND_CLOSEST(fi->denominator, fi->numerator),
@@ -2081,6 +2089,7 @@ static int ov5640_try_frame_interval(struct ov5640_dev 
*sensor,
fi->numerator = 1;
fi->denominator = best_fps;
 
+find_mode:
mode = ov5640_find_mode(sensor, rate, width, height, false);
return mode ? rate : -EINVAL;
 }
@@ -2699,8 +2708,11 @@ static int ov5640_s_frame_interval(struct v4l2_subdev 
*sd,
 
frame_rate = ov5640_try_frame_interval(sensor, &fi->interval,
   mode->hact, mode->vact);
-   if (frame_rate < 0)
-   frame_rate = OV5640_15_FPS;
+   if (frame_rate < 0) {
+   /* Always return a valid frame interval value */
+   fi->interval = sensor->frame_interval;
+   goto out;
+   }
 
mode = ov5640_find_mode(sensor, frame_rate, mode->hact,
mode->vact, true);
-- 
git-series 0.9.1


[PATCH v6 08/12] media: ov5640: Enhance FPS handling

2018-12-03 Thread Maxime Ripard
Now that we have moved the clock generation logic out of the bytes array,
these arrays are identical between the 15fps and 30fps variants.

Remove the duplicate entries, and convert the code accordingly.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 306 ++
 1 file changed, 51 insertions(+), 255 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 06afe7572f0e..abca08d669be 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -341,83 +341,7 @@ static const struct reg_value 
ov5640_init_setting_30fps_VGA[] = {
{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
 };
 
-static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
-   {0x3c07, 0x08, 0, 0},
-   {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3814, 0x31, 0, 0},
-   {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-   {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-   {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-   {0x3810, 0x00, 0, 0},
-   {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-   {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-   {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-   {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
-   {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-   {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-   {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-   {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-   {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
-   {0x3c07, 0x08, 0, 0},
-   {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3814, 0x31, 0, 0},
-   {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-   {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-   {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-   {0x3810, 0x00, 0, 0},
-   {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-   {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-   {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-   {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-   {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-   {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-   {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-   {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-   {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
-   {0x3c07, 0x08, 0, 0},
-   {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3814, 0x31, 0, 0},
-   {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-   {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-   {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-   {0x3810, 0x00, 0, 0},
-   {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-   {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-   {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-   {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
-   {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-   {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-   {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-   {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-   {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
-   {0x3c07, 0x08, 0, 0},
-   {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-   {0x3814, 0x31, 0, 0},
-   {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-   {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-   {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-   {0x3810, 0x00, 0, 0},
-   {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-   {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-   {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-   {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-   {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-   {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0,

[PATCH v6 09/12] media: ov5640: Make the return rate type more explicit

2018-12-03 Thread Maxime Ripard
In the ov5640_try_frame_interval function, the ret variable actually holds
the frame rate index to use, which is represented by the enum
ov5640_frame_rate in the driver.

Make it more obvious.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index abca08d669be..6cdf5ee0e4fa 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2052,8 +2052,8 @@ static int ov5640_try_frame_interval(struct ov5640_dev 
*sensor,
 u32 width, u32 height)
 {
const struct ov5640_mode_info *mode;
+   enum ov5640_frame_rate rate = OV5640_30_FPS;
u32 minfps, maxfps, fps;
-   int ret;
 
minfps = ov5640_framerates[OV5640_15_FPS];
maxfps = ov5640_framerates[OV5640_30_FPS];
@@ -2076,10 +2076,10 @@ static int ov5640_try_frame_interval(struct ov5640_dev 
*sensor,
else
fi->denominator = minfps;
 
-   ret = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS;
+   rate = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS;
 
-   mode = ov5640_find_mode(sensor, ret, width, height, false);
-   return mode ? ret : -EINVAL;
+   mode = ov5640_find_mode(sensor, rate, width, height, false);
+   return mode ? rate : -EINVAL;
 }
 
 static int ov5640_get_fmt(struct v4l2_subdev *sd,
-- 
git-series 0.9.1


[PATCH v6 03/12] media: ov5640: Remove the clocks registers initialization

2018-12-03 Thread Maxime Ripard
Part of the hardcoded initialization sequence is to set up the proper clock
dividers. However, this is now done dynamically through proper code and as
such, the static one is now redundant.

Let's remove it.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 46 ++-
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 4254ac958424..588b98e2e99f 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -263,8 +263,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
 static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
-   {0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0},
-   {0x3037, 0x13, 0, 0}, {0x3630, 0x36, 0, 0},
+   {0x3630, 0x36, 0, 0},
{0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
{0x3621, 0xe0, 0, 0}, {0x3704, 0xa0, 0, 0}, {0x3703, 0x5a, 0, 0},
{0x3715, 0x78, 0, 0}, {0x3717, 0x01, 0, 0}, {0x370b, 0x60, 0, 0},
@@ -347,7 +346,7 @@ static const struct reg_value 
ov5640_init_setting_30fps_VGA[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
-   {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -366,7 +365,7 @@ static const struct reg_value 
ov5640_setting_30fps_VGA_640_480[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
-   {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -385,7 +384,7 @@ static const struct reg_value 
ov5640_setting_15fps_VGA_640_480[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
-   {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -401,11 +400,10 @@ static const struct reg_value 
ov5640_setting_30fps_XGA_1024_768[] = {
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0},
-   {0x3035, 0x12, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
-   {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -424,7 +422,7 @@ static const struct reg_value 
ov5640_setting_15fps_XGA_1024_768[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
-   {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -443,7 +441,7 @@ static const struct reg_value 
ov5640_setting_30fps_QVGA_320_240[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
-   {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -462,7 +460,7 @@ static const struct reg_value 
ov5640_setting_15fps_QVGA_320_240[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
-   {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
{0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -481,7 +479,7 @@ static const struct reg_value 
ov5640_setting_30fps_QCIF_176_144[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
-   {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+   {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 

[PATCH v6 02/12] media: ov5640: Adjust the clock based on the expected rate

2018-12-03 Thread Maxime Ripard
The clock structure for the PCLK is quite obscure in the documentation, and
was hardcoded through the bytes array of each and every mode.

This is troublesome, since we cannot adjust it at runtime based on other
parameters (such as the number of bytes per pixel), and we can't support
either framerates that have not been used by the various vendors, since we
don't have the needed initialization sequence.

We can however understand how the clock tree works, and then implement some
functions to derive the various parameters from a given rate. And now that
those parameters are calculated at runtime, we can remove them from the
initialization sequence.

The modes also gained a new parameter which is the clock that they are
running at, from the register writes they were doing, so for now the switch
to the new algorithm should be transparent.

Tested-by: Adam Ford  #imx6dq
Co-Developed-by: Jacopo Mondi 
Signed-off-by: Jacopo Mondi 
Signed-off-by: Maxime Ripard 
---
 drivers/media/i2c/ov5640.c | 365 +-
 1 file changed, 364 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 807bd0e386a4..4254ac958424 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -176,6 +176,7 @@ struct ov5640_mode_info {
u32 htot;
u32 vact;
u32 vtot;
+   u32 pixel_clock;
const struct reg_value *reg_data;
u32 reg_data_size;
 };
@@ -701,6 +702,7 @@ static const struct reg_value 
ov5640_setting_15fps_QSXGA_2592_1944[] = {
 /* power-on sensor init reg table */
 static const struct ov5640_mode_info ov5640_mode_init_data = {
0, SUBSAMPLING, 640, 1896, 480, 984,
+   5600,
ov5640_init_setting_30fps_VGA,
ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
 };
@@ -710,74 +712,91 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] 
= {
{
{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 176, 1896, 144, 984,
+2800,
 ov5640_setting_15fps_QCIF_176_144,
 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 320, 1896, 240, 984,
+2800,
 ov5640_setting_15fps_QVGA_320_240,
 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 640, 1896, 480, 1080,
+2800,
 ov5640_setting_15fps_VGA_640_480,
 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 720, 1896, 480, 984,
+2800,
 ov5640_setting_15fps_NTSC_720_480,
 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
{OV5640_MODE_PAL_720_576, SUBSAMPLING,
 720, 1896, 576, 984,
+2800,
 ov5640_setting_15fps_PAL_720_576,
 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
 1024, 1896, 768, 1080,
+2800,
 ov5640_setting_15fps_XGA_1024_768,
 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
{OV5640_MODE_720P_1280_720, SUBSAMPLING,
 1280, 1892, 720, 740,
+2100,
 ov5640_setting_15fps_720P_1280_720,
 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
{OV5640_MODE_1080P_1920_1080, SCALING,
 1920, 2500, 1080, 1120,
+4200,
 ov5640_setting_15fps_1080P_1920_1080,
 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
{OV5640_MODE_QSXGA_2592_1944, SCALING,
 2592, 2844, 1944, 1968,
+8400,
 ov5640_setting_15fps_QSXGA_2592_1944,
 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
}, {
{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 176, 1896, 144, 984,
+5600,
 ov5640_setting_30fps_QCIF_176_144,
 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 320, 1896, 240, 984,
+5600,
 ov5640_setting_30fps_QVGA_320_240,
 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 640, 1896, 480, 1080,
+5600,
 ov5640_setting_30fps_VGA_640_480,
 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 720, 1896, 480, 984,
+5600,
 ov5640_setting_30fps_NTSC_720_480,
 ARRAY_S

[PATCH v6 00/12] media: ov5640: Misc cleanup and improvements

2018-12-03 Thread Maxime Ripard
Hi,

Here is a "small" series that mostly cleans up the ov5640 driver code,
slowly getting rid of the big data array for more understandable code
(hopefully).

The biggest addition would be the clock rate computation at runtime,
instead of relying on those arrays to setup the clock tree
properly. As a side effect, it fixes the framerate that was off by
around 10% on the smaller resolutions, and we now support 60fps.

This also introduces a bunch of new features.

Let me know what you think,
Maxime

Changes from v5:
  - Squashed Jacopo patches fixing MIPI-CSI

Changes from v4:
  - Squashed Jacopo patches fixing the MIPI-CSI case
  - Prefer clock rates superior to the ideal clock rate, even if it
means having a less precise one.
  - Fix the JPEG case according to Hugues suggestions
  - Rebased on 4.20

Changes from v3:
  - Rebased on current Sakari tree
  - Fixed an error when changing only the framerate

Changes from v2:
  - Rebased on latest Sakari PR
  - Fixed the issues reported by Hugues: improper FPS returned for
formats, improper rounding of the FPS, some with his suggestions,
some by simplifying the logic.
  - Expanded the clock tree comments based on the feedback from Samuel
Bobrowicz and Loic Poulain
  - Merged some of the changes made by Samuel Bobrowicz to fix the
MIPI rate computation, fix the call sites of the
ov5640_set_timings function, the auto-exposure calculation call,
etc.
  - Split the patches into smaller ones in order to make it more
readable (hopefully)

Changes from v1:
  - Integrated Hugues' suggestions to fix v4l2-compliance
  - Fixed the bus width with JPEG
  - Dropped the clock rate calculation loops for something simpler as
suggested by Sakari
  - Cache the exposure value instead of using the control value
  - Rebased on top of 4.17

Jacopo Mondi (1):
  media: ov5640: Fix set format regression

Maxime Ripard (11):
  media: ov5640: Adjust the clock based on the expected rate
  media: ov5640: Remove the clocks registers initialization
  media: ov5640: Remove redundant defines
  media: ov5640: Remove redundant register setup
  media: ov5640: Compute the clock rate at runtime
  media: ov5640: Remove pixel clock rates
  media: ov5640: Enhance FPS handling
  media: ov5640: Make the return rate type more explicit
  media: ov5640: Make the FPS clamping / rounding more extendable
  media: ov5640: Add 60 fps support
  media: ov5640: Remove duplicate auto-exposure setup

 drivers/media/i2c/ov5640.c | 764 ++
 1 file changed, 452 insertions(+), 312 deletions(-)

base-commit: 5019ebef9ac7922a8d233cb6fa7a0626454ca80d
-- 
git-series 0.9.1


[PATCH v6 04/12] media: ov5640: Remove redundant defines

2018-12-03 Thread Maxime Ripard
The OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT and OV5640_SCLK_ROOT_DIVIDER_DEFAULT
defines represent exactly the same setup, and are at the same value, than
the more consistent with the rest of the driver OV5640_SCLK2X_ROOT_DIV and
OV5640_SCLK_ROOT_DIV.

Remove them.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 588b98e2e99f..7c18120aabd1 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -95,9 +95,6 @@
 #define OV5640_REG_SDE_CTRL5   0x5585
 #define OV5640_REG_AVG_READOUT 0x56a1
 
-#define OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT 1
-#define OV5640_SCLK_ROOT_DIVIDER_DEFAULT   2
-
 enum ov5640_mode_id {
OV5640_MODE_QCIF_176_144 = 0,
OV5640_MODE_QVGA_320_240,
@@ -2086,8 +2083,8 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
sensor->last_mode = &ov5640_mode_init_data;
 
ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
-(ilog2(OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT) << 2) |
-ilog2(OV5640_SCLK_ROOT_DIVIDER_DEFAULT));
+(ilog2(OV5640_SCLK2X_ROOT_DIV) << 2) |
+ilog2(OV5640_SCLK_ROOT_DIV));
if (ret)
return ret;
 
-- 
git-series 0.9.1


[PATCH v6 01/12] media: ov5640: Fix set format regression

2018-12-03 Thread Maxime Ripard
From: Jacopo Mondi 

The set_fmt operations updates the sensor format only when the image format
is changed. When only the image sizes gets changed, the format do not get
updated causing the sensor to always report the one that was previously in
use.

Without this patch, updating frame size only fails:
  [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ...]

With this patch applied:
  [fmt:UYVY8_2X8/1024x768@1/30 field:none colorspace:srgb xfer:srgb ...]

Fixes: 6949d864776e ("media: ov5640: do not change mode if format or frame
interval is unchanged")
Signed-off-by: Jacopo Mondi 
Signed-off-by: Maxime Ripard 
---
 drivers/media/i2c/ov5640.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index a91d91715d00..807bd0e386a4 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2021,6 +2021,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
struct ov5640_dev *sensor = to_ov5640_dev(sd);
const struct ov5640_mode_info *new_mode;
struct v4l2_mbus_framefmt *mbus_fmt = &format->format;
+   struct v4l2_mbus_framefmt *fmt;
int ret;
 
if (format->pad != 0)
@@ -2038,22 +2039,20 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
if (ret)
goto out;
 
-   if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
-   struct v4l2_mbus_framefmt *fmt =
-   v4l2_subdev_get_try_format(sd, cfg, 0);
+   if (format->which == V4L2_SUBDEV_FORMAT_TRY)
+   fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+   else
+   fmt = &sensor->fmt;
 
-   *fmt = *mbus_fmt;
-   goto out;
-   }
+   *fmt = *mbus_fmt;
 
if (new_mode != sensor->current_mode) {
sensor->current_mode = new_mode;
sensor->pending_mode_change = true;
}
-   if (mbus_fmt->code != sensor->fmt.code) {
-   sensor->fmt = *mbus_fmt;
+   if (mbus_fmt->code != sensor->fmt.code)
sensor->pending_fmt_change = true;
-   }
+
 out:
mutex_unlock(&sensor->lock);
return ret;
-- 
git-series 0.9.1


[PATCH v6 05/12] media: ov5640: Remove redundant register setup

2018-12-03 Thread Maxime Ripard
The MIPI divider is also cleared as part of the clock setup sequence, so we
can remove that code.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 7c18120aabd1..e36b04912dcc 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1404,16 +1404,6 @@ static int ov5640_set_stream_dvp(struct ov5640_dev 
*sensor, bool on)
 
if (on) {
/*
-* reset MIPI PCLK/SERCLK divider
-*
-* SC PLL CONTRL1 0
-* - [3..0]:MIPI PCLK/SERCLK divider
-*/
-   ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0x0f, 0);
-   if (ret)
-   return ret;
-
-   /*
 * configure parallel port control lines polarity
 *
 * POLARITY CTRL0
-- 
git-series 0.9.1


[PATCH v6 06/12] media: ov5640: Compute the clock rate at runtime

2018-12-03 Thread Maxime Ripard
The clock rate, while hardcoded until now, is actually a function of the
resolution, framerate and bytes per pixel. Now that we have an algorithm to
adjust our clock rate, we can select it dynamically when we change the
mode.

This changes a bit the clock rate being used, with the following effect:

+--+--+--+--+-+-++---+
| Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock | 
Deviation |
+--+--+--+--+-+-++---+
|  640 |  480 | 1896 | 1080 |  15 |5600 |   61430400 | 8.84 %   
 |
|  640 |  480 | 1896 | 1080 |  30 |   11200 |  122860800 | 8.84 %   
 |
| 1024 |  768 | 1896 | 1080 |  15 |5600 |   61430400 | 8.84 %   
 |
| 1024 |  768 | 1896 | 1080 |  30 |   11200 |  122860800 | 8.84 %   
 |
|  320 |  240 | 1896 |  984 |  15 |5600 |   55969920 | 0.05 %   
 |
|  320 |  240 | 1896 |  984 |  30 |   11200 |  111939840 | 0.05 %   
 |
|  176 |  144 | 1896 |  984 |  15 |5600 |   55969920 | 0.05 %   
 |
|  176 |  144 | 1896 |  984 |  30 |   11200 |  111939840 | 0.05 %   
 |
|  720 |  480 | 1896 |  984 |  15 |5600 |   55969920 | 0.05 %   
 |
|  720 |  480 | 1896 |  984 |  30 |   11200 |  111939840 | 0.05 %   
 |
|  720 |  576 | 1896 |  984 |  15 |5600 |   55969920 | 0.05 %   
 |
|  720 |  576 | 1896 |  984 |  30 |   11200 |  111939840 | 0.05 %   
 |
| 1280 |  720 | 1892 |  740 |  15 |4200 |   42002400 | 0.01 %   
 |
| 1280 |  720 | 1892 |  740 |  30 |8400 |   84004800 | 0.01 %   
 |
| 1920 | 1080 | 2500 | 1120 |  15 |8400 |   8400 | 0.00 %   
 |
| 1920 | 1080 | 2500 | 1120 |  30 |   16800 |  16800 | 0.00 %   
 |
| 2592 | 1944 | 2844 | 1944 |  15 |8400 |  165862080 | 49.36 %  
 |
+--+--+--+--+-+-++---+

Only the 640x480, 1024x768 and 2592x1944 modes are significantly affected
by the new formula.

In this case, 640x480 and 1024x768 are actually fixed by this change.
Indeed, the sensor was sending data at, for example, 27.33fps instead of
30fps. This is -9%, which is roughly what we're seeing in the array.
Testing these modes with the new clock setup actually fix that error, and
data are now sent at around 30fps.

2592x1944, on the other hand, is probably due to the fact that this mode
can only be used using MIPI-CSI2, in a two lane mode, and never really
tested with a DVP bus.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index e36b04912dcc..536261618d0d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1992,7 +1992,8 @@ static int ov5640_set_mode(struct ov5640_dev *sensor)
 * All the formats we support have 16 bits per pixel, seems to require
 * the same rate than YUV, so we can just use 16 bpp all the time.
 */
-   rate = mode->pixel_clock * 16;
+   rate = mode->vtot * mode->htot * 16;
+   rate *= ov5640_framerates[sensor->current_fr];
if (sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY) {
rate = rate / sensor->ep.bus.mipi_csi2.num_data_lanes;
ret = ov5640_set_mipi_pclk(sensor, rate);
-- 
git-series 0.9.1


[PATCH v6 12/12] media: ov5640: Remove duplicate auto-exposure setup

2018-12-03 Thread Maxime Ripard
The autoexposure setup in the 1080p init array is redundant with the
default value of the sensor.

Remove it.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 6391ea4f712c..bef3f3aae0ed 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -505,7 +505,7 @@ static const struct reg_value 
ov5640_setting_1080P_1920_1080[] = {
{0x3a0e, 0x03, 0, 0}, {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x04, 0, 0},
{0x3a15, 0x60, 0, 0}, {0x4713, 0x02, 0, 0}, {0x4407, 0x04, 0, 0},
{0x460b, 0x37, 0, 0}, {0x460c, 0x20, 0, 0}, {0x3824, 0x04, 0, 0},
-   {0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0},
+   {0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
-- 
git-series 0.9.1


[PATCH v6 10/12] media: ov5640: Make the FPS clamping / rounding more extendable

2018-12-03 Thread Maxime Ripard
The current code uses an algorithm to clamp the FPS values and round them
to the closest supported one that isn't really allows to be extended to
more than two values.

Rework it a bit to make it much easier to extend the amount of FPS options
we support.

Signed-off-by: Maxime Ripard 
Tested-by: Adam Ford  #imx6dq
---
 drivers/media/i2c/ov5640.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 6cdf5ee0e4fa..93fa072135ad 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2053,7 +2053,8 @@ static int ov5640_try_frame_interval(struct ov5640_dev 
*sensor,
 {
const struct ov5640_mode_info *mode;
enum ov5640_frame_rate rate = OV5640_30_FPS;
-   u32 minfps, maxfps, fps;
+   int minfps, maxfps, best_fps, fps;
+   int i;
 
minfps = ov5640_framerates[OV5640_15_FPS];
maxfps = ov5640_framerates[OV5640_30_FPS];
@@ -2064,19 +2065,21 @@ static int ov5640_try_frame_interval(struct ov5640_dev 
*sensor,
return OV5640_30_FPS;
}
 
-   fps = DIV_ROUND_CLOSEST(fi->denominator, fi->numerator);
+   fps = clamp_val(DIV_ROUND_CLOSEST(fi->denominator, fi->numerator),
+   minfps, maxfps);
 
-   fi->numerator = 1;
-   if (fps > maxfps)
-   fi->denominator = maxfps;
-   else if (fps < minfps)
-   fi->denominator = minfps;
-   else if (2 * fps >= 2 * minfps + (maxfps - minfps))
-   fi->denominator = maxfps;
-   else
-   fi->denominator = minfps;
+   best_fps = minfps;
+   for (i = 0; i < ARRAY_SIZE(ov5640_framerates); i++) {
+   int curr_fps = ov5640_framerates[i];
 
-   rate = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS;
+   if (abs(curr_fps - fps) < abs(best_fps - fps)) {
+   best_fps = curr_fps;
+   rate = i;
+   }
+   }
+
+   fi->numerator = 1;
+   fi->denominator = best_fps;
 
mode = ov5640_find_mode(sensor, rate, width, height, false);
return mode ? rate : -EINVAL;
-- 
git-series 0.9.1


Re: v4l controls API

2018-12-03 Thread Hans Verkuil
On 12/03/2018 09:02 AM, Sebastian Süsens wrote:
> Hello,
> 
> I don't know how to get access to the v4l controls on a I2C camera sensor.
> 
> My driver structure looks following:
> 
> bridge driver-> csi-driver
>   -> sensor driver (includes controls)
> register-async-notifer for csi driverregister-async-notifer for 
> sensor driver
> register video device
> 
> The v4l2 API say:
> When a sub-device is registered with a V4L2 driver by calling 
> v4l2_device_register_subdev() and the ctrl_handler fields of both v4l2_subdev 
> and v4l2_device are set, then the controls of the subdev will become 
> automatically available in the V4L2 driver as well. If the subdev driver 
> contains controls that already exist in the V4L2 driver, then those will be 
> skipped (so a V4L2 driver can always override a subdev control).
> 
> But how can I get access to the controls by asynchronous registration, 
> because the controls are not added to the video device automatically?

Yes, they are via v4l2_device_register_subdev(), which is called by the async 
code
when the subdev driver arrives.

Note that this assumes that the bridge driver has a control handler that struct
v4l2_device points to (the ctrl_handler field).

Also note that certain types of drivers (media controller-based) such as the imx
driver do not 'inherit' controls since each subdev has its own v4l-subdevX 
device node
through which its controls can be set. You do not mention which bridge driver 
you are
using, so I can't tell whether or not it falls in this category.

Regards,

Hans

> 
> Normally I can use:
> 
> v4l2-ctl -l -d /dev/video0
> 
> I don't know if this forum is the right place for this question, so please 
> answer with a private e-mail s...@mycable.de
> 
> 
>Sebastian Süsens   Tel.   +49 4321 559 56-27
>mycable GmbH   Fax+49 4321 559 56-10
>Gartenstrasse 10
>24534 Neumuenster, Germany Email  s...@mycable.de
> 
>mycable GmbH, Managing Director: Michael Carstens-Behrens
>USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM
> 
>This e-mail and any files transmitted with it are confidential and
>intended solely for the use of the individual or entity to whom
>they are addressed. If you have received this e-mail in error,
>please notify the sender and delete all copies from your system.
> 
> 



v4l controls API

2018-12-03 Thread Sebastian Süsens
Hello,

I don't know how to get access to the v4l controls on a I2C camera sensor.

My driver structure looks following:

bridge driver-> csi-driver  
-> sensor driver (includes controls)
register-async-notifer for csi driverregister-async-notifer for sensor 
driver
register video device

The v4l2 API say:
When a sub-device is registered with a V4L2 driver by calling 
v4l2_device_register_subdev() and the ctrl_handler fields of both v4l2_subdev 
and v4l2_device are set, then the controls of the subdev will become 
automatically available in the V4L2 driver as well. If the subdev driver 
contains controls that already exist in the V4L2 driver, then those will be 
skipped (so a V4L2 driver can always override a subdev control).

But how can I get access to the controls by asynchronous registration, because 
the controls are not added to the video device automatically?

Normally I can use:

v4l2-ctl -l -d /dev/video0

I don't know if this forum is the right place for this question, so please 
answer with a private e-mail s...@mycable.de


   Sebastian Süsens   Tel.   +49 4321 559 56-27
   mycable GmbH   Fax+49 4321 559 56-10
   Gartenstrasse 10
   24534 Neumuenster, Germany Email  s...@mycable.de

   mycable GmbH, Managing Director: Michael Carstens-Behrens
   USt-IdNr: DE 214 231 199, Amtsgericht Kiel, HRB 1797 NM

   This e-mail and any files transmitted with it are confidential and
   intended solely for the use of the individual or entity to whom
   they are addressed. If you have received this e-mail in error,
   please notify the sender and delete all copies from your system.