Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f905c70e-b93d-5a26-cba9-c826a1fc21fe@amd.com>
Date:   Wed, 7 Sep 2022 12:18:37 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v2 1/4] drm/sched: Enable signaling for finished fence
Content-Language: en-US
To:     =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        Arvind Yadav <Arvind.Yadav@amd.com>, shashank.sharma@amd.com,
        amaranath.somalapuram@amd.com, Arunpravin.PaneerSelvam@amd.com,
        sumit.semwal@linaro.org, gustavo@padovan.org, airlied@linux.ie,
        daniel@ffwll.ch, linux-media@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
        linux-kernel@vger.kernel.org
References: <20220905163502.4032-1-Arvind.Yadav@amd.com>
 <20220905163502.4032-2-Arvind.Yadav@amd.com>
 <5568fad0-1503-a0f3-222e-c238fd4eefdd@amd.com>
 <ffb5acca-7c20-4497-d7d0-25508a6566d2@amd.com>
 <9b57b254-8d67-9937-5cec-783e7a4d8f56@amd.com>
From:   Andrey Grodzovsky <andrey.grodzovsky@amd.com>
In-Reply-To: <9b57b254-8d67-9937-5cec-783e7a4d8f56@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bDFuZG5Bc2tONHYycWs0ekh4dlJ1dFBMWTM3SXhrei9yaG5mY2JJWlU0N2E1?=
 =?utf-8?B?TlBZOEFCR213UHExRXdzdlovQmxpc2NpUkNrUXEwVDZoMW05dlIzSktOeEhV?=
 =?utf-8?B?NGJ1bERoemVJNUQxQUx6WjZ2UTMxclZzek85MnZ2R0NKcXFiSDArTWNTelRU?=
 =?utf-8?B?QjJhckRWemMxaHYzenVydTVrVmV2SGc0UlAxNnJSQmNqOTllWjBvc05wT2ZE?=
 =?utf-8?B?ZVVkM0VkaUFYcjlBVGsvbjVTenNSb3pMS1RDZG5sUkVXdE1rTkhRRFo0M3hm?=
 =?utf-8?B?cWxQa3g2R05DOFVFU2RnM0xYcmRiRFZsTnBjc1pwbGEzZC9DVG5VUE5nS0Fv?=
 =?utf-8?B?VzJVajJEbzF3NWRaLy9kUUROZytqNmY3TjAzMENXOU5YWktGK2V6VkE1WkZp?=
 =?utf-8?B?Tzg0bWJPb3R4V0htRUt3L3JQd0QrYjRTNDk3NUJRN1JHOFdOL0sralFiVGdp?=
 =?utf-8?B?M08xeldzcjg4UkdiRjRZNkxZRWxuWmhTcG1YYlJUMGs5Wm5jcnZZRWZxQWVx?=
 =?utf-8?B?YVVmSjNFTXM1WjMzdmxFV3F4TjZhM2ZsSjFvWEtZWlpTMHppK2xRajdLaVpn?=
 =?utf-8?B?dmMrUVZLd3ZTaDRpeHBFbmJPNEhDTXlUL3hTZndMSGNsNGJMcGpXNzdNckY2?=
 =?utf-8?B?MmlGSVpUVUJLTWthb0JYNWp6TEpiS2lRcUJER0RQZXByWWRNVXlhbzNyOGYx?=
 =?utf-8?B?VnVkSlB3Q1VlUFRRVUR5TElESllRVmdsblQwM1NnUWJGK0paand2MnNpZWxZ?=
 =?utf-8?B?SmtZbVJxOHdvNnJOUEh6bU1WN2g0YWdhM3BpVjMxdnA1dW5OeTFsUnBIaXRQ?=
 =?utf-8?B?NHpVRkwvcFM1MXVkTjQrakZ5d0pubjdKWW1IZ3ozSFRsQUhRSzVBMWx5WnpS?=
 =?utf-8?B?eTdqUkhQSXFJb1VoZi9acDk1N1JZblNkUUo0cWlWMFV3Z1dRRldJMlVESjhu?=
 =?utf-8?B?U0VuUFJjZFRSRDNQZ0VLMUJncFVFRXRNU09waytpTXpYUmdPOVdNZVpWWmpo?=
 =?utf-8?B?cEd6UUNIaE5EekxWR3d1dXJnOHFwTW5HZWg0OFp1VlM4L0tkVnBOTEpTOHZ3?=
 =?utf-8?B?L3dWZlNSb2loNFZMSDlkV1V0UWtwRSt6TU1kUEFWazFwZVI1bFFnWGZlU0Mx?=
 =?utf-8?B?YU00bVREc2NBbm0zWUZtOFp3aWQ1dkdzYnFUeWU1dE1QNVNZY2ZEU1p6ZUJ5?=
 =?utf-8?B?dE4xVFI0Mk0xYmFoWHY0RHM0OE9JeG9iNGJ4MDUwNTBPQVNzU3ZSOUcwS3Yy?=
 =?utf-8?B?ZUxrYXNTcG90ZWRkZ1hxY09mdUZwR0doNTRZcVZ3Z2dMb2ZtWlVoSVVueGlG?=
 =?utf-8?B?RVZQYUo5TmIrcC85a3VSUE43WXZSYVJyR2dxNzZ4STMyUVEyNXE5SHhrL3dh?=
 =?utf-8?B?czdWMy9CY01CeWkrclNPWmFaTGQ2N1Y0Qjk2ZWtLTk1ad2ZabDl4OVUvRGhQ?=
 =?utf-8?B?V3FiWG5qUGhSMENoNnJxWlZSSExBSzdnMittVXpGSDMvNnA4SWI2VXhjbWZt?=
 =?utf-8?B?blNhVVdnNk1NSUhSUEZqVEtHWm8wMDVTMjJacnlMMTlTd2wyRmsyL1VEbHdz?=
 =?utf-8?B?cytBdkxRNXNOak5RMElFQndTZGRXUVYzWGRLWjdnMVFKWVVuOXNnWFQ4N1Ex?=
 =?utf-8?B?Vi9DZzRrV2w2Y3RCNXRaT1ZjWDR4cFVxNjRvRG14cnp0cHlKOFY4VlBTNVpX?=
 =?utf-8?B?emdRY2pxU2ZWQVZ5b0F3SVlDVC9USGNodnZ3L3d0eFhuTXFqU2RnSUJRQ0NL?=
 =?utf-8?B?UDJETFd4a2UrTUtEdTJBT0pObkZEUUZOQlFYWGFSMlIrQXV3TTJ1ZmdyZFJ2?=
 =?utf-8?B?Y2ZkTGp3dmc3WTRPQ005YkRzL1UrclFWMjZ3dnQ0TmhlbUVhZTgyZy9oTjli?=
 =?utf-8?B?dHpoYTVxUXZUaEc2ak55K3BzOCtaRDU3SWxNaEg5OVBkUTFqdkFJTEphaFow?=
 =?utf-8?B?M2gxY0ppNGZmYU9SaEUzbFRrY1pxVTkraFh2T2JjOW9xaUZnbzRpTTd6TDNj?=
 =?utf-8?B?anExMldnaS9kc0x6aHEvbVl5bHZaVTVmUkVrQ1oxTWU5VExKS0l0VEMrTXdC?=
 =?utf-8?B?MGJkZjk4empMZXZNUXIwakt5WldnTGZsOXQ3U3NhTGlYYzNHSVNrOTZMTjAv?=
 =?utf-8?B?bGVGOHB2enFmM2pDckJpYk5IV1JGRDhGRW1PZC9kSVRFSDdxdmk4V2RXdm80?=
 =?utf-8?Q?Sl5I5fMMIWeezwcIDTO3JBvOEzQAUqcZZk1FQec+koHa?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6a46d6da-d663-48d0-72ea-08da90eca01e
