From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Du, Changbin" Subject: Re: [PATCH v2] drm/i915: Add interface to reserve fence registers for vGPU Date: Mon, 4 Sep 2017 14:08:52 +0800 Message-ID: <20170904060851.GA25691@intel.com> References: <1504083261-13136-1-git-send-email-changbin.du@intel.com> <1504249706-6661-1-git-send-email-changbin.du@intel.com> <150426359326.4212.13845623097409577565@mail.alporthouse.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1579533567==" Return-path: In-Reply-To: <150426359326.4212.13845623097409577565@mail.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1579533567== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GvXjxJ+pjyke8COw" Content-Disposition: inline --GvXjxJ+pjyke8COw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 01, 2017 at 11:59:53AM +0100, Chris Wilson wrote: > Quoting changbin.du@intel.com (2017-09-01 08:08:26) > > diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/dr= m/i915/i915_gem_fence_reg.c > > index 5fe2cd8..429ce5f 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c > > +++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c > > @@ -360,6 +360,50 @@ i915_vma_get_fence(struct i915_vma *vma) > > } > > =20 > > /** > > + * i915_reserve_fence - Reserve a fence for vGPU > > + * @dev_priv: i915 device private > > + * > > + * This function walks the fence regs looking for a free one and remove > > + * it from the fence_list. It is used to reserve fence for vGPU to use. > > + */ > > +struct drm_i915_fence_reg * > > +i915_reserve_fence(struct drm_i915_private *dev_priv) > > +{ > > + struct drm_i915_fence_reg *fence; > > + int ret; >=20 > lockdep_assert_held(&dev_priv->drm.struct_mutex); >=20 > A reminder for when we move fences to a seperate mutex. > ok. > > + > > + /* Host at least need one fence available for display engine. */ > > + if (unlikely(dev_priv->mm.fence_list.prev =3D=3D > > + dev_priv->mm.fence_list.next)) > > + return ERR_PTR(-ENOSPC); >=20 > I would prefer one free fence. >=20 > int count; >=20 > /* Keep at least one fence available for the display engine */ > count =3D 0; > list_for_each_entry(fence, &dev_priv->mm.fence_list, link) > count +=3D !fence->pin_count; > if (count <=3D 1) > return ERR_PTR(-ENOSPC); > Got your idea, will update. > > + > > + fence =3D fence_find(dev_priv); > > + if (IS_ERR(fence)) > > + return fence; > > + > > + if (fence->vma) { > > + /* Force-remove fence from VMA */ > > + ret =3D fence_update(fence, NULL); > > + if (ret) > > + return ERR_PTR(ret); > > + } > > + > > + list_del(&fence->link); > > + return fence; > > +} > > + > > +/** > > + * i915_unreserve_fence - Reclaim a reserved fence > > + * @fence: the fence reg > > + * > > + * This function add a reserved fence register from vGPU to the fence_= list. > > + */ > > +void i915_unreserve_fence(struct drm_i915_fence_reg *fence) > > +{ >=20 > lockdep_assert_held(&dev_priv->drm.struct_mutex); >=20 > If you agree to "one free fence", > Reviewed-by: Chris Wilson >=20 > Otherwise, we need to look carefully at what happens when the search > fails. And that may suggest we need to keep one free per crtc, or that > we can survive without (but that usually means giving up on fbc and > whatnot, thinking of fbc is why I want to keep one spare). >=20 > (Time to write some unittests for find-a-fence.) > -Chris I agree to "one free fence", which is simpler. Currently GVTg will at most = use 4x7=3D28 fence registers (4 per VM, and up to 7 VMs), so at least 4 will re= mained for host. I agree to your concern, it is significative. --=20 Thanks, Changbin Du --GvXjxJ+pjyke8COw Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJZrO3zAAoJEAanuZwLnPNUVu0H/iSuTvnRtvA/fM9R0LHr62Zl U6Xn+0lV4/X4ngbIBM+g4cvkdUdoXbKz7d7k1eAN01LVtUlHl4lzEoVVl/kNEpIY 7uLEl6Iwy2am+TP0i+CLeDSNlKocO5wUChvH0bynuBxa+G1KY5ZSCHKVcRwrojiH 3gnkPiV3R4eb5z12rFZlvjmBhErjPTunos8MVWmNdJchCFKy0zyDuWwNLgiYJ9pC EsAXnb1xeuKql+d8ZdNu2Crrdpmj7tsa6tJ6DKn8g1R2OuwMrMySkjS7xkYqjLU1 WVSYXhdb0HCdKYnu6oAOLxp5bNdUItVWkp8N1BXqPVKMUapwZFBABPYkY+pIdbw= =cqWC -----END PGP SIGNATURE----- --GvXjxJ+pjyke8COw-- --===============1579533567== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============1579533567==--