Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-20 Thread Lofstedt, Marta 
When I have some time I will try to fix the test instead of deleting it.

/Marta 

> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Tuesday, September 19, 2017 5:30 PM
> To: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
> Cc: Lofstedt, Marta <marta.lofst...@intel.com>; intel-
> g...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip 
> tests
> for sprite plane
> 
> On Tue, Sep 19, 2017 at 02:52:12PM +0100, Tvrtko Ursulin wrote:
> >
> > On 19/09/2017 14:13, Ville Syrjälä wrote:
> > > On Tue, Sep 19, 2017 at 04:01:42PM +0300, Ville Syrjälä wrote:
> > >> On Tue, Sep 19, 2017 at 03:31:21PM +0300, Marta Lofstedt wrote:
> > >>> The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
> > >>> display engine blending to be setup inorder to work in the same
> > >>> manner as the respective tests for the primary plane.
> > >>
> > >> Hmm. I don't see anything really blending related in there. It's
> > >> just using regular old XRGB framebuffers which means blending will
> > >> be off.
> > >
> > > OK. So the actual problem is that the test calls drmModePageFlip()
> > > expecting it to magically do something for the sprite plane.
> > > drmModePageFlip() by definition only operates on the primary plane
> > > of the crtc. So the fix looks correct (ie. get rid of the "flip"
> > > tests for the sprite planes) but the commit message is incorrect.
> > > This also explains why you only had to remove the tests with flip==1
> > > and didn't have to remove the flip==0 tests.
> >
> > Is it possible to flip on the sprite planes? If so would there be
> > value with keeping the tests? Just replacing the flip with some other
> > magic ioctl so those code paths are checked as well.
> 
> Setplane is the "legacy" ioctl to flip planes. In the past that took a totally
> different path through the driver so testing the pageflip ioctl separately
> made a ton of sense. These days both the pageflip and setplane ioctl both
> get converted into an atomic update internally, so from a pure driver code
> perspective they're pretty much identical.
> 
> So I supposse converting it over to setplane would be OKish. That would
> mean ripping out the flip event handling from the code as well.
> 
> >
> > Also, important thing to note is that with 90/270 the test fails
> > before the CRC check with a failure from
> > drm_framebuffer_check_src_coords. Not sure if that is something to
> investigate or another test code failure?
> 
> That is potentially due to not using atomic and having a framebuffer that
> can't accomodate both orientations. So either the plane would have to
> disabled before changing the rotation and re-enabled with the new fb
> afterwards, or the fb would have to be allocated to have
> max(w,h) x max(w,h) dimensions to accomodate both orientations.
> 
> With atomic we can do the rotation change and the fb change at the same
> time, so there is no problematic intermediate state that could exceed the fb
> dimensions.
> 
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-19 Thread Ville Syrjälä 
On Tue, Sep 19, 2017 at 02:52:12PM +0100, Tvrtko Ursulin wrote:
> 
> On 19/09/2017 14:13, Ville Syrjälä wrote:
> > On Tue, Sep 19, 2017 at 04:01:42PM +0300, Ville Syrjälä wrote:
> >> On Tue, Sep 19, 2017 at 03:31:21PM +0300, Marta Lofstedt wrote:
> >>> The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
> >>> display engine blending to be setup inorder to work in the same
> >>> manner as the respective tests for the primary plane.
> >>
> >> Hmm. I don't see anything really blending related in there. It's
> >> just using regular old XRGB framebuffers which means blending will
> >> be off.
> > 
> > OK. So the actual problem is that the test calls drmModePageFlip()
> > expecting it to magically do something for the sprite plane.
> > drmModePageFlip() by definition only operates on the primary plane of
> > the crtc. So the fix looks correct (ie. get rid of the "flip" tests for
> > the sprite planes) but the commit message is incorrect. This also
> > explains why you only had to remove the tests with flip==1 and didn't
> > have to remove the flip==0 tests.
> 
> Is it possible to flip on the sprite planes? If so would there be value 
> with keeping the tests? Just replacing the flip with some other magic 
> ioctl so those code paths are checked as well.

Setplane is the "legacy" ioctl to flip planes. In the past that took a
totally different path through the driver so testing the pageflip ioctl
separately made a ton of sense. These days both the pageflip and
setplane ioctl both get converted into an atomic update internally, so
from a pure driver code perspective they're pretty much identical.

So I supposse converting it over to setplane would be OKish. That
would mean ripping out the flip event handling from the code as well.

> 
> Also, important thing to note is that with 90/270 the test fails before 
> the CRC check with a failure from drm_framebuffer_check_src_coords. Not 
> sure if that is something to investigate or another test code failure?

That is potentially due to not using atomic and having a framebuffer
that can't accomodate both orientations. So either the plane would have
to disabled before changing the rotation and re-enabled with the new fb
afterwards, or the fb would have to be allocated to have
max(w,h) x max(w,h) dimensions to accomodate both orientations.

With atomic we can do the rotation change and the fb change at the same
time, so there is no problematic intermediate state that could exceed
the fb dimensions.
 
-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-19 Thread Tvrtko Ursulin 


On 19/09/2017 14:13, Ville Syrjälä wrote:

On Tue, Sep 19, 2017 at 04:01:42PM +0300, Ville Syrjälä wrote:

On Tue, Sep 19, 2017 at 03:31:21PM +0300, Marta Lofstedt wrote:

The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
display engine blending to be setup inorder to work in the same
manner as the respective tests for the primary plane.


Hmm. I don't see anything really blending related in there. It's
just using regular old XRGB framebuffers which means blending will
be off.


OK. So the actual problem is that the test calls drmModePageFlip()
expecting it to magically do something for the sprite plane.
drmModePageFlip() by definition only operates on the primary plane of
the crtc. So the fix looks correct (ie. get rid of the "flip" tests for
the sprite planes) but the commit message is incorrect. This also
explains why you only had to remove the tests with flip==1 and didn't
have to remove the flip==0 tests.


Is it possible to flip on the sprite planes? If so would there be value 
with keeping the tests? Just replacing the flip with some other magic 
ioctl so those code paths are checked as well.


Also, important thing to note is that with 90/270 the test fails before 
the CRC check with a failure from drm_framebuffer_check_src_coords. Not 
sure if that is something to investigate or another test code failure?


Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-19 Thread Lofstedt, Marta 


> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Tuesday, September 19, 2017 4:13 PM
> To: Lofstedt, Marta <marta.lofst...@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip 
> tests
> for sprite plane
> 
> On Tue, Sep 19, 2017 at 04:01:42PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 19, 2017 at 03:31:21PM +0300, Marta Lofstedt wrote:
> > > The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
> > > display engine blending to be setup inorder to work in the same
> > > manner as the respective tests for the primary plane.
> >
> > Hmm. I don't see anything really blending related in there. It's just
> > using regular old XRGB framebuffers which means blending will be off.
> 
> OK. So the actual problem is that the test calls drmModePageFlip() expecting
> it to magically do something for the sprite plane.
> drmModePageFlip() by definition only operates on the primary plane of the
> crtc. So the fix looks correct (ie. get rid of the "flip" tests for the 
> sprite planes)
> but the commit message is incorrect. This also explains why you only had to
> remove the tests with flip==1 and didn't have to remove the flip==0 tests.

Thanks Ville, I agree.

> 
> >
> > >
> > > Since, it is not the objective of the kms_rotation_crc to test our
> > > display blend capabilities, these subtests should be removed.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102691
> > >
> > > Signed-off-by: Marta Lofstedt <marta.lofst...@intel.com>
> > > ---
> > >  tests/kms_rotation_crc.c | 3 ---
> > >  1 file changed, 3 deletions(-)
> > >
> > > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > > index 21e264ad..064d4293 100644
> > > --- a/tests/kms_rotation_crc.c
> > > +++ b/tests/kms_rotation_crc.c
> > > @@ -647,9 +647,6 @@ igt_main
> > >   { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_90, 0 },
> > >   { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_180, 0 },
> > >   { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_270, 0 },
> > > - { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_90, 1 },
> > > - { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_180, 1 },
> > > - { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_270, 1 },
> >
> > You didn't actually remove all of them.
> >
> > >   { DRM_PLANE_TYPE_CURSOR,
> IGT_ROTATION_180, 0 },
> > >   { 0, 0, 0}
> > >   };
> > > --
> > > 2.11.0
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-19 Thread Ville Syrjälä 
On Tue, Sep 19, 2017 at 04:01:42PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 19, 2017 at 03:31:21PM +0300, Marta Lofstedt wrote:
> > The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
> > display engine blending to be setup inorder to work in the same
> > manner as the respective tests for the primary plane.
> 
> Hmm. I don't see anything really blending related in there. It's
> just using regular old XRGB framebuffers which means blending will
> be off.

