Re: [PATCH 03/29] drm/amd/display: Complete TODO item: use new DRM iterator

[Andrey Grodzovsky]

On 10/30/2017 01:14 PM, Leo wrote:



On 2017-10-30 12:36 PM, Leo wrote:



On 2017-10-26 11:06 PM, Andrey Grodzovsky wrote:



On 2017-10-26 02:34 PM, Harry Wentland wrote:

From: "Leo (Sunpeng) Li" 

Abandon new_crtcs array and use for_each_new iterator to acquire new
crtcs.

Signed-off-by: Leo (Sunpeng) Li 
Reviewed-by: Harry Wentland 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 
+--

  1 file changed, 14 insertions(+), 22 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 442b399a9400..590f80d29b56 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4013,10 +4013,8 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

  struct amdgpu_display_manager *dm = >dm;
  struct dm_atomic_state *dm_state;
  uint32_t i, j;
-uint32_t new_crtcs_count = 0;
  struct drm_crtc *crtc;
  struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-struct amdgpu_crtc *new_crtcs[MAX_STREAMS];
  unsigned long flags;
  bool wait_for_vblank = true;
  struct drm_connector *connector;
@@ -4075,25 +4073,9 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

  continue;
  }
-
  if (dm_old_crtc_state->stream)
  remove_stream(adev, acrtc, 
dm_old_crtc_state->stream);

-
-/*
- * this loop saves set mode crtcs
- * we needed to enable vblanks once all
- * resources acquired in dc after dc_commit_streams
- */
-
-/*TODO move all this into dm_crtc_state, get rid of
- * new_crtcs array and use old and new atomic states
- * instead
- */
-new_crtcs[new_crtcs_count] = acrtc;
-new_crtcs_count++;
-
-new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);

  acrtc->enabled = true;
  acrtc->hw_mode = new_crtc_state->mode;
  crtc->hwmode = new_crtc_state->mode;
@@ -4221,18 +4203,28 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
  dm_error("%s: Failed to update stream scaling!\n", 
__func__);

  }
-for (i = 0; i < new_crtcs_count; i++) {
+for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
+new_crtc_state, i) {
  /*
   * loop to enable interrupts on newly arrived crtc
   */
-struct amdgpu_crtc *acrtc = new_crtcs[i];
+struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+bool modeset_needed;
-new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);

  dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+modeset_needed = modeset_required(
+new_crtc_state,
+dm_new_crtc_state->stream,
+dm_old_crtc_state->stream);
+
+if (dm_new_crtc_state->stream == NULL || !modeset_needed)
+continue;


I feel it's a bit future bug prone to repeat the 2 checks above from 
the initial
for_each_crtc_in_state loop, somebody makes changes there and forget 
about this loop.

The array is ugly but it avoids logic duplication.


I think a better fix would be to combine this for_each section with the
one above it. The only thing keeping them separate (not shown in the
above context) is an `if (adev->dm.freesync_module)` guard, which can be
pulled into the first for_each loop. It would also eliminate a lot of
duplicated initialization code.

Thoughts?

Leo



Ignore what I said, It seems I was looking at the wrong context :) The 
fact this blob is being done after dc_commit_state() is annoying...


The maintainability of this duplicated logic is a good point. Although 
I'm not sure if going back to a shared array using custom iterator 
logic is the best approach. Perhapse we can pull this duplicated logic 
into a seperate function?


That actually a good idea.

Thanks,
Andrey



Thanks,
Leo





Thanks,
Andrey


  if (adev->dm.freesync_module)
  mod_freesync_notify_mode_change(
-adev->dm.freesync_module, 
_new_crtc_state->stream, 1);

+adev->dm.freesync_module,
+_new_crtc_state->stream, 1);
  manage_dm_interrupts(adev, acrtc, true);
  }



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/29] drm/amd/display: Complete TODO item: use new DRM iterator

[Andrey Grodzovsky]

On 10/30/2017 12:36 PM, Leo wrote:



On 2017-10-26 11:06 PM, Andrey Grodzovsky wrote:



On 2017-10-26 02:34 PM, Harry Wentland wrote:

From: "Leo (Sunpeng) Li" 

Abandon new_crtcs array and use for_each_new iterator to acquire new
crtcs.

Signed-off-by: Leo (Sunpeng) Li 
Reviewed-by: Harry Wentland 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 
+--

  1 file changed, 14 insertions(+), 22 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 442b399a9400..590f80d29b56 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4013,10 +4013,8 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

  struct amdgpu_display_manager *dm = >dm;
  struct dm_atomic_state *dm_state;
  uint32_t i, j;
-uint32_t new_crtcs_count = 0;
  struct drm_crtc *crtc;
  struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-struct amdgpu_crtc *new_crtcs[MAX_STREAMS];
  unsigned long flags;
  bool wait_for_vblank = true;
  struct drm_connector *connector;
@@ -4075,25 +4073,9 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

  continue;
  }
-
  if (dm_old_crtc_state->stream)
  remove_stream(adev, acrtc, 
dm_old_crtc_state->stream);

-
-/*
- * this loop saves set mode crtcs
- * we needed to enable vblanks once all
- * resources acquired in dc after dc_commit_streams
- */
-
-/*TODO move all this into dm_crtc_state, get rid of
- * new_crtcs array and use old and new atomic states
- * instead
- */
-new_crtcs[new_crtcs_count] = acrtc;
-new_crtcs_count++;
-
-new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);

  acrtc->enabled = true;
  acrtc->hw_mode = new_crtc_state->mode;
  crtc->hwmode = new_crtc_state->mode;
@@ -4221,18 +4203,28 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
  dm_error("%s: Failed to update stream scaling!\n", 
__func__);

  }
-for (i = 0; i < new_crtcs_count; i++) {
+for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
+new_crtc_state, i) {
  /*
   * loop to enable interrupts on newly arrived crtc
   */
-struct amdgpu_crtc *acrtc = new_crtcs[i];
+struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+bool modeset_needed;
-new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);

  dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+modeset_needed = modeset_required(
+new_crtc_state,
+dm_new_crtc_state->stream,
+dm_old_crtc_state->stream);
+
+if (dm_new_crtc_state->stream == NULL || !modeset_needed)
+continue;


I feel it's a bit future bug prone to repeat the 2 checks above from 
the initial
for_each_crtc_in_state loop, somebody makes changes there and forget 
about this loop.

The array is ugly but it avoids logic duplication.


I think a better fix would be to combine this for_each section with the
one above it. The only thing keeping them separate (not shown in the
above context) is an `if (adev->dm.freesync_module)` guard, which can be
pulled into the first for_each loop. It would also eliminate a lot of
duplicated initialization code.

Thoughts?


It might be a bit more difficult since inside the clause is flip and 
vblank interrupts enabling,

but they rely heavily on acrtc->otg_inst which is being set in between.

Thanks,
Andrey



Leo



Thanks,
Andrey


  if (adev->dm.freesync_module)
  mod_freesync_notify_mode_change(
-adev->dm.freesync_module, 
_new_crtc_state->stream, 1);

+adev->dm.freesync_module,
+_new_crtc_state->stream, 1);
  manage_dm_interrupts(adev, acrtc, true);
  }




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/29] drm/amd/display: Complete TODO item: use new DRM iterator

[Leo]

On 2017-10-30 12:36 PM, Leo wrote:



On 2017-10-26 11:06 PM, Andrey Grodzovsky wrote:



On 2017-10-26 02:34 PM, Harry Wentland wrote:

From: "Leo (Sunpeng) Li" 

Abandon new_crtcs array and use for_each_new iterator to acquire new
crtcs.

Signed-off-by: Leo (Sunpeng) Li 
Reviewed-by: Harry Wentland 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 
+--

  1 file changed, 14 insertions(+), 22 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 442b399a9400..590f80d29b56 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4013,10 +4013,8 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

  struct amdgpu_display_manager *dm = >dm;
  struct dm_atomic_state *dm_state;
  uint32_t i, j;
-    uint32_t new_crtcs_count = 0;
  struct drm_crtc *crtc;
  struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-    struct amdgpu_crtc *new_crtcs[MAX_STREAMS];
  unsigned long flags;
  bool wait_for_vblank = true;
  struct drm_connector *connector;
@@ -4075,25 +4073,9 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)

  continue;
  }
-
  if (dm_old_crtc_state->stream)
  remove_stream(adev, acrtc, dm_old_crtc_state->stream);
-
-    /*
- * this loop saves set mode crtcs
- * we needed to enable vblanks once all
- * resources acquired in dc after dc_commit_streams
- */
-
-    /*TODO move all this into dm_crtc_state, get rid of
- * new_crtcs array and use old and new atomic states
- * instead
- */
-    new_crtcs[new_crtcs_count] = acrtc;
-    new_crtcs_count++;
-
-    new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);

  acrtc->enabled = true;
  acrtc->hw_mode = new_crtc_state->mode;
  crtc->hwmode = new_crtc_state->mode;
@@ -4221,18 +4203,28 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
  dm_error("%s: Failed to update stream scaling!\n", 
__func__);

  }
-    for (i = 0; i < new_crtcs_count; i++) {
+    for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
+    new_crtc_state, i) {
  /*
   * loop to enable interrupts on newly arrived crtc
   */
-    struct amdgpu_crtc *acrtc = new_crtcs[i];
+    struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+    bool modeset_needed;
-    new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);

  dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+    dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+    modeset_needed = modeset_required(
+    new_crtc_state,
+    dm_new_crtc_state->stream,
+    dm_old_crtc_state->stream);
+
+    if (dm_new_crtc_state->stream == NULL || !modeset_needed)
+    continue;


I feel it's a bit future bug prone to repeat the 2 checks above from 
the initial
for_each_crtc_in_state loop, somebody makes changes there and forget 
about this loop.

The array is ugly but it avoids logic duplication.


I think a better fix would be to combine this for_each section with the
one above it. The only thing keeping them separate (not shown in the
above context) is an `if (adev->dm.freesync_module)` guard, which can be
pulled into the first for_each loop. It would also eliminate a lot of
duplicated initialization code.

Thoughts?

Leo



Ignore what I said, It seems I was looking at the wrong context :) The 
fact this blob is being done after dc_commit_state() is annoying...


The maintainability of this duplicated logic is a good point. Although 
I'm not sure if going back to a shared array using custom iterator logic 
is the best approach. Perhapse we can pull this duplicated logic into a 
seperate function?


Thanks,
Leo





Thanks,
Andrey


  if (adev->dm.freesync_module)
  mod_freesync_notify_mode_change(
-    adev->dm.freesync_module, 
_new_crtc_state->stream, 1);

+    adev->dm.freesync_module,
+    _new_crtc_state->stream, 1);
  manage_dm_interrupts(adev, acrtc, true);
  }



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/29] drm/amd/display: Complete TODO item: use new DRM iterator

[Leo]

On 2017-10-26 11:06 PM, Andrey Grodzovsky wrote:



On 2017-10-26 02:34 PM, Harry Wentland wrote:

From: "Leo (Sunpeng) Li" 

Abandon new_crtcs array and use for_each_new iterator to acquire new
crtcs.

Signed-off-by: Leo (Sunpeng) Li 
Reviewed-by: Harry Wentland 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 
+--

  1 file changed, 14 insertions(+), 22 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 442b399a9400..590f80d29b56 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4013,10 +4013,8 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)

  struct amdgpu_display_manager *dm = >dm;
  struct dm_atomic_state *dm_state;
  uint32_t i, j;
-    uint32_t new_crtcs_count = 0;
  struct drm_crtc *crtc;
  struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-    struct amdgpu_crtc *new_crtcs[MAX_STREAMS];
  unsigned long flags;
  bool wait_for_vblank = true;
  struct drm_connector *connector;
@@ -4075,25 +4073,9 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)

  continue;
  }
-
  if (dm_old_crtc_state->stream)
  remove_stream(adev, acrtc, dm_old_crtc_state->stream);
-
-    /*
- * this loop saves set mode crtcs
- * we needed to enable vblanks once all
- * resources acquired in dc after dc_commit_streams
- */
-
-    /*TODO move all this into dm_crtc_state, get rid of
- * new_crtcs array and use old and new atomic states
- * instead
- */
-    new_crtcs[new_crtcs_count] = acrtc;
-    new_crtcs_count++;
-
-    new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
  acrtc->enabled = true;
  acrtc->hw_mode = new_crtc_state->mode;
  crtc->hwmode = new_crtc_state->mode;
@@ -4221,18 +4203,28 @@ static void 
amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
  dm_error("%s: Failed to update stream scaling!\n", 
__func__);

  }
-    for (i = 0; i < new_crtcs_count; i++) {
+    for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
+    new_crtc_state, i) {
  /*
   * loop to enable interrupts on newly arrived crtc
   */
-    struct amdgpu_crtc *acrtc = new_crtcs[i];
+    struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+    bool modeset_needed;
-    new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);

  dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+    dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+    modeset_needed = modeset_required(
+    new_crtc_state,
+    dm_new_crtc_state->stream,
+    dm_old_crtc_state->stream);
+
+    if (dm_new_crtc_state->stream == NULL || !modeset_needed)
+    continue;


I feel it's a bit future bug prone to repeat the 2 checks above from the 
initial
for_each_crtc_in_state loop, somebody makes changes there and forget 
about this loop.

The array is ugly but it avoids logic duplication.


I think a better fix would be to combine this for_each section with the
one above it. The only thing keeping them separate (not shown in the
above context) is an `if (adev->dm.freesync_module)` guard, which can be
pulled into the first for_each loop. It would also eliminate a lot of
duplicated initialization code.

Thoughts?

Leo



Thanks,
Andrey


  if (adev->dm.freesync_module)
  mod_freesync_notify_mode_change(
-    adev->dm.freesync_module, _new_crtc_state->stream, 
1);

+    adev->dm.freesync_module,
+    _new_crtc_state->stream, 1);
  manage_dm_interrupts(adev, acrtc, true);
  }



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/29] drm/amd/display: Complete TODO item: use new DRM iterator

[Andrey Grodzovsky]

On 2017-10-26 02:34 PM, Harry Wentland wrote:

From: "Leo (Sunpeng) Li" 

Abandon new_crtcs array and use for_each_new iterator to acquire new
crtcs.

Signed-off-by: Leo (Sunpeng) Li 
Reviewed-by: Harry Wentland 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 +--
  1 file changed, 14 insertions(+), 22 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 442b399a9400..590f80d29b56 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4013,10 +4013,8 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
struct amdgpu_display_manager *dm = >dm;
struct dm_atomic_state *dm_state;
uint32_t i, j;
-   uint32_t new_crtcs_count = 0;
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-   struct amdgpu_crtc *new_crtcs[MAX_STREAMS];
unsigned long flags;
bool wait_for_vblank = true;
struct drm_connector *connector;
@@ -4075,25 +4073,9 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
continue;
}
  
-

if (dm_old_crtc_state->stream)
remove_stream(adev, acrtc, 
dm_old_crtc_state->stream);
  
-

-   /*
-* this loop saves set mode crtcs
-* we needed to enable vblanks once all
-* resources acquired in dc after dc_commit_streams
-*/
-
-   /*TODO move all this into dm_crtc_state, get rid of
-* new_crtcs array and use old and new atomic states
-* instead
-*/
-   new_crtcs[new_crtcs_count] = acrtc;
-   new_crtcs_count++;
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
acrtc->enabled = true;
acrtc->hw_mode = new_crtc_state->mode;
crtc->hwmode = new_crtc_state->mode;
@@ -4221,18 +4203,28 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
dm_error("%s: Failed to update stream scaling!\n", 
__func__);
}
  
