coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: mb/clevo/n130wu: Unhide P2SB device
by Felix Singer (Code Review) 08 May '21

08 May '21

Attention is currently required from: Michael Niewöhner. Hello build bot (Jenkins), Michael Niewöhner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/53938 to look at the new patch set (#3). Change subject: mb/clevo/n130wu: Unhide P2SB device ...................................................................... mb/clevo/n130wu: Unhide P2SB device There is no need to make P2SB hidden. Thus, enable it by removing it from mainboard devicetree, since it it is already enabled by chipset devicetree. Untested yet. Change-Id: I2e6b2e2c8dc9d08b4260fc7bc9d7795e2747de75 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/clevo/kbl-u/variants/n13xwu/devicetree.cb 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/53938/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/53938 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2e6b2e2c8dc9d08b4260fc7bc9d7795e2747de75 Gerrit-Change-Number: 53938 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/clevo/n130wu: Use device alias names in devicetree
by Felix Singer (Code Review) 08 May '21

08 May '21

Attention is currently required from: Felix Singer, Michael Niewöhner. Hello build bot (Jenkins), Michael Niewöhner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/53939 to look at the new patch set (#4). Change subject: mb/clevo/n130wu: Use device alias names in devicetree ...................................................................... mb/clevo/n130wu: Use device alias names in devicetree Switch to device alias names in devicetree. Remove unnecessary comments since the names are self-explanatory. Change-Id: Id486d9bd44bd7ba6a93a5f757af487b211e58efa Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/clevo/kbl-u/variants/n13xwu/devicetree.cb 1 file changed, 17 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/53939/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/53939 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id486d9bd44bd7ba6a93a5f757af487b211e58efa Gerrit-Change-Number: 53939 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/amd/picasso: move acpigen_dptc_call_alib to new common alib
by Felix Held (Code Review) 08 May '21

08 May '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/53940 ) Change subject: soc/amd/picasso: move acpigen_dptc_call_alib to new common alib ...................................................................... soc/amd/picasso: move acpigen_dptc_call_alib to new common alib Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ib0f7da12429b6278d1e4bc5d6650c7ee0f3b5209 --- M src/soc/amd/common/block/acpi/Kconfig M src/soc/amd/common/block/acpi/Makefile.inc A src/soc/amd/common/block/acpi/alib.c M src/soc/amd/common/block/include/amdblocks/alib.h M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/root_complex.c 6 files changed, 26 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/53940/1 diff --git a/src/soc/amd/common/block/acpi/Kconfig b/src/soc/amd/common/block/acpi/Kconfig index 716610e..846db88 100644 --- a/src/soc/amd/common/block/acpi/Kconfig +++ b/src/soc/amd/common/block/acpi/Kconfig @@ -5,5 +5,8 @@ help Select this option to use the AcpiMmio ACPI registers. +config SOC_AMD_COMMON_BLOCK_ACPI_ALIB + bool + config SOC_AMD_COMMON_BLOCK_ACPI_GPIO bool diff --git a/src/soc/amd/common/block/acpi/Makefile.inc b/src/soc/amd/common/block/acpi/Makefile.inc index e14dba2..9f774d4 100644 --- a/src/soc/amd/common/block/acpi/Makefile.inc +++ b/src/soc/amd/common/block/acpi/Makefile.inc @@ -9,6 +9,7 @@ ramstage-y += pm_state.c ramstage-y += tables.c +ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI_ALIB) += alib.c ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI_GPIO) += gpio.c endif # CONFIG_SOC_AMD_COMMON_BLOCK_ACPI diff --git a/src/soc/amd/common/block/acpi/alib.c b/src/soc/amd/common/block/acpi/alib.c new file mode 100644 index 0000000..5e1e8fb --- /dev/null +++ b/src/soc/amd/common/block/acpi/alib.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpigen.h> +#include <amdblocks/alib.h> +#include <types.h> + +void acpigen_dptc_call_alib(const char *buf_name, uint8_t *buffer, size_t size) +{ + /* Name (buf_name, Buffer(size) {...} */ + acpigen_write_name(buf_name); + acpigen_write_byte_buffer(buffer, size); + + /* \_SB.ALIB(0xc, buf_name) */ + acpigen_emit_namestring("\\_SB.ALIB"); + acpigen_write_integer(ALIB_FUNCTION_DYNAMIC_POWER_THERMAL_CONFIG); + acpigen_emit_namestring(buf_name); +} diff --git a/src/soc/amd/common/block/include/amdblocks/alib.h b/src/soc/amd/common/block/include/amdblocks/alib.h index 5e92924..b9d34c5 100644 --- a/src/soc/amd/common/block/include/amdblocks/alib.h +++ b/src/soc/amd/common/block/include/amdblocks/alib.h @@ -21,6 +21,8 @@ uint32_t value; } __packed; +void acpigen_dptc_call_alib(const char *buf_name, uint8_t *buffer, size_t size); + #endif /* !__ACPI__ */ #endif /* AMD_COMMON_ALIB_H */ diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 2f36f2e..e69fb62 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -28,6 +28,7 @@ select SOC_AMD_COMMON_BLOCK_ACP select SOC_AMD_COMMON_BLOCK_ACPI select SOC_AMD_COMMON_BLOCK_ACPIMMIO + select SOC_AMD_COMMON_BLOCK_ACPI_ALIB select SOC_AMD_COMMON_BLOCK_ACPI_GPIO select SOC_AMD_COMMON_BLOCK_AOAC select SOC_AMD_COMMON_BLOCK_APOB diff --git a/src/soc/amd/picasso/root_complex.c b/src/soc/amd/picasso/root_complex.c index 979da3e..8f79b7a 100644 --- a/src/soc/amd/picasso/root_complex.c +++ b/src/soc/amd/picasso/root_complex.c @@ -177,18 +177,6 @@ setup_ioapic((u8 *)GNB_IO_APIC_ADDR, GNB_IOAPIC_ID); } -static void dptc_call_alib(const char *buf_name, uint8_t *buffer, size_t size) -{ - /* Name (buf_name, Buffer(size) {...} */ - acpigen_write_name(buf_name); - acpigen_write_byte_buffer(buffer, size); - - /* \_SB.ALIB(0xc, buf_name) */ - acpigen_emit_namestring("\\_SB.ALIB"); - acpigen_write_integer(ALIB_FUNCTION_DYNAMIC_POWER_THERMAL_CONFIG); - acpigen_emit_namestring(buf_name); -} - static void acipgen_dptci(void) { const struct soc_amd_picasso_config *config = config_of_soc(); @@ -214,13 +202,13 @@ /* If (LEqual ("\_SB.PCI0.LPCB.EC0.TBMD", 1)) */ acpigen_write_if_lequal_namestr_int("\\_SB.PCI0.LPCB.EC0.TBMD", 1); - dptc_call_alib("TABB", (uint8_t *)(void *)&tablet_mode_input, + acpigen_dptc_call_alib("TABB", (uint8_t *)(void *)&tablet_mode_input, sizeof(tablet_mode_input)); /* Else */ acpigen_write_else(); - dptc_call_alib("DEFB", (uint8_t *)(void *)&default_input, sizeof(default_input)); + acpigen_dptc_call_alib("DEFB", (uint8_t *)(void *)&default_input, sizeof(default_input)); acpigen_pop_len(); /* Else */ -- To view, visit https://review.coreboot.org/c/coreboot/+/53940 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib0f7da12429b6278d1e4bc5d6650c7ee0f3b5209 Gerrit-Change-Number: 53940 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
by Douglas Anderson (Code Review) 08 May '21

08 May '21

Attention is currently required from: Vinod Polimera, Julius Werner. Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52959 ) Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode" ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52959 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a Gerrit-Change-Number: 52959 Gerrit-PatchSet: 7 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 07 May 2021 23:20:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common/acpi/pci_int.asl: Allow IRQ sharing
by Felix Held (Code Review) 08 May '21

08 May '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52957 ) Change subject: soc/amd/common/acpi/pci_int.asl: Allow IRQ sharing ...................................................................... soc/amd/common/acpi/pci_int.asl: Allow IRQ sharing PCI interrupts are level active low, so they can be shared. BUG=b:184766519 TEST=Boot guybrush to OS with `pci=nomsi amd_iommu=off noapic` Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I439337dd66fe56790406c6d603e73512c806a19d Reviewed-on: https://review.coreboot.org/c/coreboot/+/52957 Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/acpi/pci_int.asl 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/src/soc/amd/common/acpi/pci_int.asl b/src/soc/amd/common/acpi/pci_int.asl index 06ba0cc..306995d 100644 --- a/src/soc/amd/common/acpi/pci_int.asl +++ b/src/soc/amd/common/acpi/pci_int.asl @@ -2,14 +2,14 @@ /* PIC Possible Resource Values */ Name(IRQP, ResourceTemplate() { - Interrupt(ResourceConsumer, Level, ActiveLow, Exclusive, , , PIC){ + Interrupt(ResourceConsumer, Level, ActiveLow, Shared, , , PIC){ 1, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 14, 15 } }) /* IO-APIC Possible Resource Values */ Name(IRQI, ResourceTemplate() { - Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, , , APIC) { + Interrupt (ResourceConsumer, Level, ActiveLow, Shared, , , APIC) { 16, 17, 18, 19, 20, 21, 22, 23 } }) @@ -63,7 +63,7 @@ ResourceConsumer, \ Level, \ ActiveLow, \ - Exclusive, , , NUMB) \ + Shared, , , NUMB) \ { 0 } \ } \ CreateDWordField(local0, NUMB._INT, IRQN) \ -- To view, visit https://review.coreboot.org/c/coreboot/+/52957 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I439337dd66fe56790406c6d603e73512c806a19d Gerrit-Change-Number: 52957 Gerrit-PatchSet: 4 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
by Julius Werner (Code Review) 08 May '21

08 May '21

Attention is currently required from: Vinod Polimera, Douglas Anderson. Hello Vinod Polimera, build bot (Jenkins), Douglas Anderson, Xuxin Xiong, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52959 to look at the new patch set (#7). Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode" ...................................................................... drivers/sn65dsi86: Switch EDID reading to use "indirect mode" The SN65DSI86 eDP bridge supports two ways to read the EDID: for now we've been using "direct mode", which works by basically making the bridge I2C device listen to another chip address besides its own and proxy all requests received there directly to the eDP AUX channel. The great part about that mode is that it is super easy and hassle-free to use. The not so great part about it is that it doesn't work: for EDID extensions, the last byte (which happens to contain the checksum) is somehow always read as zero. We presume this is a hardware bug in the bridge part. The other, much more annoying way is "indirect mode", where each byte transmitted over the AUX channel has to be manually set up in the I2C registers of the bridge, just like we're already doing with DPCD transactions. Thankfully, we can reuse most of the DPCD code for this so it's not a lot of extra code. It's a bit slower but not as much as you'd expect (26ms instead of 18ms on my board), and the difference is not very relevant compared to common total times for display init. Also, some of the (previously unused) enum definitions for the AUX_CMD mode field of the bridge had just been plain wrong for some reason, and needed to be fixed to make this work. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 143 insertions(+), 75 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/52959/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/52959 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a Gerrit-Change-Number: 52959 Gerrit-PatchSet: 7 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/clevo/n130wu: Use device alias names in devicetree
by Felix Singer (Code Review) 08 May '21

08 May '21

Attention is currently required from: Michael Niewöhner. Hello build bot (Jenkins), Michael Niewöhner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/53939 to look at the new patch set (#3). Change subject: mb/clevo/n130wu: Use device alias names in devicetree ...................................................................... mb/clevo/n130wu: Use device alias names in devicetree Switch to device alias names in devicetree. Remove unnecessary comments since the names are self-explanatory. Change-Id: Id486d9bd44bd7ba6a93a5f757af487b211e58efa Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/clevo/kbl-u/variants/n13xwu/devicetree.cb 1 file changed, 16 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/53939/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/53939 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id486d9bd44bd7ba6a93a5f757af487b211e58efa Gerrit-Change-Number: 53939 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/clevo/n130wu: Unhide P2SB device
by Felix Singer (Code Review) 08 May '21

08 May '21

Attention is currently required from: Michael Niewöhner. Hello build bot (Jenkins), Michael Niewöhner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/53938 to look at the new patch set (#2). Change subject: mb/clevo/n130wu: Unhide P2SB device ...................................................................... mb/clevo/n130wu: Unhide P2SB device There is no need to make P2SB hidden. Thus, enable it by removing it from mainboard devicetree, since it it is already enabled by chipset devicetree. Change-Id: I2e6b2e2c8dc9d08b4260fc7bc9d7795e2747de75 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/clevo/kbl-u/variants/n13xwu/devicetree.cb 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/53938/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/53938 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2e6b2e2c8dc9d08b4260fc7bc9d7795e2747de75 Gerrit-Change-Number: 53938 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
by Julius Werner (Code Review) 08 May '21

08 May '21

Attention is currently required from: Vinod Polimera, Douglas Anderson. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52959 ) Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode" ...................................................................... Patch Set 6: (1 comment) File src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c: https://review.coreboot.org/c/coreboot/+/52959/comment/62c69976_cde9c204 PS5, Line 253: if (i2c_writeb(bus, chip, SN_AUX_CMD_REG, AUX_CMD_SEND | (cmd << 4)) || : !wait_ms(100, !i2c_readb(bus, chip, SN_AUX_CMD_REG, &buf) && : !(buf & AUX_CMD_SEND)) || : i2c_readb(bus, chip, SN_AUX_CMD_STATUS_REG, &buf) || : (buf & (NAT_I2C_FAIL | AUX_SHORT | AUX_DFER | AUX_RPLY_TOUT))) { : printk(BIOS_ERR, "ERROR: aux command send failed\n"); : return CB_ERR; : } > If it were me, I wouldn't have written separate commands. […] Updated the error handling. Let's ignore the SHORT issue until we actually see it be a problem, I don't see any reason why this should happen in practice. -- To view, visit https://review.coreboot.org/c/coreboot/+/52959 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a Gerrit-Change-Number: 52959 Gerrit-PatchSet: 6 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Comment-Date: Fri, 07 May 2021 23:09:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Douglas Anderson <dianders(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
by Julius Werner (Code Review) 08 May '21

08 May '21

Attention is currently required from: Vinod Polimera, Julius Werner. Hello Vinod Polimera, build bot (Jenkins), Douglas Anderson, Xuxin Xiong, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52959 to look at the new patch set (#6). Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode" ...................................................................... drivers/sn65dsi86: Switch EDID reading to use "indirect mode" The SN65DSI86 eDP bridge supports two ways to read the EDID: for now we've been using "direct mode", which works by basically making the bridge I2C device listen to another chip address besides its own and proxy all requests received there directly to the eDP AUX channel. The great part about that mode is that it is super easy and hassle-free to use. The not so great part about it is that it doesn't work: for EDID extensions, the last byte (which happens to contain the checksum) is somehow always read as zero. We presume this is a hardware bug in the bridge part. The other, much more annoying way is "indirect mode", where each byte transmitted over the AUX channel has to be manually set up in the I2C registers of the bridge, just like we're already doing with DPCD transactions. Thankfully, we can reuse most of the DPCD code for this so it's not a lot of extra code. It's a bit slower but not as much as you'd expect (26ms instead of 18ms on my board), and the difference is not very relevant compared to common total times for display init. Also, some of the (previously unused) enum definitions for the AUX_CMD mode field of the bridge had just been plain wrong for some reason, and needed to be fixed to make this work. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 138 insertions(+), 75 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/52959/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/52959 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a Gerrit-Change-Number: 52959 Gerrit-PatchSet: 6 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021