| Message ID | 20190820191203.25807-15-David.Francis@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Display Stream Compression (DSC) for AMD Navi | expand |
This should definitely be implemented as an atomic helper in drm_dp_mst_topology.c as well. On Tue, 2019-08-20 at 15:12 -0400, David Francis wrote: > Whenever a connector on an MST network is attached, detached, or > undergoes a modeset, the DSC configs for each stream on that > topology will be recalculated. This can change their required > bandwidth, requiring a full reprogramming, as though a modeset > was performed, even if that stream did not change timing. > > Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset, > for each crtc that shares a MST topology with that stream and > supports DSC, add that crtc (and all affected connectors and > planes) to the atomic state and set mode_changed on its state > > v2: Do this check only on Navi and before adding connectors > and planes on modesetting crtcs > > Cc: Leo Li <sunpeng.li@amd.com> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > Signed-off-by: David Francis <David.Francis@amd.com> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 74 +++++++++++++++++++ > 1 file changed, 74 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 145fd73025dc..e64f2a6eb71a 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -6475,7 +6475,70 @@ static int do_aquire_global_lock(struct drm_device > *dev, > > return ret < 0 ? ret : 0; > } > +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT > +static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, > struct drm_crtc *crtc) > +{ > + struct drm_connector *connector; > + struct drm_connector_state *conn_state; > + struct drm_connector_list_iter conn_iter; > + struct drm_crtc_state *new_crtc_state; > + struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add; > + int i, j; > + struct drm_crtc *crtcs_affected[AMDGPU_MAX_CRTCS] = { 0 }; > + > + for_each_new_connector_in_state(state, connector, conn_state, i) { > + if (conn_state->crtc != crtc) > + continue; > + > + aconnector = to_amdgpu_dm_connector(connector); > + if (!aconnector->port) > + aconnector = NULL; > + else > + break; > + } > + > + if (!aconnector) > + return 0; > + > + i = 0; > + drm_connector_list_iter_begin(state->dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > + if (!connector->state || !connector->state->crtc) > + continue; > + > + aconnector_to_add = to_amdgpu_dm_connector(connector); > + if (!aconnector_to_add->port) > + continue; > + > + if (aconnector_to_add->port->mgr != aconnector->port->mgr) > + continue; > > + if (!aconnector_to_add->dc_sink) > + continue; > + > + if (!aconnector_to_add->dc_sink- > >sink_dsc_caps.dsc_dec_caps.is_dsc_supported) > + continue; > + > + if (i >= AMDGPU_MAX_CRTCS) > + continue; > + > + crtcs_affected[i] = connector->state->crtc; > + i++; > + } > + drm_connector_list_iter_end(&conn_iter); > + > + for (j = 0; j < i; j++) { > + new_crtc_state = drm_atomic_get_crtc_state(state, > crtcs_affected[j]); > + if (IS_ERR(new_crtc_state)) > + return PTR_ERR(new_crtc_state); > + > + new_crtc_state->mode_changed = true; > + } > + > + return 0; > + > +} > +#endif > static void get_freesync_config_for_crtc( > struct dm_crtc_state *new_crtc_state, > struct dm_connector_state *new_con_state) > @@ -7160,6 +7223,17 @@ static int amdgpu_dm_atomic_check(struct drm_device > *dev, > if (ret) > goto fail; > > +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT > + if (adev->asic_type >= CHIP_NAVI10) { > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > + if (drm_atomic_crtc_needs_modeset(new_crtc_state)) { > + ret = add_affected_mst_dsc_crtcs(state, crtc); > + if (ret) > + goto fail; > + } > + } > + } > +#endif > for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > if (!drm_atomic_crtc_needs_modeset(new_crtc_state) && > !new_crtc_state->color_mgmt_changed &&
On 8/20/19 5:09 PM, Lyude Paul wrote: > This should definitely be implemented as an atomic helper in > drm_dp_mst_topology.c as well. This is probably reasonable, most drivers would probably want this. The issues with this in general are still: (1) Knowing if you have a DSC in the MST network to decide when to do this (2) Marking the CRTCs for the connectors in the MST chain as having their mode changed while iterating over the connectors For amdgpu it's "acceptable" based on other design constraints to just lock and grab everything on a modeset but this may not be true for every driver. Nicholas Kazlauskas > > On Tue, 2019-08-20 at 15:12 -0400, David Francis wrote: >> Whenever a connector on an MST network is attached, detached, or >> undergoes a modeset, the DSC configs for each stream on that >> topology will be recalculated. This can change their required >> bandwidth, requiring a full reprogramming, as though a modeset >> was performed, even if that stream did not change timing. >> >> Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset, >> for each crtc that shares a MST topology with that stream and >> supports DSC, add that crtc (and all affected connectors and >> planes) to the atomic state and set mode_changed on its state >> >> v2: Do this check only on Navi and before adding connectors >> and planes on modesetting crtcs >> >> Cc: Leo Li <sunpeng.li@amd.com> >> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> >> Signed-off-by: David Francis <David.Francis@amd.com> >> --- >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 74 +++++++++++++++++++ >> 1 file changed, 74 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> index 145fd73025dc..e64f2a6eb71a 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -6475,7 +6475,70 @@ static int do_aquire_global_lock(struct drm_device >> *dev, >> >> return ret < 0 ? ret : 0; >> } >> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT >> +static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, >> struct drm_crtc *crtc) >> +{ >> + struct drm_connector *connector; >> + struct drm_connector_state *conn_state; >> + struct drm_connector_list_iter conn_iter; >> + struct drm_crtc_state *new_crtc_state; >> + struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add; >> + int i, j; >> + struct drm_crtc *crtcs_affected[AMDGPU_MAX_CRTCS] = { 0 }; >> + >> + for_each_new_connector_in_state(state, connector, conn_state, i) { >> + if (conn_state->crtc != crtc) >> + continue; >> + >> + aconnector = to_amdgpu_dm_connector(connector); >> + if (!aconnector->port) >> + aconnector = NULL; >> + else >> + break; >> + } >> + >> + if (!aconnector) >> + return 0; >> + >> + i = 0; >> + drm_connector_list_iter_begin(state->dev, &conn_iter); >> + drm_for_each_connector_iter(connector, &conn_iter) { >> + if (!connector->state || !connector->state->crtc) >> + continue; >> + >> + aconnector_to_add = to_amdgpu_dm_connector(connector); >> + if (!aconnector_to_add->port) >> + continue; >> + >> + if (aconnector_to_add->port->mgr != aconnector->port->mgr) >> + continue; >> >> + if (!aconnector_to_add->dc_sink) >> + continue; >> + >> + if (!aconnector_to_add->dc_sink- >>> sink_dsc_caps.dsc_dec_caps.is_dsc_supported) >> + continue; >> + >> + if (i >= AMDGPU_MAX_CRTCS) >> + continue; >> + >> + crtcs_affected[i] = connector->state->crtc; >> + i++; >> + } >> + drm_connector_list_iter_end(&conn_iter); >> + >> + for (j = 0; j < i; j++) { >> + new_crtc_state = drm_atomic_get_crtc_state(state, >> crtcs_affected[j]); >> + if (IS_ERR(new_crtc_state)) >> + return PTR_ERR(new_crtc_state); >> + >> + new_crtc_state->mode_changed = true; >> + } >> + >> + return 0; >> + >> +} >> +#endif >> static void get_freesync_config_for_crtc( >> struct dm_crtc_state *new_crtc_state, >> struct dm_connector_state *new_con_state) >> @@ -7160,6 +7223,17 @@ static int amdgpu_dm_atomic_check(struct drm_device >> *dev, >> if (ret) >> goto fail; >> >> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT >> + if (adev->asic_type >= CHIP_NAVI10) { >> + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, >> new_crtc_state, i) { >> + if (drm_atomic_crtc_needs_modeset(new_crtc_state)) { >> + ret = add_affected_mst_dsc_crtcs(state, crtc); >> + if (ret) >> + goto fail; >> + } >> + } >> + } >> +#endif >> for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, >> new_crtc_state, i) { >> if (!drm_atomic_crtc_needs_modeset(new_crtc_state) && >> !new_crtc_state->color_mgmt_changed &&
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 145fd73025dc..e64f2a6eb71a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6475,7 +6475,70 @@ static int do_aquire_global_lock(struct drm_device *dev, return ret < 0 ? ret : 0; } +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT +static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm_crtc *crtc) +{ + struct drm_connector *connector; + struct drm_connector_state *conn_state; + struct drm_connector_list_iter conn_iter; + struct drm_crtc_state *new_crtc_state; + struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add; + int i, j; + struct drm_crtc *crtcs_affected[AMDGPU_MAX_CRTCS] = { 0 }; + + for_each_new_connector_in_state(state, connector, conn_state, i) { + if (conn_state->crtc != crtc) + continue; + + aconnector = to_amdgpu_dm_connector(connector); + if (!aconnector->port) + aconnector = NULL; + else + break; + } + + if (!aconnector) + return 0; + + i = 0; + drm_connector_list_iter_begin(state->dev, &conn_iter); + drm_for_each_connector_iter(connector, &conn_iter) { + if (!connector->state || !connector->state->crtc) + continue; + + aconnector_to_add = to_amdgpu_dm_connector(connector); + if (!aconnector_to_add->port) + continue; + + if (aconnector_to_add->port->mgr != aconnector->port->mgr) + continue; + if (!aconnector_to_add->dc_sink) + continue; + + if (!aconnector_to_add->dc_sink->sink_dsc_caps.dsc_dec_caps.is_dsc_supported) + continue; + + if (i >= AMDGPU_MAX_CRTCS) + continue; + + crtcs_affected[i] = connector->state->crtc; + i++; + } + drm_connector_list_iter_end(&conn_iter); + + for (j = 0; j < i; j++) { + new_crtc_state = drm_atomic_get_crtc_state(state, crtcs_affected[j]); + if (IS_ERR(new_crtc_state)) + return PTR_ERR(new_crtc_state); + + new_crtc_state->mode_changed = true; + } + + return 0; + +} +#endif static void get_freesync_config_for_crtc( struct dm_crtc_state *new_crtc_state, struct dm_connector_state *new_con_state) @@ -7160,6 +7223,17 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, if (ret) goto fail; +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT + if (adev->asic_type >= CHIP_NAVI10) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { + if (drm_atomic_crtc_needs_modeset(new_crtc_state)) { + ret = add_affected_mst_dsc_crtcs(state, crtc); + if (ret) + goto fail; + } + } + } +#endif for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { if (!drm_atomic_crtc_needs_modeset(new_crtc_state) && !new_crtc_state->color_mgmt_changed &&
Whenever a connector on an MST network is attached, detached, or undergoes a modeset, the DSC configs for each stream on that topology will be recalculated. This can change their required bandwidth, requiring a full reprogramming, as though a modeset was performed, even if that stream did not change timing. Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset, for each crtc that shares a MST topology with that stream and supports DSC, add that crtc (and all affected connectors and planes) to the atomic state and set mode_changed on its state v2: Do this check only on Navi and before adding connectors and planes on modesetting crtcs Cc: Leo Li <sunpeng.li@amd.com> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> Signed-off-by: David Francis <David.Francis@amd.com> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+)