Em Fri, 22 May 2020 12:46:07 +0200
Hans de Goede <hdegoede@xxxxxxxxxx> escreveu:
> Hi,
>
> On 5/19/20 7:36 PM, Mauro Carvalho Chehab wrote:
>
> <snip>
>
> > I did a lot of progress today. After identified the above bug, which
> > was turning down the ISP device, causing the firmware load to fail
> > (as the turn on code is not OK), I solved several other issues there.
> >
> > The current status is that:
> >
> > - the ISP firmware is properly loading;
> > - it can properly communicate with the camera sensor;
> > - Userspace can read video controls (tested with v4l2-ctl and qv4l2);
> > - set a video format is now working;
> > - buffers are being queued, and per-frame IRQs are arriving.
> >
> > I did a really quick test today of trying to get a video from it,
> > using a simple tool I developed for such kind of tests (v4l2grab
> > from v4l-utils package, changed to work with the only format that
> > my camera sensor supports). This tool needs uses a bare minimum
> > set of ioctls, with would avoid hitting a bug somewhere else.
> >
> > Running it makes the device to start receiving frames from the
> > hardware. Yet, there's something wrong at the part with stores
> > the data into the video frame buffers. This driver has a weird
> > mm/DMA code, based on a fork of get_user_pages() taken probably
> > during Kernel 3.10 old days.
> >
> > Addressing it has a high chance of grabbing some image from it.
> >
> > Ok, driver is at staging quality: there are lots of crap there that
> > will require changes, but it seems we're getting there.
>
> This is very good news. Hopefully you will get an actual image
> out of these soon. That would be awesome.
>
> I happened to notice an advert for a second-hand Asus T101HA
> locally, for not too much money. So now I'm the happy owner of
> an Asus T101HA myself.
Great!
> So once you have something working I can
> try to reproduce your work on identical hardware then as time
> permits help with cleaning things up. Although I might focus
> at first on trying to get your work to run on more Cherry Trail
> based models, to find out what bits we need to make configurable
> and if we can get the info from ACPI or if we need to have a
> DMI based table with model specific info.
The main ACPI related code is at this file:
drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
Originally, this was a platform driver. Now, it is just an ancillary
driver.
Inside the sensor drivers, there's just the ACPI tables, in order
for them to be probed.
I updated the driver's TODO list, but there are probably more to be
said than what's there.
Let me list the things I remember that should be done:
1) The atomisp doesn't rely at the usual i2c stuff to discover the
sensors. Instead, it calls a function from atomisp_gmin_platform.c.
There are some hacks I added there for it to wait for sensors to be
probed (with a timeout of 2 seconds or so). This should be converted
to the usual way, using V4L2 async subdev framework to wait for cameras
to be probed;
2) The Asus T101HA support (and other board-specific data) uses the a
DMI match table to retrieve sensor's data, using hard-coded values.
I did some research last week: it sounds possible to retrieve those
data directly from ACPI via this _DSM table, associated with CAM1
sensor (UUID: dc2f6c4f-045b-4f1d-97b9-882a6860a4be):
Name (C1CD, Buffer (0x0220){})
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
If ((Arg0 == ToUUID ("dc2f6c4f-045b-4f1d-97b9-882a6860a4be")))
{
Local0 = Package (0x12)
{
"CamId",
"ov2680",
"CamType",
"1",
"CsiPort",
"0",
"CsiLanes",
"1",
"CsiFmt",
"15",
"CsiBayer",
"0",
"CamClk",
"1",
"Regulator1p8v",
"0",
"Regulator2p8v",
"0"
}
Return (Local0)
}
The code there at atomisp_gmin_platform has an EFI parser, but it assumes
that the information would be stored on a different way.
Kernel has support for reading data from _DSM, via acpi_evaluate_dsm().
I suspect that it should be doable to use such infra and remove the
TA101HA DMI match. This will likely allow covering more devices that
could also be using the same EFI UUID.
3) Switch the driver to use pm_runtime stuff. Right now, it probes the
existing PMIC code and sensors call it directly.
4) There's a problem at the sensor drivers (I hacked the ov2880
to avoid that): when trying to set a video format, atomisp calls
the sensor drivers with the sensor turned off. This causes them
to fail.
I guess the right fix would be to power on the sensor when a video
device is opened (or at the first VIDIOC_ ioctl - except for
VIDIOC_QUERYCAP), powering it down at close() syscall.
5) There are several issues related to memory management, causing
crashes. This is probably something that we need to fix asap.
The atomisp splits the memory management on three separate regions:
- dynamic pool;
- reserved pool;
- generic pool
The code implementing it is at:
drivers/staging/media/atomisp/pci/hmm/
It also has a separate code for managing DMA buffers at:
drivers/staging/media/atomisp/pci/mmu/
The code there is really dirty, ugly and probably wrong. I fixed
one bug there already, but the best would be to just trash it and use
something else. Maybe the code from the newer intel driver could
serve as a model:
drivers/staging/media/ipu3/ipu3-mmu.c
But converting it to use something like that is painful and may
cause some breakages.
6) there is some issue at the frame receive logic. This is currently
my main focus.
Btw, this is the branch I'm using on my tests:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=atomisp_v3
It has basically the stuff from linux-media, plus the ACPI patch
that enables the OpRegion:
https://git.linuxtv.org/mchehab/experimental.git/commit/?h=atomisp_v3&id=d8613fcbb3c9cb21b6818b0245e320054ecb5deb
(I didn't cherry-picked the Kconfig ones here, since I have already
everything enabled at the .config file I'm using here).
Besides that, it has some patches that I'm working to address (5) and
(6).
PS.: I'm constantly rebasing the stuff there (specially the patches with
weird names, like "foo" and "HACK").
Thanks,
Mauro
