[PATCH] drm: Call sysfs_notify after changing drm_connector::dpms

[Daniel Vetter]

On Wed, Sep 27, 2017 at 12:20:21PM +0200, Karsten Wiese wrote:
> 2017-09-27 9:18 GMT+02:00 Jani Nikula <jani.nikula at linux.intel.com>:
> 
> > On Wed, 27 Sep 2017, Karsten Wiese <fzuuzf at googlemail.com> wrote:
> > > 2017-09-25 15:48 GMT+02:00 Jani Nikula <jani.nikula at linux.intel.com>:
> > >
> > >> On Tue, 19 Sep 2017, Karsten Wiese <fzuuzf at googlemail.com> wrote:
> > >> > This makes poll work for the
> > >> > /sys/class/drm/cardX/connectorY/dpms attributes.
> > >>
> > >> I guess the question is, what are you trying to achieve? What is the
> > >> problem that this solves?
> > >>
> > > ​
> > > The patch enables cpu cycle less waiting for dpms flag changes.
> > >
> > > Here it lets a screen brightness ​setting daemon know which monitor to
> > > handle.  One of the attached screens gets confused if it is set just
> > > after being switched on, hence the daemon leaves it untouched until it
> > > has been active for some seconds.
> >
> > Screen brightness settings daemon?
> >
> Running on a laptop lacking an ambient light sensor employing it's webcam
> instead to measure ambient light and adjust monitors' brightnesses
> accordingly.
> 
> 
> What exactly do you mean by "if it is
> > set"? What interface are you using to change brightness?
> 
> The external monitor's brightness is adjusted by DDC/CI via the /dev/i2c-x
> the monitor is attached to.
>  I there a saner interface to use?

There's supposed to be a backlight property on the panel, exposed by the X
driver. If there's not, then that needs to be fixed/adjusted.

There's also plans to directly link that up on the kernel side, but that's
a bit more involved due to how screwed up the backlight driver design on
linux is right now.
-Daniel

> 
> What happens
> > when the display "gets confused"?
> >
> The monitor wrongly toggles sharpness if it receives the brightness
> adjusting​
> ​ DDC/CI soon after resuming from power saving state.
> 
> >
> > My first instinct is that you're proposing a new kernel ABI to solve an
> > issue you shouldn't be solving in userspace to begin with.
> 
> Calculating ambient light from pictures acquired via ​
> ​v4l​2 in kernel seamed wrong to me.
> 
> Or that
> > perhaps the userspace should be doing this in cooperation with the drm
> > master, not standalone.
> >
> Is there a way to call into the drm-master (xscreensaver/xserver​ here​
> ​) ​with the call only returning if a monitor's power state changed?
> 
> There is DPMSInfo, but it returns immediately rendering the daemon less
> efficient.
> 
> 
> BR,
> 
> Karsten
> 
> >
> > BR,
> > Jani.
> >
> > >
> > > Without the p​atch the daemon would have to actively read the dpms flag
> > > every once in a while.
> > >
> > >
> > >>
> > >> We have zero sysfs_notify in all of drm AFAICT.
> > >
> > >
> > > Yes I noticed too and looked for some dbus signal to listen to​ but
> > didn't
> > > find any.
> > >
> > > BR,
> > > Karsten
> > >
> > >>
> > >> BR,
> > >> Jani.
> > >>
> > >>
> > >> >
> > >> > Tested with i915 suspended by XScreenServer and
> > >> > suspend to RAM.
> > >> >
> > >> > Signed-off-by: Karsten Wiese <fzuuzf at googlemail.com>
> > >> > ---
> > >> >  drivers/gpu/drm/drm_atomic.c        | 4 ++++
> > >> >  drivers/gpu/drm/drm_atomic_helper.c | 6 +++++-
> > >> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/drm_atomic.c
> > b/drivers/gpu/drm/drm_atomic.c
> > >> > index 2fd383d..b6fa87b 100644
> > >> > --- a/drivers/gpu/drm/drm_atomic.c
> > >> > +++ b/drivers/gpu/drm/drm_atomic.c
> > >> > @@ -1880,6 +1880,10 @@ int drm_atomic_connector_commit_dpms(struct
> > >> drm_atomic_state *state,
> > >> >  out:
> > >> >       if (ret != 0)
> > >> >               connector->dpms = old_mode;
> > >> > +     else
> > >> > +             if (connector->dpms != old_mode)
> > >> > +                     sysfs_notify(&connector->kdev->kobj, NULL,
> > >> "dpms");
> > >> > +
> > >> >       return ret;
> > >> >  }
> > >> >
> > >> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > >> b/drivers/gpu/drm/drm_atomic_helper.c
> > >> > index 4e53aae..6198772 100644
> > >> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > >> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > >> > @@ -921,12 +921,16 @@ drm_atomic_helper_update_
> > legacy_modeset_state(struct
> > >> drm_device *dev,
> > >> >               crtc = new_conn_state->crtc;
> > >> >               if ((!crtc && old_conn_state->crtc) ||
> > >> >                   (crtc && drm_atomic_crtc_needs_modeset(
> > crtc->state)))
> > >> {
> > >> > -                     int mode = DRM_MODE_DPMS_OFF;
> > >> > +                     int old_mode, mode = DRM_MODE_DPMS_OFF;
> > >> >
> > >> >                       if (crtc && crtc->state->active)
> > >> >                               mode = DRM_MODE_DPMS_ON;
> > >> >
> > >> > +                     old_mode = connector->dpms;
> > >> >                       connector->dpms = mode;
> > >> > +                     if (old_mode != mode)
> > >> > +                             sysfs_notify(&connector->kdev->kobj,
> > >> > +                                             NULL, "dpms");
> > >> >               }
> > >> >       }
> > >>
> > >> --
> > >> Jani Nikula, Intel Open Source Technology Center
> > >>
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
> >

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


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch