Re: [PATCH v5 14/20] platform/x86: intel_atomisp2: Move to intel sub-directory

[Hans de Goede]

Hi Robert,

I have just Cc-ed you on a series which make streaming video
from the mt9m114 sensor on the T100TA work.

On 6-Oct-24 6:54 PM, Robert Mast wrote:
> Hi,
> 
> In an attempt to put the camera led on the T100ta (baytrail) back on to repeat the success of Mauro on the ov2680 on cherry trail for the MT9M114 I compiled this led-driver in the media-atomisp branch of hgoede to try to put the camera back on by writing 1 to /sys/class/leds/atomisp::camera/brightness
> 
> However nothing happened. At boot the camera led is on, but with my Debian 12.7 install of bootia32.efi and the like the camera is already off at the boot menu, so there is another trick used to shut it off even before the kernel has been boot. Does any of you have any clue how to get that camera back on for use in v4l? Mauro did so on the 20th of may 2020 for the ov2680 on Cherry Trail with a little help of Intel.
> 
> Or am I just too short sighted for trying to get light in /dev/video0 because of the lack of libcamera support?
> 
> Should v4l2-ctl have an answer about /dev/video0 as long as there is no userspace-driver, or should I test differently?
> 
> Here a blog of my progress: https://github.com/jfwells/linux-asus-t100ta/issues/4 with some dmesg and config dumps.

About the [atomisp-]mt9m114 driver vs the intel_atomisp2_led driver,
they control different GPIOs.

There appear to be 3 different GPIOs one to force the LED off and
2 for turning the sensor on/off (reset and powerdown signals).

The LED is initially off because the GPIO forcing it off is
initially set to force it off. But as soon as asus-wmi loads
and call the ACPI WMI init method the ACPI code run on init
changes the GPIO causing the LED to turn on.

The main purpose of intel_atomisp2_led is to turn it back off,
I have now learned that this is only necessary because so far
we have been lacking a working driver for the mt9m114. Once
that has a driver and it is properly power-managed the LED
will only turn on when the sensor is on (if we don't have
intel_atomisp2_led).

With intel_atomisp2_led the LED will stay off when using
the sensor except when its brightness is set to 1, then it
will follow the sensor state (with a working mt9m114 driver).

The sensor will work fine independent of the state of
the GPIO which can force the LED off (but not on it
is only on when not forced off and the sensor is
powered). So for now if the intel_atomisp2_led driver
is build / used does not matter.

We can figure out what to do with the intel_atomisp2_led
driver once we have proper support for the mt9m114.

Regards,

Hans


p.s.

I did also see your private email to me, but I'm rather swamped with work so I did not
get around to looking at this until today.



