On Wed, Nov 01, 2017 at 01:59:00PM +0100, Noralf Trønnes wrote:
Den 01.11.2017 09.47, skrev Daniel Vetter:
On Tue, Oct 31, 2017 at 05:37:23PM +0100, Noralf Trønnes wrote:
Den 30.10.2017 10.34, skrev Daniel Vetter:
Hi Noralf,
On Sun, Oct 22, 2017 at 06:52:41PM +0200, Noralf Trønnes wrote:
Hi,
I've spent some time in the fbdev emulation code and discovered a recurring pattern around suspend/resume. Should we add some more helpers :-)
You're maybe a bit too good at spotting these for your own good :-)
But yeah, a "suspend for dummies" is one of the things which would be nice I think ... Especially since we now have the atomic suspend/resume helpers.
struct drm_device { /** * @suspend_state: * * Atomic state when suspended. * Set by drm_dev_suspend(), cleared by drm_dev_resume(). */ struct drm_atomic_state *suspend_state; };
Imo fits better when we put it into drm_mode_config.
int drm_dev_suspend(struct drm_device *dev) { struct drm_atomic_state *state;
if (!dev) return 0;
drm_kms_helper_poll_disable(dev); drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 1); state = drm_atomic_helper_suspend(dev); if (IS_ERR(state)) { drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0); drm_kms_helper_poll_enable(dev); return PTR_ERR(state); }
dev->suspend_state = state;
return 0; }
This is all about suspending the modeset side ... I'd give it a drm_mode_config prefix instead of the drm_dev_ (that's maybe a bit too generic), but then maybe type a general suspend/resume kernel-doc text in the drm-internals.rst (maybe pulled in from drm_dev.c) which references these 2 functions as the recommended way to suspend/resume the modeset side of a driver. These won't suspend/resume a render part (if present), so drm_dev_ seems a bit too much.
I just realised that this is pulling helpers (atomic, crtc, fbdev) into the core which IIRC is something that you didn't want?
Ugh right. I think starting a new drm_mode_config_helper.c for top-level helper stuff would be a reasonable solution. Or some other name if you have a better one.
Does it fit in drm_modeset_helper.c ?
/** * DOC: aux kms helpers * * This helper library contains various one-off functions which don't really fit * anywhere else in the DRM modeset helper library. */
Even better I think, avoids another file no one remembers exists :-) -Daniel