Re: atomisp kernel driver(s)

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

On 29.04.20 01:13, Mauro Carvalho Chehab wrote:
> Em Tue, 28 Apr 2020 19:59:28 +0200
> Patrik Gfeller<patrik.gfeller@gmail.com>  escreveu:
>
>> 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,
>> +    },
>>        {}
>>    };
> Thanks for testing it. Just applied this patch upstream, together with a
> bunch of other cleanup patches.
>
>>>> 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.
> Ok. Yet, I found an issue on that patch. Just merged an improvement.
>
> Could you please test it again?

Of course - I pulled the changes and recompiled the kernel. This is  
what we see if I reboot the system:

Apr 29 19:49:04 ASUS kernel: [    8.821277] atomisp_ov2680: loading 
out-of-tree module taints kernel.
Apr 29 19:49:04 ASUS kernel: [    8.824016] atomisp_ov2680: module is 
from the staging directory, the quality is unknown, you have been warned.
Apr 29 19:49:04 ASUS kernel: [    8.945856] ov2680 i2c-OVTI2680:00: 
gmin: initializing atomisp module subdev data.PMIC ID 1
Apr 29 19:49:04 ASUS kernel: [    8.949070] ov2680 i2c-OVTI2680:00: 
supply V1P8SX not found, using dummy regulator
Apr 29 19:49:04 ASUS kernel: [    8.952036] ov2680 i2c-OVTI2680:00: 
supply V2P8SX not found, using dummy regulator
Apr 29 19:49:04 ASUS kernel: [    8.954893] ov2680 i2c-OVTI2680:00: 
supply V1P2A not found, using dummy regulator
Apr 29 19:49:04 ASUS kernel: [    8.957849] ov2680 i2c-OVTI2680:00: 
supply VPROG4B not found, using dummy regulator
Apr 29 19:49:04 ASUS kernel: [    9.013717] ov2680 i2c-OVTI2680:00: 
unable to set PMC rate 1
Apr 29 19:49:04 ASUS kernel: [    9.041777] ov2680 i2c-OVTI2680:00: 
camera pdata: port: 0 lanes: 1 order: 00000002
Apr 29 19:49:04 ASUS kernel: [    9.048368] ov2680 i2c-OVTI2680:00: 
sensor_revision id = 0x2680, rev= 0
Apr 29 19:49:04 ASUS kernel: [    9.051525] ov2680 i2c-OVTI2680:00: 
register atomisp i2c module type 1

I've also added the following boot parameter to make sure we get all 
debug messages from the module: dyndbg="module atomisp_ov2680 +pm" (as 
explained by Laurent)

I've checked the code of atomisp_ov2680 and there are some dev_dbg 
calls, but either I did the configuration not correct, or we do not 
reach those lines yet (or I looked at the wrong place; I checked dmesg 
and kern.log).

>> 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.
> No. It probably returned a valid "dummy" regulator. That's not what
> we want.
>
> There are still some things that could be missing for it to work
> properly with ISP2401. I'm trying to do some cleanups in order to
> be sure that everything needed for isp2401 will be there.
Just let me know if I shall try something.
>
> Thanks,
> Mauro

kind regards,
Patrik