Hi Alex, This change only affect virtualization and doesn’t change any code path of baremetal, and I have the virtualization test passed.
As we discussed in the v2 patch, merge 2 bios post checking function. Can I have your review for the v1? Or do you like to keep the amdgpu_vpost_needed() function name? — Sincerely Yours, Pixel On 18/10/2017, 10:08 PM, "Deucher, Alexander" <[email protected]> wrote: >> -----Original Message----- >> From: amd-gfx [mailto:[email protected]] On Behalf >> Of Christian König >> Sent: Wednesday, October 18, 2017 3:23 AM >> To: Ding, Pixel; Liu, Monk; [email protected] >> Cc: Sun, Gary; Li, Bingley >> Subject: Re: [PATCH 1/3] drm/amdgpu: always consider virualised device for >> checking post >> >> I've already did, but honestly have no idea what you do here. >> >> ASIC post is not something I've every worked on. >> >> It looks odd that you add/remove some static keyword here, but I can't >> judge the technical correctness. >> >> Monk, Alex what do you think of this? > >I think we should fix the issue in amdgpu_bios.c or sort out/merge >amdgpu_need_post and amdgpu_vpost_needed. This change is really unclear. > >Alex > >> >> Sorry, >> Christian. >> >> Am 18.10.2017 um 09:19 schrieb Ding, Pixel: >> > Hi Christian, >> > >> > Would you please take a look at this change? It’s to unify the VBIOS post >> checking. >> > — >> > Sincerely Yours, >> > Pixel >> > >> > >> > >> > >> > >> > >> > >> > >> > On 18/10/2017, 10:25 AM, "Ding, Pixel" <[email protected]> wrote: >> > >> >> Hi all, >> >> >> >> Could someone review this patch? >> >> >> >> — >> >> Sincerely Yours, >> >> Pixel >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> On 17/10/2017, 4:06 PM, "amd-gfx on behalf of Ding, Pixel" <amd-gfx- >> [email protected] on behalf of [email protected]> wrote: >> >> >> >>> you can see the difference of function amdgpu_need_post. Generally >> speaking, there were 2 functions to check VBIOS posting, one considers VF >> and passthru while the other doesn’t. In fact we should always consider VF >> and PT for checking, right? For example, this checking here believe VF needs >> a posting because SCRATCH registers are not the expected value. Is it clear? >> >>> — >> >>> Sincerely Yours, >> >>> Pixel >> >>> >> >>> >> >>> >> >>> >> >>> >> >>> >> >>> >> >>> >> >>> On 17/10/2017, 5:00 PM, "Liu, Monk" <[email protected]> wrote: >> >>> >> >>> >From the patch itself I still couldn't tell the difference >> >>>> -----Original Message----- >> >>>> From: Ding, Pixel >> >>>> Sent: 2017年10月17日 15:54 >> >>>> To: Liu, Monk <[email protected]>; [email protected]; >> Koenig, Christian <[email protected]> >> >>>> Cc: Li, Bingley <[email protected]>; Sun, Gary >> <[email protected]> >> >>>> Subject: Re: [PATCH 1/3] drm/amdgpu: always consider virualised >> device for checking post >> >>>> >> >>>> It fixes a issue hidden in: >> >>>> >> >>>> 95 static bool igp_read_bios_from_vram(struct amdgpu_device *adev) >> >>>> 96 { >> >>>> 97 uint8_t __iomem *bios; >> >>>> 98 resource_size_t vram_base; >> >>>> 99 resource_size_t size = 256 * 1024; /* ??? */ >> >>>> 100 >> >>>> 101 if (!(adev->flags & AMD_IS_APU)) >> >>>> 102 if (amdgpu_need_post(adev)) >> >>>> 103 return false; >> >>>> >> >>>> >> >>>> This makes bios reading fallback to SMC INDEX/DATA register case. >> >>>> >> >>>> — >> >>>> Sincerely Yours, >> >>>> Pixel >> >>>> >> >>>> >> >>>> >> >>>> >> >>>> >> >>>> >> >>>> >> >>>> >> >>>> On 17/10/2017, 3:48 PM, "Liu, Monk" <[email protected]> wrote: >> >>>> >> >>>>> I don't understand how this patch works??? Looks like just rename >> vpost_needed to check_post >> >>>>> >> >>>>> -----Original Message----- >> >>>>> From: Pixel Ding [mailto:[email protected]] >> >>>>> Sent: 2017年10月17日 14:38 >> >>>>> To: [email protected]; Liu, Monk >> <[email protected]>; Koenig, Christian <[email protected]> >> >>>>> Cc: Li, Bingley <[email protected]>; Sun, Gary >> <[email protected]>; Ding, Pixel <[email protected]> >> >>>>> Subject: [PATCH 1/3] drm/amdgpu: always consider virualised device >> for checking post >> >>>>> >> >>>>> From: pding <[email protected]> >> >>>>> >> >>>>> The post checking on scratch registers isn't reliable for virtual >> >>>>> function. >> >>>>> >> >>>>> Signed-off-by: pding <[email protected]> >> >>>>> --- >> >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++---- >> >>>>> 1 file changed, 4 insertions(+), 4 deletions(-) >> >>>>> >> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> >>>>> index 683965b..ab8f0d6 100644 >> >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> >>>>> @@ -669,7 +669,7 @@ void amdgpu_gart_location(struct >> amdgpu_device *adev, struct amdgpu_mc *mc) >> >>>>> * or post is needed if hw reset is performed. >> >>>>> * Returns true if need or false if not. >> >>>>> */ >> >>>>> -bool amdgpu_need_post(struct amdgpu_device *adev) >> >>>>> +static bool amdgpu_check_post(struct amdgpu_device *adev) >> >>>>> { >> >>>>> uint32_t reg; >> >>>>> >> >>>>> @@ -692,7 +692,7 @@ bool amdgpu_need_post(struct >> amdgpu_device *adev) >> >>>>> >> >>>>> } >> >>>>> >> >>>>> -static bool amdgpu_vpost_needed(struct amdgpu_device *adev) >> >>>>> +bool amdgpu_need_post(struct amdgpu_device *adev) >> >>>>> { >> >>>>> if (amdgpu_sriov_vf(adev)) >> >>>>> return false; >> >>>>> @@ -716,7 +716,7 @@ static bool amdgpu_vpost_needed(struct >> amdgpu_device *adev) >> >>>>> return true; >> >>>>> } >> >>>>> } >> >>>>> - return amdgpu_need_post(adev); >> >>>>> + return amdgpu_check_post(adev); >> >>>>> } >> >>>>> >> >>>>> /** >> >>>>> @@ -2208,7 +2208,7 @@ int amdgpu_device_init(struct >> amdgpu_device *adev, >> >>>>> amdgpu_device_detect_sriov_bios(adev); >> >>>>> >> >>>>> /* Post card if necessary */ >> >>>>> - if (amdgpu_vpost_needed(adev)) { >> >>>>> + if (amdgpu_need_post(adev)) { >> >>>>> if (!adev->bios) { >> >>>>> dev_err(adev->dev, "no vBIOS found\n"); >> >>>>> >> amdgpu_vf_error_put(AMDGIM_ERROR_VF_NO_VBIOS, 0, 0); >> >>>>> -- >> >>>>> 2.9.5 >> >>>>> >> >>> _______________________________________________ >> >>> 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 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
