| Message ID | 20170513105201.17658-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
subject s/Wait/Wake/ D'oh -Chris
On Sat, May 13, 2017 at 11:52:01AM +0100, Chris Wilson wrote: > As we can have multiple tx in the queue, with individual waiters, make > sure that all are woken when any state changes (so that we are sure the > right owner of the txmsg is woken). > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> I think in practice we need probe vs. the userspace dp aux interface (or multiple userspace apps beating on this), and on multiple different mst sinks, but better safe than sorry. Applied, thanks. -Daniel > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 3bdd314f02b1..222eb1a8549b 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -855,7 +855,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref *kref) > mutex_unlock(&mstb->mgr->qlock); > > if (wake_tx) > - wake_up(&mstb->mgr->tx_waitq); > + wake_up_all(&mstb->mgr->tx_waitq); > > kref_put(kref, drm_dp_free_mst_branch_device); > } > @@ -1510,7 +1510,7 @@ static void process_single_down_tx_qlock(struct drm_dp_mst_topology_mgr *mgr) > if (txmsg->seqno != -1) > txmsg->dst->tx_slots[txmsg->seqno] = NULL; > txmsg->state = DRM_DP_SIDEBAND_TX_TIMEOUT; > - wake_up(&mgr->tx_waitq); > + wake_up_all(&mgr->tx_waitq); > } > } > > @@ -2258,7 +2258,7 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) > mstb->tx_slots[slot] = NULL; > mutex_unlock(&mgr->qlock); > > - wake_up(&mgr->tx_waitq); > + wake_up_all(&mgr->tx_waitq); > } > return ret; > } > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, May 15, 2017 at 02:04:43PM +0200, Daniel Vetter wrote: > On Sat, May 13, 2017 at 11:52:01AM +0100, Chris Wilson wrote: > > As we can have multiple tx in the queue, with individual waiters, make > > sure that all are woken when any state changes (so that we are sure the > > right owner of the txmsg is woken). > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > I think in practice we need probe vs. the userspace dp aux interface (or > multiple userspace apps beating on this), and on multiple different mst > sinks, but better safe than sorry. Someone has to figure out what I did wrong in my remote DPCD aux_dev attempt before we can actually do that: [1] git://github.com/vsyrjala/linux.git dp_mst_port_aux_dev > > Applied, thanks. > -Daniel > > --- > > drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 3bdd314f02b1..222eb1a8549b 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -855,7 +855,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref *kref) > > mutex_unlock(&mstb->mgr->qlock); > > > > if (wake_tx) > > - wake_up(&mstb->mgr->tx_waitq); > > + wake_up_all(&mstb->mgr->tx_waitq); > > > > kref_put(kref, drm_dp_free_mst_branch_device); > > } > > @@ -1510,7 +1510,7 @@ static void process_single_down_tx_qlock(struct drm_dp_mst_topology_mgr *mgr) > > if (txmsg->seqno != -1) > > txmsg->dst->tx_slots[txmsg->seqno] = NULL; > > txmsg->state = DRM_DP_SIDEBAND_TX_TIMEOUT; > > - wake_up(&mgr->tx_waitq); > > + wake_up_all(&mgr->tx_waitq); > > } > > } > > > > @@ -2258,7 +2258,7 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) > > mstb->tx_slots[slot] = NULL; > > mutex_unlock(&mgr->qlock); > > > > - wake_up(&mgr->tx_waitq); > > + wake_up_all(&mgr->tx_waitq); > > } > > return ret; > > } > > -- > > 2.11.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, May 15, 2017 at 03:33:12PM +0300, Ville Syrjälä wrote: > On Mon, May 15, 2017 at 02:04:43PM +0200, Daniel Vetter wrote: > > On Sat, May 13, 2017 at 11:52:01AM +0100, Chris Wilson wrote: > > > As we can have multiple tx in the queue, with individual waiters, make > > > sure that all are woken when any state changes (so that we are sure the > > > right owner of the txmsg is woken). > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > I think in practice we need probe vs. the userspace dp aux interface (or > > multiple userspace apps beating on this), and on multiple different mst > > sinks, but better safe than sorry. > > Someone has to figure out what I did wrong in my remote DPCD aux_dev > attempt before we can actually do that: > > [1] git://github.com/vsyrjala/linux.git dp_mst_port_aux_dev Oh, I didn't realize we don't register the full dp aux for remotes. But we do register the i2c for remotes, and that's good enough to blow up. i2c dev nodes is also more likely to be used by userspace for real (through the ddc tool). -Daniel
On Mon, 15 May 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, May 15, 2017 at 03:33:12PM +0300, Ville Syrjälä wrote: >> On Mon, May 15, 2017 at 02:04:43PM +0200, Daniel Vetter wrote: >> > On Sat, May 13, 2017 at 11:52:01AM +0100, Chris Wilson wrote: >> > > As we can have multiple tx in the queue, with individual waiters, make >> > > sure that all are woken when any state changes (so that we are sure the >> > > right owner of the txmsg is woken). >> > > >> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > >> > I think in practice we need probe vs. the userspace dp aux interface (or >> > multiple userspace apps beating on this), and on multiple different mst >> > sinks, but better safe than sorry. >> >> Someone has to figure out what I did wrong in my remote DPCD aux_dev >> attempt before we can actually do that: >> >> [1] git://github.com/vsyrjala/linux.git dp_mst_port_aux_dev > > Oh, I didn't realize we don't register the full dp aux for remotes. But we > do register the i2c for remotes, and that's good enough to blow up. i2c > dev nodes is also more likely to be used by userspace for real (through > the ddc tool). Related https://bugs.freedesktop.org/show_bug.cgi?id=100954 BR, Jani.
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 3bdd314f02b1..222eb1a8549b 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -855,7 +855,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref *kref) mutex_unlock(&mstb->mgr->qlock); if (wake_tx) - wake_up(&mstb->mgr->tx_waitq); + wake_up_all(&mstb->mgr->tx_waitq); kref_put(kref, drm_dp_free_mst_branch_device); } @@ -1510,7 +1510,7 @@ static void process_single_down_tx_qlock(struct drm_dp_mst_topology_mgr *mgr) if (txmsg->seqno != -1) txmsg->dst->tx_slots[txmsg->seqno] = NULL; txmsg->state = DRM_DP_SIDEBAND_TX_TIMEOUT; - wake_up(&mgr->tx_waitq); + wake_up_all(&mgr->tx_waitq); } } @@ -2258,7 +2258,7 @@ static int drm_dp_mst_handle_down_rep(struct drm_dp_mst_topology_mgr *mgr) mstb->tx_slots[slot] = NULL; mutex_unlock(&mgr->qlock); - wake_up(&mgr->tx_waitq); + wake_up_all(&mgr->tx_waitq); } return ret; }
As we can have multiple tx in the queue, with individual waiters, make sure that all are woken when any state changes (so that we are sure the right owner of the txmsg is woken). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)