Re: [PATCH RFC] drm/atomic: Disable planes on blanked CRTC and enable on unblank

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Nov 18, 2015 at 05:52:45PM +0200, Jyri Sarha wrote:
> On 11/16/15 18:06, Daniel Vetter wrote:
> >On Thu, Nov 05, 2015 at 05:03:09PM +0200, Jyri Sarha wrote:
> >>Disable planes if they are on to be blanked CRTC and enable them when
> >>the CRTC is turned back on by DMPS.
> >>
> >>This is desirable on HW that loses its context on blanking. When
> >>planes are enabled and disabled with the associated CRTCs, there is no
> >>need to restore the plane context in runtime_resume callback.
> >>
> >>Signed-off-by: Jyri Sarha <jsarha@ti.com>
> >>---
> >>We would need something like this to get rid off OMAPDSS somewhat
> >>messy runtime_resume code. How does this look, could this generic
> >>solution be refined to be acceptable to mainline, or should we start
> >>looking a local solution to omapdrm?
> >>
> >>BTW, with this patch the planes are sometimes disabled multiple
> >>times. So probably a boolean (or maybe two like on crtc) should be
> >>added to drm_plane_state to track and avoid multiple shutdowns.
> >
> >The recommended way to do this is to call drm_atomic_add_affected_planes
> >from your crtc's atomic_check callback when active_changed is set. This
> >will also take care of enabling all planes again. For disabling we don't
> >yet have a helper yet, but it would be easy to create a
> >drm_atomic_helper_disable_planes(crtc) which just walks over all planes in
> >an atomic update that are currently using the given crtc and disables it
> >using plane_helper_funcs->atomic_disable. That would again require
> >drm_atomic_add_affected_planes to be called first.
> >
> >This way current helper behaviour is unchanged, but it'd be easy to
> >construct the behaviour you want using helpers with
> >drm_atomic_add_affected_planes called from the crtc->disable hook.
> >
> >I thought there was an rfc patch somewhere for this, but I can't find it
> >any more.
> 
> It appears that in the end these instructions were really all I needed.
> 
> There is only one thing that still bothers me. When implementing the feature
> you described above, I had a print in the code to see if it actually did
> anything. At first - because of my mistake - it did not, but the plane
> context was still restored just fine on drm_next and mainline linux. However
> the same omapdrm code did not restore the plane context on our 4.1 based
> Linux branch.
> 
> So something similar to what I tried to accomplish has been implemented some
> where between Linux 4.1 and 4.3?
> 
> Anyway, after back-porting the above fix works perfectly on our 4.1 based
> branch.

commit 57744aa7cfb5969c5a0621f4cfa45bb83d391064
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date:   Tue May 19 16:41:03 2015 +0200

    drm/atomic: add all affected planes in drm_atomic_helper_check_modeset> 

is the commit I've forgotten about too ;-)

So the only thing left (I think) is some helper to disable all the planes
currently on the crtc, which drivers can use in their crtc->disable hook.
The enabling side is indeed already taken care of.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel