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@xxxxxxxxxxxxxxx>
Manasi
> >
> >
> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > 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@xxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
