From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D0687C35257 for ; Fri, 2 Oct 2020 17:56:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 80403206DD for ; Fri, 2 Oct 2020 17:56:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="N09JTUu/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388362AbgJBR4G (ORCPT ); Fri, 2 Oct 2020 13:56:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58576 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726096AbgJBR4G (ORCPT ); Fri, 2 Oct 2020 13:56:06 -0400 Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 133D1C0613D0; Fri, 2 Oct 2020 10:56:06 -0700 (PDT) Received: by mail-wr1-x443.google.com with SMTP id c18so2710449wrm.9; Fri, 02 Oct 2020 10:56:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=sDykvbK2l7LUktA/NiMrsHi3eOeHsUBTt6WtvcVHsDY=; b=N09JTUu/Nt/0qMN2ji4tbfluL0VPw+AisuR7aO/LSn6OudL9k7y+z5Y/nB/RjjcCXG xwmY3Ap7gvJRtWu3GpC4ma9lEaakLD9ibRJBaT7WBPhKtMfLuaLzLZyCdPCmjBQ1Udlq WbYcPkJt206ljql2riZk5u/u3cPmGMur/9jqHbST9PwMjpOKyBC1Q8edydLHHPFo2DGA Z8cw2OuGUjGpYFjiCLmMzSPDLQTUGJ7aekyUPYPQiiSHqXRtwDaKRQLG+k/s6xWkS1IQ eWp7NsTFey2DvsYd2Y+eATr+usG3hdjY938vQxlgBoe2ez98J517TkLFuSxok/jPT3qC FBGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=sDykvbK2l7LUktA/NiMrsHi3eOeHsUBTt6WtvcVHsDY=; b=GjP1+iHLHgd0VnFLGuFkbkqO1GEpKSnOZ7N8GjAje/4nzAolRMVxVzBTgLSDRjkJ03 +kX9Q11/U5XbAB1RxMbyx8WDs6AOX9Exo4KTRY1ntA5fySnWz3yjBICXqEGcRz7bYTCM ATQqY+XI/2l9OK6SAqNuRx/Gd9Fl2/4mW+0pOcyAalzAuk2prE4mP0jA9br3fyr54STp Ombh5GnlbnVaoawkSV/7FAWm/Wp6n7oU6JQi+/1RLQcTnfIceYi/QPX4F1msYNswm/Dz mXI/CZtBnzf0CzqnuG0w7sv6leDzonXM8TNi6Yjdk5zWR+fVBnIlLUJsY1plptejDL+8 l3dg== X-Gm-Message-State: AOAM533nLOD0NY8gNzZs9LzTiF4YFR1nEMdPrlOoH1B0AiARGEBYtWWf Y4HwUce0cBMFy/RPLpw3sGMVbwG7pugzfOMYgG4= X-Google-Smtp-Source: ABdhPJyqabUahTC2c5mExHpmCMP/sNSLrS9tVQAZfc0UEeGsTQko60PRoh5s/bXkHSz9NQFWOqyqC7RU2RS3SuTsaCA= X-Received: by 2002:a5d:4a0c:: with SMTP id m12mr4380696wrq.83.1601661364614; Fri, 02 Oct 2020 10:56:04 -0700 (PDT) MIME-Version: 1.0 References: <20200930211723.3028059-1-robdclark@gmail.com> <20201002105256.GA6112@intel.com> <20201002110544.GB6112@intel.com> In-Reply-To: <20201002110544.GB6112@intel.com> From: Rob Clark Date: Fri, 2 Oct 2020 10:55:52 -0700 Message-ID: Subject: Re: [PATCH v2 0/3] drm: commit_work scheduling To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= Cc: Daniel Vetter , Rob Clark , linux-arm-msm , open list , Tim Murray , dri-devel , Tejun Heo , Qais Yousef Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 2, 2020 at 4:05 AM Ville Syrj=C3=A4l=C3=A4 wrote: > > On Fri, Oct 02, 2020 at 01:52:56PM +0300, Ville Syrj=C3=A4l=C3=A4 wrote: > > On Thu, Oct 01, 2020 at 05:25:55PM +0200, Daniel Vetter wrote: > > > On Thu, Oct 1, 2020 at 5:15 PM Rob Clark wrote: > > > > > > > > On Thu, Oct 1, 2020 at 12:25 AM Daniel Vetter wro= te: > > > > > > > > > > On Wed, Sep 30, 2020 at 11:16 PM Rob Clark = wrote: > > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > The android userspace treats the display pipeline as a realtime= problem. > > > > > > And arguably, if your goal is to not miss frame deadlines (ie. = vblank), > > > > > > it is. (See https://lwn.net/Articles/809545/ for the best expl= aination > > > > > > that I found.) > > > > > > > > > > > > But this presents a problem with using workqueues for non-block= ing > > > > > > atomic commit_work(), because the SCHED_FIFO userspace thread(s= ) can > > > > > > preempt the worker. Which is not really the outcome you want..= once > > > > > > the required fences are scheduled, you want to push the atomic = commit > > > > > > down to hw ASAP. > > > > > > > > > > > > But the decision of whether commit_work should be RT or not rea= lly > > > > > > depends on what userspace is doing. For a pure CFS userspace d= isplay > > > > > > pipeline, commit_work() should remain SCHED_NORMAL. > > > > > > > > > > > > To handle this, convert non-blocking commit_work() to use per-C= RTC > > > > > > kthread workers, instead of system_unbound_wq. Per-CRTC worker= s are > > > > > > used to avoid serializing commits when userspace is using a per= -CRTC > > > > > > update loop. And the last patch exposes the task id to userspa= ce as > > > > > > a CRTC property, so that userspace can adjust the priority and = sched > > > > > > policy to fit it's needs. > > > > > > > > > > > > > > > > > > v2: Drop client cap and in-kernel setting of priority/policy in > > > > > > favor of exposing the kworker tid to userspace so that user= - > > > > > > space can set priority/policy. > > > > > > > > > > Yeah I think this looks more reasonable. Still a bit irky interfa= ce, > > > > > so I'd like to get some kworker/rt ack on this. Other opens: > > > > > - needs userspace, the usual drill > > > > > > > > fwiw, right now the userspace is "modetest + chrt".. *probably* the > > > > userspace will become a standalone helper or daemon, mostly because > > > > the chrome gpu-process sandbox does not allow setting SCHED_FIFO. = I'm > > > > still entertaining the possibility of switching between rt and cfs > > > > depending on what is in the foreground (ie. only do rt for android > > > > apps). > > > > > > > > > - we need this also for vblank workers, otherwise this wont work = for > > > > > drivers needing those because of another priority inversion. > > > > > > > > I have a thought on that, see below.. > > > > > > Hm, not seeing anything about vblank worker below? > > > > > > > > - we probably want some indication of whether this actually does > > > > > something useful, not all drivers use atomic commit helpers. Not = sure > > > > > how to do that. > > > > > > > > I'm leaning towards converting the other drivers over to use the > > > > per-crtc kwork, and then dropping the 'commit_work` from atomic sta= te. > > > > I can add a patch to that, but figured I could postpone that churn > > > > until there is some by-in on this whole idea. > > > > > > i915 has its own commit code, it's not even using the current commit > > > helpers (nor the commit_work). Not sure how much other fun there is. > > > > I don't think we want per-crtc threads for this in i915. Seems > > to me easier to guarantee atomicity across multiple crtcs if > > we just commit them from the same thread. > > Oh, and we may have to commit things in a very specific order > to guarantee the hw doesn't fall over, so yeah definitely per-crtc > thread is a no go. If I'm understanding the i915 code, this is only the case for modeset commits? I suppose we could achieve the same result by just deciding to pick the kthread of the first CRTC for modeset commits. I'm not really so much concerned about parallelism for modeset. > I don't even understand the serialization argument. If the commits > are truly independent then why isn't the unbound wq enough to avoid > the serialization? It should just spin up a new thread for each commit > no? The problem with wq is prioritization and SCHED_FIFO userspace components stomping on the feet of commit_work. That is the entire motivation of this series in the first place, so no we cannot use unbound wq. BR, -R