| Message ID | 20240123102850.390126-4-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add Display Port tunnel BW allocation support | expand |
On Tue, 2024-01-23 at 12:28 +0200, Imre Deak wrote: > On shared (Thunderbolt) links with DP tunnels, the modeset may need > to > be retried on all connectors on the link due to a link BW limitation > arising only after the atomic check phase. To support this add a > helper > function queuing a work to retry the modeset on a given port's > connector > and at the same time any MST connector with streams through the same > port. A follow-up change enabling the DP tunnel Bandwidth Allocation > Mode will take this into use. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 5 +- > drivers/gpu/drm/i915/display/intel_dp.c | 55 > ++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_dp.h | 8 +++ > .../drm/i915/display/intel_dp_link_training.c | 3 +- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 + > 5 files changed, 67 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index a92e959c8ac7b..0caebbb3e2dbb 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -8060,8 +8060,9 @@ void intel_hpd_poll_fini(struct > drm_i915_private *i915) > /* Kill all the work that may have been queued by hpd. */ > drm_connector_list_iter_begin(&i915->drm, &conn_iter); > for_each_intel_connector_iter(connector, &conn_iter) { > - if (connector->modeset_retry_work.func) > - cancel_work_sync(&connector- > >modeset_retry_work); > + if (connector->modeset_retry_work.func && > + cancel_work_sync(&connector->modeset_retry_work)) > + drm_connector_put(&connector->base); > if (connector->hdcp.shim) { > cancel_delayed_work_sync(&connector- > >hdcp.check_work); > cancel_work_sync(&connector->hdcp.prop_work); > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index ab415f41924d7..4e36c2c39888e 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2837,6 +2837,50 @@ intel_dp_audio_compute_config(struct > intel_encoder *encoder, > intel_dp_is_uhbr(pipe_config) > ; > } > > +void intel_dp_queue_modeset_retry_work(struct intel_connector > *connector) > +{ > + struct drm_i915_private *i915 = to_i915(connector->base.dev); > + > + drm_connector_get(&connector->base); > + if (!queue_work(i915->unordered_wq, &connector- > >modeset_retry_work)) > + drm_connector_put(&connector->base); > +} > + > +void > +intel_dp_queue_modeset_retry_for_link(struct intel_atomic_state > *state, > + struct intel_encoder *encoder, > + const struct intel_crtc_state > *crtc_state, > + const struct > drm_connector_state *conn_state) > +{ > + struct drm_i915_private *i915 = to_i915(crtc_state- > >uapi.crtc->dev); > + struct intel_connector *connector; > + struct intel_digital_connector_state *iter_conn_state; > + struct intel_dp *intel_dp; > + int i; > + > + if (conn_state) { > + connector = to_intel_connector(conn_state- > >connector); > + intel_dp_queue_modeset_retry_work(connector); > + > + return; > + } > + > + if (drm_WARN_ON(&i915->drm, > + !intel_crtc_has_type(crtc_state, > INTEL_OUTPUT_DP_MST))) > + return; > + > + intel_dp = enc_to_intel_dp(encoder); > + > + for_each_new_intel_connector_in_state(state, connector, > iter_conn_state, i) { > + (void)iter_conn_state; Checked iter_conn_state->base->crtc documentation: @crtc: CRTC to connect connector to, NULL if disabled. Do we need to check if connector is "disabled" or is it impossible scenario? BR, Jouni Högander > + > + if (connector->mst_port != intel_dp) > + continue; > + > + intel_dp_queue_modeset_retry_work(connector); > + } > +} > + > int > intel_dp_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config, > @@ -6436,6 +6480,14 @@ static void > intel_dp_modeset_retry_work_fn(struct work_struct *work) > mutex_unlock(&connector->dev->mode_config.mutex); > /* Send Hotplug uevent so userspace can reprobe */ > drm_kms_helper_connector_hotplug_event(connector); > + > + drm_connector_put(connector); > +} > + > +void intel_dp_init_modeset_retry_work(struct intel_connector > *connector) > +{ > + INIT_WORK(&connector->modeset_retry_work, > + intel_dp_modeset_retry_work_fn); > } > > bool > @@ -6452,8 +6504,7 @@ intel_dp_init_connector(struct > intel_digital_port *dig_port, > int type; > > /* Initialize the work for modeset in case of link train > failure */ > - INIT_WORK(&intel_connector->modeset_retry_work, > - intel_dp_modeset_retry_work_fn); > + intel_dp_init_modeset_retry_work(intel_connector); > > if (drm_WARN(dev, dig_port->max_lanes < 1, > "Not enough lanes (%d) for DP on > [ENCODER:%d:%s]\n", > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h > b/drivers/gpu/drm/i915/display/intel_dp.h > index 530cc97bc42f4..105c2086310db 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.h > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > @@ -23,6 +23,8 @@ struct intel_digital_port; > struct intel_dp; > struct intel_encoder; > > +struct work_struct; > + > struct link_config_limits { > int min_rate, max_rate; > int min_lane_count, max_lane_count; > @@ -43,6 +45,12 @@ void intel_dp_adjust_compliance_config(struct > intel_dp *intel_dp, > bool intel_dp_limited_color_range(const struct intel_crtc_state > *crtc_state, > const struct drm_connector_state > *conn_state); > int intel_dp_min_bpp(enum intel_output_format output_format); > +void intel_dp_init_modeset_retry_work(struct intel_connector > *connector); > +void intel_dp_queue_modeset_retry_work(struct intel_connector > *connector); > +void intel_dp_queue_modeset_retry_for_link(struct intel_atomic_state > *state, > + struct intel_encoder > *encoder, > + const struct > intel_crtc_state *crtc_state, > + const struct > drm_connector_state *conn_state); > bool intel_dp_init_connector(struct intel_digital_port *dig_port, > struct intel_connector > *intel_connector); > void intel_dp_set_link_params(struct intel_dp *intel_dp, > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > index 1abfafbbfa757..7b140cbf8dd31 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > @@ -1075,7 +1075,6 @@ static void > intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp, > const struct > intel_crtc_state *crtc_state) > { > struct intel_connector *intel_connector = intel_dp- > >attached_connector; > - struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > if (!intel_digital_port_connected(&dp_to_dig_port(intel_dp)- > >base)) { > lt_dbg(intel_dp, DP_PHY_DPRX, "Link Training failed > on disconnected sink.\n"); > @@ -1093,7 +1092,7 @@ static void > intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp, > } > > /* Schedule a Hotplug Uevent to userspace to start modeset */ > - queue_work(i915->unordered_wq, &intel_connector- > >modeset_retry_work); > + intel_dp_queue_modeset_retry_work(intel_connector); > } > > /* Perform the link training on all LTTPRs and the DPRX on a link. > */ > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 5fa25a5a36b55..b15e43ebf138b 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -1542,6 +1542,8 @@ static struct drm_connector > *intel_dp_add_mst_connector(struct drm_dp_mst_topolo > intel_connector->port = port; > drm_dp_mst_get_port_malloc(port); > > + intel_dp_init_modeset_retry_work(intel_connector); > + > intel_connector->dp.dsc_decompression_aux = > drm_dp_mst_dsc_aux_for_port(port); > intel_dp_mst_read_decompression_port_dsc_caps(intel_dp, > intel_connector); > intel_connector->dp.dsc_hblank_expansion_quirk =
On Mon, Jan 29, 2024 at 12:36:12PM +0200, Hogander, Jouni wrote: > On Tue, 2024-01-23 at 12:28 +0200, Imre Deak wrote: > > [...] > > +void > > +intel_dp_queue_modeset_retry_for_link(struct intel_atomic_state *state, > > + struct intel_encoder *encoder, > > + const struct intel_crtc_state *crtc_state, > > + const struct drm_connector_state *conn_state) > > +{ > > + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); > > + struct intel_connector *connector; > > + struct intel_digital_connector_state *iter_conn_state; > > + struct intel_dp *intel_dp; > > + int i; > > + > > + if (conn_state) { > > + connector = to_intel_connector(conn_state->connector); > > + intel_dp_queue_modeset_retry_work(connector); > > + > > + return; > > + } > > + > > + if (drm_WARN_ON(&i915->drm, > > + !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))) > > + return; > > + > > + intel_dp = enc_to_intel_dp(encoder); > > + > > + for_each_new_intel_connector_in_state(state, connector, iter_conn_state, i) { > > + (void)iter_conn_state; > > Checked iter_conn_state->base->crtc documentation: > > @crtc: CRTC to connect connector to, NULL if disabled. > > Do we need to check if connector is "disabled" or is it impossible > scenario? Yes, it does show if the connector is disabled and it would make sense to not notify those. However the check for that would be racy, at least during a non-blocking commit, but I think also in general where userspace could be in the middle of enabling this connector. The point of the notification is that userspace re-checks the mode it wants on each MST connector to be enabled, so to prevent that it would miss the re-check on connectors with a pending enabling like above, the notification is simply sent to all the connectors in the MST topology. > > BR, > > Jouni Högander > > > > + > > + if (connector->mst_port != intel_dp) > > + continue; > > + > > + intel_dp_queue_modeset_retry_work(connector); > > + } > > +} > > + > > int > > intel_dp_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config, > > @@ -6436,6 +6480,14 @@ static void > > intel_dp_modeset_retry_work_fn(struct work_struct *work) > > mutex_unlock(&connector->dev->mode_config.mutex); > > /* Send Hotplug uevent so userspace can reprobe */ > > drm_kms_helper_connector_hotplug_event(connector); > > + > > + drm_connector_put(connector); > > +} > > + > > +void intel_dp_init_modeset_retry_work(struct intel_connector > > *connector) > > +{ > > + INIT_WORK(&connector->modeset_retry_work, > > + intel_dp_modeset_retry_work_fn); > > } > > > > bool > > @@ -6452,8 +6504,7 @@ intel_dp_init_connector(struct > > intel_digital_port *dig_port, > > int type; > > > > /* Initialize the work for modeset in case of link train > > failure */ > > - INIT_WORK(&intel_connector->modeset_retry_work, > > - intel_dp_modeset_retry_work_fn); > > + intel_dp_init_modeset_retry_work(intel_connector); > > > > if (drm_WARN(dev, dig_port->max_lanes < 1, > > "Not enough lanes (%d) for DP on > > [ENCODER:%d:%s]\n", > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h > > b/drivers/gpu/drm/i915/display/intel_dp.h > > index 530cc97bc42f4..105c2086310db 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.h > > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > > @@ -23,6 +23,8 @@ struct intel_digital_port; > > struct intel_dp; > > struct intel_encoder; > > > > +struct work_struct; > > + > > struct link_config_limits { > > int min_rate, max_rate; > > int min_lane_count, max_lane_count; > > @@ -43,6 +45,12 @@ void intel_dp_adjust_compliance_config(struct > > intel_dp *intel_dp, > > bool intel_dp_limited_color_range(const struct intel_crtc_state > > *crtc_state, > > const struct drm_connector_state > > *conn_state); > > int intel_dp_min_bpp(enum intel_output_format output_format); > > +void intel_dp_init_modeset_retry_work(struct intel_connector > > *connector); > > +void intel_dp_queue_modeset_retry_work(struct intel_connector > > *connector); > > +void intel_dp_queue_modeset_retry_for_link(struct intel_atomic_state > > *state, > > + struct intel_encoder > > *encoder, > > + const struct > > intel_crtc_state *crtc_state, > > + const struct > > drm_connector_state *conn_state); > > bool intel_dp_init_connector(struct intel_digital_port *dig_port, > > struct intel_connector > > *intel_connector); > > void intel_dp_set_link_params(struct intel_dp *intel_dp, > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > index 1abfafbbfa757..7b140cbf8dd31 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > > @@ -1075,7 +1075,6 @@ static void > > intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp, > > const struct > > intel_crtc_state *crtc_state) > > { > > struct intel_connector *intel_connector = intel_dp- > > >attached_connector; > > - struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > if (!intel_digital_port_connected(&dp_to_dig_port(intel_dp)- > > >base)) { > > lt_dbg(intel_dp, DP_PHY_DPRX, "Link Training failed > > on disconnected sink.\n"); > > @@ -1093,7 +1092,7 @@ static void > > intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp, > > } > > > > /* Schedule a Hotplug Uevent to userspace to start modeset */ > > - queue_work(i915->unordered_wq, &intel_connector- > > >modeset_retry_work); > > + intel_dp_queue_modeset_retry_work(intel_connector); > > } > > > > /* Perform the link training on all LTTPRs and the DPRX on a link. > > */ > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index 5fa25a5a36b55..b15e43ebf138b 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -1542,6 +1542,8 @@ static struct drm_connector > > *intel_dp_add_mst_connector(struct drm_dp_mst_topolo > > intel_connector->port = port; > > drm_dp_mst_get_port_malloc(port); > > > > + intel_dp_init_modeset_retry_work(intel_connector); > > + > > intel_connector->dp.dsc_decompression_aux = > > drm_dp_mst_dsc_aux_for_port(port); > > intel_dp_mst_read_decompression_port_dsc_caps(intel_dp, > > intel_connector); > > intel_connector->dp.dsc_hblank_expansion_quirk = >
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index a92e959c8ac7b..0caebbb3e2dbb 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -8060,8 +8060,9 @@ void intel_hpd_poll_fini(struct drm_i915_private *i915) /* Kill all the work that may have been queued by hpd. */ drm_connector_list_iter_begin(&i915->drm, &conn_iter); for_each_intel_connector_iter(connector, &conn_iter) { - if (connector->modeset_retry_work.func) - cancel_work_sync(&connector->modeset_retry_work); + if (connector->modeset_retry_work.func && + cancel_work_sync(&connector->modeset_retry_work)) + drm_connector_put(&connector->base); if (connector->hdcp.shim) { cancel_delayed_work_sync(&connector->hdcp.check_work); cancel_work_sync(&connector->hdcp.prop_work); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index ab415f41924d7..4e36c2c39888e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2837,6 +2837,50 @@ intel_dp_audio_compute_config(struct intel_encoder *encoder, intel_dp_is_uhbr(pipe_config); } +void intel_dp_queue_modeset_retry_work(struct intel_connector *connector) +{ + struct drm_i915_private *i915 = to_i915(connector->base.dev); + + drm_connector_get(&connector->base); + if (!queue_work(i915->unordered_wq, &connector->modeset_retry_work)) + drm_connector_put(&connector->base); +} + +void +intel_dp_queue_modeset_retry_for_link(struct intel_atomic_state *state, + struct intel_encoder *encoder, + const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state) +{ + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); + struct intel_connector *connector; + struct intel_digital_connector_state *iter_conn_state; + struct intel_dp *intel_dp; + int i; + + if (conn_state) { + connector = to_intel_connector(conn_state->connector); + intel_dp_queue_modeset_retry_work(connector); + + return; + } + + if (drm_WARN_ON(&i915->drm, + !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))) + return; + + intel_dp = enc_to_intel_dp(encoder); + + for_each_new_intel_connector_in_state(state, connector, iter_conn_state, i) { + (void)iter_conn_state; + + if (connector->mst_port != intel_dp) + continue; + + intel_dp_queue_modeset_retry_work(connector); + } +} + int intel_dp_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, @@ -6436,6 +6480,14 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work) mutex_unlock(&connector->dev->mode_config.mutex); /* Send Hotplug uevent so userspace can reprobe */ drm_kms_helper_connector_hotplug_event(connector); + + drm_connector_put(connector); +} + +void intel_dp_init_modeset_retry_work(struct intel_connector *connector) +{ + INIT_WORK(&connector->modeset_retry_work, + intel_dp_modeset_retry_work_fn); } bool @@ -6452,8 +6504,7 @@ intel_dp_init_connector(struct intel_digital_port *dig_port, int type; /* Initialize the work for modeset in case of link train failure */ - INIT_WORK(&intel_connector->modeset_retry_work, - intel_dp_modeset_retry_work_fn); + intel_dp_init_modeset_retry_work(intel_connector); if (drm_WARN(dev, dig_port->max_lanes < 1, "Not enough lanes (%d) for DP on [ENCODER:%d:%s]\n", diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h index 530cc97bc42f4..105c2086310db 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.h +++ b/drivers/gpu/drm/i915/display/intel_dp.h @@ -23,6 +23,8 @@ struct intel_digital_port; struct intel_dp; struct intel_encoder; +struct work_struct; + struct link_config_limits { int min_rate, max_rate; int min_lane_count, max_lane_count; @@ -43,6 +45,12 @@ void intel_dp_adjust_compliance_config(struct intel_dp *intel_dp, bool intel_dp_limited_color_range(const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state); int intel_dp_min_bpp(enum intel_output_format output_format); +void intel_dp_init_modeset_retry_work(struct intel_connector *connector); +void intel_dp_queue_modeset_retry_work(struct intel_connector *connector); +void intel_dp_queue_modeset_retry_for_link(struct intel_atomic_state *state, + struct intel_encoder *encoder, + const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state); bool intel_dp_init_connector(struct intel_digital_port *dig_port, struct intel_connector *intel_connector); void intel_dp_set_link_params(struct intel_dp *intel_dp, diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c index 1abfafbbfa757..7b140cbf8dd31 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c @@ -1075,7 +1075,6 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state) { struct intel_connector *intel_connector = intel_dp->attached_connector; - struct drm_i915_private *i915 = dp_to_i915(intel_dp); if (!intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base)) { lt_dbg(intel_dp, DP_PHY_DPRX, "Link Training failed on disconnected sink.\n"); @@ -1093,7 +1092,7 @@ static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp, } /* Schedule a Hotplug Uevent to userspace to start modeset */ - queue_work(i915->unordered_wq, &intel_connector->modeset_retry_work); + intel_dp_queue_modeset_retry_work(intel_connector); } /* Perform the link training on all LTTPRs and the DPRX on a link. */ diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 5fa25a5a36b55..b15e43ebf138b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1542,6 +1542,8 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo intel_connector->port = port; drm_dp_mst_get_port_malloc(port); + intel_dp_init_modeset_retry_work(intel_connector); + intel_connector->dp.dsc_decompression_aux = drm_dp_mst_dsc_aux_for_port(port); intel_dp_mst_read_decompression_port_dsc_caps(intel_dp, intel_connector); intel_connector->dp.dsc_hblank_expansion_quirk =
On shared (Thunderbolt) links with DP tunnels, the modeset may need to be retried on all connectors on the link due to a link BW limitation arising only after the atomic check phase. To support this add a helper function queuing a work to retry the modeset on a given port's connector and at the same time any MST connector with streams through the same port. A follow-up change enabling the DP tunnel Bandwidth Allocation Mode will take this into use. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 5 +- drivers/gpu/drm/i915/display/intel_dp.c | 55 ++++++++++++++++++- drivers/gpu/drm/i915/display/intel_dp.h | 8 +++ .../drm/i915/display/intel_dp_link_training.c | 3 +- drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 + 5 files changed, 67 insertions(+), 6 deletions(-)