Re: atomisp kernel driver(s)

[Patrik Gfeller <patrik.gfeller@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 7228 bytes --]


On 27.04.20 23:50, Mauro Carvalho Chehab wrote:
> Em Mon, 27 Apr 2020 20:31:31 +0200
> Patrik Gfeller <patrik.gfeller@gmail.com> escreveu:
>> On 26.04.20 18:50, Mauro Carvalho Chehab wrote:
>>> No, you're looking at the wrong place. This should be at the system
>>> board's BIOS, and not at something that the driver would load.
>>>
>>> Just run (as root):
>>>
>>> 	# dmidecode
>>>
>>> and check the name of your board. It should be similar to this:
>>>
>>> 	...
>>> 	System Information
>>> 	        Manufacturer: Intel Corporation
>>> 	        Product Name: (something)
>>>
>>> The "(something)" is the board name. The atomisp driver will seek for
>>> it. So, you need to patch the driver (using the example I gave) in
>>> order for it to look at the right DMI_MATCH() table. Right now,
>>> the driver knows only those:
>>>
>>> 	$ git grep DMI_MATCH drivers/staging/media/atomisp/
>>> 	drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:                       DMI_MATCH(DMI_BOARD_NAME, "BYT-T FFD8"),
>>> 	drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:                       DMI_MATCH(DMI_BOARD_NAME, "T100TA"),
>>> 	drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:                       DMI_MATCH(DMI_BOARD_NAME, "TABLET"),	
>>> 	drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:                       DMI_MATCH(DMI_BOARD_VERSION, "MRD 7"),
>>> 	drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:                       DMI_MATCH(DMI_BOARD_NAME, "ST70408"),
>>> 	drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:                       DMI_MATCH(DMI_BOARD_NAME, "VTA0803"),
>>>
>>> Your asus board should likely use "ASUS", "_ASUS_" or something similar.
>>> So, you should take a look on the patch I sent you and modify it to
>>> match whatever name dmidecode printed.
>>>
>>> See for example this patch:
>>>
>>> 	https://www.spinics.net/lists/linux-media/msg126880.html
>>>
>>> If it finds the right table, it should end by calling gmin_subdev_add(),
>>> with should use DTST table, also from the ACPI table inside the system's BIOS.
>> Now I understood :-). I've made the modifications as you explained and
>> indeed the errors regarding
>>
>> OVTI2680:00_CamClk
>> OVTI2680:00_ClkSrc
>> OVTI2680:00_CsiPort
>> OVTI2680:00_CsiLanes
>>
>> are gone.
> Great! Could you please submit the exact patch you applied? I'll place
> it on my experimental tree:

Here is the patch for the change I made:

diff --git 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index eef7123a586f..081f9be6ec29 100644
--- 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -269,6 +269,15 @@ static struct gmin_cfg_var i8880_vars[] = {
      {},
  };

+static struct gmin_cfg_var asus_vars[] = {
+    {"OVTI2680:00_CsiPort", "0"},
+    {"OVTI2680:00_CsiLanes", "1"},
+    {"OVTI2680:00_CsiFmt", "15"},
+    {"OVTI2680:00_CsiBayer", "0"},
+    {"OVTI2680:00_CamClk", "1"},
+    {},
+};
+
  static const struct dmi_system_id gmin_vars[] = {
      {
          .ident = "BYT-T FFD8",
@@ -306,6 +315,13 @@ static const struct dmi_system_id gmin_vars[] = {
          },
          .driver_data = i8880_vars,
      },
+    {
+        .ident = "T101HA",
+        .matches = {
+            DMI_MATCH(DMI_BOARD_NAME, "T101HA"),
+        },
+        .driver_data = asus_vars,
+    },
      {}
  };

>> Now we have the following in dmesg:
>>
>> [    8.815960] kernel: mc: Linux media interface: v0.10
>> [    8.858458] kernel: videodev: Linux video capture interface: v2.00
>> [    8.876864] kernel: input: Intel HID events as
>> /devices/pci0000:00/INT33D5:00/input/input16
>> [    8.887625] kernel: 8086228A:01: ttyS5 at MMIO 0x91a1f000 (irq = 40,
>> base_baud = 2764800) is a 16550A
>> [    8.894655] kernel: dw_dmac INTL9C60:01: DesignWare DMA Controller, 8
>> channels
>> [    8.929818] kernel: atomisp_ov2680: module is from the staging
>> directory, the quality is unknown, you have been warned.
>> [    8.989630] kernel: ov2680 i2c-OVTI2680:00: gmin: initializing
>> atomisp module subdev data.PMIC ID 1
>> [    8.989887] kernel: ov2680 i2c-OVTI2680:00: supply V1P8SX not found,
>> using dummy regulator
>> [    8.989954] kernel: ov2680 i2c-OVTI2680:00: supply V2P8SX not found,
>> using dummy regulator
> Did you apply this patch also?
>
> 	https://git.linuxtv.org/mchehab/experimental.git/commit/?h=atomisp&id=898564642042fdd136a16c3ee120a00061c13940
>
> I guess this would get rid of the two above warnings.
>
The patch was already applied when I did the test width the driver - the 
warnings are also present with it.

But if I read the code correctly this is expected, as we try to get 
those regulators in any case - only if we have an error on two of them 
we try the "Regulator1p8v" & "Regulator2p8v". As we do not see warnings 
for those two regulators I assume this worked.

f (pmic_id == PMIC_REGULATOR) {
         gmin_subdevs[i].v1p8_reg = regulator_get(dev, "V1P8SX");
         gmin_subdevs[i].v2p8_reg = regulator_get(dev, "V2P8SX");
         gmin_subdevs[i].v1p2_reg = regulator_get(dev, "V1P2A");
         gmin_subdevs[i].v2p8_vcm_reg = regulator_get(dev, "VPROG4B");

         /*
          * Based on DTST dumps on newer Atom E3800 devices, it seems that
          * the regulators data now have new names.
          */
         if (IS_ERR(gmin_subdevs[i].v1p8_reg))
             gmin_subdevs[i].v1p8_reg = regulator_get(dev, "Regulator1p8v");

         if (IS_ERR(gmin_subdevs[i].v2p8_reg))
             gmin_subdevs[i].v2p8_reg = regulator_get(dev, "Regulator2p8v");

>> [    8.989977] kernel: ov2680 i2c-OVTI2680:00: supply V1P2A not found,
>> using dummy regulator
>> [    8.989998] kernel: ov2680 i2c-OVTI2680:00: supply VPROG4B not found,
>> using dummy regulator
>> [    9.033505] kernel: ov2680 i2c-OVTI2680:00: unable to set PMC rate 1
>> [    9.061511] kernel: ov2680 i2c-OVTI2680:00: camera pdata: port: 0
>> lanes: 1 order: 00000002
>> [    9.065178] kernel: ov2680 i2c-OVTI2680:00: sensor_revision id =
>> 0x2680, rev= 0
>> [    9.071095] kernel: ov2680 i2c-OVTI2680:00: register atomisp i2c
>> module type 1
> We need to double check if everything is ok on the above.
>
> That's said, with the current code, I don't expect ISP2401 to work out
> of the box, as the Kernel is set for ISP2400. I'm trying to address
> this on my spare time.
Thanks - just let me know if I shall test anything on my side.
>> Laurent explained me how to enable internal debug messages. I'll read
>> more about this to understand how to use it and hope this will give a
>> more detailed insight.
>>
>>
>> great to see some progress :-),
> Yes!
>
>> thanks and kind regards,
>>
>> Patrik
>>
>>
>>> Thanks,
>>> Mauro
>
>
> Thanks,
> Mauro

with kind regards,

Patrik


[-- Attachment #2: asus.patch --]
[-- Type: text/x-patch, Size: 980 bytes --]

diff --git a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index eef7123a586f..081f9be6ec29 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -269,6 +269,15 @@ static struct gmin_cfg_var i8880_vars[] = {
 	{},
 };
 
+static struct gmin_cfg_var asus_vars[] = {
+	{"OVTI2680:00_CsiPort", "0"},
+	{"OVTI2680:00_CsiLanes", "1"},
+	{"OVTI2680:00_CsiFmt", "15"},
+	{"OVTI2680:00_CsiBayer", "0"},
+	{"OVTI2680:00_CamClk", "1"},
+	{},
+};
+
 static const struct dmi_system_id gmin_vars[] = {
 	{
 		.ident = "BYT-T FFD8",
@@ -306,6 +315,13 @@ static const struct dmi_system_id gmin_vars[] = {
 		},
 		.driver_data = i8880_vars,
 	},
+	{
+		.ident = "T101HA",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "T101HA"),
+		},
+		.driver_data = asus_vars,
+	},
 	{}
 };