Re: [Intel-gfx] [PATCH] drm/i915: Fix the disabling sequence for Bigjoiner

[Imre Deak <imre.deak@intel.com>]

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.lisovskiy@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.

>  			continue;
>  
>  		intel_old_crtc_state_disables(state, old_crtc_state,
> -- 
> 2.37.3
>