From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Hi Dave,
This series reworks the previous patch. Patch 1 is a v2 of the previous, and additional patches are from the feedback received. They apply on top of your drm-next-amd-dc-staging branch.
Thanks, Leo
Leo (Sunpeng) Li (6): drm/amd/display: Use DRM new-style object iterators. drm/amd/display: Use new DRM API where possible drm/amd/display: Unify DRM state variable namings. drm/amd/display: Unify amdgpu_dm state variable namings. drm/amd/display: Fix typo drm/amd/display: Remove useless pcrtc pointer
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 320 +++++++++++----------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +- 2 files changed, 156 insertions(+), 167 deletions(-)
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
List of affected functions:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var) + struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state( - state, - connector, - conn_state, - i) { - crtc_from_state = - from_state_var ? - conn_state->crtc : - connector->state->crtc; + for_each_new_connector_in_state(state, connector, conn_state, i) { + crtc_from_state = conn_state->crtc;
if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector); @@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags;
/* update planes when needed */ - for_each_new_plane_in_state(state, plane, old_plane_state, i) { + for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb; @@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */ - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state);
new_stream = new_acrtc_state->stream; - aconnector = - amdgpu_dm_find_first_crct_matching_connector( + aconnector = amdgpu_dm_find_first_crct_matching_connector( state, - &new_crtcs[i]->base, - false); + &new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/ - for_each_new_connector_in_state(state, connector, old_conn_state, i) { + for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/ - for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) { + for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state);
if (new_acrtc_state->stream) @@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags); - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (acrtc->base.state->event) @@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
- aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true); + aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc);
/* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect(
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var); + struct drm_crtc *crtc);
struct amdgpu_framebuffer;
On 10/12/2017 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li"sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
List of affected functions:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
It seems to me you missed that one.
Thanks, Andrey
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
Signed-off-by: Leo (Sunpeng) Lisunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var)
- struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state(
state,connector,conn_state,i) {crtc_from_state =from_state_var ?conn_state->crtc :connector->state->crtc;
for_each_new_connector_in_state(state, connector, conn_state, i) {
crtc_from_state = conn_state->crtc;if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags;
/* update planes when needed */
- for_each_new_plane_in_state(state, plane, old_plane_state, i) {
- for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state);
new_stream = new_acrtc_state->stream;
aconnector =amdgpu_dm_find_first_crct_matching_connector(
aconnector = amdgpu_dm_find_first_crct_matching_connector( state,
&new_crtcs[i]->base,false);
&new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n",@@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/
- for_each_new_connector_in_state(state, connector, old_conn_state, i) {
- for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state);
@@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/
- for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state);
if (new_acrtc_state->stream)
@@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (acrtc->base.state->event)
@@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc);/* TODO This hack should go away */ if (aconnector) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect(
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var);
struct drm_crtc *crtc);
struct amdgpu_framebuffer;
-- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 2017-10-13 11:03 AM, Andrey Grodzovsky wrote:
On 10/12/2017 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li"sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
List of affected functions:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
It seems to me you missed that one.
Thanks, Andrey
Good catch, seems like that change was stripped out while I was cp-ing from the internal tree. Some changes in this function have not been promoted to Dave's branch yet.
I'll remove this comment for now, I think it makes sense to have a follow-up patch to this after more changes have been promoted.
Thanks, Leo
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
Signed-off-by: Leo (Sunpeng) Lisunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle) struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var) + struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state; - for_each_new_connector_in_state( - state, - connector, - conn_state, - i) { - crtc_from_state = - from_state_var ? - conn_state->crtc : - connector->state->crtc; + for_each_new_connector_in_state(state, connector, conn_state, i) { + crtc_from_state = conn_state->crtc; if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector); @@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags; /* update planes when needed */ - for_each_new_plane_in_state(state, plane, old_plane_state, i) { + for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb; @@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state); /* update changed items */ - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state; @@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); new_stream = new_acrtc_state->stream; - aconnector = - amdgpu_dm_find_first_crct_matching_connector( + aconnector = amdgpu_dm_find_first_crct_matching_connector( state, - &new_crtcs[i]->base, - false); + &new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( } /* Handle scaling and undersacn changes*/ - for_each_new_connector_in_state(state, connector, old_conn_state, i) { + for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( } /* update planes when needed per crtc*/ - for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) { + for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state); if (new_acrtc_state->stream) @@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags); - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); if (acrtc->base.state->event) @@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc); - aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true); + aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc); /* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect( struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var); + struct drm_crtc *crtc); struct amdgpu_framebuffer; -- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 10/13/2017 11:41 AM, Leo wrote:
On 2017-10-13 11:03 AM, Andrey Grodzovsky wrote:
On 10/12/2017 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li"sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
List of affected functions:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
It seems to me you missed that one.
Thanks, Andrey
Good catch, seems like that change was stripped out while I was cp-ing from the internal tree. Some changes in this function have not been promoted to Dave's branch yet.
I'll remove this comment for now, I think it makes sense to have a follow-up patch to this after more changes have been promoted.
Thanks, Leo
With that fixed the change is Reviewed-by: Andrey Grodzovsky andrey.grodzovsky@amd.com
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
Signed-off-by: Leo (Sunpeng) Lisunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle) struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var)
- struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state(
state,connector,conn_state,i) {crtc_from_state =from_state_var ?conn_state->crtc :connector->state->crtc;
- for_each_new_connector_in_state(state, connector, conn_state, i) {
crtc_from_state = conn_state->crtc; if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector);@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags; /* update planes when needed */
- for_each_new_plane_in_state(state, plane, old_plane_state, i) {
- for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state); /* update changed items */
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); new_stream = new_acrtc_state->stream;
aconnector =amdgpu_dm_find_first_crct_matching_connector(
aconnector = amdgpu_dm_find_first_crct_matching_connector( state,
&new_crtcs[i]->base,false);
&new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to findconnector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( } /* Handle scaling and undersacn changes*/
- for_each_new_connector_in_state(state, connector,
old_conn_state, i) {
- for_each_old_connector_in_state(state, connector,
old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( } /* update planes when needed per crtc*/
- for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
- for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state); if (new_acrtc_state->stream)
@@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); if (acrtc->base.state->event)
@@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
aconnector =amdgpu_dm_find_first_crct_matching_connector(state, crtc, true);
aconnector =amdgpu_dm_find_first_crct_matching_connector(state, crtc); /* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect( struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var);
- struct drm_crtc *crtc); struct amdgpu_framebuffer;
-- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 2017-10-13 11:56 AM, Andrey Grodzovsky wrote:
On 10/13/2017 11:41 AM, Leo wrote:
On 2017-10-13 11:03 AM, Andrey Grodzovsky wrote:
On 10/12/2017 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li"sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
List of affected functions:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
It seems to me you missed that one.
Thanks, Andrey
Good catch, seems like that change was stripped out while I was cp-ing from the internal tree. Some changes in this function have not been promoted to Dave's branch yet.
I'll remove this comment for now, I think it makes sense to have a follow-up patch to this after more changes have been promoted.
Thanks, Leo
With that fixed the change is Reviewed-by: Andrey Grodzovsky andrey.grodzovsky@amd.com
On second look, this comment is addressing the change within Dave's patch, on which this series apply. I was trying to justify all the changes made, including the ones already done by Dave. See here:
https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next-amd-dc-stagin...
I think changing "List of affected functions" to "The following functions were considered" would make it less confusing, and will still make sense if it gets squashed with Dave's patch.
Leo
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
Signed-off-by: Leo (Sunpeng) Lisunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle) struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var) + struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state; - for_each_new_connector_in_state( - state, - connector, - conn_state, - i) { - crtc_from_state = - from_state_var ? - conn_state->crtc : - connector->state->crtc; + for_each_new_connector_in_state(state, connector, conn_state, i) { + crtc_from_state = conn_state->crtc; if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector); @@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags; /* update planes when needed */ - for_each_new_plane_in_state(state, plane, old_plane_state, i) { + for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb; @@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state); /* update changed items */ - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state; @@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); new_stream = new_acrtc_state->stream; - aconnector = - amdgpu_dm_find_first_crct_matching_connector( + aconnector = amdgpu_dm_find_first_crct_matching_connector( state, - &new_crtcs[i]->base, - false); + &new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( } /* Handle scaling and undersacn changes*/ - for_each_new_connector_in_state(state, connector, old_conn_state, i) { + for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( } /* update planes when needed per crtc*/ - for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) { + for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state); if (new_acrtc_state->stream) @@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags); - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); if (acrtc->base.state->event) @@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc); - aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true); + aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc); /* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect( struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var); + struct drm_crtc *crtc); struct amdgpu_framebuffer; -- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 10/13/2017 12:35 PM, Leo wrote:
On 2017-10-13 11:56 AM, Andrey Grodzovsky wrote:
On 10/13/2017 11:41 AM, Leo wrote:
On 2017-10-13 11:03 AM, Andrey Grodzovsky wrote:
On 10/12/2017 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li"sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
List of affected functions:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
It seems to me you missed that one.
Thanks, Andrey
Good catch, seems like that change was stripped out while I was cp-ing from the internal tree. Some changes in this function have not been promoted to Dave's branch yet.
I'll remove this comment for now, I think it makes sense to have a follow-up patch to this after more changes have been promoted.
Thanks, Leo
With that fixed the change is Reviewed-by: Andrey Grodzovsky andrey.grodzovsky@amd.com
On second look, this comment is addressing the change within Dave's patch, on which this series apply. I was trying to justify all the changes made, including the ones already done by Dave. See here:
https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next-amd-dc-stagin...
I think changing "List of affected functions" to "The following functions were considered" would make it less confusing, and will still make sense if it gets squashed with Dave's patch.
Leo
Makes sense to me to.
Thanks, Andrey
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
Signed-off-by: Leo (Sunpeng) Lisunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle) struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var)
- struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state(
state,connector,conn_state,i) {crtc_from_state =from_state_var ?conn_state->crtc :connector->state->crtc;
- for_each_new_connector_in_state(state, connector, conn_state,
i) {
crtc_from_state = conn_state->crtc; if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector);@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags; /* update planes when needed */
- for_each_new_plane_in_state(state, plane, old_plane_state, i) {
- for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state); /* update changed items */
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); new_stream = new_acrtc_state->stream;
aconnector =- amdgpu_dm_find_first_crct_matching_connector(
aconnector =amdgpu_dm_find_first_crct_matching_connector( state,
&new_crtcs[i]->base,false);
&new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to findconnector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( } /* Handle scaling and undersacn changes*/
- for_each_new_connector_in_state(state, connector,
old_conn_state, i) {
- for_each_old_connector_in_state(state, connector,
old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( } /* update planes when needed per crtc*/
- for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
- for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state); if (new_acrtc_state->stream)
@@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); if (acrtc->base.state->event)
@@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
aconnector =amdgpu_dm_find_first_crct_matching_connector(state, crtc, true);
aconnector =amdgpu_dm_find_first_crct_matching_connector(state, crtc); /* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect( struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var);
- struct drm_crtc *crtc); struct amdgpu_framebuffer;
-- 2.7.4 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
The following functions were considered:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
v3: Say "functions considered" instead of "affected functions". The latter implies that changes are made to each.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var) + struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state( - state, - connector, - conn_state, - i) { - crtc_from_state = - from_state_var ? - conn_state->crtc : - connector->state->crtc; + for_each_new_connector_in_state(state, connector, conn_state, i) { + crtc_from_state = conn_state->crtc;
if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector); @@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags;
/* update planes when needed */ - for_each_new_plane_in_state(state, plane, old_plane_state, i) { + for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb; @@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */ - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state);
new_stream = new_acrtc_state->stream; - aconnector = - amdgpu_dm_find_first_crct_matching_connector( + aconnector = amdgpu_dm_find_first_crct_matching_connector( state, - &new_crtcs[i]->base, - false); + &new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/ - for_each_new_connector_in_state(state, connector, old_conn_state, i) { + for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/ - for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) { + for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state);
if (new_acrtc_state->stream) @@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags); - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (acrtc->base.state->event) @@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
- aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true); + aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc);
/* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect(
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var); + struct drm_crtc *crtc);
struct amdgpu_framebuffer;
On 2017-10-13 03:29 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
The following functions were considered:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
v3: Say "functions considered" instead of "affected functions". The latter implies that changes are made to each.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
Patches 1-2 (v3) are also Reviewed-by: Harry Wentland harry.wentland@amd.com
Harry
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var)
- struct drm_crtc *crtc)
{ uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state(
state,connector,conn_state,i) {crtc_from_state =from_state_var ?conn_state->crtc :connector->state->crtc;
for_each_new_connector_in_state(state, connector, conn_state, i) {
crtc_from_state = conn_state->crtc;if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags;
/* update planes when needed */
- for_each_new_plane_in_state(state, plane, old_plane_state, i) {
- for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;
@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state);
new_stream = new_acrtc_state->stream;
aconnector =amdgpu_dm_find_first_crct_matching_connector(
aconnector = amdgpu_dm_find_first_crct_matching_connector( state,
&new_crtcs[i]->base,false);
&new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n",@@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/
- for_each_new_connector_in_state(state, connector, old_conn_state, i) {
- for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state);
@@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/
- for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state);
if (new_acrtc_state->stream)
@@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (acrtc->base.state->event)
@@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc);/* TODO This hack should go away */ if (aconnector) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect(
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var);
- struct drm_crtc *crtc);
struct amdgpu_framebuffer;
On 10/13/2017 03:29 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
The following functions were considered:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
v3: Say "functions considered" instead of "affected functions". The latter implies that changes are made to each.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle)
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var)
- struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state(
state,connector,conn_state,i) {crtc_from_state =from_state_var ?conn_state->crtc :connector->state->crtc;
for_each_new_connector_in_state(state, connector, conn_state, i) {
crtc_from_state = conn_state->crtc;if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector);
@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags;
/* update planes when needed */
- for_each_new_plane_in_state(state, plane, old_plane_state, i) {
- for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
Better just use for_each_new_old here, so you can get both old and new from the iterator.
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state);
new_stream = new_acrtc_state->stream;
aconnector =amdgpu_dm_find_first_crct_matching_connector(
aconnector = amdgpu_dm_find_first_crct_matching_connector( state,
&new_crtcs[i]->base,false);
&new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n",@@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/
- for_each_new_connector_in_state(state, connector, old_conn_state, i) {
- for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state);
@@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/
- for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
- for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state);
Why did you switch to for_each_old iterator here ? if you use the for_each_new iterator you can get rid of new_acrtc_state = to_dm_crtc_state(pcrtc->state); and just use the iterator's state
if (new_acrtc_state->stream)@@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
Same as above
Thanks, Andrey
if (acrtc->base.state->event)@@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc);/* TODO This hack should go away */ if (aconnector) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect(
struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var);
struct drm_crtc *crtc);
struct amdgpu_framebuffer;
On 2017-10-13 04:36 PM, Andrey Grodzovsky wrote:
On 10/13/2017 03:29 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
The following functions were considered:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
v3: Say "functions considered" instead of "affected functions". The latter implies that changes are made to each.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle) struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var) + struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state; - for_each_new_connector_in_state( - state, - connector, - conn_state, - i) { - crtc_from_state = - from_state_var ? - conn_state->crtc : - connector->state->crtc; + for_each_new_connector_in_state(state, connector, conn_state, i) { + crtc_from_state = conn_state->crtc; if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector); @@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags; /* update planes when needed */ - for_each_new_plane_in_state(state, plane, old_plane_state, i) { + for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb; @@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state); /* update changed items */ - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
Better just use for_each_new_old here, so you can get both old and new from the iterator.
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state; @@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); new_stream = new_acrtc_state->stream; - aconnector = - amdgpu_dm_find_first_crct_matching_connector( + aconnector = amdgpu_dm_find_first_crct_matching_connector( state, - &new_crtcs[i]->base, - false); + &new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to find connector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( } /* Handle scaling and undersacn changes*/ - for_each_new_connector_in_state(state, connector, old_conn_state, i) { + for_each_old_connector_in_state(state, connector, old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( } /* update planes when needed per crtc*/ - for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) { + for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state);
Why did you switch to for_each_old iterator here ? if you use the for_each_new iterator you can get rid of new_acrtc_state = to_dm_crtc_state(pcrtc->state); and just use the iterator's state
if (new_acrtc_state->stream) @@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags); - for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
Same as above
Thanks, Andrey
Patches 1/6 and 2/6 should really be squashed, sorry for the confusion. What you've pointed out is addressed in 2/6 :)
Leo
if (acrtc->base.state->event) @@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc); - aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc, true); + aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc); /* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect( struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state, - struct drm_crtc *crtc, - bool from_state_var); + struct drm_crtc *crtc); struct amdgpu_framebuffer;
On 10/13/2017 05:01 PM, Leo wrote:
On 2017-10-13 04:36 PM, Andrey Grodzovsky wrote:
On 10/13/2017 03:29 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use the correct for_each_new/old_* iterators instead of for_each_*
The following functions were considered:
amdgpu_dm_find_first_crtc_matching_connector: use for_each_new - Old from_state_var flag was always choosing the new state
amdgpu_dm_display_resume: use for_each_new - drm_atomic_helper_duplicate_state is called during suspend to cache the state - It sets 'state' within the state triplet to 'new_state'
amdgpu_dm_commit_planes: use for_each_old - Called after the state was swapped (via atomic commit tail)
amdgpu_dm_atomic_commit: use for_each_new - Called before the state is swapped
amdgpu_dm_atomic_commit_tail: use for_each_old - Called after the state was swapped
dm_update_crtcs_state: use for_each_new - Called before the state is swapped (via atomic check)
amdgpu_dm_atomic_check: use for_each_new - Called before the state is swapped
v2: Split out typo fixes to a new patch.
v3: Say "functions considered" instead of "affected functions". The latter implies that changes are made to each.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++++++++--------------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-- 2 files changed, 12 insertions(+), 23 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 9bfe1f9..cc024ab 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -570,23 +570,15 @@ static int dm_suspend(void *handle) struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var)
- struct drm_crtc *crtc) { uint32_t i; struct drm_connector_state *conn_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state(
state,connector,conn_state,i) {crtc_from_state =from_state_var ?conn_state->crtc :connector->state->crtc;
- for_each_new_connector_in_state(state, connector, conn_state, i) {
crtc_from_state = conn_state->crtc; if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector);@@ -3890,7 +3882,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, unsigned long flags; /* update planes when needed */
- for_each_new_plane_in_state(state, plane, old_plane_state, i) {
- for_each_old_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; struct drm_framebuffer *fb = plane_state->fb;
@@ -4024,7 +4016,7 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state); /* update changed items */
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
Better just use for_each_new_old here, so you can get both old and new from the iterator.
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_crtc_state *new_state = crtc->state;@@ -4113,11 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); new_stream = new_acrtc_state->stream;
aconnector =amdgpu_dm_find_first_crct_matching_connector(
aconnector = amdgpu_dm_find_first_crct_matching_connector( state,
&new_crtcs[i]->base,false);
&new_crtcs[i]->base); if (!aconnector) { DRM_DEBUG_DRIVER("Atomic commit: Failed to findconnector for acrtc id:%d " "skipping freesync init\n", @@ -4151,7 +4141,7 @@ void amdgpu_dm_atomic_commit_tail( } /* Handle scaling and undersacn changes*/
- for_each_new_connector_in_state(state, connector,
old_conn_state, i) {
- for_each_old_connector_in_state(state, connector,
old_conn_state, i) { struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); struct dm_connector_state *con_new_state = to_dm_connector_state(aconnector->base.state); @@ -4205,7 +4195,7 @@ void amdgpu_dm_atomic_commit_tail( } /* update planes when needed per crtc*/
- for_each_new_crtc_in_state(state, pcrtc, old_crtc_state, j) {
- for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { new_acrtc_state = to_dm_crtc_state(pcrtc->state);
Why did you switch to for_each_old iterator here ? if you use the for_each_new iterator you can get rid of new_acrtc_state = to_dm_crtc_state(pcrtc->state); and just use the iterator's state
if (new_acrtc_state->stream)@@ -4218,7 +4208,7 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_new_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
Same as above
Thanks, Andrey
Patches 1/6 and 2/6 should really be squashed, sorry for the confusion. What you've pointed out is addressed in 2/6 :)
Leo
Oh, I see.
Thanks, Andrey
if (acrtc->base.state->event)@@ -4402,7 +4392,7 @@ static int dm_update_crtcs_state( new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
aconnector =amdgpu_dm_find_first_crct_matching_connector(state, crtc, true);
aconnector =amdgpu_dm_find_first_crct_matching_connector(state, crtc); /* TODO This hack should go away */ if (aconnector) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index 630e6cd..1c55a0b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -228,8 +228,7 @@ void amdgpu_dm_update_connector_after_detect( struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_atomic_state *state,
- struct drm_crtc *crtc,
- bool from_state_var);
- struct drm_crtc *crtc); struct amdgpu_framebuffer;
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
To conform to DRM's new API, we should not be accessing a DRM object's internal state directly. Rather, the DRM for_each_old/new_* iterators, and drm_atomic_get_old/new_* interface should be used.
This is an ongoing process. For now, update the DRM-facing atomic functions, where the atomic state object is given.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 131 +++++++++++----------- 1 file changed, 66 insertions(+), 65 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 cc024ab..d4426b3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3873,28 +3873,31 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, { uint32_t i; struct drm_plane *plane; - struct drm_plane_state *old_plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; struct dc_stream_state *dc_stream_attach; struct dc_plane_state *plane_states_constructed[MAX_SURFACES]; struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc); - struct dm_crtc_state *acrtc_state = to_dm_crtc_state(pcrtc->state); + struct drm_crtc_state *new_pcrtc_state = + drm_atomic_get_new_crtc_state(state, pcrtc); + struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state); int planes_count = 0; unsigned long flags;
/* update planes when needed */ - for_each_old_plane_in_state(state, plane, old_plane_state, i) { - struct drm_plane_state *plane_state = plane->state; - struct drm_crtc *crtc = plane_state->crtc; - struct drm_framebuffer *fb = plane_state->fb; + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { + struct drm_crtc *crtc = new_plane_state->crtc; + struct drm_crtc_state *new_crtc_state = + drm_atomic_get_new_crtc_state(state, crtc); + struct drm_framebuffer *fb = new_plane_state->fb; bool pflip_needed; - struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state); + struct dm_plane_state *dm_plane_state = to_dm_plane_state(new_plane_state);
if (plane->type == DRM_PLANE_TYPE_CURSOR) { handle_cursor_update(plane, old_plane_state); continue; }
- if (!fb || !crtc || pcrtc != crtc || !crtc->state->active) + if (!fb || !crtc || pcrtc != crtc || !new_crtc_state->active) continue;
pflip_needed = !state->allow_modeset; @@ -3918,13 +3921,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, dc_stream_attach = acrtc_state->stream; planes_count++;
- } else if (crtc->state->planes_changed) { + } else if (new_crtc_state->planes_changed) { /* Assume even ONE crtc with immediate flip means * entire can't wait for VBLANK * TODO Check if it's correct */ *wait_for_vblank = - pcrtc->state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? + new_pcrtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? false : true;
/* TODO: Needs rework for multiplane flip */ @@ -3942,7 +3945,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, if (planes_count) { unsigned long flags;
- if (pcrtc->state->event) { + if (new_pcrtc_state->event) {
drm_crtc_vblank_get(pcrtc);
@@ -3968,7 +3971,7 @@ int amdgpu_dm_atomic_commit( bool nonblock) { struct drm_crtc *crtc; - struct drm_crtc_state *new_state; + struct drm_crtc_state *old_crtc_state, *new_state; struct amdgpu_device *adev = dev->dev_private; int i;
@@ -3979,8 +3982,8 @@ int amdgpu_dm_atomic_commit( * it will update crtc->dm_crtc_state->stream pointer which is used in * the ISRs. */ - for_each_new_crtc_in_state(state, crtc, new_state, i) { - struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(crtc->state); + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_state, i) { + struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(old_crtc_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (drm_atomic_crtc_needs_modeset(new_state) && old_acrtc_state->stream) @@ -4002,13 +4005,13 @@ void amdgpu_dm_atomic_commit_tail( uint32_t i, j; uint32_t new_crtcs_count = 0; struct drm_crtc *crtc, *pcrtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct amdgpu_crtc *new_crtcs[MAX_STREAMS]; struct dc_stream_state *new_stream = NULL; unsigned long flags; bool wait_for_vblank = true; struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *old_conn_state, *new_con_state; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state;
drm_atomic_helper_update_legacy_modeset_state(dev, state); @@ -4016,11 +4019,10 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */ - for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); - struct drm_crtc_state *new_state = crtc->state;
- new_acrtc_state = to_dm_crtc_state(new_state); + new_acrtc_state = to_dm_crtc_state(new_crtc_state); old_acrtc_state = to_dm_crtc_state(old_crtc_state);
DRM_DEBUG_DRIVER( @@ -4028,18 +4030,18 @@ void amdgpu_dm_atomic_commit_tail( "planes_changed:%d, mode_changed:%d,active_changed:%d," "connectors_changed:%d\n", acrtc->crtc_id, - new_state->enable, - new_state->active, - new_state->planes_changed, - new_state->mode_changed, - new_state->active_changed, - new_state->connectors_changed); + new_crtc_state->enable, + new_crtc_state->active, + new_crtc_state->planes_changed, + new_crtc_state->mode_changed, + new_crtc_state->active_changed, + new_crtc_state->connectors_changed);
/* handles headless hotplug case, updating new_state and * aconnector as needed */
- if (modeset_required(new_state, new_acrtc_state->stream, old_acrtc_state->stream)) { + if (modeset_required(new_crtc_state, new_acrtc_state->stream, old_acrtc_state->stream)) {
DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
@@ -4082,10 +4084,11 @@ void amdgpu_dm_atomic_commit_tail( 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 = crtc->state->mode; - crtc->hwmode = crtc->state->mode; - } else if (modereset_required(new_state)) { + acrtc->hw_mode = new_crtc_state->mode; + crtc->hwmode = new_crtc_state->mode; + } else if (modereset_required(new_crtc_state)) { DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
/* i.e. reset mode */ @@ -4102,7 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( for (i = 0; i < new_crtcs_count; i++) { struct amdgpu_dm_connector *aconnector = NULL;
- new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); + new_crtc_state = drm_atomic_get_new_crtc_state(state, + &new_crtcs[i]->base); + new_acrtc_state = to_dm_crtc_state(new_crtc_state);
new_stream = new_acrtc_state->stream; aconnector = amdgpu_dm_find_first_crct_matching_connector( @@ -4123,11 +4128,10 @@ void amdgpu_dm_atomic_commit_tail( if (dm_state->context) WARN_ON(!dc_commit_state(dm->dc, dm_state->context));
- - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- new_acrtc_state = to_dm_crtc_state(crtc->state); + new_acrtc_state = to_dm_crtc_state(new_crtc_state);
if (new_acrtc_state->stream != NULL) { const struct dc_stream_status *status = @@ -4141,24 +4145,24 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/ - for_each_old_connector_in_state(state, connector, old_conn_state, i) { - struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - struct dm_connector_state *con_new_state = - to_dm_connector_state(aconnector->base.state); - struct dm_connector_state *con_old_state = - to_dm_connector_state(old_conn_state); + for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_con_state, i) { + struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state); + struct dm_connector_state *con_old_state = to_dm_connector_state(old_conn_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); struct dc_stream_status *status = NULL;
+ if (acrtc) + new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base); + /* Skip any modesets/resets */ - if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state)) + if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state)) continue;
/* Skip any thing not scale or underscan changes */ if (!is_scaling_state_different(con_new_state, con_old_state)) continue;
- new_acrtc_state = to_dm_crtc_state(acrtc->base.state); + new_acrtc_state = to_dm_crtc_state(new_crtc_state);
update_stream_scaling_settings(&con_new_state->base.crtc->mode, con_new_state, (struct dc_stream_state *)new_acrtc_state->stream); @@ -4185,7 +4189,8 @@ void amdgpu_dm_atomic_commit_tail( */ struct amdgpu_crtc *acrtc = new_crtcs[i];
- new_acrtc_state = to_dm_crtc_state(acrtc->base.state); + new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base); + new_acrtc_state = to_dm_crtc_state(new_crtc_state);
if (adev->dm.freesync_module) mod_freesync_notify_mode_change( @@ -4195,8 +4200,8 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/ - for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { - new_acrtc_state = to_dm_crtc_state(pcrtc->state); + for_each_new_crtc_in_state(state, pcrtc, new_crtc_state, j) { + new_acrtc_state = to_dm_crtc_state(new_crtc_state);
if (new_acrtc_state->stream) amdgpu_dm_commit_planes(state, dev, dm, pcrtc, &wait_for_vblank); @@ -4208,13 +4213,12 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags); - for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
- if (acrtc->base.state->event) - drm_send_event_locked(dev, &crtc->state->event->base); + if (new_crtc_state->event) + drm_send_event_locked(dev, &new_crtc_state->event->base);
- acrtc->base.state->event = NULL; + new_crtc_state->event = NULL; } spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
@@ -4371,7 +4375,7 @@ static int dm_update_crtcs_state( bool *lock_and_validation_needed) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *crtc_state; int i; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); @@ -4380,7 +4384,7 @@ static int dm_update_crtcs_state(
/*TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set */ /* update changed items */ - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) { struct amdgpu_crtc *acrtc = NULL; struct amdgpu_dm_connector *aconnector = NULL; struct drm_connector_state *conn_state = NULL; @@ -4388,7 +4392,7 @@ static int dm_update_crtcs_state(
new_stream = NULL;
- old_acrtc_state = to_dm_crtc_state(crtc->state); + old_acrtc_state = to_dm_crtc_state(old_crtc_state); new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc);
@@ -4521,7 +4525,7 @@ static int dm_update_planes_state( bool *lock_and_validation_needed) { struct drm_crtc *new_plane_crtc, *old_plane_crtc; - struct drm_crtc_state *new_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; struct dm_crtc_state *new_acrtc_state, *old_acrtc_state; @@ -4552,10 +4556,9 @@ static int dm_update_planes_state( if (!old_plane_crtc) continue;
- old_acrtc_state = to_dm_crtc_state( - drm_atomic_get_old_crtc_state( - state, - old_plane_crtc)); + old_crtc_state = drm_atomic_get_old_crtc_state( + state, old_plane_crtc); + old_acrtc_state = to_dm_crtc_state(old_crtc_state);
if (!old_acrtc_state->stream) continue; @@ -4643,7 +4646,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, struct dc *dc = adev->dm.dc; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *old_con_state, *conn_state; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state;
@@ -4710,16 +4713,14 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * new stream into context w\o causing full reset. Need to * decide how to handle. */ - for_each_new_connector_in_state(state, connector, conn_state, i) { - struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - struct dm_connector_state *con_old_state = - to_dm_connector_state(aconnector->base.state); - struct dm_connector_state *con_new_state = - to_dm_connector_state(conn_state); + for_each_oldnew_connector_in_state(state, connector, old_con_state, conn_state, i) { + struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state); + struct dm_connector_state *con_new_state = to_dm_connector_state(conn_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
/* Skip any modesets/resets */ - if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state)) + if (!acrtc || drm_atomic_crtc_needs_modeset( + drm_atomic_get_new_crtc_state(state, &acrtc->base))) continue;
/* Skip any thing not scale or underscan changes */
On 2017-10-12 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
To conform to DRM's new API, we should not be accessing a DRM object's internal state directly. Rather, the DRM for_each_old/new_* iterators, and drm_atomic_get_old/new_* interface should be used.
This is an ongoing process. For now, update the DRM-facing atomic functions, where the atomic state object is given.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 131 +++++++++++----------- 1 file changed, 66 insertions(+), 65 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 cc024ab..d4426b3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3873,28 +3873,31 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, { uint32_t i; struct drm_plane *plane;
- struct drm_plane_state *old_plane_state;
- struct drm_plane_state *old_plane_state, *new_plane_state; struct dc_stream_state *dc_stream_attach; struct dc_plane_state *plane_states_constructed[MAX_SURFACES]; struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc);
- struct dm_crtc_state *acrtc_state = to_dm_crtc_state(pcrtc->state);
struct drm_crtc_state *new_pcrtc_state =
drm_atomic_get_new_crtc_state(state, pcrtc);struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state); int planes_count = 0; unsigned long flags;
/* update planes when needed */
- for_each_old_plane_in_state(state, plane, old_plane_state, i) {
struct drm_plane_state *plane_state = plane->state;struct drm_crtc *crtc = plane_state->crtc;struct drm_framebuffer *fb = plane_state->fb;
- for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
struct drm_crtc *crtc = new_plane_state->crtc;struct drm_crtc_state *new_crtc_state =drm_atomic_get_new_crtc_state(state, crtc);struct drm_framebuffer *fb = new_plane_state->fb;
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
struct dm_plane_state *dm_plane_state = to_dm_plane_state(new_plane_state);if (plane->type == DRM_PLANE_TYPE_CURSOR) { handle_cursor_update(plane, old_plane_state); continue; }
if (!fb || !crtc || pcrtc != crtc || !crtc->state->active)
if (!fb || !crtc || pcrtc != crtc || !new_crtc_state->active) continue;pflip_needed = !state->allow_modeset;
@@ -3918,13 +3921,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, dc_stream_attach = acrtc_state->stream; planes_count++;
} else if (crtc->state->planes_changed) {
} else if (new_crtc_state->planes_changed) { /* Assume even ONE crtc with immediate flip means * entire can't wait for VBLANK * TODO Check if it's correct */ *wait_for_vblank =
pcrtc->state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ?
new_pcrtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? false : true; /* TODO: Needs rework for multiplane flip */@@ -3942,7 +3945,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, if (planes_count) { unsigned long flags;
if (pcrtc->state->event) {
if (new_pcrtc_state->event) { drm_crtc_vblank_get(pcrtc);@@ -3968,7 +3971,7 @@ int amdgpu_dm_atomic_commit( bool nonblock) { struct drm_crtc *crtc;
- struct drm_crtc_state *new_state;
- struct drm_crtc_state *old_crtc_state, *new_state; struct amdgpu_device *adev = dev->dev_private; int i;
@@ -3979,8 +3982,8 @@ int amdgpu_dm_atomic_commit( * it will update crtc->dm_crtc_state->stream pointer which is used in * the ISRs. */
- for_each_new_crtc_in_state(state, crtc, new_state, i) {
struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(crtc->state);
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_state, i) {
struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(old_crtc_state);struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (drm_atomic_crtc_needs_modeset(new_state) && old_acrtc_state->stream)
@@ -4002,13 +4005,13 @@ void amdgpu_dm_atomic_commit_tail( uint32_t i, j; uint32_t new_crtcs_count = 0; struct drm_crtc *crtc, *pcrtc;
- struct drm_crtc_state *old_crtc_state;
- struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct amdgpu_crtc *new_crtcs[MAX_STREAMS]; struct dc_stream_state *new_stream = NULL; unsigned long flags; bool wait_for_vblank = true; struct drm_connector *connector;
- struct drm_connector_state *old_conn_state;
struct drm_connector_state *old_conn_state, *new_con_state; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state;
drm_atomic_helper_update_legacy_modeset_state(dev, state);
@@ -4016,11 +4019,10 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
struct drm_crtc_state *new_state = crtc->state;new_acrtc_state = to_dm_crtc_state(new_state);
new_acrtc_state = to_dm_crtc_state(new_crtc_state);old_acrtc_state = to_dm_crtc_state(old_crtc_state);
DRM_DEBUG_DRIVER(
@@ -4028,18 +4030,18 @@ void amdgpu_dm_atomic_commit_tail( "planes_changed:%d, mode_changed:%d,active_changed:%d," "connectors_changed:%d\n", acrtc->crtc_id,
new_state->enable,new_state->active,new_state->planes_changed,new_state->mode_changed,new_state->active_changed,new_state->connectors_changed);
new_crtc_state->enable,new_crtc_state->active,new_crtc_state->planes_changed,new_crtc_state->mode_changed,new_crtc_state->active_changed,new_crtc_state->connectors_changed);/* handles headless hotplug case, updating new_state and
- aconnector as needed
*/
if (modeset_required(new_state, new_acrtc_state->stream, old_acrtc_state->stream)) {
if (modeset_required(new_crtc_state, new_acrtc_state->stream, old_acrtc_state->stream)) { DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);@@ -4082,10 +4084,11 @@ void amdgpu_dm_atomic_commit_tail( 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 = crtc->state->mode;crtc->hwmode = crtc->state->mode;} else if (modereset_required(new_state)) {
acrtc->hw_mode = new_crtc_state->mode;crtc->hwmode = new_crtc_state->mode;} else if (modereset_required(new_crtc_state)) { DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc); /* i.e. reset mode */@@ -4102,7 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( for (i = 0; i < new_crtcs_count; i++) { struct amdgpu_dm_connector *aconnector = NULL;
new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state);
new_crtc_state = drm_atomic_get_new_crtc_state(state,&new_crtcs[i]->base);new_acrtc_state = to_dm_crtc_state(new_crtc_state); new_stream = new_acrtc_state->stream; aconnector = amdgpu_dm_find_first_crct_matching_connector(@@ -4123,11 +4128,10 @@ void amdgpu_dm_atomic_commit_tail( if (dm_state->context) WARN_ON(!dc_commit_state(dm->dc, dm_state->context));
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
- for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
new_acrtc_state = to_dm_crtc_state(crtc->state);
new_acrtc_state = to_dm_crtc_state(new_crtc_state);if (new_acrtc_state->stream != NULL) { const struct dc_stream_status *status =
@@ -4141,24 +4145,24 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/
- for_each_old_connector_in_state(state, connector, old_conn_state, i) {
struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);struct dm_connector_state *con_new_state =to_dm_connector_state(aconnector->base.state);struct dm_connector_state *con_old_state =to_dm_connector_state(old_conn_state);
for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_con_state, i) {
struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state);struct dm_connector_state *con_old_state = to_dm_connector_state(old_conn_state);struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); struct dc_stream_status *status = NULL;
if (acrtc)new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base);/* Skip any modesets/resets */
if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state))
if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state)) continue;/* Skip any thing not scale or underscan changes */ if (!is_scaling_state_different(con_new_state, con_old_state)) continue;
new_acrtc_state = to_dm_crtc_state(acrtc->base.state);
new_acrtc_state = to_dm_crtc_state(new_crtc_state);update_stream_scaling_settings(&con_new_state->base.crtc->mode, con_new_state, (struct dc_stream_state *)new_acrtc_state->stream);
@@ -4185,7 +4189,8 @@ void amdgpu_dm_atomic_commit_tail( */ struct amdgpu_crtc *acrtc = new_crtcs[i];
new_acrtc_state = to_dm_crtc_state(acrtc->base.state);
new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base);new_acrtc_state = to_dm_crtc_state(new_crtc_state);if (adev->dm.freesync_module) mod_freesync_notify_mode_change(
@@ -4195,8 +4200,8 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/
- for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) {
new_acrtc_state = to_dm_crtc_state(pcrtc->state);
for_each_new_crtc_in_state(state, pcrtc, new_crtc_state, j) {
new_acrtc_state = to_dm_crtc_state(new_crtc_state);if (new_acrtc_state->stream) amdgpu_dm_commit_planes(state, dev, dm, pcrtc, &wait_for_vblank);
@@ -4208,13 +4213,12 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
Andrey might be able to comment on this better, but weren't we intentionally trying to iterate the old crtc here to make sure we send any even on it?
Harry
if (acrtc->base.state->event)drm_send_event_locked(dev, &crtc->state->event->base);
if (new_crtc_state->event)drm_send_event_locked(dev, &new_crtc_state->event->base);
acrtc->base.state->event = NULL;
new_crtc_state->event = NULL;@@ -4371,7 +4375,7 @@ static int dm_update_crtcs_state( bool *lock_and_validation_needed) { struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_crtc_state *old_crtc_state, *crtc_state; int i; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
@@ -4380,7 +4384,7 @@ static int dm_update_crtcs_state(
/*TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set */ /* update changed items */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) { struct amdgpu_crtc *acrtc = NULL; struct amdgpu_dm_connector *aconnector = NULL; struct drm_connector_state *conn_state = NULL;
@@ -4388,7 +4392,7 @@ static int dm_update_crtcs_state(
new_stream = NULL;
old_acrtc_state = to_dm_crtc_state(crtc->state);
old_acrtc_state = to_dm_crtc_state(old_crtc_state);@@ -4521,7 +4525,7 @@ static int dm_update_planes_state( bool *lock_and_validation_needed) { struct drm_crtc *new_plane_crtc, *old_plane_crtc;
- struct drm_crtc_state *new_crtc_state;
- struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; struct dm_crtc_state *new_acrtc_state, *old_acrtc_state;
@@ -4552,10 +4556,9 @@ static int dm_update_planes_state( if (!old_plane_crtc) continue;
old_acrtc_state = to_dm_crtc_state(drm_atomic_get_old_crtc_state(state,old_plane_crtc));
old_crtc_state = drm_atomic_get_old_crtc_state(state, old_plane_crtc);old_acrtc_state = to_dm_crtc_state(old_crtc_state); if (!old_acrtc_state->stream) continue;@@ -4643,7 +4646,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, struct dc *dc = adev->dm.dc; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); struct drm_connector *connector;
- struct drm_connector_state *conn_state;
- struct drm_connector_state *old_con_state, *conn_state; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state;
@@ -4710,16 +4713,14 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * new stream into context w\o causing full reset. Need to * decide how to handle. */
- for_each_new_connector_in_state(state, connector, conn_state, i) {
struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);struct dm_connector_state *con_old_state =to_dm_connector_state(aconnector->base.state);struct dm_connector_state *con_new_state =to_dm_connector_state(conn_state);
for_each_oldnew_connector_in_state(state, connector, old_con_state, conn_state, i) {
struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state);struct dm_connector_state *con_new_state = to_dm_connector_state(conn_state);struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
/* Skip any modesets/resets */
if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state))
if (!acrtc || drm_atomic_crtc_needs_modeset(drm_atomic_get_new_crtc_state(state, &acrtc->base))) continue;/* Skip any thing not scale or underscan changes */
On 10/13/2017 12:18 PM, Harry Wentland wrote:
On 2017-10-12 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
To conform to DRM's new API, we should not be accessing a DRM object's internal state directly. Rather, the DRM for_each_old/new_* iterators, and drm_atomic_get_old/new_* interface should be used.
This is an ongoing process. For now, update the DRM-facing atomic functions, where the atomic state object is given.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 131 +++++++++++----------- 1 file changed, 66 insertions(+), 65 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 cc024ab..d4426b3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3873,28 +3873,31 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, { uint32_t i; struct drm_plane *plane;
- struct drm_plane_state *old_plane_state;
- struct drm_plane_state *old_plane_state, *new_plane_state; struct dc_stream_state *dc_stream_attach; struct dc_plane_state *plane_states_constructed[MAX_SURFACES]; struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc);
- struct dm_crtc_state *acrtc_state = to_dm_crtc_state(pcrtc->state);
struct drm_crtc_state *new_pcrtc_state =
drm_atomic_get_new_crtc_state(state, pcrtc);struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state); int planes_count = 0; unsigned long flags;
/* update planes when needed */
- for_each_old_plane_in_state(state, plane, old_plane_state, i) {
struct drm_plane_state *plane_state = plane->state;struct drm_crtc *crtc = plane_state->crtc;struct drm_framebuffer *fb = plane_state->fb;
- for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
struct drm_crtc *crtc = new_plane_state->crtc;struct drm_crtc_state *new_crtc_state =drm_atomic_get_new_crtc_state(state, crtc);struct drm_framebuffer *fb = new_plane_state->fb;
struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
struct dm_plane_state *dm_plane_state = to_dm_plane_state(new_plane_state);if (plane->type == DRM_PLANE_TYPE_CURSOR) { handle_cursor_update(plane, old_plane_state); continue; }
if (!fb || !crtc || pcrtc != crtc || !crtc->state->active)
if (!fb || !crtc || pcrtc != crtc || !new_crtc_state->active) continue;pflip_needed = !state->allow_modeset;
@@ -3918,13 +3921,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, dc_stream_attach = acrtc_state->stream; planes_count++;
} else if (crtc->state->planes_changed) {
} else if (new_crtc_state->planes_changed) { /* Assume even ONE crtc with immediate flip means * entire can't wait for VBLANK * TODO Check if it's correct */ *wait_for_vblank =
pcrtc->state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ?
new_pcrtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? false : true; /* TODO: Needs rework for multiplane flip */@@ -3942,7 +3945,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, if (planes_count) { unsigned long flags;
if (pcrtc->state->event) {
if (new_pcrtc_state->event) { drm_crtc_vblank_get(pcrtc);@@ -3968,7 +3971,7 @@ int amdgpu_dm_atomic_commit( bool nonblock) { struct drm_crtc *crtc;
- struct drm_crtc_state *new_state;
- struct drm_crtc_state *old_crtc_state, *new_state; struct amdgpu_device *adev = dev->dev_private; int i;
@@ -3979,8 +3982,8 @@ int amdgpu_dm_atomic_commit( * it will update crtc->dm_crtc_state->stream pointer which is used in * the ISRs. */
- for_each_new_crtc_in_state(state, crtc, new_state, i) {
struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(crtc->state);
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_state, i) {
struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(old_crtc_state);struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
if (drm_atomic_crtc_needs_modeset(new_state) && old_acrtc_state->stream)
@@ -4002,13 +4005,13 @@ void amdgpu_dm_atomic_commit_tail( uint32_t i, j; uint32_t new_crtcs_count = 0; struct drm_crtc *crtc, *pcrtc;
- struct drm_crtc_state *old_crtc_state;
- struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct amdgpu_crtc *new_crtcs[MAX_STREAMS]; struct dc_stream_state *new_stream = NULL; unsigned long flags; bool wait_for_vblank = true; struct drm_connector *connector;
- struct drm_connector_state *old_conn_state;
struct drm_connector_state *old_conn_state, *new_con_state; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state;
drm_atomic_helper_update_legacy_modeset_state(dev, state);
@@ -4016,11 +4019,10 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state);
/* update changed items */
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
struct drm_crtc_state *new_state = crtc->state;new_acrtc_state = to_dm_crtc_state(new_state);
new_acrtc_state = to_dm_crtc_state(new_crtc_state);old_acrtc_state = to_dm_crtc_state(old_crtc_state);
DRM_DEBUG_DRIVER(
@@ -4028,18 +4030,18 @@ void amdgpu_dm_atomic_commit_tail( "planes_changed:%d, mode_changed:%d,active_changed:%d," "connectors_changed:%d\n", acrtc->crtc_id,
new_state->enable,new_state->active,new_state->planes_changed,new_state->mode_changed,new_state->active_changed,new_state->connectors_changed);
new_crtc_state->enable,new_crtc_state->active,new_crtc_state->planes_changed,new_crtc_state->mode_changed,new_crtc_state->active_changed,new_crtc_state->connectors_changed);/* handles headless hotplug case, updating new_state and
- aconnector as needed
*/
if (modeset_required(new_state, new_acrtc_state->stream, old_acrtc_state->stream)) {
if (modeset_required(new_crtc_state, new_acrtc_state->stream, old_acrtc_state->stream)) { DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);@@ -4082,10 +4084,11 @@ void amdgpu_dm_atomic_commit_tail( 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 = crtc->state->mode;crtc->hwmode = crtc->state->mode;} else if (modereset_required(new_state)) {
acrtc->hw_mode = new_crtc_state->mode;crtc->hwmode = new_crtc_state->mode;} else if (modereset_required(new_crtc_state)) { DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc); /* i.e. reset mode */@@ -4102,7 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( for (i = 0; i < new_crtcs_count; i++) { struct amdgpu_dm_connector *aconnector = NULL;
new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state);
new_crtc_state = drm_atomic_get_new_crtc_state(state,&new_crtcs[i]->base);new_acrtc_state = to_dm_crtc_state(new_crtc_state); new_stream = new_acrtc_state->stream; aconnector = amdgpu_dm_find_first_crct_matching_connector(@@ -4123,11 +4128,10 @@ void amdgpu_dm_atomic_commit_tail( if (dm_state->context) WARN_ON(!dc_commit_state(dm->dc, dm_state->context));
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
- for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
new_acrtc_state = to_dm_crtc_state(crtc->state);
new_acrtc_state = to_dm_crtc_state(new_crtc_state);if (new_acrtc_state->stream != NULL) { const struct dc_stream_status *status =
@@ -4141,24 +4145,24 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/
- for_each_old_connector_in_state(state, connector, old_conn_state, i) {
struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);struct dm_connector_state *con_new_state =to_dm_connector_state(aconnector->base.state);struct dm_connector_state *con_old_state =to_dm_connector_state(old_conn_state);
for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_con_state, i) {
struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state);struct dm_connector_state *con_old_state = to_dm_connector_state(old_conn_state);struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); struct dc_stream_status *status = NULL;
if (acrtc)new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base);/* Skip any modesets/resets */
if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state))
if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state)) continue;/* Skip any thing not scale or underscan changes */ if (!is_scaling_state_different(con_new_state, con_old_state)) continue;
new_acrtc_state = to_dm_crtc_state(acrtc->base.state);
new_acrtc_state = to_dm_crtc_state(new_crtc_state);update_stream_scaling_settings(&con_new_state->base.crtc->mode, con_new_state, (struct dc_stream_state *)new_acrtc_state->stream);
@@ -4185,7 +4189,8 @@ void amdgpu_dm_atomic_commit_tail( */ struct amdgpu_crtc *acrtc = new_crtcs[i];
new_acrtc_state = to_dm_crtc_state(acrtc->base.state);
new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base);new_acrtc_state = to_dm_crtc_state(new_crtc_state);if (adev->dm.freesync_module) mod_freesync_notify_mode_change(
@@ -4195,8 +4200,8 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/
- for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) {
new_acrtc_state = to_dm_crtc_state(pcrtc->state);
for_each_new_crtc_in_state(state, pcrtc, new_crtc_state, j) {
new_acrtc_state = to_dm_crtc_state(new_crtc_state);if (new_acrtc_state->stream) amdgpu_dm_commit_planes(state, dev, dm, pcrtc, &wait_for_vblank);
@@ -4208,13 +4213,12 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags);
- for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
Andrey might be able to comment on this better, but weren't we intentionally trying to iterate the old crtc here to make sure we send any even on it?
Harry
The states we are interested in are still the new states because that where the event we want to send resides (drm_send_event_locked(dev, &crtc->state->event->base), with the introduction of for_each_new* iterators we should use them to iterate the new states.
Thanks, Andrey
if (acrtc->base.state->event)drm_send_event_locked(dev, &crtc->state->event->base);
if (new_crtc_state->event)drm_send_event_locked(dev, &new_crtc_state->event->base);
acrtc->base.state->event = NULL;
new_crtc_state->event = NULL;@@ -4371,7 +4375,7 @@ static int dm_update_crtcs_state( bool *lock_and_validation_needed) { struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_crtc_state *old_crtc_state, *crtc_state; int i; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
@@ -4380,7 +4384,7 @@ static int dm_update_crtcs_state(
/*TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set */ /* update changed items */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) { struct amdgpu_crtc *acrtc = NULL; struct amdgpu_dm_connector *aconnector = NULL; struct drm_connector_state *conn_state = NULL;
@@ -4388,7 +4392,7 @@ static int dm_update_crtcs_state(
new_stream = NULL;
old_acrtc_state = to_dm_crtc_state(crtc->state);
old_acrtc_state = to_dm_crtc_state(old_crtc_state);@@ -4521,7 +4525,7 @@ static int dm_update_planes_state( bool *lock_and_validation_needed) { struct drm_crtc *new_plane_crtc, *old_plane_crtc;
- struct drm_crtc_state *new_crtc_state;
- struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; struct dm_crtc_state *new_acrtc_state, *old_acrtc_state;
@@ -4552,10 +4556,9 @@ static int dm_update_planes_state( if (!old_plane_crtc) continue;
old_acrtc_state = to_dm_crtc_state(drm_atomic_get_old_crtc_state(state,old_plane_crtc));
old_crtc_state = drm_atomic_get_old_crtc_state(state, old_plane_crtc);old_acrtc_state = to_dm_crtc_state(old_crtc_state); if (!old_acrtc_state->stream) continue;@@ -4643,7 +4646,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, struct dc *dc = adev->dm.dc; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); struct drm_connector *connector;
- struct drm_connector_state *conn_state;
- struct drm_connector_state *old_con_state, *conn_state; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state;
@@ -4710,16 +4713,14 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * new stream into context w\o causing full reset. Need to * decide how to handle. */
- for_each_new_connector_in_state(state, connector, conn_state, i) {
struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);struct dm_connector_state *con_old_state =to_dm_connector_state(aconnector->base.state);struct dm_connector_state *con_new_state =to_dm_connector_state(conn_state);
for_each_oldnew_connector_in_state(state, connector, old_con_state, conn_state, i) {
struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state);struct dm_connector_state *con_new_state = to_dm_connector_state(conn_state);struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
/* Skip any modesets/resets */
if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state))
if (!acrtc || drm_atomic_crtc_needs_modeset(drm_atomic_get_new_crtc_state(state, &acrtc->base))) continue;/* Skip any thing not scale or underscan changes */
On 2017-10-13 01:26 PM, Andrey Grodzovsky wrote:
On 10/13/2017 12:18 PM, Harry Wentland wrote:
On 2017-10-12 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
To conform to DRM's new API, we should not be accessing a DRM object's internal state directly. Rather, the DRM for_each_old/new_* iterators, and drm_atomic_get_old/new_* interface should be used.
This is an ongoing process. For now, update the DRM-facing atomic functions, where the atomic state object is given.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 131 +++++++++++----------- 1 file changed, 66 insertions(+), 65 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 cc024ab..d4426b3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3873,28 +3873,31 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, { uint32_t i; struct drm_plane *plane; - struct drm_plane_state *old_plane_state; + struct drm_plane_state *old_plane_state, *new_plane_state; struct dc_stream_state *dc_stream_attach; struct dc_plane_state *plane_states_constructed[MAX_SURFACES]; struct amdgpu_crtc *acrtc_attach = to_amdgpu_crtc(pcrtc); - struct dm_crtc_state *acrtc_state = to_dm_crtc_state(pcrtc->state); + struct drm_crtc_state *new_pcrtc_state = + drm_atomic_get_new_crtc_state(state, pcrtc); + struct dm_crtc_state *acrtc_state = to_dm_crtc_state(new_pcrtc_state); int planes_count = 0; unsigned long flags; /* update planes when needed */ - for_each_old_plane_in_state(state, plane, old_plane_state, i) { - struct drm_plane_state *plane_state = plane->state; - struct drm_crtc *crtc = plane_state->crtc; - struct drm_framebuffer *fb = plane_state->fb; + for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { + struct drm_crtc *crtc = new_plane_state->crtc; + struct drm_crtc_state *new_crtc_state = + drm_atomic_get_new_crtc_state(state, crtc); + struct drm_framebuffer *fb = new_plane_state->fb; bool pflip_needed; - struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state); + struct dm_plane_state *dm_plane_state = to_dm_plane_state(new_plane_state); if (plane->type == DRM_PLANE_TYPE_CURSOR) { handle_cursor_update(plane, old_plane_state); continue; } - if (!fb || !crtc || pcrtc != crtc || !crtc->state->active) + if (!fb || !crtc || pcrtc != crtc || !new_crtc_state->active) continue; pflip_needed = !state->allow_modeset; @@ -3918,13 +3921,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, dc_stream_attach = acrtc_state->stream; planes_count++; - } else if (crtc->state->planes_changed) { + } else if (new_crtc_state->planes_changed) { /* Assume even ONE crtc with immediate flip means * entire can't wait for VBLANK * TODO Check if it's correct */ *wait_for_vblank = - pcrtc->state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? + new_pcrtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? false : true; /* TODO: Needs rework for multiplane flip */ @@ -3942,7 +3945,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, if (planes_count) { unsigned long flags; - if (pcrtc->state->event) { + if (new_pcrtc_state->event) { drm_crtc_vblank_get(pcrtc); @@ -3968,7 +3971,7 @@ int amdgpu_dm_atomic_commit( bool nonblock) { struct drm_crtc *crtc; - struct drm_crtc_state *new_state; + struct drm_crtc_state *old_crtc_state, *new_state; struct amdgpu_device *adev = dev->dev_private; int i; @@ -3979,8 +3982,8 @@ int amdgpu_dm_atomic_commit( * it will update crtc->dm_crtc_state->stream pointer which is used in * the ISRs. */ - for_each_new_crtc_in_state(state, crtc, new_state, i) { - struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(crtc->state); + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_state, i) { + struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(old_crtc_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); if (drm_atomic_crtc_needs_modeset(new_state) && old_acrtc_state->stream) @@ -4002,13 +4005,13 @@ void amdgpu_dm_atomic_commit_tail( uint32_t i, j; uint32_t new_crtcs_count = 0; struct drm_crtc *crtc, *pcrtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct amdgpu_crtc *new_crtcs[MAX_STREAMS]; struct dc_stream_state *new_stream = NULL; unsigned long flags; bool wait_for_vblank = true; struct drm_connector *connector; - struct drm_connector_state *old_conn_state; + struct drm_connector_state *old_conn_state, *new_con_state; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; drm_atomic_helper_update_legacy_modeset_state(dev, state); @@ -4016,11 +4019,10 @@ void amdgpu_dm_atomic_commit_tail( dm_state = to_dm_atomic_state(state); /* update changed items */ - for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); - struct drm_crtc_state *new_state = crtc->state; - new_acrtc_state = to_dm_crtc_state(new_state); + new_acrtc_state = to_dm_crtc_state(new_crtc_state); old_acrtc_state = to_dm_crtc_state(old_crtc_state); DRM_DEBUG_DRIVER( @@ -4028,18 +4030,18 @@ void amdgpu_dm_atomic_commit_tail( "planes_changed:%d, mode_changed:%d,active_changed:%d," "connectors_changed:%d\n", acrtc->crtc_id, - new_state->enable, - new_state->active, - new_state->planes_changed, - new_state->mode_changed, - new_state->active_changed, - new_state->connectors_changed); + new_crtc_state->enable, + new_crtc_state->active, + new_crtc_state->planes_changed, + new_crtc_state->mode_changed, + new_crtc_state->active_changed, + new_crtc_state->connectors_changed); /* handles headless hotplug case, updating new_state and * aconnector as needed */ - if (modeset_required(new_state, new_acrtc_state->stream, old_acrtc_state->stream)) { + if (modeset_required(new_crtc_state, new_acrtc_state->stream, old_acrtc_state->stream)) { DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc); @@ -4082,10 +4084,11 @@ void amdgpu_dm_atomic_commit_tail( 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 = crtc->state->mode; - crtc->hwmode = crtc->state->mode; - } else if (modereset_required(new_state)) { + acrtc->hw_mode = new_crtc_state->mode; + crtc->hwmode = new_crtc_state->mode; + } else if (modereset_required(new_crtc_state)) { DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc); /* i.e. reset mode */ @@ -4102,7 +4105,9 @@ void amdgpu_dm_atomic_commit_tail( for (i = 0; i < new_crtcs_count; i++) { struct amdgpu_dm_connector *aconnector = NULL; - new_acrtc_state = to_dm_crtc_state(new_crtcs[i]->base.state); + new_crtc_state = drm_atomic_get_new_crtc_state(state, + &new_crtcs[i]->base); + new_acrtc_state = to_dm_crtc_state(new_crtc_state); new_stream = new_acrtc_state->stream; aconnector = amdgpu_dm_find_first_crct_matching_connector( @@ -4123,11 +4128,10 @@ void amdgpu_dm_atomic_commit_tail( if (dm_state->context) WARN_ON(!dc_commit_state(dm->dc, dm_state->context)); - - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); - new_acrtc_state = to_dm_crtc_state(crtc->state); + new_acrtc_state = to_dm_crtc_state(new_crtc_state); if (new_acrtc_state->stream != NULL) { const struct dc_stream_status *status = @@ -4141,24 +4145,24 @@ void amdgpu_dm_atomic_commit_tail( } /* Handle scaling and undersacn changes*/ - for_each_old_connector_in_state(state, connector, old_conn_state, i) { - struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - struct dm_connector_state *con_new_state = - to_dm_connector_state(aconnector->base.state); - struct dm_connector_state *con_old_state = - to_dm_connector_state(old_conn_state); + for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_con_state, i) { + struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state); + struct dm_connector_state *con_old_state = to_dm_connector_state(old_conn_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); struct dc_stream_status *status = NULL; + if (acrtc) + new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base);
/* Skip any modesets/resets */ - if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state)) + if (!acrtc || drm_atomic_crtc_needs_modeset(new_crtc_state)) continue; /* Skip any thing not scale or underscan changes */ if (!is_scaling_state_different(con_new_state, con_old_state)) continue; - new_acrtc_state = to_dm_crtc_state(acrtc->base.state); + new_acrtc_state = to_dm_crtc_state(new_crtc_state); update_stream_scaling_settings(&con_new_state->base.crtc->mode, con_new_state, (struct dc_stream_state *)new_acrtc_state->stream); @@ -4185,7 +4189,8 @@ void amdgpu_dm_atomic_commit_tail( */ struct amdgpu_crtc *acrtc = new_crtcs[i]; - new_acrtc_state = to_dm_crtc_state(acrtc->base.state); + new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base); + new_acrtc_state = to_dm_crtc_state(new_crtc_state); if (adev->dm.freesync_module) mod_freesync_notify_mode_change( @@ -4195,8 +4200,8 @@ void amdgpu_dm_atomic_commit_tail( } /* update planes when needed per crtc*/ - for_each_old_crtc_in_state(state, pcrtc, old_crtc_state, j) { - new_acrtc_state = to_dm_crtc_state(pcrtc->state); + for_each_new_crtc_in_state(state, pcrtc, new_crtc_state, j) { + new_acrtc_state = to_dm_crtc_state(new_crtc_state); if (new_acrtc_state->stream) amdgpu_dm_commit_planes(state, dev, dm, pcrtc, &wait_for_vblank); @@ -4208,13 +4213,12 @@ void amdgpu_dm_atomic_commit_tail( * mark consumed event for drm_atomic_helper_commit_hw_done */ spin_lock_irqsave(&adev->ddev->event_lock, flags); - for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
Andrey might be able to comment on this better, but weren't we intentionally trying to iterate the old crtc here to make sure we send any even on it?
Harry
The states we are interested in are still the new states because that where the event we want to send resides (drm_send_event_locked(dev, &crtc->state->event->base), with the introduction of for_each_new* iterators we should use them to iterate the new states.
I think this just looked odd because of the patches were structured but the logic is sound.
Series is Reviewed-by: Harry Wentland harry.wentland@amd.com
Harry
Thanks, Andrey
- if (acrtc->base.state->event) - drm_send_event_locked(dev, &crtc->state->event->base); + if (new_crtc_state->event) + drm_send_event_locked(dev, &new_crtc_state->event->base); - acrtc->base.state->event = NULL; + new_crtc_state->event = NULL; } spin_unlock_irqrestore(&adev->ddev->event_lock, flags); @@ -4371,7 +4375,7 @@ static int dm_update_crtcs_state( bool *lock_and_validation_needed) { struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *crtc_state; int i; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); @@ -4380,7 +4384,7 @@ static int dm_update_crtcs_state( /*TODO Move this code into dm_crtc_atomic_check once we get rid of dc_validation_set */ /* update changed items */ - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) { struct amdgpu_crtc *acrtc = NULL; struct amdgpu_dm_connector *aconnector = NULL; struct drm_connector_state *conn_state = NULL; @@ -4388,7 +4392,7 @@ static int dm_update_crtcs_state( new_stream = NULL; - old_acrtc_state = to_dm_crtc_state(crtc->state); + old_acrtc_state = to_dm_crtc_state(old_crtc_state); new_acrtc_state = to_dm_crtc_state(crtc_state); acrtc = to_amdgpu_crtc(crtc); @@ -4521,7 +4525,7 @@ static int dm_update_planes_state( bool *lock_and_validation_needed) { struct drm_crtc *new_plane_crtc, *old_plane_crtc; - struct drm_crtc_state *new_crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; struct dm_crtc_state *new_acrtc_state, *old_acrtc_state; @@ -4552,10 +4556,9 @@ static int dm_update_planes_state( if (!old_plane_crtc) continue; - old_acrtc_state = to_dm_crtc_state( - drm_atomic_get_old_crtc_state( - state, - old_plane_crtc)); + old_crtc_state = drm_atomic_get_old_crtc_state( + state, old_plane_crtc); + old_acrtc_state = to_dm_crtc_state(old_crtc_state); if (!old_acrtc_state->stream) continue; @@ -4643,7 +4646,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, struct dc *dc = adev->dm.dc; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); struct drm_connector *connector; - struct drm_connector_state *conn_state; + struct drm_connector_state *old_con_state, *conn_state; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; @@ -4710,16 +4713,14 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * new stream into context w\o causing full reset. Need to * decide how to handle. */ - for_each_new_connector_in_state(state, connector, conn_state, i) { - struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); - struct dm_connector_state *con_old_state = - to_dm_connector_state(aconnector->base.state); - struct dm_connector_state *con_new_state = - to_dm_connector_state(conn_state); + for_each_oldnew_connector_in_state(state, connector, old_con_state, conn_state, i) { + struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state); + struct dm_connector_state *con_new_state = to_dm_connector_state(conn_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); /* Skip any modesets/resets */ - if (!acrtc || drm_atomic_crtc_needs_modeset(acrtc->base.state)) + if (!acrtc || drm_atomic_crtc_needs_modeset( + drm_atomic_get_new_crtc_state(state, &acrtc->base))) continue; /* Skip any thing not scale or underscan changes */
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use new_*_state and old_*_state for their respective new/old DRM object states.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 +++++++++++------------ 1 file changed, 40 insertions(+), 40 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 d4426b3..fe0b658 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -573,12 +573,12 @@ struct amdgpu_dm_connector *amdgpu_dm_find_first_crct_matching_connector( struct drm_crtc *crtc) { uint32_t i; - struct drm_connector_state *conn_state; + struct drm_connector_state *new_con_state; struct drm_connector *connector; struct drm_crtc *crtc_from_state;
- for_each_new_connector_in_state(state, connector, conn_state, i) { - crtc_from_state = conn_state->crtc; + for_each_new_connector_in_state(state, connector, new_con_state, i) { + crtc_from_state = new_con_state->crtc;
if (crtc_from_state == crtc) return to_amdgpu_dm_connector(connector); @@ -608,7 +608,7 @@ int amdgpu_dm_display_resume(struct amdgpu_device *adev) struct amdgpu_dm_connector *aconnector; struct drm_connector *connector; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state; int ret = 0; int i;
@@ -644,8 +644,8 @@ int amdgpu_dm_display_resume(struct amdgpu_device *adev) }
/* Force mode set in atomic comit */ - for_each_new_crtc_in_state(adev->dm.cached_state, crtc, crtc_state, i) - crtc_state->active_changed = true; + for_each_new_crtc_in_state(adev->dm.cached_state, crtc, new_crtc_state, i) + new_crtc_state->active_changed = true;
ret = drm_atomic_helper_resume(ddev, adev->dm.cached_state);
@@ -3971,7 +3971,7 @@ int amdgpu_dm_atomic_commit( bool nonblock) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state, *new_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct amdgpu_device *adev = dev->dev_private; int i;
@@ -3982,11 +3982,11 @@ int amdgpu_dm_atomic_commit( * it will update crtc->dm_crtc_state->stream pointer which is used in * the ISRs. */ - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_state, i) { + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(old_crtc_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- if (drm_atomic_crtc_needs_modeset(new_state) && old_acrtc_state->stream) + if (drm_atomic_crtc_needs_modeset(new_crtc_state) && old_acrtc_state->stream) manage_dm_interrupts(adev, acrtc, false); }
@@ -4011,7 +4011,7 @@ void amdgpu_dm_atomic_commit_tail( unsigned long flags; bool wait_for_vblank = true; struct drm_connector *connector; - struct drm_connector_state *old_conn_state, *new_con_state; + struct drm_connector_state *old_con_state, *new_con_state; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state;
drm_atomic_helper_update_legacy_modeset_state(dev, state); @@ -4145,9 +4145,9 @@ void amdgpu_dm_atomic_commit_tail( }
/* Handle scaling and undersacn changes*/ - for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_con_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state); - struct dm_connector_state *con_old_state = to_dm_connector_state(old_conn_state); + struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); struct dc_stream_status *status = NULL;
@@ -4375,7 +4375,7 @@ static int dm_update_crtcs_state( bool *lock_and_validation_needed) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state, *crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i; struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); @@ -4384,34 +4384,34 @@ static int dm_update_crtcs_state(
/*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, crtc_state, i) { + 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 *conn_state = NULL; + struct drm_connector_state *new_con_state = NULL; struct dm_connector_state *dm_conn_state = NULL;
new_stream = NULL;
old_acrtc_state = to_dm_crtc_state(old_crtc_state); - new_acrtc_state = to_dm_crtc_state(crtc_state); + new_acrtc_state = to_dm_crtc_state(new_crtc_state); acrtc = to_amdgpu_crtc(crtc);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc);
/* TODO This hack should go away */ if (aconnector) { - conn_state = drm_atomic_get_connector_state(state, - &aconnector->base); + new_con_state = drm_atomic_get_connector_state(state, + &aconnector->base);
- if (IS_ERR(conn_state)) { - ret = PTR_ERR_OR_ZERO(conn_state); + if (IS_ERR(new_con_state)) { + ret = PTR_ERR_OR_ZERO(new_con_state); break; }
- dm_conn_state = to_dm_connector_state(conn_state); + dm_conn_state = to_dm_connector_state(new_con_state);
new_stream = create_stream_for_sink(aconnector, - &crtc_state->mode, + &new_crtc_state->mode, dm_conn_state);
/* @@ -4431,14 +4431,14 @@ static int dm_update_crtcs_state( if (dc_is_stream_unchanged(new_stream, old_acrtc_state->stream)) {
- crtc_state->mode_changed = false; + new_crtc_state->mode_changed = false;
- DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d", - crtc_state->mode_changed); + DRM_DEBUG_DRIVER("Mode change not required, setting mode_changed to %d", + new_crtc_state->mode_changed); }
- if (!drm_atomic_crtc_needs_modeset(crtc_state)) + if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) goto next_crtc;
DRM_DEBUG_DRIVER( @@ -4446,12 +4446,12 @@ static int dm_update_crtcs_state( "planes_changed:%d, mode_changed:%d,active_changed:%d," "connectors_changed:%d\n", acrtc->crtc_id, - crtc_state->enable, - crtc_state->active, - crtc_state->planes_changed, - crtc_state->mode_changed, - crtc_state->active_changed, - crtc_state->connectors_changed); + new_crtc_state->enable, + new_crtc_state->active, + new_crtc_state->planes_changed, + new_crtc_state->mode_changed, + new_crtc_state->active_changed, + new_crtc_state->connectors_changed);
/* Remove stream for any changed/disabled CRTC */ if (!enable) { @@ -4478,10 +4478,10 @@ static int dm_update_crtcs_state(
} else {/* Add stream for any updated/enabled CRTC */
- if (modereset_required(crtc_state)) + if (modereset_required(new_crtc_state)) goto next_crtc;
- if (modeset_required(crtc_state, new_stream, + if (modeset_required(new_crtc_state, new_stream, old_acrtc_state->stream)) {
WARN_ON(new_acrtc_state->stream); @@ -4646,9 +4646,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, struct dc *dc = adev->dm.dc; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); struct drm_connector *connector; - struct drm_connector_state *old_con_state, *conn_state; + struct drm_connector_state *old_con_state, *new_con_state; struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *new_crtc_state;
/* * This bool will be set for true for any modeset/reset @@ -4667,8 +4667,8 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * Hack: Commit needs planes right now, specifically for gamma * TODO rework commit to check CRTC for gamma change */ - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { - if (crtc_state->color_mgmt_changed) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { + if (new_crtc_state->color_mgmt_changed) { ret = drm_atomic_add_affected_planes(state, crtc); if (ret) goto fail; @@ -4713,9 +4713,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * new stream into context w\o causing full reset. Need to * decide how to handle. */ - for_each_oldnew_connector_in_state(state, connector, old_con_state, conn_state, i) { + for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state); - struct dm_connector_state *con_new_state = to_dm_connector_state(conn_state); + struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc);
/* Skip any modesets/resets */
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Use dm_new_*_state and dm_old_*_state for their respective amdgpu_dm new and old object states. Helps with readability, and enforces use of new DRM api (choose either new, or old).
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 137 +++++++++++----------- 1 file changed, 68 insertions(+), 69 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 fe0b658..de88ee1 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3890,7 +3890,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, drm_atomic_get_new_crtc_state(state, crtc); struct drm_framebuffer *fb = new_plane_state->fb; bool pflip_needed; - struct dm_plane_state *dm_plane_state = to_dm_plane_state(new_plane_state); + struct dm_plane_state *dm_new_plane_state = to_dm_plane_state(new_plane_state);
if (plane->type == DRM_PLANE_TYPE_CURSOR) { handle_cursor_update(plane, old_plane_state); @@ -3914,9 +3914,9 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
if (!pflip_needed) { - WARN_ON(!dm_plane_state->dc_state); + WARN_ON(!dm_new_plane_state->dc_state);
- plane_states_constructed[planes_count] = dm_plane_state->dc_state; + plane_states_constructed[planes_count] = dm_new_plane_state->dc_state;
dc_stream_attach = acrtc_state->stream; planes_count++; @@ -3983,10 +3983,10 @@ int amdgpu_dm_atomic_commit( * the ISRs. */ for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { - struct dm_crtc_state *old_acrtc_state = to_dm_crtc_state(old_crtc_state); + struct dm_crtc_state *dm_old_crtc_state = to_dm_crtc_state(old_crtc_state); struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- if (drm_atomic_crtc_needs_modeset(new_crtc_state) && old_acrtc_state->stream) + if (drm_atomic_crtc_needs_modeset(new_crtc_state) && dm_old_crtc_state->stream) manage_dm_interrupts(adev, acrtc, false); }
@@ -4012,7 +4012,7 @@ void amdgpu_dm_atomic_commit_tail( bool wait_for_vblank = true; struct drm_connector *connector; struct drm_connector_state *old_con_state, *new_con_state; - struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; + struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state;
drm_atomic_helper_update_legacy_modeset_state(dev, state);
@@ -4022,8 +4022,8 @@ void amdgpu_dm_atomic_commit_tail( for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- new_acrtc_state = to_dm_crtc_state(new_crtc_state); - old_acrtc_state = to_dm_crtc_state(old_crtc_state); + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); + dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
DRM_DEBUG_DRIVER( "amdgpu_crtc id:%d crtc_state_flags: enable:%d, active:%d, " @@ -4041,11 +4041,11 @@ void amdgpu_dm_atomic_commit_tail( * aconnector as needed */
- if (modeset_required(new_crtc_state, new_acrtc_state->stream, old_acrtc_state->stream)) { + if (modeset_required(new_crtc_state, dm_new_crtc_state->stream, dm_old_crtc_state->stream)) {
DRM_DEBUG_DRIVER("Atomic commit: SET crtc id %d: [%p]\n", acrtc->crtc_id, acrtc);
- if (!new_acrtc_state->stream) { + if (!dm_new_crtc_state->stream) { /* * this could happen because of issues with * userspace notifications delivery. @@ -4067,8 +4067,8 @@ void amdgpu_dm_atomic_commit_tail( }
- if (old_acrtc_state->stream) - remove_stream(adev, acrtc, old_acrtc_state->stream); + if (dm_old_crtc_state->stream) + remove_stream(adev, acrtc, dm_old_crtc_state->stream);
/* @@ -4092,8 +4092,8 @@ void amdgpu_dm_atomic_commit_tail( DRM_DEBUG_DRIVER("Atomic commit: RESET. crtc id %d:[%p]\n", acrtc->crtc_id, acrtc);
/* i.e. reset mode */ - if (old_acrtc_state->stream) - remove_stream(adev, acrtc, old_acrtc_state->stream); + if (dm_old_crtc_state->stream) + remove_stream(adev, acrtc, dm_old_crtc_state->stream); } } /* for_each_crtc_in_state() */
@@ -4107,9 +4107,9 @@ void amdgpu_dm_atomic_commit_tail(
new_crtc_state = drm_atomic_get_new_crtc_state(state, &new_crtcs[i]->base); - new_acrtc_state = to_dm_crtc_state(new_crtc_state); + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- new_stream = new_acrtc_state->stream; + new_stream = dm_new_crtc_state->stream; aconnector = amdgpu_dm_find_first_crct_matching_connector( state, &new_crtcs[i]->base); @@ -4131,14 +4131,14 @@ void amdgpu_dm_atomic_commit_tail( for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- new_acrtc_state = to_dm_crtc_state(new_crtc_state); + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- if (new_acrtc_state->stream != NULL) { + if (dm_new_crtc_state->stream != NULL) { const struct dc_stream_status *status = - dc_stream_get_status(new_acrtc_state->stream); + dc_stream_get_status(dm_new_crtc_state->stream);
if (!status) - DC_ERR("got no status for stream %p on acrtc%p\n", new_acrtc_state->stream, acrtc); + DC_ERR("got no status for stream %p on acrtc%p\n", dm_new_crtc_state->stream, acrtc); else acrtc->otg_inst = status->primary_otg_inst; } @@ -4146,9 +4146,9 @@ void amdgpu_dm_atomic_commit_tail(
/* Handle scaling and undersacn changes*/ for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { - struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state); - struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state); - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); + struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state); + struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state); + struct amdgpu_crtc *acrtc = to_amdgpu_crtc(dm_new_con_state->base.crtc); struct dc_stream_status *status = NULL;
if (acrtc) @@ -4159,19 +4159,19 @@ void amdgpu_dm_atomic_commit_tail( continue;
/* Skip any thing not scale or underscan changes */ - if (!is_scaling_state_different(con_new_state, con_old_state)) + if (!is_scaling_state_different(dm_new_con_state, dm_old_con_state)) continue;
- new_acrtc_state = to_dm_crtc_state(new_crtc_state); + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- update_stream_scaling_settings(&con_new_state->base.crtc->mode, - con_new_state, (struct dc_stream_state *)new_acrtc_state->stream); + update_stream_scaling_settings(&dm_new_con_state->base.crtc->mode, + dm_new_con_state, (struct dc_stream_state *)dm_new_crtc_state->stream);
- status = dc_stream_get_status(new_acrtc_state->stream); + status = dc_stream_get_status(dm_new_crtc_state->stream); WARN_ON(!status); WARN_ON(!status->plane_count);
- if (!new_acrtc_state->stream) + if (!dm_new_crtc_state->stream) continue;
/*TODO How it works with MPO ?*/ @@ -4179,7 +4179,7 @@ void amdgpu_dm_atomic_commit_tail( dm->dc, status->plane_states, status->plane_count, - new_acrtc_state->stream)) + dm_new_crtc_state->stream)) dm_error("%s: Failed to update stream scaling!\n", __func__); }
@@ -4190,20 +4190,20 @@ void amdgpu_dm_atomic_commit_tail( struct amdgpu_crtc *acrtc = new_crtcs[i];
new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base); - new_acrtc_state = to_dm_crtc_state(new_crtc_state); + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
if (adev->dm.freesync_module) mod_freesync_notify_mode_change( - adev->dm.freesync_module, &new_acrtc_state->stream, 1); + adev->dm.freesync_module, &dm_new_crtc_state->stream, 1);
manage_dm_interrupts(adev, acrtc, true); }
/* update planes when needed per crtc*/ for_each_new_crtc_in_state(state, pcrtc, new_crtc_state, j) { - new_acrtc_state = to_dm_crtc_state(new_crtc_state); + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- if (new_acrtc_state->stream) + if (dm_new_crtc_state->stream) amdgpu_dm_commit_planes(state, dev, dm, pcrtc, &wait_for_vblank); }
@@ -4377,7 +4377,7 @@ static int dm_update_crtcs_state( struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i; - struct dm_crtc_state *old_acrtc_state, *new_acrtc_state; + struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); struct dc_stream_state *new_stream; int ret = 0; @@ -4392,8 +4392,8 @@ static int dm_update_crtcs_state(
new_stream = NULL;
- old_acrtc_state = to_dm_crtc_state(old_crtc_state); - new_acrtc_state = to_dm_crtc_state(new_crtc_state); + 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);
aconnector = amdgpu_dm_find_first_crct_matching_connector(state, crtc); @@ -4428,8 +4428,7 @@ static int dm_update_crtcs_state( } }
- if (dc_is_stream_unchanged(new_stream, - old_acrtc_state->stream)) { + if (dc_is_stream_unchanged(new_stream, dm_old_crtc_state->stream)) {
new_crtc_state->mode_changed = false;
@@ -4456,7 +4455,7 @@ static int dm_update_crtcs_state( /* Remove stream for any changed/disabled CRTC */ if (!enable) {
- if (!old_acrtc_state->stream) + if (!dm_old_crtc_state->stream) goto next_crtc;
DRM_DEBUG_DRIVER("Disabling DRM crtc: %d\n", @@ -4466,13 +4465,13 @@ static int dm_update_crtcs_state( if (!dc_remove_stream_from_ctx( dc, dm_state->context, - old_acrtc_state->stream)) { + dm_old_crtc_state->stream)) { ret = -EINVAL; goto fail; }
- dc_stream_release(old_acrtc_state->stream); - new_acrtc_state->stream = NULL; + dc_stream_release(dm_old_crtc_state->stream); + dm_new_crtc_state->stream = NULL;
*lock_and_validation_needed = true;
@@ -4482,11 +4481,11 @@ static int dm_update_crtcs_state( goto next_crtc;
if (modeset_required(new_crtc_state, new_stream, - old_acrtc_state->stream)) { + dm_old_crtc_state->stream)) {
- WARN_ON(new_acrtc_state->stream); + WARN_ON(dm_new_crtc_state->stream);
- new_acrtc_state->stream = new_stream; + dm_new_crtc_state->stream = new_stream; dc_stream_retain(new_stream);
DRM_DEBUG_DRIVER("Enabling DRM crtc: %d\n", @@ -4495,7 +4494,7 @@ static int dm_update_crtcs_state( if (!dc_add_stream_to_ctx( dc, dm_state->context, - new_acrtc_state->stream)) { + dm_new_crtc_state->stream)) { ret = -EINVAL; goto fail; } @@ -4528,9 +4527,9 @@ static int dm_update_planes_state( struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; - struct dm_crtc_state *new_acrtc_state, *old_acrtc_state; + struct dm_crtc_state *dm_new_crtc_state, *dm_old_crtc_state; struct dm_atomic_state *dm_state = to_dm_atomic_state(state); - struct dm_plane_state *new_dm_plane_state, *old_dm_plane_state; + struct dm_plane_state *dm_new_plane_state, *dm_old_plane_state; int i ; /* TODO return page_flip_needed() function */ bool pflip_needed = !state->allow_modeset; @@ -4543,8 +4542,8 @@ static int dm_update_planes_state( for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { new_plane_crtc = new_plane_state->crtc; old_plane_crtc = old_plane_state->crtc; - new_dm_plane_state = to_dm_plane_state(new_plane_state); - old_dm_plane_state = to_dm_plane_state(old_plane_state); + dm_new_plane_state = to_dm_plane_state(new_plane_state); + dm_old_plane_state = to_dm_plane_state(old_plane_state);
/*TODO Implement atomic check for cursor plane */ if (plane->type == DRM_PLANE_TYPE_CURSOR) @@ -4558,9 +4557,9 @@ static int dm_update_planes_state(
old_crtc_state = drm_atomic_get_old_crtc_state( state, old_plane_crtc); - old_acrtc_state = to_dm_crtc_state(old_crtc_state); + dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
- if (!old_acrtc_state->stream) + if (!dm_old_crtc_state->stream) continue;
DRM_DEBUG_DRIVER("Disabling DRM plane: %d on DRM crtc %d\n", @@ -4568,8 +4567,8 @@ static int dm_update_planes_state(
if (!dc_remove_plane_from_context( dc, - old_acrtc_state->stream, - old_dm_plane_state->dc_state, + dm_old_crtc_state->stream, + dm_old_plane_state->dc_state, dm_state->context)) {
ret = EINVAL; @@ -4577,8 +4576,8 @@ static int dm_update_planes_state( }
- dc_plane_state_release(old_dm_plane_state->dc_state); - new_dm_plane_state->dc_state = NULL; + dc_plane_state_release(dm_old_plane_state->dc_state); + dm_new_plane_state->dc_state = NULL;
*lock_and_validation_needed = true;
@@ -4591,27 +4590,27 @@ static int dm_update_planes_state( continue;
new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_crtc); - new_acrtc_state = to_dm_crtc_state(new_crtc_state); + dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
- if (!new_acrtc_state->stream) + if (!dm_new_crtc_state->stream) continue;
- WARN_ON(new_dm_plane_state->dc_state); + WARN_ON(dm_new_plane_state->dc_state);
- new_dm_plane_state->dc_state = dc_create_plane_state(dc); + dm_new_plane_state->dc_state = dc_create_plane_state(dc);
DRM_DEBUG_DRIVER("Enabling DRM plane: %d on DRM crtc %d\n", plane->base.id, new_plane_crtc->base.id);
- if (!new_dm_plane_state->dc_state) { + if (!dm_new_plane_state->dc_state) { ret = -EINVAL; return ret; }
ret = fill_plane_attributes( new_plane_crtc->dev->dev_private, - new_dm_plane_state->dc_state, + dm_new_plane_state->dc_state, new_plane_state, new_crtc_state, false); @@ -4621,8 +4620,8 @@ static int dm_update_planes_state(
if (!dc_add_plane_to_context( dc, - new_acrtc_state->stream, - new_dm_plane_state->dc_state, + dm_new_crtc_state->stream, + dm_new_plane_state->dc_state, dm_state->context)) {
ret = -EINVAL; @@ -4714,9 +4713,9 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * decide how to handle. */ for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { - struct dm_connector_state *con_old_state = to_dm_connector_state(old_con_state); - struct dm_connector_state *con_new_state = to_dm_connector_state(new_con_state); - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(con_new_state->base.crtc); + struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state); + struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state); + struct amdgpu_crtc *acrtc = to_amdgpu_crtc(dm_new_con_state->base.crtc);
/* Skip any modesets/resets */ if (!acrtc || drm_atomic_crtc_needs_modeset( @@ -4724,7 +4723,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, continue;
/* Skip any thing not scale or underscan changes */ - if (!is_scaling_state_different(con_new_state, con_old_state)) + if (!is_scaling_state_different(dm_new_con_state, dm_old_con_state)) continue;
lock_and_validation_needed = true;
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
undersacn -> underscan
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 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 de88ee1..67222ff 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4144,7 +4144,7 @@ void amdgpu_dm_atomic_commit_tail( } }
- /* Handle scaling and undersacn changes*/ + /* Handle scaling and underscan changes*/ for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state); struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state); @@ -4707,7 +4707,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, if (ret) goto fail;
- /* Check scaling and undersacn changes*/ + /* Check scaling and underscan changes*/ /*TODO Removed scaling changes validation due to inability to commit * new stream into context w\o causing full reset. Need to * decide how to handle.
On Thu, Oct 12, 2017 at 5:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
undersacn -> underscan
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 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 de88ee1..67222ff 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4144,7 +4144,7 @@ void amdgpu_dm_atomic_commit_tail( } }
/* Handle scaling and undersacn changes*/
/* Handle scaling and underscan changes*/ for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state); struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state);@@ -4707,7 +4707,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, if (ret) goto fail;
/* Check scaling and undersacn changes*/
/* Check scaling and underscan changes*/ /*TODO Removed scaling changes validation due to inability to commit * new stream into context w\o causing full reset. Need to * decide how to handle.-- 2.7.4
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
in amdgpu_dm_atomic_commit_tail. Just use crtc instead.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 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 67222ff..f9b5769 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4004,7 +4004,7 @@ void amdgpu_dm_atomic_commit_tail( struct dm_atomic_state *dm_state; uint32_t i, j; uint32_t new_crtcs_count = 0; - struct drm_crtc *crtc, *pcrtc; + struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct amdgpu_crtc *new_crtcs[MAX_STREAMS]; struct dc_stream_state *new_stream = NULL; @@ -4200,11 +4200,11 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/ - for_each_new_crtc_in_state(state, pcrtc, new_crtc_state, j) { + for_each_new_crtc_in_state(state, crtc, new_crtc_state, j) { dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
if (dm_new_crtc_state->stream) - amdgpu_dm_commit_planes(state, dev, dm, pcrtc, &wait_for_vblank); + amdgpu_dm_commit_planes(state, dev, dm, crtc, &wait_for_vblank); }
On Thu, Oct 12, 2017 at 5:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
in amdgpu_dm_atomic_commit_tail. Just use crtc instead.
Signed-off-by: Leo (Sunpeng) Li sunpeng.li@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 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 67222ff..f9b5769 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4004,7 +4004,7 @@ void amdgpu_dm_atomic_commit_tail( struct dm_atomic_state *dm_state; uint32_t i, j; uint32_t new_crtcs_count = 0;
struct drm_crtc *crtc, *pcrtc;
struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct amdgpu_crtc *new_crtcs[MAX_STREAMS]; struct dc_stream_state *new_stream = NULL;@@ -4200,11 +4200,11 @@ void amdgpu_dm_atomic_commit_tail( }
/* update planes when needed per crtc*/
for_each_new_crtc_in_state(state, pcrtc, new_crtc_state, j) {
for_each_new_crtc_in_state(state, crtc, new_crtc_state, j) { dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); if (dm_new_crtc_state->stream)
amdgpu_dm_commit_planes(state, dev, dm, pcrtc, &wait_for_vblank);
amdgpu_dm_commit_planes(state, dev, dm, crtc, &wait_for_vblank); }-- 2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Patches 3-6 are Reviewed-by: Harry Wentland harry.wentland@amd.com
Harry
On 2017-10-12 05:15 PM, sunpeng.li@amd.com wrote:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Hi Dave,
This series reworks the previous patch. Patch 1 is a v2 of the previous, and additional patches are from the feedback received. They apply on top of your drm-next-amd-dc-staging branch.
Thanks, Leo
Leo (Sunpeng) Li (6): drm/amd/display: Use DRM new-style object iterators. drm/amd/display: Use new DRM API where possible drm/amd/display: Unify DRM state variable namings. drm/amd/display: Unify amdgpu_dm state variable namings. drm/amd/display: Fix typo drm/amd/display: Remove useless pcrtc pointer
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 320 +++++++++++----------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +- 2 files changed, 156 insertions(+), 167 deletions(-)
Op 12-10-17 om 23:15 schreef sunpeng.li@amd.com:
From: "Leo (Sunpeng) Li" sunpeng.li@amd.com
Hi Dave,
This series reworks the previous patch. Patch 1 is a v2 of the previous, and additional patches are from the feedback received. They apply on top of your drm-next-amd-dc-staging branch.
Thanks, Leo
Leo (Sunpeng) Li (6): drm/amd/display: Use DRM new-style object iterators. drm/amd/display: Use new DRM API where possible drm/amd/display: Unify DRM state variable namings. drm/amd/display: Unify amdgpu_dm state variable namings. drm/amd/display: Fix typo drm/amd/display: Remove useless pcrtc pointer
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 320 +++++++++++----------- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +- 2 files changed, 156 insertions(+), 167 deletions(-)
Better, only scanned through the series but
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Andrey Grodzovsky -
Harry Wentland -
Leo -
Maarten Lankhorst -
sunpeng.li＠amd.com