Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1599668

Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

Show all headers | View raw

On 03/13/2017 06:55 AM, Philipp Zabel wrote:
> On Mon, 2017-03-13 at 13:27 +0000, Russell King - ARM Linux wrote:
>> On Mon, Mar 13, 2017 at 03:16:48PM +0200, Sakari Ailus wrote:
>>> The vast majority of existing drivers do not implement them nor the user
>>> space expects having to set them. Making that mandatory would break existing
>>> user space.
>>>
>>> In addition, that does not belong to link validation either: link validation
>>> should only include static properties of the link that are required for
>>> correct hardware operation. Frame rate is not such property: hardware that
>>> supports the MC interface generally does not recognise such concept (with
>>> the exception of some sensors). Additionally, it is dynamic: the frame rate
>>> can change during streaming, making its validation at streamon time useless.
>>
>> So how do we configure the CSI, which can do frame skipping?
>>
>> With what you're proposing, it means it's possible to configure the
>> camera sensor source pad to do 50fps.  Configure the CSI sink pad to
>> an arbitary value, such as 30fps, and configure the CSI source pad to
>> 15fps.
>>
>> What you actually get out of the CSI is 25fps, which bears very little
>> with the actual values used on the CSI source pad.
>>
>> You could say "CSI should ask the camera sensor" - well, that's fine
>> if it's immediately downstream, but otherwise we'd need to go walking
>> down the graph to find something that resembles its source - there may
>> be mux and CSI2 interface subdev blocks in that path.  Or we just accept
>> that frame rates are completely arbitary and bear no useful meaning what
>> so ever.
>
> Which would include the frame interval returned by VIDIOC_G_PARM on the
> connected video device, as that gets its information from the CSI output
> pad's frame interval.
>

I'm kinda in the middle on this topic. I agree with Sakari that
frame rate can fluctuate, but that should only be temporary. If
the frame rate permanently shifts from what a subdev reports via
g_frame_interval, then that is a system problem. So I agree with
Phillip and Russell that a link validation of frame interval still
makes sense.

But I also have to agree with Sakari that a subdev that has no
control over frame rate has no business implementing those ops.

And then I agree with Russell that for subdevs that do have control
over frame rate, they would have to walk the graph to find the frame
rate source.

So we're stuck in a broken situation: either the subdevs have to walk
the graph to find the source of frame rate, or s_frame_interval
would have to be mandatory and validated between pads, same as set_fmt.

Steve

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-13 14:20 +0100
  Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 14:30 +0100
    Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Philipp Zabel <p.zabel@pengutronix.de> - 2017-03-13 15:00 +0100
      Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-13 19:10 +0100
        Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-13 22:10 +0100
          Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 22:40 +0100
          Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-14 08:40 +0100
            Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Philipp Zabel <p.zabel@pengutronix.de> - 2017-03-14 11:50 +0100
    Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-13 22:00 +0100
      Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 22:10 +0100

csiph-web