Re: [PATCH v2] mt9v032: Export horizontal and vertical blanking as V4L2 controls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

Laurent Pinchart wrote:
On Saturday 28 July 2012 00:27:23 Sakari Ailus wrote:
On Fri, Jul 27, 2012 at 01:02:04AM +0200, Laurent Pinchart wrote:
On Thursday 26 July 2012 23:54:01 Sakari Ailus wrote:
On Tue, Jul 24, 2012 at 01:10:42AM +0200, Laurent Pinchart wrote:
Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---

  drivers/media/video/mt9v032.c |   36  ++++++++++++++++++++++++++++---
  1 files changed, 33 insertions(+), 3 deletions(-)

Changes since v1:

- Make sure the total horizontal time will not go below 660 when
   setting the horizontal blanking control

- Restrict the vertical blanking value to 3000 as documented in the
   datasheet. Increasing the exposure time actually extends vertical
   blanking, as long as the user doesn't forget to turn auto-exposure
   off...

Does binning either horizontally or vertically affect the blanking
limits? If the process is analogue then the answer is typically "yes".

The datasheet doesn't specify whether binning and blanking can influence
each other.

Vertical binning is often analogue since digital binning would require as
much temporary memory as the row holds pixels. This means the hardware
already does binning before a/d conversion and there's only need to actually
read half the number of rows, hence the effect on frame length.

That will affect the frame length, but why would it affect vertical blanking ?

Frame length == image height + vertical blanking.

The SMIA++ driver (at least) associates the blanking controls to the pixel array subdev. They might be more naturally placed to the source (either binner or scaler) but the width and height (to calculate the frame and line length) are related to the dimensions of the pixel array crop rectangle.

So when the binning configuration changes, that changes the limits for blanking and thus possibly also blanking itself.

It's not directly related to this patch, but the effect of the driver
just exposing one sub-device really shows better now. Besides lacking
the way to specify binning as in the V4L2 subdev API (compose selection
target), the user also can't use the crop bounds selection target to get
the size of the pixel array.

We could either accept this for the time being and fix it later on of
fix it now.

I prefer fixing it right now but admit that this patch isn't breaking
anything, it rather is missing quite relevant functionality to control
the sensor in a generic way.

I'd rather apply this patch first, as it doesn't break anything :-) Fixing
the problem will require discussions, and that will take time.

How so? There's nothing special in this sensor as far as I can see.

Binning/skipping is a pretty common feature in sensors. Exposing two sub-
devices like the SMIA++ driver does is one way to fix the problem, but do
we really want to do that for the vast majority of sensors ?

If we want to expose the sensor's functionality using the V4L2 subdev API
and not a sensor specific API, the answer is "yes". The bottom line is that
we have just one API to configure scaling and cropping and that API
(selections) is the same independently of where the operation is being
performed; whether it's the sensor or the ISP.

If we want to expose two subdevices for every sensor we will need to get both
kernelspace (ISP drivers) and userspace ready for this. I'm not against the
idea, but we need to plan it properly.

I fully agree that this has to be planned properly; e.g. libv4l support for
controlling low level devices required by this is completely missing right
now.

Cheers,

--
Sakari Ailus
sakari.ailus@xxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux