Thanks Alex. Some comments inline.
â??
Sincerely Yours,
Pixel
On 19/10/2017, 10:34 AM, "Alex Deucher" <alexdeucher at gmail.com> wrote:
>On Wed, Oct 18, 2017 at 9:44 PM, Pixel Ding <Pixel.Ding at amd.com> wrote:
>> From: pding <Pixel.Ding at amd.com>
>>
>> The post checking on scratch registers isn't reliable for virtual function.
>>
>> v2: only change in IGP reading bios.
>
>
>Subject has "drm/amdgpu: drm/amdgpu: " drop one of them.
>[Pixel] get it.
>
>
>>
>> Signed-off-by: pding <Pixel.Ding at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>> 3 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 0280ae5..caabc5b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1840,6 +1840,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
>> /* Common functions */
>> int amdgpu_gpu_reset(struct amdgpu_device *adev);
>> bool amdgpu_need_backup(struct amdgpu_device *adev);
>> +bool amdgpu_vpost_needed(struct amdgpu_device *adev);
>> void amdgpu_pci_config_reset(struct amdgpu_device *adev);
>> bool amdgpu_need_post(struct amdgpu_device *adev);
>
>amdgpu_need_post can be dropped now.
[Pixel] some other functions still invoke the amdgpu_need_post(). Only if we change all to use amdgpu_vpost_needed(). thatâ??s what the v1 patch did, however itâ??s confused. the only difference is that original patch uses amdgpu_need_post() function name instead of amdgpu_vpost_needed(), I thought itâ??s more generic.
>
>> void amdgpu_update_display_priority(struct amdgpu_device *adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> index c21adf6..25f43eb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> @@ -99,7 +99,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
>> resource_size_t size = 256 * 1024; /* ??? */
>>
>> if (!(adev->flags & AMD_IS_APU))
>> - if (amdgpu_need_post(adev))
>> + if (amdgpu_vpost_needed(adev))
>> return false;
>>
>> adev->bios = NULL;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index a601d87..098cd44 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -764,7 +764,7 @@ bool amdgpu_need_post(struct amdgpu_device *adev)
>>
>> }
>>
>> -static bool amdgpu_vpost_needed(struct amdgpu_device *adev)
>> +bool amdgpu_vpost_needed(struct amdgpu_device *adev)
>
>Please also make amdgpu_need_post() static now.
>
>Is there a way we can just merge amdgpu_need_post() into
>amdgpu_vpost_needed()? For bare metal it should be fine. Might need
>some logic adjustments for sr-iov amdgpu_device_resume(). That can be
>a follow on patch if you want.
[Pixel] the original patch replaces all amdgpu_need_post() with amdgpu_vpost_needed(), then rename the amdgpu_vpost_needed() to amdgpu_need_post(), and amdgpu_need_post() to static amdgpu_check_post(). After these transform it looks unclear. By now I think we can go back to that one.
>
>Alex
>
>
>> {
>> if (amdgpu_sriov_vf(adev))
>> return false;
>> --
>> 2.9.5
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
