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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D525C433F5 for ; Mon, 18 Oct 2021 17:16:28 +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 1063761212 for ; Mon, 18 Oct 2021 17:16:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 1063761212 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=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 93C6F6E9FA; Mon, 18 Oct 2021 17:16:27 +0000 (UTC) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by gabe.freedesktop.org (Postfix) with ESMTPS id C0C4A6E9FA for ; Mon, 18 Oct 2021 17:16:26 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10141"; a="215231921" X-IronPort-AV: E=Sophos;i="5.85,382,1624345200"; d="scan'208";a="215231921" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Oct 2021 10:14:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,382,1624345200"; d="scan'208";a="573149644" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.171]) by fmsmga002.fm.intel.com with SMTP; 18 Oct 2021 10:14:06 -0700 Received: by stinkbox (sSMTP sendmail emulation); Mon, 18 Oct 2021 20:14:04 +0300 Date: Mon, 18 Oct 2021 20:14:04 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: "Lisovskiy, Stanislav" Cc: intel-gfx@lists.freedesktop.org Message-ID: References: <20211018115030.3547-1-ville.syrjala@linux.intel.com> <20211018115030.3547-6-ville.syrjala@linux.intel.com> <20211018120634.GA16624@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20211018120634.GA16624@intel.com> X-Patchwork-Hint: comment Subject: Re: [Intel-gfx] [PATCH 5/9] drm/i915: Split skl+ plane update into noarm+arm pair 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Mon, Oct 18, 2021 at 03:06:34PM +0300, Lisovskiy, Stanislav wrote: > On Mon, Oct 18, 2021 at 02:50:26PM +0300, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Chop skl_program_plane() into two halves. Fist half becomes > > the _noarm() variant, second part the _arm() variant. > > > > Fortunately I have already previously grouped the register > > writes into roughtly the correct order, so the split looks > > surprisingly clean. > > > > A few notable oddities I did not realize were self arming > > are AUX_DIST and COLOR_CTL. > > > > i915_update_info doesn't look too terrible on my cfl running > > kms_atomic_transition --r plane-all-transition --extended: > > w/o patch w/ patch > > Updates: 2178 Updates: 2018 > > | | > > 1us | 1us | > > | | > > 4us | 4us |***** > > |********* |********** > > 16us |********** 16us |******* > > |*** | > > 66us | 66us | > > | | > > 262us | 262us | > > | | > > 1ms | 1ms | > > | | > > 4ms | 4ms | > > | | > > 17ms | 17ms | > > | | > > Min update: 8332ns Min update: 6164ns > > Max update: 48758ns Max update: 31808ns > > Average update: 19959ns Average update: 13159ns > > Overruns > 100us: 0 Overruns > 100us: 0 > > > > And with lockdep enabled: > > w/o patch w/ patch > > Updates: 2177 Updates: 2172 > > | | > > 1us | 1us | > > | | > > 4us | 4us | > > |******* |********* > > 16us |********** 16us |********** > > |******* |* > > 66us | 66us | > > | | > > 262us | 262us | > > | | > > 1ms | 1ms | > > | | > > 4ms | 4ms | > > | | > > 17ms | 17ms | > > | | > > Min update: 12645ns Min update: 9980ns > > Max update: 50153ns Max update: 33533ns > > Average update: 25337ns Average update: 18245ns > > Overruns > 250us: 0 Overruns > 250us: 0 > > > > TODO: On icl+ everything seems to be armed by PLANE_SURF, so we > > can optimize this even further on modern platforms. But I > > think there's a bit of refactoring to be done first to > > figure out the best way to go about it (eg. just reusing > > the current skl+ functions, or doing a lower level split). > > > > TODO: Split scaler programming as well, but IIRC the scaler > > has some oddball double buffering behaviour on some > > platforms, so needs proper reverse engineering > > > > Cc: Stanislav Lisovskiy > > Signed-off-by: Ville Syrjälä > > Should I use that one as a base for further splitting, i.e for DG2? > Which refactoring has to be done first, as I understand should be > pretty safe to leave only PLANE_SURF update in arm section, and > of course scaler is a different thing. I'm not really sure which way we should do the skl+ vs. icl+ split. Various ideas I've had: - Definitly pull all the icl+ specific things out from the skl+ functions and stuff them into icl_plane_update_noarm() - After that just call the remaining skl_plane_update_noarm()+arm() back to back from icl_update_noarm() maybe? I don't like this idea much actually. - Maybe instead pull some sequences of register writes into small helpers (eg. colorkey registers could be one). But dunno if there are other clear groups to make this super useful. - Or perhaps just pull most fiddly register value calculations (aux_dist,ckey+alpha things,maybe others) into small helpers to avoid duplicating themm but otherwise fully duplicate all the actual register writes? The sweet spot is probably some combination of those. I was also thinking of doing an overhaul of the register defines to use REG_BI() & co. That might help with approaches that involves any amount of duplication. -- Ville Syrjälä Intel