[igt-dev] [PATCH i-g-t] tests/kms_fbcon_fbt: Do a full connector probe.

[Daniel Vetter]

On Wed, Feb 6, 2019 at 4:29 AM Dhinakaran Pandiyan
<dhinakaran.pandiyan at intel.com> wrote:
>
> On Tue, 2019-02-05 at 10:45 -0800, Souza, Jose wrote:
> > On Mon, 2019-02-04 at 12:55 -0800, Dhinakaran Pandiyan wrote:
> > > kms_fbtcon_fbt appears to fail when it is the first test that is
> > > executed
> > > after module load.
> > > 'Test requirement not met in function set_mode_for_one_screen, file
> > > ../tests/kms_fbcon_fbt.c:153:
> > > Test requirement: i < drm->res->count_connectors'
> > > The problem seems to be that the 1920x1080 bios fb is re-used and
> > > that
> > > prunes the 4k mode before test execution. Later, when the test does
> > > not
> > > do a full-probe, the eDP connector does not have any valid modes.
> >
> > drmModeGetConnectorCurrent() will get only one mode, the active one.
> I don't see any reference to an active mode in getconnector ioctl
> implementation.
>
> >
> > >
> > > <7>[   12.444411] [drm:drm_mode_debug_printmodeline] Modeline
> > > 191:"3840x2160" 60 533250 3840 3888 3920 4000 2160 2163 2168 2222
> > > 0x48
> > > 0xa
> > > <7>[   12.444418] [drm:drm_mode_prune_invalid] Not using 3840x2160
> > > mode:
> > >
> > >
> > > Fix this by calling drmModeGetConnector() instead
> > > drmModeGetConnectorCurrent()
> > >
> > > Note that that test is still expected to fail as PSR status checks
> > > are
> > > broken.
> >
> > Reviewed-by: José Roberto de Souza <jose.souza at intel.com>
> Thanks for the review.
>
> >
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109518
>
> Daniel,
>
> We are seeing a rare failure, which I think is because .count_modes ==
> 0 when this test is run as the first test after module loading. Here's
> what I think is happening-
> drm_fb_helper.c: drm_setup_crtcs(fb_helper, fb_helper->fb->width,
> fb_helper->fb->height) results in pruning the only eDP mode(3840x2160)
> because the fb limits are lower. Further, when the test calls
> drmModeGetConnectorCurrent(), there is no call to ->fill_modes() to
> update available modes

Yup that's the point of GetConnectorCurrent, we want to just get what
the kernel has cached (which should be correct, assuming hotplug and
driver load and all that works).

> git blame shows that you switched the libdrm call from
> drmModeGetConnector() to drmModeGetConnectorCurrent() in kms_fbcon_fbt.
> Can you confirm if the below fix is right? I just want to be sure I am
> not overlooking a kernel bug.

You're papering over a kernel bug. Especially for panels (edp or dsi
or whatever) the kernel really should have the right mode list
already. If we need to reprobe to get the kernel to do the right
thing, on a static sink, that's definitely a kernel bug.

Also, I see no supporting evidence that this patch actually fixes
anything ... drm_setup_crtcs also forces a full probe and so already
should call ->fill_modes.
-Daniel


> -DK
>
>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > ---
> > >  tests/kms_fbcon_fbt.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > > index 9d0d5a36..40465a1c 100644
> > > --- a/tests/kms_fbcon_fbt.c
> > > +++ b/tests/kms_fbcon_fbt.c
> > > @@ -67,7 +67,7 @@ static void setup_drm(struct drm_info *drm)
> > >     igt_assert(drm->res->count_connectors <= MAX_CONNECTORS);
> > >
> > >     for (i = 0; i < drm->res->count_connectors; i++)
> > > -           drm->connectors[i] = drmModeGetConnectorCurrent(drm-
> > > > fd,
> > >
> > > +           drm->connectors[i] = drmModeGetConnector(drm->fd,
> > >                                             drm->res-
> > > > connectors[i]);
> > >
> > >
> > >     kmstest_set_vt_graphics_mode();
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch