Hi Stefan.

On Tue, 25 Jun 2019 at 17:30, Stefan Wahren <wahre...@gmx.net> wrote:
>
> Hi Dan,
> hi Dave,
>
> Am 25.06.19 um 09:55 schrieb Dan Carpenter:
> > On Tue, Jun 25, 2019 at 12:13:15AM +0200, Stefan Wahren wrote:
> >> The commit 52c4dfcead49 ("Staging: vc04_services: Cleanup in
> >> ctrl_set_bitrate()") changed the return behavior of ctrl_set_bitrate().
> >> This breaks probing of bcm2835-camera:
> >>
> >>     bcm2835-v4l2: mmal_init: failed to set all camera controls: -3
> >>     Cleanup: Destroy video encoder
> >>     Cleanup: Destroy image encoder
> >>     Cleanup: Destroy video render
> >>     Cleanup: Destroy camera
> >>     bcm2835-v4l2: bcm2835_mmal_probe: mmal init failed: -3
> >>     bcm2835-camera: probe of bcm2835-camera failed with error -3
> >>
> >> So restore the old behavior and fix this issue.
> >>
> >> Fixes: 52c4dfcead49 ("Staging: vc04_services: Cleanup in 
> >> ctrl_set_bitrate()")
> >> Signed-off-by: Stefan Wahren <wahre...@gmx.net>
> > I feel like this papers over the issue.  It would be better to figure
> > out why this is failing and fix it properly.  -3 is -ESRCH and when I
> > grep for ESRCH I only see it used in the ioctl so that can't be it.
> >
> > I think it must be -MMAL_MSG_STATUS_EINVAL actually, but it comes from
> > the firmware or something so we can't grep for it.
> yes this is a result from the VC4 firmware, there is nothing i can do
> about it. Even the firmware has been fixed, the driver must be
> compatible with older firmware version.
> > Can we do some more digging to find out why it's failing or otherwise
> > we could add a comment.
> >
> >       /*
> >        * FIXME:  port_parameter_set() sometimes fails with
> >        * -MMAL_MSG_STATUS_EINVAL and we don't know why so we're
> >        * ignoring those errors for now.
> >        *
> >        */
> >       return 0;
>
> I will add a comment and a v4l2_dbg entry.
>
> @Dave
>
> I used a Raspberry Pi 3 with a V1.3 camera and get this with an
> additional v4l2_dbg in ctrl_set_bitrate()
>
> [    1.472805] raspberrypi-firmware soc:firmware: Attached to firmware
> from 2019-03-27 15:48
> ...
> [    7.441639] videodev: Linux video capture interface: v2.00
> [    7.511659] bcm2835_v4l2: module is from the staging directory, the
> quality is unknown, you have been warned.
> ...
> [    8.162504] bcm2835-v4l2: Set fps range to 30000/1000 to 30000/1000
> [    8.163104] bcm2835-v4l2: Set fps range to 30000/1000 to 30000/1000
> [    8.163624] bcm2835-v4l2: Set fps range to 30000/1000 to 30000/1000
> [    8.164395] bcm2835-v4l2: mmal_ctrl:eecd7187 ctrl id:0x98091f ctrl
> val:0 imagefx:0x0 color_effect:false u:0 v:0 ret 0(0)
> [    8.164493] bcm2835-v4l2: ctrl_set_colfx: After: mmal_ctrl:1ec18e37
> ctrl id:0x98092a ctrl val:32896 ret 0(0)
> [    8.165413] bcm2835-v4l2: ctrl_set_bitrate: After: mmal_ctrl:b01a3b48
> ctrl id:0x9909cf ctrl val:10000000 ret -3(-22)
> [    8.165872] bcm2835-v4l2: scene mode selected 0, was 0
> [    8.166249] bcm2835-v4l2: V4L2 device registered as video0 - stills
> mode > 1280x720
> [    8.166596] bcm2835-v4l2: vid_cap - set up encode comp
> [    8.171208] bcm2835-v4l2: JPG - buf size now 786432 was 786432
> [    8.171220] bcm2835-v4l2: vid_cap - cur_buf.size set to 786432
> [    8.171228] bcm2835-v4l2: Set dev->capture.fmt 4745504A, 1024x768,
> stride 0, size 786432
> [    8.171234] bcm2835-v4l2: Broadcom 2835 MMAL video capture ver 0.0.2
> loaded.
>
> Looks like the firmware dislike val:10000000
>
> Any thoughts?

I'd had a quick dig yesterday, but forgot to hit send :-/
It looks like the firmware is getting upset over the ordering of
things in setting the default bitrate and the bitrate mode. Setting
the bitrate mode to the default of VBR fails (return code is ignored)
as the bitrate is 0 at that point. Setting the bitrate then fails as
the firmware default mode is "disabled" (ie specify the Qp
parameters).

Setting the bitrate is also done via the MMAL port format when
enabling the stream, so I believe it's only the setting of the default
values that fails.

I'll sort a firmware fix, but a comment here along the lines you
propose is definitely worth it.
 /*
  * Older firmware versions (pre July 2019) have a bug in handling
  * MMAL_PARAMETER_VIDEO_BIT_RATE that result in the call
  * returning -MMAL_MSG_STATUS_EINVAL.
  * Ignore errors from this call.
  */
  return 0;
(apologies for the formatting).

  Dave

PS Is linux-rpi-kernel actually behaving for other people? I didn't
see this patch when it was submitted, and it isn't showing in the list
archive either.
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to