On Fri, Apr 21, 2023 at 12:26:08PM +0300, Ville Syrjälä wrote: > On Fri, Apr 21, 2023 at 11:35:20AM +0300, Stanislav Lisovskiy wrote: > > 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 <stanislav.lisovskiy@xxxxxxxxx> > > --- > > 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; > > Wrong place. Also we're already copying dpll_hw_state which is in the > same union, and on first blush looks bigger than this thing. So why is > that not working? No we aren't copying it, we are "saving" it earlier, however it doesn't help at all: /* preserve some things from the slave's original crtc state */ saved_state->uapi = slave_crtc_state->uapi; saved_state->scaler_state = slave_crtc_state->scaler_state; saved_state->shared_dpll = slave_crtc_state->shared_dpll; saved_state->dpll_hw_state = slave_crtc_state->dpll_hw_state; saved_state->crc_enabled = slave_crtc_state->crc_enabled; intel_crtc_free_hw_state(slave_crtc_state); memcpy(slave_crtc_state, saved_state, sizeof(*slave_crtc_state)); kfree(saved_state); /* Re-init hw state */ memset(&slave_crtc_state->hw, 0, sizeof(slave_crtc_state->hw)); slave_crtc_state->hw.enable = master_crtc_state->hw.enable; slave_crtc_state->hw.active = master_crtc_state->hw.active; because I guess it just didn't have that stuff initially. It starts to work without those verify WARN's only if I copy it from master_crtc_state. Stan > > > > > copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc); > > > > -- > > 2.37.3 > > -- > Ville Syrjälä > Intel
