2017-09-27 14:58 GMT+02:00 Jani Nikula <[email protected]>:
> On Wed, 27 Sep 2017, Daniel Vetter <[email protected]> wrote: > > On Wed, Sep 27, 2017 at 12:20:21PM +0200, Karsten Wiese wrote: > >> 2017-09-27 9:18 GMT+02:00 Jani Nikula <[email protected]>: > >> > >> > On Wed, 27 Sep 2017, Karsten Wiese <[email protected]> wrote: > >> > > 2017-09-25 15:48 GMT+02:00 Jani Nikula <[email protected] > >: > >> > > > >> > >> On Tue, 19 Sep 2017, Karsten Wiese <[email protected]> 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. > > For eDP/LVDS in Intel driver yes, for external displays not. For > external displays DDC/CI is currently probably the best bet. > > However, DDC/CI is overall not very robust. I think we have prior bug > reports about it, but it's never really been very high on our list of > priorities. > > > 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. > > It's going to take forever and a half before DDC/CI for external > displays get that treatment, if ever. > > It still feels wrong to add sysfs notify support for just one of our > properties, for just this one use case. In particular if the reason is > to workaround bugs in another interface. > Would you accept a patch supporting all relevant attributes? BR, Karsten > > BR, > Jani. > > > > -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 > >> v4l2 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 patch 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 <[email protected]> > >> > >> > --- > >> > >> > 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 > >> [email protected] > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center >
_______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
