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

["Ding, Pixel" <Pixel.Ding-5C7GfCeVMHo@public.gmane.org>]

Sorry for the confusion, while I still believe this is the right way to go.

amdgpu_need_post and amdgpu_vpost_needed both try to check the VBIOS posting, amdgpu_need_post doesn’t consider VF/PT devices. They were both in use.

This patch is to unify these 2 functions to single one and always consider the VF/PT devices.

So the amdgpu_vpost_needed is renamed to amdgpu_need_post, and the previous amdgpu_need_post becomes a static function.
— 
Sincerely Yours,
Pixel







On 18/10/2017, 3:23 PM, "amd-gfx on behalf of Christian König" <amd-gfx-bounces@lists.freedesktop.org on behalf of christian.koenig@amd.com> wrote:

>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?
>
>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@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@lists.freedesktop.org on behalf of Pixel.Ding@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@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@amd.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>
>>>>> Cc: Li, Bingley <Bingley.Li@amd.com>; Sun, Gary <Gary.Sun@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@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@amd.com]
>>>>>> Sent: 2017年10月17日 14:38
>>>>>> To: amd-gfx@lists.freedesktop.org; Liu, Monk <Monk.Liu@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
>>>>>> Cc: Li, Bingley <Bingley.Li@amd.com>; Sun, Gary <Gary.Sun@amd.com>; Ding, Pixel <Pixel.Ding@amd.com>
>>>>>> Subject: [PATCH 1/3] drm/amdgpu: always consider virualised device for checking post
>>>>>>
>>>>>> From: pding <Pixel.Ding@amd.com>
>>>>>>
>>>>>> The post checking on scratch registers isn't reliable for virtual function.
>>>>>>
>>>>>> Signed-off-by: pding <Pixel.Ding@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@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>_______________________________________________
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx