Re: [PATCH 2/2] drm/amd/display: Move iteration out of dm_update_crtcs
On 1/7/19 10:53 AM, sunpeng...@amd.com wrote:
> From: Leo Li
>
> [Why]
> To reduce indent in dm_update_crtcs, and to make it operate on single
> instances.
>
> [How]
> Move iteration of plane states into atomic_check.
> No functional change is intended.
>
> Signed-off-by: Leo Li
Series is:
Reviewed-by: Nicholas Kazlauskas
Looks good to me. I guess we don't need the other two patches from the
last series if we're not going to be making use of the helper.
Nicholas Kazlauskas
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 341
> +++---
> 1 file changed, 175 insertions(+), 166 deletions(-)
>
> 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 0b6bce5..70cd8bc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5550,15 +5550,15 @@ static void reset_freesync_config_for_crtc(
> sizeof(new_crtc_state->vrr_infopacket));
> }
>
> -static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
> - struct drm_atomic_state *state,
> - bool enable,
> - bool *lock_and_validation_needed)
> +static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
> + struct drm_atomic_state *state,
> + struct drm_crtc *crtc,
> + struct drm_crtc_state *old_crtc_state,
> + struct drm_crtc_state *new_crtc_state,
> + bool enable,
> + bool *lock_and_validation_needed)
> {
> struct dm_atomic_state *dm_state = NULL;
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> - int i;
> struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state;
> struct dc_stream_state *new_stream;
> int ret = 0;
> @@ -5567,200 +5567,199 @@ static int dm_update_crtcs_state(struct
> amdgpu_display_manager *dm,
>* TODO Move this code into dm_crtc_atomic_check once we get rid of
> dc_validation_set
>* update changed items
>*/
> - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - struct amdgpu_crtc *acrtc = NULL;
> - struct amdgpu_dm_connector *aconnector = NULL;
> - struct drm_connector_state *drm_new_conn_state = NULL,
> *drm_old_conn_state = NULL;
> - struct dm_connector_state *dm_new_conn_state = NULL,
> *dm_old_conn_state = NULL;
> - struct drm_plane_state *new_plane_state = NULL;
> + struct amdgpu_crtc *acrtc = NULL;
> + struct amdgpu_dm_connector *aconnector = NULL;
> + struct drm_connector_state *drm_new_conn_state = NULL,
> *drm_old_conn_state = NULL;
> + struct dm_connector_state *dm_new_conn_state = NULL, *dm_old_conn_state
> = NULL;
> + struct drm_plane_state *new_plane_state = NULL;
>
> - new_stream = NULL;
> + new_stream = NULL;
>
> - dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
> - dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> - acrtc = to_amdgpu_crtc(crtc);
> + dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
> + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
> + acrtc = to_amdgpu_crtc(crtc);
>
> - new_plane_state = drm_atomic_get_new_plane_state(state,
> new_crtc_state->crtc->primary);
> + new_plane_state = drm_atomic_get_new_plane_state(state,
> new_crtc_state->crtc->primary);
>
> - if (new_crtc_state->enable && new_plane_state &&
> !new_plane_state->fb) {
> - ret = -EINVAL;
> - goto fail;
> - }
> + if (new_crtc_state->enable && new_plane_state && !new_plane_state->fb) {
> + ret = -EINVAL;
> + goto fail;
> + }
>
> - aconnector =
> amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
> + aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
>
> - /* TODO This hack should go away */
> - if (aconnector && enable) {
> - /* Make sure fake sink is created in plug-in scenario */
> - drm_new_conn_state =
> drm_atomic_get_new_connector_state(state,
> -
> &aconnector->base);
> - drm_old_conn_state =
> drm_atomic_get_old_connector_state(state,
> -
> &aconnector->base);
> + /* TODO This hack should go away */
> + if (aconnector && enable) {
> + /* Make sure fake sink is created in plug-in scenario */
> + drm_new_conn_state = drm_atomic_get_new_connector_state(st
[PATCH 2/2] drm/amd/display: Move iteration out of dm_update_crtcs
From: Leo Li
[Why]
To reduce indent in dm_update_crtcs, and to make it operate on single
instances.
[How]
Move iteration of plane states into atomic_check.
No functional change is intended.
Signed-off-by: Leo Li
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 341 +++---
1 file changed, 175 insertions(+), 166 deletions(-)
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 0b6bce5..70cd8bc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5550,15 +5550,15 @@ static void reset_freesync_config_for_crtc(
sizeof(new_crtc_state->vrr_infopacket));
}
-static int dm_update_crtcs_state(struct amdgpu_display_manager *dm,
-struct drm_atomic_state *state,
-bool enable,
-bool *lock_and_validation_needed)
+static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
+ struct drm_atomic_state *state,
+ struct drm_crtc *crtc,
+ struct drm_crtc_state *old_crtc_state,
+ struct drm_crtc_state *new_crtc_state,
+ bool enable,
+ bool *lock_and_validation_needed)
{
struct dm_atomic_state *dm_state = NULL;
- struct drm_crtc *crtc;
- struct drm_crtc_state *old_crtc_state, *new_crtc_state;
- int i;
struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state;
struct dc_stream_state *new_stream;
int ret = 0;
@@ -5567,200 +5567,199 @@ static int dm_update_crtcs_state(struct
amdgpu_display_manager *dm,
* TODO Move this code into dm_crtc_atomic_check once we get rid of
dc_validation_set
* update changed items
*/
- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
- struct amdgpu_crtc *acrtc = NULL;
- struct amdgpu_dm_connector *aconnector = NULL;
- struct drm_connector_state *drm_new_conn_state = NULL,
*drm_old_conn_state = NULL;
- struct dm_connector_state *dm_new_conn_state = NULL,
*dm_old_conn_state = NULL;
- struct drm_plane_state *new_plane_state = NULL;
+ struct amdgpu_crtc *acrtc = NULL;
+ struct amdgpu_dm_connector *aconnector = NULL;
+ struct drm_connector_state *drm_new_conn_state = NULL,
*drm_old_conn_state = NULL;
+ struct dm_connector_state *dm_new_conn_state = NULL, *dm_old_conn_state
= NULL;
+ struct drm_plane_state *new_plane_state = NULL;
- new_stream = NULL;
+ new_stream = NULL;
- dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
- dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- acrtc = to_amdgpu_crtc(crtc);
+ dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+ dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+ acrtc = to_amdgpu_crtc(crtc);
- new_plane_state = drm_atomic_get_new_plane_state(state,
new_crtc_state->crtc->primary);
+ new_plane_state = drm_atomic_get_new_plane_state(state,
new_crtc_state->crtc->primary);
- if (new_crtc_state->enable && new_plane_state &&
!new_plane_state->fb) {
- ret = -EINVAL;
- goto fail;
- }
+ if (new_crtc_state->enable && new_plane_state && !new_plane_state->fb) {
+ ret = -EINVAL;
+ goto fail;
+ }
- aconnector =
amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
+ aconnector = amdgpu_dm_find_first_crtc_matching_connector(state, crtc);
- /* TODO This hack should go away */
- if (aconnector && enable) {
- /* Make sure fake sink is created in plug-in scenario */
- drm_new_conn_state =
drm_atomic_get_new_connector_state(state,
-
&aconnector->base);
- drm_old_conn_state =
drm_atomic_get_old_connector_state(state,
-
&aconnector->base);
+ /* TODO This hack should go away */
+ if (aconnector && enable) {
+ /* Make sure fake sink is created in plug-in scenario */
+ drm_new_conn_state = drm_atomic_get_new_connector_state(state,
+ &aconnector->base);
+ drm_old_conn_state = drm_atomic_get_old_connector_state(state,
+ &aconnector->base);
- if (IS_ERR(drm_new_conn_state)) {
- ret = PTR_ERR_OR_ZERO(drm
