From patchwork Mon Feb 13 09:05:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Lankhorst, Maarten" X-Patchwork-Id: 9569023 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2BE8660442 for ; Mon, 13 Feb 2017 09:05:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1C85020499 for ; Mon, 13 Feb 2017 09:05:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 112C527165; Mon, 13 Feb 2017 09:05:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 5F75B20499 for ; Mon, 13 Feb 2017 09:05:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 639146E072; Mon, 13 Feb 2017 09:05:43 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6C3876E072; Mon, 13 Feb 2017 09:05:41 +0000 (UTC) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Feb 2017 01:05:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,156,1484035200"; d="scan'208";a="64014755" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by orsmga005.jf.intel.com with ESMTP; 13 Feb 2017 01:05:39 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.230]) by IRSMSX151.ger.corp.intel.com ([169.254.4.20]) with mapi id 14.03.0248.002; Mon, 13 Feb 2017 09:05:38 +0000 From: "Lankhorst, Maarten" To: "Pandiyan, Dhinakaran" Thread-Topic: [Intel-gfx] [PATCH v3 7/8] drm: Connector helper function to release resources Thread-Index: AQHSgp9JP0CbuCd7NEiMBsfZHKTe6KFgYNsAgACmCgCABaSJAA== Date: Mon, 13 Feb 2017 09:05:37 +0000 Message-ID: <1486976736.7640.11.camel@intel.com> References: <1486622291-3524-1-git-send-email-dhinakaran.pandiyan@intel.com> <1486622291-3524-8-git-send-email-dhinakaran.pandiyan@intel.com> <1486630868.30626.17.camel@intel.com> <1486667623.32142.7.camel@dk-H97M-D3H> In-Reply-To: <1486667623.32142.7.camel@dk-H97M-D3H> Accept-Language: nl-NL, en-US Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: x-originating-ip: [10.252.8.211] MIME-Version: 1.0 Cc: "daniel.vetter@ffwll.ch" , "intel-gfx@lists.freedesktop.org" , "dri-devel@lists.freedesktop.org" , "bskeggs@redhat.com" , "alexander.deucher@amd.com" , "harry.wentland@amd.com" Subject: Re: [Intel-gfx] [PATCH v3 7/8] drm: Connector helper function to release resources X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Pandiyan, Dhinakaran schreef op do 09-02-2017 om 18:55 [+0000]: > On Thu, 2017-02-09 at 09:01 +0000, Lankhorst, Maarten wrote: > > > > Dhinakaran Pandiyan schreef op wo 08-02-2017 om 22:38 [-0800]: > > > > > > Having a ->atomic_release callback is useful to release shared > > > resources > > > that get allocated in compute_config(). This function is expected > > > to > > > be > > > called in the atomic_check() phase before new resources are > > > acquired. > > > > > > v2: Moved the caller hunk to this patch (Daniel) > > > > > > Suggested-by: Daniel Vetter > > > Signed-off-by: Dhinakaran Pandiyan > > > > > > --- > > >  drivers/gpu/drm/drm_atomic_helper.c      | 19 > > > +++++++++++++++++++ > > >  include/drm/drm_modeset_helper_vtables.h | 13 +++++++++++++ > > >  2 files changed, 32 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > > b/drivers/gpu/drm/drm_atomic_helper.c > > > index 8795088..92bd741 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -576,6 +576,25 @@ drm_atomic_helper_check_modeset(struct > > > drm_device *dev, > > >   } > > >   } > > >   > > > + for_each_connector_in_state(state, connector, > > > connector_state, i) { > > > + const struct drm_connector_helper_funcs > > > *conn_funcs; > > > + struct drm_crtc_state *crtc_state; > > > + > > > + conn_funcs = connector->helper_private; > > > + if (!conn_funcs->atomic_release) > > > + continue; > > > + > > > + if (!connector->state->crtc) > > > + continue; > > > + > > > + crtc_state = > > > drm_atomic_get_existing_crtc_state(state, connector->state- > > > >crtc); > > > + > > > + if (crtc_state->connectors_changed || > > > +     crtc_state->mode_changed || > > > +     (crtc_state->active_changed && !crtc_state- > > > > > > > > active)) > > > + conn_funcs->atomic_release(connector, > > > connector_state); > > > + } > > > > Could we deal with the VCPI state separately in > > intel_modeset_checks, > > like we do with dpll? > > We'd want to release the VCPI slots before they are acquired in > ->compute_config(). intel_modeset_checks() will be too late to > release > them. Are you suggesting both acquiring and releasing slots should be > done in intel_modeset_checks()? That makes things a bit more nasty. Maybe add a conn_funcs->atomic_check that always gets called, something like I did below? I'd love to use it for some atomic connector properties too. > > > > > > Maybe implementing the relevant VCPI state could be done as an > > atomic > > helper function too, so other atomic drivers can just plug it in. > > > The idea was to reduce boilerplate in the drivers and use the > private_obj state for different object types. > > > > > > Not sure how doable this is, but if it's not too hard, then it's > > probably cleaner :) > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 39cbacd8cd07..1e5f0a95c685 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -381,11 +381,24 @@ mode_fixup(struct drm_atomic_state *state) for_each_new_connector_in_state(state, connector, conn_state, i) { const struct drm_encoder_helper_funcs *funcs; + const struct drm_connector_helper_funcs *conn_funcs; struct drm_encoder *encoder; + conn_funcs = connector->helper_private; + WARN_ON(!!conn_state->best_encoder != !!conn_state->crtc); - if (!conn_state->crtc || !conn_state->best_encoder) + if (!conn_state->crtc || !conn_state->best_encoder) { + if (conn_funcs && conn_funcs->atomic_check) { + ret = conn_funcs->atomic_check(connector, conn_state); + + if (ret) { + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] check failed\n", + connector->base.id, connector->name); + return ret; + } + } + continue; crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); @@ -404,7 +417,15 @@ mode_fixup(struct drm_atomic_state *state) return -EINVAL; } - if (funcs && funcs->atomic_check) { + if (conn_funcs && conn_funcs->atomic_check) { + ret = conn_funcs->atomic_check(connector, conn_state); + + if (ret) { + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] check failed\n", + connector->base.id, connector->name); + return ret; + } + } else if (funcs && funcs->atomic_check) { ret = funcs->atomic_check(encoder, crtc_state, conn_state); if (ret) {