[Intel-gfx] [PATCH i-g-t 3/4] igt_kms: Do not reset plane position on assigning a fb

[Tvrtko Ursulin]

On 05/08/2015 01:03 AM, Konduru, Chandra wrote:

>> -----Original Message-----
>> From: Tvrtko Ursulin [mailto:tvrtko.ursulin at linux.intel.com]
>> Sent: Thursday, May 07, 2015 2:15 AM
>> To: Konduru, Chandra; Intel-gfx at lists.freedesktop.org
>> 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