[PATCH] drm/amdkfd: Disable Packet Manager in non HWS mode except Hawaii

[PATCH] drm/amdkfd: Disable Packet Manager in non HWS mode except Hawaii

[Zhao, Yong]

The packet manager is only needed for HWS mode, as well as Hawaii in non
HWS mode. So only initialize it under those scenarios. This is useful
especially for emulation environment when things are slow.

Change-Id: Iedfa07c94241e3252463e1e5ea537543c2ccef03
Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 1d6b15788ebf..ec83914d9867 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -911,13 +911,22 @@ static void uninitialize(struct device_queue_manager *dqm)
 
 static int start_nocpsch(struct device_queue_manager *dqm)
 {
+	int ret = 0;
 	init_interrupts(dqm);
-	return pm_init(&dqm->packets, dqm);
+	/* Cache flushing on Hawaii in non HWS mode is done through packet
+	 * manager (PM), so we need to initialize PM for Hawaii.
+	 */
+	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
+		ret = pm_init(&dqm->packets, dqm);
+
+	return ret;
 }
 
 static int stop_nocpsch(struct device_queue_manager *dqm)
 {
-	pm_uninit(&dqm->packets);
+	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
+		pm_uninit(&dqm->packets);
+
 	return 0;
 }
 
-- 
2.17.1

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

Re: [PATCH] drm/amdkfd: Disable Packet Manager in non HWS mode except Hawaii

[Kuehling, Felix]

On 2019-04-17 4:54 p.m., Zhao, Yong wrote:
> The packet manager is only needed for HWS mode, as well as Hawaii in non
> HWS mode. So only initialize it under those scenarios. This is useful
> especially for emulation environment when things are slow.

I never thought of packet manager initialization as something expensive. 
Why does this matter? In emulation, the GPU is slow, but the CPU should 
be OK. Packet manager initialization doesn't do any waiting for the GPU, 
so I don't see how this would have any measurable impact.

Anyway, see one cosmetic comment inline.


>
> Change-Id: Iedfa07c94241e3252463e1e5ea537543c2ccef03
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
> ---
>   .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 1d6b15788ebf..ec83914d9867 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -911,13 +911,22 @@ static void uninitialize(struct device_queue_manager *dqm)
>   
>   static int start_nocpsch(struct device_queue_manager *dqm)
>   {
> +	int ret = 0;

checkpatch.pl would complain that there should be an empty line after 
variable declarations.


>   	init_interrupts(dqm);
> -	return pm_init(&dqm->packets, dqm);
> +	/* Cache flushing on Hawaii in non HWS mode is done through packet
> +	 * manager (PM), so we need to initialize PM for Hawaii.
> +	 */
> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
> +		ret = pm_init(&dqm->packets, dqm);
> +
> +	return ret;
>   }
>   
>   static int stop_nocpsch(struct device_queue_manager *dqm)
>   {
> -	pm_uninit(&dqm->packets);
> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
> +		pm_uninit(&dqm->packets);
> +
>   	return 0;
>   }
>   
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdkfd: Disable Packet Manager in non HWS mode except Hawaii

[Zhao, Yong]

I can fix that cosmetic line. I did not measure the difference, but 
driver initialization usually takes a longer time, that's why I am 
trying to reduce it. Also, it means one less thing to worry about during 
non HWS mode bringup, because we don't need to deal with HIQ any more. 
With that, what do you think now?

Regards,

Yong

On 2019-04-17 5:06 p.m., Kuehling, Felix wrote:
> On 2019-04-17 4:54 p.m., Zhao, Yong wrote:
>> The packet manager is only needed for HWS mode, as well as Hawaii in non
>> HWS mode. So only initialize it under those scenarios. This is useful
>> especially for emulation environment when things are slow.
> I never thought of packet manager initialization as something expensive.
> Why does this matter? In emulation, the GPU is slow, but the CPU should
> be OK. Packet manager initialization doesn't do any waiting for the GPU,
> so I don't see how this would have any measurable impact.
>
> Anyway, see one cosmetic comment inline.
>
>
>> Change-Id: Iedfa07c94241e3252463e1e5ea537543c2ccef03
>> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
>> ---
>>    .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   | 13 +++++++++++--
>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> index 1d6b15788ebf..ec83914d9867 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -911,13 +911,22 @@ static void uninitialize(struct device_queue_manager *dqm)
>>    
>>    static int start_nocpsch(struct device_queue_manager *dqm)
>>    {
>> +	int ret = 0;
> checkpatch.pl would complain that there should be an empty line after
> variable declarations.
>
>
>>    	init_interrupts(dqm);
>> -	return pm_init(&dqm->packets, dqm);
>> +	/* Cache flushing on Hawaii in non HWS mode is done through packet
>> +	 * manager (PM), so we need to initialize PM for Hawaii.
>> +	 */
>> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
>> +		ret = pm_init(&dqm->packets, dqm);
>> +
>> +	return ret;
>>    }
>>    
>>    static int stop_nocpsch(struct device_queue_manager *dqm)
>>    {
>> -	pm_uninit(&dqm->packets);
>> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
>> +		pm_uninit(&dqm->packets);
>> +
>>    	return 0;
>>    }
>>    
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdkfd: Disable Packet Manager in non HWS mode except Hawaii

[Kuehling, Felix]

If you want to optimize driver init time, you should check what actually 
takes the most time. Randomly micro-optimizing things that may not even 
matter only increases complexity for no benefit.

Typically the things that make initialization slow are points where we 
synchronize with the GPU or random sleeps or delays to allow the 
hardware to complete something. A few lines of code that only run on the 
CPU will not make an appreciable difference.

