Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <915537e2-ac5b-ab0e-3697-2b16a9ec8f91@amd.com>
Date:   Thu, 17 Mar 2022 17:44:57 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 2/3] drm/msm/gpu: Park scheduler threads for system
 suspend
Content-Language: en-US
To:     Rob Clark <robdclark@gmail.com>
Cc:     Andrey Grodzovsky <andrey.grodzovsky@amd.com>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        freedreno <freedreno@lists.freedesktop.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Rob Clark <robdclark@chromium.org>,
        Sean Paul <sean@poorly.run>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>,
        David Airlie <airlied@linux.ie>,
        Akhil P Oommen <quic_akhilpo@quicinc.com>,
        Jonathan Marek <jonathan@marek.ca>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Vladimir Lypak <vladimir.lypak@gmail.com>,
        open list <linux-kernel@vger.kernel.org>
References: <20220310234611.424743-1-robdclark@gmail.com>
 <20220310234611.424743-3-robdclark@gmail.com>
 <YjMGac4Hnjmg1wE8@phenom.ffwll.local>
 <3945551d-47d2-1974-f637-1dbc61e14702@amd.com>
 <CAF6AEGv36V8bLoDn5O1SW3iTUtzd3O1XeuT5gJxyLMxd1E-o3Q@mail.gmail.com>
 <865abcff-9f52-dca4-df38-b11189c739ff@amd.com>
 <CAF6AEGuoBeYoMTR6-KM9xGZ05XSSnSJWMDciawczi7qtiLN9Vw@mail.gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <CAF6AEGuoBeYoMTR6-KM9xGZ05XSSnSJWMDciawczi7qtiLN9Vw@mail.gmail.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?SUZTMzQ4NDhtRVREUmNSVmVCUVVza2o5UWVVeW1vQmhXdE9jQnpNV05wYVph?=
 =?utf-8?B?ejQvNSs3RkJtR0NmUitQb0ZYWitjQ0FFNlFQVFhOemVCQ1RzVm01QUJxWnlm?=
 =?utf-8?B?UkhsYy92RjNwY29DNmRjR2RpOHdkalhyWCsxeGlabnJ3dEZxWlV4M0dZNGhh?=
 =?utf-8?B?Z29ITTVwYmtLNkN3NE1QL2J6OC8vR09XN0I2cmlPOXZlVFp5SzUzeTZsZkhm?=
 =?utf-8?B?SGNQcmQ0QW5Bd0ZxMHo0L2xUeHJRRnJmTzRWTEFOUHE3a2wrSUZTdHVxL3Ax?=
 =?utf-8?B?OFFtWTd1MEJ2OFIvNDdtb3N5MkVRNEk5UXljSXJIQU1VV3dCMVF6THZXMGt2?=
 =?utf-8?B?RU54VHBYUzZxNUdEZkJqNkk2UkZOdjR2K01Na0FJQW9hWUpzVThLZE5wSCtW?=
 =?utf-8?B?VVNiSXJNS2V4ZklaNHQ0MHZ6RlhJZDZQS1U2Z09BT3BmTnZGaC9kNyt4d3k2?=
 =?utf-8?B?NFFHTEhKRnlEV3hZb0V4dWRpNCs3djVYcmlGSzNubm5QN0hJK01Lc3RZZkFs?=
 =?utf-8?B?T0VHYU9YOWZXL3FFT3hYZ1RiK0dSVGRXclVZcWc0RWV5eG1RblkyeTJCWVoy?=
 =?utf-8?B?bmxZc3RjWnlhUFF0SlQ3bms2YVpPcVVRUkRJcFZuMHZ3VklESjdJV3F5U1hk?=
 =?utf-8?B?em1pWmNnaHFINnc3anBzL3p3YU1CU2IrMURpTklMSzRZckt4SC94SXpzdUcr?=
 =?utf-8?B?ZGlvRlorakQrbGw1cUpOSWZhMWp6RmRHNXpLbFhEQUlWVXVkWVJFdjZiSFhT?=
 =?utf-8?B?NlV5aXdrVjRLWWNHOTdETTdKWjAyMVFiUnc2R0J1aEJCZUpzTFBqNTN2Wklk?=
 =?utf-8?B?YUxLRGtRRlV0S1FDeVVlaTB6WWpiNDJYeFJOVld5V0U4WXZuM2ZRR1I1cTA2?=
 =?utf-8?B?TjNCZXNrVHljZDRHM1VOUDNsVnMzVUFINzhwT0VPQUNVYXJhajllMmhRZlRq?=
 =?utf-8?B?UzZlR2xmbjVVWlJoenhFNERRcUlVSVZIZkY3ajBxYWpIZ1o5UXZCaUxMaDhZ?=
 =?utf-8?B?ZlorMC8veVNkUHlIamNYSVJDM095eVFKODhMb3dNZTNZUnJDbE9PUnJmb0Z4?=
 =?utf-8?B?aldzSVJwYUFZbDNOYStoV3NrTjRyRjRMc1lKVjV1blZQVWdmOVhLQ040cVpz?=
 =?utf-8?B?NXB1T0Z1T215aDFJeUo5VnZIRjZvaW1CNldId0hlY2JwR1NZY0RjL05wODJu?=
 =?utf-8?B?ZkNaaG1pcVB3aWxVV200cVpQb2p4bGlLaVVmYU1oTGg5VTF1Z1BYa25NbEpw?=
 =?utf-8?B?OFY4OTMvd05mVDdXWHlaUDZ3TjZ2OUsrTG41L1pQTEhoZjdaLzZGTFlhUGVt?=
 =?utf-8?B?eC9PSzVsazhraXEvaGFvRnp4bGxNSXIxZXQxUDlRZzhxQldSVWdaTjE1bW96?=
 =?utf-8?B?N09wY3lpS3VqVEMxOWdKQW11OHdLVVFKMWNpT0hSRFZic3BUdUhUa1JvUlRD?=
 =?utf-8?B?alNDMkZHLzBvRmFqUjJJWDhMN1FIeWI2djVMVkxRZERMeHY2RllVbnlMTU40?=
 =?utf-8?B?OEhYSjlhLzBiMTkvMDJIV3kzbnB1Z3ZpdkpkMTRPR3hCbHF3VU1zVTZ2c3h1?=
 =?utf-8?B?SVNtRmxGZEIyVXdsTUwwUGZNSGsyUXoyUC9OZytETlZaWFNlK2hNa1ZMRkVh?=
 =?utf-8?B?d214enM0QUNPSWZtbEIxOSsyUkRoUVZ4MWlJRlBhMVBoSXM1cm8wOWc0NCs1?=
 =?utf-8?B?b2dMN08zWUlxM0FRVEtTSjJLTjRpSWVkLy9sekQvZTRlY1pScTFjMzdFRGRD?=
 =?utf-8?B?L2dXSXMxOVpsc0U2dXdYdUwyeGFOVDNBcUVrRGtKSTBtVHp1MzRncEhZL1E5?=
 =?utf-8?B?Skx0dU5tazdTWHh6VGNYR25XanJnbHZOL3dMeXhQVjhhUXBNSWwvNGk0dmVi?=
 =?utf-8?B?MC9uSkh4WmVSc1JjNlJJeHhsVzE0WlVBWll2V3FYN3RvVmFPMzZ3YThaREps?=
 =?utf-8?B?UWJGODNOTGNtdTJlRzZ3TUkyYXh4WDBTY2Nlc2g0R0Y3VXo1Nit2MGxDVnRw?=
 =?utf-8?B?MlRjRXZEeWJ6MlVCeXNiQTJRYnd3WU9ZeVV3aUlneHl4UitKMFVGdkJzVlU4?=
 =?utf-8?Q?Fxi9c2?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 633c378c-9145-43b9-88a4-08da08357d23
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Mar 2022 16:45:05.3834
 (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: +iac8RW+dFk+RDiRpBz4MkAKSmv+OIHGywVruxoH0W00SIV1kl9/CaSi9mNdEXg/
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5969
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=no 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

Am 17.03.22 um 17:18 schrieb Rob Clark:
> On Thu, Mar 17, 2022 at 9:04 AM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 17.03.22 um 16:10 schrieb Rob Clark:
>>> [SNIP]
>>> userspace frozen != kthread frozen .. that is what this patch is
>>> trying to address, so we aren't racing between shutting down the hw
>>> and the scheduler shoveling more jobs at us.
>> Well exactly that's the problem. The scheduler is supposed to shoveling
>> more jobs at us until it is empty.
>>
>> Thinking more about it we will then keep some dma_fence instance
>> unsignaled and that is and extremely bad idea since it can lead to
>> deadlocks during suspend.
> Hmm, perhaps that is true if you need to migrate things out of vram?
> It is at least not a problem when vram is not involved.

No, it's much wider than that.

See what can happen is that the memory management shrinkers want to wait 
for a dma_fence during suspend.

And if you stop the scheduler they will just wait forever.

What you need to do instead is to drain the scheduler, e.g. call 
drm_sched_entity_flush() with a proper timeout for each entity you have 
created.

Regards,
Christian.

>
>> So this patch here is an absolute clear NAK from my side. If amdgpu is
>> doing something similar that is a severe bug and needs to be addressed
>> somehow.
> I think amdgpu's use of kthread_park is not related to suspend, but
> didn't look too closely.
>
> And perhaps the solution for this problem is more complex in the case
> of amdgpu, I'm not super familiar with the constraints there.  But I
> think it is a fine solution for integrated GPUs.
>
> BR,
> -R
>
>> Regards,
>> Christian.
>>
>>> BR,
>>> -R
>>>