On 05/08/2015 01:03 AM, Konduru, Chandra wrote:
-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@xxxxxxxxxxxxxxx]
Sent: Thursday, May 07, 2015 2:15 AM
To: Konduru, Chandra; Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Ursulin, Tvrtko
Subject: Re: [PATCH i-g-t 3/4] igt_kms: Do not reset plane position on assigning
a fb
On 05/06/2015 08:02 PM, Konduru, Chandra wrote:
diff --git a/lib/igt_kms.c b/lib/igt_kms.c index b5ba273..0665d70
100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1765,9 +1765,7 @@ void igt_plane_set_fb(igt_plane_t *plane,
struct igt_fb *fb)
plane->fb = fb;
/* hack to keep tests working that don't call igt_plane_set_size() */
if (fb) {
- /* set default plane pos/size as fb size */
- plane->crtc_x = 0;
- plane->crtc_y = 0;
Reason for doing this is to make existing kms tests to run without
modification.
Otherwise every existing test has to explicitly call
igt_plane_set_position to make sure it works. Can you make sure removing
above 2 lines doesn't break existing tests?
Hm, before your patch tests could set a plane position, change the fb, and plane
position would remain the same. After your patch changing the fb resets the
plane position. My patch reverts this behaviour to old one.
So I am not sure in what cases resetting plane position on fb changes helps and
which tests?
Which tests do you think should be run?
Ignore my previous comment, I was thinking that this change will
make plane->crtc_x/y uninitialized before calling igt_drm_plane_commit.
But igt_display_init() does memsets display which will initialize these to 0s.
So no initialization required as part here. Your changes are fine.
So r-b on patches 1-3 ?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
