From: Ovidiu Bunea ovidiu.bunea@amd.com
[ Upstream commit 327aba7f558187e451636c77a1662a2858438dc9 ]
[why & how] Header misalignment in struct dmub_cmd_replay_copy_settings_data and struct dmub_alpm_auxless_data causes incorrect data read between driver and dmub. Fix the misalignment and ensure that everything is aligned to 4-byte boundaries.
Reviewed-by: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Signed-off-by: Ovidiu Bunea ovidiu.bunea@amd.com Signed-off-by: Ivan Lipski ivan.lipski@amd.com Tested-by: Daniel Wheeler daniel.wheeler@amd.com Signed-off-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes and why it matters - The patch corrects structure layout so the driver and DMUB firmware agree on field offsets. The commit message explicitly states that header misalignment in the DMUB command payloads caused “incorrect data read between driver and dmub,” which can lead to wrong parameter values being consumed by firmware for Replay/ALPM sequences — a real user-visible reliability/power bug. - AMD’s DMUB command ABI uses strict 4‑byte alignment across command headers and payloads; this patch brings the new Replay/ALPM-related payloads back into conformance.
- Specific code changes that address the bug - drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h - struct dmub_alpm_auxless_data: add an explicit 1‑byte padding field to enforce 4‑byte alignment. - Added: `uint8_t pad[1];` - Rationale: prior layout had a total size not divisible by 4 (several 16‑bit fields + an 8‑bit field), so the next field in the containing payload could be misaligned. - struct dmub_cmd_replay_copy_settings_data: reorder and align fields and ensure 4‑byte boundary padding at the end. - The two 8‑bit HPO instance fields were moved to follow the `auxless_alpm_data` sub‑structure: - Added here: `uint8_t hpo_stream_enc_inst;` and `uint8_t hpo_link_enc_inst;` - Removed from their earlier position near other `*_inst` fields. - The struct retains explicit padding at the end to maintain 4‑byte alignment: `uint8_t pad[2];` - Net effect: the nested `auxless_alpm_data` is now 4‑byte aligned itself, and the subsequent 8‑bit instance fields and final pad keep the overall payload size and alignment consistent with the DMUB ABI expectations. - struct dmub_rb_cmd_replay_copy_settings remains the same, but now wraps a correctly aligned payload.
- Scope, risk, and stable suitability - Scope is tightly confined to a single header (`dmub_cmd.h`) and to DMUB command payload definitions for Replay/ALPM — no functional logic changes and no architectural churn. - The change follows existing patterns in `dmub_cmd.h`, which already uses explicit “pad” members to guarantee 4‑byte alignment in many payloads. - Regression risk is low: it fixes a clear ABI/layout defect. The only compatibility consideration is driver–firmware agreement; given the commit message states the misalignment caused DMUB to read incorrect data, the fix aligns the driver to firmware’s expected layout rather than introducing a new protocol. - No new features are introduced; this is a correctness fix with minimal code delta.
- Backport guidance - Good candidate for stable backporting to branches that already contain: - `struct dmub_alpm_auxless_data` and - the `hpo_stream_enc_inst`/`hpo_link_enc_inst` fields in `struct dmub_cmd_replay_copy_settings_data`. - Branches that predate these fields (i.e., older DMUB interfaces without AUX‑less ALPM/HPO support) don’t need this patch. - As with all DMUB ABI fixes, ensure the target stable branch’s expected DMUB firmware matches this layout (the fix exists precisely because mismatch caused bad reads).
Given it fixes real misreads at the driver–firmware boundary with a minimal, alignment-only change confined to `drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h`, it meets stable backport criteria.
drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h index 6a69a788abe80..6fa25b0375858 100644 --- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h +++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h @@ -4015,6 +4015,10 @@ struct dmub_alpm_auxless_data { uint16_t lfps_t1_t2_override_us; short lfps_t1_t2_offset_us; uint8_t lttpr_count; + /* + * Padding to align structure to 4 byte boundary. + */ + uint8_t pad[1]; };
/** @@ -4092,6 +4096,14 @@ struct dmub_cmd_replay_copy_settings_data { */ struct dmub_alpm_auxless_data auxless_alpm_data;
+ /** + * @hpo_stream_enc_inst: HPO stream encoder instance + */ + uint8_t hpo_stream_enc_inst; + /** + * @hpo_link_enc_inst: HPO link encoder instance + */ + uint8_t hpo_link_enc_inst; /** * @pad: Align structure to 4 byte boundary. */