Message-ID: <97416541-5051-932d-c364-a7c602d54aae@amd.com>
Date:   Mon, 20 Mar 2023 22:55:54 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v2 23/23] drm/sched: Add (optional) fence signaling
 annotation
Content-Language: en-CA
To:     Rob Clark <robdclark@gmail.com>, dri-devel@lists.freedesktop.org
Cc:     Daniel Vetter <daniel@ffwll.ch>, freedreno@lists.freedesktop.org,
        linux-arm-msm@vger.kernel.org, Rob Clark <robdclark@chromium.org>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>,
        open list <linux-kernel@vger.kernel.org>
References: <20230320144356.803762-1-robdclark@gmail.com>
 <20230320144356.803762-24-robdclark@gmail.com>
From:   Luben Tuikov <luben.tuikov@amd.com>
In-Reply-To: <20230320144356.803762-24-robdclark@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dHlwL1dPNWRsSUVHNVlod3hhUXNZS3ZXVFpIN3VmVTJjVTBrbXA3aG5xMjls?=
 =?utf-8?B?MEhVKzVCcHg3amI4bHFCRDY0bS9YMXplL0x2RWpmdTZvRWZlbjdQbk9yd2Jh?=
 =?utf-8?B?MUVLNWNFL1VUQUxXdnJqbnY3ZEdHYW5LRmhkRHhuek9aQVNZcG00bTBQcjdE?=
 =?utf-8?B?YWdxbUp2aUdMdVNCbk5DVGdFcXRzZTcyVE9wTDdZcVdXTnNCemlrU3RleFdZ?=
 =?utf-8?B?alRKVXhHamtEd0ZlMWtuaWFSeEpIWDkvZm1PNk81YXRqOS8zVDBUY1VGMDJO?=
 =?utf-8?B?VWw5YWpFL1N3NU1kRkJabzJjZjQ1RmR5TWorSXkwaGZnWUl0RXZ2OVpMemFx?=
 =?utf-8?B?VFpvVmdFekQ0aS9XVWdpbVBvNDNCaHgxaG5KNzl5RTZXVHZlakZ1T1dtUG5y?=
 =?utf-8?B?U0Q5V3Q0ME90ZjFxMHEyekpDVFF1NjJFR0VCRmE4a1BHOHZTWVFYUjZtQUFx?=
 =?utf-8?B?UU14L1dWTFlBL2tZbXM5bHlsY0lIR1hQWVZqUWIxUW9OYitDL2ZNVHQwcXBT?=
 =?utf-8?B?aWowdHJKUGlDRUJRdG9jbE50M2piOWFkMlUyUlZ5Q3hIcGNuUU5Ha1VsNE90?=
 =?utf-8?B?U0ZGamNhSVNFeDRTQWx0dFhGNnJaNG9nSk12OEpmdDQ2eUVLSUxqcm5FL1RG?=
 =?utf-8?B?NXM5VndHbnZqWEhtOC9va2srSkFjTDJBb3dwNUxZRUh0aE1QYlZQRXZqNUM5?=
 =?utf-8?B?R1BjOGFVOEFsbiszUEJuczBGZExZdVRsTjYzOUovbnZ2SElNOFV4SVNobUxx?=
 =?utf-8?B?djgraTR0Y3ZpL1lDN0p0NVErekgxMHJFbmtiaUhJdlNXUDBjSFBrcjVtbFIv?=
 =?utf-8?B?SUZLcVRKNWgwR25xaThrMTFtbFl4TnQ2Z2tpTU5Fa1ZWaTZNS05KMTdNeEE1?=
 =?utf-8?B?U0RGekZ2c3lPblZ5MkF3aUdWbys5dklhN1ltenV4RzViSE93N1NWRUQ3R2xS?=
 =?utf-8?B?ZVc2eWdzZkd2TjdnU0Fha2JXU0hKVlpiVXlzQngrcmNjT2F6SzRlVk1HWCtl?=
 =?utf-8?B?MGlhVUhXVVU3d3pUd2ljSUdNUXNyM1dKbmNpWXhyTXdJMkhGNGs1Y2RVL1FO?=
 =?utf-8?B?Y2p3WW5UdEtPYTljdWRmTUpkQ2JBRkpzd1d2WnBxNW44UVQ0cW12VVl3aCtq?=
 =?utf-8?B?akprSEQ5b2d4MVlNUEFXV0RnazZjeDdTZFpvRFBOb09rU0pTRmRvUTRWdTBP?=
 =?utf-8?B?Z1g1VW9PZEl4WnpJekQ5d29DdDB3aWx0Z21kREZWNzZEZ210cDM5OWVIUTlz?=
 =?utf-8?B?QkhPc080VzIySHdJVGNVOHJNalYvaXI0ZVhKTW5mMzZ5S1VDWFlXR1BYWFNy?=
 =?utf-8?B?U3h4WUI3eFZ1NWxwa20vWkpwTDBDMWw4Nml3STJHUEtRZmg1YjBmREVRR3Z6?=
 =?utf-8?B?Sm5xL01NbnBhd1A0RkxuYnZMZTFqeERta0RIQ3BKWHh1T2VrdHFNSEFTemxm?=
 =?utf-8?B?L0hEQkt3TFN3TTdQWjE5bndOV3NtVkV3Q0ljQlFmSlkzc3pqdzJtMUsrNkhD?=
 =?utf-8?B?OXlPRlN5V2pCZFpVV1VCWDR5a2ZsUFV3bHRIeHJJM3NCOURNdXE0bXF5bjdO?=
 =?utf-8?B?d1lDVXZEeWVZdExtQzBVRjlZNlA1cnhZd3VXS0VneU14ejU3aGp2UGczL0lu?=
 =?utf-8?B?cmhqa3NXZnRlQlo5QU8zSm9Oc1VmWE5hdHZkRkl3bWpqSmhJdjJhZDMydDhK?=
 =?utf-8?B?TDhmc3JyQnNlK3pJVjNrL0d0SENSNVljR052cm5oSEtKbkNWNVZOOXNDQkZB?=
 =?utf-8?B?dnZ4LzBvRUJFSkZ1c2gyUzRpNnNhR0VuZitVYjRZNk5talloRk1wQTN6bnhG?=
 =?utf-8?B?TkVLdkwrbTRYUW5KRjluaWYwSmJYZkJNS2MrakdyZWh2Ny8zOE5aNDc4VW5X?=
 =?utf-8?B?K0Eyc2JHV09DM1JLOG5sUEdRKzVpb05kT3AxL2NwVFJweVkzNkd1S1NDTzJa?=
 =?utf-8?B?djFlRXA3SW81OGVZWkhKaTZkQXZqY1FVN0hDZjh2VGNqc3RHWGgwUnY5M3RG?=
 =?utf-8?B?elVqSTdQaTZIOGdBQUdTbW8yS21IYVpKdVV2TDN3eGFKU3cwNGtRSWNWV0hH?=
 =?utf-8?B?Mnllc2VMbUhjcG9JaDVvbkplR3FQNUtBNG1MZnFkczloMm93aDNjbGM1NktP?=
 =?utf-8?Q?fTRJAdpTcnACZOsTBG03cKJ4a?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e90e4582-7dfb-4b2c-9ca7-08db29b7cc19
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3370.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Mar 2023 02:55:58.5072
 (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: Zd9Y+6ZwY3jEgb7rEu8K0MVXfIyrVrFTJROdCVxbxt7MMaftoj52TmJg3uMg2jPH
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7217
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

This is good. Hopefully it helps us catch lockdep bugs.

Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>

Regards,
Luben

On 2023-03-20 10:43, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Based on
> https://lore.kernel.org/dri-devel/20200604081224.863494-10-daniel.vetter@ffwll.ch/
> but made to be optional.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/msm_ringbuffer.c   | 1 +
>  drivers/gpu/drm/scheduler/sched_main.c | 9 +++++++++
>  include/drm/gpu_scheduler.h            | 2 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c
> index b60199184409..7e42baf16cd0 100644
> --- a/drivers/gpu/drm/msm/msm_ringbuffer.c
> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
> @@ -93,6 +93,7 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu *gpu, int id,
>  	 /* currently managing hangcheck ourselves: */
>  	sched_timeout = MAX_SCHEDULE_TIMEOUT;
>  
> +	ring->sched.fence_signaling = true;
>  	ret = drm_sched_init(&ring->sched, &msm_sched_ops,
>  			num_hw_submissions, 0, sched_timeout,
>  			NULL, NULL, to_msm_bo(ring->bo)->name, gpu->dev->dev);
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 4e6ad6e122bc..c2ee44d6224b 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -978,10 +978,15 @@ static bool drm_sched_blocked(struct drm_gpu_scheduler *sched)
>  static int drm_sched_main(void *param)
>  {
>  	struct drm_gpu_scheduler *sched = (struct drm_gpu_scheduler *)param;
> +	const bool fence_signaling = sched->fence_signaling;
> +	bool fence_cookie;
>  	int r;
>  
>  	sched_set_fifo_low(current);
>  
> +	if (fence_signaling)
> +		fence_cookie = dma_fence_begin_signalling();
> +
>  	while (!kthread_should_stop()) {
>  		struct drm_sched_entity *entity = NULL;
>  		struct drm_sched_fence *s_fence;
> @@ -1039,6 +1044,10 @@ static int drm_sched_main(void *param)
>  
>  		wake_up(&sched->job_scheduled);
>  	}
> +
> +	if (fence_signaling)
> +		dma_fence_end_signalling(fence_cookie);
> +
>  	return 0;
>  }
>  
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 9db9e5e504ee..8f23ea522e22 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -483,6 +483,7 @@ struct drm_sched_backend_ops {
>   * @ready: marks if the underlying HW is ready to work
>   * @free_guilty: A hit to time out handler to free the guilty job.
>   * @dev: system &struct device
> + * @fence_signaling: Opt in to fence signaling annotations
>   *
>   * One scheduler is implemented for each hardware ring.
>   */
> @@ -507,6 +508,7 @@ struct drm_gpu_scheduler {
>  	bool				ready;
>  	bool				free_guilty;
>  	struct device			*dev;
> +	bool 				fence_signaling;
>  };
>  
>  int drm_sched_init(struct drm_gpu_scheduler *sched,