Re: [kms-test] [PATCH 09/10] tests: Add plane zpos test

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Kieran Bingham <kieran.bingham@ideasonboard.com>
Cc: linux-renesas-soc@vger.kernel.org
Subject: Re: [kms-test] [PATCH 09/10] tests: Add plane zpos test
Date: Wed, 29 Jun 2022 21:08:07 +0300	[thread overview]
Message-ID: <YryVBwNB5tS3JKog@pendragon.ideasonboard.com> (raw)
In-Reply-To: <165651793098.2049236.2538448937224001374@Monstersaurus>

On Wed, Jun 29, 2022 at 04:52:10PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2022-06-10 00:40:30)
> > Add a test that enables multiple planes with different zpos values.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  tests/kms-test-plane-zpos.py | 102 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 102 insertions(+)
> >  create mode 100755 tests/kms-test-plane-zpos.py
> > 
> > diff --git a/tests/kms-test-plane-zpos.py b/tests/kms-test-plane-zpos.py
> > new file mode 100755
> > index 000000000000..052eea542ec0
> > --- /dev/null
> > +++ b/tests/kms-test-plane-zpos.py
> > @@ -0,0 +1,102 @@
> > +#!/usr/bin/python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# SPDX-FileCopyrightText: 2022 Renesas Electronics Corporation
> > +
> > +import kmstest
> > +import pykms
> > +
> > +class PlaneZPosTest(kmstest.KMSTest):
> > +    """Test composition with multiple planes and custom z-pos."""
> > +
> > +    def handle_page_flip(self, frame, time):
> > +        self.logger.log('Page flip complete')
> > +
> > +    def find_pipeline(self):
> > +        # Find a CRTC that has multiple planes with a connected connector
> > +        for connector in self.output_connectors():
> > +            # Skip disconnected connectors
> > +            if not connector.connected():
> > +                continue
> > +
> > +            # Add the connector to the map
> > +            for crtc in connector.get_possible_crtcs():
> > +                planes = []
> > +                for plane in self.card.planes:
> > +                    if plane.supports_crtc(crtc) and plane != crtc.primary_plane:
> > +                        planes.append(plane)
> > +
> > +                if len(planes):
> > +                    return crtc, connector, planes
> > +
> > +        return None, None, None
> > +
> > +    def main(self):
> > +        self.start('composition with z-pos control')
> > +
> > +        crtc, connector, planes = self.find_pipeline()
> > +        if crtc is None:
> > +            self.skip('no suitable pipeline')
> > +            return
> > +
> > +        # Get the default mode
> > +        try:
> > +            mode = connector.get_default_mode()
> > +        except KeyError:
> > +            self.skip('no mode available')
> > +            return
> > +
> > +        self.logger.log(f'Testing connector {connector.fullname}, CRTC {crtc.id}, '
> > +                        f'mode {mode.name} with {len(planes)} planes '
> > +                        f'(P: {crtc.primary_plane.id}, O: {[plane.id for plane in planes]})')
> > +
> > +        # Create a frame buffer
> > +        fb = pykms.DumbFramebuffer(self.card, mode.hdisplay, mode.vdisplay, 'XR24')
> > +        pykms.draw_test_pattern(fb)
> > +
> > +        # Set the mode with a primary plane, and position it on top of the
> > +        # stack. Make it transparent to visualize the overlay planes that will
> 
> How handy that there's now transparency ;-)
> 
> > +        # be positioned underneath.
> > +        zpos = len(planes)
> > +        ret = self.atomic_crtc_mode_set(crtc, connector, mode, fb)
> > +        if ret < 0:
> > +            self.fail(f'atomic mode set failed with {ret}')
> > +            return
> > +
> > +        req = kmstest.AtomicRequest(self)
> > +        req.add(crtc.primary_plane, 'alpha', '50%')
> > +        req.add(crtc.primary_plane, 'zpos', zpos)
> 
> Not that it matters, but those could both be added in a single statement
> right?

Yes, that's right.

> (not needed, just understanding the API of .add() )
> 
> > +        ret = req.commit_sync(True)
> > +        if ret < 0:
> > +            self.fail(f'failed to set properties for primary plane: {ret}')
> > +            return
> > +
> > +        self.run(3)
> > +
> > +        # Add all other planes one by one
> > +        offset = 100 + 50 * (len(planes) - 1)
> > +
> > +        for plane in planes:
> > +            zpos -= 1
> > +
> > +            source = kmstest.Rect(0, 0, fb.width, fb.height)
> > +            destination = kmstest.Rect(offset, offset, fb.width, fb.height)
> > +            ret = self.atomic_plane_set(plane, crtc, source, destination, fb, alpha='100%', zpos=zpos)
> 
> Seems pretty good to me.
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > +            if ret < 0:
> > +                self.fail(f'atomic plane set failed with {ret}')
> > +                break
> > +
> > +            self.logger.log(f'Adding plane {plane.id}')
> > +            self.run(1)
> > +
> > +            if self.flips == 0:
> > +                self.fail('No page flip registered')
> > +                break
> > +
> > +            offset -= 50
> > +
> > +        else:
> > +            self.success()
> > +
> > +        self.atomic_crtc_disable(crtc)
> > +
> > +PlaneZPosTest().execute()

-- 
Regards,

Laurent Pinchart