Received-SPF: pass (google.com: domain of linux-kernel+bounces-27483-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <5012eab1-3aaa-4559-b410-0972e0a18b7c@amd.com>
Date: Tue, 16 Jan 2024 15:52:17 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/1] drm/ttm: allocate dummy_read_page without DMA32 on
 fail
Content-Language: en-US
To: Yangyu Chen <cyy@cyyself.name>, dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, Huang Rui <ray.huang@amd.com>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>,
 David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
 Jiuyang Liu <liu@jiuyang.me>, Yichuan Gao <i@gycis.me>,
 Icenowy Zheng <uwu@icenowy.me>
References: <tencent_40DF99E09A3681E339EE570C430878232106@qq.com>
 <tencent_7D66A3085F83608576A8E309EE714C5CC806@qq.com>
From: =?UTF-8?Q?Christian_K=C3=B6nig?= <christian.koenig@amd.com>
In-Reply-To: <tencent_7D66A3085F83608576A8E309EE714C5CC806@qq.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?V0NlNVJXc29nZlFyNGNvRUwwYnhrVVFHQUNpSDdWQmJ5Um9ab3dZb2FKcnkx?=
 =?utf-8?B?ZDRXaVZxSk5Ub0FERHNWUEt1OVpQMkRkN09NN21nRUlQSlI4UzVhZkY2SEtT?=
 =?utf-8?B?V1BRamxZOEFxUmFnWVFCVTBhYkVJMUpNcGZuak5XTFNlQ3JFazRaMUIrVnJi?=
 =?utf-8?B?QmdSVXNPZm5PQ3pWOXFzbmJPdHB6eXl1NW9Hc0pFc3NhRTFWSFZCZE82TzF2?=
 =?utf-8?B?TnVFNDhZYkJiUys1OHczanZOcTJjS0RjTDBmY3phVVNZRC96VFZYMXdHUGVq?=
 =?utf-8?B?b0pwNyt1bXdGVVNtb21PSnFjbFlkNllKcURXNnNpZ05Ea2p3YklmQmpPN1VY?=
 =?utf-8?B?R2hYazl1ZWJHbTl0d091R1dLNW92alVTcmVjK3pQRzNLRlZHbXFYdEs4VHVp?=
 =?utf-8?B?L2xYOXJBTE1iU0llSzdZVjF3d3I5eSt5VUJxbUJOOTJ6WHkxVFZiTldkWXZt?=
 =?utf-8?B?VEtLNEpFRzRTTVF0ajk1OTA3VDJ0c0hVZ2pjOXJUV2ozNlRmSjFZdmpDbkNo?=
 =?utf-8?B?ZUt6OEhHZjJwRVNDWGNnZFRFSzNNNDZTN2cvYUFibnRWRUlHNXQyT0s4RWdo?=
 =?utf-8?B?OTBTMGlrdUlVSVBxWWVqdWI5cUxGZVlwZ2U5ZGpnTHJwa2RDb0ZkS3JzSUNE?=
 =?utf-8?B?b0F2bGFZYnpOeEtuWTVZZEhFQ1BIclhsazdXNlF5M2NWdW5LQ2d5UXVZRm9t?=
 =?utf-8?B?WUhlQWdnTVEycm5RYkZDRlNraS9QblVJZVBTT3Rra1VjYlV6UTdRemVNMWxK?=
 =?utf-8?B?S25oZ1hrOXl1Mm9qZUNmVGtNd1V6WU52TW5MMUEzQXZFQTN5TWwvd0lzRm11?=
 =?utf-8?B?V016TG5tR05QSUlzSG5sVHB1L1dxOUduSWJBZjhlNlFxY0xVTnh5WHp1eFlN?=
 =?utf-8?B?MkdEWHVTQTM1Z1NlbEZIZTE3VHp3YjZuNDZQRUlyYzh0ekJqMTF6VENubjlj?=
 =?utf-8?B?aUNKSDNjUlNwOG9sbWR4YXhKZ1dMdW9sT3RVZG1ET2xCYlVpcjhxYWpKbzdk?=
 =?utf-8?B?MDZaR0M5bmFFOUJsWWZXNDBQeHhDQVpOUWtFY0JsUTNvcTNic3lXeFpIanh0?=
 =?utf-8?B?RzdZd0JoYWZFZ01xOXNPcEtWeFJRMWN2TE93dERZRTNpeVVpVjZralBMTDI0?=
 =?utf-8?B?UjlJV3RDZWUrY3p5K1daRlNBS0EvWWtqRXRFME9XVVRGQlZoQm9yRjhDUThU?=
 =?utf-8?B?SkxQdXNrZjVBSUxRNmk2cGo4dmo5VEgxRDhOT244UmpBUG9YdDE2RHBsRmZM?=
 =?utf-8?B?TUdHUzNJdGxiN3lpaURQQllNSEZROFNzMmlSVlV5L2IwVEpBb1F4ODBOc1ZV?=
 =?utf-8?B?bGFaZnl1Nld5N1Jpcldobklkdy9pVUlPRTdweFBPSE85emZJQUZHMFkzZmlH?=
 =?utf-8?B?U1BTbW1aZS9Xa1Rkb0IzR1kwVm5sb25UZDBndEkwcjZvMEZQa2dmUXdrcXJv?=
 =?utf-8?B?bk0vNjBSK1ZjNFU0ZlZ2SU9XUVlLS0JUblhvc2R2ZUJ6MlgzL0hxcGZzcURV?=
 =?utf-8?B?NDBZYkorYnQzdWZVQTlqQUNIaFhoMUFXa0gzMFRZSHQzMGVmK2hFczB6bCt6?=
 =?utf-8?B?dGJ6MnVTMHlWaXVaZ2d1MjJsNjFIRDlsRWs5TlRBdnAyRUNoNnlFNUptQnFX?=
 =?utf-8?B?NE1VbnFreGdQbldTSGNFWHRueEJSNWszd052YlBUUWMyQVhhU21xbVNMdjZU?=
 =?utf-8?B?L3ZBMUwvQnc4anRzZVVTcmNGUzhyUUhWb2NLUXZ0WDRvVlpLRXJ1cEJHS0tk?=
 =?utf-8?B?ODMyaGxqR0l2ZDNmS1l4Z1YyWGltOWJDT2xZWm40MmcxWmViNm95aTA5V0VM?=
 =?utf-8?B?UzlYZyt5SEdOcHF3T1oxdForZlhNRFQ3Rm13VTlQdmxYUWFxLzI4NEI3ZFNF?=
 =?utf-8?B?L3BxZkw5cU1vUUFMKzQyS2x3eW42bm9xV2U3L01EclNDeEZCM0ttOXZPNXhP?=
 =?utf-8?B?ZmsvcnNHWWI1YnM4UVdIZEFBYVB3TytSb3JkeWhQTDV5NVZYREh0NThwMnVq?=
 =?utf-8?B?ZHZrdTdJOVNJcysxQkk2UmxoNUtVQ0cwOUNrRzZ2b2hjS1JqRWd5R1hXbEk2?=
 =?utf-8?B?R0NSckl0NzJveDNaZ1VrUW5Kb1hJNnZLUmlac285VTZKajd0cEZ4SzVGYmRU?=
 =?utf-8?Q?AAPJlHYC460E4EXZQDLhAi3K2?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8148f783-b8bd-4c45-ada2-08dc16a2bf98
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB5685.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jan 2024 14:52:23.6302
 (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: /PiESttYz2O//U9Opvmnf5AEWak0B/xjeAixPj1ACTxdM/eN7QhyW2EyrKwG5P9q
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4903

Am 16.01.24 um 14:02 schrieb Yangyu Chen:
> Some platforms may not have any memory in ZONE_DMA32 and use IOMMU to allow
> 32-bit-DMA-only device to work. Forcing GFP_DMA32 on dummy_read_page will
> fail on such platforms. Retry after fail will get this works on such
> platforms.
>
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> ---
>   drivers/gpu/drm/ttm/ttm_device.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index d48b39132b32..a07d9ea919b6 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -98,8 +98,13 @@ static int ttm_global_init(void)
>   	glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32);

Please add  __GFP_NOWARN here to avoid the backtrace and warning on 
allocation failure.

>   
>   	if (unlikely(glob->dummy_read_page == NULL)) {
> -		ret = -ENOMEM;
> -		goto out;
> +		/* Retry without GFP_DMA32 */

Well that is obvious, you need to describe why you retry without GFP_DMA32.

Something like /* Retry without GFP_DMA32 for platforms without 
ZONE_DMA32 */, and probably better placed above the "if".

> +		glob->dummy_read_page = alloc_page(__GFP_ZERO);
> +		if (unlikely(glob->dummy_read_page == NULL)) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +		pr_warn("Failed to allocate dummy_read_page with GFP_DMA32, some old graphics card only has 32bit DMA may not work properly.\n");

Well maybe make that a bit shorter, something like "Using GFP_DMA32 
fallback for the dummy page".

Regards,
Christian.

>   	}
>   
>   	INIT_LIST_HEAD(&glob->device_list);