[Intel-gfx] [PATCH i-g-t 1/5] lib/igt_kms: Fix drm_plane leak

[Robert Foss]

On 2017-02-17 12:54 PM, Brian Starkey wrote:
> In the loop looking for planes on a pipe, we always want to free up
> the drm_plane afterwards.
>
> Fixes: 36656239ef96 lib/igt_kms: Implement dynamic plane count support
> Signed-off-by: Brian Starkey <brian.starkey at arm.com>
> ---
>
> Hi,
>
> This series cleans up igt_display_init a bit.
>
>  - Fixes a memory leak.
>  - Fixes out-of-bounds array access on cards with no primary plane.
>  - Fixes memory corruption/crashes on cards with no cursor plane.
>  - Cleans up some left-over stuff which wasn't really relevant after
>    the dynamic planes change.
>
> There's one detail (patch 4) I'm not really sure about - the dynamic
> planes stuff means that the "drm_plane-less" cursor plane doesn't
> work/make sense anymore, as it will never be found by
> igt_pipe_get_plane_type(). I couldn't find any tests which seemed to
> rely on having that cursor plane present, but I don't have any
> hardware with a cursor to test on.
>
> igt_display_init() could be simplified a bit further by not putting
> the primary/cursor planes at the start/end respectively, but at least
> kms_cursor_legacy appears to rely on a non-cursor plane being index 0,
> so I've left it as-is for now.
>
> I've given this a cursory test on Mali-DP, but if anyone is able to
> test it more thoroughly on Intel HW that might be a good idea.

Regarding testing, I pestered tsa at freenode about running some tests on 
intel-ci for me while writing dyn_n_planes, and he was very helpful.

I used a severely shrunk tests/intel-ci/fast-feedback.testlist
that you can find here: https://hastebin.com/tajoxawewi.pl


On a separate note, this patch looks good to me:
Reviewed-by: Robert Foss <robert.foss at collabora.com>


Rob.

>
> -Brian
>
>  lib/igt_kms.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index f59af6e75348..4ca9145726e2 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1759,12 +1759,10 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>  						    plane_resources->planes[j]);
>  			igt_assert(drm_plane);
>
> -			if (!(drm_plane->possible_crtcs & (1 << i))) {
> -				drmModeFreePlane(drm_plane);
> -				continue;
> -			}
> +			if (drm_plane->possible_crtcs & (1 << i))
> +				n_planes++;
>
> -			n_planes++;
> +			drmModeFreePlane(drm_plane);
>  		}
>
>  		igt_assert_lte(0, n_planes);
>