Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41607 )
Change subject: drivers/intel/mipi_camera: camera SSDT generation ......................................................................
Patch Set 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... PS6, Line 159: config->ssdb.platform = 9; : if (!config->ssdb.flash_support) : config->ssdb.flash_support = 2; : if (!config->ssdb.privacy_led) : config->ssdb.privacy_led = 1; : if (!config->ssdb.mipi_define) : config->ssdb.mipi_define = 1; : if (!config->ssdb.mclk_speed) : config->ssdb.mclk_speed = 19200000;
Some of these values can be found in Intel's "SkyCam" camera driver SDK (example files Skycam/public […]
Thanks Matt, Sugnan could you add those in?
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... PS6, Line 188: /* Method (_DSM, 4, NotSerialized) */ : acpigen_write_method("_DSM", 0x4); : : /* ToBuffer (Arg0, Local0) */ : acpigen_write_to_buffer(ARG0_OP, LOCAL0_OP); : : /* If (LEqual (Local0, ToUUID(uuid))) */ : acpigen_write_if(); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_byte(LOCAL0_OP); : acpigen_write_uuid("822ace8f-2814-4174-a56b-5f029fe079ee"); : acpigen_write_return_string(config->sensor_name ? config->sensor_name : "UNKNOWN"); : acpigen_pop_len(); /* If */ : : /* If (LEqual (Local0, ToUUID(uuid))) */ : acpigen_write_if(); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_byte(LOCAL0_OP); : acpigen_write_uuid("26257549-9271-4ca4-bb43-c4899d5a4881"); : /* ToInteger (Arg2, Local1) */ : acpigen_write_to_integer(ARG2_OP, LOCAL1_OP); : : /* If (LEqual (Local1, 1)) */ : acpigen_write_if_lequal_op_int(LOCAL1_OP, 1); : acpigen_write_return_integer((config->ssdb.vcm_type ? 1 : 0) + : (config->ssdb.rom_type ? 1 : 0) + 1); : acpigen_pop_len(); /* If Arg2=1 */ : : /* If (LEqual (Local1, 2)) */ : acpigen_write_if_lequal_op_int(LOCAL1_OP, 2); : acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 | : ((uint32_t)i2c_addr) << 8 | 0x0); /* type 0 for sensor */ : acpigen_pop_len(); /* If Arg2=2 */ : : if (config->ssdb.vcm_type) { : /* If (LEqual (Local1, 3)) */ : acpigen_write_if_lequal_op_int(LOCAL1_OP, 3); : acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 | /* type 1 for vcm */ : ((uint32_t)config->vcm_address) << 8 | 0x01); : acpigen_pop_len(); /* If Arg2=3 */ : } : if (config->ssdb.rom_type) { : /* If (LEqual (Local1, 3 or 4)) */ : acpigen_write_if_lequal_op_int(LOCAL1_OP, 3 + (config->ssdb.vcm_type ? 1 : 0)); : acpigen_write_return_integer(((uint32_t)i2c_bus) << 24 | /* type 2 for rom */ : ((uint32_t)config->rom_address) << 8 | 0x02); : acpigen_pop_len(); /* If Arg2=3 or 4 */ : } : : acpigen_pop_len(); /* If uuid */ : : /* Return (Buffer (One) { 0x0 }) */ : acpigen_write_return_singleton_buffer(0x0); : : acpigen_pop_len(); /* Method _DSM */
I think _DSM is mostly used in Intel's design for Windows drivers. […]
You should be able to use acpigen_write_dsm. It takes lists of corresponding UUIDs and callback functions to add what code to run for each `if (arg2 == uuid[0]), etc.`
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... PS6, Line 445: if (config->dep) { : acpigen_write_name("_DEP"); : acpigen_write_package(1); : acpigen_emit_namestring(config->dep); : acpigen_pop_len(); : }
To include a dependency similar to one in the following link. […]
Is the kernel driver using _DEP for something unusual? _DEP is used to "designate device objects that OSPM should assign a higher priority in start ordering due to future operation region accesses."
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... PS6, Line 79: uint8_t reserved[13];
It pads SSDB out so the binary blob in ACPI is the same size as seen on other firmwares. […]
Ack
https://review.coreboot.org/c/coreboot/+/41607/6/src/drivers/intel/mipi_came... PS6, Line 107: const char *pr0; : const char *pr3; /* _PR3 shouldn't be needed
Idea was to push @Matt patch mostly as it is and the incremental changes related to power resource i […]
A later patch is OK with me.
-
Tim Wawrzynczak (Code Review)