coreboot-gerrit July 2020

coreboot-gerrit@coreboot.org

1 participants
3166 discussions

Change in coreboot[master]: pci_ids: Add TGL & JSL IPU PCI IDs
by Tim Wawrzynczak (Code Review) 07 Jul '20

07 Jul '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42811 ) Change subject: pci_ids: Add TGL & JSL IPU PCI IDs ...................................................................... pci_ids: Add TGL & JSL IPU PCI IDs Add PCI IDs for Intel's Image Processing Unit (IPU) for TGL & JSL Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I71dc95a6692e82ca25b0252b4f0789ee059df89f --- M src/include/device/pci_ids.h 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/42811/1 diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 4b17567..ed0629a 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -3714,6 +3714,9 @@ #define PCI_DEVICE_ID_GrP_6SERIES_1_WIFI 0x51f0 #define PCI_DEVICE_ID_GrP_6SERIES_2_WIFI 0x7af0 +#define PCI_DEVICE_ID_INTEL_TGL_IPU 0x9a19 +#define PCI_DEVICE_ID_INTEL_JSL_IPU 0x4e19 + #define PCI_VENDOR_ID_COMPUTONE 0x8e0e #define PCI_DEVICE_ID_COMPUTONE_IP2EX 0x0291 #define PCI_DEVICE_ID_COMPUTONE_PG 0x0302 -- To view, visit https://review.coreboot.org/c/coreboot/+/42811 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71dc95a6692e82ca25b0252b4f0789ee059df89f Gerrit-Change-Number: 42811 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: util/crossgcc: Always bootstrap for gcc < 4.9
by Patrick Georgi (Code Review) 07 Jul '20

07 Jul '20

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43143 ) Change subject: util/crossgcc: Always bootstrap for gcc < 4.9 ...................................................................... util/crossgcc: Always bootstrap for gcc < 4.9 Building cbfstool requires at least 4.9 due to optimizer bugs in gcc 3.x to 4.8.x, so let's not work around ancient compilers in our tree but ensure that users get a newer compiler. Change-Id: I4e0f80e2790514e6a1b5d5de1a373f365df1569c Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M util/crossgcc/buildgcc 1 file changed, 15 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/43143/1 diff --git a/util/crossgcc/buildgcc b/util/crossgcc/buildgcc index 17c89e8..03c7532 100755 --- a/util/crossgcc/buildgcc +++ b/util/crossgcc/buildgcc @@ -1178,33 +1178,25 @@ LANGUAGES="${DEFAULT_LANGUAGES}" fi fi +if [ "$BOOTSTRAP" != 1 ] && \ + { [ "$(hostcc_major)" -lt 4 ] || \ + { [ "$(hostcc_major)" -eq 4 ] && \ + [ "$(hostcc_minor)" -lt 9 ] ; } ; } +then + printf "\n${red}WARNING${NC}\n" + printf "Building coreboot requires a host compiler newer than 4.9.x while\n" + printf "yours is $(hostcc_version).\n" + printf "Enabling bootstrapping to provide a sufficiently new compiler:\n" + printf "This will take significantly longer than a usual build.\n" + printf "Alternatively you can abort now and update your host compiler.\n" + timeout 15 + BOOTSTRAP=1 +fi if ada_requested; then - if have_gnat; then - if [ "$BOOTSTRAP" != 1 ] && \ - { [ "$(hostcc_major)" -lt 4 ] || \ - { [ "$(hostcc_major)" -eq 4 ] && \ - [ "$(hostcc_minor)" -lt 9 ] ; } ; } - then - printf "\n${red}WARNING${NC}\n" - printf "Building the Ada compiler (GNAT $(buildcc_version)) with a host compiler older\n" - printf "than 4.9.x (yours $(hostcc_version)) requires bootstrapping. This will take\n" - printf "significantly longer than a usual build. You can abort and update\n" - printf "your host GNAT or disable Ada support with BUILD_LANGUAGES=c (or\n" - printf "\`-l c\` in case you invoke \`buildgcc\` directly).\n" - timeout 15 - BOOTSTRAP=1 - fi - else + if !have_gnat; then please_install gnat gcc-ada exit 1 fi -else - if [ "$(hostcc_major)" -lt 4 ] && [ "$BOOTSTRAP" != 1 ]; then - printf "\n${red}WARNING${NC}\n" - printf "Building GCC $(buildcc_version) with a very old host compiler ($(hostcc_version)).\n" - printf "Bootstrapping (-b) is recommended.\n" - timeout 10 - fi fi fi # GCC -- To view, visit https://review.coreboot.org/c/coreboot/+/43143 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e0f80e2790514e6a1b5d5de1a373f365df1569c Gerrit-Change-Number: 43143 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

4 8

Change in coreboot[master]: drivers/intel/mipi_camera: Add camera power resource to SSDT
by Patrick Georgi (Code Review) 07 Jul '20

