Skip to content

Commit dd141b1

Browse files
Dillon Varonealexdeucher
authored andcommitted
drm/amd/display: Fix race in dmub_srv_wait_for_pending
[WHY] If commands are being submitted to DMCUB while concurrently waiting for pending commands to complete, rptr and wptr may never match again, and reported command count will not update. [HOW] Modify dmub_srv_wait_for_pending to constantly check wptr and rptr match, and update inbox status whenever a message is sent to avoid the race and determine message completion or idle as quickly as possible. Reviewed-by: Nicholas Kazlauskas <[email protected]> Signed-off-by: Dillon Varone <[email protected]> Signed-off-by: Ray Wu <[email protected]> Tested-by: Daniel Wheeler <[email protected]> Signed-off-by: Alex Deucher <[email protected]>
1 parent 7ac37f0 commit dd141b1

File tree

3 files changed

+49
-28
lines changed

3 files changed

+49
-28
lines changed

drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ static bool dc_dmub_srv_fb_cmd_list_queue_execute(struct dc_dmub_srv *dc_dmub_sr
207207
return false;
208208

209209
do {
210-
status = dmub_srv_wait_for_inbox_free(dmub, 100000, count - i);
210+
status = dmub_srv_wait_for_inbox_free(dmub, 100000, count - i);
211211
} while (dc_dmub_srv->ctx->dc->debug.disable_timeout && status != DMUB_STATUS_OK);
212212

213213
/* Requeue the command. */
@@ -247,6 +247,9 @@ bool dc_dmub_srv_cmd_list_queue_execute(struct dc_dmub_srv *dc_dmub_srv,
247247
} else {
248248
res = dc_dmub_srv_fb_cmd_list_queue_execute(dc_dmub_srv, count, cmd_list);
249249
}
250+
251+
if (res)
252+
res = dmub_srv_update_inbox_status(dc_dmub_srv->dmub) == DMUB_STATUS_OK;
250253
}
251254

252255
return res;

drivers/gpu/drm/amd/display/dmub/dmub_srv.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,8 @@ struct dmub_srv_hw_funcs {
445445

446446
uint32_t (*emul_get_inbox1_rptr)(struct dmub_srv *dmub);
447447

448+
uint32_t (*emul_get_inbox1_wptr)(struct dmub_srv *dmub);
449+
448450
void (*emul_set_inbox1_wptr)(struct dmub_srv *dmub, uint32_t wptr_offset);
449451

450452
bool (*is_supported)(struct dmub_srv *dmub);
@@ -1053,4 +1055,16 @@ enum dmub_status dmub_srv_wait_for_inbox_free(struct dmub_srv *dmub,
10531055
uint32_t timeout_us,
10541056
uint32_t num_free_required);
10551057

1058+
/**
1059+
* dmub_srv_update_inbox_status() - Updates pending status for inbox & reg inbox0
1060+
* @dmub: the dmub service
1061+
*
1062+
* Return:
1063+
* DMUB_STATUS_OK - success
1064+
* DMUB_STATUS_TIMEOUT - wait for buffer to flush timed out
1065+
* DMUB_STATUS_HW_FAILURE - issue with HW programming
1066+
* DMUB_STATUS_INVALID - unspecified error
1067+
*/
1068+
enum dmub_status dmub_srv_update_inbox_status(struct dmub_srv *dmub);
1069+
10561070
#endif /* _DMUB_SRV_H_ */

drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -952,10 +952,8 @@ enum dmub_status dmub_srv_wait_for_pending(struct dmub_srv *dmub,
952952
!dmub->hw_funcs.get_inbox1_wptr)
953953
return DMUB_STATUS_INVALID;
954954

955-
/* take a snapshot of the required mailbox state */
956-
scratch_inbox1.rb.wrpt = dmub->hw_funcs.get_inbox1_wptr(dmub);
957-
958955
for (i = 0; i <= timeout_us; i += polling_interval_us) {
956+
scratch_inbox1.rb.wrpt = dmub->hw_funcs.get_inbox1_wptr(dmub);
959957
scratch_inbox1.rb.rptr = dmub->hw_funcs.get_inbox1_rptr(dmub);
960958

961959
scratch_reg_inbox0.is_pending = scratch_reg_inbox0.is_pending &&
@@ -978,30 +976,6 @@ enum dmub_status dmub_srv_wait_for_pending(struct dmub_srv *dmub,
978976
return DMUB_STATUS_TIMEOUT;
979977
}
980978

981-
static enum dmub_status dmub_srv_update_inbox_status(struct dmub_srv *dmub)
982-
{
983-
uint32_t rptr;
984-
985-
/* update inbox1 state */
986-
rptr = dmub->hw_funcs.get_inbox1_rptr(dmub);
987-
988-
if (rptr > dmub->inbox1.rb.capacity)
989-
return DMUB_STATUS_HW_FAILURE;
990-
991-
if (dmub->inbox1.rb.rptr > rptr) {
992-
/* rb wrapped */
993-
dmub->inbox1.num_reported += (rptr + dmub->inbox1.rb.capacity - dmub->inbox1.rb.rptr) / DMUB_RB_CMD_SIZE;
994-
} else {
995-
dmub->inbox1.num_reported += (rptr - dmub->inbox1.rb.rptr) / DMUB_RB_CMD_SIZE;
996-
}
997-
dmub->inbox1.rb.rptr = rptr;
998-
999-
/* update reg_inbox0 */
1000-
dmub_srv_update_reg_inbox0_status(dmub);
1001-
1002-
return DMUB_STATUS_OK;
1003-
}
1004-
1005979
enum dmub_status dmub_srv_wait_for_idle(struct dmub_srv *dmub,
1006980
uint32_t timeout_us)
1007981
{
@@ -1353,3 +1327,33 @@ enum dmub_status dmub_srv_wait_for_inbox_free(struct dmub_srv *dmub,
13531327

13541328
return DMUB_STATUS_TIMEOUT;
13551329
}
1330+
1331+
enum dmub_status dmub_srv_update_inbox_status(struct dmub_srv *dmub)
1332+
{
1333+
uint32_t rptr;
1334+
1335+
if (!dmub->hw_init)
1336+
return DMUB_STATUS_INVALID;
1337+
1338+
if (dmub->power_state != DMUB_POWER_STATE_D0)
1339+
return DMUB_STATUS_POWER_STATE_D3;
1340+
1341+
/* update inbox1 state */
1342+
rptr = dmub->hw_funcs.get_inbox1_rptr(dmub);
1343+
1344+
if (rptr > dmub->inbox1.rb.capacity)
1345+
return DMUB_STATUS_HW_FAILURE;
1346+
1347+
if (dmub->inbox1.rb.rptr > rptr) {
1348+
/* rb wrapped */
1349+
dmub->inbox1.num_reported += (rptr + dmub->inbox1.rb.capacity - dmub->inbox1.rb.rptr) / DMUB_RB_CMD_SIZE;
1350+
} else {
1351+
dmub->inbox1.num_reported += (rptr - dmub->inbox1.rb.rptr) / DMUB_RB_CMD_SIZE;
1352+
}
1353+
dmub->inbox1.rb.rptr = rptr;
1354+
1355+
/* update reg_inbox0 */
1356+
dmub_srv_update_reg_inbox0_status(dmub);
1357+
1358+
return DMUB_STATUS_OK;
1359+
}

0 commit comments

Comments
 (0)