Nouveau - Jul 2019 - [PATCH 0/4] Refuse to load if the power cable are not connected

trello card:
https://trello.com/c/admzDRvd/50-reduce-performance-refuse-to-boot-if-not-all-the-power-connectors-are-plugged

Mark Menzynski (4):
  moved gpio so it is sorted by values
  gpio: checking if gpu power cable is connected
  gpio: added power check for another GPIO
  gpio: added power check for another GPIO

 drm/nouveau/include/nvkm/subdev/bios/gpio.h |  5 ++++-
 drm/nouveau/nvkm/subdev/gpio/base.c         | 25 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.21.0

Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
---
 drm/nouveau/include/nvkm/subdev/bios/gpio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
index b71a3555..2f40935f 100644
--- a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
+++ b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
@@ -3,9 +3,9 @@
 #define __NVBIOS_GPIO_H__
 enum dcb_gpio_func_name {
 	DCB_GPIO_PANEL_POWER = 0x01,
+	DCB_GPIO_FAN = 0x09,
 	DCB_GPIO_TVDAC0 = 0x0c,
 	DCB_GPIO_TVDAC1 = 0x2d,
-	DCB_GPIO_FAN = 0x09,
 	DCB_GPIO_FAN_SENSE = 0x3d,
 	DCB_GPIO_LOGO_LED_PWM = 0x84,
 	DCB_GPIO_UNUSED = 0xff,
-- 
2.21.0

Tested on TU104, GP106 and GF100

Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
---
 drm/nouveau/include/nvkm/subdev/bios/gpio.h |  1 +
 drm/nouveau/nvkm/subdev/gpio/base.c         | 23 +++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
index 2f40935f..a70ec9e8 100644
--- a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
+++ b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
@@ -7,6 +7,7 @@ enum dcb_gpio_func_name {
 	DCB_GPIO_TVDAC0 = 0x0c,
 	DCB_GPIO_TVDAC1 = 0x2d,
 	DCB_GPIO_FAN_SENSE = 0x3d,
+	DCB_GPIO_EXT_POWER_LOW = 0x79,
 	DCB_GPIO_LOGO_LED_PWM = 0x84,
 	DCB_GPIO_UNUSED = 0xff,
 	DCB_GPIO_VID0 = 0x04,
diff --git a/drm/nouveau/nvkm/subdev/gpio/base.c
b/drm/nouveau/nvkm/subdev/gpio/base.c
index 1399d923..c4685807 100644
--- a/drm/nouveau/nvkm/subdev/gpio/base.c
+++ b/drm/nouveau/nvkm/subdev/gpio/base.c
@@ -182,12 +182,35 @@ static const struct dmi_system_id gpio_reset_ids[] = {
 	{ }
 };
 
+static enum dcb_gpio_func_name power_checks[] = {
+	DCB_GPIO_EXT_POWER_LOW,
+};
+
 static int
 nvkm_gpio_init(struct nvkm_subdev *subdev)
 {
 	struct nvkm_gpio *gpio = nvkm_gpio(subdev);
+	struct dcb_gpio_func func;
+	int ret;
+	int i;
+
 	if (dmi_check_system(gpio_reset_ids))
 		nvkm_gpio_reset(gpio, DCB_GPIO_UNUSED);
+
+	for (i = 0; i < ARRAY_SIZE(power_checks); ++i) {
+		ret = nvkm_gpio_find(gpio, 0, power_checks[i], DCB_GPIO_UNUSED,
+				     &func);
+		if (ret)
+			continue;
+
+		ret = nvkm_gpio_get(gpio, 0, func.func, func.line);
+		if (ret) {
+			nvkm_error(&gpio->subdev,
+				   "not enough power, check GPU power cable\n");
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.21.0

this one is mainly used on Rankine and Curie and rarely on Tesla GPUs

untested, wrote according to documentation

Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
---
 drm/nouveau/include/nvkm/subdev/bios/gpio.h | 1 +
 drm/nouveau/nvkm/subdev/gpio/base.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
index a70ec9e8..fc2b5fb0 100644
--- a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
+++ b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
@@ -5,6 +5,7 @@ enum dcb_gpio_func_name {
 	DCB_GPIO_PANEL_POWER = 0x01,
 	DCB_GPIO_FAN = 0x09,
 	DCB_GPIO_TVDAC0 = 0x0c,
+	DCB_GPIO_THERM_EXT_POWER_EVENT = 0x10,
 	DCB_GPIO_TVDAC1 = 0x2d,
 	DCB_GPIO_FAN_SENSE = 0x3d,
 	DCB_GPIO_EXT_POWER_LOW = 0x79,
diff --git a/drm/nouveau/nvkm/subdev/gpio/base.c
b/drm/nouveau/nvkm/subdev/gpio/base.c
index c4685807..0c886543 100644
--- a/drm/nouveau/nvkm/subdev/gpio/base.c
+++ b/drm/nouveau/nvkm/subdev/gpio/base.c
@@ -183,6 +183,7 @@ static const struct dmi_system_id gpio_reset_ids[] = {
 };
 
 static enum dcb_gpio_func_name power_checks[] = {
+	DCB_GPIO_THERM_EXT_POWER_EVENT,
 	DCB_GPIO_EXT_POWER_LOW,
 };
 
-- 
2.21.0

this one is mainly used on Tesla and somtimes on Fermi GPUs

untested, wrote according to documentation

Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
---
 drm/nouveau/include/nvkm/subdev/bios/gpio.h | 1 +
 drm/nouveau/nvkm/subdev/gpio/base.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
index fc2b5fb0..784c5f22 100644
--- a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
+++ b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
@@ -8,6 +8,7 @@ enum dcb_gpio_func_name {
 	DCB_GPIO_THERM_EXT_POWER_EVENT = 0x10,
 	DCB_GPIO_TVDAC1 = 0x2d,
 	DCB_GPIO_FAN_SENSE = 0x3d,
+	DCB_GPIO_POWER_ALERT = 0x4c,
 	DCB_GPIO_EXT_POWER_LOW = 0x79,
 	DCB_GPIO_LOGO_LED_PWM = 0x84,
 	DCB_GPIO_UNUSED = 0xff,
diff --git a/drm/nouveau/nvkm/subdev/gpio/base.c
b/drm/nouveau/nvkm/subdev/gpio/base.c
index 0c886543..755f016f 100644
--- a/drm/nouveau/nvkm/subdev/gpio/base.c
+++ b/drm/nouveau/nvkm/subdev/gpio/base.c
@@ -184,6 +184,7 @@ static const struct dmi_system_id gpio_reset_ids[] = {
 
 static enum dcb_gpio_func_name power_checks[] = {
 	DCB_GPIO_THERM_EXT_POWER_EVENT,
+	DCB_GPIO_POWER_ALERT,
 	DCB_GPIO_EXT_POWER_LOW,
 };
 
-- 
2.21.0

On Fri, 12 Jul 2019 at 00:42, Mark Menzynski <mmenzyns at redhat.com>
wrote:

Hello Mark,

Thank you for your first kernel patch series to nouveau!
This is a good starting point. However, I would suggest a couple of general
improvements which apply to the whole series:

1. Invest a little bit more time in writing the commit messages.

There is a slightly subjective element to what makes a "good" commit
message, but generally the principle is that it should concisely
convey the "what" of the patch, but more importantly the
"why" of the patch.

A future reader (which might be yourself!) can understand *what* the patch
does from the code, to a lesser or so amount of time.
The *why* of a patch really can only be communicated within the commit
message, so a little extra time spent here is well spent.

A helpful approach I suggest is trying to follow the style of the folder,
submodule or driver:

  $ git log -n20 drivers/gpu/drm/nouveau/

There's also a web resource here: https://chris.beams.io/posts/git-commit/
(although it's not Linux kernel specific)

2. Run ./scripts/checkpatch.pl on the final patch series

You may have already done this, as I noticed the current patches report no
errors or warnings. If so, well done!

It's a good habit to re-run this script against the final v2 patch series
again though.

You can also ask questions on freenode #nouveau where many of the nouveau
graphics stack driver developers hang out.

Rhys

trello card:
>
>
https://trello.com/c/admzDRvd/50-reduce-performance-refuse-to-boot-if-not-all-the-power-connectors-are-plugged
>
> Mark Menzynski (4):
>   moved gpio so it is sorted by values
>   gpio: checking if gpu power cable is connected
>   gpio: added power check for another GPIO
>   gpio: added power check for another GPIO
>
I'd recommend adding to the first line of the commit message of these two
patches something which distinguishes them
apart. e.g. it appears you've identified that there are different gpio pin
functions for different nvidia gpu families, so perhaps
use the pre- / and post- family names to separate the two patches for a
reader.

Also, take a look at how the prefix before the ":" is usually written
in
this area of the code:

  $ git log -n10 --oneline drivers/gpu/drm/nouveau/<relevant subfolders>

>
>  drm/nouveau/include/nvkm/subdev/bios/gpio.h |  5 ++++-
>  drm/nouveau/nvkm/subdev/gpio/base.c         | 25 +++++++++++++++++++++
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20190712/b0a630d1/attachment.html>

On Fri, 12 Jul 2019 at 00:42, Mark Menzynski <mmenzyns at redhat.com>
wrote:
> Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
>
In addition to the general comments I provided to this patch series, you
should add a prefix before ":" to the first line of
this commit message:

Maybe something like "drm/nouveau/bios/gpio: sort gpio function names by
value"

You can see the general approach with:

  $ git log -n10 --oneline
drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/gpio.h

---
>  drm/nouveau/include/nvkm/subdev/bios/gpio.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
> b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
> index b71a3555..2f40935f 100644
> --- a/drm/nouveau/include/nvkm/subdev/bios/gpio.h
> +++ b/drm/nouveau/include/nvkm/subdev/bios/gpio.h
> @@ -3,9 +3,9 @@
>  #define __NVBIOS_GPIO_H__
>  enum dcb_gpio_func_name {
>         DCB_GPIO_PANEL_POWER = 0x01,
> +       DCB_GPIO_FAN = 0x09,
>         DCB_GPIO_TVDAC0 = 0x0c,
>         DCB_GPIO_TVDAC1 = 0x2d,
> -       DCB_GPIO_FAN = 0x09,
>         DCB_GPIO_FAN_SENSE = 0x3d,
>         DCB_GPIO_LOGO_LED_PWM = 0x84,
>         DCB_GPIO_UNUSED = 0xff,
> --
> 2.21.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20190712/de648dd8/attachment.html>