07 Jul '20

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41624 ) Change subject: drivers/intel/mipi_camera: Add camera power resource to SSDT ...................................................................... drivers/intel/mipi_camera: Add camera power resource to SSDT This change adds support function to parse entries in the devicetree to generate PowerResource entries for the MIPI camera. Change-Id: I31e198b50acf2c64035aff9cb054fbe3602dd83e Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41624 Reviewed-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 159 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Rizwan Qureshi: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index 03e5ab6..f32f744 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -17,6 +17,7 @@ #define DEFAULT_REMOTE_NAME "\\_SB.PCI0.CIO2" #define CIO2_PCI_DEV 0x14 #define CIO2_PCI_FN 0x3 +#define POWER_RESOURCE_NAME "PRIC" /* * This implementation assumes there is only 1 endpoint at each end of every data port. It also @@ -435,6 +436,58 @@ acpi_dp_write(dsd); } +static void fill_power_res_sequence(struct drivers_intel_mipi_camera_config *config, + struct operation_seq *seq) +{ + unsigned int i; + uint8_t index; + uint8_t gpio_num; + + for (i = 0; i < seq->ops_cnt; i++) { + switch (seq->ops[i].type) { + case IMGCLK: + index = seq->ops[i].index; + if (seq->ops[i].action == ENABLE) { + acpigen_emit_namestring("MCON"); + acpigen_write_byte(config->clk_panel.clks[index].clknum); + acpigen_write_byte(config->clk_panel.clks[index].freq); + } else if (seq->ops[i].action == DISABLE) { + acpigen_emit_namestring("MCOF"); + acpigen_write_byte(config->clk_panel.clks[index].clknum); + } else { + acpigen_write_debug_string("Unsupported clock action"); + printk(BIOS_ERR, "Unsupported clock action: %x\n", + seq->ops[i].action); + printk(BIOS_ERR, "OS camera driver will likely not work"); + } + + break; + case GPIO: + index = seq->ops[i].index; + gpio_num = config->gpio_panel.gpio[index].gpio_num; + if (seq->ops[i].action == ENABLE) { + acpigen_soc_set_tx_gpio(gpio_num); + } else if (seq->ops[i].action == DISABLE) { + acpigen_soc_clear_tx_gpio(gpio_num); + } else { + acpigen_write_debug_string("Unsupported GPIO action"); + printk(BIOS_ERR, "Unsupported GPIO action: %x\n", + seq->ops[i].action); + printk(BIOS_ERR, "OS camera driver will likely not work"); + } + + break; + default: + printk(BIOS_ERR, "Unsupported power operation: %x\n", seq->ops[i].type); + printk(BIOS_ERR, "OS camera driver will likely not work"); + break; + } + + if (seq->ops[i].delay_ms) + acpigen_write_sleep(seq->ops[i].delay_ms); + } +} + static void write_pci_camera_device(const struct device *dev) { struct drivers_intel_mipi_camera_config *config = dev->chip_info; @@ -467,6 +520,40 @@ acpigen_write_device(acpi_device_name(dev)); + /* add power resource */ + if (config->has_power_resource) { + acpigen_write_power_res(POWER_RESOURCE_NAME, 0, 0, NULL, 0); + acpigen_write_name_integer("STA", 0); + acpigen_write_STA_ext("STA"); + + acpigen_write_method_serialized("_ON", 0); + acpigen_write_if(); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_namestring("STA"); + acpigen_write_integer(0); + + fill_power_res_sequence(config, &config->on_seq); + + acpigen_write_store_op_to_namestr(1, "STA"); + acpigen_pop_len(); /* if */ + acpigen_pop_len(); /* _ON */ + + /* _OFF operations */ + acpigen_write_method_serialized("_OFF", 0); + acpigen_write_if(); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_namestring("STA"); + acpigen_write_integer(1); + + fill_power_res_sequence(config, &config->off_seq); + + acpigen_write_store_op_to_namestr(0, "STA"); + acpigen_pop_len(); /* if */ + acpigen_pop_len(); /* _ON */ + + acpigen_pop_len(); /* Power Resource */ + } + if (config->device_type == INTEL_ACPI_CAMERA_SENSOR) acpigen_write_name_integer("_ADR", 0); @@ -519,6 +606,17 @@ acpigen_write_name_integer("CAMD", config->device_type); } + if (config->pr0 || config->has_power_resource) { + acpigen_write_name("_PR0"); + acpigen_write_package(1); + if (config->pr0) + acpigen_emit_namestring(config->pr0); /* External power resource */ + else + acpigen_emit_namestring(POWER_RESOURCE_NAME); + + acpigen_pop_len(); /* _PR0 */ + } + switch (config->device_type) { case INTEL_ACPI_CAMERA_CIO2: camera_fill_cio2(dev); diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index 92d3eac..d133f7d 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -10,6 +10,18 @@ #define MAX_PWDB_ENTRIES 12 #define MAX_PORT_ENTRIES 4 #define MAX_LINK_FREQ_ENTRIES 4 +#define MAX_CLK_CONFIGS 2 +#define MAX_GPIO_CONFIGS 4 +#define MAX_PWR_OPS 5 + +#define SEQ_OPS_CLK_ENABLE(ind, delay) \ + { .type = IMGCLK, .index = (ind), .action = ENABLE, .delay_ms = (delay) } +#define SEQ_OPS_CLK_DISABLE(ind, delay) \ + { .type = IMGCLK, .index = (ind), .action = DISABLE, .delay_ms = (delay) } +#define SEQ_OPS_GPIO_ENABLE(ind, delay) \ + { .type = GPIO, .index = (ind), .action = ENABLE, .delay_ms = (delay) } +#define SEQ_OPS_GPIO_DISABLE(ind, delay) \ + { .type = GPIO, .index = (ind), .action = DISABLE, .delay_ms = (delay) } enum camera_device_type { DEV_TYPE_SENSOR = 0, @@ -57,6 +69,47 @@ INTEL_ACPI_CAMERA_GPIO, }; +enum ctrl_type { + IMGCLK = 1, + GPIO +}; + +enum action_type { + ENABLE = 1, + DISABLE +}; + +struct clk_config { + /* IMGCLKOUT_x being used for a port */ + uint8_t clknum; + /* frequency setting: 0:24Mhz, 1:19.2 Mhz */ + uint8_t freq; +} __packed; + +struct gpio_config { + uint8_t gpio_num; +} __packed; + +struct clock_ctrl_panel { + struct clk_config clks[MAX_CLK_CONFIGS]; +} __packed; + +struct gpio_ctrl_panel { + struct gpio_config gpio[MAX_GPIO_CONFIGS]; +} __packed; + +struct operation_type { + enum ctrl_type type; + uint8_t index; + enum action_type action; + uint32_t delay_ms; +} __packed; + +struct operation_seq { + struct operation_type ops[MAX_PWR_OPS]; + uint8_t ops_cnt; +} __packed; + struct intel_ssdb { uint8_t version; /* Current version */ uint8_t sensor_card_sku; /* CRD Board type */ @@ -122,6 +175,11 @@ } __packed; struct drivers_intel_mipi_camera_config { + struct clock_ctrl_panel clk_panel; + struct gpio_ctrl_panel gpio_panel; + struct operation_seq on_seq; + struct operation_seq off_seq; + struct intel_ssdb ssdb; struct intel_pwdb pwdb[MAX_PWDB_ENTRIES]; enum intel_camera_device_type device_type; @@ -130,6 +188,7 @@ const char *acpi_name; const char *chip_name; unsigned int acpi_uid; + const char *pr0; /* Settings specific to CIO2 device */ uint32_t cio2_num_ports; @@ -163,6 +222,8 @@ /* Settings specific to vcm */ const char *vcm_compat; + /* Does the device have a power resource entries */ + bool has_power_resource; }; #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/41624 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31e198b50acf2c64035aff9cb054fbe3602dd83e Gerrit-Change-Number: 41624 Gerrit-PatchSet: 34 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Daniel Kang <daniel.h.kang(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kiran2 Kumar <kiran2.kumar(a)intel.corp-partner.google.com> Gerrit-CC: Matt Delco <delco(a)chromium.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: acpi: add STA function to return external variable
by Sugnan Prabhu S (Code Review) 07 Jul '20

07 Jul '20

Sugnan Prabhu S has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42245 ) Change subject: acpi: add STA function to return external variable ...................................................................... acpi: add STA function to return external variable This change adds support function to add STA function which returns an external variable. Change-Id: I31755a76ee985ee6059289ae194537d531270761 Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> --- M src/acpi/acpigen.c M src/include/acpi/acpigen.h 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/42245/1 diff --git a/src/acpi/acpigen.c b/src/acpi/acpigen.c index 7a507d6..1fd4662 100644 --- a/src/acpi/acpigen.c +++ b/src/acpi/acpigen.c @@ -742,6 +742,17 @@ acpigen_pop_len(); } +void acpigen_write_STA_ext(const char *ext_val) +{ + /* + * Method (_STA, 0, NotSerialized) { Return (ext_val) } + */ + acpigen_write_method("_STA", 0); + acpigen_emit_byte(RETURN_OP); + acpigen_emit_namestring(ext_val); + acpigen_pop_len(); +} + /* * Generates a func with max supported P-states. */ diff --git a/src/include/acpi/acpigen.h b/src/include/acpi/acpigen.h index 565abcf..4c26464 100644 --- a/src/include/acpi/acpigen.h +++ b/src/include/acpi/acpigen.h @@ -319,6 +319,7 @@ void acpigen_write_empty_PTC(void); void acpigen_write_PRW(u32 wake, u32 level); void acpigen_write_STA(uint8_t status); +void acpigen_write_STA_ext(const char *ext_val); void acpigen_write_TPC(const char *gnvs_tpc_limit); void acpigen_write_PSS_package(u32 coreFreq, u32 power, u32 transLat, u32 busmLat, u32 control, u32 status); -- To view, visit https://review.coreboot.org/c/coreboot/+/42245 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31755a76ee985ee6059289ae194537d531270761 Gerrit-Change-Number: 42245 Gerrit-PatchSet: 1 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-MessageType: newchange

5 9

Change in coreboot[master]: drivers/intel/mipi_camera: SSDT changes to handle CIO2 device
by Sugnan Prabhu S (Code Review) 07 Jul '20

07 Jul '20

Hello Matt Delco, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42470 to review the following change. Change subject: drivers/intel/mipi_camera: SSDT changes to handle CIO2 device ...................................................................... drivers/intel/mipi_camera: SSDT changes to handle CIO2 device This change updates mipi_camera driver to handle CIO2 device type. Change-Id: I521740524bc1c4da3d8593f011a033542e4a872c Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 113 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/42470/1 diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index ddafccf..bcce7a6 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -10,9 +10,83 @@ #include <device/pci_def.h> #include "chip.h" +#define CIO2_PCI_DEV 0x14 +#define CIO2_PCI_FN 0x3 #define SENSOR_NAME_UUID "822ace8f-2814-4174-a56b-5f029fe079ee" #define SENSOR_TYPE_UUID "26257549-9271-4ca4-bb43-c4899d5a4881" +/* + * This implementation assumes there is only 1 endpoint at each end of every data port. It also + * assumes there are no clock lanes. It also assumes that any VCM or NVM for a CAM is on the + * same I2C bus as the CAM. + */ + +/* + * Adds settings for a CIO2 device (typically at "\_SB.PCI0.CIO2"). A _DSD is added that + * specifies a child table for each port (e.g., PRT0 for "port0" and PRT1 for "port1"). Each + * PRTx table specifies a table for each endpoint (though only 1 endpoint is supported by this + * implementation so the table only has an "endpoint0" that points to a EPx0 table). The EPx0 + * table primarily describes the # of lanes in "data-lines" and specifies the name of the other + * side in "remote-endpoint" (the name is usually of the form "\_SB.PCI0.I2Cx.CAM0" for the + * first port/cam and "\_SB.PCI0.I2Cx.CAM1" if there's a second port/cam). + */ +static void camera_fill_cio2(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *dsd = acpi_dp_new_table("_DSD"); + char *ep_table_name[MAX_PORT_ENTRIES] = {NULL}; + char *port_table_name[MAX_PORT_ENTRIES] = {NULL}; + char *port_name[MAX_PORT_ENTRIES] = {NULL}; + unsigned int i, j; + char name[BUS_PATH_MAX]; + struct acpi_dp *ep_table = NULL; + struct acpi_dp *port_table = NULL; + struct acpi_dp *remote = NULL; + struct acpi_dp *lanes = NULL; + + for (i = 0; i < config->cio2_num_ports && i < MAX_PORT_ENTRIES; ++i) { + snprintf(name, sizeof(name), "EP%u0", i); + ep_table_name[i] = strdup(name); + ep_table = acpi_dp_new_table(ep_table_name[i]); + acpi_dp_add_integer(ep_table, "endpoint", 0); + acpi_dp_add_integer(ep_table, "clock-lanes", 0); + + if (config->cio2_lanes_used[i] > 0) { + lanes = acpi_dp_new_table("data-lanes"); + + for (j = 1; j <= config->cio2_lanes_used[i]; ++j) + acpi_dp_add_integer(lanes, NULL, j); + acpi_dp_add_array(ep_table, lanes); + } + + if (config->cio2_lane_endpoint[i]) { + remote = acpi_dp_new_table("remote-endpoint"); + acpi_dp_add_reference(remote, NULL, config->cio2_lane_endpoint[i]); + acpi_dp_add_integer(remote, NULL, 0); + acpi_dp_add_integer(remote, NULL, 0); + acpi_dp_add_array(ep_table, remote); + } + + snprintf(name, sizeof(name), "PRT%u", i); + port_table_name[i] = strdup(name); + port_table = acpi_dp_new_table(port_table_name[i]); + acpi_dp_add_integer(port_table, "port", config->cio2_prt[i]); + acpi_dp_add_child(port_table, "endpoint0", ep_table); + + snprintf(name, sizeof(name), "port%u", i); + port_name[i] = strdup(name); + acpi_dp_add_child(dsd, port_name[i], port_table); + } + + acpi_dp_write(dsd); + + for (i = 0; i < config->cio2_num_ports; ++i) { + free(ep_table_name[i]); + free(port_table_name[i]); + free(port_name[i]); + } +} + static void apply_pld_defaults(struct drivers_intel_mipi_camera_config *config) { if (!config->pld.ignore_color) @@ -142,6 +216,11 @@ static void camera_fill_sensor_defaults(struct drivers_intel_mipi_camera_config *config) { + struct device *cio2 = pcidev_on_root(CIO2_PCI_DEV, CIO2_PCI_FN); + struct drivers_intel_mipi_camera_config *cio2_config; + if (!config->ssdb.bdf_value) + config->ssdb.bdf_value = PCI_DEVFN(CIO2_PCI_DEV, CIO2_PCI_FN); + if (!config->ssdb.platform) config->ssdb.platform = PLATFORM_SKC; @@ -156,6 +235,31 @@ if (!config->ssdb.mclk_speed) config->ssdb.mclk_speed = CLK_FREQ_19_2MHZ; + + if (!config->ssdb.lanes_used) { + cio2_config = cio2 ? cio2->chip_info : NULL; + + if (!cio2_config) { + printk(BIOS_ERR, "Failed to get CIO2 config\n"); + } else if (cio2_config->device_type != INTEL_ACPI_CAMERA_CIO2) { + printk(BIOS_ERR, "Device type isn't CIO2: %u\n", + (u32)cio2_config->device_type); + } else if (config->ssdb.link_used >= cio2_config->cio2_num_ports) { + printk(BIOS_ERR, "%u exceeds CIO2's %u links\n", + (u32)config->ssdb.link_used, + (u32)cio2_config->cio2_num_ports); + } else { + config->ssdb.lanes_used = + cio2_config->cio2_lanes_used[config->ssdb.link_used]; + } + } + + if (!config->remote_name) { + if (cio2) + config->remote_name = acpi_device_path(cio2); + else + config->remote_name = "\\_SB.PCI0.CIO2"; + } } /* @@ -409,6 +513,9 @@ } switch (config->device_type) { + case INTEL_ACPI_CAMERA_CIO2: + camera_fill_cio2(dev); + break; case INTEL_ACPI_CAMERA_SENSOR: camera_fill_sensor(dev); break; diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index 95a3ebb..82223c1 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -127,6 +127,12 @@ const char *chip_name; unsigned int acpi_uid; + /* Settings specific to CIO2 device */ + uint32_t cio2_num_ports; + uint32_t cio2_lanes_used[MAX_PORT_ENTRIES]; + const char *cio2_lane_endpoint[MAX_PORT_ENTRIES]; + uint32_t cio2_prt[MAX_PORT_ENTRIES]; + /* Settings specific to camera sensor */ bool disable_ssdb_defaults; -- To view, visit https://review.coreboot.org/c/coreboot/+/42470 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I521740524bc1c4da3d8593f011a033542e4a872c Gerrit-Change-Number: 42470 Gerrit-PatchSet: 1 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

