From: Ivan Lipski <[email protected]> [Why&How] The cursor programming path caches the last-written cursor state in SW and, for the enable bit, skips the register write when the cache already matches the request. Under cursor offload the SW cache is updated even though the actual register write is deferred to firmware. When an offloaded cursor update is aborted (e.g. an overlay geometry change reprograms the cursor directly in the same frame), the queued firmware write is dropped but the cache is left stale, so the following direct programming pass skips the enable write and the cursor disappears over the overlay plane.
Add a refresh_cursor_state() hubp/dpp hook that reads the cursor position and attribute registers back from hardware into the SW cache, and call it from dcn35_abort_cursor_offload_update() so the cache is truthful after an abort. This keeps the enable-change optimization intact for all ASICs and is race-robust regardless of whether firmware applied the queued write before the abort. Only dcn60 (the sole offload user) wires up the hook. Assisted-by: Copilot:Claude-Opus-4.8 Reviewed-by: Nicholas Kazlauskas <[email protected]> Signed-off-by: Ivan Lipski <[email protected]> Signed-off-by: George Zhang <[email protected]> --- .../amd/display/dc/dpp/dcn401/dcn401_dpp.h | 2 + .../amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c | 19 +++++++ .../drm/amd/display/dc/dpp/dcn60/dcn60_dpp.c | 1 + .../amd/display/dc/hubp/dcn401/dcn401_hubp.c | 54 +++++++++++++++++++ .../amd/display/dc/hubp/dcn401/dcn401_hubp.h | 2 + .../amd/display/dc/hubp/dcn60/dcn60_hubp.c | 1 + .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c | 11 ++++ drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 2 + drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h | 2 + 9 files changed, 94 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp.h b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp.h index a57eb1cd77a6..e34d291de37b 100644 --- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp.h +++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp.h @@ -725,6 +725,8 @@ void dpp401_set_cursor_position( uint32_t width, uint32_t height); +void dpp401_cursor_refresh_state(struct dpp *dpp_base); + void dpp401_set_optional_cursor_attributes( struct dpp *dpp_base, struct dpp_cursor_attributes *attr); diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c index 340111e3deac..c6de8518108e 100644 --- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c +++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c @@ -144,6 +144,25 @@ void dpp401_set_cursor_position( dpp_base->att.cur0_ctl.bits.cur0_enable = cur_en; } +/* Resync the SW cursor cache from real hardware state. */ +void dpp401_cursor_refresh_state(struct dpp *dpp_base) +{ + struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base); + uint32_t cur0_enable = 0, mode = 0, expansion_mode = 0, rom_en = 0; + + REG_GET_4(CURSOR0_CONTROL, + CUR0_ENABLE, &cur0_enable, + CUR0_MODE, &mode, + CUR0_EXPANSION_MODE, &expansion_mode, + CUR0_ROM_EN, &rom_en); + + dpp_base->pos.cur0_ctl.bits.cur0_enable = cur0_enable; + dpp_base->att.cur0_ctl.bits.cur0_enable = cur0_enable; + dpp_base->att.cur0_ctl.bits.mode = mode; + dpp_base->att.cur0_ctl.bits.expansion_mode = expansion_mode; + dpp_base->att.cur0_ctl.bits.cur0_rom_en = rom_en; +} + void dpp401_set_optional_cursor_attributes( struct dpp *dpp_base, struct dpp_cursor_attributes *attr) diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn60/dcn60_dpp.c b/drivers/gpu/drm/amd/display/dc/dpp/dcn60/dcn60_dpp.c index 24a686e94171..f02aebcedd70 100644 --- a/drivers/gpu/drm/amd/display/dc/dpp/dcn60/dcn60_dpp.c +++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn60/dcn60_dpp.c @@ -308,6 +308,7 @@ static struct dpp_funcs dcn60_dpp_funcs = { .dpp_cnv_set_alpha_keyer = dpp2_cnv_set_alpha_keyer, .set_cursor_attributes = dpp401_set_cursor_attributes, .set_cursor_position = dpp401_set_cursor_position, + .refresh_cursor_state = dpp401_cursor_refresh_state, .set_optional_cursor_attributes = dpp401_set_optional_cursor_attributes, .dpp_dppclk_control = dpp1_dppclk_control, .dpp_set_hdr_multiplier = dpp3_set_hdr_multiplier, diff --git a/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c b/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c index 91e87fb51fe4..798d0dad9153 100644 --- a/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c +++ b/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c @@ -862,6 +862,60 @@ void hubp401_cursor_set_position( hubp->cur_rect.y = param->cur_rect_y; } +void hubp401_cursor_refresh_state(struct hubp *hubp) +{ + struct dcn20_hubp *hubp2 = TO_DCN20_HUBP(hubp); + uint32_t enable = 0, x_pos = 0, y_pos = 0, x_hot = 0, y_hot = 0, dst_x_offset = 0; + uint32_t addr_high = 0, addr_low = 0, width = 0, height = 0; + uint32_t mode = 0, magnify = 0, pitch = 0, lpc = 0; + uint32_t dst_y_offset = 0, chunk_hdl_adjust = 0; + + /* Resync the SW cursor cache from real hardware state. */ + + /* Position */ + REG_GET(CURSOR_CONTROL, CURSOR_ENABLE, &enable); + REG_GET_2(CURSOR_POSITION, + CURSOR_X_POSITION, &x_pos, + CURSOR_Y_POSITION, &y_pos); + REG_GET_2(CURSOR_HOT_SPOT, + CURSOR_HOT_SPOT_X, &x_hot, + CURSOR_HOT_SPOT_Y, &y_hot); + REG_GET(CURSOR_DST_OFFSET, CURSOR_DST_X_OFFSET, &dst_x_offset); + + hubp->pos.cur_ctl.bits.cur_enable = enable; + hubp->pos.position.bits.x_pos = x_pos; + hubp->pos.position.bits.y_pos = y_pos; + hubp->pos.hot_spot.bits.x_hot = x_hot; + hubp->pos.hot_spot.bits.y_hot = y_hot; + hubp->pos.dst_offset.bits.dst_x_offset = dst_x_offset; + + /* Attributes */ + REG_GET(CURSOR_SURFACE_ADDRESS_HIGH, CURSOR_SURFACE_ADDRESS_HIGH, &addr_high); + REG_GET(CURSOR_SURFACE_ADDRESS, CURSOR_SURFACE_ADDRESS, &addr_low); + REG_GET_2(CURSOR_SIZE, + CURSOR_WIDTH, &width, + CURSOR_HEIGHT, &height); + REG_GET_4(CURSOR_CONTROL, + CURSOR_MODE, &mode, + CURSOR_2X_MAGNIFY, &magnify, + CURSOR_PITCH, &pitch, + CURSOR_LINES_PER_CHUNK, &lpc); + REG_GET_2(CURSOR_SETTINGS, + CURSOR0_DST_Y_OFFSET, &dst_y_offset, + CURSOR0_CHUNK_HDL_ADJUST, &chunk_hdl_adjust); + + hubp->att.SURFACE_ADDR_HIGH = addr_high; + hubp->att.SURFACE_ADDR = addr_low; + hubp->att.size.bits.width = width; + hubp->att.size.bits.height = height; + hubp->att.cur_ctl.bits.mode = mode; + hubp->att.cur_ctl.bits.cur_2x_magnify = magnify; + hubp->att.cur_ctl.bits.pitch = pitch; + hubp->att.cur_ctl.bits.line_per_chunk = lpc; + hubp->att.settings.bits.dst_y_offset = dst_y_offset; + hubp->att.settings.bits.chunk_hdl_adjust = chunk_hdl_adjust; +} + void hubp401_read_state(struct hubp *hubp) { struct dcn20_hubp *hubp2 = TO_DCN20_HUBP(hubp); diff --git a/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.h b/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.h index 818cea479272..05078a21a123 100644 --- a/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.h +++ b/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.h @@ -313,6 +313,8 @@ void hubp401_cursor_set_position( const struct dc_cursor_position *pos, const struct dc_cursor_mi_param *param); +void hubp401_cursor_refresh_state(struct hubp *hubp); + void hubp401_read_state(struct hubp *hubp); bool hubp401_construct( diff --git a/drivers/gpu/drm/amd/display/dc/hubp/dcn60/dcn60_hubp.c b/drivers/gpu/drm/amd/display/dc/hubp/dcn60/dcn60_hubp.c index 01b6b3ed5a99..4af60e473bd9 100644 --- a/drivers/gpu/drm/amd/display/dc/hubp/dcn60/dcn60_hubp.c +++ b/drivers/gpu/drm/amd/display/dc/hubp/dcn60/dcn60_hubp.c @@ -427,6 +427,7 @@ static struct hubp_funcs dcn60_hubp_funcs = { .mem_program_viewport = hubp401_set_viewport, .set_cursor_attributes = hubp60_cursor_set_attributes, .set_cursor_position = hubp401_cursor_set_position, + .refresh_cursor_state = hubp401_cursor_refresh_state, .hubp_clk_cntl = hubp2_clk_cntl, .hubp_vtg_sel = hubp2_vtg_sel, .dmdata_set_attributes = hubp3_dmdata_set_attributes, diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c index f9d8ff6ce010..a959d8c37b6a 100644 --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c @@ -1671,6 +1671,17 @@ void dcn35_abort_cursor_offload_update(struct dmub_srv *dmub, struct dpp *dpp, s if (dc->hwss.commit_cursor_offload_update) dc->hwss.commit_cursor_offload_update(dmub, dpp, hubp, stream_idx); + + /* + * The aborted payload is dropped by firmware, so resync the SW cursor + * cache from real hardware state. This lets the next direct + * set_cursor_position re-program CURSOR_ENABLE instead of skipping it + * because of a stale cache. + */ + if (dpp->funcs->refresh_cursor_state) + dpp->funcs->refresh_cursor_state(dpp); + if (hubp && hubp->funcs->refresh_cursor_state) + hubp->funcs->refresh_cursor_state(hubp); } void dcn35_begin_cursor_offload_update(struct dmub_srv *dmub, struct dpp *dpp, struct hubp *hubp, uint32_t stream_idx) diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h index 930b5533256f..71244d3b6c90 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h @@ -330,6 +330,8 @@ struct dpp_funcs { uint32_t height ); + void (*refresh_cursor_state)(struct dpp *dpp_base); + void (*dpp_set_hdr_multiplier)( struct dpp *dpp_base, uint32_t multiplier); diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h b/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h index a648ed3b012a..d1cbcb87d271 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h @@ -219,6 +219,8 @@ struct hubp_funcs { const struct dc_cursor_position *pos, const struct dc_cursor_mi_param *param); + void (*refresh_cursor_state)(struct hubp *hubp); + void (*hubp_disconnect)(struct hubp *hubp); void (*hubp_clk_cntl)(struct hubp *hubp, bool enable); -- 2.55.0
