Hi
Am 18.02.26 um 15:29 schrieb Shenghao Yang:
Hi,
On 18/2/26 16:04, Thomas Zimmermann wrote:
Hi
Am 18.02.26 um 05:22 schrieb Shenghao Yang:
Hi,
Thanks for taking a look!
On 6/2/26 15:52, Thomas Zimmermann wrote:
+void gud_crtc_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *drm = crtc->dev;
+ struct gud_device *gdrm = to_gud_device(drm);
+ int idx;
+
+ if (!drm_dev_enter(drm, &idx))
+ return;
+
+ if (crtc->state->mode_changed || crtc->state->connectors_changed) {
I think you can do this unconditionally. Atomic_enable is supposed to be a full
modeset and take some time. For simple pageflips, this function doesn't run at
all.
I was trying this with proptest and toggling DPMS - the atomic_disable() and
atomic_enable() functions get called in that path even if the mode doesn't
change.
The driver in 6.12 LTS (before the atomic changes) didn't send
SET_CONTROLLER_ENABLE commands on DPMS changes either, so I thought it'd be
safer to do the same here.
Leave it as it is if you like. But in DRM, we don't have real DPMS support for
atomic modesetting. The DPMS on state is a full atomic_enable and the off and
blanking states are atomic_disable IIRC. Doing that would be the correct
handling.
Ah - gotcha. I'll do some testing with the conditional removed.
The worry about changing command sequences was a bit pedantic anyway -
DPMS would have caused an instant oops in the current tree and nobody else
has reported anything yet.
Would y'all be happy to take a v5 afterwards? Apologies for all the
iterations here :/
I'm OK with that. No worries.
Best regards
Thomas
Thanks,
Shenghao
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
