[igt-dev] [PATCH i-g-t v3 3/6] tests/fbcon_fbt: Add and user psr_long_wait_update()
Souza, Jose
jose.souza at intel.com
Fri Apr 12 15:56:37 UTC 2019
On Thu, 2019-04-11 at 21:25 -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2019-04-10 at 15:07 -0700, José Roberto de Souza wrote:
> > When fbcon is enabled, PSR will be active between cursor blinks so
> > what it should really use to test PSR is psr_wait_entry(), so a new
> > feature callback was added.
> We could add a PSR only blinking subtest that verifies both entry and
> exit
> happen as the next step.
>
> > But the fbcon cursor blinks at 5hz what give us 200ms between each
> > screen update what make psr_wait_update() prone to fail the test
> > because it timed out before a blink could happen, so here adding
> > and
> > using psr_long_wait_update() that have a longer timeout.
>
> Patch 4 that sets up the test conditions should come before this
> patch that
> validates the kernel's response.
Okay reordering.
>
> Also, is there a way to read the blink rate?
There is no way, I found a old patch adding it but it was not merged.
>
> > v3:
> > - 3 previous patches squashed in this one (Maarten)
> > - Back to !feature->wait_until_enabled() to test feature state when
> > all CRTCS are disabled(Dhinakaran)
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> > lib/igt_psr.c | 5 +++++
> > lib/igt_psr.h | 1 +
> > tests/kms_fbcon_fbt.c | 17 +++++++++++++++--
> > 3 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index b5847bfd..b92ea73f 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -54,6 +54,11 @@ bool psr_wait_update(int debugfs_fd, enum
> > psr_mode mode)
> > return igt_wait(!psr_active_check(debugfs_fd, mode), 40, 10);
> > }
> >
> > +bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode)
> > +{
> > + return igt_wait(!psr_active_check(debugfs_fd, mode), 500, 10);
> nit: We could reduce the debugfs read frequency as the timeout is
> also larger,
> haven't really given enough though into what's a sensible upper
> limit. But,
> something like 50 should be okay.
>
> > +}
> > +
> > static ssize_t psr_write(int debugfs_fd, const char *buf)
> > {
> > return igt_sysfs_write(debugfs_fd, "i915_edp_psr_debug", buf,
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index 49599cf8..ca385736 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -37,6 +37,7 @@ enum psr_mode {
> >
> > bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
> > bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
> > +bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
> > bool psr_enable(int debugfs_fd, enum psr_mode);
> > bool psr_disable(int debugfs_fd);
> > bool psr_sink_support(int debugfs_fd, enum psr_mode);
> > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > index a9d91839..a5340d85 100644
> > --- a/tests/kms_fbcon_fbt.c
> > +++ b/tests/kms_fbcon_fbt.c
> > @@ -130,6 +130,11 @@ static bool fbc_wait_until_enabled(int
> > debugfs_fd)
> > return r;
> > }
> >
> > +static bool fbc_wait_until_update(int debugfs)
> > +{
> This warrants an explanation as to why fbc_wait_until_update ==
> !fbc_wait_until_enabled.
>
This was just moved from what it was doing before.
> My understanding is that we do not expect fbc to become active at all
> with
> fbcon. Can you confirm that?
It is not expected because fbcon do not uses a tiled framebuffer so a
fence can not be setup on the framebuffer and FBC code requires a fence
to accurate track frontbuffer modifications(what maybe is not necessary
anymore as we now have intel_fbc_invalidate()/flush()).
If one day fbcon starts to use a tiled framebuffer we would need to
check the 'Compressing' status as in each blink it would disable the
compressing but FBC would still be enabled.
Adding it.
>
> With an explanation included and patches re-ordered,
>
> Reviewed-by: Dhinkaran Pandiyan <dhinakaran.pandiyan at intel.com>
>
> > + return !fbc_wait_until_enabled(debugfs);
> > +}
> > +
> > typedef bool (*connector_possible_fn)(drmModeConnectorPtr
> > connector);
> >
> > static void set_mode_for_one_screen(struct drm_info *drm, struct
> > igt_fb *fb,
> > @@ -196,6 +201,11 @@ static bool psr_supported_on_chipset(int
> > debugfs_fd)
> > return psr_sink_support(debugfs_fd, PSR_MODE_1);
> > }
> >
> > +static bool psr_wait_until_update(int debugfs_fd)
> > +{
> > + return psr_long_wait_update(debugfs_fd, PSR_MODE_1);
> > +}
> > +
> > static void disable_features(int debugfs_fd)
> > {
> > igt_set_module_param_int("enable_fbc", 0);
> > @@ -215,16 +225,19 @@ static inline void psr_debugfs_enable(int
> > debugfs_fd)
> > struct feature {
> > bool (*supported_on_chipset)(int debugfs_fd);
> > bool (*wait_until_enabled)(int debugfs_fd);
> > + bool (*wait_until_update)(int debugfs_fd);
> > bool (*connector_possible_fn)(drmModeConnectorPtr connector);
> > void (*enable)(int debugfs_fd);
> > } fbc = {
> > .supported_on_chipset = fbc_supported_on_chipset,
> > .wait_until_enabled = fbc_wait_until_enabled,
> > + .wait_until_update = fbc_wait_until_update,
> > .connector_possible_fn = connector_can_fbc,
> > .enable = fbc_modparam_enable,
> > }, psr = {
> > .supported_on_chipset = psr_supported_on_chipset,
> > .wait_until_enabled = psr_wait_until_enabled,
> > + .wait_until_update = psr_wait_until_update,
> > .connector_possible_fn = connector_can_psr,
> > .enable = psr_debugfs_enable,
> > };
> > @@ -263,13 +276,13 @@ static void subtest(struct feature *feature,
> > bool
> > suspend)
> > sleep(3);
> >
> > wait_user("Back to fbcon.");
> > - igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
> > + igt_assert(feature->wait_until_update(drm.debugfs_fd));
> >
> > if (suspend) {
> > igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > SUSPEND_TEST_NONE);
> > sleep(5);
> > - igt_assert(!feature-
> > >wait_until_enabled(drm.debugfs_fd));
> > + igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > }
> > }
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190412/b974bb4f/attachment.sig>
More information about the igt-dev
mailing list