Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: Modeset only the tiled connectors with CRTC

[Manasi Navare <manasi.d.navare@intel.com>]

On Thu, Jan 23, 2020 at 02:47:04PM -0800, Manasi Navare wrote:
> On Thu, Jan 23, 2020 at 08:27:12PM +0200, Ville Syrjälä wrote:
> > On Wed, Jan 22, 2020 at 04:24:15PM -0800, Manasi Navare wrote:
> > 
> > Missing commit msg.
> 
> Yes will add the commit message
>

Capturing an Ack from Ville from IRC after adding the commit message:

Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Manasi
 
> > 
> > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Fixes: a603f5bd1691 ("drm/i915/dp: Make sure all tiled connectors get added to the state with full modeset")
> > > Closes: https://gitlab.freedesktop.org/drm/intel/issues/516
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 79f9054078ea..c8d6f6e8fc7f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -13155,11 +13155,12 @@ intel_modeset_pipe_config(struct intel_crtc_state *pipe_config)
> > >  	}
> > >  
> > >  	/* Get total number of tiled connectors in state that belong to
> > > -	 * this tile group.
> > > +	 * this tile group and that have a CRTC
> > >  	 */
> > >  	for_each_new_connector_in_state(state, connector, connector_state, i) {
> > >  		if (connector->has_tile &&
> > > -		    connector->tile_group->id == tile_group_id)
> > > +		    connector->tile_group->id == tile_group_id &&
> > > +		    connector_state->crtc)
> > 
> > It might have a crtc, but that crtc might not be active. So feels like
> > this fixes things more by luck that by design.
> 
> In case of kms_flip test cases, the second connector did not have crtc and it was
> still getting added to the state trying to do commit on that causing it to fail.
> 
> So adding this check fixes it. So I think this fix is definitely needed plus I will look at
> adding a check for crtc_active using your suggested approach in your branch too.
> 
> > 
> > The real problem of course is that we can't tell whether other crtcs are
> > active or not during intel_modeset_pipe_config(). All pipes must finish
> > that stage before we can make an actual decision based on what's active
> > and what's inactive. And that requires the two stage approach I proposed
> > in my branch before xmas.
> 
> Perhaps I was thinking of adding another check after intel_modeset_pipe_config() but in intel_atomic_check()
> something like intel_port_sync_check() where we count the number of conns with active crtc and if that is
> less than the num tiles and if trans_port_sync mode, then reset port sync mode assignments so it commits
> in the normal non port sync mode.
> What do you think of this approach? I will also take a look at your branch and try to understand that.
> 
> Manasi
> 
> > 
> > >  			num_tiled_conns++;
> > >  	}
> > >  
> > > @@ -14507,13 +14508,14 @@ intel_modeset_all_tiles(struct intel_atomic_state *state, int tile_grp_id)
> > >  			continue;
> > >  		conn_state = drm_atomic_get_connector_state(&state->base,
> > >  							    connector);
> > > +
> > >  		if (IS_ERR(conn_state)) {
> > >  			ret =  PTR_ERR(conn_state);
> > >  			break;
> > >  		}
> > >  
> > >  		if (!conn_state->crtc)
> > > -			continue;
> > > +			break;
> > >  
> > >  		crtc_state = drm_atomic_get_crtc_state(&state->base,
> > >  						       conn_state->crtc);
> > > @@ -14550,6 +14552,8 @@ intel_atomic_check_tiled_conns(struct intel_atomic_state *state)
> > >  			continue;
> > >  		if (!intel_connector_needs_modeset(state, connector))
> > >  			continue;
> > > +		if (!new_conn_state->crtc)
> > > +			continue;
> > >  
> > >  		ret = intel_modeset_all_tiles(state, connector->tile_group->id);
> > >  		if (ret)
> > > -- 
> > > 2.19.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx