[PATCH 1/3] drm/amdgpu: always consider virualised device for checking post

Thu Oct 19 03:32:37 UTC 2017

> -----Original Message-----
> From: Ding, Pixel
> Sent: Wednesday, October 18, 2017 11:23 PM
> To: Deucher, Alexander; Koenig, Christian; 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
> 
> 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"
> <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