From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Kocialkowski Subject: Re: [PATCH v2 0/3] drm/vc4: Add a load tracker Date: Wed, 28 Nov 2018 14:32:25 +0100 Message-ID: <675b218d459006bf8f7c224ced1bd06f47ce2118.camel@bootlin.com> References: <20181025124546.22145-1-boris.brezillon@bootlin.com> <20181128102940.455d592c@bbrezillon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0685562780==" Return-path: Received: from mail.bootlin.com (mail.bootlin.com [62.4.15.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 1DF8989C46 for ; Wed, 28 Nov 2018 13:32:37 +0000 (UTC) In-Reply-To: <20181128102940.455d592c@bbrezillon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Boris Brezillon Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0685562780== Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-mF7Uiw0+nGp8vkjb8gd9" --=-mF7Uiw0+nGp8vkjb8gd9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, On Wed, 2018-11-28 at 10:29 +0100, Boris Brezillon wrote: > On Wed, 28 Nov 2018 10:16:17 +0100 > Paul Kocialkowski wrote: >=20 > > Hi, > >=20 > > On Thu, 2018-10-25 at 14:45 +0200, Boris Brezillon wrote: > > > Hello, > > >=20 > > > This is the 2nd version of the VC4 load tracker patch. > > >=20 > > > Daniel, as you suggested, I've implemented a generic infrastructure t= o > > > track and report underrun errors (patch 1). Not sure this is what you > > > had in mind, but it seems to do the job for my use case, and should > > > allow me to easily track regressions in the load tracking logic with = a > > > bunch of IGT tests. Let me know if you want it done differently. > > >=20 > > > Patch 2 is implementing the generic underrun interface in the VC4 > > > driver, and patch 3 is just adding the load tracking logic (hasn't > > > changed since the RFC except for the unused 'ret' var removal). =20 > >=20 > > For the whole series: > > Tested-by: Paul Kocialkowski > >=20 > > I am currently integrating this with IGT testing and have a few general > > remarks: > >=20 > > - I think it would make sense to have a driver-specific debugfs entry > > for enabling/disabling the rejection of commits by the load tracker. > > This would be useful for testing that there is no mismatch between the > > load tracker's behavior and hardware-detected underruns. >=20 > Yep, makes sense. >=20 > > - Underrun reporting with a generic debugfs entry is a good fit for IGT > > (and userspace reporting in general), but it would be useful to have an > > intermediary state reported between applying a commit and getting the > > underrun status. > >=20 > > Something like returning '?' between setting a commit and the next > > vblank. This way, there is no chance that userspace reads the underrun > > status related to the previous configuration. >=20 > You will never get the result of the previous atomic-set since I reset > the underrun state to 0 before committing the changes, but you might > read the underrun file before the underrun event happened. So yes, > waiting for at least one VBLANK sounds reasonable. Not sure we want to > automate that in the driver though, as this would imply activating > vblank interrupts to update the underrun state even if the user doesn't > care. Maybe you can use the DRM_IOCTL_WAIT_VBLANK ioctl instead. Right, that works just fine on VC4 and I guess it's fair to expect that future hardware that uses an underrun indication will have received the underrun indication by the time vblank happens. --=20 Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com --=-mF7Uiw0+nGp8vkjb8gd9 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEJZpWjZeIetVBefti3cLmz3+fv9EFAlv+mOkACgkQ3cLmz3+f v9Gpngf/Qo9TLfiMHSkbSfaktNhcUSYw8clMQCsjCo5SW4Id+u/ushGhNuP9HSI2 YsZHhAw9g8Ljti+I+UQkOUtsihRj5EVVUHbA1e8g5ZvYyMjsqVPO2NSsTx8f62sq KGW8E4NtSCEs2bNSxLOVYfryvIOxwHTbY1asnskLcSMgI6q61odalleyBpky313B RJBRZYnJOBBRcvyZf5I0V1ZAhplLlPiGtqgN1oc2kVGmSUaMbp4d7vevq/ORQP0S wwMmo83kukd6AxVTqArVciZClwpMzkotuuChbiINyc1mMDayCuXK0i96ny9JyCvc rNaZ3GY5cirWN9ksWk8DDk8BELlGJg== =vYv2 -----END PGP SIGNATURE----- --=-mF7Uiw0+nGp8vkjb8gd9-- --===============0685562780== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0685562780==--