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

[Gustavo Padovan]

Hi Ville,

2015-11-13 Ville Syrjälä :

> On Fri, Nov 13, 2015 at 11:45:58AM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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 
> 
> 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 
> ---
>  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-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html