[Intel-gfx] [PATCH v2 9/9] drm/dp: Track MST link bandwidth

[Daniel Vetter]

On Wed, Jan 25, 2017 at 09:00:02PM +0000, Pandiyan, Dhinakaran wrote:
> On Wed, 2017-01-25 at 07:15 +0100, Daniel Vetter wrote:
> > On Tue, Jan 24, 2017 at 03:49:37PM -0800, Dhinakaran Pandiyan wrote:
> > > Use the added helpers to track MST link bandwidth for atomic modesets.
> > > Link bw is acquired in the ->atomic_check() phase when CRTCs are being
> > > enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().
> > > Similarly, link bw is released during ->atomic_check() with the connector
> > > helper callback ->atomic_release() when CRTCs are disabled.
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c |  9 ++++++++-
> > >  drivers/gpu/drm/i915/intel_dp_mst.c | 13 ++++++++++++-
> > >  2 files changed, 20 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > index dd34d21..0d42173 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -385,8 +385,15 @@ mode_fixup(struct drm_atomic_state *state)
> > >  
> > >  		WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc);
> > >  
> > > -		if (!conn_state->crtc || !conn_state->best_encoder)
> > > +		if (!conn_state->crtc || !conn_state->best_encoder) {
> > > +			const struct drm_connector_helper_funcs *conn_funcs;
> > > +
> > > +			conn_funcs = connector->helper_private;
> > > +			if (conn_funcs->atomic_release)
> > > +				conn_funcs->atomic_release(connector,
> > > +							   conn_state);
> > 
> > I wonder whether this won't surprise drivers: The way you coded this
> > release only gets called when the connector is fully disabled. But it does
> > not get called when you atomically switch it from one crtc to the other
> > (in which case you also might want to release resources, before allocating
> > new ones). I think that case of directly switching stuff is even a bug in
> > your current atomic tracking code ...
> > 
> > We also need to extract the release calls into a separate loop which runs
> > _before_ we allocate any resources. Otherwise if you do an atomic commit
> > where you disable connector2 and enable connector1 and they can't run both
> > at the same time it'll fail, because we'll release the vcpi for connector2
> > only after we've tried to get it for connnector1.
> > 
> 
> 
> Yeah, you are right. I thought of this problem and then forgot about it.
> Is there any igt test to test the switching?

Not sure we have any direct mode-switching tests. Please chat with Mika
Kahola and make sure we do have that test coverage. Mika is working on
atomic test coverage, and having this would be good in general (not just
for mst).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch