[PATCH v3] platform/x86: int3472: support the POWER1 GPIO type

Jakob Berg Jespersen posted 1 patch 4 weeks ago
drivers/platform/x86/intel/int3472/discrete.c | 8 ++++++++
include/linux/platform_data/x86/int3472.h     | 2 ++
2 files changed, 10 insertions(+)
[PATCH v3] platform/x86: int3472: support the POWER1 GPIO type
Posted by Jakob Berg Jespersen 4 weeks ago
INT3472 can describe a second sensor power rail as a GPIO of type
POWER1 (0x08), which the driver does not recognise, so the rail is left
unmapped and never enabled:

  int3472-discrete INT3472:01: GPIO type 0x08 unknown; the sensor may
  not work

On the Microsoft Surface Pro 7+ the rear camera's INT3472 (INT347A,
ov8865) has such a pin; without it the ov8865 "dvdd" supply resolves to
a dummy regulator and the sensor never probes.

Define the POWER0 (0x07) and POWER1 (0x08) GPIO types and map POWER1 to
a regulator with con_id "dvdd" for all devices, the supply the in-tree
ov8865 driver already requests. POWER0 is defined but left unmapped, as
no device that uses it is known.

Out-of-tree work approached the same rail by exposing it to the sensor
as a new "pwr1" supply (linux-surface PR #1867 for the Surface Pro 9,
PR #2201 for the Pro 7+ ov8865); mapping POWER1 to the existing "dvdd"
keeps the change contained to int3472.

With this change the Surface Pro 7+ rear camera probes and streams
reliably.

Link: https://github.com/linux-surface/linux-surface/pull/1867
Link: https://github.com/linux-surface/linux-surface/pull/2201
Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Jakob Berg Jespersen <dev@berg.pm>
---
Changes in v3:
- No functional changes; the patch is identical to v2, rebased onto
  current pdx86/for-next (which has since gained the multi-HID GPIO map
  entries and the IMX471 "vana" entries).
- Resent with linux-media@vger.kernel.org on Cc, as requested.
- Link to v2: https://patch.msgid.link/20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm

Changes in v2:

- Reworked from a per-device INT347A quirk into a generic mapping of the
  POWER1 (0x08) GPIO type to "dvdd" for all devices, per review.
- Added INT3472_GPIO_TYPE_POWER0 (0x07) and POWER1 (0x08) definitions;
  POWER0 is defined but left unmapped for now.
- Retested on the Surface Pro 7+ (kernel 7.1.5): the rear ov8865 probes,
  instantiates its VCM, and streams.
- Link to v1: https://patch.msgid.link/20260719-sp7plus-int3472-v1-1-521a43f5c191@berg.pm
---
 drivers/platform/x86/intel/int3472/discrete.c | 8 ++++++++
 include/linux/platform_data/x86/int3472.h     | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 6c729fcfce5d..6ab8e589fcd3 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -266,6 +266,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
 		*con_id = "avdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
 		break;
+	case INT3472_GPIO_TYPE_POWER1:
+		*con_id = "dvdd";
+		*gpio_flags = GPIO_ACTIVE_HIGH;
+		break;
 	case INT3472_GPIO_TYPE_DOVDD:
 		*con_id = "dovdd";
 		*gpio_flags = GPIO_ACTIVE_HIGH;
@@ -296,6 +300,8 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
  * 0x00 Reset
  * 0x01 Power down
  * 0x02 Strobe
+ * 0x07 Power 0
+ * 0x08 Power 1
  * 0x0b Power enable
  * 0x0c Clock enable
  * 0x0d Privacy LED
@@ -382,6 +388,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 	case INT3472_GPIO_TYPE_STROBE:
 	case INT3472_GPIO_TYPE_POWER_ENABLE:
+	case INT3472_GPIO_TYPE_POWER1:
 	case INT3472_GPIO_TYPE_DOVDD:
 	case INT3472_GPIO_TYPE_HANDSHAKE:
 		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
@@ -408,6 +415,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 		case INT3472_GPIO_TYPE_POWER_ENABLE:
 			second_sensor = int3472->quirks.avdd_second_sensor;
 			fallthrough;
+		case INT3472_GPIO_TYPE_POWER1:
 		case INT3472_GPIO_TYPE_DOVDD:
 		case INT3472_GPIO_TYPE_HANDSHAKE:
 			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
index a73841dfae27..b1040e36deb8 100644
--- a/include/linux/platform_data/x86/int3472.h
+++ b/include/linux/platform_data/x86/int3472.h
@@ -25,6 +25,8 @@
 #define INT3472_GPIO_TYPE_RESET					0x00
 #define INT3472_GPIO_TYPE_POWERDOWN				0x01
 #define INT3472_GPIO_TYPE_STROBE				0x02
+#define INT3472_GPIO_TYPE_POWER0				0x07
+#define INT3472_GPIO_TYPE_POWER1				0x08
 #define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
 #define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
 #define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d

---
base-commit: 5b05bb3f6c5716fab6911e12d60dd1f43ad9806a
change-id: 20260719-sp7plus-int3472-8f014b8de79c

Best regards,
--  
Jakob Berg Jespersen <dev@berg.pm>
Re: [PATCH v3] platform/x86: int3472: support the POWER1 GPIO type
Posted by D. Manresa 3 weeks, 4 days ago
Since this thread is where the INT3472 _DSM GPIO type codes are being
discussed, relaying a piece of vendor-side documentation that surfaced in
the linux-surface work: the type-name table inside Intel's own Windows
driver. Extracted by the tester known as fildunsky on GitHub (relayed here
with their permission and at their request - full context and discussion in
https://github.com/linux-surface/linux-surface/pull/2252):

From iactrllogic64.sys, "Intel(R) Control Logic", 04/04/2024, shipped in the
Surface Pro 8 driver package (analysable without a Windows install):

  0x00  Reset          0x07  Power0         0x0E  AF
  0x01  Enable         0x08  Power1         0x0F  IO
  0x02  Strobe         0x09  Standby        0x10  Avdd
  0x03  Torch          0x0A  WriteProtect   0x11  Core
  0x04  Flash          0x0B  PowerEn        0x12  (Handshake)
  0x05  LedRear        0x0C  Mclk
  0x06  LedFront       0x0D  PrivateLED

Extraction data, for anyone who wants to reproduce or challenge it: name
pointer array in .data at VA 0x140021060 (file offset 0x1F260), stride 8,
indexed by the _DSM type code; strings in .rdata at VA 0x14001E4C0;
SetGpioOutput at VA 0x1400027C0 rejects type >= 0x13 and indexes per-type
state as base + 0x50 + type*32, consistent with that layout.

What this does and does not say:

- It confirms 0x07/0x08 are simply "Power0"/"Power1" on the vendor side -
  generic numbered rails with no supply semantics - which if anything
  supports mapping them by what the consuming sensor driver requests, as
  this patch does with "dvdd".

- It says the vendor calls 0x10 "Avdd" and 0x11 "Core", while mainline since
  v7.0 names 0x10 INT3472_GPIO_TYPE_DOVDD and registers "dovdd". Worth
  knowing, with two caveats: it is a single artifact and the electrical
  claim is unproven (the Windows control logic raises every described line
  in sequence regardless of name, so a working camera under Windows proves
  nothing about which rail is which); and con_id in int3472 follows what
  in-tree sensor drivers request rather than vendor naming anyway (0x0b is
  "PowerEn" in this table and is registered as "avdd"). Empirically, on the
  Surface Pro 8 the consumer of the 0x10 rail is the ST VD55G0, which
  requests "vddio" - lining up with neither name and resolved there by a
  per-HID mapping.

- For the enable-delay discussions, the same binary's power-on sequence
  (discrete::DiscreteControl::SensorOn):

    Enable -> 2ms -> Power0 -> 5ms -> Power1 -> 5ms -> PowerEn -> 2ms ->
    Avdd -> 2ms -> Reset held -> 2ms -> Mclk (or the ACPI clock when there
    is no Mclk GPIO) -> Reset released -> 2ms -> Enable asserted -> Handshake

fildunsky still has the binary and is happy to re-check it against specific
questions; anything for them is best routed through the PR thread above.

D. Manresa <dmanresa@gmail.com>
Re: [PATCH v3] platform/x86: int3472: support the POWER1 GPIO type
Posted by Hans de Goede 3 weeks, 6 days ago
Hi,

On 29-Aug-26 10:29, Jakob Berg Jespersen wrote:
> INT3472 can describe a second sensor power rail as a GPIO of type
> POWER1 (0x08), which the driver does not recognise, so the rail is left
> unmapped and never enabled:
> 
>   int3472-discrete INT3472:01: GPIO type 0x08 unknown; the sensor may
>   not work
> 
> On the Microsoft Surface Pro 7+ the rear camera's INT3472 (INT347A,
> ov8865) has such a pin; without it the ov8865 "dvdd" supply resolves to
> a dummy regulator and the sensor never probes.
> 
> Define the POWER0 (0x07) and POWER1 (0x08) GPIO types and map POWER1 to
> a regulator with con_id "dvdd" for all devices, the supply the in-tree
> ov8865 driver already requests. POWER0 is defined but left unmapped, as
> no device that uses it is known.
> 
> Out-of-tree work approached the same rail by exposing it to the sensor
> as a new "pwr1" supply (linux-surface PR #1867 for the Surface Pro 9,
> PR #2201 for the Pro 7+ ov8865); mapping POWER1 to the existing "dvdd"
> keeps the change contained to int3472.
> 
> With this change the Surface Pro 7+ rear camera probes and streams
> reliably.
> 
> Link: https://github.com/linux-surface/linux-surface/pull/1867
> Link: https://github.com/linux-surface/linux-surface/pull/2201
> Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Jakob Berg Jespersen <dev@berg.pm>


Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans

> ---
> Changes in v3:
> - No functional changes; the patch is identical to v2, rebased onto
>   current pdx86/for-next (which has since gained the multi-HID GPIO map
>   entries and the IMX471 "vana" entries).
> - Resent with linux-media@vger.kernel.org on Cc, as requested.
> - Link to v2: https://patch.msgid.link/20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm
> 
> Changes in v2:
> 
> - Reworked from a per-device INT347A quirk into a generic mapping of the
>   POWER1 (0x08) GPIO type to "dvdd" for all devices, per review.
> - Added INT3472_GPIO_TYPE_POWER0 (0x07) and POWER1 (0x08) definitions;
>   POWER0 is defined but left unmapped for now.
> - Retested on the Surface Pro 7+ (kernel 7.1.5): the rear ov8865 probes,
>   instantiates its VCM, and streams.
> - Link to v1: https://patch.msgid.link/20260719-sp7plus-int3472-v1-1-521a43f5c191@berg.pm
> ---
>  drivers/platform/x86/intel/int3472/discrete.c | 8 ++++++++
>  include/linux/platform_data/x86/int3472.h     | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 6c729fcfce5d..6ab8e589fcd3 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -266,6 +266,10 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>  		*con_id = "avdd";
>  		*gpio_flags = GPIO_ACTIVE_HIGH;
>  		break;
> +	case INT3472_GPIO_TYPE_POWER1:
> +		*con_id = "dvdd";
> +		*gpio_flags = GPIO_ACTIVE_HIGH;
> +		break;
>  	case INT3472_GPIO_TYPE_DOVDD:
>  		*con_id = "dovdd";
>  		*gpio_flags = GPIO_ACTIVE_HIGH;
> @@ -296,6 +300,8 @@ static void int3472_get_con_id_and_polarity(struct int3472_discrete_device *int3
>   * 0x00 Reset
>   * 0x01 Power down
>   * 0x02 Strobe
> + * 0x07 Power 0
> + * 0x08 Power 1
>   * 0x0b Power enable
>   * 0x0c Clock enable
>   * 0x0d Privacy LED
> @@ -382,6 +388,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  	case INT3472_GPIO_TYPE_PRIVACY_LED:
>  	case INT3472_GPIO_TYPE_STROBE:
>  	case INT3472_GPIO_TYPE_POWER_ENABLE:
> +	case INT3472_GPIO_TYPE_POWER1:
>  	case INT3472_GPIO_TYPE_DOVDD:
>  	case INT3472_GPIO_TYPE_HANDSHAKE:
>  		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
> @@ -408,6 +415,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  		case INT3472_GPIO_TYPE_POWER_ENABLE:
>  			second_sensor = int3472->quirks.avdd_second_sensor;
>  			fallthrough;
> +		case INT3472_GPIO_TYPE_POWER1:
>  		case INT3472_GPIO_TYPE_DOVDD:
>  		case INT3472_GPIO_TYPE_HANDSHAKE:
>  			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
> diff --git a/include/linux/platform_data/x86/int3472.h b/include/linux/platform_data/x86/int3472.h
> index a73841dfae27..b1040e36deb8 100644
> --- a/include/linux/platform_data/x86/int3472.h
> +++ b/include/linux/platform_data/x86/int3472.h
> @@ -25,6 +25,8 @@
>  #define INT3472_GPIO_TYPE_RESET					0x00
>  #define INT3472_GPIO_TYPE_POWERDOWN				0x01
>  #define INT3472_GPIO_TYPE_STROBE				0x02
> +#define INT3472_GPIO_TYPE_POWER0				0x07
> +#define INT3472_GPIO_TYPE_POWER1				0x08
>  #define INT3472_GPIO_TYPE_POWER_ENABLE				0x0b
>  #define INT3472_GPIO_TYPE_CLK_ENABLE				0x0c
>  #define INT3472_GPIO_TYPE_PRIVACY_LED				0x0d
> 
> ---
> base-commit: 5b05bb3f6c5716fab6911e12d60dd1f43ad9806a
> change-id: 20260719-sp7plus-int3472-8f014b8de79c
> 
> Best regards,
> --  
> Jakob Berg Jespersen <dev@berg.pm>
>