From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gustavo Padovan Subject: Re: [PATCH 4/9] drm/i915: split intel_update_plane into check() and commit() Date: Fri, 29 Aug 2014 12:09:39 -0300 Message-ID: <20140829150939.GA3749@joana> References: <1409247613-14232-1-git-send-email-gustavo@padovan.org> <1409247613-14232-4-git-send-email-gustavo@padovan.org> <20140829075541.GT4193@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20140829075541.GT4193@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, Gustavo Padovan , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 2014-08-29 Ville Syrj=E4l=E4 : > On Thu, Aug 28, 2014 at 02:40:08PM -0300, Gustavo Padovan wrote: > > From: Gustavo Padovan > > = > > Due to the upcoming atomic modesetting feature we need to separate > > some update functions into a check step that can fail and a commit > > step that should, ideally, never fail. > > = > > This commit splits intel_update_plane() and its commit part can still > > fail due to the fb pinning procedure. > > = > > Signed-off-by: Gustavo Padovan > > --- > > drivers/gpu/drm/i915/intel_sprite.c | 128 ++++++++++++++++++++++++++--= -------- > > 1 file changed, 93 insertions(+), 35 deletions(-) > > = > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915= /intel_sprite.c > > index 4cbe286..b1cb606 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane *= intel_plane) > > return key.flags !=3D I915_SET_COLORKEY_NONE; > > } > > = > > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst, > = > get_visisble() is not a good name here IMO, also I think this split is at > a slightly too low level. What we really want is to start creating some > kind of plane config struct that can be passed to both check and commit, > and then check can already store all the clipped coordinates etc. to the > plane config and commit can just look them up w/o recomputing them. What do you really mean by too low level here? Would grabbing struct drm_plane_state from the atomic branch fix this for you? I'll get a v2 of these patches working with struct drm_plane_state. Gustavo