From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 9/9] drm/i915: split intel_pipe_set_base() into check() and commit() Date: Fri, 29 Aug 2014 11:06:13 +0300 Message-ID: <20140829080613.GU4193@intel.com> References: <1409247613-14232-1-git-send-email-gustavo@padovan.org> <1409247613-14232-9-git-send-email-gustavo@padovan.org> 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: <1409247613-14232-9-git-send-email-gustavo@padovan.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Gustavo Padovan Cc: intel-gfx@lists.freedesktop.org, Gustavo Padovan , dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Aug 28, 2014 at 02:40:13PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > = > Take out some parts of code that can fail from it and move them to > intel_pipe_check_base(), the only failure point in intel_pipe_set_base() > now is the fb pinning procudure. I'd like to see intel_pipe_set_base() replaced entirely with the primary plane setplane. Right now we have duplicated code paths all over the place due to intel_pipe_set_base(). After that's done the same plane config treatment should be applied to primary planes as I suggested for sprites. > = > Signed-off-by: Gustavo Padovan > --- > drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++++++++++++++--= ------ > 1 file changed, 31 insertions(+), 8 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index eb6febf..ead2f24 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2661,17 +2661,11 @@ static bool intel_crtc_has_pending_flip(struct dr= m_crtc *crtc) > } > = > static int > -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > +intel_pipe_check_base(struct drm_crtc *crtc, int x, int y, > struct drm_framebuffer *fb) > { > struct drm_device *dev =3D crtc->dev; > - struct drm_i915_private *dev_priv =3D dev->dev_private; > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > - enum pipe pipe =3D intel_crtc->pipe; > - struct drm_framebuffer *old_fb =3D crtc->primary->fb; > - struct drm_i915_gem_object *obj =3D intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj =3D intel_fb_obj(old_fb); > - int ret; > = > if (intel_crtc_has_pending_flip(crtc)) { > DRM_ERROR("pipe is still busy with an old pageflip\n"); > @@ -2691,6 +2685,22 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, = int y, > return -EINVAL; > } > = > + return 0; > +} > + > +static int > +intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > + struct drm_framebuffer *fb) > +{ > + struct drm_device *dev =3D crtc->dev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > + enum pipe pipe =3D intel_crtc->pipe; > + struct drm_framebuffer *old_fb =3D crtc->primary->fb; > + struct drm_i915_gem_object *obj =3D intel_fb_obj(fb); > + struct drm_i915_gem_object *old_obj =3D intel_fb_obj(old_fb); > + int ret; > + > mutex_lock(&dev->struct_mutex); > ret =3D intel_pin_and_fence_fb_obj(dev, obj, NULL); > if (ret =3D=3D 0) > @@ -9868,6 +9878,10 @@ free_work: > if (ret =3D=3D -EIO) { > out_hang: > intel_crtc_wait_for_pending_flips(crtc); > + ret =3D intel_pipe_check_base(crtc, crtc->x, crtc->y, fb); > + if (ret) > + return ret; > + > ret =3D intel_pipe_set_base(crtc, crtc->x, crtc->y, fb); > if (ret =3D=3D 0 && event) > drm_send_vblank_event(dev, pipe, event); > @@ -11396,6 +11410,10 @@ static int intel_crtc_set_config(struct drm_mode= _set *set) > = > intel_crtc_wait_for_pending_flips(set->crtc); > = > + ret =3D intel_pipe_check_base(set->crtc, set->x, set->y, set->fb); > + if (ret) > + goto fail; > + > ret =3D intel_pipe_set_base(set->crtc, > set->x, set->y, set->fb); > = > @@ -11620,12 +11638,17 @@ intel_check_primary_plane(struct drm_plane *pla= ne, struct drm_crtc *crtc, > .x2 =3D intel_crtc->active ? intel_crtc->config.pipe_src_w : 0, > .y2 =3D intel_crtc->active ? intel_crtc->config.pipe_src_h : 0, > }; > + int ret; > = > - return drm_plane_helper_check_update(plane, crtc, fb, > + ret =3D drm_plane_helper_check_update(plane, crtc, fb, > &src, &dest, &clip, > DRM_PLANE_HELPER_NO_SCALING, > DRM_PLANE_HELPER_NO_SCALING, > false, true, visible); > + if (ret) > + return ret; > + > + return intel_pipe_check_base(crtc, src.x1, src.y1, fb); > } > = > static int > -- = > 1.9.3 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC