On Wed, Jul 05, 2023 at 06:32:51PM +0300, Imre Deak wrote: > On Thu, May 25, 2023 at 01:10:36PM +0300, Stanislav Lisovskiy wrote: > > According to BSpec 49190, when enabling crtcs, we first setup > > slave and then master crtc, however for disabling it should go > > vice versa, i.e first master, then slave, however current code > > does disabling in a same way as enabling. Fix this, by skipping > > non-master crtcs, instead of non-slaves. > > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovs...@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 0490c6412ab5..68958ba0ef49 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -6662,7 +6662,7 @@ static void intel_commit_modeset_disables(struct > > intel_atomic_state *state) > > */ > > if (!is_trans_port_sync_slave(old_crtc_state) && > > !intel_dp_mst_is_slave_trans(old_crtc_state) && > > - !intel_crtc_is_bigjoiner_slave(old_crtc_state)) > > + !intel_crtc_is_bigjoiner_master(old_crtc_state)) > > I don't see what does this fix. The sequence is correct at the moment > and this change would break it, leaving the encoder PLL enabled > incorrectly when the encoder->post_pll_disable() hook is called. Hence > it's NAK from side.
Citing BSpec: Follow the steps to disable Planes, Pipe, and Transcoder for pipe B and transcoder B(master) Follow the steps to Disable Planes and Pipe for pipe C(slave) Disable port associated with Transcoder B however because of !intel_crtc_is_bigjoiner_slave(old_crtc_state), we are skipping masters and do disabling for the slave crtc first. If fixing that going to break something - that is another story(anyway doesn't mean we shouldn't fix) Stan > > > continue; > > > > intel_old_crtc_state_disables(state, old_crtc_state, > > -- > > 2.37.3 > >
