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