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

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

[Gustavo Padovan]

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(+)

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

Re: [PATCH] drm/atomic: only run atomic_check() if crtc is active

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/atomic: only run atomic_check() if crtc is active

[Gustavo Padovan]

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