From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vijay Purushothaman Subject: Re: [PATCH] drm/i915: Don't wait for vblank for sprite plane flips Date: Mon, 01 Jul 2013 10:56:06 +0530 Message-ID: <51D112EE.9060909@intel.com> References: <1372428931-24144-1-git-send-email-vijay.a.purushothaman@intel.com> <20130628142450.GH5004@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 0E7A6E5C2F for ; Sun, 30 Jun 2013 22:26:09 -0700 (PDT) In-Reply-To: <20130628142450.GH5004@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: =?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= Cc: Intel Graphics List-Id: intel-gfx@lists.freedesktop.org On 6/28/2013 7:54 PM, Ville Syrj=E4l=E4 wrote: > On Fri, Jun 28, 2013 at 07:45:31PM +0530, Vijay Purushothaman wrote: >> Since the sprite planes are using synchronized MMIO based flip, no need >> to wait for vblank. Removing this wait allows us to get a nice >> performance boost to both 3D & media workloads based on sprite (~60 fps >> from ~20 fps) > > Nak. We can't unpin the buffer until the hardware has finished reading > from it. Thanks much for the review feedback. We have removed this check in our = android production branch so far we have not seen any side effect or = artifact. Is this a conservative check or do we have any use case which = will fail without this piece of code? Apparently the windows driver team have been using MMIO flips for Sprite = and the windows driver also didn't wait for vblank for such flips all along. Could you please help me understand this a bit better? This wait reduces = the perf to ~20 fps and thus prevent us from using sprite for any OGL = layer mapping in hwcomposer and we are losing significant amount of = power. For video content playback the problem is not that bad. > > The proper fix is to do the unpin asynchronously after the flip has > completed. That's one part of the bigger atomic pageflip story. > Any idea when are we planning to merge this atomic flip in d-i-n-q? Thanks, Vijay >> >> Signed-off-by: Vijay Purushothaman >> Signed-off-by: Gary Smith >> --- >> drivers/gpu/drm/i915/intel_sprite.c | 14 +------------- >> 1 file changed, 1 insertion(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/= intel_sprite.c >> index 1fa5612..1d14fc0 100644 >> --- a/drivers/gpu/drm/i915/intel_sprite.c >> +++ b/drivers/gpu/drm/i915/intel_sprite.c >> @@ -828,20 +828,8 @@ intel_update_plane(struct drm_plane *plane, struct = drm_crtc *crtc, >> intel_disable_primary(crtc); >> >> /* Unpin old obj after new one is active to avoid ugliness */ >> - if (old_obj) { >> - /* >> - * It's fairly common to simply update the position of >> - * an existing object. In that case, we don't need to >> - * wait for vblank to avoid ugliness, we only need to >> - * do the pin & ref bookkeeping. >> - */ >> - if (old_obj !=3D obj) { >> - mutex_unlock(&dev->struct_mutex); >> - intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe); >> - mutex_lock(&dev->struct_mutex); >> - } >> + if (old_obj) >> intel_unpin_fb_obj(old_obj); >> - } >> >> out_unlock: >> mutex_unlock(&dev->struct_mutex); >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >