From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from h7.fbrelay.privateemail.com (h7.fbrelay.privateemail.com [162.0.218.230]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A678972623 for ; Thu, 7 May 2026 00:23:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=162.0.218.230 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778113404; cv=none; b=egd977RTUmQUGK0u/B3KQouBtTXRA+XSlqJSnulDEydUSKwdu73DmxzG9rYJeAKVQFaCviNV0P0g+U/QqOfcE5AQZKAD2TDx3Mxe/MRCF5dFcWEqAyY+vGjfCm+zDmyoID5yqJku2C34sN9VaDcdZJHWf98dRqoJ145d+PqQV8A= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778113404; c=relaxed/simple; bh=xG9tJEOoMh89rxc+QJGr5Ezl157ijsppru3OZbbaNdc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=FrghCMpRo7fSqbAAFGYzvXko7tMHJi6HiGccgRnA54ntZmqjM4jJ5xuIZsDpqfcsAjK+1jHrOi4qyGBs4utUbma0UO9UycdK+Px8WzH6fmSMvzmbwZxfHKciL6Yex/D6hrXVIUVWoC0VMGSCrL5nAeCzWHkRpTy5fUnZYn6c6cI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=effective-light.com; spf=pass smtp.mailfrom=effective-light.com; arc=none smtp.client-ip=162.0.218.230 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=effective-light.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=effective-light.com Received: from MTA-13-4.privateemail.com (mta-13-1.privateemail.com [198.54.122.107]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by h7.fbrelay.privateemail.com (Postfix) with ESMTPSA id 4g9tJL52sDz2xD6 for ; Wed, 6 May 2026 20:23:14 -0400 (EDT) Received: from mta-13.privateemail.com (localhost [127.0.0.1]) by mta-13.privateemail.com (Postfix) with ESMTP id 4g9tJB64Szz3hhTD; Wed, 6 May 2026 20:23:06 -0400 (EDT) Received: from hal-station.localdomain (unknown [174.91.51.28]) by mta-13.privateemail.com (Postfix) with ESMTPA; Wed, 6 May 2026 20:22:32 -0400 (EDT) Date: Wed, 6 May 2026 20:22:26 -0400 From: Hamza Mahfooz To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: dri-devel@lists.freedesktop.org, Harry Wentland , Leo Li , Rodrigo Siqueira , Alex Deucher , Christian =?iso-8859-1?Q?K=F6nig?= , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Mario Limonciello , Alex Hung , Wayne Lin , Timur =?iso-8859-1?Q?Krist=F3f?= , Aurabindo Pillai , "Mario Limonciello (AMD)" , Ivan Lipski , Chenyu Chen , Matthew Schwartz , Tom Chung , Roman Li , Takashi Iwai , Colin Ian King , Charlene Liu , Kees Cook , amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 1/2] drm/atomic: attempt full modeset on page flip timeout Message-ID: References: <20260505182105.420525-1-someguy@effective-light.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Virus-Scanned: ClamAV using ClamSMTP On Wed, May 06, 2026 at 09:47:05PM +0300, Ville Syrjälä wrote: > On Tue, May 05, 2026 at 02:20:57PM -0400, Hamza Mahfooz wrote: > > We should try to recover from page flip timeouts. Forcing > > a full modeset should be generic across all atomic KMS drivers, > > so try that first. > > > > Signed-off-by: Hamza Mahfooz > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 49 +++++++++++++++++++++++++++-- > > 1 file changed, 46 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > index a768398a1884..7ee9d52f63c5 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -1926,6 +1926,43 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev, > > } > > EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks); > > > > +static int force_full_modeset(struct drm_crtc *crtc) > > +{ > > + struct drm_modeset_acquire_ctx ctx; > > + struct drm_crtc_state *crtc_state; > > + struct drm_atomic_state *state; > > + int ret; > > + int err; > > + > > + if (drm_atomic_crtc_needs_modeset(crtc->state)) > > + return -EBUSY; > > + > > + DRM_MODESET_LOCK_ALL_BEGIN(crtc->dev, ctx, 0, err); > > + state = drm_atomic_state_alloc(crtc->dev); > > + if (!state) > > + return -ENOMEM; > > + > > + state->acquire_ctx = &ctx; > > + > > + crtc_state = drm_atomic_get_crtc_state(state, crtc); > > + if (IS_ERR(crtc_state)) { > > + ret = PTR_ERR(crtc_state); > > + goto out; > > + } > > + > > + crtc_state->mode_changed = true; > > + > > + drm_info(crtc->dev, > > + "[CRTC:%d:%s] Attempting force full modeset...\n", > > + crtc->base.id, crtc->name); > > + > > + ret = drm_atomic_commit(state); > > +out: > > + drm_atomic_state_put(state); > > + DRM_MODESET_LOCK_ALL_END(crtc->dev, ctx, err); > > + return ret; > > +} > > + > > /** > > * drm_atomic_helper_wait_for_flip_done - wait for all page flips to be done > > * @dev: DRM device > > @@ -1949,17 +1986,23 @@ void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev, > > > > for (i = 0; i < dev->mode_config.num_crtc; i++) { > > struct drm_crtc_commit *commit = state->crtcs[i].commit; > > - int ret; > > > > crtc = state->crtcs[i].ptr; > > > > if (!crtc || !commit) > > continue; > > > > - ret = wait_for_completion_timeout(&commit->flip_done, 10 * HZ); > > - if (ret == 0) > > + if (!wait_for_completion_timeout(&commit->flip_done, 10 * HZ)) { > > + int ret; > > drm_err(dev, "[CRTC:%d:%s] flip_done timed out\n", > > crtc->base.id, crtc->name); > > + > > + ret = force_full_modeset(crtc); > > This looks like some kind of ugly hack to paper over a driver bug. > I really don't want this for i915/xe because all it'll end up doing > is make it harder to debug any real issues. In that case, would you be okay with having drm_atomic_helper_wait_for_flip_done() return an error code, or did you have something else in mind? > > > + if (ret) > > + drm_err(dev, > > + "[CRTC:%d:%s] force full modeset failed! ret=%d\n", > > + crtc->base.id, crtc->name, ret); > > + } > > } > > > > if (state->fake_commit) > > -- > > 2.54.0 > > -- > Ville Syrjälä > Intel