6 28

Change in coreboot[master]: drivers/intel/mipi_camera: SSDT changes to add PLD
by Sugnan Prabhu S (Code Review) 07 Jul '20

07 Jul '20

Hello Matt Delco, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42469 to review the following change. Change subject: drivers/intel/mipi_camera: SSDT changes to add PLD ...................................................................... drivers/intel/mipi_camera: SSDT changes to add PLD This change updates mipi_camera driver to add PLD section to SSDT. Change-Id: If65b9cbabca95e9645d8e5023ce7fd78b0625d1e Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 53 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/42469/1 diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index a3a9323..ddafccf 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -13,6 +13,53 @@ #define SENSOR_NAME_UUID "822ace8f-2814-4174-a56b-5f029fe079ee" #define SENSOR_TYPE_UUID "26257549-9271-4ca4-bb43-c4899d5a4881" +static void apply_pld_defaults(struct drivers_intel_mipi_camera_config *config) +{ + if (!config->pld.ignore_color) + config->pld.ignore_color = 1; + + if (!config->pld.visible) + config->pld.visible = 1; + + if (!config->pld.vertical_offset) + config->pld.vertical_offset = 0xffff; + + if (!config->pld.horizontal_offset) + config->pld.horizontal_offset = 0xffff; + + /* + * PLD_PANEL_TOP has a value of zero, so the following will change any instance of + * PLD_PANEL_TOP to PLD_PANEL_FRONT unless disable_pld_defaults is set. + */ + if (!config->pld.panel) + config->pld.panel = PLD_PANEL_FRONT; + + /* + * PLD_HORIZONTAL_POSITION_LEFT has a value of zero, so the following will change any + * instance of that value to PLD_HORIZONTAL_POSITION_CENTER unless disable_pld_defaults + * is set. + */ + if (!config->pld.horizontal_position) + config->pld.horizontal_position = PLD_HORIZONTAL_POSITION_CENTER; + + /* + * The desired default for |vertical_position| is PLD_VERTICAL_POSITION_UPPER, which + * has a value of zero so no work is needed to set a default. The same applies for + * setting |shape| to PLD_SHAPE_ROUND. + */ +} + +static void camera_generate_pld(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + + if (config->use_pld && !config->disable_pld_defaults) + apply_pld_defaults(config); + + if (config->use_pld) + acpigen_write_pld(&config->pld); +} + static uint32_t address_for_dev_type(const struct device *dev, uint8_t dev_type) { struct drivers_intel_mipi_camera_config *config = dev->chip_info; @@ -140,6 +187,8 @@ const char *vcm_name = NULL; struct acpi_dp *lens_focus = NULL; + camera_generate_pld(dev); + if (!config->disable_ssdb_defaults) camera_fill_sensor_defaults(config); diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index f55ee1d..95a3ebb 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -4,6 +4,7 @@ #define __INTEL_MIPI_CAMERA_CHIP_H__ #include <stdint.h> +#include <acpi/acpi_pld.h> #define MAX_PWDB_ENTRIES 12 #define MAX_PORT_ENTRIES 4 @@ -134,6 +135,9 @@ const char *sensor_name; /* default "UNKNOWN" */ const char *remote_name; /* default "\_SB.PCI0.CIO2" */ const char *vcm_name; /* defaults to |vcm_address| device */ + bool use_pld; + bool disable_pld_defaults; + struct acpi_pld pld; uint16_t rom_address; /* I2C to use if ssdb.rom_type != 0 */ uint16_t vcm_address; /* I2C to use if ssdb.vcm_type != 0 */ /* Settings specific to nvram. Many values, if left as zero, will be assigned a default. -- To view, visit https://review.coreboot.org/c/coreboot/+/42469 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If65b9cbabca95e9645d8e5023ce7fd78b0625d1e Gerrit-Change-Number: 42469 Gerrit-PatchSet: 1 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

