All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>
Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_fbcon_fbt: Do a full connector probe.
Date: Fri, 26 Apr 2019 08:53:27 +0200	[thread overview]
Message-ID: <CAKMK7uHC4gkw=5+BGmkegOXKoF8=1y=qjeeRYq+ZDVFyEEgSZA@mail.gmail.com> (raw)
In-Reply-To: <aef08224273249f9e4f3c888460fe14721a264bc.camel@intel.com>

On Wed, Feb 6, 2019 at 4:29 AM Dhinakaran Pandiyan
<dhinakaran.pandiyan@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@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@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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2019-04-26  6:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-04 20:55 [igt-dev] [PATCH i-g-t] tests/kms_fbcon_fbt: Do a full connector probe Dhinakaran Pandiyan
2019-02-05  4:22 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2019-02-05  5:14 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-02-05 18:45 ` [igt-dev] [PATCH i-g-t] " Souza, Jose
2019-02-06  3:29   ` Dhinakaran Pandiyan
2019-04-26  6:53     ` Daniel Vetter [this message]
2019-02-28 18:38 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_fbcon_fbt: Do a full connector probe. (rev2) Patchwork
2019-02-28 22:10 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-04-06  0:20 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_fbcon_fbt: Do a full connector probe. (rev3) Patchwork
2019-04-07  0:29 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKMK7uHC4gkw=5+BGmkegOXKoF8=1y=qjeeRYq+ZDVFyEEgSZA@mail.gmail.com' \
    --to=daniel.vetter@ffwll.ch \
    --cc=dhinakaran.pandiyan@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.