| Message ID | 1385583848-2805-1-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Nov 27, 2013 at 06:24:08PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Sometimes we want to disable all the screens on a system, because that > will allow the graphics card to be put into low-power states. The > problem is that, for example, while all screens are disabled, if we > get a hotplug interrupt, fbcon will decide to set a mode instead of > keeping everything disabled, which will remove us from our low power > states. > > Let's assume that if there's a DRM master, it will be able to do > whatever is appropriate when we get the hotplug. > > This problem can be reproduced by the runtime PM test program from > intel-gpu-tools: we disable all the screens so the graphics device can > be put into D3, then something triggers a hotplug interrupt, fbcon > sets a mode and breaks our test suite. The problem can be reproduced > more easily by the "i2c" subtest. > > Other approaches considered for the problem: > - Return "false" if "bound == 0" and the caller of > drm_fb_helper_is_bound is a hotplug handler. This would break > the case where the machine boots with no outputs connected, then > the user plugs a monitor. > - Add a new IOCTL to force fbcon to not set modes. This would keep > all the current applications behaving the same, but adding a new > IOCTL is not always the greatest idea. > - Return false only if "dev->primary->master && bound == 0". This > was my first implementation, but Chris suggested we should do > the check irrespective of the "bound" variable. > > Thanks to Daniel Vetter for the investigation, ideas and the > implementation of the hotplug alternative. > > v2: - Do the check first, irrespective of "bound". > - Cc dri-devel > > Cc: dri-devel@lists.freedesktop.org > Credits-to: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_fb_helper.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 0a19401..98a0363 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -359,6 +359,11 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) > struct drm_crtc *crtc; > int bound = 0, crtcs_bound = 0; > > + /* Sometimes user space wants everything disabled, so don't steal the > + * display if there's a master. */ > + if (dev->primary->master) > + return false; > + > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > if (crtc->fb) > crtcs_bound++; > @@ -368,6 +373,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) > > if (bound < crtcs_bound) > return false; > + > return true; > } > > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi On Wed, Nov 27, 2013 at 9:24 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Sometimes we want to disable all the screens on a system, because that > will allow the graphics card to be put into low-power states. The > problem is that, for example, while all screens are disabled, if we > get a hotplug interrupt, fbcon will decide to set a mode instead of > keeping everything disabled, which will remove us from our low power > states. > > Let's assume that if there's a DRM master, it will be able to do > whatever is appropriate when we get the hotplug. > > This problem can be reproduced by the runtime PM test program from > intel-gpu-tools: we disable all the screens so the graphics device can > be put into D3, then something triggers a hotplug interrupt, fbcon > sets a mode and breaks our test suite. The problem can be reproduced > more easily by the "i2c" subtest. > > Other approaches considered for the problem: > - Return "false" if "bound == 0" and the caller of > drm_fb_helper_is_bound is a hotplug handler. This would break > the case where the machine boots with no outputs connected, then > the user plugs a monitor. > - Add a new IOCTL to force fbcon to not set modes. This would keep > all the current applications behaving the same, but adding a new > IOCTL is not always the greatest idea. > - Return false only if "dev->primary->master && bound == 0". This > was my first implementation, but Chris suggested we should do > the check irrespective of the "bound" variable. > > Thanks to Daniel Vetter for the investigation, ideas and the > implementation of the hotplug alternative. > > v2: - Do the check first, irrespective of "bound". > - Cc dri-devel Thank god all that will get dropped with CONFIG_FB=n. Anyhow: Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks David > Cc: dri-devel@lists.freedesktop.org > Credits-to: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/drm_fb_helper.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 0a19401..98a0363 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -359,6 +359,11 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) > struct drm_crtc *crtc; > int bound = 0, crtcs_bound = 0; > > + /* Sometimes user space wants everything disabled, so don't steal the > + * display if there's a master. */ > + if (dev->primary->master) > + return false; > + > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > if (crtc->fb) > crtcs_bound++; > @@ -368,6 +373,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) > > if (bound < crtcs_bound) > return false; > + > return true; > } > > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0a19401..98a0363 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -359,6 +359,11 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) struct drm_crtc *crtc; int bound = 0, crtcs_bound = 0; + /* Sometimes user space wants everything disabled, so don't steal the + * display if there's a master. */ + if (dev->primary->master) + return false; + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { if (crtc->fb) crtcs_bound++; @@ -368,6 +373,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper) if (bound < crtcs_bound) return false; + return true; }