Date:   Fri, 24 Dec 2021 12:55:37 +0100
From:   =?iso-8859-1?Q?Jos=E9_Exp=F3sito?= <jose.exposito89@gmail.com>
To:     Melissa Wen <mwen@igalia.com>
Cc:     rodrigosiqueiramelo@gmail.com, hamohammed.sa@gmail.com,
        airlied@linux.ie, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, melissa.srw@gmail.com
Subject: Re: [PATCH 0/3] drm/vkms: add support for multiple overlay planes
Message-ID: <20211224115537.GA9505@elementary>
References: <20211213181131.17223-1-jose.exposito89@gmail.com>
 <20211223203525.4sr7rjfq6l42il5r@mail.igalia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20211223203525.4sr7rjfq6l42il5r@mail.igalia.com>
Precedence: bulk

Hi Melissa,

Thank you very much for your review.

> On Thu, Dec 23, 2021 at 07:35:48PM -0100, Melissa Wen wrote:
> What test did you run? Indeed, not all kms tests are passing and fixes
>?are welcome :)
>?
>?Last time, I used these testcases for overlay: kms_plane_cursor,
>?kms_atomic; and these tests were fine too: kms_cursor_crc, kms_writeback,
>?kms_flip

For the different patches I have been working on I have tested mainly
with kms_atomic, kms_plane_cursor and?kms_plane_alpha_blend.

For some reason,?kms_cursor_crc suspends my PC. I still need to
investigate the?cause.

I'll include a table with success/skip/fail tests before and after
the patch on v2 :)

> However, I think we need some limits for this number
>?of planes. I would suggest to just expand the enable_overlay option to
>?expose a predefined number of planes
> [...]
> I don't have a strong opinion on an exact/practical number. I took a
>?quick look at other drivers and exposing 8 planes seems reasonable to
>?me.

8 planes sound reasonable to me, I'll change it and send a revision
of [1] as well using the new constant.

Thanks again for taking the time to review this,
Jos? Exp?sito

[1]?https://lore.kernel.org/dri-devel/20211223081030.16629-1-jose.exposito89@gmail.com/T/