Commit b7408a06 authored by Nicholas Kazlauskas's avatar Nicholas Kazlauskas Committed by Alex Deucher

drm/amd/display: Flush framebuffer data before passing to DMCUB

[Why]
There's a data race that can occur between when we update the
inbox write pointer vs when the memory for the command actually gets
flushed from the map to the framebuffer.

DMCUB can read stale or partially invalid data when this race occurs.

[How]
Before updating the write pointer we can read back all pending commands
to ensure that we stall for the writes to be flushed to framebuffer.

We don't need to worry about choosing HDP vs VM flush with this
mechanism.

Drop the dmub_srv_cmd_submit() while we're updating this to work
correctly since nothing was actually using this API and the caller
should be explicit about the API flow here - by doing this on execute
we can give some extra time for the flush to finish while
preparing other commands.

We should try to avoid writing single commands
because of this overhead.
Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: default avatarTony Cheng <Tony.Cheng@amd.com>
Acked-by: default avatarHarry Wentland <harry.wentland@amd.com>
Acked-by: default avatarRodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 87f24027
...@@ -113,6 +113,23 @@ static inline bool dmub_rb_pop_front(struct dmub_rb *rb) ...@@ -113,6 +113,23 @@ static inline bool dmub_rb_pop_front(struct dmub_rb *rb)
return true; return true;
} }
static inline void dmub_rb_flush_pending(const struct dmub_rb *rb)
{
uint8_t buf[DMUB_RB_CMD_SIZE];
uint32_t rptr = rb->rptr;
uint32_t wptr = rb->wrpt;
while (rptr != wptr) {
const uint8_t *data = (const uint8_t *)rb->base_address + rptr;
dmub_memcpy(buf, data, DMUB_RB_CMD_SIZE);
rptr += DMUB_RB_CMD_SIZE;
if (rptr >= rb->capacity)
rptr %= rb->capacity;
}
}
static inline void dmub_rb_init(struct dmub_rb *rb, static inline void dmub_rb_init(struct dmub_rb *rb,
struct dmub_rb_init_params *init_params) struct dmub_rb_init_params *init_params)
{ {
......
...@@ -444,25 +444,6 @@ enum dmub_status dmub_srv_cmd_queue(struct dmub_srv *dmub, ...@@ -444,25 +444,6 @@ enum dmub_status dmub_srv_cmd_queue(struct dmub_srv *dmub,
*/ */
enum dmub_status dmub_srv_cmd_execute(struct dmub_srv *dmub); enum dmub_status dmub_srv_cmd_execute(struct dmub_srv *dmub);
/**
* dmub_srv_cmd_submit() - submits a command to the DMUB immediately
* @dmub: the dmub service
* @cmd: the command to submit
* @timeout_us: the maximum number of microseconds to wait
*
* Submits a command to the DMUB with an optional timeout.
* If timeout_us is given then the service will attempt to
* resubmit for the given number of microseconds.
*
* Return:
* DMUB_STATUS_OK - success
* DMUB_STATUS_TIMEOUT - wait for submit timed out
* DMUB_STATUS_INVALID - unspecified error
*/
enum dmub_status dmub_srv_cmd_submit(struct dmub_srv *dmub,
const struct dmub_cmd_header *cmd,
uint32_t timeout_us);
/** /**
* dmub_srv_wait_for_auto_load() - Waits for firmware auto load to complete * dmub_srv_wait_for_auto_load() - Waits for firmware auto load to complete
* @dmub: the dmub service * @dmub: the dmub service
......
...@@ -405,33 +405,17 @@ enum dmub_status dmub_srv_cmd_execute(struct dmub_srv *dmub) ...@@ -405,33 +405,17 @@ enum dmub_status dmub_srv_cmd_execute(struct dmub_srv *dmub)
if (!dmub->hw_init) if (!dmub->hw_init)
return DMUB_STATUS_INVALID; return DMUB_STATUS_INVALID;
/**
* Read back all the queued commands to ensure that they've
* been flushed to framebuffer memory. Otherwise DMCUB might
* read back stale, fully invalid or partially invalid data.
*/
dmub_rb_flush_pending(&dmub->inbox1_rb);
dmub->hw_funcs.set_inbox1_wptr(dmub, dmub->inbox1_rb.wrpt); dmub->hw_funcs.set_inbox1_wptr(dmub, dmub->inbox1_rb.wrpt);
return DMUB_STATUS_OK; return DMUB_STATUS_OK;
} }
enum dmub_status dmub_srv_cmd_submit(struct dmub_srv *dmub,
const struct dmub_cmd_header *cmd,
uint32_t timeout_us)
{
uint32_t i = 0;
if (!dmub->hw_init)
return DMUB_STATUS_INVALID;
for (i = 0; i <= timeout_us; ++i) {
dmub->inbox1_rb.rptr = dmub->hw_funcs.get_inbox1_rptr(dmub);
if (dmub_rb_push_front(&dmub->inbox1_rb, cmd)) {
dmub->hw_funcs.set_inbox1_wptr(dmub,
dmub->inbox1_rb.wrpt);
return DMUB_STATUS_OK;
}
udelay(1);
}
return DMUB_STATUS_TIMEOUT;
}
enum dmub_status dmub_srv_wait_for_auto_load(struct dmub_srv *dmub, enum dmub_status dmub_srv_wait_for_auto_load(struct dmub_srv *dmub,
uint32_t timeout_us) uint32_t timeout_us)
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment