| Message ID | 20230420221248.2511314-2-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/mtl: Add the missing CPU transcoder mask in intel_device_info | expand |
On 4/21/2023 3:42 AM, Radhakrishna Sripada wrote: > From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > 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@intel.com> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> Acked-by: Haridhar Kalvala <haridhar.kalvala@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; > > copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc); >
Quoting Radhakrishna Sripada (2023-04-20 19:12:48) >From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > >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@intel.com> >Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@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; 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 >
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; copy_bigjoiner_crtc_state_nomodeset(state, slave_crtc);