| Message ID | 20190819155038.1771-15-David.Francis@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Display Stream Compression (DSC) for AMD Navi | expand |
On 8/19/19 11:50 AM, 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 > > 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 | 80 +++++++++++++++++++ > 1 file changed, 80 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..8d5357aec5e8 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,78 @@ 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, ret; > + struct drm_crtc *crtcs_affected[MAX_PIPES] = { 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); I don't like that we're grabbing the global connector lock every single time any CRTC undergoes a modeset even for ASICs that don't support DSC. We do lock everything below in atomic check anyway for FULL updates but I'd like to avoid adding more code that does this if possible. Maybe a check at the start that only does this if the ASIC has DSC support would be OK. > + 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 >= MAX_PIPES) > + continue; > + > + crtcs_affected[i] = connector->state->crtc; Drop this crtcs_affected array and just perform the logic below right here. We don't really need two loops here, redundant calls to drm_atomic_get_crtc_state and the other helpers are fine. > + 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; > + > + ret = drm_atomic_add_affected_connectors(state, crtcs_affected[j]); > + if (ret) > + return ret; > + > + ret = drm_atomic_add_affected_planes(state, crtcs_affected[j]); > + if (ret) > + return ret; > + } > + > + return 0; > + > +} > +#endif > static void get_freesync_config_for_crtc( > struct dm_crtc_state *new_crtc_state, > struct dm_connector_state *new_con_state) > @@ -7178,6 +7249,15 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, > goto fail; > } > > +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT > + 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 > /* > * Add all primary and overlay planes on the CRTC to the state > * whenever a plane is enabled to maintain correct z-ordering >
>> 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 >> >> 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 | 80 +++++++++++++++++++ >> 1 file changed, 80 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..8d5357aec5e8 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,78 @@ 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, ret; >> + struct drm_crtc *crtcs_affected[MAX_PIPES] = { 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); > >I don't like that we're grabbing the global connector lock every single >time any CRTC undergoes a modeset even for ASICs that don't support DSC. > >We do lock everything below in atomic check anyway for FULL updates but >I'd like to avoid adding more code that does this if possible. Maybe a >check at the start that only does this if the ASIC has DSC support would >be OK. > Will do >> + 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 >= MAX_PIPES) >> + continue; >> + >> + crtcs_affected[i] = connector->state->crtc; > >Drop this crtcs_affected array and just perform the logic below right >here. We don't really need two loops here, redundant calls to >drm_atomic_get_crtc_state and the other helpers are fine. > Unfortunately, calling drm_atomic_get_crtc_state inside drm_for_each_connector_iter causes a lockdep warning >> + 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; >> + >> + ret = drm_atomic_add_affected_connectors(state, crtcs_affected[j]); >> + if (ret) >> + return ret; >> + >> + ret = drm_atomic_add_affected_planes(state, crtcs_affected[j]); >> + if (ret) >> + return ret; >> + } >> + >> + return 0; >> + >> +} >> +#endif >> static void get_freesync_config_for_crtc( >> struct dm_crtc_state *new_crtc_state, >> struct dm_connector_state *new_con_state) >> @@ -7178,6 +7249,15 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, >> goto fail; >> } >> >> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT >> + 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 >> /* >> * Add all primary and overlay planes on the CRTC to the state >> * whenever a plane is enabled to maintain correct z-ordering
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..8d5357aec5e8 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,78 @@ 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, ret; + struct drm_crtc *crtcs_affected[MAX_PIPES] = { 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 >= MAX_PIPES) + 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; + + ret = drm_atomic_add_affected_connectors(state, crtcs_affected[j]); + if (ret) + return ret; + + ret = drm_atomic_add_affected_planes(state, crtcs_affected[j]); + if (ret) + return ret; + } + + return 0; + +} +#endif static void get_freesync_config_for_crtc( struct dm_crtc_state *new_crtc_state, struct dm_connector_state *new_con_state) @@ -7178,6 +7249,15 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, goto fail; } +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT + 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 /* * Add all primary and overlay planes on the CRTC to the state * whenever a plane is enabled to maintain correct z-ordering
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 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 | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+)