diff mbox series

[RFT,i-g-t] tests/prime_vgem/basic-fence-flip: Probe display resolution

Message ID 20190410114322.3379-1-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [RFT,i-g-t] tests/prime_vgem/basic-fence-flip: Probe display resolution | expand

Commit Message

Tvrtko Ursulin April 10, 2019, 11:43 a.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Some displays might not support hardcoded 1024x768.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109294
---
 tests/prime_vgem.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Chris Wilson April 10, 2019, 11:48 a.m. UTC | #1 
Quoting Tvrtko Ursulin (2019-04-10 12:43:22)
> @@ -754,8 +768,8 @@ static void test_flip(int i915, int vgem, unsigned hang)
>                 uint32_t offsets[4] = {};
>                 int fd;
>  
> -               bo[i].width = 1024;
> -               bo[i].height = 768;
> +               bo[i].width = mode->hdisplay;
> +               bo[i].height = mode->vdisplay;
>                 bo[i].bpp = 32;
>                 vgem_create(vgem, &bo[i]);

That may not result in a buffer that we are able to flip to. :|
width = ALIGN(hdisplay, 16); vdisplay should be ok.

I would query what happened to the scalers though :)
-Chris
Tvrtko Ursulin April 10, 2019, 12:11 p.m. UTC | #2 
On 10/04/2019 12:48, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-04-10 12:43:22)
>> @@ -754,8 +768,8 @@ static void test_flip(int i915, int vgem, unsigned hang)
>>                  uint32_t offsets[4] = {};
>>                  int fd;
>>   
>> -               bo[i].width = 1024;
>> -               bo[i].height = 768;
>> +               bo[i].width = mode->hdisplay;
>> +               bo[i].height = mode->vdisplay;
>>                  bo[i].bpp = 32;
>>                  vgem_create(vgem, &bo[i]);
> 
> That may not result in a buffer that we are able to flip to. :|
> width = ALIGN(hdisplay, 16); vdisplay should be ok.

Oh.. well I don't know. Maarten helpfully described in the BZ that the 
skip is due BO being too small for the FB. Aligning width would then 
make it too large. Is that OK? Who assigned this display related IGT bug 
to me anyway? :))

> I would query what happened to the scalers though :)

Are they supposed to automagicaly apply any fb to any output? Or an 
explicit step is required? Regardless - it may be better to involve less 
of the driver and hardware stack in a simple test.

Regards,

Tvrtko
Tvrtko Ursulin May 7, 2019, 8:14 a.m. UTC | #3 
+Maarten - could you please have a quick look at this patch to see if it 
makes sense to you? (https://patchwork.freedesktop.org/series/59284/ - 
if you don't have the mailing list history.)

Thanks,

Tvrtko

On 10/04/2019 13:11, Tvrtko Ursulin wrote:
> 
> On 10/04/2019 12:48, Chris Wilson wrote:
>> Quoting Tvrtko Ursulin (2019-04-10 12:43:22)
>>> @@ -754,8 +768,8 @@ static void test_flip(int i915, int vgem, 
>>> unsigned hang)
>>>                  uint32_t offsets[4] = {};
>>>                  int fd;
>>> -               bo[i].width = 1024;
>>> -               bo[i].height = 768;
>>> +               bo[i].width = mode->hdisplay;
>>> +               bo[i].height = mode->vdisplay;
>>>                  bo[i].bpp = 32;
>>>                  vgem_create(vgem, &bo[i]);
>>
>> That may not result in a buffer that we are able to flip to. :|
>> width = ALIGN(hdisplay, 16); vdisplay should be ok.
> 
> Oh.. well I don't know. Maarten helpfully described in the BZ that the 
> skip is due BO being too small for the FB. Aligning width would then 
> make it too large. Is that OK? Who assigned this display related IGT bug 
> to me anyway? :))
> 
>> I would query what happened to the scalers though :)
> 
> Are they supposed to automagicaly apply any fb to any output? Or an 
> explicit step is required? Regardless - it may be better to involve less 
> of the driver and hardware stack in a simple test.
> 
> Regards,
> 
> Tvrtko
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
Kahola, Mika May 9, 2019, 10:51 a.m. UTC | #4 
On Wed, 2019-04-10 at 13:11 +0100, Tvrtko Ursulin wrote:
> On 10/04/2019 12:48, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-04-10 12:43:22)
> > > @@ -754,8 +768,8 @@ static void test_flip(int i915, int vgem,
> > > unsigned hang)
> > >                  uint32_t offsets[4] = {};
> > >                  int fd;
> > >   
> > > -               bo[i].width = 1024;
> > > -               bo[i].height = 768;
> > > +               bo[i].width = mode->hdisplay;
> > > +               bo[i].height = mode->vdisplay;
> > >                  bo[i].bpp = 32;
> > >                  vgem_create(vgem, &bo[i]);
> > 
> > That may not result in a buffer that we are able to flip to. :|
> > width = ALIGN(hdisplay, 16); vdisplay should be ok.
> 
> Oh.. well I don't know. Maarten helpfully described in the BZ that
> the 
> skip is due BO being too small for the FB. Aligning width would then 
> make it too large. Is that OK? Who assigned this display related IGT
> bug 
> to me anyway? :))
I don't know about that. I have a task to improve the test in my
backlog too :) 

