Quoting Radhakrishna Sripada (2023-04-20 19:12:48)
>From: Stanislav Lisovskiy <[email protected]>
>
>We try to verify pll registers in sw state for slave crtc with the hw state.
>However in case of bigjoiner we don't calculate those at all, so this 
>verification
>will then always fail.
>So we should either skip the verification for Bigjoiner slave crtc or copy sw 
>state
>from master crtc.
>
>Signed-off-by: Stanislav Lisovskiy <[email protected]>
>Signed-off-by: Radhakrishna Sripada <[email protected]>
>---
> drivers/gpu/drm/i915/display/intel_display.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
>b/drivers/gpu/drm/i915/display/intel_display.c
>index bf391a6cd8d6..83c98791fea3 100644
>--- a/drivers/gpu/drm/i915/display/intel_display.c
>+++ b/drivers/gpu/drm/i915/display/intel_display.c
>@@ -4556,6 +4556,7 @@ copy_bigjoiner_crtc_state_modeset(struct 
>intel_atomic_state *state,
>        drm_mode_copy(&slave_crtc_state->hw.adjusted_mode,
>                      &master_crtc_state->hw.adjusted_mode);
>        slave_crtc_state->hw.scaling_filter = 
> master_crtc_state->hw.scaling_filter;
>+  slave_crtc_state->cx0pll_state = master_crtc_state->cx0pll_state;

We are unconditionally copying cx0pll_state here, which is part of a union.

I haven't checked, but does the verification issue mentioned in the message
above happens only for cx0? In that case, one option would be having the line
above guarded with something that tells us that we are really using the cx0 phys
(maybe DISPLAY_VERSION(i915) >= 14?).

That said, I think it would be even better (and safer) if we correctly copied
the union data no matter which is the currently active member.

> 
>        copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc);
> 
>-- 
>2.34.1
>

Reply via email to