Hi Jyri,

Thank you for the patch.

On Friday 27 Jan 2017 12:04:54 Jyri Sarha wrote:
> This reverts commit dadf4659d0608e034b6633f30300c2eff2dafb4c.
> 
> If planes are not disabled when the they are not on any crtc anymore
> they will remain active and may show as "ghosts" when the crtc they
> were last on is active again.

Sorry for the breakage.

The drm_atomic_helper_commit_planes() helper documentation states

 * Unless otherwise needed, drivers are advised to set the ACTIVE_ONLY flag in
 * @flags in order not to receive plane update notifications related to a
 * disabled CRTC. This avoids the need to manually ignore plane updates in
 * driver code when the driver and/or hardware can't or just don't need to
 * deal with updates on disabled CRTCs, for example when supporting runtime
 * PM.

I wonder what this implies when CRTCs are being disabled. I see very few cases 
where the hardware wouldn't need the plane atomic disable operation being 
called when a plane is being disabled due to its CRTC being disabled. Maybe 
this should thus be addressed in the core. Daniel, any comment on this ?

> Signed-off-by: Jyri Sarha <jsa...@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 8 +-------
>  drivers/gpu/drm/omapdrm/omap_drv.c  | 3 +--
>  2 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index dd47dc1..b68c70e 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -410,13 +410,7 @@ static void omap_crtc_atomic_flush(struct drm_crtc
> *crtc, dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
>       }
> 
> -     /*
> -      * Only flush the CRTC if it is currently enabled. CRTCs that require 
a
> -      * mode set are disabled prior plane updates and enabled afterwards.
> -      * They are thus not active (regardless of what their CRTC core state
> -      * reports) and the DRM core could thus call this function even though
> -      * the CRTC is currently disabled. Do nothing in that case.
> -      */
> +     /* Only flush the CRTC if it is currently enabled. */
>       if (!omap_crtc->enabled)
>               return;
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index 00aa214..5e55f1b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -96,8 +96,7 @@ static void omap_atomic_complete(struct
> omap_atomic_state_commit *commit) dispc_runtime_get();
> 
>       drm_atomic_helper_commit_modeset_disables(dev, old_state);
> -     drm_atomic_helper_commit_planes(dev, old_state,
> -                                     DRM_PLANE_COMMIT_ACTIVE_ONLY);
> +     drm_atomic_helper_commit_planes(dev, old_state, 0);
>       drm_atomic_helper_commit_modeset_enables(dev, old_state);
> 
>       omap_atomic_wait_for_completion(dev, old_state);

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to