[PATCH] drm/amdgpu: Update ring scheduler info as needed

Lazar, Lijo lijo.lazar at amd.com
Tue Jul 18 15:54:42 UTC 2023



On 7/18/2023 8:39 PM, James Zhu wrote:
> 
> On 2023-07-18 10:19, Lazar, Lijo wrote:
>>
>>
>> On 7/18/2023 7:30 PM, James Zhu wrote:
>>>
>>> On 2023-07-18 08:21, Lijo Lazar wrote:
>>>> Not all rings have scheduler associated. Only update scheduler data for
>>>> rings with scheduler. It could result in out of bound access as total
>>>> rings are more than those associated with particular IPs.
>>>>
>>>> Signed-off-by: Lijo Lazar <lijo.lazar at amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>>>> index 72b629a78c62..d0fc62784e82 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>>>> @@ -134,7 +134,7 @@ static int aqua_vanjaram_xcp_sched_list_update(
>>>>       for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
>>>>           ring = adev->rings[i];
>>>> -        if (!ring || !ring->sched.ready)
>>>> +        if (!ring || !ring->sched.ready || ring->no_scheduler)
>>> [JZ] any case for ring->no_scheduler = true, but ring->sched.ready = 
>>> true?
>>
>> amdgpu_gfx_kiq_init_ring sets no_scheduler flag true for kiq rings. 
>> amdgpu_device_init_schedulers inits schedulers only for rings which 
>> doesn't have no_scheduler. However in gfx_v9_4_3_xcc_kiq_resume, the 
>> ring is marked with ring->sched.ready = true;
>>
>> I'm not sure why it's done this way, but this is the case even for gfxv9.
> [JZ] I think the sched.ready confuses people. here just means ring is 
> ready be used.

Thanks for the clarification. One question is then do we need to check 
ring ready status for assigning xcp ids or just check if the ring is 
associated with a scheduler?

Keep only this - if (!ring || ring->no_scheduler) to assign xcp ids and 
leave the ring ready status to the logic which really accepts jobs on 
the ring?

Thanks,
Lijo

>> This patch is Reviewed-by: James Zhu <James.Zhu at amd.com>
> 
>> Thanks,
>> Lijo
>>
>>>>               continue;
>>>>           aqua_vanjaram_xcp_gpu_sched_update(adev, ring, ring->xcp_id);


More information about the amd-gfx mailing list