OK. So the actual problem is that the test calls drmModePageFlip()
expecting it to magically do something for the sprite plane.
drmModePageFlip() by definition only operates on the primary plane of
the crtc. So the fix looks correct (ie. get rid of the "flip" tests for
the sprite planes) but the commit message is incorrect. This also
explains why you only had to remove the tests with flip==1 and didn't
have to remove the flip==0 tests.

> 
> > 
> > Since, it is not the objective of the kms_rotation_crc to test our
> > display blend capabilities, these subtests should be removed.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102691
> > 
> > Signed-off-by: Marta Lofstedt 
> > ---
> >  tests/kms_rotation_crc.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index 21e264ad..064d4293 100644
> > --- a/tests/kms_rotation_crc.c
> > +++ b/tests/kms_rotation_crc.c
> > @@ -647,9 +647,6 @@ igt_main
> > { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
> > { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
> > { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
> > -   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
> > -   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
> > -   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
> 
> You didn't actually remove all of them.
> 
> > { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
> > { 0, 0, 0}
> > };
> > -- 
> > 2.11.0
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-19 Thread Lofstedt, Marta 


> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Tuesday, September 19, 2017 4:02 PM
> To: Lofstedt, Marta <marta.lofst...@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip 
> tests
> for sprite plane
> 
> On Tue, Sep 19, 2017 at 03:31:21PM +0300, Marta Lofstedt wrote:
> > The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
> > display engine blending to be setup inorder to work in the same manner
> > as the respective tests for the primary plane.
> 
> Hmm. I don't see anything really blending related in there. It's just using
> regular old XRGB framebuffers which means blending will be off.
> 
Exactly, the overlay isn't belended against anything hence we are not seeing
The same result as for the primary plane.

> >
> > Since, it is not the objective of the kms_rotation_crc to test our
> > display blend capabilities, these subtests should be removed.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102691
> >
> > Signed-off-by: Marta Lofstedt <marta.lofst...@intel.com>
> > ---
> >  tests/kms_rotation_crc.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c index
> > 21e264ad..064d4293 100644
> > --- a/tests/kms_rotation_crc.c
> > +++ b/tests/kms_rotation_crc.c
> > @@ -647,9 +647,6 @@ igt_main
> > { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_90, 0 },
> > { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_180, 0 },
> > { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_270, 0 },
> > -   { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_90, 1 },
> > -   { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_180, 1 },
> > -   { DRM_PLANE_TYPE_OVERLAY,
> IGT_ROTATION_270, 1 },
> 
> You didn't actually remove all of them.
> 
> > { DRM_PLANE_TYPE_CURSOR,
> IGT_ROTATION_180, 0 },
> > { 0, 0, 0}
> > };
> > --
> > 2.11.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-19 Thread Ville Syrjälä 
On Tue, Sep 19, 2017 at 03:31:21PM +0300, Marta Lofstedt wrote:
> The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
> display engine blending to be setup inorder to work in the same
> manner as the respective tests for the primary plane.

Hmm. I don't see anything really blending related in there. It's
just using regular old XRGB framebuffers which means blending will
be off.

> 
> Since, it is not the objective of the kms_rotation_crc to test our
> display blend capabilities, these subtests should be removed.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102691
> 
> Signed-off-by: Marta Lofstedt 
> ---
>  tests/kms_rotation_crc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 21e264ad..064d4293 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -647,9 +647,6 @@ igt_main
>   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
>   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
>   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
> - { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
> - { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
> - { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },

You didn't actually remove all of them.

>   { DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
>   { 0, 0, 0}
>   };
> -- 
> 2.11.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] igt/kms_rotation_crc : Remove flip tests for sprite plane

2017-09-19 Thread Marta Lofstedt 
The kms_rotation_crc@sprite-rotation-*-flip subtests, would need
display engine blending to be setup inorder to work in the same
manner as the respective tests for the primary plane.

Since, it is not the objective of the kms_rotation_crc to test our
display blend capabilities, these subtests should be removed.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102691

Signed-off-by: Marta Lofstedt 
---
 tests/kms_rotation_crc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 21e264ad..064d4293 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -647,9 +647,6 @@ igt_main
{ DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 0 },
{ DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 0 },
{ DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 0 },
-   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_90, 1 },
-   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_180, 1 },
-   { DRM_PLANE_TYPE_OVERLAY, IGT_ROTATION_270, 1 },
{ DRM_PLANE_TYPE_CURSOR, IGT_ROTATION_180, 0 },
{ 0, 0, 0}
};
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx