On Mon, Nov 25, 2019 at 4:05 AM Liu, Monk <[email protected]> wrote: > commit a9a8a788e5e946a9835a1365256fc4ce9e96ba2c > > Author: Rex Zhu <[email protected]> > > Date: Wed Aug 22 18:54:45 2018 +0800 > > > > drm/amdgpu: Change kiq ring initialize sequence on gfx9 > > > > 1. initialize kiq before initialize gfx ring. > > 2. set kiq ring ready immediately when kiq initialize > > successfully. > > 3. split function gfx_v9_0_kiq_resume into two functions. > > gfx_v9_0_kiq_resume is for kiq initialize. > > gfx_v9_0_kcq_resume is for kcq initialize. > > > > Reviewed-by: Alex Deucher <[email protected]> > > Signed-off-by: Rex Zhu <[email protected]> > > Signed-off-by: Alex Deucher <[email protected]> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > index 21e66f8..3594704a 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > @@ -2684,7 +2684,6 @@ static int gfx_v9_0_kiq_kcq_enable(struct > amdgpu_device *adev) > > queue_mask |= (1ull << i); > > } > > > > - kiq_ring->ready = true; > > r = amdgpu_ring_alloc(kiq_ring, (7 * adev->gfx.num_compute_rings) > + 8); > > if (r) { > > DRM_ERROR("Failed to lock KIQ (%d).\n", r); > > @@ -3091,26 +3090,33 @@ static int gfx_v9_0_kcq_init_queue(struct > amdgpu_ring *ring) > > > > static int gfx_v9_0_kiq_resume(struct amdgpu_device *adev) > > { > > - struct amdgpu_ring *ring = NULL; > > - int r = 0, i; > > - > > - gfx_v9_0_cp_compute_enable(adev, true); > > + struct amdgpu_ring *ring; > > + int r; > > > > ring = &adev->gfx.kiq.ring; > > > > r = amdgpu_bo_reserve(ring->mqd_obj, false); > > if (unlikely(r != 0)) > > - goto done; > > + return r; > > > > r = amdgpu_bo_kmap(ring->mqd_obj, (void **)&ring->mqd_ptr); > > - if (!r) { > > - r = gfx_v9_0_kiq_init_queue(ring); > > - amdgpu_bo_kunmap(ring->mqd_obj); > > - ring->mqd_ptr = NULL; > > - } > > + if (unlikely(r != 0)) > > + return r; > > + > > + gfx_v9_0_kiq_init_queue(ring); > > + amdgpu_bo_kunmap(ring->mqd_obj); //not invoked before change > > + ring->mqd_ptr = NULL; //not invoked > before change > > amdgpu_bo_unreserve(ring->mqd_obj); //not invoked before change > > - if (r) > > - goto done; > > + ring->ready = true; > > + return 0; > > +} > > + > > +static int gfx_v9_0_kcq_resume(struct amdgpu_device *adev) > > +{ > > > > Hi guys > > > > I found in last year, Rex submitted a change which introduced additional > “amdgpu_bo_kunmap() and amdgpu_bo_unreserve()” > > During the kiq_resume() routine, > > > > I’m wondering why we need this change and I’m also suspecting it has > potential regression: > > See that the KIQ’s mqd object is unreserved, so it now available in LRU > which means TTM can evict it by will, > > > > But the MQD of KIQ is supposed to be pinned otherwise incorrect memory > access would introduced by a world switch > > Since MEC always consider KIQ’ MQD is pinned > > > > @Koenig, Christian <[email protected]> you know about that > change’s background ? > > >
I think the unreserve here is just to match the reserve at the top of this function for the kmapping. It should still be pinned because we call amdgpu_bo_create_kernel() to allocate it. Alex > _____________________________________ > > Monk Liu|GPU Virtualization Team |AMD > > [image: sig-cloud-gpu] > > > _______________________________________________ > 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
