Am 18.07.2018 um 16:22 schrieb James Zhu: > > > On 2018-07-18 10:11 AM, Christian König wrote: >> Am 18.07.2018 um 15:40 schrieb James Zhu: >>> >>> On 2018-07-18 08:55 AM, Christian König wrote: >>>> Going to completely rework the context to ring mapping with Nayan's >>>> GSoC >>>> work, but for now just stopping to expose the second UVD instance >>>> should >>>> do it. >>>> >>>> Signed-off-by: Christian König <christian.koenig at amd.com> >>>> --- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c      | 13 +++++-------- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c | 9 ++------- >>>>  2 files changed, 7 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >>>> index e451a3f25beb..75da3c41f3b3 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c >>>> @@ -286,7 +286,7 @@ static int amdgpu_info_ioctl(struct drm_device >>>> *dev, void *data, struct drm_file >>>>      struct drm_crtc *crtc; >>>>      uint32_t ui32 = 0; >>>>      uint64_t ui64 = 0; >>>> -   int i, j, found; >>>> +   int i, found; >>>>      int ui32_size = sizeof(ui32); >>>>       if (!info->return_size || !info->return_pointer) >>>> @@ -348,8 +348,7 @@ static int amdgpu_info_ioctl(struct drm_device >>>> *dev, void *data, struct drm_file >>>>              break; >>>>          case AMDGPU_HW_IP_UVD: >>>>              type = AMD_IP_BLOCK_TYPE_UVD; >>>> -           for (i = 0; i < adev->uvd.num_uvd_inst; i++) >>>> -               ring_mask |= adev->uvd.inst[i].ring.ready << i; >>>> +           ring_mask |= adev->uvd.inst[0].ring.ready; >>>>              ib_start_alignment = 64; >>>>              ib_size_alignment = 64; >>>>              break; >>>> @@ -362,11 +361,9 @@ static int amdgpu_info_ioctl(struct drm_device >>>> *dev, void *data, struct drm_file >>>>              break; >>>>          case AMDGPU_HW_IP_UVD_ENC: >>>>              type = AMD_IP_BLOCK_TYPE_UVD; >>>> -           for (i = 0; i < adev->uvd.num_uvd_inst; i++) >>>> -               for (j = 0; j < adev->uvd.num_enc_rings; j++) >>>> -                   ring_mask |= >>>> -                   adev->uvd.inst[i].ring_enc[j].ready << >>>> -                   (j + i * adev->uvd.num_enc_rings); >>>> +           for (i = 0; i < adev->uvd.num_enc_rings; i++) >>>> +               ring_mask |= >>>> +                   adev->uvd.inst[0].ring_enc[i].ready << i; >>>>              ib_start_alignment = 64; >>>>              ib_size_alignment = 64; >>>>              break; >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c >>>> index ea9850c9224d..d8357290ad09 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c >>>> @@ -66,8 +66,6 @@ static int amdgpu_identity_map(struct >>>> amdgpu_device *adev, >>>>                     u32 ring, >>>>                     struct amdgpu_ring **out_ring) >>>>  { >>>> -   u32 instance; >>>> - >>>>      switch (mapper->hw_ip) { >>>>      case AMDGPU_HW_IP_GFX: >>>>          *out_ring = &adev->gfx.gfx_ring[ring]; >>>> @@ -79,16 +77,13 @@ static int amdgpu_identity_map(struct >>>> amdgpu_device *adev, >>>>          *out_ring = &adev->sdma.instance[ring].ring; >>>>          break; >>>>      case AMDGPU_HW_IP_UVD: >>>> -       instance = ring; >>>> -       *out_ring = &adev->uvd.inst[instance].ring; >>>> +       *out_ring = &adev->uvd.inst[0].ring; >>>>          break; >>>>      case AMDGPU_HW_IP_VCE: >>>>          *out_ring = &adev->vce.ring[ring]; >>>>          break; >>>>      case AMDGPU_HW_IP_UVD_ENC: >>>> -       instance = ring / adev->uvd.num_enc_rings; >>>> -       *out_ring = >>>> - &adev->uvd.inst[instance].ring_enc[ring%adev->uvd.num_enc_rings]; >>>> +       *out_ring = &adev->uvd.inst[0].ring_enc[ring]; >>> >>> Hi Christian, >>> >>> If here uses amdgpu_identity_map, I think only first instance's ring >>> is in use. >>> >>> In current Vega20 situation, should we change to use amdgpu_lru_map >>> or something else to schedule which instance/ring for use ? >> >> Yes and no. I'm going to completely remove the queue manager and >> replace it with Nayan's work on the scheduler. >> >> So no more lru map, but instead a function which picks the best ring >> based on actual load during context creation. >> >> Regards, >> Christian. > Got it. So do we totally remove instance and ring concept out of user > space scope? Yes, that's at least the idea. > This design is good for use, but it lost flexibility on fine resource > management. It may be requested by cgroup's use case in the future. I actually do this to allow simpler cgroup implementation on top of this. In other words cgroup can still be used to limit certain applications to certain hardware engines and/or sw priorities, etc... Regards, Christian. > > Regards! > James >> >>> >>> James >>> >>>>          break; >>>>      case AMDGPU_HW_IP_VCN_DEC: >>>>          *out_ring = &adev->vcn.ring_dec; >>> >> >