| Message ID | 20181026203549.1796-4-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/dp_mst: Improve VCPI helpers, use in nouveau | expand |
On Fri, Oct 26, 2018 at 04:35:48PM -0400, Lyude Paul wrote: > It occurred to me that we never actually check this! So let's start > doing that. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> One thought on testing all this: I think long term some unti tests, where we have a fake driver doing fake mst branch/ports and a bunch of allocations and then checking that it all works and validates would be nice. Longer term project ofc, and maybe after Kunit has been merged ... -Daniel > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index dcfab7536914..8bb03700e199 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -3238,7 +3238,7 @@ int drm_dp_mst_atomic_check(struct drm_dp_mst_topology_state *state) > { > struct drm_dp_mst_topology_mgr *mgr = state->mgr; > struct drm_dp_vcpi_allocation *pos; > - int avail_slots = 63; > + int avail_slots = 63, payload_count = 0; > > list_for_each_entry(pos, &state->vcpis, next) { > DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", > @@ -3251,6 +3251,12 @@ int drm_dp_mst_atomic_check(struct drm_dp_mst_topology_state *state) > avail_slots + pos->vcpi); > return -ENOSPC; > } > + > + if (++payload_count > mgr->max_payloads) { > + DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many payloads (max=%d)\n", > + mgr, state, mgr->max_payloads); > + return -EINVAL; > + } > } > DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n", > mgr, state, avail_slots, 63 - avail_slots); > -- > 2.17.2 >
On Mon, 2018-10-29 at 15:25 +0100, Daniel Vetter wrote: > On Fri, Oct 26, 2018 at 04:35:48PM -0400, Lyude Paul wrote: > > It occurred to me that we never actually check this! So let's start > > doing that. > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > One thought on testing all this: I think long term some unti tests, where > we have a fake driver doing fake mst branch/ports and a bunch of > allocations and then checking that it all works and validates would be > nice. Longer term project ofc, and maybe after Kunit has been merged ... mhm-you see why I was considering how we could use vkms with this :), but yeah getting unit tests is one of the reasons I'm hoping to move some more MST logic out into the DRM helpers > -Daniel > > > --- > > drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > index dcfab7536914..8bb03700e199 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -3238,7 +3238,7 @@ int drm_dp_mst_atomic_check(struct > > drm_dp_mst_topology_state *state) > > { > > struct drm_dp_mst_topology_mgr *mgr = state->mgr; > > struct drm_dp_vcpi_allocation *pos; > > - int avail_slots = 63; > > + int avail_slots = 63, payload_count = 0; > > > > list_for_each_entry(pos, &state->vcpis, next) { > > DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", > > @@ -3251,6 +3251,12 @@ int drm_dp_mst_atomic_check(struct > > drm_dp_mst_topology_state *state) > > avail_slots + pos->vcpi); > > return -ENOSPC; > > } > > + > > + if (++payload_count > mgr->max_payloads) { > > + DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many > > payloads (max=%d)\n", > > + mgr, state, mgr->max_payloads); > > + return -EINVAL; > > + } > > } > > DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n", > > mgr, state, avail_slots, 63 - avail_slots); > > -- > > 2.17.2 > > > >
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index dcfab7536914..8bb03700e199 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3238,7 +3238,7 @@ int drm_dp_mst_atomic_check(struct drm_dp_mst_topology_state *state) { struct drm_dp_mst_topology_mgr *mgr = state->mgr; struct drm_dp_vcpi_allocation *pos; - int avail_slots = 63; + int avail_slots = 63, payload_count = 0; list_for_each_entry(pos, &state->vcpis, next) { DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", @@ -3251,6 +3251,12 @@ int drm_dp_mst_atomic_check(struct drm_dp_mst_topology_state *state) avail_slots + pos->vcpi); return -ENOSPC; } + + if (++payload_count > mgr->max_payloads) { + DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many payloads (max=%d)\n", + mgr, state, mgr->max_payloads); + return -EINVAL; + } } DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n", mgr, state, avail_slots, 63 - avail_slots);
It occurred to me that we never actually check this! So let's start doing that. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)