From: Manasi Navare <manasi.d.navare@intel.com> To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: Manasi Navare <manasi.d.navare@intel.com>, Daniel Vetter <daniel.vetter@intel.com> Subject: [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY" Date: Tue, 9 Feb 2021 16:14:01 -0800 [thread overview] Message-ID: <20210210001401.463-1-manasi.d.navare@intel.com> (raw) This reverts commit fb6473a48b635c55d04eb94e579eede52ef39550. These additional checks added to avoid EBUSY give unnecessary WARN_ON in case of big joiner used in i915 in which case even if the modeset is requested on a single pipe, internally another consecutive pipe is stolen and used to drive half of the transcoder timings. So in this case it is expected that requested crtc and affected crtcs do not match. Hence the added WARN ON becomes irrelevant. But there is no easy solution to get the bigjoiner information here at drm level. So for now revert this until we work out a better solution. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/drm_atomic.c | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index b1efa9322be2..48b2262d69f6 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -320,10 +320,6 @@ EXPORT_SYMBOL(__drm_atomic_state_free); * needed. It will also grab the relevant CRTC lock to make sure that the state * is consistent. * - * WARNING: Drivers may only add new CRTC states to a @state if - * drm_atomic_state.allow_modeset is set, or if it's a driver-internal commit - * not created by userspace through an IOCTL call. - * * Returns: * * Either the allocated state or the error code encoded into the pointer. When @@ -1306,15 +1302,10 @@ int drm_atomic_check_only(struct drm_atomic_state *state) struct drm_crtc_state *new_crtc_state; struct drm_connector *conn; struct drm_connector_state *conn_state; - unsigned requested_crtc = 0; - unsigned affected_crtc = 0; int i, ret = 0; DRM_DEBUG_ATOMIC("checking %p\n", state); - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) - requested_crtc |= drm_crtc_mask(crtc); - for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { ret = drm_atomic_plane_check(old_plane_state, new_plane_state); if (ret) { @@ -1362,26 +1353,6 @@ int drm_atomic_check_only(struct drm_atomic_state *state) } } - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) - affected_crtc |= drm_crtc_mask(crtc); - - /* - * For commits that allow modesets drivers can add other CRTCs to the - * atomic commit, e.g. when they need to reallocate global resources. - * This can cause spurious EBUSY, which robs compositors of a very - * effective sanity check for their drawing loop. Therefor only allow - * drivers to add unrelated CRTC states for modeset commits. - * - * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL as an output - * so compositors know what's going on. - */ - if (affected_crtc != requested_crtc) { - DRM_DEBUG_ATOMIC("driver added CRTC to commit: requested 0x%x, affected 0x%0x\n", - requested_crtc, affected_crtc); - WARN(!state->allow_modeset, "adding CRTC not allowed without modesets: requested 0x%x, affected 0x%0x\n", - requested_crtc, affected_crtc); - } - return 0; } EXPORT_SYMBOL(drm_atomic_check_only); -- 2.19.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Manasi Navare <manasi.d.navare@intel.com> To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Cc: Daniel Vetter <daniel.vetter@intel.com> Subject: [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY" Date: Tue, 9 Feb 2021 16:14:01 -0800 [thread overview] Message-ID: <20210210001401.463-1-manasi.d.navare@intel.com> (raw) This reverts commit fb6473a48b635c55d04eb94e579eede52ef39550. These additional checks added to avoid EBUSY give unnecessary WARN_ON in case of big joiner used in i915 in which case even if the modeset is requested on a single pipe, internally another consecutive pipe is stolen and used to drive half of the transcoder timings. So in this case it is expected that requested crtc and affected crtcs do not match. Hence the added WARN ON becomes irrelevant. But there is no easy solution to get the bigjoiner information here at drm level. So for now revert this until we work out a better solution. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/drm_atomic.c | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index b1efa9322be2..48b2262d69f6 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -320,10 +320,6 @@ EXPORT_SYMBOL(__drm_atomic_state_free); * needed. It will also grab the relevant CRTC lock to make sure that the state * is consistent. * - * WARNING: Drivers may only add new CRTC states to a @state if - * drm_atomic_state.allow_modeset is set, or if it's a driver-internal commit - * not created by userspace through an IOCTL call. - * * Returns: * * Either the allocated state or the error code encoded into the pointer. When @@ -1306,15 +1302,10 @@ int drm_atomic_check_only(struct drm_atomic_state *state) struct drm_crtc_state *new_crtc_state; struct drm_connector *conn; struct drm_connector_state *conn_state; - unsigned requested_crtc = 0; - unsigned affected_crtc = 0; int i, ret = 0; DRM_DEBUG_ATOMIC("checking %p\n", state); - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) - requested_crtc |= drm_crtc_mask(crtc); - for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { ret = drm_atomic_plane_check(old_plane_state, new_plane_state); if (ret) { @@ -1362,26 +1353,6 @@ int drm_atomic_check_only(struct drm_atomic_state *state) } } - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) - affected_crtc |= drm_crtc_mask(crtc); - - /* - * For commits that allow modesets drivers can add other CRTCs to the - * atomic commit, e.g. when they need to reallocate global resources. - * This can cause spurious EBUSY, which robs compositors of a very - * effective sanity check for their drawing loop. Therefor only allow - * drivers to add unrelated CRTC states for modeset commits. - * - * FIXME: Should add affected_crtc mask to the ATOMIC IOCTL as an output - * so compositors know what's going on. - */ - if (affected_crtc != requested_crtc) { - DRM_DEBUG_ATOMIC("driver added CRTC to commit: requested 0x%x, affected 0x%0x\n", - requested_crtc, affected_crtc); - WARN(!state->allow_modeset, "adding CRTC not allowed without modesets: requested 0x%x, affected 0x%0x\n", - requested_crtc, affected_crtc); - } - return 0; } EXPORT_SYMBOL(drm_atomic_check_only); -- 2.19.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next reply other threads:[~2021-02-10 0:09 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-10 0:14 Manasi Navare [this message] 2021-02-10 0:14 ` [Intel-gfx] [PATCH] Revert "drm/atomic: document and enforce rules around "spurious" EBUSY" Manasi Navare 2021-02-10 2:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork 2021-02-10 6:19 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2021-02-10 13:16 ` [Intel-gfx] [PATCH] " Daniel Vetter 2021-02-10 13:16 ` Daniel Vetter 2021-02-10 13:38 ` Simon Ser 2021-02-10 13:38 ` Simon Ser 2021-02-10 15:07 ` Ville Syrjälä 2021-02-10 15:07 ` Ville Syrjälä 2021-02-10 23:26 ` Navare, Manasi 2021-02-10 23:26 ` Navare, Manasi 2021-02-11 15:46 ` Daniel Vetter 2021-02-11 15:46 ` Daniel Vetter 2021-02-11 16:14 ` Daniel Stone 2021-02-11 16:14 ` Daniel Stone 2021-02-11 16:55 ` Ville Syrjälä 2021-02-11 16:55 ` Ville Syrjälä
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210210001401.463-1-manasi.d.navare@intel.com \ --to=manasi.d.navare@intel.com \ --cc=daniel.vetter@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.