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=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 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 59530C4363D for ; Thu, 24 Sep 2020 11:01:42 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BDF822395B for ; Thu, 24 Sep 2020 11:01:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BDF822395B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 240C76E1BA; Thu, 24 Sep 2020 11:01:41 +0000 (UTC) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4C5126E1BA; Thu, 24 Sep 2020 11:01:40 +0000 (UTC) IronPort-SDR: 2R1QqWiEJIncFhKiuYIDHk8uVWE5c+wzY45dqUIsznQkHIzxXp90qcAwaAzSI+ImNRCDfvsNXi 5tVWqeQ8WtaA== X-IronPort-AV: E=McAfee;i="6000,8403,9753"; a="141184569" X-IronPort-AV: E=Sophos;i="5.77,297,1596524400"; d="scan'208";a="141184569" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2020 04:01:39 -0700 IronPort-SDR: 40EV3xhybvGzIHYU0Ng65UYYdevJxB6Nm9nsV6TLE66lpxB2vm8MS9OF27Da5/3Zf+LDoSpDK/ PruilT+9+Sjw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,297,1596524400"; d="scan'208";a="349219856" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by orsmga007.jf.intel.com with SMTP; 24 Sep 2020 04:01:36 -0700 Received: by stinkbox (sSMTP sendmail emulation); Thu, 24 Sep 2020 14:01:35 +0300 Date: Thu, 24 Sep 2020 14:01:35 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Pekka Paalanen Message-ID: <20200924110135.GJ6112@intel.com> References: <20200923105737.2943649-1-daniel.vetter@ffwll.ch> <20200923151852.2952812-1-daniel.vetter@ffwll.ch> <20200923191724.GA62596@xpredator> <20200924104101.63be1c13@eldfell> <20200924131056.54beb12e@eldfell> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200924131056.54beb12e@eldfell> X-Patchwork-Hint: comment User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [Intel-gfx] [PATCH] drm/atomic: document and enforce rules around "spurious" EBUSY X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter , Intel Graphics Development , Marius Vlad , DRI Development , Daniel Vetter Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Thu, Sep 24, 2020 at 01:10:56PM +0300, Pekka Paalanen wrote: > On Thu, 24 Sep 2020 10:04:12 +0200 > Daniel Vetter wrote: > = > > On Thu, Sep 24, 2020 at 9:41 AM Pekka Paalanen wr= ote: > > > > > > On Wed, 23 Sep 2020 22:01:25 +0200 > > > Daniel Vetter wrote: > > > = > > > > On Wed, Sep 23, 2020 at 9:17 PM Marius Vlad wrote: = > > > > > > > > > > On Wed, Sep 23, 2020 at 05:18:52PM +0200, Daniel Vetter wrote: = > > > > > > When doing an atomic modeset with ALLOW_MODESET drivers are all= owed to > > > > > > pull in arbitrary other resources, including CRTCs (e.g. when > > > > > > reconfiguring global resources). = > > > > > > ... > > > = > > > > > > @@ -1313,6 +1322,26 @@ int drm_atomic_check_only(struct drm_ato= mic_state *state) > > > > > > } > > > > > > } > > > > > > > > > > > > + for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) > > > > > > + affected_crtc |=3D drm_crtc_mask(crtc); > > > > > > + > > > > > > + /* > > > > > > + * For commits that allow modesets drivers can add other = CRTCs to the > > > > > > + * atomic commit, e.g. when they need to reallocate globa= l resources. > > > > > > + * This can cause spurious EBUSY, which robs compositors = of a very > > > > > > + * effective sanity check for their drawing loop. Therefo= r only allow > > > > > > + * drivers to add unrelated CRTC states for modeset commi= ts. > > > > > > + * > > > > > > + * FIXME: Should add affected_crtc mask to the ATOMIC IOC= TL as an output > > > > > > + * so compositors know what's going on. > > > > > > + */ > > > > > > + if (affected_crtc !=3D requested_crtc) { > > > > > > + DRM_DEBUG_ATOMIC("driver added CRTC to commit: re= quested 0x%x, affected 0x%0x\n", > > > > > > + requested_crtc, affected_crtc); > > > > > > + WARN(!state->allow_modeset, "adding CRTC not allo= wed without modesets: requested 0x%x, affected 0x%0x\n", > > > > > > + requested_crtc, affected_crtc); = > > > > > Previous patch had the warn on state->allow_modeset now is > > > > > !state->allow_modeset. Is that correct? = > > > > > > > > We need to fire a warning when allow_modeset is _not_ set. An earli= er > > > > version got that wrong, and yes that would have caused a _ton_ of > > > > warnings on any fairly new intel platform. > > > > = > > > > > I haven't followed the entire thread on this matter, but I guess = the idea > > > > > is that somehow the kernel would pass to userspace a CRTC mask of > > > > > affected_crtc (somehow, we don't know how atm) and with it, users= pace > > > > > can then issue a new commit (this commit blocking) with those? = > > > > > > > > Either that, or just use that to track all the in-flight drm events. > > > > Userspace will get events for all the crtc, not just the one it ask= ed > > > > to update. = > > > > > > Wait, does that happen already? Getting CRTC events for CRTCs userspa= ce > > > didn't include in the atomic commit? = > > = > > Yeah I'm pretty sure. With the affected_crtc mask you could update > > your internal book-keeping to catch these, which should also prevent > > all the spurious EBUSY. But I'm not entirely sure, I just read the > > code, haven't tested. > = > If that actually happens, how does userspace know whether the > userdata argument with the event is valid or not? At some point I was worried about the kernel potentially sending spurious events, but IIRC I managed to convince myself that it shouldn't happen. I think I came to the conclusion the events were populated before the core calls into the driver. But maybe I misanalyzed it, or something has since broken? -- = Ville Syrj=E4l=E4 Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx