Hi Mauro,
On 11/8/21 08:41, Mauro Carvalho Chehab wrote:
> Em Mon, 8 Nov 2021 00:39:38 +0100
> Hans de Goede <[email protected]> escreveu:
>
>> Hi,
>>
>> On 10/21/21 11:52, Tsuchiya Yuto wrote:
>>> Thank you for your comment :-)
>>>
>>> First, I need to correct what I said in the previous mail. I later found
>>> that loading only "atomisp" (as well as its dependency,
>>> atomisp_gmin_platform) does not cause this issue.
>>>
>>> What causes this issue is rather, loading sensor drivers (as well as its
>>> dependency, atomisp_gmin_platform).
>>>
>>> These sensor drivers for surface3 are both not upstream, but I made them
>>> as similar as possible to the upstreamed ones. So, I guess this issue
>>> can still be reproducible on some other devices.
>>
>> I've run some test on my own surface3 and the problem is the writing
>> of 0x62 (which becomes just 0x02) to the 0x57 register of the PMIC,
>> writing 0x00 to that after loading the sensor driver makes things work
>> again.
>>
>> I have not had time to investigate this further.
>>
>> I used media-staging + your sensor drivers from:
>> https://github.com/kitakar5525/surface3-atomisp-cameras.git
>>
>> Which was enough to figure this out, but I've not actually gotten
>> either of the cameras working :| I get:
>>
>> [user@fedora nvt]$ ./atomisp-test.sh
>> p0: OPEN video device `/dev/video2'
>
> After the patch that moved the output preview to be the first one,
> you should probably use /dev/video0 here:
Thanks for the hint, but I've not rebased my tree to those latest couple
of patches yet, the same tree does work on the T101HA with /dev/video2 :)
I think this may be a module load ordering issue, I believe that the
sensor drivers need to be loaded before the atomisp driver itself
and on the T101HA we are hitting this "sweet spot", where as on
the surface I was loading the not yet merged sensor drivers manually,
causing them to be loaded later.
I still need to verify this theory, Tsuchiya can you perhaps confirm
that the modules must be loaded in this order?
Regards,
Hans
Em Mon, 8 Nov 2021 08:55:53 +0100
Hans de Goede <[email protected]> escreveu:
> Hi Mauro,
>
> On 11/8/21 08:41, Mauro Carvalho Chehab wrote:
> > Em Mon, 8 Nov 2021 00:39:38 +0100
> > Hans de Goede <[email protected]> escreveu:
> >
> >> Hi,
> >>
> >> On 10/21/21 11:52, Tsuchiya Yuto wrote:
> >>> Thank you for your comment :-)
> >>>
> >>> First, I need to correct what I said in the previous mail. I later found
> >>> that loading only "atomisp" (as well as its dependency,
> >>> atomisp_gmin_platform) does not cause this issue.
> >>>
> >>> What causes this issue is rather, loading sensor drivers (as well as its
> >>> dependency, atomisp_gmin_platform).
> >>>
> >>> These sensor drivers for surface3 are both not upstream, but I made them
> >>> as similar as possible to the upstreamed ones. So, I guess this issue
> >>> can still be reproducible on some other devices.
> >>
> >> I've run some test on my own surface3 and the problem is the writing
> >> of 0x62 (which becomes just 0x02) to the 0x57 register of the PMIC,
> >> writing 0x00 to that after loading the sensor driver makes things work
> >> again.
> >>
> >> I have not had time to investigate this further.
> >>
> >> I used media-staging + your sensor drivers from:
> >> https://github.com/kitakar5525/surface3-atomisp-cameras.git
> >>
> >> Which was enough to figure this out, but I've not actually gotten
> >> either of the cameras working :| I get:
> >>
> >> [user@fedora nvt]$ ./atomisp-test.sh
> >> p0: OPEN video device `/dev/video2'
> >
> > After the patch that moved the output preview to be the first one,
> > you should probably use /dev/video0 here:
>
> Thanks for the hint, but I've not rebased my tree to those latest couple
> of patches yet, the same tree does work on the T101HA with /dev/video2 :)
>
> I think this may be a module load ordering issue, I believe that the
> sensor drivers need to be loaded before the atomisp driver itself
> and on the T101HA we are hitting this "sweet spot", where as on
> the surface I was loading the not yet merged sensor drivers manually,
> causing them to be loaded later.
Could be. The atomisp driver should be using V4L2 async kAPI, in
order to properly register the sensors as their init code finishes
to register.
Right now, the registration logic inside atomisp waits for the first
sensor to be available. So, everything works fine on devices with
just one sensor, like Asus T101, but devices with multiple sensors
may end having just one of them registered.
Regards,
Mauro
On Mon, 2021-11-08 at 08:55 +0100, Hans de Goede wrote:
> Hi Mauro,
>
> On 11/8/21 08:41, Mauro Carvalho Chehab wrote:
> > Em Mon, 8 Nov 2021 00:39:38 +0100
> > Hans de Goede <[email protected]> escreveu:
> >
> > > Hi,
> > >
> > > On 10/21/21 11:52, Tsuchiya Yuto wrote:
> > > > Thank you for your comment :-)
> > > >
> > > > First, I need to correct what I said in the previous mail. I later found
> > > > that loading only "atomisp" (as well as its dependency,
> > > > atomisp_gmin_platform) does not cause this issue.
> > > >
> > > > What causes this issue is rather, loading sensor drivers (as well as its
> > > > dependency, atomisp_gmin_platform).
> > > >
> > > > These sensor drivers for surface3 are both not upstream, but I made them
> > > > as similar as possible to the upstreamed ones. So, I guess this issue
> > > > can still be reproducible on some other devices.
> > >
> > > I've run some test on my own surface3 and the problem is the writing
> > > of 0x62 (which becomes just 0x02) to the 0x57 register of the PMIC,
> > > writing 0x00 to that after loading the sensor driver makes things work
> > > again.
> > >
> > > I have not had time to investigate this further.
> > >
> > > I used media-staging + your sensor drivers from:
> > > https://github.com/kitakar5525/surface3-atomisp-cameras.git
> > >
> > > Which was enough to figure this out, but I've not actually gotten
> > > either of the cameras working :| I get:
> > >
> > > [user@fedora nvt]$ ./atomisp-test.sh
> > > p0: OPEN video device `/dev/video2'
> >
> > After the patch that moved the output preview to be the first one,
> > you should probably use /dev/video0 here:
>
> Thanks for the hint, but I've not rebased my tree to those latest couple
> of patches yet, the same tree does work on the T101HA with /dev/video2 :)
>
> I think this may be a module load ordering issue, I believe that the
> sensor drivers need to be loaded before the atomisp driver itself
> and on the T101HA we are hitting this "sweet spot",
> where as on
> the surface I was loading the not yet merged sensor drivers manually,
> causing them to be loaded later.
>
> I still need to verify this theory, Tsuchiya can you perhaps confirm
> that the modules must be loaded in this order?
Sorry I forgot to add a note about a load order in my sensor driver repo
for the case if you insmod external drivers. Yes, you need to load sensor
drivers _before_ the main atomisp driver.
So, here is the script to load sensor drivers in a proper order. Using
rmmod because as I sent a bug report ("[BUG 4/5] [BUG] media: atomisp:
`modprobe -r` not working well (dup video4linux, ATOMISP_SUBDEV_{0,1})"),
`modprobe -r` does not work well for me.
rmmod atomisp
insmod atomisp-ar0330.ko
insmod atomisp-ov883x.ko
# IIRC, modprobe works but try insmod instead if weird
modprobe atomisp
Here is the insmod script I use for the record:
# load drivers needed for atomisp first for insmod
# for sensor drivers
sudo modprobe media # needed for older LTS
sudo modprobe videodev
sudo modprobe v4l2_common # needed for older LTS
sudo modprobe v4l2_async # if using async_register
# for atomisp
sudo modprobe videobuf-core
sudo modprobe videobuf-vmalloc
# load upstreamed atomisp drivers
sudo insmod upst/atomisp_gmin_platform.ko
sudo insmod upst/surface3/atomisp-ar0330.ko
sudo insmod upst/surface3/atomisp-ov883x.ko
sudo insmod upst/atomisp.ko dbg_level=1 #dyndbg
I'm denylisting all the atomisp drivers and I load these manually
currently to prevent oopses in case I use non-patched kernel.
Btw, this load order issue is indeed another problem even when the sensor
drivers are built as in-tree module. I sometimes see atomisp fails to
register camera device(s) on surface3.
On mipad2, things are worse. It uses regulator driver but sensor drivers
do not wait for the driver to initialize the regulators. This results in
probe failure for sensor drivers.
Regards,
Tsuchiya Yuto