-	for (i = 0; i < new_crtcs_count; i++) {

+   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
+   new_crtc_state, i) {
/*
 * loop to enable interrupts on newly arrived crtc
 */
-   struct amdgpu_crtc *acrtc = new_crtcs[i];
+   struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+   bool modeset_needed;
  
-		new_crtc_state = drm_atomic_get_new_crtc_state(state, >base);

dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+   dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+   modeset_needed = modeset_required(
+   new_crtc_state,
+   dm_new_crtc_state->stream,
+   dm_old_crtc_state->stream);
+
+   if (dm_new_crtc_state->stream == NULL || !modeset_needed)
+   continue;


I feel it's a bit future bug prone to repeat the 2 checks above from the 
initial
for_each_crtc_in_state loop, somebody makes changes there and forget 
about this loop.

The array is ugly but it avoids logic duplication.

Thanks,
Andrey

  
  		if (adev->dm.freesync_module)

mod_freesync_notify_mode_change(
-   adev->dm.freesync_module, 
_new_crtc_state->stream, 1);
+   adev->dm.freesync_module,
+   _new_crtc_state->stream, 1);
  
  		manage_dm_interrupts(adev, acrtc, true);

}


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 03/29] drm/amd/display: Complete TODO item: use new DRM iterator

[Harry Wentland]

From: "Leo (Sunpeng) Li" 

Abandon new_crtcs array and use for_each_new iterator to acquire new
crtcs.

Signed-off-by: Leo (Sunpeng) Li 
Reviewed-by: Harry Wentland 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 +--
 1 file changed, 14 insertions(+), 22 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 442b399a9400..590f80d29b56 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4013,10 +4013,8 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
struct amdgpu_display_manager *dm = >dm;
struct dm_atomic_state *dm_state;
uint32_t i, j;
-   uint32_t new_crtcs_count = 0;
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-   struct amdgpu_crtc *new_crtcs[MAX_STREAMS];
unsigned long flags;
bool wait_for_vblank = true;
struct drm_connector *connector;
@@ -4075,25 +4073,9 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
continue;
}
 
-
if (dm_old_crtc_state->stream)
remove_stream(adev, acrtc, 
dm_old_crtc_state->stream);
 
-
-   /*
-* this loop saves set mode crtcs
-* we needed to enable vblanks once all
-* resources acquired in dc after dc_commit_streams
-*/
-
-   /*TODO move all this into dm_crtc_state, get rid of
-* new_crtcs array and use old and new atomic states
-* instead
-*/
-   new_crtcs[new_crtcs_count] = acrtc;
-   new_crtcs_count++;
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
acrtc->enabled = true;
acrtc->hw_mode = new_crtc_state->mode;
crtc->hwmode = new_crtc_state->mode;
@@ -4221,18 +4203,28 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
dm_error("%s: Failed to update stream scaling!\n", 
__func__);
}
 
-   for (i = 0; i < new_crtcs_count; i++) {
+   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
+   new_crtc_state, i) {
/*
 * loop to enable interrupts on newly arrived crtc
 */
-   struct amdgpu_crtc *acrtc = new_crtcs[i];
+   struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+   bool modeset_needed;
 
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+   dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+   modeset_needed = modeset_required(
+   new_crtc_state,
+   dm_new_crtc_state->stream,
+   dm_old_crtc_state->stream);
+
+   if (dm_new_crtc_state->stream == NULL || !modeset_needed)
+   continue;
 
if (adev->dm.freesync_module)
mod_freesync_notify_mode_change(
-   adev->dm.freesync_module, 
_new_crtc_state->stream, 1);
+   adev->dm.freesync_module,
+   _new_crtc_state->stream, 1);
 
manage_dm_interrupts(adev, acrtc, true);
}
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx