On Tue, May 09, 2023 at 02:14:41PM +0300, Stanislav Lisovskiy wrote: > There is a suspicion that we might not have all bigjoiner pipes > set in correspodent mask, which leads to that not all crtc are added to the state, > however because we are copying for instance crtc reference from master crtc > to slave crtc, we might be trying to get it via intel_atomic_get_new_crtc_state, > which might the return NULL. > This is surely not a fix, but at least the WARN should give us some clue and > "red light" when this happens. > In future we might need to evaluate the logic of adding crtc to the state, > to make sure that we always have all affected crtcs in the state, > even though such functions already exist, there seem to be still some > glitches in this logic. > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_atomic_plane.c | 13 +++++++++++++ > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_display.h | 1 + > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > index 4125ee07a271..03cbd755261b 100644 > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c > @@ -695,6 +695,19 @@ int intel_plane_atomic_check(struct intel_atomic_state *state, > > new_master_plane_state = > intel_atomic_get_new_plane_state(state, master_plane); > + > + /* > + * We would be copying plane state from master crtc > + * however if crtc_state->bigjoiner_pipes doesn't contain both > + * master and slave, that means that quite likely we didn't call > + * intel_atomic_get_crtc_state for both, which can cause issues, > + * like intel_atomic_get_new_crtc_state returning NULL suddently, > + * when we for example try to use hw.crtc from that plane state. > + * This WARN might sched some light on out existing issues, also > + * prevent others from happening in future. > + */ > + drm_WARN_ON(state->base.dev, intel_bigjoiner_num_pipes(new_crtc_state) < 2); What you are doing here is basically just if (bigjoiner_pipes) assert(bigjoiner_pipes != 1); which is not going to catch anything. We can trivially see that it will never happen given how bigjoiner_pipes is initialized. -- Ville Syrjälä Intel
