[Intel-gfx] [PATCH] drm/i915/mtl: Copy c10 phy pll sw state from master to slave for bigjoiner

[Lisovskiy, Stanislav]

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 at intel.com>
> > ---
> >  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