This patch definitely improves the test. However, I wasn't able to
apply the patch cleanly on my tree. Maybe it needs a rebase? Anyway, CI
seems to be happy with the change. 

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> 
> > I would query what happened to the scalers though :)
> 
> Are they supposed to automagicaly apply any fb to any output? Or an 
> explicit step is required? Regardless - it may be better to involve
> less 
> of the driver and hardware stack in a simple test.
> 
> Regards,
> 
> Tvrtko
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tvrtko Ursulin May 9, 2019, 12:14 p.m. UTC | #5 
On 09/05/2019 11:51, Kahola, Mika wrote:
> On Wed, 2019-04-10 at 13:11 +0100, Tvrtko Ursulin wrote:
>> On 10/04/2019 12:48, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-04-10 12:43:22)
>>>> @@ -754,8 +768,8 @@ static void test_flip(int i915, int vgem,
>>>> unsigned hang)
>>>>                   uint32_t offsets[4] = {};
>>>>                   int fd;
>>>>    
>>>> -               bo[i].width = 1024;
>>>> -               bo[i].height = 768;
>>>> +               bo[i].width = mode->hdisplay;
>>>> +               bo[i].height = mode->vdisplay;
>>>>                   bo[i].bpp = 32;
>>>>                   vgem_create(vgem, &bo[i]);
>>>
>>> That may not result in a buffer that we are able to flip to. :|
>>> width = ALIGN(hdisplay, 16); vdisplay should be ok.
>>
>> Oh.. well I don't know. Maarten helpfully described in the BZ that
>> the
>> skip is due BO being too small for the FB. Aligning width would then
>> make it too large. Is that OK? Who assigned this display related IGT
>> bug
>> to me anyway? :))
> I don't know about that. I have a task to improve the test in my
> backlog too :)
> 
> This patch definitely improves the test. However, I wasn't able to
> apply the patch cleanly on my tree. Maybe it needs a rebase? Anyway, CI
> seems to be happy with the change.
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>

Thanks, pushed!

One less thing on your todo list now. :)

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
index 60bb951c8cbe..458846add9ee 100644
--- a/tests/prime_vgem.c
+++ b/tests/prime_vgem.c
@@ -744,8 +744,22 @@  static void flip_to_vgem(int i915, int vgem,
 
 static void test_flip(int i915, int vgem, unsigned hang)
 {
-	struct vgem_bo bo[2];
+	drmModeModeInfo *mode = NULL;
 	uint32_t fb_id[2], handle[2], crtc_id;
+	igt_display_t display;
+	igt_output_t *output;
+	struct vgem_bo bo[2];
+	enum pipe pipe;
+
+	igt_display_require(&display, i915);
+	igt_display_require_output(&display);
+
+	for_each_pipe_with_valid_output(&display, pipe, output) {
+		mode = igt_output_get_mode(output);
+		break;
+	}
+
+	igt_assert(mode);
 
 	signal(SIGHUP, sighandler);
 
@@ -754,8 +768,8 @@  static void test_flip(int i915, int vgem, unsigned hang)
 		uint32_t offsets[4] = {};
 		int fd;
 
-		bo[i].width = 1024;
-		bo[i].height = 768;
+		bo[i].width = mode->hdisplay;
+		bo[i].height = mode->vdisplay;
 		bo[i].bpp = 32;
 		vgem_create(vgem, &bo[i]);