[igt-dev] [PATCH i-g-t 7/7] tests/kms_ccs: Larger fb to fully cover up the primary plane

Wed Feb 27 14:18:27 UTC 2019

On Tue, Feb 26, 2019 at 11:50:23PM -0800, Dhinakaran Pandiyan wrote:
> On Sat, 2019-02-23 at 15:43 +0200, Juha-Pekka Heikkilä wrote:
> > 
> > Clinton Taylor kirjoitti 22.2.2019 klo 18.18:
> > > 
> > > On 2/22/19 6:19 AM, Juha-Pekka Heikkila wrote:
> > > > I wouldn't go reverting this limitation. Idea of this test is to
> > > > see 
> > > > if ccs work but not how big planes can be used with ccs. Matt's
> > > > patch 
> > > > was to fix that two planes 1080p problem on APL but if you go see
> > > > the 
> > > > machine https://intel-gfx-ci.01.org/hardware.html#fi-apl-guc
> > > > you'll 
> > > > notice it support 4096x2160 at 60Hz at highest. After removing this 
> > > > limitation all you need to do to fail this test is to change the
> > > > monitor.
> > > > 
> > > 
> > > The test has been changed to the current active size of the pipe
> > > and not 
> > > the maximum size supported by the hardware.
> > > 
> > 
> > I didn't understand the reasoning. This test did fail on different 
> > machines because of plane sizes which required more bandwith than
> > was 
> > available in budget.
> 
> There is just 
>  one primary plane scanning out a fb sized to match connector's default
> mode
>  an overlay scanning out a fb of size 256x256. 
> 
> This to me is a reasonable configuration that should not exceeding any
> hardware limits. 

Let's not guess. Ie. you should calculate the max configuration we can
actually support. If that exceeds your typical 4k at 60 display then I
guess we can revert this. Otherwise we're just arming CI to explode
when when someone decides that we need a bigger display on the BXT.

Also double check whether the test actually turns off the other pipes
or not. If not we have even bigger problems.

> The commit that changes the primary fb size does not
> specifically call out which platforms the problem was on and what exact
> hardware limit the test exceeds. 
> 
> >  Watermark calculation was adjusted but it doesn't 
> > change limitations on hardware.
> 
> Looking at the bug, it does seem like the real problem was the second
> plane not meeting the minimum watermark limits due to a buggy watermark
> algorithm. And that problem has been fixed. If you see the original
> list of failures, it is across all platforms and the test fails only
> when both primary and sprite plane are enabled. 
> 
> > 
> > Is there benefit in reindroducing this point of failure?
> There is not much point in testing an artificial configuration (2000 *
> vdisplay) in order to make the test pass. We should know when a sane
> configuration fails so that it can be debugged.
> 
> > 
> > /Juha-Pekka
> > 
> > > 
> > > 
> > > > This igt test was originally limited due to this machine 
> > > > https://intel-gfx-ci.01.org/hardware.html#fi-kbl-soraka which is
> > > > meant 
> > > > to be Chromebook with 2400x1600 size display. Quickly looking it
> > > > seems 
> > > > Soraka has not been recently running so much CI runs though.
> > > > 
> > > > /Juha-Pekka
> > > > 
> > > > On 21.2.2019 16.41, Dhinakaran Pandiyan wrote:
> > > > > This is a revert of commit 27fa97d16294 ("tests/kms_ccs: Avoid
> > > > > using 
> > > > > plane
> > > > > sizes which exceed hw capability"). The watermarks issue that
> > > > > the patch
> > > > > refers to has been fixed as per the bugzilla, so let's use an
> > > > > fb that
> > > > > covers the whole screen.
> > > > > 
> > > > > Cc: Clinton Taylor <clinton.a.taylor at intel.com>
> > > > > Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=105458
> > > > > Signed-off-by: Dhinakaran Pandiyan <
> > > > > dhinakaran.pandiyan at intel.com>
> > > > > ---
> > > > >   tests/kms_ccs.c | 16 +++-------------
> > > > >   1 file changed, 3 insertions(+), 13 deletions(-)
> > > > > 
> > > > > diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> > > > > index 8cbf100f..48f10af0 100644
> > > > > --- a/tests/kms_ccs.c
> > > > > +++ b/tests/kms_ccs.c
> > > > > @@ -69,15 +69,6 @@ const struct {
> > > > >       {0.0, 1.0, 0.0}
> > > > >   };
> > > > >   -/*
> > > > > - * Limit maximum used sprite plane width so this test will
> > > > > not 
> > > > > mistakenly
> > > > > - * fail on hardware limitations which are not interesting to
> > > > > this test.
> > > > > - * On this test too wide sprite plane may fail during creation
> > > > > with 
> > > > > dmesg
> > > > > - * comment saying:
> > > > > - * "Requested display configuration exceeds system watermark 
> > > > > limitations"
> > > > > - */
> > > > > -#define MAX_SPRITE_PLANE_WIDTH 2000
> > > > > -
> > > > >   struct local_drm_format_modifier {
> > > > >          /* Bitmask of formats in get_plane format list this
> > > > > info 
> > > > > applies to. The
> > > > >       * offset allows a sliding window of which 64 formats
> > > > > (bits).
> > > > > @@ -313,13 +304,12 @@ static bool try_config(data_t *data,
> > > > > enum 
> > > > > test_fb_flags fb_flags,
> > > > >       if (data->plane && fb_flags & FB_COMPRESSED) {
> > > > >           if (!plane_has_format_with_ccs(data, data->plane, 
> > > > > DRM_FORMAT_XRGB8888))
> > > > >               return false;
> > > > > -        generate_fb(data, &fb, min(MAX_SPRITE_PLANE_WIDTH, 
> > > > > drm_mode->hdisplay),
> > > > > -                drm_mode->vdisplay,
> > > > > +        generate_fb(data, &fb, drm_mode->hdisplay, drm_mode-
> > > > > >vdisplay,
> > > > >                   (fb_flags & ~FB_COMPRESSED) | FB_HAS_PLANE);
> > > > >           generate_fb(data, &fb_sprite, 256, 256, fb_flags);
> > > > >       } else {
> > > > > -        generate_fb(data, &fb, min(MAX_SPRITE_PLANE_WIDTH, 
> > > > > drm_mode->hdisplay),
> > > > > -                drm_mode->vdisplay, fb_flags);
> > > > > +        generate_fb(data, &fb, drm_mode->hdisplay, drm_mode-
> > > > > >vdisplay,
> > > > > +                fb_flags);
> > > > >       }
> > > > >         if (data->flags & TEST_FAIL_ON_ADDFB2)
> > > > > 

-- 
Ville Syrjälä
Intel