From patchwork Tue Feb 13 22:45:55 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dhinakaran Pandiyan X-Patchwork-Id: 10217675 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D4E6E6055C for ; Tue, 13 Feb 2018 22:46:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B707C28EE0 for ; Tue, 13 Feb 2018 22:46:07 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AB75428EE7; Tue, 13 Feb 2018 22:46:07 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 1FBD228EE0 for ; Tue, 13 Feb 2018 22:46:06 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BB3C56E072; Tue, 13 Feb 2018 22:45:58 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 55FBB6E072 for ; Tue, 13 Feb 2018 22:45:57 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Feb 2018 14:45:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,509,1511856000"; d="scan'208";a="203961541" Received: from orsmsx105.amr.corp.intel.com ([10.22.225.132]) by fmsmga006.fm.intel.com with ESMTP; 13 Feb 2018 14:45:56 -0800 Received: from orsmsx109.amr.corp.intel.com ([169.254.11.199]) by ORSMSX105.amr.corp.intel.com ([169.254.2.246]) with mapi id 14.03.0319.002; Tue, 13 Feb 2018 14:45:56 -0800 From: "Pandiyan, Dhinakaran" To: "chris@chris-wilson.co.uk" Thread-Topic: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags. Thread-Index: AQHTpRQbgmH/0N8hkkOGwtlUd8vazaOjZk8AgAALIoD///rqgIAADuYA Date: Tue, 13 Feb 2018 22:45:55 +0000 Message-ID: <1518563348.2572.92.camel@dk-H97M-D3H> References: <151842803599.18923.7772336419122374756@mail.alporthouse.com> <20180213214613.3936-1-dhinakaran.pandiyan@intel.com> <151855885001.31524.3077465741450828603@mail.alporthouse.com> <1518561241.2572.86.camel@dk-H97M-D3H> <151856014990.31524.15847751506798267937@mail.alporthouse.com> In-Reply-To: <151856014990.31524.15847751506798267937@mail.alporthouse.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.54.75.17] Content-ID: MIME-Version: 1.0 Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "intel-gfx@lists.freedesktop.org" , "Vivi, Rodrigo" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 2018-02-13 at 22:15 +0000, Chris Wilson wrote: > Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48) > > > > > > > > On Tue, 2018-02-13 at 21:54 +0000, Chris Wilson wrote: > > > Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13) > > > > DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor > > > > plane MMIOs are written to. But this flush is not necessary for PSR as > > > > hardware tracking takes care of exiting PSR when the MMIO's are written. > > > > > > > > Introduce a new fb_op_origin enum to differentiate flushes due to a BO > > > > being pinned from those originating due to a dirty fbdev buffer. Now, this > > > > enum can be ignored in psr_flush and psr_invalidate. > > > > > > > > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris) > > > > > > > > Cc: Rodrigo Vivi > > > > Cc: Ville Syrjälä > > > > Cc: Chris Wilson > > > > Signed-off-by: Dhinakaran Pandiyan > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > > > drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++-- > > > > drivers/gpu/drm/i915/intel_psr.c | 6 ++++-- > > > > 3 files changed, 14 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > > index 81886b74c750..3bf6c6ec0509 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > @@ -637,6 +637,7 @@ enum fb_op_origin { > > > > ORIGIN_CS, > > > > ORIGIN_FLIP, > > > > ORIGIN_DIRTYFB, > > > > + ORIGIN_PINNEDFB, > > > > }; > > > > > > > > struct intel_fbc { > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > > > index fc68b35854df..405acf3562de 100644 > > > > --- a/drivers/gpu/drm/i915/i915_gem.c > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > > > @@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > > > > > > > > vma->display_alignment = max_t(u64, vma->display_alignment, alignment); > > > > > > > > - /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */ > > > > + /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to > > > > + * flush the caches. > > > > + */ > > > > __i915_gem_object_flush_for_display(obj); > > > > - intel_fb_obj_flush(obj, ORIGIN_DIRTYFB); > > > > + > > > > + /* Features like PSR might want to rely on HW to do the frontbuffer > > > > + * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB > > > > + * so that their flush implementations can handle it accordingly. > > > > + */ > > > > > > So why it is different? Why can't the dirtyfb ioctl benefit from HW, which the > > > application is meant to call every frame to *all* dirty framebuffers > > > (which include cursors in atomic)? > > > > > > > Because the hardware requires a write to one of the pipe registers. When > > applications write to the buffer via fbdev, it doesn't lead to pipe MMIO > > write and hence does not benefit from HW triggered PSR exit. > > Somewhere you have to have that explanation, that you rely on a > subsequent mmioflip of the framebuffer to trigger the frontbuffer flush. + * features like PSR can delegate the flush to HW, which gets triggered when + * flip or cursor move MMIO's are written to. */ struct i915_vma * i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, Like this? > That probably also deserves lifting out of pin_to_display_plane as > currently there's no requirement that pin_to_display is followed by a > flip. Does pin_to_display imply ready to scan out? And I didn't get the part about "lifting out of pin_to_display_plane" -DK > -Chris diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 405acf3562de..c669fef5d388 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4071,9 +4071,13 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, } /* - * Prepare buffer for display plane (scanout, cursors, etc). - * Can be called from an uninterruptible phase (modesetting) and allows - * any flushes to be pipelined (for pageflips). + * Prepare buffer for display plane (scanout, cursors, etc). Can be called from + * an uninterruptible phase (modesetting) and allows any flushes to be pipelined + * (for pageflips). We only flush the caches while preparing the buffer for + * display and this may not lead to the buffer being scanned out if + * intel_fb_obj_flush() consumers ignore ORIGIN_PINNEDFB. This is useful because