6 12

Change in coreboot[master]: drivers/intel/mipi_camera: SSDT changes to add DSM
by Sugnan Prabhu S (Code Review) 07 Jul '20

07 Jul '20

Hello Matt Delco, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42468 to review the following change. Change subject: drivers/intel/mipi_camera: SSDT changes to add DSM ...................................................................... drivers/intel/mipi_camera: SSDT changes to add DSM This change updates mipi_camera driver to add DSM section to SSDT. Change-Id: Ic60e972b6aebad171a7b77fe0d99781693adfb20 Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> --- M src/drivers/intel/mipi_camera/camera.c 1 file changed, 83 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/42468/1 diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index 81cd968..a3a9323 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -13,6 +13,86 @@ #define SENSOR_NAME_UUID "822ace8f-2814-4174-a56b-5f029fe079ee" #define SENSOR_TYPE_UUID "26257549-9271-4ca4-bb43-c4899d5a4881" +static uint32_t address_for_dev_type(const struct device *dev, uint8_t dev_type) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + uint16_t i2c_bus = dev->bus ? dev->bus->secondary : 0xFFFF; + uint16_t i2c_addr = dev->path.i2c.device; + + switch (dev_type) { + case DEV_TYPE_SENSOR: + return (((uint32_t)i2c_bus) << 24 | ((uint32_t)i2c_addr) << 8 | + dev_type); + case DEV_TYPE_VCM: + return (((uint32_t)i2c_bus) << 24 | ((uint32_t)config->vcm_address) << 8 | + dev_type); + case DEV_TYPE_ROM: + return (((uint32_t)i2c_bus) << 24 | ((uint32_t)config->rom_address) << 8 | + dev_type); + } + + return 0; +} + +static void camera_generate_dsm(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + int local1_ret = (config->ssdb.vcm_type ? 1 : 0) + (config->ssdb.rom_type ? 1 : 0) + 1; + int next_local1 = 1; + /* Method (_DSM, 4, NotSerialized) */ + acpigen_write_method("_DSM", 4); + + /* 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(SENSOR_NAME_UUID); + 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(SENSOR_TYPE_UUID); + /* ToInteger (Arg2, Local1) */ + acpigen_write_to_integer(ARG2_OP, LOCAL1_OP); + + /* If (LEqual (Local1, 1)) */ + acpigen_write_if_lequal_op_int(LOCAL1_OP, next_local1++); + acpigen_write_return_integer(local1_ret); + acpigen_pop_len(); /* If Arg2=1 */ + + /* If (LEqual (Local1, 2)) */ + acpigen_write_if_lequal_op_int(LOCAL1_OP, next_local1++); + acpigen_write_return_integer(address_for_dev_type(dev, DEV_TYPE_SENSOR)); + acpigen_pop_len(); /* If Arg2=2 */ + + if (config->ssdb.vcm_type) { + /* If (LEqual (Local1, 3)) */ + acpigen_write_if_lequal_op_int(LOCAL1_OP, next_local1++); + acpigen_write_return_integer(address_for_dev_type(dev, DEV_TYPE_VCM)); + 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, next_local1); + acpigen_write_return_integer(address_for_dev_type(dev, DEV_TYPE_ROM)); + 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 */ +} + static void camera_fill_sensor_defaults(struct drivers_intel_mipi_camera_config *config) { if (!config->ssdb.platform) @@ -63,6 +143,9 @@ if (!config->disable_ssdb_defaults) camera_fill_sensor_defaults(config); + /* _DSM */ + camera_generate_dsm(dev); + ep00 = acpi_dp_new_table("EP00"); acpi_dp_add_integer(ep00, "endpoint", 0); acpi_dp_add_integer(ep00, "clock-lanes", 0); -- To view, visit https://review.coreboot.org/c/coreboot/+/42468 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic60e972b6aebad171a7b77fe0d99781693adfb20 Gerrit-Change-Number: 42468 Gerrit-PatchSet: 1 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

5 14

Change in coreboot[master]: drivers/intel/mipi_camera: SSDT changes to handle camera sensor
by Sugnan Prabhu S (Code Review) 07 Jul '20

07 Jul '20

