[PATCH] drm/amdgpu: drm/amdgpu: always consider virualised device for checking post (v2)

[Ding, Pixel]

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