On Wed, Feb 19, 2025 at 06:02:39PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrj...@linux.intel.com> > > Video players (eg. mpv) do periodic XResetScreenSaver() calls to > keep the screen on while the video playing. The modesetting ddx > plumbs these straight through into the kernel as DPMS setproperty > ioctls, without any filtering whatsoever. When implemented via > atomic these end up as full commits on the crtc, which leads to a > dropped frame every time XResetScreenSaver() is called.
I think you should add here that it's just an empty commit, because we do filter out redundant commits where crtc->active_changed does nothing. Except we still run the entire machinery with timestamps and drm_event and everything. And I don't think it's worth to filter that out at the atomic level, because it's really only legacy ioctl that had this "complete noop" behaviour. With the commit message augmented: Reviewed-by: Simona Vetter <simona.vet...@ffwll.ch> Might also be nice to have a igt for this? Plus also wondering whether we should cc: stable it. Cheers, Sima > Let's just filter out redundant DPMS property changes in the > kernel to avoid this issue. > > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > --- > drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index 2765ba90ad8f..c2726af6698e 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -957,6 +957,10 @@ int drm_atomic_connector_commit_dpms(struct > drm_atomic_state *state, > > if (mode != DRM_MODE_DPMS_ON) > mode = DRM_MODE_DPMS_OFF; > + > + if (connector->dpms == mode) > + goto out; > + > connector->dpms = mode; > > crtc = connector->state->crtc; > -- > 2.45.3 > -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
