> -----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
