| Message ID | 20171026003726.2503-1-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Qua, 2017-10-25 às 17:37 -0700, Dhinakaran Pandiyan escreveu: > The frontbuffer_tracking PSR tests fail if PSR cannot be activated > when > there is sink support. But, there are several other requirements > related to > mode timings that have to be satisfied before PSR can be enabled. No > reason > to fail these tests when PSR cannot be enabled. > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > tests/kms_frontbuffer_tracking.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tests/kms_frontbuffer_tracking.c > b/tests/kms_frontbuffer_tracking.c > index a068c8af..13c948de 100644 > --- a/tests/kms_frontbuffer_tracking.c > +++ b/tests/kms_frontbuffer_tracking.c > @@ -1548,12 +1548,12 @@ static void teardown_fbc(void) > { > } > > -static bool psr_sink_has_support(void) > +static bool psr_has_support(void) > { > char buf[256]; > > debugfs_read("i915_edp_psr_status", buf); > - return strstr(buf, "Sink_Support: yes\n"); > + return strstr(buf, "Enabled: yes\n"); What if PSR can be enabled but is just not enabled due to a bug? I suppose we could print on debugfs whether source+sink support equals yes. > } > > static void setup_psr(void) > @@ -1564,8 +1564,8 @@ static void setup_psr(void) > return; > } > > - if (!psr_sink_has_support()) { > - igt_info("Can't test PSR: not supported by > sink.\n"); > + if (!psr_has_support()) { > + igt_info("Can't test PSR: not supported.\n"); > return; > } > psr.can_test = true;
On Thu, Oct 26, 2017 at 01:29:57PM +0000, Paulo Zanoni wrote: > Em Qua, 2017-10-25 às 17:37 -0700, Dhinakaran Pandiyan escreveu: > > The frontbuffer_tracking PSR tests fail if PSR cannot be activated > > when > > there is sink support. But, there are several other requirements > > related to > > mode timings that have to be satisfied before PSR can be enabled. No > > reason > > to fail these tests when PSR cannot be enabled. > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > tests/kms_frontbuffer_tracking.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/tests/kms_frontbuffer_tracking.c > > b/tests/kms_frontbuffer_tracking.c > > index a068c8af..13c948de 100644 > > --- a/tests/kms_frontbuffer_tracking.c > > +++ b/tests/kms_frontbuffer_tracking.c > > @@ -1548,12 +1548,12 @@ static void teardown_fbc(void) > > { > > } > > > > -static bool psr_sink_has_support(void) > > +static bool psr_has_support(void) > > { > > char buf[256]; > > > > debugfs_read("i915_edp_psr_status", buf); > > - return strstr(buf, "Sink_Support: yes\n"); > > + return strstr(buf, "Enabled: yes\n"); > > What if PSR can be enabled but is just not enabled due to a bug? then psr basic should be the one failing... > > I suppose we could print on debugfs whether source+sink support equals > yes. or maybe here instead of sink: yes or enable: yes we could check sink: yes && source: yes if sink yes and source yes and psr is still disabled when we expect that enabled than we have the bug paulo mentioned. > > > } > > > > static void setup_psr(void) > > @@ -1564,8 +1564,8 @@ static void setup_psr(void) > > return; > > } > > > > - if (!psr_sink_has_support()) { > > - igt_info("Can't test PSR: not supported by > > sink.\n"); > > + if (!psr_has_support()) { > > + igt_info("Can't test PSR: not supported.\n"); > > return; > > } > > psr.can_test = true;
Em Qui, 2017-10-26 às 12:32 -0700, Rodrigo Vivi escreveu: > On Thu, Oct 26, 2017 at 01:29:57PM +0000, Paulo Zanoni wrote: > > Em Qua, 2017-10-25 às 17:37 -0700, Dhinakaran Pandiyan escreveu: > > > The frontbuffer_tracking PSR tests fail if PSR cannot be > > > activated > > > when > > > there is sink support. But, there are several other requirements > > > related to > > > mode timings that have to be satisfied before PSR can be enabled. > > > No > > > reason > > > to fail these tests when PSR cannot be enabled. > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com > > > > > > > --- > > > tests/kms_frontbuffer_tracking.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/tests/kms_frontbuffer_tracking.c > > > b/tests/kms_frontbuffer_tracking.c > > > index a068c8af..13c948de 100644 > > > --- a/tests/kms_frontbuffer_tracking.c > > > +++ b/tests/kms_frontbuffer_tracking.c > > > @@ -1548,12 +1548,12 @@ static void teardown_fbc(void) > > > { > > > } > > > > > > -static bool psr_sink_has_support(void) > > > +static bool psr_has_support(void) > > > { > > > char buf[256]; > > > > > > debugfs_read("i915_edp_psr_status", buf); > > > - return strstr(buf, "Sink_Support: yes\n"); > > > + return strstr(buf, "Enabled: yes\n"); > > > > What if PSR can be enabled but is just not enabled due to a bug? > > then psr basic should be the one failing... But if we apply this patch we'll skip it instead of fail. > > > > > I suppose we could print on debugfs whether source+sink support > > equals > > yes. > > or maybe here instead of sink: yes or enable: yes > we could check > sink: yes && source: yes > > if sink yes and source yes and psr is still disabled when we > expect that enabled than we have the bug paulo mentioned. > > > > > > } > > > > > > static void setup_psr(void) > > > @@ -1564,8 +1564,8 @@ static void setup_psr(void) > > > return; > > > } > > > > > > - if (!psr_sink_has_support()) { > > > - igt_info("Can't test PSR: not supported by > > > sink.\n"); > > > + if (!psr_has_support()) { > > > + igt_info("Can't test PSR: not supported.\n"); > > > return; > > > } > > > psr.can_test = true;
On Thu, 2017-10-26 at 17:35 -0200, Paulo Zanoni wrote: > Em Qui, 2017-10-26 às 12:32 -0700, Rodrigo Vivi escreveu: > > On Thu, Oct 26, 2017 at 01:29:57PM +0000, Paulo Zanoni wrote: > > > Em Qua, 2017-10-25 às 17:37 -0700, Dhinakaran Pandiyan escreveu: > > > > The frontbuffer_tracking PSR tests fail if PSR cannot be > > > > activated > > > > when > > > > there is sink support. But, there are several other requirements > > > > related to > > > > mode timings that have to be satisfied before PSR can be enabled. > > > > No > > > > reason > > > > to fail these tests when PSR cannot be enabled. > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com > > > > > > > > > --- > > > > tests/kms_frontbuffer_tracking.c | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/tests/kms_frontbuffer_tracking.c > > > > b/tests/kms_frontbuffer_tracking.c > > > > index a068c8af..13c948de 100644 > > > > --- a/tests/kms_frontbuffer_tracking.c > > > > +++ b/tests/kms_frontbuffer_tracking.c > > > > @@ -1548,12 +1548,12 @@ static void teardown_fbc(void) > > > > { > > > > } > > > > > > > > -static bool psr_sink_has_support(void) > > > > +static bool psr_has_support(void) > > > > { > > > > char buf[256]; > > > > > > > > debugfs_read("i915_edp_psr_status", buf); > > > > - return strstr(buf, "Sink_Support: yes\n"); > > > > + return strstr(buf, "Enabled: yes\n"); > > > > > > What if PSR can be enabled but is just not enabled due to a bug? My reasoning was psr.enabled will be true as long as intel_psr_enable() is called. But, yeah there is a chance that intel_psr_enable() is not called. > > > > then psr basic should be the one failing... > > But if we apply this patch we'll skip it instead of fail. > > > > > > > > > I suppose we could print on debugfs whether source+sink support > > > equals > > > yes. > > > > or maybe here instead of sink: yes or enable: yes > > we could check > > sink: yes && source: yes > > That's how I wrote this initially, but source_ok appears to be in the wrong place in the driver, i.e., source_ok is set inside psr_enable() and hence has the same effect as checking for enabled. Moving source_ok = true inside psr_compute_config() and changing the IGT to test for sink_support && source_ok will solve our problem. > > if sink yes and source yes and psr is still disabled when we > > expect that enabled than we have the bug paulo mentioned. > > > > > > > > > } > > > > > > > > static void setup_psr(void) > > > > @@ -1564,8 +1564,8 @@ static void setup_psr(void) > > > > return; > > > > } > > > > > > > > - if (!psr_sink_has_support()) { > > > > - igt_info("Can't test PSR: not supported by > > > > sink.\n"); > > > > + if (!psr_has_support()) { > > > > + igt_info("Can't test PSR: not supported.\n"); > > > > return; > > > > } > > > > psr.can_test = true;
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c index a068c8af..13c948de 100644 --- a/tests/kms_frontbuffer_tracking.c +++ b/tests/kms_frontbuffer_tracking.c @@ -1548,12 +1548,12 @@ static void teardown_fbc(void) { } -static bool psr_sink_has_support(void) +static bool psr_has_support(void) { char buf[256]; debugfs_read("i915_edp_psr_status", buf); - return strstr(buf, "Sink_Support: yes\n"); + return strstr(buf, "Enabled: yes\n"); } static void setup_psr(void) @@ -1564,8 +1564,8 @@ static void setup_psr(void) return; } - if (!psr_sink_has_support()) { - igt_info("Can't test PSR: not supported by sink.\n"); + if (!psr_has_support()) { + igt_info("Can't test PSR: not supported.\n"); return; } psr.can_test = true;
The frontbuffer_tracking PSR tests fail if PSR cannot be activated when there is sink support. But, there are several other requirements related to mode timings that have to be satisfied before PSR can be enabled. No reason to fail these tests when PSR cannot be enabled. Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- tests/kms_frontbuffer_tracking.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)