| Message ID | 1489648231-30700-9-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 16-03-17 om 08:10 schreef Dhinakaran Pandiyan: > From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> > > 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. > > v2: > Squashed atomic_release() implementation and caller (Daniel) > Fixed logic for connector-crtc switching case (Daniel) > Fixed logic for suspend-resume case. > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Archit Taneja <architt@codeaurora.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Harry Wentland <Harry.wentland@amd.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/i915/intel_dp_mst.c | 38 ++++++++++++++++++++++++++++++------- > 1 file changed, 31 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > index c1f62eb..a8f40fa 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > struct intel_dp *intel_dp = &intel_dig_port->dp; > struct intel_connector *connector = > to_intel_connector(conn_state->connector); > - struct drm_atomic_state *state; > + struct drm_atomic_state *state = pipe_config->base.state; > int bpp; > - int lane_count, slots; > + int lane_count, slots = 0; > const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > int mst_pbn; > > @@ -57,30 +57,53 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > * seem to suggest we should do otherwise. > */ > lane_count = drm_dp_max_lane_count(intel_dp->dpcd); > - > pipe_config->lane_count = lane_count; > > pipe_config->pipe_bpp = bpp; > pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); > > - state = pipe_config->base.state; > - > if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port)) > pipe_config->has_audio = true; > - mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); > > + mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); > pipe_config->pbn = mst_pbn; > - slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn); > > intel_link_compute_m_n(bpp, lane_count, > adjusted_mode->crtc_clock, > pipe_config->port_clock, > &pipe_config->dp_m_n); > > + if (pipe_config->base.active) { > + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, > + connector->port, mst_pbn); > + if (slots < 0) { > + DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots); > + return false; > + } > + } > pipe_config->dp_m_n.tu = slots; > > return true; > +} > + > +static void intel_dp_mst_atomic_release(struct drm_connector *connector, > + struct drm_connector_state *conn_state) > +{ > + struct intel_dp_mst_encoder *intel_mst; > + struct drm_dp_mst_topology_mgr *mgr; > + struct drm_encoder *encoder; > + struct intel_connector *intel_connector = to_intel_connector(connector); > + struct drm_atomic_state *state = conn_state->state; > + int slots; > + > + encoder = connector->state->best_encoder; > + intel_mst = enc_to_mst(encoder); > + mgr = &intel_mst->primary->dp.mst_mgr; > > + slots = drm_dp_atomic_release_vcpi_slots(state, mgr, > + intel_connector->port); > + if (slots < 0) > + DRM_DEBUG_KMS("failed releasing vcpi slots:%d\n", slots); > } > > static void intel_mst_disable_dp(struct intel_encoder *encoder, > @@ -387,6 +410,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun > .mode_valid = intel_dp_mst_mode_valid, > .atomic_best_encoder = intel_mst_atomic_best_encoder, > .best_encoder = intel_mst_best_encoder, > + .atomic_release = intel_dp_mst_atomic_release, > }; > > static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder) Is there any issue into attempting to release vcpi slots when they're already released? If not, for patches 1-3 5-8 Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
On Wed, 2017-03-22 at 13:30 +0100, Maarten Lankhorst wrote: > Op 16-03-17 om 08:10 schreef Dhinakaran Pandiyan: > > From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> > > > > 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. > > > > v2: > > Squashed atomic_release() implementation and caller (Daniel) > > Fixed logic for connector-crtc switching case (Daniel) > > Fixed logic for suspend-resume case. > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Archit Taneja <architt@codeaurora.org> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Harry Wentland <Harry.wentland@amd.com> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp_mst.c | 38 ++++++++++++++++++++++++++++++------- > > 1 file changed, 31 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > > index c1f62eb..a8f40fa 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > > @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > > struct intel_dp *intel_dp = &intel_dig_port->dp; > > struct intel_connector *connector = > > to_intel_connector(conn_state->connector); > > - struct drm_atomic_state *state; > > + struct drm_atomic_state *state = pipe_config->base.state; > > int bpp; > > - int lane_count, slots; > > + int lane_count, slots = 0; > > const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > > int mst_pbn; > > > > @@ -57,30 +57,53 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > > * seem to suggest we should do otherwise. > > */ > > lane_count = drm_dp_max_lane_count(intel_dp->dpcd); > > - > > pipe_config->lane_count = lane_count; > > > > pipe_config->pipe_bpp = bpp; > > pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); > > > > - state = pipe_config->base.state; > > - > > if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port)) > > pipe_config->has_audio = true; > > - mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); > > > > + mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); > > pipe_config->pbn = mst_pbn; > > - slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn); > > > > intel_link_compute_m_n(bpp, lane_count, > > adjusted_mode->crtc_clock, > > pipe_config->port_clock, > > &pipe_config->dp_m_n); > > > > + if (pipe_config->base.active) { > > + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, > > + connector->port, mst_pbn); > > + if (slots < 0) { > > + DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots); > > + return false; > > + } > > + } > > pipe_config->dp_m_n.tu = slots; > > > > return true; > > +} > > + > > +static void intel_dp_mst_atomic_release(struct drm_connector *connector, > > + struct drm_connector_state *conn_state) > > +{ > > + struct intel_dp_mst_encoder *intel_mst; > > + struct drm_dp_mst_topology_mgr *mgr; > > + struct drm_encoder *encoder; > > + struct intel_connector *intel_connector = to_intel_connector(connector); > > + struct drm_atomic_state *state = conn_state->state; > > + int slots; > > + > > + encoder = connector->state->best_encoder; > > + intel_mst = enc_to_mst(encoder); > > + mgr = &intel_mst->primary->dp.mst_mgr; > > > > + slots = drm_dp_atomic_release_vcpi_slots(state, mgr, > > + intel_connector->port); > > + if (slots < 0) > > + DRM_DEBUG_KMS("failed releasing vcpi slots:%d\n", slots); > > } > > > > static void intel_mst_disable_dp(struct intel_encoder *encoder, > > @@ -387,6 +410,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun > > .mode_valid = intel_dp_mst_mode_valid, > > .atomic_best_encoder = intel_mst_atomic_best_encoder, > > .best_encoder = intel_mst_best_encoder, > > + .atomic_release = intel_dp_mst_atomic_release, > > }; > > > > static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder) > > Is there any issue into attempting to release vcpi slots when they're already released? If not, for patches 1-3 5-8 > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Thanks for the review. If we already had an atomic commit that released the slots, then vcpi.num_slots in 'struct drm_dp_mst_port' would have been reset to 0. The second time, release_vcpi_slots() is called, the topology_state slot tracking won't be updated because it gets the current allocation from per-port structure. So, it should work fine. -DK > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Mar 22, 2017 at 01:30:30PM +0100, Maarten Lankhorst wrote: > Op 16-03-17 om 08:10 schreef Dhinakaran Pandiyan: > Is there any issue into attempting to release vcpi slots when they're already released? If not, for patches 1-3 5-8 > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Merged patches 1-3 to drm-misc-next, thanks. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index c1f62eb..a8f40fa 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_dp *intel_dp = &intel_dig_port->dp; struct intel_connector *connector = to_intel_connector(conn_state->connector); - struct drm_atomic_state *state; + struct drm_atomic_state *state = pipe_config->base.state; int bpp; - int lane_count, slots; + int lane_count, slots = 0; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; @@ -57,30 +57,53 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, * seem to suggest we should do otherwise. */ lane_count = drm_dp_max_lane_count(intel_dp->dpcd); - pipe_config->lane_count = lane_count; pipe_config->pipe_bpp = bpp; pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); - state = pipe_config->base.state; - if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port)) pipe_config->has_audio = true; - mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); + mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp); pipe_config->pbn = mst_pbn; - slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn); intel_link_compute_m_n(bpp, lane_count, adjusted_mode->crtc_clock, pipe_config->port_clock, &pipe_config->dp_m_n); + if (pipe_config->base.active) { + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, + connector->port, mst_pbn); + if (slots < 0) { + DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots); + return false; + } + } pipe_config->dp_m_n.tu = slots; return true; +} + +static void intel_dp_mst_atomic_release(struct drm_connector *connector, + struct drm_connector_state *conn_state) +{ + struct intel_dp_mst_encoder *intel_mst; + struct drm_dp_mst_topology_mgr *mgr; + struct drm_encoder *encoder; + struct intel_connector *intel_connector = to_intel_connector(connector); + struct drm_atomic_state *state = conn_state->state; + int slots; + + encoder = connector->state->best_encoder; + intel_mst = enc_to_mst(encoder); + mgr = &intel_mst->primary->dp.mst_mgr; + slots = drm_dp_atomic_release_vcpi_slots(state, mgr, + intel_connector->port); + if (slots < 0) + DRM_DEBUG_KMS("failed releasing vcpi slots:%d\n", slots); } static void intel_mst_disable_dp(struct intel_encoder *encoder, @@ -387,6 +410,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun .mode_valid = intel_dp_mst_mode_valid, .atomic_best_encoder = intel_mst_atomic_best_encoder, .best_encoder = intel_mst_best_encoder, + .atomic_release = intel_dp_mst_atomic_release, }; static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)