Hello Matt Delco, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42467 to review the following change. Change subject: drivers/intel/mipi_camera: SSDT changes to handle camera sensor ...................................................................... drivers/intel/mipi_camera: SSDT changes to handle camera sensor This change updates mipi_camera driver to handle camera sensor. Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 179 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/1 diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index 955c430..81cd968 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -10,6 +10,141 @@ #include <device/pci_def.h> #include "chip.h" +#define SENSOR_NAME_UUID "822ace8f-2814-4174-a56b-5f029fe079ee" +#define SENSOR_TYPE_UUID "26257549-9271-4ca4-bb43-c4899d5a4881" + +static void camera_fill_sensor_defaults(struct drivers_intel_mipi_camera_config *config) +{ + if (!config->ssdb.platform) + config->ssdb.platform = PLATFORM_SKC; + + if (!config->ssdb.flash_support) + config->ssdb.flash_support = FLASH_DISABLE; + + if (!config->ssdb.privacy_led) + config->ssdb.privacy_led = PRIVACY_LED_A_16mA; + + if (!config->ssdb.mipi_define) + config->ssdb.mipi_define = MIPI_INFO_ACPI_DEFINED; + + if (!config->ssdb.mclk_speed) + config->ssdb.mclk_speed = CLK_FREQ_19_2MHZ; +} + +/* + * Adds settings for a camera sensor device (typically at "\_SB.PCI0.I2Cx.CAMy"). The drivers + * for Linux tends to expect the camera sensor device and any related nvram / vcm devices to be + * separate ACPI devices, while the drivers for Windows want all of these to be grouped + * together in the camera sensor ACPI device. This implementation tries to satisfy both, + * though the unfortunate tradeoff is that the same I2C address for nvram and vcm is advertised + * by multiple devices in ACPI (via "_CRS"). The Windows driver can use the "_DSM" method to + * disambiguate the I2C resources in the camera sensor ACPI device. Drivers for Windows + * typically query "SSDB" for configuration information (represented as a binary blob dump of + * struct), while Linux drivers typically consult individual parameters in "_DSD". + * + * The tree of tables in "_DSD" is analogous to what's used for the "CIO2" device. The _DSD + * specifies a child table for the sensor's port (e.g., PRT0 for "port0"--this implementation + * assumes a camera only has 1 port). The PRT0 table specifies a table for each endpoint + * (though only 1 endpoint is supported by this implementation so the table only has an + * "endpoint0" that points to a EP00 table). The EP00 table primarily describes the # of lanes + * in "data-lines", a list of frequencies in "list-frequencies", and specifies the name of the + * other side in "remote-endpoint" (typically "\_SB.PCI0.CIO2"). + */ +static void camera_fill_sensor(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *ep00 = NULL; + struct acpi_dp *prt0 = NULL; + struct acpi_dp *dsd = NULL; + struct acpi_dp *remote = NULL; + const char *vcm_name = NULL; + struct acpi_dp *lens_focus = NULL; + + if (!config->disable_ssdb_defaults) + camera_fill_sensor_defaults(config); + + ep00 = acpi_dp_new_table("EP00"); + acpi_dp_add_integer(ep00, "endpoint", 0); + acpi_dp_add_integer(ep00, "clock-lanes", 0); + + if (config->ssdb.lanes_used > 0) { + struct acpi_dp *lanes = acpi_dp_new_table("data-lanes"); + uint32_t i; + for (i = 1; i <= config->ssdb.lanes_used; ++i) + acpi_dp_add_integer(lanes, NULL, i); + acpi_dp_add_array(ep00, lanes); + } + + if (config->num_freq_entries) { + struct acpi_dp *freq = acpi_dp_new_table("link-frequencies"); + uint32_t i; + for (i = 0; i < config->num_freq_entries && i < MAX_LINK_FREQ_ENTRIES; ++i) + acpi_dp_add_integer(freq, NULL, config->link_freq[i]); + acpi_dp_add_array(ep00, freq); + } + + remote = acpi_dp_new_table("remote-endpoint"); + + acpi_dp_add_reference(remote, NULL, config->remote_name); + acpi_dp_add_integer(remote, NULL, config->ssdb.link_used); + acpi_dp_add_integer(remote, NULL, 0); /* endpoint */ + acpi_dp_add_array(ep00, remote); + + prt0 = acpi_dp_new_table("PRT0"); + + acpi_dp_add_integer(prt0, "port", 0); + acpi_dp_add_child(prt0, "endpoint0", ep00); + dsd = acpi_dp_new_table("_DSD"); + + acpi_dp_add_integer(dsd, "clock-frequency", config->ssdb.mclk_speed); + + if (config->ssdb.degree) + acpi_dp_add_integer(dsd, "rotation", 180); + + if (config->ssdb.vcm_type) { + if (config->vcm_name) { + vcm_name = config->vcm_name; + } else { + const struct device_path path = { + .type = DEVICE_PATH_I2C, + .i2c.device = config->vcm_address, + }; + struct device *vcm_dev = find_dev_path(dev->bus, &path); + struct drivers_intel_mipi_camera_config *vcm_config; + vcm_config = vcm_dev ? vcm_dev->chip_info : NULL; + + if (!vcm_config) + printk(BIOS_ERR, "Failed to get VCM\n"); + else if (vcm_config->device_type != INTEL_ACPI_CAMERA_VCM) + printk(BIOS_ERR, "Device isn't VCM\n"); + else + vcm_name = acpi_device_path(vcm_dev); + } + } + + if (vcm_name) { + lens_focus = acpi_dp_new_table("lens-focus"); + acpi_dp_add_reference(lens_focus, NULL, vcm_name); + acpi_dp_add_array(dsd, lens_focus); + } + + acpi_dp_add_child(dsd, "port0", prt0); + acpi_dp_write(dsd); + + acpigen_write_method_serialized("SSDB", 0); + acpigen_write_return_byte_buffer((uint8_t *)&config->ssdb, sizeof(config->ssdb)); + acpigen_pop_len(); /* Method */ + + /* Fill Power Sequencing Data */ + if (config->num_pwdb_entries > 0) { + acpigen_write_method_serialized("PWDB", 0); + acpigen_write_return_byte_buffer((uint8_t *)&config->pwdb, + sizeof(struct intel_pwdb) * + config->num_pwdb_entries); + acpigen_pop_len(); /* Method */ + } +} + static void camera_fill_nvm(const struct device *dev) { struct drivers_intel_mipi_camera_config *config = dev->chip_info; @@ -142,6 +277,9 @@ } switch (config->device_type) { + case INTEL_ACPI_CAMERA_SENSOR: + camera_fill_sensor(dev); + break; case INTEL_ACPI_CAMERA_VCM: camera_fill_vcm(dev); break; diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index 5aa2183..f55ee1d 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -6,6 +6,39 @@ #include <stdint.h> #define MAX_PWDB_ENTRIES 12 +#define MAX_PORT_ENTRIES 4 +#define MAX_LINK_FREQ_ENTRIES 4 + +enum camera_device_type { + DEV_TYPE_SENSOR = 0, + DEV_TYPE_VCM, + DEV_TYPE_ROM +}; + +enum intel_camera_platform_type { + PLATFORM_SKC = 9, + PLATFORM_CNL = 10 +}; + +enum intel_camera_flash_type { + FLASH_DEFAULT = 0, + FLASH_DISABLE = 2, + FLASH_ENABLE = 3 +}; + +enum intel_camera_led_type { + PRIVACY_LED_DEFAULT = 0, + PRIVACY_LED_A_16mA +}; + +enum intel_camera_mipi_info { + MIPI_INFO_SENSOR_DRIVER = 0, + MIPI_INFO_ACPI_DEFINED +}; + +#define CLK_FREQ_19_2MHZ 19200000 +#define CLK_FREQ_24MHZ 24000000 +#define CLK_FREQ_20MHZ 20000000 enum intel_camera_device_type { INTEL_ACPI_CAMERA_CIO2, @@ -65,6 +98,7 @@ uint8_t degree; /* Camera Orientation */ uint8_t mipi_define; /* MIPI info defined in ACPI or sensor driver */ + uint32_t mclk_speed; /* Clock info for sensor */ uint32_t mclk; /* Clock info for sensor */ uint8_t control_logic_id; /* PMIC device node used for the camera sensor */ @@ -91,6 +125,13 @@ const char *acpi_name; const char *chip_name; unsigned int acpi_uid; + + /* Settings specific to camera sensor */ + bool disable_ssdb_defaults; + + uint8_t num_freq_entries; /* # of elements in link_freq */ + uint32_t link_freq[MAX_LINK_FREQ_ENTRIES]; + const char *sensor_name; /* default "UNKNOWN" */ const char *remote_name; /* default "\_SB.PCI0.CIO2" */ const char *vcm_name; /* defaults to |vcm_address| device */ uint16_t rom_address; /* I2C to use if ssdb.rom_type != 0 */ -- To view, visit https://review.coreboot.org/c/coreboot/+/42467 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Gerrit-Change-Number: 42467 Gerrit-PatchSet: 1 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