X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1947.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Sep 2022 16:18:40.0940
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: E1t7haCHM3M/HF/X5GEkzvC2s/BcGUiovV9pEYE2OV7SKkHWYmQsKcqL3bdz0J6pxxMYtUCDniVwrMt4WSTgRQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5762
X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2022-09-07 02:37, Christian König wrote:
> Am 06.09.22 um 21:55 schrieb Andrey Grodzovsky:
>>
>> On 2022-09-06 02:34, Christian König wrote:
>>> Am 05.09.22 um 18:34 schrieb Arvind Yadav:
>>>> Here's enabling software signaling for finished fence.
>>>>
>>>> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
>>>> ---
>>>>
>>>> Changes in v1 :
>>>> 1- Addressing Christian's comment to remove CONFIG_DEBUG_FS check from
>>>> this patch.
>>>> 2- The version of this patch is also changed and previously
>>>> it was [PATCH 2/4]
>>>>
>>>> ---
>>>>   drivers/gpu/drm/scheduler/sched_main.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
>>>> b/drivers/gpu/drm/scheduler/sched_main.c
>>>> index e0ab14e0fb6b..fe72de0e2911 100644
>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>>> @@ -962,6 +962,8 @@ static int drm_sched_main(void *param)
>>>>               /* Drop for original kref_init of the fence */
>>>>               dma_fence_put(fence);
>>>>   + dma_fence_enable_sw_signaling(&s_fence->finished);
>>>
>>> Ok, this makes it a lot clearer. Previously I though that we have 
>>> some bug in dma_fence_add_callback().
>>>
>>> This is essentially the wrong place to call this, the finished fence 
>>> should be enabled by the caller and not here.
>>>
>>> There is also another problem in dma_fence_enable_sw_signaling(), it 
>>> returns early when the fence is already signaled:
>>>
>>>         if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>>                 return;
>>>
>>> Please remove that one first.
>>
>>
>> Why we even need this explicit call if dma_fence_add_callback calls 
>> __dma_fence_enable_signaling anyway ?
>
> Two different fence objects.
>
> The dma_fence_add_callback() is done on the hw fence we get in return 
> of submitting the job.
>
> The dma_fence_enable_sw_signaling() here is done on the finished fence 
> we use to signal the completion externally.
>
> Key point is the finished fence should be used by the frontend drivers 
> which uses the scheduler and not by the scheduler itself.
>
> Christian.


Oh, so we need to explicitly call this because dma_fence_add_callback is 
not always used for finished fence right ?

Yea, then it makes sense that the client needs to manage this since each 
one has his own logic what to do with it.

Andrey


>
>>
>> Andrey
>>
>>
>>>
>>> Thanks,
>>> Christian.
>>>
>>>
>>>> +
>>>>               r = dma_fence_add_callback(fence, &sched_job->cb,
>>>>                              drm_sched_job_done_cb);
>>>>               if (r == -ENOENT)
>>>
>