> -----Original Message----- > From: Ding, Pixel > Sent: Wednesday, October 18, 2017 11:23 PM > To: Deucher, Alexander; Koenig, Christian; Liu, Monk; amd- > [email protected] > Cc: Sun, Gary; Li, Bingley > Subject: Re: [PATCH 1/3] drm/amdgpu: always consider virualised device for > checking post > > 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?
I don't care what function name we use. Just merge them into one if it's ok for virtualization. Alex > > — > 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]>; amd- > [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