> Op 20-8-2021 om 13:04 schreef Andy Shevchenko:
>> From: Kate Hsuan <hpa@xxxxxxxxxx>
>>
>> Move Intel AtomISP v2 drivers to intel sub-directory
>> to improve readability.
>>
>> Signed-off-by: Kate Hsuan <hpa@xxxxxxxxxx>
>> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>> ---
>>   MAINTAINERS                                   |  4 +-
>>   drivers/platform/x86/Kconfig                  | 34 ---------------
>>   drivers/platform/x86/Makefile                 |  2 -
>>   drivers/platform/x86/intel/Kconfig            |  1 +
>>   drivers/platform/x86/intel/Makefile           |  1 +
>>   drivers/platform/x86/intel/atomisp2/Kconfig   | 43 +++++++++++++++++++
>>   drivers/platform/x86/intel/atomisp2/Makefile  |  9 ++++
>>   .../atomisp2/led.c}                           |  0
>>   .../atomisp2/pm.c}                            |  0
>>   9 files changed, 56 insertions(+), 38 deletions(-)
>>   create mode 100644 drivers/platform/x86/intel/atomisp2/Kconfig
>>   create mode 100644 drivers/platform/x86/intel/atomisp2/Makefile
>>   rename drivers/platform/x86/{intel_atomisp2_led.c => intel/atomisp2/led.c} (100%)
>>   rename drivers/platform/x86/{intel_atomisp2_pm.c => intel/atomisp2/pm.c} (100%)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index cf29f7154889..429b8b5c5283 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9212,13 +9212,13 @@ INTEL ATOMISP2 DUMMY / POWER-MANAGEMENT DRIVER
>>   M:    Hans de Goede <hdegoede@xxxxxxxxxx>
>>   L:    platform-driver-x86@xxxxxxxxxxxxxxx
>>   S:    Maintained
>> -F:    drivers/platform/x86/intel_atomisp2_pm.c
>> +F:    drivers/platform/x86/intel/atomisp2/pm.c
>>     INTEL ATOMISP2 LED DRIVER
>>   M:    Hans de Goede <hdegoede@xxxxxxxxxx>
>>   L:    platform-driver-x86@xxxxxxxxxxxxxxx
>>   S:    Maintained
>> -F:    drivers/platform/x86/intel_atomisp2_led.c
>> +F:    drivers/platform/x86/intel/atomisp2/led.c
>>     INTEL BIOS SAR INT1092 DRIVER
>>   M:    Shravan S <s.shravan@xxxxxxxxx>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 55820fc60a63..81eaa00983bd 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -668,40 +668,6 @@ config THINKPAD_LMI
>>     source "drivers/platform/x86/intel/Kconfig"
>>   -config INTEL_ATOMISP2_LED
>> -    tristate "Intel AtomISP2 camera LED driver"
>> -    depends on GPIOLIB && LEDS_GPIO
>> -    help
>> -      Many Bay Trail and Cherry Trail devices come with a camera attached
>> -      to Intel's Image Signal Processor. Linux currently does not have a
>> -      driver for these, so they do not work as a camera. Some of these
>> -      camera's have a LED which is controlled through a GPIO.
>> -
>> -      Some of these devices have a firmware issue where the LED gets turned
>> -      on at boot. This driver will turn the LED off at boot and also allows
>> -      controlling the LED (repurposing it) through the sysfs LED interface.
>> -
>> -      Which GPIO is attached to the LED is usually not described in the
>> -      ACPI tables, so this driver contains per-system info about the GPIO
>> -      inside the driver, this means that this driver only works on systems
>> -      the driver knows about.
>> -
>> -      To compile this driver as a module, choose M here: the module
>> -      will be called intel_atomisp2_led.
>> -
>> -config INTEL_ATOMISP2_PM
>> -    tristate "Intel AtomISP2 dummy / power-management driver"
>> -    depends on PCI && IOSF_MBI && PM
>> -    depends on !INTEL_ATOMISP
>> -    help
>> -      Power-management driver for Intel's Image Signal Processor found on
>> -      Bay Trail and Cherry Trail devices. This dummy driver's sole purpose
>> -      is to turn the ISP off (put it in D3) to save power and to allow
>> -      entering of S0ix modes.
>> -
>> -      To compile this driver as a module, choose M here: the module
>> -      will be called intel_atomisp2_pm.
>> -
>>   config INTEL_HID_EVENT
>>       tristate "INTEL HID Event"
>>       depends on ACPI
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index 1b1f4337a77f..6c682114e4fe 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -71,8 +71,6 @@ obj-$(CONFIG_THINKPAD_LMI)    += think-lmi.o
>>   # Intel
>>   obj-$(CONFIG_X86_PLATFORM_DRIVERS_INTEL)        += intel/
>>   -obj-$(CONFIG_INTEL_ATOMISP2_LED)    += intel_atomisp2_led.o
>> -obj-$(CONFIG_INTEL_ATOMISP2_PM)        += intel_atomisp2_pm.o
>>   obj-$(CONFIG_INTEL_HID_EVENT)        += intel-hid.o
>>   obj-$(CONFIG_INTEL_INT0002_VGPIO)    += intel_int0002_vgpio.o
>>   obj-$(CONFIG_INTEL_OAKTRAIL)        += intel_oaktrail.o
>> diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
>> index 0c044b31e822..e8e1bad38113 100644
>> --- a/drivers/platform/x86/intel/Kconfig
>> +++ b/drivers/platform/x86/intel/Kconfig
>> @@ -16,6 +16,7 @@ menuconfig X86_PLATFORM_DRIVERS_INTEL
>>     if X86_PLATFORM_DRIVERS_INTEL
>>   +source "drivers/platform/x86/intel/atomisp2/Kconfig"
>>   source "drivers/platform/x86/intel/int1092/Kconfig"
>>   source "drivers/platform/x86/intel/int33fe/Kconfig"
>>   source "drivers/platform/x86/intel/int3472/Kconfig"
>> diff --git a/drivers/platform/x86/intel/Makefile b/drivers/platform/x86/intel/Makefile
>> index 9858657f3f39..8ce9894e5efe 100644
>> --- a/drivers/platform/x86/intel/Makefile
>> +++ b/drivers/platform/x86/intel/Makefile
>> @@ -4,6 +4,7 @@
>>   # Intel x86 Platform-Specific Drivers
>>   #
>>   +obj-$(CONFIG_INTEL_ATOMISP2_PDX86)    += atomisp2/
>>   obj-$(CONFIG_INTEL_SAR_INT1092)        += int1092/
>>   obj-$(CONFIG_INTEL_CHT_INT33FE)        += int33fe/
>>   obj-$(CONFIG_INTEL_SKL_INT3472)        += int3472/
>> diff --git a/drivers/platform/x86/intel/atomisp2/Kconfig b/drivers/platform/x86/intel/atomisp2/Kconfig
>> new file mode 100644
>> index 000000000000..35dd2be9d2a1
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel/atomisp2/Kconfig
>> @@ -0,0 +1,43 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Intel x86 Platform Specific Drivers
>> +#
>> +
>> +config INTEL_ATOMISP2_PDX86
>> +    bool
>> +
>> +config INTEL_ATOMISP2_LED
>> +    tristate "Intel AtomISP v2 camera LED driver"
>> +    depends on GPIOLIB && LEDS_GPIO
>> +    select INTEL_ATOMISP2_PDX86
>> +    help
>> +      Many Bay Trail and Cherry Trail devices come with a camera attached
>> +      to Intel's Image Signal Processor. Linux currently does not have a
>> +      driver for these, so they do not work as a camera. Some of these
>> +      camera's have a LED which is controlled through a GPIO.
>> +
>> +      Some of these devices have a firmware issue where the LED gets turned
>> +      on at boot. This driver will turn the LED off at boot and also allows
>> +      controlling the LED (repurposing it) through the sysfs LED interface.
>> +
>> +      Which GPIO is attached to the LED is usually not described in the
>> +      ACPI tables, so this driver contains per-system info about the GPIO
>> +      inside the driver, this means that this driver only works on systems
>> +      the driver knows about.
>> +
>> +      To compile this driver as a module, choose M here: the module
>> +      will be called intel_atomisp2_led.
>> +
>> +config INTEL_ATOMISP2_PM
>> +    tristate "Intel AtomISP v2 dummy / power-management driver"
>> +    depends on PCI && IOSF_MBI && PM
>> +    depends on !INTEL_ATOMISP
>> +    select INTEL_ATOMISP2_PDX86
>> +    help
>> +      Power-management driver for Intel's Image Signal Processor found on
>> +      Bay Trail and Cherry Trail devices. This dummy driver's sole purpose
>> +      is to turn the ISP off (put it in D3) to save power and to allow
>> +      entering of S0ix modes.
>> +
>> +      To compile this driver as a module, choose M here: the module
>> +      will be called intel_atomisp2_pm.
>> diff --git a/drivers/platform/x86/intel/atomisp2/Makefile b/drivers/platform/x86/intel/atomisp2/Makefile
>> new file mode 100644
>> index 000000000000..96b1e877d1f1
>> --- /dev/null
>> +++ b/drivers/platform/x86/intel/atomisp2/Makefile
>> @@ -0,0 +1,9 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Intel x86 Platform Specific Drivers
>> +#
>> +
>> +intel_atomisp2_led-y            := led.o
>> +obj-$(CONFIG_INTEL_ATOMISP2_LED)    += intel_atomisp2_led.o
>> +intel_atomisp2_pm-y            += pm.o
>> +obj-$(CONFIG_INTEL_ATOMISP2_PM)        += intel_atomisp2_pm.o
>> diff --git a/drivers/platform/x86/intel_atomisp2_led.c b/drivers/platform/x86/intel/atomisp2/led.c
>> similarity index 100%
>> rename from drivers/platform/x86/intel_atomisp2_led.c
>> rename to drivers/platform/x86/intel/atomisp2/led.c
>> diff --git a/drivers/platform/x86/intel_atomisp2_pm.c b/drivers/platform/x86/intel/atomisp2/pm.c
>> similarity index 100%
>> rename from drivers/platform/x86/intel_atomisp2_pm.c
>> rename to drivers/platform/x86/intel/atomisp2/pm.c
>