Re: S3C244X/S3C64XX SoC camera host interface driver questions

[Sylwester Nawrocki <sylvester.nawrocki@gmail.com>]

Hi Andrey,

On 11/11/2012 02:26 PM, Andrey Gusakov wrote:
> Hi.
>
> Patch v2 attached. Comments taken into account.

Thanks, I had to rework the S3C-CAMIF subdev controls handling to avoid
races when the control's value is modified in the control framwework and
accessed from interrupt context in the driver.

I've pushed all patches to branch s3c-camif-for-v3.8 at
git://linuxtv.org/snawrocki/media.git

I'd like to squash all the s3c-camif patches before sending upstream,
if you don't mind. And to add your Signed-off at the final patch.
Please let me know if you'd rather keep your patch separate.

I might have introduced bugs in the image effects handling, hopefully
there is none. I couldn't test it though. Could you test that on your
side with s3c64xx ?

I'm planning to send a pull request for the CAMIF driver this week.

>>> I often get "VIDIOC_QUERYCAP: failed: Inappropriate ioctl for device"
>> This is an issue in the v4l2-ctl, it is going to be fixed by adding
>> VIDIOC_SUBDEV_QUERYCAP ioctl for subdevs. It has been just discussed today.
>> I guess you get it when running v4l2-ctl on /dev/v4l-subdev* ?
> Yes.
>>> or "system error: Inappropriate ioctl for device"
>> I think this one is caused by unimplemented VIDIOC_G/S_PARM ioctls
>> at the s3c-camif driver.
>>> Is it because of not implemented set/get framerate func? How this
>> Yes, I think so. ioctls as above.
> Ok. I'll implement this ioctls and see what happens.

They are not supposed to be implemented in the s3c-camif driver.
Instead they should be emulated by a user-space library, based on
the sensor subdev operations.

>>> should work? I mean framerate heavy depend of sensor's settings. So
>>> set/get framerate call to fimc should get/set framerate from sensor.
>>> What is mechanism of such things?
>>
>>
>> With user space subdev API one should control frame interval directly
>> on the sensor subdev device node [1]. For Gstreamer to work with
>> VIDIOC_G/S_PARM ioctls we need a dedicated v4l2 library (possibly with
>> a plugin for s3c-camif, but that shouldn't be needed since it is very
>> simple driver) that will translate those video node ioctls into the
>> subdev node ioctls [2]. Unfortunately such library is still not available.
>>
>>
>>> And same question about synchronizing format of sensor and FIMC pads.
>>> I make ov2640 work, but if did not call media-ctl for sensor, format
>>> of FIMC sink pad and format of sensor source pad different. I think I
>>> missed something, but reading other sources did not help.
>>
>>
>> As I explained previously, s3c-fimc is supposed to synchronize format
>> with the sensor subdev. Have you got pad level get_fmt callback
>> implemented in the ov2640 driver ?
> Yes.
>> Could you post your 'media-ctl -p' output, run right after the system boot ?
>
> Looks like I messed up, after starting formats are the same:
>
> Opening media device /dev/media0ov2640: ov2640_open:1381
>
> Enumerating entities
> Found 4 entities
> Enumerating pads and links
> Media controller API version 0.0.0
>
> Media device information
> ------------------------
> driver          s3c-camif
> model           SAMSUNG S3C6410 CAMIF
> serial
> bus info        platform:%s

I've removed this stray "%s" already.

> hw revision     0x32
> driver version  0.0.0
>
> Device topology
> - entity 1: ov2640 (1 pad, 1 link)
>              type V4L2 subdev subtype Sensor
>              device node name /dev/v4l-subdev0
>          pad0: Source [YUYV2X8 176x144]
>                  ->  "S3C-CAMIF":0 [ENABLED,IMMUTABLE]
>
> - entity 2: S3C-CAMIF (3 pads, 3 links)
>              type V4L2 subdev subtype Unknown
>              device node name /dev/v4l-subdev1
>          pad0: Sink [YUYV2X8 176x144 (0,0)/176x144]
>                  <- "ov2640":0 [ENABLED,IMMUTABLE]
>          pad1: Source [YUYV2X8 176x144]
>                  ->  "camif-codec":0 [ENABLED,IMMUTABLE]
>          pad2: Source [YUYV2X8 176x144]
>                  ->  "camif-preview":0 [ENABLED,IMMUTABLE]
>
> - entity 3: camif-codec (1 pad, 1 link)
>              type Node subtype V4L
>              device node name /dev/video0
>          pad0: Sink
>                  <- "S3C-CAMIF":1 [ENABLED,IMMUTABLE]
>
> - entity 4: camif-preview (1 pad, 1 link)
>              type Node subtype V4L
>              device node name /dev/video1
>          pad0: Sink
>                  <- "S3C-CAMIF":2 [ENABLED,IMMUTABLE]

Looks fine, thanks.