From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id D4BAD10E6E0 for ; Wed, 23 Mar 2022 13:52:19 +0000 (UTC) Date: Wed, 23 Mar 2022 15:52:14 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Abhinav Kumar Message-ID: References: <1647991289-19891-1-git-send-email-quic_abhinavk@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1647991289-19891-1-git-send-email-quic_abhinavk@quicinc.com> Subject: Re: [igt-dev] [PATCH i-g-t] lib/igt_kms: commit only the primary plane which is prepared List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: petri.latvala@intel.com, swboyd@chromium.org, igt-dev@lists.freedesktop.org, nganji@codeaurora.org, seanpaul@chromium.org, quic_aravindh@quicinc.com, quic_khsieh@quicinc.com Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Tue, Mar 22, 2022 at 04:21:29PM -0700, Abhinav Kumar wrote: > kms_atomic's test-only sub-test prepares only one primary plane > but igt_plane_commit() tries to commit all the primary planes. > > For drivers having only one primary plane per display still > work fine but when there is more than one primary plane, since > FB_ID is not assigned for the second one, the API > igt_primary_plane_commit_legacy() ends up calling the CRTC > disable path incorrectly and hence failing the test. > > Since only one primary plane has been prepared, commit only > that one by matching the index correctly. > > Signed-off-by: Abhinav Kumar > --- > lib/igt_kms.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 34a2aa0..98a6e1b 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -3189,6 +3189,8 @@ static int igt_plane_commit(igt_plane_t *plane, > enum igt_commit_style s, > bool fail_on_error) > { > + igt_plane_t *plane_primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY); > + > if (pipe->display->first_commit || (s == COMMIT_UNIVERSAL && > igt_plane_is_prop_changed(plane, IGT_PLANE_ROTATION))) { > int ret; > @@ -3199,7 +3201,8 @@ static int igt_plane_commit(igt_plane_t *plane, > > if (plane->type == DRM_PLANE_TYPE_CURSOR && s == COMMIT_LEGACY) { > return igt_cursor_commit_legacy(plane, pipe, fail_on_error); > - } else if (plane->type == DRM_PLANE_TYPE_PRIMARY && s == COMMIT_LEGACY) { > + } else if (plane->type == DRM_PLANE_TYPE_PRIMARY && s == COMMIT_LEGACY && > + (plane->index == plane_primary->index)) { I think you should be able to simplify that to just 'plane == plane_primary && s == COMMIT_LEGACY' > return igt_primary_plane_commit_legacy(plane, pipe, > fail_on_error); > } else { > -- > 2.7.4 -- Ville Syrjälä Intel