From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Add Baytrail PSR Support. Date: Tue, 4 Feb 2014 14:54:24 +0100 Message-ID: <20140204135424.GT17001@phenom.ffwll.local> References: <20140129145648.GE9454@intel.com> <1391010606-9206-1-git-send-email-rodrigo.vivi@gmail.com> <20140130130236.GI29091@nuc-i3427.alporthouse.com> <20140204105259.GF17001@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f47.google.com (mail-ee0-f47.google.com [74.125.83.47]) by gabe.freedesktop.org (Postfix) with ESMTP id A553DFA4C8 for ; Tue, 4 Feb 2014 07:02:08 -0800 (PST) Received: by mail-ee0-f47.google.com with SMTP id d49so4328485eek.6 for ; Tue, 04 Feb 2014 07:02:01 -0800 (PST) Content-Disposition: inline In-Reply-To: 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: Rodrigo Vivi Cc: intel-gfx List-Id: intel-gfx@lists.freedesktop.org On Tue, Feb 04, 2014 at 11:03:25AM -0200, Rodrigo Vivi wrote: > >> > In the case of a moving cursor that means indefinitely. > >> That's true... So I think we really need a work queue delaying the enable. > >> Or do you have any better idea? > > > > Yeah, sounds like we need a delayed work-queue to re-enable psr, also for > > gtt mmap writes. See Chris' latest crazy example of what the X server is > > currently allowed to do. > > http://cgit.freedesktop.org/~vivijim/drm-intel/commit/?h=psr-baytrail-wq&id=f356c599db47dca4966dfb173282b111ce3055f5 > > But I'm not sure if I should do all with delayed schedule or still > calling this psr_update on mark_idle and just schedule work on cursor. > what do you think? You also need to tear down gtt mmaps to make sure we catch them, at least for the gtt mmap write case. And add a bit of code to gem_fault to invalidate psr if needed. > Please notice that besides the wq it also has mutex psr added on this: > > http://cgit.freedesktop.org/~vivijim/drm-intel/commit/?h=psr-baytrail-wq&id=b18fb62af0d591cec593a75f7cf896b46e0cc91e tbh I haven't thought much yet about locking, but iirc the current stuff is hapzardous. Ville looked into fixing this, but it seems to be fairly complicated. Not sure whether we should aim for a common locking between fbc/psr or not (since they both are closely related wrt their interactions between modeset code and gem stuff). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch