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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id E7107E732E7 for ; Thu, 28 Sep 2023 16:21:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B141310E6A2; Thu, 28 Sep 2023 16:21:40 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6A1DB10E691 for ; Thu, 28 Sep 2023 16:21:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695918099; x=1727454099; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=7hZgDZjdPIRvs3GRVr8JNrfL7Ome7MPa2URXVzuRd2E=; b=geDOHqocs0BYkVREB+rIDaFdd1W2Gb5jP9Sf225PdL853HChelhOpoeH nTGaYj5xzzc01qJAVVEkeVwqHKst7cFOsajjzb6xClrE7CZDt1PCotGLb lPo2Y2z+aNYcrivyGxz6IsV3/cu9IcjZvSon4VXSQAsPnDDuFhNgnfEHl 4qQA1XCOoqsRh5/q/C4kZnUkV6gSVKqX1MVDx1tUFz6C7cnnZLKMCamJ/ ZIhORMpqSyqWzIBQu1yXIBkrQcAKxNpp/61JcXYUUGPqVT/g4sp1whyuT oe6nl/p7StriUzk0Bxiv+QBleNLS5kbpIBi38vP2daTQN22f1ieTbId3X A==; X-IronPort-AV: E=McAfee;i="6600,9927,10847"; a="380988106" X-IronPort-AV: E=Sophos;i="6.03,184,1694761200"; d="scan'208";a="380988106" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Sep 2023 09:21:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10847"; a="749646921" X-IronPort-AV: E=Sophos;i="6.03,184,1694761200"; d="scan'208";a="749646921" Received: from stinkpipe.fi.intel.com (HELO stinkbox) ([10.237.72.153]) by orsmga002.jf.intel.com with SMTP; 28 Sep 2023 09:21:35 -0700 Received: by stinkbox (sSMTP sendmail emulation); Thu, 28 Sep 2023 19:21:34 +0300 Date: Thu, 28 Sep 2023 19:21:34 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: "Hogander, Jouni" Message-ID: References: <20230927073125.3825836-1-jouni.hogander@intel.com> <2fd37d47aacb192f9d4b4e8491743323536f364c.camel@intel.com> <85245068-4cce-8851-e527-29792385f78b@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Patchwork-Hint: comment Subject: Re: [Intel-xe] [RFC PATCH 0/3] Xe dma fence handling on atomic commit X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Nikula, Jani" , "intel-xe@lists.freedesktop.org" Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, Sep 28, 2023 at 07:10:46PM +0300, Ville Syrjälä wrote: > On Thu, Sep 28, 2023 at 08:23:41AM +0000, Hogander, Jouni wrote: > > On Wed, 2023-09-27 at 14:35 +0200, Maarten Lankhorst wrote: > > > Hey, > > > > > > On 2023-09-27 12:45, Hogander, Jouni wrote: > > > > On Wed, 2023-09-27 at 12:33 +0200, Maarten Lankhorst wrote: > > > > > Hey, > > > > > > > > > > When we wrote the original display support, we purposely decided > > > > > on > > > > > not > > > > > adding i915_sw_fence support. > > > > > > > > > > In this case, I think a better approach would be to remove this > > > > > code > > > > > from i915 as well, and end up with cleaner display code for both > > > > > drivers. > > > > > > > > Yes, I agree eventually this would be the goal. I did some > > > > experiments > > > > here: > > > > > > > > https://patchwork.freedesktop.org/patch/558982/?series=123898&rev=4 > > > > > > > > Which is replacing i915_sw_fence with same code I'm using for Xe > > > > driver > > > > in this patch set. The problem is GPU reset detection. I don't have > > > > currently good ideas how to tackle that without compromizing i915 > > > > functionality in this scenario -> ended up doing this only for Xe > > > > to > > > > ensure this is not blocking upstreaming Xe. Would this be > > > > acceptable as > > > > temporary solution to be solved after upstreaming? Anyways what I'm > > > > doing in these patches is not really an i915_sw_fence revised, but > > > > using dma_fences. > > > In atomic, plane_state->fence is set for all fences, and that should > > > be > > > enough. I think creating a reservation object is a massive overkill, > > > and we should try to use the drm_atomic_helper_wait_for_fences if at > > > all > > > possible. > > > > Checked this plane_state->fence. To me it looks like it's for userspace > > to set? I don't think this is the case for i915. For Xe, I don't know > > is it done? > > > > Also i915 is waiting for fences on old frame buffer objects and new fb > > objects. Shouldn't we do the same for Xe as well? > > xe shouldn't need the old fb fence since userspace there won't be > doing any MI_SCANLINE_WAIT stuff. > > > Anyways there is > > currently no mechanism in dma_fence implementation to have common fence > > for two separate dma_resv objects (dma_resv_get_singleton) -> I did > > this by creating a new dma_resv for this purpose. If there is no reason > > to wait for possible fences on old fb object, maybe we could just do > > dma_resv_get_singleton in prepare planes for Xe and wait for that and > > uapi.fence in commit? > > drm_gem_plane_helper_prepare_fb() supposedly does something that already > works for xe. Looks like it does a few things: > - populates plane_state->fence with dma_resv_get_singleton() if using > implicit fencing > - uses dma_fence_chain internally to add additional (kernel internal > for memory movement/etc. maybe?) fences to the uapi supplied explicit fence > > Presumably we should be able to glue on the old fb fence using dma_fence_chain > as well? Oh and the display vs. GPU reset chicken/egg problem might be the hard nut to crack. We need to get the pending display commits flushed out before we can let a GPU reset clobber the display, but the display commits are waiting on the fences and thus neither the display commits nor the GPU reset can proceed, I think. Having to wait for some kind of huge fence timeout in the display commit seems a bit sub-optimal. Or would the fences get signalled when we detect the GPU hang? If not, can we achieve something like that? -- Ville Syrjälä Intel