Re: [Intel-gfx] [PATCH 4/4] drm/atomic-helpers: Make encoder picking more robust
On Tue, Aug 04, 2015 at 11:56:08AM +0300, Ander Conselvan De Oliveira wrote: > For the series: > > Reviewed-by: Ander Conselvan de Oliveira 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 > > 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 > > Cc: Linus Torvalds > > Signed-off-by: Daniel Vetter > > --- > > 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
Re: [Intel-gfx] [PATCH 4/4] drm/atomic-helpers: Make encoder picking more robust
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 > 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 > Cc: Linus Torvalds > Signed-off-by: Daniel Vetter > --- > 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 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
For the series: Reviewed-by: Ander Conselvan de Oliveira 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 > 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 > Cc: Linus Torvalds > Signed-off-by: Daniel Vetter > --- > 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