Change in coreboot[master]: drivers/intel/mipi_camera: SSDT changes to handle camera sensor
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@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 */
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#2).
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@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 181 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: SSDT changes to handle camera sensor ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42467/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42467/2//COMMIT_MSG@7 PS2, Line 7: drivers/intel/mipi_camera: SSDT changes to handle camera sensor Please make it a statement by adding a verb (in imperative mood).
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: SSDT changes to handle camera sensor ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 90: 0); /* endpoint */ nit: could use a macro, #define DEFAULT_ENDPOINT 0, then the comment isn't necessary
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 25: FLASH_DISABLE = 2, Does 1 not mean anything?
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 39: #define CLK_FREQ_19_2MHZ 19200000 : #define CLK_FREQ_24MHZ 24000000 : #define CLK_FREQ_20MHZ 20000000 nit: line up the numbers with tabs
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 109: uint8_t reserved[13]; nit: maybe a comment why this was required?
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: SSDT changes to handle camera sensor ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 25: FLASH_DISABLE = 2,
Does 1 not mean anything?
These's no definition for 1 in skycam's dsdt.h.
Hello Matt Delco, build bot (Jenkins), Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#3).
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 182 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/3
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#4).
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 182 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/4
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#5).
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 183 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 25: FLASH_DISABLE = 2,
These's no definition for 1 in skycam's dsdt.h.
Ack
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 5: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/42467/5/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42467/5/src/drivers/intel/mipi_came... PS5, Line 68: 0 nit: DEFAULT_ENDPOINT 😊
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#6).
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 183 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/6
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42467/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42467/2//COMMIT_MSG@7 PS2, Line 7: drivers/intel/mipi_camera: SSDT changes to handle camera sensor
Please make it a statement by adding a verb (in imperative mood).
Done
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 90: 0); /* endpoint */
nit: could use a macro, #define DEFAULT_ENDPOINT 0, then the comment isn't necessary
Done
https://review.coreboot.org/c/coreboot/+/42467/5/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42467/5/src/drivers/intel/mipi_came... PS5, Line 68: 0
nit: DEFAULT_ENDPOINT 😊
Done
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... File src/drivers/intel/mipi_camera/chip.h:
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 39: #define CLK_FREQ_19_2MHZ 19200000 : #define CLK_FREQ_24MHZ 24000000 : #define CLK_FREQ_20MHZ 20000000
nit: line up the numbers with tabs
Done
https://review.coreboot.org/c/coreboot/+/42467/2/src/drivers/intel/mipi_came... PS2, Line 109: uint8_t reserved[13];
nit: maybe a comment why this was required?
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 6: Code-Review+2
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#7).
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 185 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/7
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 7: Code-Review+2
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#9).
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 187 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/9
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42467/11/src/drivers/intel/mipi_cam... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42467/11/src/drivers/intel/mipi_cam... PS11, Line 124: acpi_device_path since this is not used immediately, should we add a strdup here to ensure it's safe?
Hello build bot (Jenkins), Matt Delco, Tim Wawrzynczak, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42467
to look at the new patch set (#12).
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 187 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/42467/12
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42467/11/src/drivers/intel/mipi_cam... File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/42467/11/src/drivers/intel/mipi_cam... PS11, Line 124: acpi_device_path
since this is not used immediately, should we add a strdup here to ensure it's safe?
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 12: Code-Review+2
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
Patch Set 15: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42467 )
Change subject: drivers/intel/mipi_camera: Add support for camera sensor in SSDT ......................................................................
drivers/intel/mipi_camera: Add support for camera sensor in SSDT
This change updates mipi_camera driver to handle camera sensor.
Change-Id: I581c9bf9b87eac69e88ec11724c3b26ee5fa9431 Signed-off-by: Sugnan Prabhu S sugnan.prabhu.s@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42467 Reviewed-by: Rizwan Qureshi rizwan.qureshi@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/mipi_camera/camera.c M src/drivers/intel/mipi_camera/chip.h 2 files changed, 187 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Rizwan Qureshi: Looks good to me, approved
diff --git a/src/drivers/intel/mipi_camera/camera.c b/src/drivers/intel/mipi_camera/camera.c index b1075b8..5230a2a 100644 --- a/src/drivers/intel/mipi_camera/camera.c +++ b/src/drivers/intel/mipi_camera/camera.c @@ -10,6 +10,144 @@ #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" +#define DEFAULT_ENDPOINT 0 + +static void camera_fill_sensor_defaults(struct drivers_intel_mipi_camera_config *config) +{ + if (config->disable_ssdb_defaults) + return; + + 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; + + camera_fill_sensor_defaults(config); + + ep00 = acpi_dp_new_table("EP00"); + acpi_dp_add_integer(ep00, "endpoint", DEFAULT_ENDPOINT); + 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, DEFAULT_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; @@ -135,6 +273,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 b04a100..c962c9a 100644 --- a/src/drivers/intel/mipi_camera/chip.h +++ b/src/drivers/intel/mipi_camera/chip.h @@ -5,7 +5,41 @@
#include <stdint.h>
-#define MAX_PWDB_ENTRIES 12 +#define DEFAULT_LINK_FREQ 450000000 +#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,12 +99,16 @@ 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 */ uint8_t mipi_data_format; /* MIPI data format */ uint8_t silicon_version; /* Silicon version */ uint8_t customer_id; /* Customer ID */ + uint8_t mclk_port; + uint8_t reserved[13]; /* Pads SSDB out so the binary blob in ACPI is + the same size as seen on other firmwares.*/ } __packed;
struct intel_pwdb { @@ -91,6 +129,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 */
-
Matt Delco (Code Review) -
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Rizwan Qureshi (Code Review)
-
Sugnan Prabhu S (Code Review)
-
Tim Wawrzynczak (Code Review)