Nouveau - Sep 2015 - [PATCH 2/2] gr/gf100: do not assume a PMU is present

Two trivial PMU-related fixes for Tegra:
- Add a dummy func member to GK20A, since nvkm_pmmu_pgob() unconditionally
  dereferences it
- Check whether a PMU actually exists before calling nvkm_pmmu_pgob(). During
  early bringup we are likely to not have a PMU

Alexandre Courbot (2):
  pmu/gk20a: add dummy func
  gr/gf100: do not assume a PMU is present

 drm/nouveau/nvkm/engine/gr/gf100.c  | 3 ++-
 drm/nouveau/nvkm/subdev/pmu/gk20a.c | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.5.1

Some parts of the driver expect the PMU to have a func member, so set
one for GK20A to avoid NULL pointer derefences.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drm/nouveau/nvkm/subdev/pmu/gk20a.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
index 6689d0290a7e..f46e3310e21a 100644
--- a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
+++ b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
@@ -202,6 +202,10 @@ gk20a_dvfs_data= {
 	.p_smooth = 1,
 };
 
+static const struct nvkm_pmu_func
+gk20a_pmu_func = {
+};
+
 static const struct nvkm_subdev_func
 gk20a_pmu = {
 	.init = gk20a_pmu_init,
@@ -221,6 +225,7 @@ gk20a_pmu_new(struct nvkm_device *device, int index, struct
nvkm_pmu **ppmu)
 	*ppmu = &pmu->base;
 
 	nvkm_subdev_ctor(&gk20a_pmu, device, index, 0, &pmu->base.subdev);
+	pmu->base.func = &gk20a_pmu_func;
 	pmu->data = &gk20a_dvfs_data;
 	nvkm_alarm_init(&pmu->alarm, gk20a_pmu_dvfs_work);
 	return 0;
-- 
2.5.1

Some devices may not have a PMU. Avoid a NULL pointer dereference in
such cases.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drm/nouveau/nvkm/engine/gr/gf100.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drm/nouveau/nvkm/engine/gr/gf100.c
b/drm/nouveau/nvkm/engine/gr/gf100.c
index f1358a564e3e..f252fa2d7cf9 100644
--- a/drm/nouveau/nvkm/engine/gr/gf100.c
+++ b/drm/nouveau/nvkm/engine/gr/gf100.c
@@ -1499,7 +1499,8 @@ gf100_gr_oneinit(struct nvkm_gr *base)
 	struct nvkm_device *device = gr->base.engine.subdev.device;
 	int ret, i, j;
 
-	nvkm_pmu_pgob(device->pmu, false);
+	if (device->pmu)
+		nvkm_pmu_pgob(device->pmu, false);
 
 	ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 0x1000, 256, false,
 			      &gr->unk4188b4);
-- 
2.5.1

On 09/03/2015 03:32 PM, Alexandre Courbot wrote:
> Two trivial PMU-related fixes for Tegra:
> - Add a dummy func member to GK20A, since nvkm_pmmu_pgob() unconditionally
>    dereferences it
> - Check whether a PMU actually exists before calling nvkm_pmmu_pgob().
During
>    early bringup we are likely to not have a PMU
Forgot to mention: it would be nice to have these merged in the 4.3 
cycle, since GK20A needs at least the first patch to probe successfully.

Thanks!

On 3 September 2015 at 16:32, Alexandre Courbot <acourbot at nvidia.com>
wrote:
> Some parts of the driver expect the PMU to have a func member, so set
> one for GK20A to avoid NULL pointer derefences.
This part should already be done in the current code.
>
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
> ---
>  drm/nouveau/nvkm/subdev/pmu/gk20a.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
> index 6689d0290a7e..f46e3310e21a 100644
> --- a/drm/nouveau/nvkm/subdev/pmu/gk20a.c
> +++ b/drm/nouveau/nvkm/subdev/pmu/gk20a.c
> @@ -202,6 +202,10 @@ gk20a_dvfs_data= {
>         .p_smooth = 1,
>  };
>
> +static const struct nvkm_pmu_func
> +gk20a_pmu_func = {
> +};
> +
>  static const struct nvkm_subdev_func
>  gk20a_pmu = {
>         .init = gk20a_pmu_init,
> @@ -221,6 +225,7 @@ gk20a_pmu_new(struct nvkm_device *device, int index,
struct nvkm_pmu **ppmu)
>         *ppmu = &pmu->base;
>
>         nvkm_subdev_ctor(&gk20a_pmu, device, index, 0,
&pmu->base.subdev);
> +       pmu->base.func = &gk20a_pmu_func;
>         pmu->data = &gk20a_dvfs_data;
>         nvkm_alarm_init(&pmu->alarm, gk20a_pmu_dvfs_work);
>         return 0;
> --
> 2.5.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

On 3 September 2015 at 16:32, Alexandre Courbot <acourbot at nvidia.com>
wrote:
> Some devices may not have a PMU. Avoid a NULL pointer dereference in
> such cases.
>
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
> ---
>  drm/nouveau/nvkm/engine/gr/gf100.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drm/nouveau/nvkm/engine/gr/gf100.c
b/drm/nouveau/nvkm/engine/gr/gf100.c
> index f1358a564e3e..f252fa2d7cf9 100644
> --- a/drm/nouveau/nvkm/engine/gr/gf100.c
> +++ b/drm/nouveau/nvkm/engine/gr/gf100.c
> @@ -1499,7 +1499,8 @@ gf100_gr_oneinit(struct nvkm_gr *base)
>         struct nvkm_device *device = gr->base.engine.subdev.device;
>         int ret, i, j;
>
> -       nvkm_pmu_pgob(device->pmu, false);
> +       if (device->pmu)
> +               nvkm_pmu_pgob(device->pmu, false);
I'd probably just change the condition in nvkm_pmu_pgob() to (pmu &&
pmu->func->pgob) ?
>
>         ret = nvkm_memory_new(device, NVKM_MEM_TARGET_INST, 0x1000, 256,
false,
>                               &gr->unk4188b4);
> --
> 2.5.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau