From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S968263AbcHBR4K (ORCPT ); Tue, 2 Aug 2016 13:56:10 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34085 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966923AbcHBRzr (ORCPT ); Tue, 2 Aug 2016 13:55:47 -0400 Date: Tue, 2 Aug 2016 15:38:41 +0200 From: Daniel Vetter To: Keith Packard Cc: linux-kernel@vger.kernel.org, Daniel Vetter , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/i915: cleanup_plane_fb: also drop reference to current state wait_req Message-ID: <20160802133841.GN6232@phenom.ffwll.local> Mail-Followup-To: Keith Packard , linux-kernel@vger.kernel.org, Daniel Vetter , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org References: <1469951691-5938-1-git-send-email-keithp@keithp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1469951691-5938-1-git-send-email-keithp@keithp.com> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jul 31, 2016 at 12:54:51AM -0700, Keith Packard wrote: > There are two paths into intel_cleanup_plane_fb, the normal completion > path and the failure path. > > In the failure case, intel_cleanup_plane_fb is called before > drm_atomic_helper_swap_state, so any wait_req reference made in > intel_prepare_plane_fb will be in old_intel_state->wait_req. > > In the normal completion path, drm_atomic_helper_swap_state has > already been called, so the plane state holding the just-used wait_req > will not be in old_intel_state->wait_req, rather it will be in the > state associated with the plane itself. > > Clearing this reference ensures that the wait_req will be freed as > soon as it the related mode setting operation is complete, rather than > waiting for some future mode setting operation to eventually > dereference it. > > The existing dereference of old_intel_state->wait_req is still > required as that will hold the wait_req when the mode setting > operation fails. > > cc: Daniel Vetter > cc: David Airlie > cc: intel-gfx@lists.freedesktop.org > cc: dri-devel@lists.freedesktop.org > Signed-off-by: Keith Packard Hm, I think we should just clean up wiat_req in ->atomic_destroy_state instead of littering cleanup code all over. But this gets the job done, so applied. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3074c56..dbabaf3 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13924,6 +13924,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane, > struct drm_device *dev = plane->dev; > struct intel_plane *intel_plane = to_intel_plane(plane); > struct intel_plane_state *old_intel_state; > + struct intel_plane_state *intel_state = to_intel_plane_state(plane->state); > struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); > struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); > > @@ -13941,6 +13942,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane, > (obj && !(obj->frontbuffer_bits & intel_plane->frontbuffer_bit))) > i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit); > > + i915_gem_request_assign(&intel_state->wait_req, NULL); > i915_gem_request_assign(&old_intel_state->wait_req, NULL); > } > > -- > 2.8.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch