diff mbox

drm/atomic: only run atomic_check() if crtc is active

Message ID 1447422358-18891-1-git-send-email-gustavo@padovan.org (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo Padovan Nov. 13, 2015, 1:45 p.m. UTC
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Fixes an regression added by 3ae2436 (drm/exynos/mixer: replace
direct cross-driver call with drm mode) and while this regression was
caused by a change in the exynos driver it makes sense to add the
check on atomic core to benefit other drivers as well.

The whole atomic update fails if the exynos hdmi display is not
present/active.  Add a test to only run atomic_check() if the CRTC is
active.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/drm_atomic_helper.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Ville Syrjälä Nov. 13, 2015, 2:34 p.m. UTC | #1
On Fri, Nov 13, 2015 at 11:45:58AM -0200, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Fixes an regression added by 3ae2436 (drm/exynos/mixer: replace
> direct cross-driver call with drm mode) and while this regression was
> caused by a change in the exynos driver it makes sense to add the
> check on atomic core to benefit other drivers as well.
> 
> The whole atomic update fails if the exynos hdmi display is not
> present/active.  Add a test to only run atomic_check() if the CRTC is
> active.

The check must be performed even when the crtc is not active.

Especially important for the (enabled && !active) case (ie. DPMS off)
since "DPMS on" must not fail, so any state change while in DPMS off
must be checked as if the crtc was active.

But even for the !enabled case we want to do the check so that
everything gets properly recomputed when fully disabling a crtc.

I suppose we might be able to get away with not checking in the
!enabled -> !enabled case. But I doubt that's really a useful
optimization.

> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 0c6f621..7e3cb48 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -510,6 +510,9 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
>  		const struct drm_crtc_helper_funcs *funcs;
>  
> +		if (!crtc_state->active)
> +			continue;
> +
>  		funcs = crtc->helper_private;
>  
>  		if (!funcs || !funcs->atomic_check)
> -- 
> 2.1.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Gustavo Padovan Nov. 13, 2015, 5:33 p.m. UTC | #2
Hi Ville,

2015-11-13 Ville Syrjälä <ville.syrjala@linux.intel.com>:

> On Fri, Nov 13, 2015 at 11:45:58AM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > Fixes an regression added by 3ae2436 (drm/exynos/mixer: replace
> > direct cross-driver call with drm mode) and while this regression was
> > caused by a change in the exynos driver it makes sense to add the
> > check on atomic core to benefit other drivers as well.
> > 
> > The whole atomic update fails if the exynos hdmi display is not
> > present/active.  Add a test to only run atomic_check() if the CRTC is
> > active.
> 
> The check must be performed even when the crtc is not active.
> 
> Especially important for the (enabled && !active) case (ie. DPMS off)
> since "DPMS on" must not fail, so any state change while in DPMS off
> must be checked as if the crtc was active.
> 
> But even for the !enabled case we want to do the check so that
> everything gets properly recomputed when fully disabling a crtc.

You are right. I'll fix this locally in exynos for now.

	Gustavo
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 0c6f621..7e3cb48 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -510,6 +510,9 @@  drm_atomic_helper_check_planes(struct drm_device *dev,
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		const struct drm_crtc_helper_funcs *funcs;
 
+		if (!crtc_state->active)
+			continue;
+
 		funcs = crtc->helper_private;
 
 		if (!funcs || !funcs->atomic_check)