Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Patrik Gfeller <patrik.gfeller@gmail.com>,
	Francescodario Cuzzocrea 
	<francescodario.cuzzocrea@mail.polimi.it>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>
Subject: Re: [GIT PULL] Ressurect the atomisp staging driver
Date: Sat, 23 May 2020 10:00:00 +0200
Message-ID: <20200523100000.1131bd29@coco.lan> (raw)
In-Reply-To: <20200522134203.0fe139d6@coco.lan>

Em Fri, 22 May 2020 13:42:03 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> Em Fri, 22 May 2020 12:46:07 +0200
> Hans de Goede <hdegoede@redhat.com> 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

Btw, this can also be useful:

	https://git.linuxtv.org/mchehab/experimental.git/log/?h=yocto_intel_aero_ported_v2

This is basically the Yocto Aero patchset from:

	https://github.com/intel-aero/meta-intel-aero-base

applied on the top of Kernel 4.4.76 and then ported to 
Kernel 5.7-rc2, making it run there.

On such version, I tried to preserve the patch history as much
as possible and minimize the changes, while not touching at the media
framework. This version contains 3 new I2C sensor drivers.

From the new sensors, I ported only the ov8858 code to be built
on the top of v5.7-rc2, but aiming another device I have here,
using ipu3. So, it got removed from all atomisp-dependent code.

Thanks,
Mauro

  reply index

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-01 19:57 Mauro Carvalho Chehab
2020-05-02  6:03 ` Greg KH
2020-05-20  7:48   ` Mauro Carvalho Chehab
2020-05-03  9:19 ` Francescodario Cuzzocrea
2020-05-03 10:07   ` Mauro Carvalho Chehab
2020-05-03 15:32   ` Mauro Carvalho Chehab
     [not found]     ` <CADnVkj96W0QfthukTKQ0a-i2fH1buooH3BEgfy22J9H9=_PcKA@mail.gmail.com>
2020-05-03 16:07       ` Patrik Gfeller
2020-05-04  8:16         ` Patrik Gfeller
2020-05-04  8:49           ` Mauro Carvalho Chehab
2020-05-04 10:45             ` Patrik Gfeller
2020-05-04 12:08               ` Mauro Carvalho Chehab
2020-05-04 13:44                 ` Patrik Gfeller
2020-05-15  8:32                   ` Mauro Carvalho Chehab
2020-05-15  9:10                     ` Hans de Goede
2020-05-15  9:42                       ` Mauro Carvalho Chehab
2020-05-19  7:39                         ` Mauro Carvalho Chehab
2020-05-19 17:36                           ` Mauro Carvalho Chehab
2020-05-22 10:46                             ` Hans de Goede
2020-05-22 11:42                               ` Mauro Carvalho Chehab
2020-05-23  8:00                                 ` Mauro Carvalho Chehab [this message]
2020-05-27 16:23                                   ` Hans de Goede
2020-05-27 17:36                                     ` Mauro Carvalho Chehab
2020-05-15  9:37                   ` Ressurect the atomisp staging driver - current progress Mauro Carvalho Chehab
2020-05-06 15:10                 ` [GIT PULL] Ressurect the atomisp staging driver Mauro Carvalho Chehab
2020-05-03  9:23 ` bosconovic
2020-05-16 13:23 [GIT,PULL] " Finn Rayment
     [not found] ` <20200516115459.4a24063b@coco.lan>
2020-05-16 19:53   ` Finn Rayment
2020-05-19 17:37     ` Mauro Carvalho Chehab

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200523100000.1131bd29@coco.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=francescodario.cuzzocrea@mail.polimi.it \
    --cc=hdegoede@redhat.com \
    --cc=linux-media@vger.kernel.org \
    --cc=patrik.gfeller@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git