[Intel-gfx] [PATCH] drm/atomic: Introduce drm_atomic_helper_shutdown

[Daniel Vetter]

On Mon, Mar 27, 2017 at 10:15:12AM +0300, Tomi Valkeinen wrote:
> On 21/03/17 18:41, Daniel Vetter wrote:
> > The trouble here is that it does multiple atomic commits under one
> > drm_modeset_lock_all, which breaks the behind-the-scenes acquire
> > context magic that function pulls off. It's much better to have one
> > overall atomic commit. That we still have multiple atomic commits
> > prevents us from adding some pretty useful debug checks to the atomic
> > machinery.
> > 
> > Hence it is really a bad idea to call the legacy
> > drm_crtc_force_disable_all() function. There's 2 atomic drivers using
> > this still, nouveau and tinydrm. To fix this, introduce a new
> > drm_atomic_helper_shutdown() by extracting the code from i915.
> > 
> > While at it improve kernel-doc and catch future offenders by
> > sprinkling a WARN_ON into the legacy function. We should probably move
> > those into the legacy modeset helpers, too ...
> > 
> > v2: Make it compile on arm drivers too (Noralf).
> > 
> > v3: Correct kerneldoc to point at _disable_all().
> > 
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Acked-by: Noralf Trønnes <noralf at tronnes.org>
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Cc: Noralf Trønnes <noralf at tronnes.org>
> > Cc: Ben Skeggs <bskeggs at redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> 
> Tested-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> 
> As a side note, I find it a bit odd that when fbdev is disabled, the
> crtcs stays enabled even when DRM userspace app quits. Or is that just
> omapdrm behavior? I presume not, as this shutdown on unload would not be
> needed otherwise.

With atomic we've stopped killing the entire CRTC when you the last
userspace reference for the framebuffer on the primary plane disappears,
but just shut down the primary plane. Assuming the driver can do that, we
fall back to full CRTC shutdown if not. That avoids a pile of flickering
and unecessary modesets.

But yeah downside is that the crtc is active even when unloading. Note
that with the fb refcounting years ago the CRTC (including planes) was
already kept alive when you have fbdev emulation enabled.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch