drm/amdgpu:make ctx_add_fence interruptible(v2)

PATCH In review Monk Liu 3 participants 8 messages Sep 19, 2017 – Sep 20, 2017 4 messages not in the archive

Subsystems (2)

DRM drivers Radeon and amdgpu DRM drivers

lore

Share & tools

lore

Monk Liu

author

2017-09-19 06:41 UTC #0 lore raw

PATCH 1/3 drm/amdgpu:make ctx_add_fence interruptible(v2)

otherwise a gpu hang will make application couldn't be killed under timedout=0 mode

v2:
Fix memoryleak job/job->s_fence issue
unlock mn
remove the ERROR msg after waiting being interrupted

Change-Id I6051b5b3ae1188983f49325a2438c84a6c12374a

Signed-off-by Monk Liu <[email protected]>

---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 17 +++++++++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 12 +++++++-----
 3 files changed, 24 insertions(+), 9 deletions(-)

drivers/gpu/drm/amd/amdgpu/amdgpu.h +2 -2

index cc9a232..6ff2959 100644
@@ -736,8 +736,8 @@ struct amdgpu_ctx_mgr {
 struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id);
 int amdgpu_ctx_put(struct amdgpu_ctx *ctx);
 
-uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
-			      struct dma_fence *fence);
+int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
+			      struct dma_fence *fence, uint64_t *seq);
 struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
 				   struct amdgpu_ring *ring, uint64_t seq);

drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +15 -2

index b59749d..9bd4834 100644
@@ -1043,6 +1043,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity;
 	struct amdgpu_job *job;
 	unsigned i;
+	uint64_t seq;
+
 	int r;
 
 	amdgpu_mn_lock(p->mn);

@@ -1071,8 +1073,19 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	job->owner = p->filp;
 	job->fence_ctx = entity->fence_context;
 	p->fence = dma_fence_get(&job->base.s_fence->finished);
-	cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence);
-	job->uf_sequence = cs->out.handle;
+	r = amdgpu_ctx_add_fence(p->ctx, ring, p->fence, &seq);
+	if (r) {
+		/* release job include the sched fence as well */
+		dma_fence_put(&job->base.s_fence->finished);
+		dma_fence_put(&job->base.s_fence->scheduled);
+		amdgpu_job_free(job);
+		amdgpu_mn_unlock(p->mn);
+		dma_fence_put(p->fence);
+		return r;
+	}
+
+	cs->out.handle = seq;
+	job->uf_sequence = seq;
 	amdgpu_job_free_resources(job);
 
 	trace_amdgpu_cs_ioctl(job);

drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +7 -5

index a11e443..551f114 100644
@@ -246,8 +246,8 @@ int amdgpu_ctx_put(struct amdgpu_ctx *ctx)
 	return 0;
 }
 
-uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
-			      struct dma_fence *fence)
+int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
+			      struct dma_fence *fence, uint64_t* handler)
 {
 	struct amdgpu_ctx_ring *cring = & ctx->rings[ring->idx];
 	uint64_t seq = cring->sequence;

@@ -258,9 +258,9 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
 	other = cring->fences[idx];
 	if (other) {
 		signed long r;
-		r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT);
+		r = dma_fence_wait_timeout(other, true, MAX_SCHEDULE_TIMEOUT);
 		if (r < 0)
-			DRM_ERROR("Error (%ld) waiting for fence!\n", r);
+			return -ERESTARTSYS;
 	}
 
 	dma_fence_get(fence);

@@ -271,8 +271,10 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
 	spin_unlock(&ctx->ring_lock);
 
 	dma_fence_put(other);
+	if (handler)
+		*handler = seq;
 
-	return seq;
+	return 0;
 }
 
 struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,

Signature · 6 lines

2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 1505803274-8237-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org

Monk Liu

author

2017-09-19 06:41 UTC #2 lore raw

PATCH 2/3 drm/amdgpu/sriov:fix memory leak after gpu reset

GPU reset will require all hw doing hw_init thus ucode_init_bo will be invoked again, which lead to memory leak

skip the fw_buf allocation during sriov gpu reset to avoid memory leak.

Change-Id I31131eda1bd45ea2f5bdc50c5da5fc5a9fe9027d

Signed-off-by Monk Liu <[email protected]>

---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 64 +++++++++++++++----------------
 2 files changed, 35 insertions(+), 32 deletions(-)

drivers/gpu/drm/amd/amdgpu/amdgpu.h +3 -0

index 6ff2959..3d0c633 100644
@@ -1185,6 +1185,9 @@ struct amdgpu_firmware {
 
 	/* gpu info firmware data pointer */
 	const struct firmware *gpu_info_fw;
+
+	void *fw_buf_ptr;
+	uint64_t fw_buf_mc;
 };
 
 /*

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +32 ~~-32~~

91 lines not on this page · raw message

Signature · 6 lines

2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 1505803274-8237-2-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org

Christian König

Acked-by

2017-09-19 11:43 UTC #4 lore raw

Re: [PATCH 2/3] drm/amdgpu/sriov:fix memory leak after gpu reset

Am 19.09.2017 um 08:41 schrieb Monk Liu:

Quoted 9 lines

GPU reset will require all hw doing hw_init thus ucode_init_bo will be invoked again, which lead to memory leak
skip the fw_buf allocation during sriov gpu reset to avoid memory leak.
Change-Id I31131eda1bd45ea2f5bdc50c5da5fc5a9fe9027d
Signed-off-by Monk Liu <[email protected]>

Acked-by Christian König <[email protected]> for now.

But we should really clean this up and do the allocation during SW init.

Christian.

Quoted 107 lines

---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  3 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 64 +++++++++++++++----------------
  2 files changed, 35 insertions(+), 32 deletions(-)

drivers/gpu/drm/amd/amdgpu/amdgpu.h +3 -0

index 6ff2959..3d0c633 100644
@@ -1185,6 +1185,9 @@ struct amdgpu_firmware {
 
  	/* gpu info firmware data pointer */
  	const struct firmware *gpu_info_fw;
