| Message ID | 1485301777-3465-9-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 24, 2017 at 03:49:36PM -0800, Dhinakaran Pandiyan wrote: > Implement the ->atomic_release() callback to release the shared link > bandwidth that was originally acquired during compute_config() > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/i915/intel_dp_mst.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > index f51574f..2f57a56 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -78,6 +78,33 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > > } > > +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_dp_mst_topology_state *topology_state; > + struct drm_encoder *encoder; > + struct intel_connector *intel_connector = to_intel_connector(connector); > + > + encoder = connector->state->best_encoder; > + if (!encoder || to_intel_encoder(encoder)->type != INTEL_OUTPUT_DP_MST) > + return; NULL encoder should imo be caught in core. Type != DP_MST is impossible, if you're unsure make it into a BUG_ON for testing. > + > + intel_mst = enc_to_mst(encoder); > + mgr = &intel_mst->primary->dp.mst_mgr; > + > + topology_state = drm_atomic_get_mst_topology_state(conn_state->state, > + mgr); > + if (IS_ERR(topology_state)) { > + DRM_DEBUG_KMS("failed to create MST topology state %ld\n", > + PTR_ERR(topology_state)); > + return; > + } > + > + drm_dp_atomic_release_vcpi_slots(topology_state, intel_connector->port); I think it'd be great to merge this together into 1 helper that both gets the state and releases the vcpi, for less code in drivers. > +} > + > static void intel_mst_disable_dp(struct intel_encoder *encoder, > struct intel_crtc_state *old_crtc_state, > struct drm_connector_state *old_conn_state) > @@ -401,6 +428,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) > -- > 2.7.4 >
On Wed, 2017-01-25 at 07:16 +0100, Daniel Vetter wrote: > On Tue, Jan 24, 2017 at 03:49:36PM -0800, Dhinakaran Pandiyan wrote: > > Implement the ->atomic_release() callback to release the shared link > > bandwidth that was originally acquired during compute_config() > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp_mst.c | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > > index f51574f..2f57a56 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > > @@ -78,6 +78,33 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > > > > } > > > > +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_dp_mst_topology_state *topology_state; > > + struct drm_encoder *encoder; > > + struct intel_connector *intel_connector = to_intel_connector(connector); > > + > > + encoder = connector->state->best_encoder; > > + if (!encoder || to_intel_encoder(encoder)->type != INTEL_OUTPUT_DP_MST) > > + return; > > NULL encoder should imo be caught in core. Type != DP_MST is impossible, > if you're unsure make it into a BUG_ON for testing. I don't get why (type != INTEL_OUTPUT_DP_MST) is impossible. Is that because we have the implementation for atomic_release() only for MST connectors? But, that is only for now. -DK > > + > > + intel_mst = enc_to_mst(encoder); > > + mgr = &intel_mst->primary->dp.mst_mgr; > > + > > + topology_state = drm_atomic_get_mst_topology_state(conn_state->state, > > + mgr); > > + if (IS_ERR(topology_state)) { > > + DRM_DEBUG_KMS("failed to create MST topology state %ld\n", > > + PTR_ERR(topology_state)); > > + return; > > + } > > + > > + drm_dp_atomic_release_vcpi_slots(topology_state, intel_connector->port); > > I think it'd be great to merge this together into 1 helper that both gets > the state and releases the vcpi, for less code in drivers. Agreed. > > +} > > + > > static void intel_mst_disable_dp(struct intel_encoder *encoder, > > struct intel_crtc_state *old_crtc_state, > > struct drm_connector_state *old_conn_state) > > @@ -401,6 +428,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) > > -- > > 2.7.4 > > >
On Wed, Jan 25, 2017 at 08:53:18PM +0000, Pandiyan, Dhinakaran wrote: > On Wed, 2017-01-25 at 07:16 +0100, Daniel Vetter wrote: > > On Tue, Jan 24, 2017 at 03:49:36PM -0800, Dhinakaran Pandiyan wrote: > > > Implement the ->atomic_release() callback to release the shared link > > > bandwidth that was originally acquired during compute_config() > > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp_mst.c | 28 ++++++++++++++++++++++++++++ > > > 1 file changed, 28 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > > > index f51574f..2f57a56 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > > > @@ -78,6 +78,33 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, > > > > > > } > > > > > > +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_dp_mst_topology_state *topology_state; > > > + struct drm_encoder *encoder; > > > + struct intel_connector *intel_connector = to_intel_connector(connector); > > > + > > > + encoder = connector->state->best_encoder; > > > + if (!encoder || to_intel_encoder(encoder)->type != INTEL_OUTPUT_DP_MST) > > > + return; > > > > NULL encoder should imo be caught in core. Type != DP_MST is impossible, > > if you're unsure make it into a BUG_ON for testing. > > > I don't get why (type != INTEL_OUTPUT_DP_MST) is impossible. Is that > because we have the implementation for atomic_release() only for MST > connectors? But, that is only for now. Yes. And if we add atomic_release for other encoders, then ofcourse we will not reuse the mst one there. See all the other intel_dp_mst_* hooks in the same vfunc table, those also don't check for the encoder type. This would be like doing a runtime type check in C++ for the "this" pointer :-) -Daniel
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index f51574f..2f57a56 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -78,6 +78,33 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, } +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_dp_mst_topology_state *topology_state; + struct drm_encoder *encoder; + struct intel_connector *intel_connector = to_intel_connector(connector); + + encoder = connector->state->best_encoder; + if (!encoder || to_intel_encoder(encoder)->type != INTEL_OUTPUT_DP_MST) + return; + + intel_mst = enc_to_mst(encoder); + mgr = &intel_mst->primary->dp.mst_mgr; + + topology_state = drm_atomic_get_mst_topology_state(conn_state->state, + mgr); + if (IS_ERR(topology_state)) { + DRM_DEBUG_KMS("failed to create MST topology state %ld\n", + PTR_ERR(topology_state)); + return; + } + + drm_dp_atomic_release_vcpi_slots(topology_state, intel_connector->port); +} + static void intel_mst_disable_dp(struct intel_encoder *encoder, struct intel_crtc_state *old_crtc_state, struct drm_connector_state *old_conn_state) @@ -401,6 +428,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)
Implement the ->atomic_release() callback to release the shared link bandwidth that was originally acquired during compute_config() Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/i915/intel_dp_mst.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)