Re: [PATCHv3] [media] rcar-vin: add Renesas R-Car VIN driver

2016-04-03 Thread Hans Verkuil 

Hi Niklas,

Some quick review comments...

On 04/02/2016 10:52 AM, Niklas Söderlund wrote:

A V4L2 driver for Renesas R-Car VIN driver that do not depend on
soc_camera. The driver is heavily based on its predecessor and aims to
replace it.

Signed-off-by: Niklas Söderlund 
---

The driver is tested on Koelsch and can do streaming using qv4l2 and
grab frames using yavta. It passes a v4l2-compliance (git master) run
without any failures, see bellow for output. Some issues I know about
but will have to wait for future work in other patches.
  - One can not bind/unbind the subdevice and continue using the driver.
  - Do not support FIELD_ALTERNATE.
  - Suggested compat string "renesas,rcar-gen2-vin" is not included. Will
address this in a separate patch together with gen3.

The goal is to replace the soc_camera driver completely to prepare for
Gen3 enablement. I have therefor chosen to inherit the
CONFIG_VIDEO_RCAR_VIN name for this new driver and renamed the
soc_camera driver CONFIG_VIDEO_RCAR_VIN_OLD.

* Changes since v2
- Fix review comments from Hans Verkuil, thanks!
 - Update description in Kconfig
 - Drop V4L2_SEL_TGT_COMPOSE_PADDED
 - Wrong size for NV16 image
 - Copy ycbcr_enc and xfer_func when keeping old format.
 - Add vidioc_cropcap
 - Return -ENOBUFS in start_streaming to signal more buffers are
   needed instead of sleeping in a critical section...
 - Move all v4l2 ioctls and file ops to rcar-v4l2.c (and as a follow
   up moved all HW functions to rcar-dma.c to increase readability).
- Fixed RGB formats 's/V4L2_PIX_FMT_RGB555X/V4L2_PIX_FMT_XRGB555' and
   's/V4L2_PIX_FMT_RGB32/V4L2_PIX_FMT_XBGR32'. This was an error carried
   over from soc-camera dirver, whit this fix I get correct colors in
   qv4l2.
- Rework how media bus type and flags are handled. Instead of defining
   own values and a unsigned int use struct v4l2_mbus_config to store the
   configuration parsed from DT.
- Remove duplicated code from the v4l2_file_operations release code
   path. There is no need to try and stop the streaming from here. If
   start_streaming have been called stop_streaming will be called by the
   framework stopping the streaming.
- Remove all special checks for the chip RCAR_E1. There are no compat
   string that will select this chip model. Neither for this driver or
   its predecessor in soc-camera.
- Force an width alignment of 32 if the NV16 format is used due to HW
   limitation.

* Changes since RFC/PATCH
- Fixed review comments from Hans Verkuil, thanks for reviewing.
- Added vidioc_[gs]_selection crop and composition is supported. Thanks
   Laurent for taking the time and explaining to me how to do
   composition.
- Reworked the DMA flow to better support single and continues frame
   grabbing mode.
- Dropped a lot of the formats that was ported from soc_camera, once I
   looked at it in a working driver it was obvious that the rcar_vin
   soc_camera driver did not support them.
- Added better comments for the core structs
- Fixed copyright in file headers
- A lot more testing.

I have made a few small additions to the adv7180.c driver while doing
this driver but are posted in a separate patch. For completeness I
included the output of v4l2-compliance both with and with out the
adv7180 enhancements. The adv7180 additions enables the std and tvnorms
code paths so it tests a bit more of this driver.

There is a failure reported here but it's a false positive and is
addressed in Hans Verkuil series '[PATCHv2 0/2] v4l2-ioctl: cropcap
improvement'. If I apply that series to my tree the failure goes away.






diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
b/drivers/media/platform/rcar-vin/rcar-dma.c
new file mode 100644
index 000..85a1be7
--- /dev/null
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c





+static bool rvin_fill_hw(struct rvin_dev *vin)
+{
+   int slot, limit;
+
+   limit = vin->continuous ? HW_BUFFER_NUM : 1;
+
+   for (slot = 0; slot < limit; slot++)
+   if (!rvin_fill_hw_slot(vin, slot))
+   return false;
+   return true;
+}
+
+static irqreturn_t rvin_irq(int irq, void *data)
+{
+   struct rvin_dev *vin = data;
+   u32 int_status;
+   int slot;
+   unsigned int sequence, handled = 0;
+   unsigned long flags;
+
+   spin_lock_irqsave(>qlock, flags);
+
+   int_status = rvin_get_interrupt_status(vin);
+   if (!int_status)
+   goto done;
+
+   rvin_ack_interrupt(vin);
+   handled = 1;
+
+   /* Nothing to do if capture status is 'STOPPED' */
+   if (vin->state == STOPPED) {
+   vin_dbg(vin, "IRQ state stopped\n");
+   goto done;
+   }
+
+   /* Wait for HW to shutdown */
+   if (vin->state == STOPPING) {
+   if (!rvin_capture_active(vin)) {
+   vin_dbg(vin, "IRQ hw stopped and we are stopping\n");
+

[PATCHv3] [media] rcar-vin: add Renesas R-Car VIN driver

2016-04-02 Thread Niklas Söderlund 
A V4L2 driver for Renesas R-Car VIN driver that do not depend on
soc_camera. The driver is heavily based on its predecessor and aims to
replace it.

Signed-off-by: Niklas Söderlund 
---

The driver is tested on Koelsch and can do streaming using qv4l2 and
grab frames using yavta. It passes a v4l2-compliance (git master) run
without any failures, see bellow for output. Some issues I know about
but will have to wait for future work in other patches.
 - One can not bind/unbind the subdevice and continue using the driver.
 - Do not support FIELD_ALTERNATE.
 - Suggested compat string "renesas,rcar-gen2-vin" is not included. Will
   address this in a separate patch together with gen3.

The goal is to replace the soc_camera driver completely to prepare for
Gen3 enablement. I have therefor chosen to inherit the
CONFIG_VIDEO_RCAR_VIN name for this new driver and renamed the
soc_camera driver CONFIG_VIDEO_RCAR_VIN_OLD.

* Changes since v2
- Fix review comments from Hans Verkuil, thanks!
- Update description in Kconfig
- Drop V4L2_SEL_TGT_COMPOSE_PADDED
- Wrong size for NV16 image
- Copy ycbcr_enc and xfer_func when keeping old format.
- Add vidioc_cropcap
- Return -ENOBUFS in start_streaming to signal more buffers are
  needed instead of sleeping in a critical section...
- Move all v4l2 ioctls and file ops to rcar-v4l2.c (and as a follow
  up moved all HW functions to rcar-dma.c to increase readability).
- Fixed RGB formats 's/V4L2_PIX_FMT_RGB555X/V4L2_PIX_FMT_XRGB555' and
  's/V4L2_PIX_FMT_RGB32/V4L2_PIX_FMT_XBGR32'. This was an error carried
  over from soc-camera dirver, whit this fix I get correct colors in
  qv4l2.
- Rework how media bus type and flags are handled. Instead of defining
  own values and a unsigned int use struct v4l2_mbus_config to store the
  configuration parsed from DT.
- Remove duplicated code from the v4l2_file_operations release code
  path. There is no need to try and stop the streaming from here. If
  start_streaming have been called stop_streaming will be called by the
  framework stopping the streaming.
- Remove all special checks for the chip RCAR_E1. There are no compat
  string that will select this chip model. Neither for this driver or
  its predecessor in soc-camera.
- Force an width alignment of 32 if the NV16 format is used due to HW
  limitation.

* Changes since RFC/PATCH
- Fixed review comments from Hans Verkuil, thanks for reviewing.
- Added vidioc_[gs]_selection crop and composition is supported. Thanks
  Laurent for taking the time and explaining to me how to do
  composition.
- Reworked the DMA flow to better support single and continues frame
  grabbing mode.
- Dropped a lot of the formats that was ported from soc_camera, once I
  looked at it in a working driver it was obvious that the rcar_vin
  soc_camera driver did not support them.
- Added better comments for the core structs
- Fixed copyright in file headers
- A lot more testing.

I have made a few small additions to the adv7180.c driver while doing
this driver but are posted in a separate patch. For completeness I
included the output of v4l2-compliance both with and with out the
adv7180 enhancements. The adv7180 additions enables the std and tvnorms
code paths so it tests a bit more of this driver.

There is a failure reported here but it's a false positive and is
addressed in Hans Verkuil series '[PATCHv2 0/2] v4l2-ioctl: cropcap
improvement'. If I apply that series to my tree the failure goes away.

(without adv7180 enhancements)
# ./v4l2-compliance -d 27 -s -f
Driver Info:
Driver name   : rcar_vin
Card type : R_Car_VIN
Bus info  : platform:e6ef1000.video
Driver version: 4.6.0
Capabilities  : 0x8521
Video Capture
Read/Write
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0521
Video Capture
Read/Write
Streaming
Extended Pix Format

Compliance test for device /dev/video27 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK

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)