Regards,
   Felix

On 2019-04-17 5:17 p.m., Zhao, Yong wrote:
> I can fix that cosmetic line. I did not measure the difference, but
> driver initialization usually takes a longer time, that's why I am
> trying to reduce it. Also, it means one less thing to worry about during
> non HWS mode bringup, because we don't need to deal with HIQ any more.
> With that, what do you think now?
>
> Regards,
>
> Yong
>
> On 2019-04-17 5:06 p.m., Kuehling, Felix wrote:
>> On 2019-04-17 4:54 p.m., Zhao, Yong wrote:
>>> The packet manager is only needed for HWS mode, as well as Hawaii in non
>>> HWS mode. So only initialize it under those scenarios. This is useful
>>> especially for emulation environment when things are slow.
>> I never thought of packet manager initialization as something expensive.
>> Why does this matter? In emulation, the GPU is slow, but the CPU should
>> be OK. Packet manager initialization doesn't do any waiting for the GPU,
>> so I don't see how this would have any measurable impact.
>>
>> Anyway, see one cosmetic comment inline.
>>
>>
>>> Change-Id: Iedfa07c94241e3252463e1e5ea537543c2ccef03
>>> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
>>> ---
>>>     .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   | 13 +++++++++++--
>>>     1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> index 1d6b15788ebf..ec83914d9867 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> @@ -911,13 +911,22 @@ static void uninitialize(struct device_queue_manager *dqm)
>>>     
>>>     static int start_nocpsch(struct device_queue_manager *dqm)
>>>     {
>>> +	int ret = 0;
>> checkpatch.pl would complain that there should be an empty line after
>> variable declarations.
>>
>>
>>>     	init_interrupts(dqm);
>>> -	return pm_init(&dqm->packets, dqm);
>>> +	/* Cache flushing on Hawaii in non HWS mode is done through packet
>>> +	 * manager (PM), so we need to initialize PM for Hawaii.
>>> +	 */
>>> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
>>> +		ret = pm_init(&dqm->packets, dqm);
>>> +
>>> +	return ret;
>>>     }
>>>     
>>>     static int stop_nocpsch(struct device_queue_manager *dqm)
>>>     {
>>> -	pm_uninit(&dqm->packets);
>>> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
>>> +		pm_uninit(&dqm->packets);
>>> +
>>>     	return 0;
>>>     }
>>>     
> _______________________________________________
> 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

Re: [PATCH] drm/amdkfd: Disable Packet Manager in non HWS mode except Hawaii

[Zhao, Yong]

Okay, I will abandon it.

Yong

On 2019-04-17 5:44 p.m., Kuehling, Felix wrote:
> If you want to optimize driver init time, you should check what actually
> takes the most time. Randomly micro-optimizing things that may not even
> matter only increases complexity for no benefit.
>
> Typically the things that make initialization slow are points where we
> synchronize with the GPU or random sleeps or delays to allow the
> hardware to complete something. A few lines of code that only run on the
> CPU will not make an appreciable difference.
>
> Regards,
>     Felix
>
> On 2019-04-17 5:17 p.m., Zhao, Yong wrote:
>> I can fix that cosmetic line. I did not measure the difference, but
>> driver initialization usually takes a longer time, that's why I am
>> trying to reduce it. Also, it means one less thing to worry about during
>> non HWS mode bringup, because we don't need to deal with HIQ any more.
>> With that, what do you think now?
>>
>> Regards,
>>
>> Yong
>>
>> On 2019-04-17 5:06 p.m., Kuehling, Felix wrote:
>>> On 2019-04-17 4:54 p.m., Zhao, Yong wrote:
>>>> The packet manager is only needed for HWS mode, as well as Hawaii in non
>>>> HWS mode. So only initialize it under those scenarios. This is useful
>>>> especially for emulation environment when things are slow.
>>> I never thought of packet manager initialization as something expensive.
>>> Why does this matter? In emulation, the GPU is slow, but the CPU should
>>> be OK. Packet manager initialization doesn't do any waiting for the GPU,
>>> so I don't see how this would have any measurable impact.
>>>
>>> Anyway, see one cosmetic comment inline.
>>>
>>>
>>>> Change-Id: Iedfa07c94241e3252463e1e5ea537543c2ccef03
>>>> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
>>>> ---
>>>>      .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   | 13 +++++++++++--
>>>>      1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>>> index 1d6b15788ebf..ec83914d9867 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>>> @@ -911,13 +911,22 @@ static void uninitialize(struct device_queue_manager *dqm)
>>>>      
>>>>      static int start_nocpsch(struct device_queue_manager *dqm)
>>>>      {
>>>> +	int ret = 0;
>>> checkpatch.pl would complain that there should be an empty line after
>>> variable declarations.
>>>
>>>
>>>>      	init_interrupts(dqm);
>>>> -	return pm_init(&dqm->packets, dqm);
>>>> +	/* Cache flushing on Hawaii in non HWS mode is done through packet
>>>> +	 * manager (PM), so we need to initialize PM for Hawaii.
>>>> +	 */
>>>> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
>>>> +		ret = pm_init(&dqm->packets, dqm);
>>>> +
>>>> +	return ret;
>>>>      }
>>>>      
>>>>      static int stop_nocpsch(struct device_queue_manager *dqm)
>>>>      {
>>>> -	pm_uninit(&dqm->packets);
>>>> +	if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
>>>> +		pm_uninit(&dqm->packets);
>>>> +
>>>>      	return 0;
>>>>      }
>>>>      
>> _______________________________________________
>> 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