For non-CRTC displays, the drm_atomic_helper_connector_dpms will always set the connector back the old DPMS state before returning. This makes it impossible to change DPMS state.
fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd
Signed-off-by: Marta Lofstedt marta.lofstedt@intel.com --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 494680c..6a5acb9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, fail: if (ret == -EDEADLK) goto backoff; - - connector->dpms = old_mode; + if (ret != 0) + connector->dpms = old_mode; drm_atomic_state_put(state); return ret;
On Thu, Dec 01, 2016 at 03:30:01PM +0200, Marta Lofstedt wrote:
For non-CRTC displays,
A connector not attached to a CRTC works. It is the active connectors that were broken.
the drm_atomic_helper_connector_dpms will always set the connector back the old DPMS state before returning. This makes it impossible to change DPMS state.
fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd
Signed-off-by: Marta Lofstedt marta.lofstedt@intel.com
drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 494680c..6a5acb9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, fail: if (ret == -EDEADLK) goto backoff;
- connector->dpms = old_mode;
- if (ret != 0)
connector->dpms = old_mode;
Patch however is correct.
Fix the subject line to reflect the impact correctly, amend the fixes tag and add the appropriate CCs
Fixes: 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state") Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Eric Engestrom eric.engestrom@imgtec.com Cc: Sean Paul seanpaul@chromium.org Cc: dri-devel@lists.freedesktop.org Cc: drm-intel-fixes@lists.freedesktop.org
And mention if there was a testcase exercising the failure. If not please suggest one. -Chris
-----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Thursday, December 1, 2016 4:15 PM To: Lofstedt, Marta marta.lofstedt@intel.com Cc: dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm: allow changing DPMS mode for non-CRTC displays
On Thu, Dec 01, 2016 at 03:30:01PM +0200, Marta Lofstedt wrote:
For non-CRTC displays,
A connector not attached to a CRTC works. It is the active connectors that were broken.
the drm_atomic_helper_connector_dpms will always set the connector back the old DPMS state before returning. This makes it impossible to change DPMS state.
fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd
Signed-off-by: Marta Lofstedt marta.lofstedt@intel.com
drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 494680c..6a5acb9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, fail: if (ret == -EDEADLK) goto backoff;
- connector->dpms = old_mode;
- if (ret != 0)
connector->dpms = old_mode;Patch however is correct.
Fix the subject line to reflect the impact correctly, amend the fixes tag and add the appropriate CCs
Fixes: 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state") Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Eric Engestrom eric.engestrom@imgtec.com Cc: Sean Paul seanpaul@chromium.org Cc: dri-devel@lists.freedesktop.org Cc: drm-intel-fixes@lists.freedesktop.org
And mention if there was a testcase exercising the failure. If not please suggest one.
Thanks for the comments Chris.
About tests, I can't seem to find any test that is actually checking that we entered DPMS mode. The suggestion is to extend the kms_properties test, or maybe we should have a new dpms test. I CC Petri, so he can take part in such a discussion.
/Marta
-Chris
-- Chris Wilson, Intel Open Source Technology Centre
On Thursday, 2016-12-01 14:15:05 +0000, Chris Wilson wrote:
On Thu, Dec 01, 2016 at 03:30:01PM +0200, Marta Lofstedt wrote:
For non-CRTC displays,
A connector not attached to a CRTC works. It is the active connectors that were broken.
the drm_atomic_helper_connector_dpms will always set the connector back the old DPMS state before returning. This makes it impossible to change DPMS state.
fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd
Signed-off-by: Marta Lofstedt marta.lofstedt@intel.com
drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 494680c..6a5acb9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, fail: if (ret == -EDEADLK) goto backoff;
- connector->dpms = old_mode;
- if (ret != 0)
connector->dpms = old_mode;Patch however is correct.
Fix the subject line to reflect the impact correctly, amend the fixes tag and add the appropriate CCs
Fixes: 0853695c3ba4 ("drm: Add reference counting to drm_atomic_state") Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Eric Engestrom eric.engestrom@imgtec.com
Good catch! Reviewed-by: Eric Engestrom eric.engestrom@imgtec.com
I just had another look at the change (0853695c3ba4) and I don't believe there was any other unintended behaviour change.
Cc: Sean Paul seanpaul@chromium.org Cc: dri-devel@lists.freedesktop.org Cc: drm-intel-fixes@lists.freedesktop.org
And mention if there was a testcase exercising the failure. If not please suggest one. -Chris
-- Chris Wilson, Intel Open Source Technology Centre
The drm_atomic_helper_connector_dpms will set the connector back the old DPMS state before returning. This makes it impossible to change DPMS state of the connector.
Fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd v2: edit of commit message Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Eric Engestrom eric.engestrom@imgtec.com Cc: Sean Paul seanpaul@chromium.org Cc: dri-devel@lists.freedesktop.org Cc: drm-intel-fixes@lists.freedesktop.org Signed-off-by: Marta Lofstedt marta.lofstedt@intel.com --- drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 494680c..6a5acb9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, fail: if (ret == -EDEADLK) goto backoff; - - connector->dpms = old_mode; + if (ret != 0) + connector->dpms = old_mode; drm_atomic_state_put(state); return ret;
On Mon, Dec 05, 2016 at 02:04:08PM +0200, Marta Lofstedt wrote:
The drm_atomic_helper_connector_dpms will set the connector back the old DPMS state before returning. This makes it impossible to change DPMS state of the connector.
Fixes: 0853695c3ba46f97dfc0b5885f7b7e640ca212dd v2: edit of commit message Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Eric Engestrom eric.engestrom@imgtec.com Cc: Sean Paul seanpaul@chromium.org Cc: dri-devel@lists.freedesktop.org Cc: drm-intel-fixes@lists.freedesktop.org Signed-off-by: Marta Lofstedt marta.lofstedt@intel.com
Applied to drm-misc, thanks. -Daniel
drivers/gpu/drm/drm_atomic_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 494680c..6a5acb9 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2885,8 +2885,8 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, fail: if (ret == -EDEADLK) goto backoff;
- connector->dpms = old_mode;
- if (ret != 0)
connector->dpms = old_mode;-- 2.9.3
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Daniel Vetter -
Eric Engestrom -
Lofstedt, Marta -
Marta Lofstedt