Re: [PATCH v2 0/3] drm/vc4: Add a load tracker

[Paul Kocialkowski <paul.kocialkowski@bootlin.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2745 bytes --]

Hi,

On Wed, 2018-11-28 at 10:29 +0100, Boris Brezillon wrote:
> On Wed, 28 Nov 2018 10:16:17 +0100
> Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote:
> 
> > Hi,
> > 
> > On Thu, 2018-10-25 at 14:45 +0200, Boris Brezillon wrote:
> > > Hello,
> > > 
> > > This is the 2nd version of the VC4 load tracker patch.
> > > 
> > > Daniel, as you suggested, I've implemented a generic infrastructure to
> > > 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.
> > > 
> > > 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).  
> > 
> > For the whole series:
> > Tested-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > 
> > I am currently integrating this with IGT testing and have a few general
> > remarks:
> > 
> > - 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.
> 
> Yep, makes sense.
> 
> > - 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.
> > 
> > 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.
> 
> 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.

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel