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" <Alexander.Deucher at amd.com> wrote: >> -----Original Message----- >> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf >> Of Christian König >> Sent: Wednesday, October 18, 2017 3:23 AM >> To: Ding, Pixel; Liu, Monk; amd-gfx at lists.freedesktop.org >> 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" <Pixel.Ding at amd.com> 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- >> bounces at lists.freedesktop.org on behalf of Pixel.Ding at amd.com> 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" <Monk.Liu at amd.com> 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 <Monk.Liu at amd.com>; amd-gfx at lists.freedesktop.org; >> Koenig, Christian <Christian.Koenig at amd.com> >> >>>> Cc: Li, Bingley <Bingley.Li at amd.com>; Sun, Gary >> <Gary.Sun at amd.com> >> >>>> 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" <Monk.Liu at amd.com> wrote: >> >>>> >> >>>>> I don't understand how this patch works??? Looks like just rename >> vpost_needed to check_post >> >>>>> >> >>>>> -----Original Message----- >> >>>>> From: Pixel Ding [mailto:Pixel.Ding at amd.com] >> >>>>> Sent: 2017å¹´10æ??17æ?¥ 14:38 >> >>>>> To: amd-gfx at lists.freedesktop.org; Liu, Monk >> <Monk.Liu at amd.com>; Koenig, Christian <Christian.Koenig at amd.com> >> >>>>> Cc: Li, Bingley <Bingley.Li at amd.com>; Sun, Gary >> <Gary.Sun at amd.com>; Ding, Pixel <Pixel.Ding at amd.com> >> >>>>> Subject: [PATCH 1/3] drm/amdgpu: always consider virualised device >> for checking post >> >>>>> >> >>>>> From: pding <Pixel.Ding at amd.com> >> >>>>> >> >>>>> The post checking on scratch registers isn't reliable for virtual function. >> >>>>> >> >>>>> Signed-off-by: pding <Pixel.Ding at amd.com> >> >>>>> --- >> >>>>> 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 >> >>> amd-gfx at lists.freedesktop.org >> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
