Hi Jacopo, On May 16, 2023 at 09:46:57 +0200, Jacopo Mondi wrote: > Hi Jai, > thanks for testing > > On Mon, May 15, 2023 at 05:25:55PM +0530, Jai Luthra wrote: > > Hi Jacopo, Guoniu, > > > > On 05/05/23 12:46, Jacopo Mondi wrote: > > > While looking at Guoniu Zhou patches I noticed that there were a few cleanups > > > related to the usage of frame_interval fileds for MIPI CSI-2 framerate > > > calculations. > > > > > > No functional changes intended, just cleanups. > > > > > > Guoniu: could you please test these on your setup as well ? A tested-by tag > > > would be useful! > > > > > > > Thanks for the latest fixes! > > > > Testing on my setup (CSI module w/ 2 lanes), I notice two weird issues and > > wonder if you see the same behavior on your setups? > > > > Issue 1 > > ------- > > > > On a fresh boot the sensor streams at 60fps, and checking link_freq from > > v4l2-ctl I get 384Mhz. But G_FRAME_INTERVAL returns 30FPS when using > > `media-ctl -p`: > > [stream:0 fmt:UYVY8_1X16/640x480@1/30] > > the g/s_frame_interval calls are not relevant for MIPI CSI-2 > > I wonder if we should/could return -EINVAL in this case > > > > > > Issue 2 > > ------- > > > > If I manually set the frame interval to @1/60 using media-ctl, and then > > stream it - actual framerate gets reduced to 30FPS: > > Ah this shouldn't happen. s_frame_interval -should not- modify the > timings on a CSI-2 setup > > If not returning -EINVAL, we should at least return immediately > > > > > root@am62xx-evm:~# yavta -s 640x480 -f UYVY /dev/video0 -c5 > > .... > > 0 (0) [-] any 0 614400 B 401.488754 401.488855 12.719 fps ts mono/EoF > > 1 (1) [-] any 1 614400 B 401.522057 401.522147 30.027 fps ts mono/EoF > > 2 (2) [-] any 2 614400 B 401.555434 401.555584 29.961 fps ts mono/EoF > > 3 (3) [-] any 3 614400 B 401.588723 401.588814 30.040 fps ts mono/EoF > > 4 (4) [-] any 4 614400 B 401.622051 401.622135 30.005 fps ts mono/EoF > > Captured 5 frames in 0.212005 seconds (23.584252 fps, 14490164.140730 B/s). > > 8 buffers released. > > > > After setting frame interval to @1/60, the link-frequency got reduced to > > 192Mhz, which probably explains the low framerate. > > > > root@am62xx-evm:~# v4l2-ctl -d /dev/v4l-subdev2 -C link_frequency > > link_frequency: 19 (192000000 0xb71b000) > > > > I will take a deeper look at update_pixel_rate() function to try and fix > > this - but wanted to confirm if this also happens on your CSI sensors? > > > > I also repeated same tests without this series and still saw both issues. In > > fact Issue 2 was worse because the sensor did not stream *at all* if I > > changed frame interval to @1/60. My guess is PATCH 2/2 fixes that by not > > updating the VBLANK using the DVP values. > > Probably yes, and this confirms to me that we should return early in > s_frame_interval if we're CSI-2 (or if this doesn't contradict the > specification even return an error). Oh makes sense, thanks. I can update s_frame_interval to return -EINVAL for CSI-2 as a follow-up series. Will also try if g_frame_interval can report correct framerate (60fps vs 30fps) depending upon the bus type, as I don't think returning -EINVAL would be correct behavior. If that does not work maybe we can unset the ops hooks before registering the subdev with the framework. > > Thanks > j > > > > > For the series: > > > > Tested-by: Jai Luthra <j-luthra@xxxxxx> > > > > Thanks, > > Jai > > > > > Thanks > > > j > > > > > > Jacopo Mondi (2): > > > media: ov5640: Remove unused 'framerate' parameter > > > media: ov5640: Drop dead code using frame_interval > > > > > > drivers/media/i2c/ov5640.c | 17 +---------------- > > > 1 file changed, 1 insertion(+), 16 deletions(-) > > > > > > -- > > > 2.40.1 > > > > > > > >
Attachment:
signature.asc
Description: PGP signature