6 21

Change in coreboot[master]: drivers/intel/mipi_camera: Handle NVM and VCM device type
by Sugnan Prabhu S (Code Review) 07 Jul '20

07 Jul '20

Hello Matt Delco, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42466 to review the following change. Change subject: drivers/intel/mipi_camera: Handle NVM and VCM device type ...................................................................... drivers/intel/mipi_camera: Handle NVM and VCM device type This change adds support in mipi_camera driver to handle NVM and VCM device types. Change-Id: I24cb7f010d89bc8d14e0b4c8fe693ba6e9c68941 Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 65 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/42466/1 diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index 2f4b953..955c430 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -10,6 +10,46 @@ #include <device/pci_def.h> #include "chip.h" +static void camera_fill_nvm(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *dsd = acpi_dp_new_table("_DSD"); + + /* It might be possible to default size or width based on type. */ + if (!config->disable_nvm_defaults && !config->nvm_pagesize) + config->nvm_pagesize = 1; + + if (!config->disable_nvm_defaults && !config->nvm_readonly) + config->nvm_readonly = 1; + + if (config->nvm_size) + acpi_dp_add_integer(dsd, "size", config->nvm_size); + + if (config->nvm_pagesize) + acpi_dp_add_integer(dsd, "pagesize", config->nvm_pagesize); + + if (config->nvm_readonly) + acpi_dp_add_integer(dsd, "read-only", config->nvm_readonly); + + if (config->nvm_width) + acpi_dp_add_integer(dsd, "address-width", config->nvm_width); + + acpi_dp_write(dsd); +} + +static void camera_fill_vcm(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + struct acpi_dp *dsd; + + if (!config->vcm_compat) + return; + + dsd = acpi_dp_new_table("_DSD"); + acpi_dp_add_string(dsd, "compatible", config->vcm_compat); + acpi_dp_write(dsd); +} + static void write_pci_camera_device(const struct device *dev, const char *scope) { struct drivers_intel_mipi_camera_config *config = dev->chip_info; @@ -100,6 +140,19 @@ config->device_type == INTEL_ACPI_CAMERA_VCM) { acpigen_write_name_integer("CAMD", config->device_type); } + + switch (config->device_type) { + case INTEL_ACPI_CAMERA_VCM: + camera_fill_vcm(dev); + break; + case INTEL_ACPI_CAMERA_NVM: + camera_fill_nvm(dev); + break; + case INTEL_ACPI_CAMERA_IMGU: + case INTEL_ACPI_CAMERA_PMIC: + default: + break; + } } static void camera_fill_ssdt(const struct device *dev) diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index c029570..5aa2183 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -95,6 +95,18 @@ const char *vcm_name; /* defaults to |vcm_address| device */ uint16_t rom_address; /* I2C to use if ssdb.rom_type != 0 */ uint16_t vcm_address; /* I2C to use if ssdb.vcm_type != 0 */ + /* Settings specific to nvram. Many values, if left as zero, will be assigned a default. + Set disable_nvm_defaults to non-zero if you want to disable the defaulting behavior + so you can use zero for a value. */ + + bool disable_nvm_defaults; + uint32_t nvm_size; + uint32_t nvm_pagesize; + uint32_t nvm_readonly; + uint32_t nvm_width; + + /* Settings specific to vcm */ + const char *vcm_compat; }; #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/42466 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I24cb7f010d89bc8d14e0b4c8fe693ba6e9c68941 Gerrit-Change-Number: 42466 Gerrit-PatchSet: 1 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

4 10

Change in coreboot[master]: drivers/intel/mipi_camera: Handle acpi_name and common code
by Patrick Georgi (Code Review) 07 Jul '20

07 Jul '20

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41607 ) Change subject: drivers/intel/mipi_camera: Handle acpi_name and common code ...................................................................... drivers/intel/mipi_camera: Handle acpi_name and common code This change updates the mipi_camera driver to handle acpi_name based on device_type, if acpi_name is not set in the devicetree and moves some of the common code to separate methods. Change-Id: I15979f345fb823df2560db269e902a1ea650b69e Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41607 Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 130 insertions(+), 24 deletions(-) Approvals: build bot (Jenkins): Verified Rizwan Qureshi: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index 61f72ab..4356b0a 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -7,12 +7,27 @@ #include <device/i2c_simple.h> #include <device/device.h> #include <device/path.h> +#include <device/pci_def.h> #include "chip.h" -static void camera_fill_ssdt(const struct device *dev) +static void write_pci_camera_device(const struct device *dev) { struct drivers_intel_mipi_camera_config *config = dev->chip_info; - const char *scope = acpi_device_scope(dev); + + if (dev->path.type != DEVICE_PATH_PCI) { + printk(BIOS_ERR, "CIO2/IMGU devices require PCI\n"); + return; + } + + acpigen_write_device(acpi_device_name(dev)); + acpigen_write_ADR_pci_device(dev); + acpigen_write_name_string("_DDN", config->device_type == INTEL_ACPI_CAMERA_CIO2 ? + "Camera and Imaging Subsystem" : "Imaging Unit"); +} + +static void write_i2c_camera_device(const struct device *dev, const char *scope) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; struct acpi_i2c i2c = { .address = dev->path.i2c.device, .mode_10bit = dev->path.i2c.mode_10bit, @@ -20,13 +35,23 @@ .resource = scope, }; - if (!dev->enabled || !scope) + if (dev->path.type != DEVICE_PATH_I2C) { + printk(BIOS_ERR, "Non-CIO2/IMGU devices require I2C\n"); return; + } - /* Device */ - acpigen_write_scope(scope); acpigen_write_device(acpi_device_name(dev)); - acpigen_write_name_string("_HID", config->acpi_hid); + + if (config->device_type == INTEL_ACPI_CAMERA_SENSOR) + acpigen_write_name_integer("_ADR", 0); + + if (config->acpi_hid) + acpigen_write_name_string("_HID", config->acpi_hid); + else if (config->device_type == INTEL_ACPI_CAMERA_VCM) + acpigen_write_name_string("_HID", ACPI_DT_NAMESPACE_HID); + else if (config->device_type == INTEL_ACPI_CAMERA_NVM) + acpigen_write_name_string("_HID", "INT3499"); + acpigen_write_name_integer("_UID", config->acpi_uid); acpigen_write_name_string("_DDN", config->chip_name); acpigen_write_STA(acpi_device_status(dev)); @@ -35,35 +60,111 @@ acpigen_write_name("_CRS"); acpigen_write_resourcetemplate_header(); acpi_device_write_i2c(&i2c); - acpigen_write_resourcetemplate_footer(); - /* Mark it as Camera related device */ - acpigen_write_name_integer("CAMD", config->device_type); - - /* Create Device specific data */ - if (config->device_type == INTEL_ACPI_CAMERA_SENSOR) { - acpigen_write_method_serialized("SSDB", 0); - acpigen_write_return_byte_buffer((uint8_t *)&config->ssdb, - sizeof(config->ssdb)); - acpigen_pop_len(); /* Method */ + /* + * The optional vcm/nvram devices are presumed to be on the same I2C bus as the camera + * sensor. + */ + if (config->device_type == INTEL_ACPI_CAMERA_SENSOR && + config->ssdb.vcm_type && config->vcm_address) { + struct acpi_i2c i2c_vcm = i2c; + i2c_vcm.address = config->vcm_address; + acpi_device_write_i2c(&i2c_vcm); } - /* Fill Power Sequencing Data */ - acpigen_write_method_serialized("PWDB", 0); - acpigen_write_return_byte_buffer((uint8_t *)&config->pwdb, - (sizeof(struct intel_pwdb) * config->num_pwdb_entries)); - acpigen_pop_len(); /* Method */ + if (config->device_type == INTEL_ACPI_CAMERA_SENSOR && + config->ssdb.rom_type && config->rom_address) { + struct acpi_i2c i2c_rom = i2c; + i2c_rom.address = config->rom_address; + acpi_device_write_i2c(&i2c_rom); + } + + acpigen_write_resourcetemplate_footer(); +} + +static void write_camera_device_common(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + + /* Mark it as Camera related device */ + if (config->device_type == INTEL_ACPI_CAMERA_CIO2 || + config->device_type == INTEL_ACPI_CAMERA_IMGU || + config->device_type == INTEL_ACPI_CAMERA_SENSOR || + config->device_type == INTEL_ACPI_CAMERA_VCM) { + acpigen_write_name_integer("CAMD", config->device_type); + } +} + +static void camera_fill_ssdt(const struct device *dev) +{ + struct drivers_intel_mipi_camera_config *config = dev->chip_info; + const char *scope = acpi_device_scope(dev); + + if (!dev->enabled || !scope) + return; + + /* Device */ + acpigen_write_scope(scope); + + if (config->device_type == INTEL_ACPI_CAMERA_CIO2 || + config->device_type == INTEL_ACPI_CAMERA_IMGU) + write_pci_camera_device(dev); + else + write_i2c_camera_device(dev, scope); + + write_camera_device_common(dev); acpigen_pop_len(); /* Device */ acpigen_pop_len(); /* Scope */ - printk(BIOS_INFO, "%s: %s address 0%xh\n", acpi_device_path(dev), - dev->chip_ops->name, dev->path.i2c.device); + + if (dev->path.type == DEVICE_PATH_PCI) { + printk(BIOS_INFO, "%s: %s PCI address 0%x\n", acpi_device_path(dev), + dev->chip_ops->name, dev->path.pci.devfn); + } else { + printk(BIOS_INFO, "%s: %s I2C address 0%xh\n", acpi_device_path(dev), + dev->chip_ops->name, dev->path.i2c.device); + } } static const char *camera_acpi_name(const struct device *dev) { + const char *prefix = NULL; + static char name[5]; struct drivers_intel_mipi_camera_config *config = dev->chip_info; - return config->acpi_name; + + if (config->acpi_name) + return config->acpi_name; + + switch (config->device_type) { + case INTEL_ACPI_CAMERA_CIO2: + return "CIO2"; + case INTEL_ACPI_CAMERA_IMGU: + return "IMGU"; + case INTEL_ACPI_CAMERA_PMIC: + return "PMIC"; + case INTEL_ACPI_CAMERA_SENSOR: + prefix = "CAM"; + break; + case INTEL_ACPI_CAMERA_VCM: + prefix = "VCM"; + break; + case INTEL_ACPI_CAMERA_NVM: + prefix = "NVM"; + break; + default: + printk(BIOS_ERR, "Invalid device type: %x\n", config->device_type); + return NULL; + } + + /* + * The camera # knows which link # they use, so that's used as the basis for the + * instance #. The VCM and NVM don't have this information, so the best we can go on is + * the _UID. + */ + snprintf(name, sizeof(name), "%s%1u", prefix, + config->device_type == INTEL_ACPI_CAMERA_SENSOR ? + config->ssdb.link_used : config->acpi_uid); + return name; } static struct device_operations camera_ops = { diff --git a/src/drivers/intel/mipi_camera/chip.h b/src/drivers/intel/mipi_camera/chip.h index 7b6c5d5..c029570 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -12,6 +12,7 @@ INTEL_ACPI_CAMERA_IMGU, INTEL_ACPI_CAMERA_SENSOR, INTEL_ACPI_CAMERA_VCM, + INTEL_ACPI_CAMERA_NVM, INTEL_ACPI_CAMERA_PMIC = 100, }; @@ -90,6 +91,10 @@ const char *acpi_name; const char *chip_name; unsigned int acpi_uid; + const char *remote_name; /* default "\_SB.PCI0.CIO2" */ + const char *vcm_name; /* defaults to |vcm_address| device */ + uint16_t rom_address; /* I2C to use if ssdb.rom_type != 0 */ + uint16_t vcm_address; /* I2C to use if ssdb.vcm_type != 0 */ }; #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/41607 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I15979f345fb823df2560db269e902a1ea650b69e Gerrit-Change-Number: 41607 Gerrit-PatchSet: 33 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Daniel Kang <daniel.h.kang(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Kiran2 Kumar <kiran2.kumar(a)intel.corp-partner.google.com> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2020