[PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Pixel Ding]

From: pding <Pixel.Ding@amd.com>

Move kfd probe prior to device init. Release exclusive mode
after hw_init if kfd is not enabled.

Signed-off-by: pding <Pixel.Ding@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 400dfaa..e46ec51 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
 		adev->ip_blocks[i].status.hw = true;
 	}
 
+	if (amdgpu_sriov_vf(adev) && !adev->kfd)
+		amdgpu_virt_release_full_gpu(adev, true);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 3e9760d..e91907c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	    !pci_is_thunderbolt_attached(dev->pdev))
 		flags |= AMD_IS_PX;
 
+	amdgpu_amdkfd_device_probe(adev);
+
 	/* amdgpu_device_init should report only fatal error
 	 * like memory allocation failure or iomapping failure,
 	 * or memory manager initialization failure, it must
@@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
 				"Error during ACPI methods call\n");
 	}
 
-	amdgpu_amdkfd_device_probe(adev);
 	amdgpu_amdkfd_device_init(adev);
 
 	if (amdgpu_device_is_px(dev)) {
@@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
 		pm_runtime_put_autosuspend(dev->dev);
 	}
 
-	if (amdgpu_sriov_vf(adev))
+	if (amdgpu_sriov_vf(adev) && adev->kfd)
 		amdgpu_virt_release_full_gpu(adev, true);
 
 out:
-- 
2.9.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Oded Gabbay]

On Mon, Oct 30, 2017 at 9:57 AM, Pixel Ding <Pixel.Ding@amd.com> wrote:
> From: pding <Pixel.Ding@amd.com>
>
> Move kfd probe prior to device init. Release exclusive mode
> after hw_init if kfd is not enabled.
>
> Signed-off-by: pding <Pixel.Ding@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 400dfaa..e46ec51 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
>                 adev->ip_blocks[i].status.hw = true;
>         }
>
> +       if (amdgpu_sriov_vf(adev) && !adev->kfd)
> +               amdgpu_virt_release_full_gpu(adev, true);
> +
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 3e9760d..e91907c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>             !pci_is_thunderbolt_attached(dev->pdev))
>                 flags |= AMD_IS_PX;
>
> +       amdgpu_amdkfd_device_probe(adev);
> +
>         /* amdgpu_device_init should report only fatal error
>          * like memory allocation failure or iomapping failure,
>          * or memory manager initialization failure, it must
> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>                                 "Error during ACPI methods call\n");
>         }
>
> -       amdgpu_amdkfd_device_probe(adev);
>         amdgpu_amdkfd_device_init(adev);
>
>         if (amdgpu_device_is_px(dev)) {
> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>                 pm_runtime_put_autosuspend(dev->dev);
>         }
>
> -       if (amdgpu_sriov_vf(adev))
> +       if (amdgpu_sriov_vf(adev) && adev->kfd)
>                 amdgpu_virt_release_full_gpu(adev, true);
>
>  out:
> --
> 2.9.5
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

The amdkfd probe function uses the pdev field inside adev. That field
is initialized in device init, so you can't call amdkfd probe before
that function.
Oded
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Ding, Pixel]

Get your point. I don’t know why the test is passed however will revise later.

I definitely want the know if KFD is enabled before device init. Any suggestion? Or do you mind if the pdev is passed to probe in other way?

— 
Sincerely Yours,
Pixel








On 30/10/2017, 4:20 PM, "Oded Gabbay" <oded.gabbay@gmail.com> wrote:

>On Mon, Oct 30, 2017 at 9:57 AM, Pixel Ding <Pixel.Ding@amd.com> wrote:
>> From: pding <Pixel.Ding@amd.com>
>>
>> Move kfd probe prior to device init. Release exclusive mode
>> after hw_init if kfd is not enabled.
>>
>> Signed-off-by: pding <Pixel.Ding@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 400dfaa..e46ec51 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
>>                 adev->ip_blocks[i].status.hw = true;
>>         }
>>
>> +       if (amdgpu_sriov_vf(adev) && !adev->kfd)
>> +               amdgpu_virt_release_full_gpu(adev, true);
>> +
>>         return 0;
>>  }
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index 3e9760d..e91907c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>             !pci_is_thunderbolt_attached(dev->pdev))
>>                 flags |= AMD_IS_PX;
>>
>> +       amdgpu_amdkfd_device_probe(adev);
>> +
>>         /* amdgpu_device_init should report only fatal error
>>          * like memory allocation failure or iomapping failure,
>>          * or memory manager initialization failure, it must
>> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>                                 "Error during ACPI methods call\n");
>>         }
>>
>> -       amdgpu_amdkfd_device_probe(adev);
>>         amdgpu_amdkfd_device_init(adev);
>>
>>         if (amdgpu_device_is_px(dev)) {
>> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>                 pm_runtime_put_autosuspend(dev->dev);
>>         }
>>
>> -       if (amdgpu_sriov_vf(adev))
>> +       if (amdgpu_sriov_vf(adev) && adev->kfd)
>>                 amdgpu_virt_release_full_gpu(adev, true);
>>
>>  out:
>> --
>> 2.9.5
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>The amdkfd probe function uses the pdev field inside adev. That field
>is initialized in device init, so you can't call amdkfd probe before
>that function.
>Oded
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Oded Gabbay]

Except from pdev, kfd doesn't access any other fields in adev, so
passing pdev as a different parameter seems fine.
The problem with that approach is we need to remember for the future
not to access other adev fields, and that seems risky and not correct




On Mon, Oct 30, 2017 at 10:30 AM, Ding, Pixel <Pixel.Ding@amd.com> wrote:
> Get your point. I don’t know why the test is passed however will revise later.
>
> I definitely want the know if KFD is enabled before device init. Any suggestion? Or do you mind if the pdev is passed to probe in other way?
>
> —
> Sincerely Yours,
> Pixel
>
>
>
>
>
>
>
>
> On 30/10/2017, 4:20 PM, "Oded Gabbay" <oded.gabbay@gmail.com> wrote:
>
>>On Mon, Oct 30, 2017 at 9:57 AM, Pixel Ding <Pixel.Ding@amd.com> wrote:
>>> From: pding <Pixel.Ding@amd.com>
>>>
>>> Move kfd probe prior to device init. Release exclusive mode
>>> after hw_init if kfd is not enabled.
>>>
>>> Signed-off-by: pding <Pixel.Ding@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
>>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 400dfaa..e46ec51 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
>>>                 adev->ip_blocks[i].status.hw = true;
>>>         }
>>>
>>> +       if (amdgpu_sriov_vf(adev) && !adev->kfd)
>>> +               amdgpu_virt_release_full_gpu(adev, true);
>>> +
>>>         return 0;
>>>  }
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 3e9760d..e91907c 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>             !pci_is_thunderbolt_attached(dev->pdev))
>>>                 flags |= AMD_IS_PX;
>>>
>>> +       amdgpu_amdkfd_device_probe(adev);
>>> +
>>>         /* amdgpu_device_init should report only fatal error
>>>          * like memory allocation failure or iomapping failure,
>>>          * or memory manager initialization failure, it must
>>> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>                                 "Error during ACPI methods call\n");
>>>         }
>>>
>>> -       amdgpu_amdkfd_device_probe(adev);
>>>         amdgpu_amdkfd_device_init(adev);
>>>
>>>         if (amdgpu_device_is_px(dev)) {
>>> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>                 pm_runtime_put_autosuspend(dev->dev);
>>>         }
>>>
>>> -       if (amdgpu_sriov_vf(adev))
>>> +       if (amdgpu_sriov_vf(adev) && adev->kfd)
>>>                 amdgpu_virt_release_full_gpu(adev, true);
>>>
>>>  out:
>>> --
>>> 2.9.5
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>>The amdkfd probe function uses the pdev field inside adev. That field
>>is initialized in device init, so you can't call amdkfd probe before
>>that function.
>>Oded
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Ding, Pixel]

Another option is adding a function with the same logics as top half probe to determine if KFD is enabled or not, like amdgpu_amdkfd_enabled().

I think it’s okay to pass pdev separately. The KFD probe implementation also has pdev parameter, it doesn’t retrieve that from adev, while the middle layer does this.

Which one do you prefer?

— 
Sincerely Yours,
Pixel







On 30/10/2017, 4:52 PM, "Oded Gabbay" <oded.gabbay@gmail.com> wrote:

>Except from pdev, kfd doesn't access any other fields in adev, so
>passing pdev as a different parameter seems fine.
>The problem with that approach is we need to remember for the future
>not to access other adev fields, and that seems risky and not correct
>
>
>
>
>On Mon, Oct 30, 2017 at 10:30 AM, Ding, Pixel <Pixel.Ding@amd.com> wrote:
>> Get your point. I don’t know why the test is passed however will revise later.
>>
>> I definitely want the know if KFD is enabled before device init. Any suggestion? Or do you mind if the pdev is passed to probe in other way?
>>
>> —
>> Sincerely Yours,
>> Pixel
>>
>>
>>
>>
>>
>>
>>
>>
>> On 30/10/2017, 4:20 PM, "Oded Gabbay" <oded.gabbay@gmail.com> wrote:
>>
>>>On Mon, Oct 30, 2017 at 9:57 AM, Pixel Ding <Pixel.Ding@amd.com> wrote:
>>>> From: pding <Pixel.Ding@amd.com>
>>>>
>>>> Move kfd probe prior to device init. Release exclusive mode
>>>> after hw_init if kfd is not enabled.
>>>>
>>>> Signed-off-by: pding <Pixel.Ding@amd.com>
>>>> ---
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
>>>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index 400dfaa..e46ec51 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
>>>>                 adev->ip_blocks[i].status.hw = true;
>>>>         }
>>>>
>>>> +       if (amdgpu_sriov_vf(adev) && !adev->kfd)
>>>> +               amdgpu_virt_release_full_gpu(adev, true);
>>>> +
>>>>         return 0;
>>>>  }
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> index 3e9760d..e91907c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>>             !pci_is_thunderbolt_attached(dev->pdev))
>>>>                 flags |= AMD_IS_PX;
>>>>
>>>> +       amdgpu_amdkfd_device_probe(adev);
>>>> +
>>>>         /* amdgpu_device_init should report only fatal error
>>>>          * like memory allocation failure or iomapping failure,
>>>>          * or memory manager initialization failure, it must
>>>> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>>                                 "Error during ACPI methods call\n");
>>>>         }
>>>>
>>>> -       amdgpu_amdkfd_device_probe(adev);
>>>>         amdgpu_amdkfd_device_init(adev);
>>>>
>>>>         if (amdgpu_device_is_px(dev)) {
>>>> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>>                 pm_runtime_put_autosuspend(dev->dev);
>>>>         }
>>>>
>>>> -       if (amdgpu_sriov_vf(adev))
>>>> +       if (amdgpu_sriov_vf(adev) && adev->kfd)
>>>>                 amdgpu_virt_release_full_gpu(adev, true);
>>>>
>>>>  out:
>>>> --
>>>> 2.9.5
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>>The amdkfd probe function uses the pdev field inside adev. That field
>>>is initialized in device init, so you can't call amdkfd probe before
>>>that function.
>>>Oded
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Oded Gabbay]

I would pass only pdev to kgd2kfd_probe, instead of adev.
Then, I would initialize adev inside kgd2kfd_device_init
That way, you can call kgd2kfd_probe before device_init of amdgpu and
you can know if amdkfd is supposed to handle this device.
Does that make sense ?

On Mon, Oct 30, 2017 at 11:13 AM, Ding, Pixel <Pixel.Ding@amd.com> wrote:
> Another option is adding a function with the same logics as top half probe to determine if KFD is enabled or not, like amdgpu_amdkfd_enabled().
>
> I think it’s okay to pass pdev separately. The KFD probe implementation also has pdev parameter, it doesn’t retrieve that from adev, while the middle layer does this.
>
> Which one do you prefer?
>
> —
> Sincerely Yours,
> Pixel
>
>
>
>
>
>
>
> On 30/10/2017, 4:52 PM, "Oded Gabbay" <oded.gabbay@gmail.com> wrote:
>
>>Except from pdev, kfd doesn't access any other fields in adev, so
>>passing pdev as a different parameter seems fine.
>>The problem with that approach is we need to remember for the future
>>not to access other adev fields, and that seems risky and not correct
>>
>>
>>
>>
>>On Mon, Oct 30, 2017 at 10:30 AM, Ding, Pixel <Pixel.Ding@amd.com> wrote:
>>> Get your point. I don’t know why the test is passed however will revise later.
>>>
>>> I definitely want the know if KFD is enabled before device init. Any suggestion? Or do you mind if the pdev is passed to probe in other way?
>>>
>>> —
>>> Sincerely Yours,
>>> Pixel
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On 30/10/2017, 4:20 PM, "Oded Gabbay" <oded.gabbay@gmail.com> wrote:
>>>
>>>>On Mon, Oct 30, 2017 at 9:57 AM, Pixel Ding <Pixel.Ding@amd.com> wrote:
>>>>> From: pding <Pixel.Ding@amd.com>
>>>>>
>>>>> Move kfd probe prior to device init. Release exclusive mode
>>>>> after hw_init if kfd is not enabled.
>>>>>
>>>>> Signed-off-by: pding <Pixel.Ding@amd.com>
>>>>> ---
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
>>>>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> index 400dfaa..e46ec51 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
>>>>>                 adev->ip_blocks[i].status.hw = true;
>>>>>         }
>>>>>
>>>>> +       if (amdgpu_sriov_vf(adev) && !adev->kfd)
>>>>> +               amdgpu_virt_release_full_gpu(adev, true);
>>>>> +
>>>>>         return 0;
>>>>>  }
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> index 3e9760d..e91907c 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>>>> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>>>             !pci_is_thunderbolt_attached(dev->pdev))
>>>>>                 flags |= AMD_IS_PX;
>>>>>
>>>>> +       amdgpu_amdkfd_device_probe(adev);
>>>>> +
>>>>>         /* amdgpu_device_init should report only fatal error
>>>>>          * like memory allocation failure or iomapping failure,
>>>>>          * or memory manager initialization failure, it must
>>>>> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>>>                                 "Error during ACPI methods call\n");
>>>>>         }
>>>>>
>>>>> -       amdgpu_amdkfd_device_probe(adev);
>>>>>         amdgpu_amdkfd_device_init(adev);
>>>>>
>>>>>         if (amdgpu_device_is_px(dev)) {
>>>>> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>>>>                 pm_runtime_put_autosuspend(dev->dev);
>>>>>         }
>>>>>
>>>>> -       if (amdgpu_sriov_vf(adev))
>>>>> +       if (amdgpu_sriov_vf(adev) && adev->kfd)
>>>>>                 amdgpu_virt_release_full_gpu(adev, true);
>>>>>
>>>>>  out:
>>>>> --
>>>>> 2.9.5
>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>
>>>>The amdkfd probe function uses the pdev field inside adev. That field
>>>>is initialized in device init, so you can't call amdkfd probe before
>>>>that function.
>>>>Oded
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Tom Stellard]

On 10/30/2017 12:57 AM, Pixel Ding wrote:
> From: pding <Pixel.Ding@amd.com>
> 
> Move kfd probe prior to device init. Release exclusive mode
> after hw_init if kfd is not enabled.
> 

What happens if only the amdgpu module is loaded at startup, and then the
user manually loads the amdkfd module at some point later on.  Will the driver
still behave correctly in this case with this patch?

-Tom


> Signed-off-by: pding <Pixel.Ding@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 400dfaa..e46ec51 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
>  		adev->ip_blocks[i].status.hw = true;
>  	}
>  
> +	if (amdgpu_sriov_vf(adev) && !adev->kfd)
> +		amdgpu_virt_release_full_gpu(adev, true);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 3e9760d..e91907c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>  	    !pci_is_thunderbolt_attached(dev->pdev))
>  		flags |= AMD_IS_PX;
>  
> +	amdgpu_amdkfd_device_probe(adev);
> +
>  	/* amdgpu_device_init should report only fatal error
>  	 * like memory allocation failure or iomapping failure,
>  	 * or memory manager initialization failure, it must
> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>  				"Error during ACPI methods call\n");
>  	}
>  
> -	amdgpu_amdkfd_device_probe(adev);
>  	amdgpu_amdkfd_device_init(adev);
>  
>  	if (amdgpu_device_is_px(dev)) {
> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>  		pm_runtime_put_autosuspend(dev->dev);
>  	}
>  
> -	if (amdgpu_sriov_vf(adev))
> +	if (amdgpu_sriov_vf(adev) && adev->kfd)
>  		amdgpu_virt_release_full_gpu(adev, true);
>  
>  out:
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Ding, Pixel]

I’m not for sure about this case you talked about. Assume that it could happen and the KFD probe and init are invoked when loading it manually.

For baremetal device, it’s always correct.
For SRIOV virtual function, it doesn’t behave correctly with or without this patch. KFD initialization also needs to access VF in exclusive mode, while the exclusive mode request/release messages are sent in amdgpu_device_init.

—
Sincerely Yours,
Pixel







On 31/10/2017, 11:06 PM, "Tom Stellard" <tstellar@redhat.com> wrote:

>On 10/30/2017 12:57 AM, Pixel Ding wrote:
>> From: pding <Pixel.Ding@amd.com>
>> 
>> Move kfd probe prior to device init. Release exclusive mode
>> after hw_init if kfd is not enabled.
>> 
>
>What happens if only the amdgpu module is loaded at startup, and then the
>user manually loads the amdkfd module at some point later on.  Will the driver
>still behave correctly in this case with this patch?
>
>-Tom
>
>
>> Signed-off-by: pding <Pixel.Ding@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 400dfaa..e46ec51 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
>>  		adev->ip_blocks[i].status.hw = true;
>>  	}
>>  
>> +	if (amdgpu_sriov_vf(adev) && !adev->kfd)
>> +		amdgpu_virt_release_full_gpu(adev, true);
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index 3e9760d..e91907c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>  	    !pci_is_thunderbolt_attached(dev->pdev))
>>  		flags |= AMD_IS_PX;
>>  
>> +	amdgpu_amdkfd_device_probe(adev);
>> +
>>  	/* amdgpu_device_init should report only fatal error
>>  	 * like memory allocation failure or iomapping failure,
>>  	 * or memory manager initialization failure, it must
>> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>  				"Error during ACPI methods call\n");
>>  	}
>>  
>> -	amdgpu_amdkfd_device_probe(adev);
>>  	amdgpu_amdkfd_device_init(adev);
>>  
>>  	if (amdgpu_device_is_px(dev)) {
>> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
>>  		pm_runtime_put_autosuspend(dev->dev);
>>  	}
>>  
>> -	if (amdgpu_sriov_vf(adev))
>> +	if (amdgpu_sriov_vf(adev) && adev->kfd)
>>  		amdgpu_virt_release_full_gpu(adev, true);
>>  
>>  out:
>> 
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: release exclusive mode after hw_init if no kfd

[Oded Gabbay]

Tom,
AFAIK, you can't manually load the amdkfd driver. i.e. it simply won't
work, with or without this change.
It must be loaded in a certain order (in relation to amdgpu and
amd_iommu_v2) during Linux boot.
That issue is one that has been with us from the start and has
received justified criticism over the years.
Unfortunately, no one bothered to fix it yet.

Oded

On Wed, Nov 1, 2017 at 3:25 AM, Ding, Pixel <Pixel.Ding@amd.com> wrote:
>
> I’m not for sure about this case you talked about. Assume that it could happen and the KFD probe and init are invoked when loading it manually.
>
> For baremetal device, it’s always correct.
> For SRIOV virtual function, it doesn’t behave correctly with or without this patch. KFD initialization also needs to access VF in exclusive mode, while the exclusive mode request/release messages are sent in amdgpu_device_init.
>
> —
> Sincerely Yours,
> Pixel
>
>
>
>
>
>
>
> On 31/10/2017, 11:06 PM, "Tom Stellard" <tstellar@redhat.com> wrote:
>
> >On 10/30/2017 12:57 AM, Pixel Ding wrote:
> >> From: pding <Pixel.Ding@amd.com>
> >>
> >> Move kfd probe prior to device init. Release exclusive mode
> >> after hw_init if kfd is not enabled.
> >>
> >
> >What happens if only the amdgpu module is loaded at startup, and then the
> >user manually loads the amdkfd module at some point later on.  Will the driver
> >still behave correctly in this case with this patch?
> >
> >-Tom
> >
> >
> >> Signed-off-by: pding <Pixel.Ding@amd.com>
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 5 +++--
> >>  2 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 400dfaa..e46ec51 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -1716,6 +1716,9 @@ static int amdgpu_init(struct amdgpu_device *adev)
> >>              adev->ip_blocks[i].status.hw = true;
> >>      }
> >>
> >> +    if (amdgpu_sriov_vf(adev) && !adev->kfd)
> >> +            amdgpu_virt_release_full_gpu(adev, true);
> >> +
> >>      return 0;
> >>  }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> index 3e9760d..e91907c 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> @@ -138,6 +138,8 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
> >>          !pci_is_thunderbolt_attached(dev->pdev))
> >>              flags |= AMD_IS_PX;
> >>
> >> +    amdgpu_amdkfd_device_probe(adev);
> >> +
> >>      /* amdgpu_device_init should report only fatal error
> >>       * like memory allocation failure or iomapping failure,
> >>       * or memory manager initialization failure, it must
> >> @@ -170,7 +172,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
> >>                              "Error during ACPI methods call\n");
> >>      }
> >>
> >> -    amdgpu_amdkfd_device_probe(adev);
> >>      amdgpu_amdkfd_device_init(adev);
> >>
> >>      if (amdgpu_device_is_px(dev)) {
> >> @@ -182,7 +183,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags)
> >>              pm_runtime_put_autosuspend(dev->dev);
> >>      }
> >>
> >> -    if (amdgpu_sriov_vf(adev))
> >> +    if (amdgpu_sriov_vf(adev) && adev->kfd)
> >>              amdgpu_virt_release_full_gpu(adev, true);
> >>
> >>  out:
> >>
> >
> _______________________________________________
> 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