Re: [igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

Souza, Jose
2019-01-24T18:20:29Z

[-- Attachment #1.1: Type: text/plain, Size: 3191 bytes --]

On Wed, 2019-01-23 at 22:39 -0800, Rodrigo Vivi wrote:
> On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran Pandiyan wrote:
> > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> > > If resolution that will be used in PSR tests is bigger than the
> > > resolution that PSR2 HW supports, PSR2 will not be enabled so
> > > those
> > > tests should be skiped. Most of eDP panels only supports one
> > > resolution that is why we don't even try to use other resolution.
> > >
> > > BSpec: 7713
> > I don't like the fact that we are re-implementing driver code in
> > the
> > test to avoid false-positives. We should really replace the panel
> > on
> > the WHL machine with a panel that is expected to work.
> >
>
> I fully agree. The debugfs should be auto-suficient.
>
> In the past it was the idea of Source OK. if sink support yes and
> source ok
> and we tried to enabled but it is not getting active,
> than we have a bug somewhere.
>
> I don't know the current status of the psr debugfs anymore, but
> I believe the test case should infer this information from there
> somehow instead of re-implement the driver code here.
>
> > But, as a temporary measure to make CI happy and enable PSR2.
>
> why is this only affecting PSR2 case?

PSR2 has a maximum resolution suported and the only resolution that CI
eDP panel supports is bigger than that.

>
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >
> > Rodrigo, what's your opinion on this?
> >
> > -DK
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > tests/kms_psr.c | 21 +++++++++++++++++++--
> > > 1 file changed, 19 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > index 3e16a6bf..4792e158 100644
> > > --- a/tests/kms_psr.c
> > > +++ b/tests/kms_psr.c
> > > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> > > igt_require_f(sink_support(&data, PSR_MODE_1),
> > > "Sink does not support PSR\n");
> > >
> > > - data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> > > -
> > > data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> > > 4096);
> > > igt_assert(data.bufmgr);
> > > drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
> > >
> > > display_init(&data);
> > > +
> > > + data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> > > + if (data.supports_psr2) {
> > > + uint32_t devid =
> > > intel_get_drm_devid(data.drm_fd);
> > > + uint16_t max_h = 0, max_v = 0;
> > > + int gen = intel_gen(devid);
> > > +
> > > + if (gen >= 10 || IS_GEMINILAKE(devid)) {
> > > + max_h = 4096;
> > > + max_v = 2304;
> > > + } else if (gen == 9) {
> > > + max_h = 3640;
> > > + max_v = 2304;
> > > + }
> > > +
> > > + if (data.mode->hdisplay > max_h ||
> > > + data.mode->vdisplay > max_v)
> > > + data.supports_psr2 = false;
> > > + }
> > > }
> > >
> > > for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> > > PSR_MODE_2;

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]