From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vandana Kannan Subject: Re: [PATCH 4/5] drm/i915: Idleness detection for DRRS Date: Wed, 18 Dec 2013 15:39:31 +0530 Message-ID: <52B1745B.1040507@intel.com> References: <1387258107-19232-1-git-send-email-vandana.kannan@intel.com> <1387258107-19232-5-git-send-email-vandana.kannan@intel.com> <20131217122928.GI22448@nuc-i3427.alporthouse.com> <52B15A44.9030104@intel.com> <20131218090442.GY22448@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id A1E5BFBBDB for ; Wed, 18 Dec 2013 02:09:35 -0800 (PST) In-Reply-To: <20131218090442.GY22448@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Chris Wilson Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Dec-18-2013 2:34 PM, Chris Wilson wrote: > On Wed, Dec 18, 2013 at 01:48:12PM +0530, Vandana Kannan wrote: >> On Dec-17-2013 5:59 PM, Chris Wilson wrote: >>> On Tue, Dec 17, 2013 at 10:58:26AM +0530, Vandana Kannan wrote: >>>> Adding support to detect display idleness by tracking page flip from >>>> user space. Switch to low refresh rate is triggered after 2 seconds of >>>> idleness. The delay is configurable. If there is a page flip or call to >>>> update the plane, then high refresh rate is applied. >>>> The feature is not used in dual-display mode. >>> >>> Looks very inconsistent next to intel_fbc_disable/intel_fbc_update. >>> -Chris >>> >> We have implemented this in a way that it is similar to fbc >> implementation. Could you explain some more about your review comment? > > See the split between intel_fbc_disable and intel_fbc_update and how it > would make your code more tidy, your API harder to get wrong and make it > easier to integrate all of these triggers into a single routine. Also > note that you miss out on frontbuffer rendering detection. > -Chris > The current implementation makes use of "bool update" to differentiate between an update/disable. I will make changes so that the implementation is similar to intel_fbc_disable/intel_fbc_update. Could you give more information on the miss on frontbuffer rendering detection?