+
+	void *fw_buf_ptr;
+	uint64_t fw_buf_mc;
  };
 
  /*

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +30 ~~-30~~

index f306374..6564902 100644
@@ -360,8 +360,6 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
  int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
  {
  	struct amdgpu_bo **bo = &adev->firmware.fw_buf;
-	uint64_t fw_mc_addr;
-	void *fw_buf_ptr = NULL;
  	uint64_t fw_offset = 0;
  	int i, err;
  	struct amdgpu_firmware_info *ucode = NULL;

@@ -372,37 +370,39 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
  		return 0;
  	}
 
-	err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true,
-				amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
-				AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
-				NULL, NULL, 0, bo);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err);
-		goto failed;
-	}
+	if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) {
+		err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true,
+					amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
+					AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
+					NULL, NULL, 0, bo);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err);
+			goto failed;
+		}
 
-	err = amdgpu_bo_reserve(*bo, false);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err);
-		goto failed_reserve;
-	}
+		err = amdgpu_bo_reserve(*bo, false);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err);
+			goto failed_reserve;
+		}
 
-	err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
-				&fw_mc_addr);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err);
-		goto failed_pin;
-	}
+		err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
+					&adev->firmware.fw_buf_mc);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err);
+			goto failed_pin;
+		}
 
-	err = amdgpu_bo_kmap(*bo, &fw_buf_ptr);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err);
-		goto failed_kmap;
-	}
+		err = amdgpu_bo_kmap(*bo, &adev->firmware.fw_buf_ptr);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err);
+			goto failed_kmap;
+		}
 
-	amdgpu_bo_unreserve(*bo);
+		amdgpu_bo_unreserve(*bo);
+	}
 
-	memset(fw_buf_ptr, 0, adev->firmware.fw_size);
+	memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
 
  	/*
  	 * if SMU loaded firmware, it needn't add SMC, UVD, and VCE

@@ -421,14 +421,14 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
  		ucode = &adev->firmware.ucode[i];
  		if (ucode->fw) {
  			header = (const struct common_firmware_header *)ucode->fw->data;
-			amdgpu_ucode_init_single_fw(adev, ucode, fw_mc_addr + fw_offset,
-						    (void *)((uint8_t *)fw_buf_ptr + fw_offset));
+			amdgpu_ucode_init_single_fw(adev, ucode, adev->firmware.fw_buf_mc + fw_offset,
+						    adev->firmware.fw_buf_ptr + fw_offset);
  			if (i == AMDGPU_UCODE_ID_CP_MEC1 &&

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c int amdgpu_ucode_init_bo(struct amdgpu_device *adev) · line 434

Quoted 10 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +2 -2

  			    adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
  				const struct gfx_firmware_header_v1_0 *cp_hdr;
  				cp_hdr = (const struct gfx_firmware_header_v1_0 *)ucode->fw->data;
-				amdgpu_ucode_patch_jt(ucode, fw_mc_addr + fw_offset,
-						    fw_buf_ptr + fw_offset);
+				amdgpu_ucode_patch_jt(ucode,  adev->firmware.fw_buf_mc + fw_offset,
+						    adev->firmware.fw_buf_ptr + fw_offset);
  				fw_offset += ALIGN(le32_to_cpu(cp_hdr->jt_size) << 2, PAGE_SIZE);
  			}
  			fw_offset += ALIGN(ucode->ucode_size, PAGE_SIZE);

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 1c39869f-5d12-437c-5b08-dba488b9c937-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Liu, Monk

author Acked-by Christian König

2017-09-20 06:17 UTC #6 lore raw

RE: [PATCH 2/3] drm/amdgpu/sriov:fix memory leak after gpu reset

No ,we cannot do this in SW init, because PSP need all other component finish their HW init and get the fw_size, before it can call ucode_init, so if we put this in SW init, the fw_size is still 0....

BR Monk

-----Original Message-----
From: Christian König [mailto:[email protected]]
Sent: 2017年9月19日 19:44
To: Liu, Monk <[email protected]>; [email protected]
Subject: Re: [PATCH 2/3] drm/amdgpu/sriov:fix memory leak after gpu reset

Am 19.09.2017 um 08:41 schrieb Monk Liu:

Quoted 8 lines

GPU reset will require all hw doing hw_init thus ucode_init_bo will be invoked again, which lead to memory leak
skip the fw_buf allocation during sriov gpu reset to avoid memory leak.
Change-Id I31131eda1bd45ea2f5bdc50c5da5fc5a9fe9027d
Signed-off-by Monk Liu <[email protected]>

Acked-by Christian König <[email protected]> for now.

But we should really clean this up and do the allocation during SW init.

Christian.

Quoted 109 lines

---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  3 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 64 +++++++++++++++----------------
  2 files changed, 35 insertions(+), 32 deletions(-)

drivers/gpu/drm/amd/amdgpu/amdgpu.h +3 -0

index 6ff2959..3d0c633 100644
@@ -1185,6 +1185,9 @@ struct amdgpu_firmware {
 
  	/* gpu info firmware data pointer */
  	const struct firmware *gpu_info_fw;
+
+	void *fw_buf_ptr;
+	uint64_t fw_buf_mc;
  };
 
  /*

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +0 -2

index f306374..6564902 100644
@@ -360,8 +360,6 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
  int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
  {
  	struct amdgpu_bo **bo = &adev->firmware.fw_buf;
-	uint64_t fw_mc_addr;
-	void *fw_buf_ptr = NULL;
  	uint64_t fw_offset = 0;
  	int i, err;
  	struct amdgpu_firmware_info *ucode = NULL; @@ -372,37 +370,39 @@

int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
return 0;
}

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +30 ~~-29~~

-	err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true,
-				amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
-				AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
-				NULL, NULL, 0, bo);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err);
-		goto failed;
-	}
+	if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) {
+		err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true,
+					amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
+					AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
+					NULL, NULL, 0, bo);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err);
+			goto failed;
+		}
 
-	err = amdgpu_bo_reserve(*bo, false);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err);
-		goto failed_reserve;
-	}
+		err = amdgpu_bo_reserve(*bo, false);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err);
+			goto failed_reserve;
+		}
 
-	err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
-				&fw_mc_addr);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err);
-		goto failed_pin;
-	}
+		err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
+					&adev->firmware.fw_buf_mc);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err);
+			goto failed_pin;
+		}
 
-	err = amdgpu_bo_kmap(*bo, &fw_buf_ptr);
-	if (err) {
-		dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err);
-		goto failed_kmap;
-	}
+		err = amdgpu_bo_kmap(*bo, &adev->firmware.fw_buf_ptr);
+		if (err) {
+			dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err);
+			goto failed_kmap;
+		}
 
-	amdgpu_bo_unreserve(*bo);
+		amdgpu_bo_unreserve(*bo);
+	}
 
-	memset(fw_buf_ptr, 0, adev->firmware.fw_size);
+	memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
 
  	/*
  	 * if SMU loaded firmware, it needn't add SMC, UVD, and VCE @@ 
-421,14 +421,14 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
  		ucode = &adev->firmware.ucode[i];
  		if (ucode->fw) {
  			header = (const struct common_firmware_header *)ucode->fw->data;
-			amdgpu_ucode_init_single_fw(adev, ucode, fw_mc_addr + fw_offset,
-						    (void *)((uint8_t *)fw_buf_ptr + fw_offset));
+			amdgpu_ucode_init_single_fw(adev, ucode, adev->firmware.fw_buf_mc + fw_offset,
+						    adev->firmware.fw_buf_ptr + fw_offset);
  			if (i == AMDGPU_UCODE_ID_CP_MEC1 &&

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c int amdgpu_ucode_init_bo(struct amdgpu_device *adev)

Quoted 10 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c +2 -2

  			    adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) {
  				const struct gfx_firmware_header_v1_0 *cp_hdr;
  				cp_hdr = (const struct gfx_firmware_header_v1_0 *)ucode->fw->data;
-				amdgpu_ucode_patch_jt(ucode, fw_mc_addr + fw_offset,
-						    fw_buf_ptr + fw_offset);
+				amdgpu_ucode_patch_jt(ucode,  adev->firmware.fw_buf_mc + fw_offset,
+						    adev->firmware.fw_buf_ptr + fw_offset);
  				fw_offset += ALIGN(le32_to_cpu(cp_hdr->jt_size) << 2, PAGE_SIZE);
  			}
  			fw_offset += ALIGN(ucode->ucode_size, PAGE_SIZE);

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Monk Liu

author

2017-09-19 06:41 UTC #7 lore raw

PATCH 3/3 drm/amdgpu:fix typo

In reply to a message not in the archive

previously a patch has typo error, correct it

Change-Id I91bcefd7148b5db1c7d957c868e13a46ca40ef74

Signed-off-by Monk Liu <[email protected]>

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +1 -1

index 4eee2ef..35cc5ee 100644
@@ -256,7 +256,7 @@ static int psp_hw_start(struct psp_context *psp)
 	struct amdgpu_device *adev = psp->adev;
 	int ret;
 
-	if (amdgpu_sriov_vf(adev) && !adev->in_sriov_reset) {
+	if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) {
 		ret = psp_bootloader_load_sysdrv(psp);
 		if (ret)
 			return ret;

Signature · 6 lines

2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 1505803274-8237-3-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org

Christian König

Acked-by

2017-09-19 11:44 UTC #9 lore raw

Re: [PATCH 3/3] drm/amdgpu:fix typo

Am 19.09.2017 um 08:41 schrieb Monk Liu:

Quoted 4 lines

previously a patch has typo error, correct it
Change-Id I91bcefd7148b5db1c7d957c868e13a46ca40ef74
Signed-off-by Monk Liu <[email protected]>

Acked-by Christian König <[email protected]>

Quoted 9 lines

---
  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +0 -0

index 4eee2ef..35cc5ee 100644

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c static int psp_hw_start(struct psp_context *psp) · line 262

Quoted 8 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +1 -1

@@ -256,7 +256,7 @@ static int psp_hw_start(struct psp_context *psp)
  	struct amdgpu_device *adev = psp->adev;
  	int ret;
 
-	if (amdgpu_sriov_vf(adev) && !adev->in_sriov_reset) {
+	if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) {
  		ret = psp_bootloader_load_sysdrv(psp);
  		if (ret)
  			return ret;

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Alex Deucher

2017-09-19 14:25 UTC #10 lore raw

Re: [PATCH 3/3] drm/amdgpu:fix typo

In reply to a message not in the archive

Quoted 1 line · Monk Liu

previously a patch has typo error, correct it

Please squash this into the previous broken patch before committing it.

Alex

Quoted 23 lines

Change-Id I91bcefd7148b5db1c7d957c868e13a46ca40ef74

Signed-off-by Monk Liu <[email protected]>

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +1 -2

index 4eee2ef..35cc5ee 100644
@@ -256,7 +256,7 @@ static int psp_hw_start(struct psp_context *psp)
        struct amdgpu_device *adev = psp->adev;
        int ret;
 
-       if (amdgpu_sriov_vf(adev) && !adev->in_sriov_reset) {
+       if (!amdgpu_sriov_vf(adev) || !adev->in_sriov_reset) {
                ret = psp_bootloader_load_sysdrv(psp);
                if (ret)
                        return ret;
--

2.7.4

Quoted 4 lines

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Christian König

2017-09-19 11:42 UTC #11 lore raw

In reply to a message not in the archive

Am 19.09.2017 um 08:41 schrieb Monk Liu:

Quoted 82 lines

otherwise a gpu hang will make application couldn't be killed under timedout=0 mode

v2:
Fix memoryleak job/job->s_fence issue
unlock mn
remove the ERROR msg after waiting being interrupted

Change-Id I6051b5b3ae1188983f49325a2438c84a6c12374a

Signed-off-by Monk Liu <[email protected]>

---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 17 +++++++++++++++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 12 +++++++-----
  3 files changed, 24 insertions(+), 9 deletions(-)

drivers/gpu/drm/amd/amdgpu/amdgpu.h +2 -2

index cc9a232..6ff2959 100644
@@ -736,8 +736,8 @@ struct amdgpu_ctx_mgr {
  struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id);
  int amdgpu_ctx_put(struct amdgpu_ctx *ctx);
 
-uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
-			      struct dma_fence *fence);
+int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
+			      struct dma_fence *fence, uint64_t *seq);
  struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
  				   struct amdgpu_ring *ring, uint64_t seq);

drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +15 -2

index b59749d..9bd4834 100644
@@ -1043,6 +1043,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
  	struct amd_sched_entity *entity = &p->ctx->rings[ring->idx].entity;
  	struct amdgpu_job *job;
  	unsigned i;
+	uint64_t seq;
+
  	int r;
 
  	amdgpu_mn_lock(p->mn);

@@ -1071,8 +1073,19 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
  	job->owner = p->filp;
  	job->fence_ctx = entity->fence_context;
  	p->fence = dma_fence_get(&job->base.s_fence->finished);
-	cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence);
-	job->uf_sequence = cs->out.handle;
+	r = amdgpu_ctx_add_fence(p->ctx, ring, p->fence, &seq);
+	if (r) {
+		/* release job include the sched fence as well */
+		dma_fence_put(&job->base.s_fence->finished);
+		dma_fence_put(&job->base.s_fence->scheduled);
+		amdgpu_job_free(job);
+		amdgpu_mn_unlock(p->mn);
+		dma_fence_put(p->fence);
+		return r;
+	}
+
+	cs->out.handle = seq;
+	job->uf_sequence = seq;
  	amdgpu_job_free_resources(job);
 
  	trace_amdgpu_cs_ioctl(job);

drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +2 -2

index a11e443..551f114 100644
@@ -246,8 +246,8 @@ int amdgpu_ctx_put(struct amdgpu_ctx *ctx)
  	return 0;
  }
 
-uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
-			      struct dma_fence *fence)
+int amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
+			      struct dma_fence *fence, uint64_t* handler)
  {
  	struct amdgpu_ctx_ring *cring = & ctx->rings[ring->idx];
  	uint64_t seq = cring->sequence;

drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, · line 263

Quoted 8 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +2 -2

@@ -258,9 +258,9 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
  	other = cring->fences[idx];
  	if (other) {
  		signed long r;
-		r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT);
+		r = dma_fence_wait_timeout(other, true, MAX_SCHEDULE_TIMEOUT);
  		if (r < 0)
-			DRM_ERROR("Error (%ld) waiting for fence!\n", r);
+			return -ERESTARTSYS;

Return the original error code here, e.g. "r".

With that fixed the patch is Reviewed-by: Christian König <[email protected]>

Regards,
Christian.

Quoted 5 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +0 -0

  	}
 
  	dma_fence_get(fence);

@@ -271,8 +271,10 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
  	spin_unlock(&ctx->ring_lock);

drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring, · line 280

Quoted 10 lines

drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +3 -1

 
  	dma_fence_put(other);
+	if (handler)
+		*handler = seq;
 
-	return seq;
+	return 0;
  }
 
  struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx