From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0CB44C43603 for ; Fri, 20 Dec 2019 18:41:19 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B865720866 for ; Fri, 20 Dec 2019 18:41:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B865720866 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 578CB6EC90; Fri, 20 Dec 2019 18:41:18 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id A3A7B6EC8E for ; Fri, 20 Dec 2019 18:41:16 +0000 (UTC) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Dec 2019 10:41:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,336,1571727600"; d="scan'208";a="267591223" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by FMSMGA003.fm.intel.com with SMTP; 20 Dec 2019 10:41:12 -0800 Received: by stinkbox (sSMTP sendmail emulation); Fri, 20 Dec 2019 20:41:12 +0200 Date: Fri, 20 Dec 2019 20:41:12 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Manasi Navare Message-ID: <20191220184112.GZ1208@intel.com> References: <20191219215117.929-1-manasi.d.navare@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20191219215117.929-1-manasi.d.navare@intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [Intel-gfx] [PATCH v2 1/3] drm/i915/dp: Make sure all tiled connectors get added to the state with full modeset X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Thu, Dec 19, 2019 at 01:51:15PM -0800, Manasi Navare wrote: > In case of tiled displays, all the tiles are linke dto each other > for transcoder port sync. So in intel_atomic_check() we need to make > sure that we add all the tiles to the modeset and if one of the > tiles needs a full modeset then mark all other tiles for a full modeset. > = > v2: > * Change crtc_state scope, remove tile_grp_id (Ville) > * Use intel_connector_needs_modeset() (Ville) > * Add modeset_synced_crtcs (Ville) > * Make sure synced crtcs are forced full modeset > after fastset check (Ville) > = > Suggested-by: Ville Syrj=E4l=E4 > Cc: Ville Syrj=E4l=E4 > Cc: Jos=E9 Roberto de Souza > Cc: Matt Roper > Bugzilla: https://gitlab.freedesktop.org/drm/intel/issues/5 > Signed-off-by: Manasi Navare > --- > drivers/gpu/drm/i915/display/intel_display.c | 143 +++++++++++++++++-- > 1 file changed, 131 insertions(+), 12 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/d= rm/i915/display/intel_display.c > index a3f9430493ae..00608d8cef50 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -13910,18 +13910,6 @@ static void intel_crtc_check_fastset(const struc= t intel_crtc_state *old_crtc_sta > new_crtc_state->uapi.mode_changed =3D false; > new_crtc_state->update_pipe =3D true; > = > - /* > - * If we're not doing the full modeset we want to > - * keep the current M/N values as they may be > - * sufficiently different to the computed values > - * to cause problems. > - * > - * FIXME: should really copy more fuzzy state here > - */ > - new_crtc_state->fdi_m_n =3D old_crtc_state->fdi_m_n; > - new_crtc_state->dp_m_n =3D old_crtc_state->dp_m_n; > - new_crtc_state->dp_m2_n2 =3D old_crtc_state->dp_m2_n2; > - new_crtc_state->has_drrs =3D old_crtc_state->has_drrs; > } > = > static int intel_crtc_add_planes_to_state(struct intel_atomic_state *sta= te, > @@ -14032,6 +14020,105 @@ static int intel_atomic_check_crtcs(struct inte= l_atomic_state *state) > return 0; > } > = > +static void > +intel_dp_modeset_synced_crtcs(struct intel_atomic_state *state) > +{ > + struct intel_crtc_state *new_crtc_state; > + struct intel_crtc *crtc; > + int i; > + > + for_each_new_intel_crtc_in_state(state, crtc, > + new_crtc_state, i) { > + if (is_trans_port_sync_mode(new_crtc_state)) { > + new_crtc_state->uapi.mode_changed =3D true; > + new_crtc_state->update_pipe =3D false; > + } Upon further thinking this is not quite right even for the post fastset check part. You're not checking at all whether these crtcs here are actually synced to the crtc that still had its needs_modeset() flagged. So this can end up forcing a modeset on an entirely unrelated crtc. We don't want that. You could fix that by passing in a transcoder bitmask, and then only flagging a modeset on crtcs whose cpu_transcoder matches the bitmask. (see intel_modeset_transcoders() in my branch). > + } > +} > + > +static void > +intel_dp_atomic_check_synced_crtcs(struct intel_atomic_state *state) > +{ > + struct intel_crtc_state *new_crtc_state; > + struct intel_crtc *crtc; > + int i; > + > + for_each_new_intel_crtc_in_state(state, crtc, > + new_crtc_state, i) { > + if (!is_trans_port_sync_mode(new_crtc_state) || > + !needs_modeset(new_crtc_state)) > + continue; > + > + intel_dp_modeset_synced_crtcs(state); > + } > +} > + > +static int > +intel_dp_modeset_all_tiles(struct intel_atomic_state *state, int tile_gr= p_id) > +{ > + struct drm_i915_private *dev_priv =3D to_i915(state->base.dev); > + struct drm_connector *connector; > + struct drm_connector_list_iter conn_list_iter; > + > + drm_connector_list_iter_begin(&dev_priv->drm, &conn_list_iter); > + drm_for_each_connector_iter(connector, &conn_list_iter) { > + struct drm_connector_state *conn_iter_state; > + struct drm_crtc_state *crtc_state; > + > + if (!(connector->has_tile && > + connector->tile_group->id =3D=3D tile_grp_id)) > + continue; > + conn_iter_state =3D drm_atomic_get_connector_state(&state->base, > + connector); > + if (IS_ERR(conn_iter_state)) { > + drm_connector_list_iter_end(&conn_list_iter); > + return PTR_ERR(conn_iter_state); > + } > + > + if (!conn_iter_state->crtc) > + continue; > + > + crtc_state =3D drm_atomic_get_crtc_state(&state->base, > + conn_iter_state->crtc); > + if (IS_ERR(crtc_state)) { > + drm_connector_list_iter_end(&conn_list_iter); > + return PTR_ERR(conn_iter_state); > + } > + crtc_state->mode_changed =3D true; > + } > + drm_connector_list_iter_end(&conn_list_iter); > + > + return 0; > +} > + > +static int > +intel_dp_atomic_check_tiled_conns(struct intel_atomic_state *state) > +{ > + struct drm_i915_private *dev_priv =3D to_i915(state->base.dev); > + struct drm_connector *connector; > + struct drm_connector_state *old_conn_state, *new_conn_state; > + int i, ret; > + > + if (INTEL_GEN(dev_priv) < 11) > + return 0; > + > + /* Is tiled, mark all other tiled CRTCs as needing a modeset */ > + for_each_oldnew_connector_in_state(&state->base, connector, > + old_conn_state, new_conn_state, i) { > + if (!connector->has_tile) > + continue; > + if (!intel_connector_needs_modeset(state, old_conn_state, > + new_conn_state)) > + continue; > + > + ret =3D intel_dp_modeset_all_tiles(state, connector->tile_group->id); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > /** > * intel_atomic_check - validate state object > * @dev: drm device > @@ -14059,6 +14146,12 @@ static int intel_atomic_check(struct drm_device = *dev, > if (ret) > goto fail; > = > + ret =3D intel_dp_atomic_check_tiled_conns(state); > + if (ret) > + goto fail; Since this is not sufficient for the pre-compute_config() check I think we should drop this part for now. It's a somewhat unlikely scenario after all so we can do it as a followup. > + > + intel_dp_atomic_check_synced_crtcs(state); > + > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > if (!needs_modeset(new_crtc_state)) { > @@ -14089,6 +14182,32 @@ static int intel_atomic_check(struct drm_device = *dev, > any_ms =3D true; > } > = > + /* > + * In case of port synced crtcs, if one of the synced crtcs > + * needs a full modeset, all other synced crtcs should be > + * forced a full modeset. > + */ > + intel_dp_atomic_check_synced_crtcs(state); > + > + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > + new_crtc_state, i) { > + if (needs_modeset(new_crtc_state)) > + continue; > + > + /* > + * If we're not doing the full modeset we want to > + * keep the current M/N values as they may be > + * sufficiently different to the computed values > + * to cause problems. > + * > + * FIXME: should really copy more fuzzy state here > + */ > + new_crtc_state->fdi_m_n =3D old_crtc_state->fdi_m_n; > + new_crtc_state->dp_m_n =3D old_crtc_state->dp_m_n; > + new_crtc_state->dp_m2_n2 =3D old_crtc_state->dp_m2_n2; > + new_crtc_state->has_drrs =3D old_crtc_state->has_drrs; > + } > + > if (any_ms && !check_digital_port_conflicts(state)) { > DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n"); > ret =3D EINVAL; > -- = > 2.19.1 -- = Ville Syrj=E4l=E4 Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx