Re: [PATCH 2/3] drm/amdgpu: allocate gart memory when it's required (v2)

["Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]

On 06/28/2018 12:52 AM, Felix Kuehling wrote:
> On 2018-06-26 09:42 PM, Zhang, Jerry (Junwei) wrote:
>>
>> BTW, kfd2kgd_calls kfd2kgd looks duplicated in amdkfd_gfx_v7/8/9.c
>> we may initialize it in a common place(at least for common members).
>> If it has other purpose, please ignore that.
>
> Some of the function pointers in kfd2kgd_calls are HW-specific (static
> functions implemented in amdgpu_amdkfd_gfx_v[789].c). Therefore they are
> not really duplicates. They may look the same in the source code, but
> they'll end up pointing to different functions.

Thanks for your explanation.
I see.

Just a suggestion. In this case, we may add the version for each func
to clarify the different HW support, e.g. kgd_program_sh_mem_settings_v9.
Then the common func would be identified clearly, like alloc_gtt_mem() for all HWs.
So do HW-specific func.

(seems to talk too much regardless of this thread, thanks for your discussion)

Jerry

>
> Regards,
>    Felix
>
>>
>> Jerry
>>
>>>
>>> Regards,
>>>     Felix
>>>
>>>>        ret = amdgpu_bo_kmap(bo, kptr);
>>>>        if (ret) {
>>>>            pr_err("Failed to map bo to kernel. ret %d\n", ret);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>>> index cb88d7e..3079ea8 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
>>>> @@ -96,11 +96,16 @@ static void amdgpu_benchmark_move(struct
>>>> amdgpu_device *adev, unsigned size,
>>>>        if (unlikely(r != 0))
>>>>            goto out_cleanup;
>>>>        r = amdgpu_bo_pin(sobj, sdomain);
>>>> -    saddr = amdgpu_bo_gpu_offset(sobj);
>>>> +    if (r) {
>>>> +        amdgpu_bo_unreserve(sobj);
>>>> +        goto out_cleanup;
>>>> +    }
>>>> +    r = amdgpu_ttm_alloc_gart(&sobj->tbo);
>>>>        amdgpu_bo_unreserve(sobj);
>>>>        if (r) {
>>>>            goto out_cleanup;
>>>>        }
>>>> +    saddr = amdgpu_bo_gpu_offset(sobj);
>>>>        bp.domain = ddomain;
>>>>        r = amdgpu_bo_create(adev, &bp, &dobj);
>>>>        if (r) {
>>>> @@ -110,11 +115,16 @@ static void amdgpu_benchmark_move(struct
>>>> amdgpu_device *adev, unsigned size,
>>>>        if (unlikely(r != 0))
>>>>            goto out_cleanup;
>>>>        r = amdgpu_bo_pin(dobj, ddomain);
>>>> -    daddr = amdgpu_bo_gpu_offset(dobj);
>>>> +    if (r) {
>>>> +        amdgpu_bo_unreserve(sobj);
>>>> +        goto out_cleanup;
>>>> +    }
>>>> +    r = amdgpu_ttm_alloc_gart(&dobj->tbo);
>>>>        amdgpu_bo_unreserve(dobj);
>>>>        if (r) {
>>>>            goto out_cleanup;
>>>>        }
>>>> +    daddr = amdgpu_bo_gpu_offset(dobj);
>>>>
>>>>        if (adev->mman.buffer_funcs) {
>>>>            time = amdgpu_benchmark_do_move(adev, size, saddr, daddr, n);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>> index 036b6f7..7d6a36b 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>>> @@ -194,6 +194,12 @@ int amdgpu_display_crtc_page_flip_target(struct
>>>> drm_crtc *crtc,
>>>>            goto unreserve;
>>>>        }
>>>>
>>>> +    r = amdgpu_ttm_alloc_gart(&new_abo->tbo);
>>>> +    if (unlikely(r != 0)) {
>>>> +        DRM_ERROR("%p bind failed\n", new_abo);
>>>> +        goto unpin;
>>>> +    }
>>>> +
>>>>        r = reservation_object_get_fences_rcu(new_abo->tbo.resv,
>>>> &work->excl,
>>>>                              &work->shared_count,
>>>>                              &work->shared);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>> index 462b7a1..cd68a2e 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
>>>> @@ -173,6 +173,14 @@ static int amdgpufb_create_pinned_object(struct
>>>> amdgpu_fbdev *rfbdev,
>>>>            amdgpu_bo_unreserve(abo);
>>>>            goto out_unref;
>>>>        }
>>>> +
>>>> +    ret = amdgpu_ttm_alloc_gart(&abo->tbo);
>>>> +    if (ret) {
>>>> +        amdgpu_bo_unreserve(abo);
>>>> +        dev_err(adev->dev, "%p bind failed\n", abo);
>>>> +        goto out_unref;
>>>> +    }
>>>> +
>>>>        ret = amdgpu_bo_kmap(abo, NULL);
>>>>        amdgpu_bo_unreserve(abo);
>>>>        if (ret) {
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> index 79cdbf1..7f7c221 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> @@ -257,6 +257,13 @@ int amdgpu_bo_create_reserved(struct
>>>> amdgpu_device *adev,
>>>>            dev_err(adev->dev, "(%d) kernel bo pin failed\n", r);
>>>>            goto error_unreserve;
>>>>        }
>>>> +
>>>> +    r = amdgpu_ttm_alloc_gart(&(*bo_ptr)->tbo);
>>>> +    if (r) {
>>>> +        dev_err(adev->dev, "%p bind failed\n", *bo_ptr);
>>>> +        goto error_unpin;
>>>> +    }
>>>> +
>>>>        if (gpu_addr)
>>>>            *gpu_addr = amdgpu_bo_gpu_offset(*bo_ptr);
>>>>
>>>> @@ -270,6 +277,8 @@ int amdgpu_bo_create_reserved(struct
>>>> amdgpu_device *adev,
>>>>
>>>>        return 0;
>>>>
>>>> +error_unpin:
>>>> +    amdgpu_bo_unpin(*bo_ptr);
>>>>    error_unreserve:
>>>>        amdgpu_bo_unreserve(*bo_ptr);
>>>>
>>>> @@ -903,12 +912,6 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo
>>>> *bo, u32 domain,
>>>>            goto error;
>>>>        }
>>>>
>>>> -    r = amdgpu_ttm_alloc_gart(&bo->tbo);
>>>> -    if (unlikely(r)) {
>>>> -        dev_err(adev->dev, "%p bind failed\n", bo);
>>>> -        goto error;
>>>> -    }
>>>> -
>>>>        bo->pin_count = 1;
>>>>
>>>>        domain = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
>>>> index 622affc..d6eeea1 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
>>>> @@ -102,6 +102,11 @@ static void amdgpu_do_test_moves(struct
>>>> amdgpu_device *adev)
>>>>                DRM_ERROR("Failed to pin GTT object %d\n", i);
>>>>                goto out_lclean_unres;
>>>>            }
>>>> +        r = amdgpu_ttm_alloc_gart(&gtt_obj[i]->tbo);
>>>> +        if (r) {
>>>> +            DRM_ERROR("%p bind failed\n", gtt_obj[i]);
>>>> +            goto out_lclean_unpin;
>>>> +        }
>>>>            gart_addr = amdgpu_bo_gpu_offset(gtt_obj[i]);
>>>>
>>>>            r = amdgpu_bo_kmap(gtt_obj[i], &gtt_map);
>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> index 31652c1e..d433428 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -3110,13 +3110,22 @@ static int dm_plane_helper_prepare_fb(struct
>>>> drm_plane *plane,
>>>>            domain = AMDGPU_GEM_DOMAIN_VRAM;
>>>>
>>>>        r = amdgpu_bo_pin(rbo, domain);
>>>> -    amdgpu_bo_unreserve(rbo);
>>>> -
>>>>        if (unlikely(r != 0)) {
>>>>            if (r != -ERESTARTSYS)
>>>>                DRM_ERROR("Failed to pin framebuffer with error %d\n",
>>>> r);
>>>> +        amdgpu_bo_unreserve(rbo);
>>>> +        return r;
>>>> +    }
>>>> +
>>>> +    r = amdgpu_ttm_alloc_gart(&rbo->tbo);
>>>> +    if (unlikely(r != 0)) {
>>>> +        amdgpu_bo_unpin(rbo);
>>>> +        amdgpu_bo_unreserve(rbo);
>>>> +        DRM_ERROR("%p bind failed\n", rbo);
>>>>            return r;
>>>>        }
>>>> +    amdgpu_bo_unreserve(rbo);
>>>> +
>>>>        afb->address = amdgpu_bo_gpu_offset(rbo);
>>>>
>>>>        amdgpu_bo_ref(rbo);
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx