Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
soc/intel/skylake: enable GMM in devicetree
Enables Gaussian Mixture Model (GMM) only if the corresponding pci device is enabled in the device tree
Change-Id: I21409adf85b70bccc30dd8e12a03ad7921544b3c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/skylake/chip_fsp20.c M src/soc/intel/skylake/include/soc/pci_devs.h 2 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/35168/1
diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index 064f71e..c54de3a 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -441,6 +441,10 @@ dev->enabled = 0; params->XdciEnable = dev->enabled;
+ /* Enable or disable Gaussian Mixture Model in devicetree */ + dev = pcidev_path_on_root(SA_DEVFN_GMM); + params->GmmEnable = dev->enabled; + /* * Send VR specific mailbox commands: * 000b - no VR specific command sent diff --git a/src/soc/intel/skylake/include/soc/pci_devs.h b/src/soc/intel/skylake/include/soc/pci_devs.h index 0e8bb68..e7f860d 100644 --- a/src/soc/intel/skylake/include/soc/pci_devs.h +++ b/src/soc/intel/skylake/include/soc/pci_devs.h @@ -42,6 +42,9 @@ #define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) #define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
+#define SA_DEV_SLOT_GMM 0x08 +#define SA_DEVFN_GMM PCI_DEVFN(SA_DEV_SLOT_GMM, 0) +#define SA_DEV_GMM PCI_DEV(0, SA_DEV_SLOT_IGD, 0) /* PCH Devices */
#define PCH_DEV_SLOT_ISH 0x13
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35168
to look at the new patch set (#2).
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
soc/intel/skylake: enable GMM in devicetree
Enables Gaussian Mixture Model (GMM) only if the corresponding pci device is enabled in the device tree
Tested on Asrock H110M DVS motherboard
Change-Id: I21409adf85b70bccc30dd8e12a03ad7921544b3c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/skylake/chip_fsp20.c M src/soc/intel/skylake/include/soc/pci_devs.h 2 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/35168/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/chip_... File src/soc/intel/skylake/chip_fsp20.c:
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/chip_... PS2, Line 445: dev What if 'dev' is NULL? Then, 'dev->enabled' is a null pointer dereference...
*looks above* oh no...
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/inclu... PS2, Line 47: SA_DEV_SLOT_IGD Copypasta error, should be 'SA_DEV_SLOT_GMM'
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/chip_... File src/soc/intel/skylake/chip_fsp20.c:
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/chip_... PS2, Line 445: dev
What if 'dev' is NULL? Then, 'dev->enabled' is a null pointer dereference... […]
Take a look at CB:35173, which should address null pointer derefs on the other device-dependent configs.
Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35168
to look at the new patch set (#3).
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
soc/intel/skylake: enable GMM in devicetree
Enables Gaussian Mixture Model (GMM) only if the corresponding pci device is enabled in the device tree
Tested on Asrock H110M DVS motherboard
Change-Id: I21409adf85b70bccc30dd8e12a03ad7921544b3c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/skylake/chip_fsp20.c M src/soc/intel/skylake/include/soc/pci_devs.h 2 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/35168/3
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 3:
(2 comments)
Thanks for the review
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/chip_... File src/soc/intel/skylake/chip_fsp20.c:
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/chip_... PS2, Line 445: dev
Take a look at CB:35173, which should address null pointer derefs on the other device-dependent conf […]
stupid mistake for me :(
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/35168/2/src/soc/intel/skylake/inclu... PS2, Line 47: SA_DEV_SLOT_IGD
Copypasta error, should be 'SA_DEV_SLOT_GMM'
fixed. Thanks
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35168/3/src/soc/intel/skylake/chip_... File src/soc/intel/skylake/chip_fsp20.c:
https://review.coreboot.org/c/coreboot/+/35168/3/src/soc/intel/skylake/chip_... PS3, Line 446: (dev) why the ()?
Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35168
to look at the new patch set (#4).
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
soc/intel/skylake: enable GMM in devicetree
Enables Gaussian Mixture Model (GMM) only if the corresponding pci device is enabled in the device tree
Tested on Asrock H110M DVS motherboard
Change-Id: I21409adf85b70bccc30dd8e12a03ad7921544b3c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/skylake/chip_fsp20.c M src/soc/intel/skylake/include/soc/pci_devs.h 2 files changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/35168/4
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35168/3/src/soc/intel/skylake/chip_... File src/soc/intel/skylake/chip_fsp20.c:
https://review.coreboot.org/c/coreboot/+/35168/3/src/soc/intel/skylake/chip_... PS3, Line 446: (dev)
why the ()?
fixed Thanks
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/35168 )
Change subject: soc/intel/skylake: enable GMM in devicetree ......................................................................
soc/intel/skylake: enable GMM in devicetree
Enables Gaussian Mixture Model (GMM) only if the corresponding pci device is enabled in the device tree
Tested on Asrock H110M DVS motherboard
Change-Id: I21409adf85b70bccc30dd8e12a03ad7921544b3c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35168 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- M src/soc/intel/skylake/chip_fsp20.c M src/soc/intel/skylake/include/soc/pci_devs.h 2 files changed, 7 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index 064f71e..85d3edf 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -441,6 +441,10 @@ dev->enabled = 0; params->XdciEnable = dev->enabled;
+ /* Enable or disable Gaussian Mixture Model in devicetree */ + dev = pcidev_path_on_root(SA_DEVFN_GMM); + params->GmmEnable = dev ? dev->enabled : 0; + /* * Send VR specific mailbox commands: * 000b - no VR specific command sent diff --git a/src/soc/intel/skylake/include/soc/pci_devs.h b/src/soc/intel/skylake/include/soc/pci_devs.h index 0e8bb68..ff6b8c1 100644 --- a/src/soc/intel/skylake/include/soc/pci_devs.h +++ b/src/soc/intel/skylake/include/soc/pci_devs.h @@ -42,6 +42,9 @@ #define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) #define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
+#define SA_DEV_SLOT_GMM 0x08 +#define SA_DEVFN_GMM PCI_DEVFN(SA_DEV_SLOT_GMM, 0) +#define SA_DEV_GMM PCI_DEV(0, SA_DEV_SLOT_GMM, 0) /* PCH Devices */
#define PCH_DEV_SLOT_ISH 0x13