Re: [Intel-gfx] [PATCH 4/4] drm/atomic-helpers: Make encoder picking more robust

2015-08-04 Thread Ander Conselvan De Oliveira
For the series:

Reviewed-by: Ander Conselvan de Oliveira conselv...@gmail.com


On Mon, 2015-08-03 at 17:24 +0200, Daniel Vetter wrote:
 We've had a few issues with atomic where subtle bugs in the encoder
 picking logic lead to accidental self-stealing of the encoder,
 resulting in a NULL connector_state-crtc in update_connector_routing
 and subsequent.
 
 Linus applied some duct-tape for an mst regression in
 
 commit 27667f4744fc5a0f3e50910e78740bac5670d18b
 Author: Linus Torvalds torva...@linux-foundation.org
 Date:   Wed Jul 29 22:18:16 2015 -0700
 
 i915: temporary fix for DP MST docking station NULL pointer dereference
 
 But that was incomplete (the code will still oops when debuggin is
 enabled) and mangled the state even further. So instead WARN and bail
 out as the more future-proof option.
 
 Cc: Theodore Ts'o ty...@mit.edu
 Cc: Linus Torvalds torva...@linux-foundation.org
 Signed-off-by: Daniel Vetter daniel.vet...@intel.com
 ---
  drivers/gpu/drm/drm_atomic_helper.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
 b/drivers/gpu/drm/drm_atomic_helper.c
 index 8694ca9beee3..9dcc7280e572 100644
 --- a/drivers/gpu/drm/drm_atomic_helper.c
 +++ b/drivers/gpu/drm/drm_atomic_helper.c
 @@ -234,13 +234,14 @@ update_connector_routing(struct drm_atomic_state 
 *state, int conn_idx)
   }
   }
  
 + if (WARN_ON(!connector_state-crtc))
 + return -EINVAL;
 +
   connector_state-best_encoder = new_encoder;
 - if (connector_state-crtc) {
 - idx = drm_crtc_index(connector_state-crtc);
 + idx = drm_crtc_index(connector_state-crtc);
  
 - crtc_state = state-crtc_states[idx];
 - crtc_state-mode_changed = true;
 - }
 + crtc_state = state-crtc_states[idx];
 + crtc_state-mode_changed = true;
  
   DRM_DEBUG_ATOMIC([CONNECTOR:%d:%s] using [ENCODER:%d:%s] on 
 [CRTC:%d]\n,
connector-base.id,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/4] drm/atomic-helpers: Make encoder picking more robust

2015-08-04 Thread Thierry Reding
On Mon, Aug 03, 2015 at 05:24:11PM +0200, Daniel Vetter wrote:
 We've had a few issues with atomic where subtle bugs in the encoder
 picking logic lead to accidental self-stealing of the encoder,
 resulting in a NULL connector_state-crtc in update_connector_routing
 and subsequent.
 
 Linus applied some duct-tape for an mst regression in
 
 commit 27667f4744fc5a0f3e50910e78740bac5670d18b
 Author: Linus Torvalds torva...@linux-foundation.org
 Date:   Wed Jul 29 22:18:16 2015 -0700
 
 i915: temporary fix for DP MST docking station NULL pointer dereference
 
 But that was incomplete (the code will still oops when debuggin is
 enabled) and mangled the state even further. So instead WARN and bail
 out as the more future-proof option.
 
 Cc: Theodore Ts'o ty...@mit.edu
 Cc: Linus Torvalds torva...@linux-foundation.org
 Signed-off-by: Daniel Vetter daniel.vet...@intel.com
 ---
  drivers/gpu/drm/drm_atomic_helper.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
 b/drivers/gpu/drm/drm_atomic_helper.c
 index 8694ca9beee3..9dcc7280e572 100644
 --- a/drivers/gpu/drm/drm_atomic_helper.c
 +++ b/drivers/gpu/drm/drm_atomic_helper.c
 @@ -234,13 +234,14 @@ update_connector_routing(struct drm_atomic_state 
 *state, int conn_idx)
   }
   }
  
 + if (WARN_ON(!connector_state-crtc))
 + return -EINVAL;
 +
   connector_state-best_encoder = new_encoder;
 - if (connector_state-crtc) {
 - idx = drm_crtc_index(connector_state-crtc);
 + idx = drm_crtc_index(connector_state-crtc);
  
 - crtc_state = state-crtc_states[idx];
 - crtc_state-mode_changed = true;
 - }
 + crtc_state = state-crtc_states[idx];
 + crtc_state-mode_changed = true;
  
   DRM_DEBUG_ATOMIC([CONNECTOR:%d:%s] using [ENCODER:%d:%s] on 
 [CRTC:%d]\n,
connector-base.id,

Reviewed-by: Thierry Reding tred...@nvidia.com


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/4] drm/atomic-helpers: Make encoder picking more robust

2015-08-04 Thread Daniel Vetter
On Tue, Aug 04, 2015 at 11:56:08AM +0300, Ander Conselvan De Oliveira wrote:
 For the series:
 
 Reviewed-by: Ander Conselvan de Oliveira conselv...@gmail.com

Thanks for the review.
-Daniel
 
 
 On Mon, 2015-08-03 at 17:24 +0200, Daniel Vetter wrote:
  We've had a few issues with atomic where subtle bugs in the encoder
  picking logic lead to accidental self-stealing of the encoder,
  resulting in a NULL connector_state-crtc in update_connector_routing
  and subsequent.
  
  Linus applied some duct-tape for an mst regression in
  
  commit 27667f4744fc5a0f3e50910e78740bac5670d18b
  Author: Linus Torvalds torva...@linux-foundation.org
  Date:   Wed Jul 29 22:18:16 2015 -0700
  
  i915: temporary fix for DP MST docking station NULL pointer dereference
  
  But that was incomplete (the code will still oops when debuggin is
  enabled) and mangled the state even further. So instead WARN and bail
  out as the more future-proof option.
  
  Cc: Theodore Ts'o ty...@mit.edu
  Cc: Linus Torvalds torva...@linux-foundation.org
  Signed-off-by: Daniel Vetter daniel.vet...@intel.com
  ---
   drivers/gpu/drm/drm_atomic_helper.c | 11 ++-
   1 file changed, 6 insertions(+), 5 deletions(-)
  
  diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
  b/drivers/gpu/drm/drm_atomic_helper.c
  index 8694ca9beee3..9dcc7280e572 100644
  --- a/drivers/gpu/drm/drm_atomic_helper.c
  +++ b/drivers/gpu/drm/drm_atomic_helper.c
  @@ -234,13 +234,14 @@ update_connector_routing(struct drm_atomic_state 
  *state, int conn_idx)
  }
  }
   
  +   if (WARN_ON(!connector_state-crtc))
  +   return -EINVAL;
  +
  connector_state-best_encoder = new_encoder;
  -   if (connector_state-crtc) {
  -   idx = drm_crtc_index(connector_state-crtc);
  +   idx = drm_crtc_index(connector_state-crtc);
   
  -   crtc_state = state-crtc_states[idx];
  -   crtc_state-mode_changed = true;
  -   }
  +   crtc_state = state-crtc_states[idx];
  +   crtc_state-mode_changed = true;
   
  DRM_DEBUG_ATOMIC([CONNECTOR:%d:%s] using [ENCODER:%d:%s] on 
  [CRTC:%d]\n,
   connector-base.id,

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 4/4] drm/atomic-helpers: Make encoder picking more robust

2015-08-03 Thread Daniel Vetter
We've had a few issues with atomic where subtle bugs in the encoder
picking logic lead to accidental self-stealing of the encoder,
resulting in a NULL connector_state-crtc in update_connector_routing
and subsequent.

Linus applied some duct-tape for an mst regression in

commit 27667f4744fc5a0f3e50910e78740bac5670d18b
Author: Linus Torvalds torva...@linux-foundation.org
Date:   Wed Jul 29 22:18:16 2015 -0700

i915: temporary fix for DP MST docking station NULL pointer dereference

But that was incomplete (the code will still oops when debuggin is
enabled) and mangled the state even further. So instead WARN and bail
out as the more future-proof option.

Cc: Theodore Ts'o ty...@mit.edu
Cc: Linus Torvalds torva...@linux-foundation.org
Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 drivers/gpu/drm/drm_atomic_helper.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 8694ca9beee3..9dcc7280e572 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -234,13 +234,14 @@ update_connector_routing(struct drm_atomic_state *state, 
int conn_idx)
}
}
 
+   if (WARN_ON(!connector_state-crtc))
+   return -EINVAL;
+
connector_state-best_encoder = new_encoder;
-   if (connector_state-crtc) {
-   idx = drm_crtc_index(connector_state-crtc);
+   idx = drm_crtc_index(connector_state-crtc);
 
-   crtc_state = state-crtc_states[idx];
-   crtc_state-mode_changed = true;
-   }
+   crtc_state = state-crtc_states[idx];
+   crtc_state-mode_changed = true;
 
DRM_DEBUG_ATOMIC([CONNECTOR:%d:%s] using [ENCODER:%d:%s] on 
[CRTC:%d]\n,
 connector-base.id,
-- 
2.5.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx