[PATCH v4 7/9] drm: vkms: Refactor the plane composer to accept new formats

[Pekka Paalanen]

On Sun, 20 Feb 2022 22:02:12 -0300
Igor Torrente <igormtorrente at gmail.com> wrote:

> Hi Melissa,
> 
> On 2/9/22 18:45, Melissa Wen wrote:
> > On 02/08, Igor Torrente wrote:  
> >> Hi Melissa,
> >>
> >> On 2/8/22 07:40, Melissa Wen wrote:  
> >>> On 01/21, Igor Torrente wrote:  
> >>>> Currently the blend function only accepts XRGB_8888 and ARGB_8888
> >>>> as a color input.
> >>>>
> >>>> This patch refactors all the functions related to the plane composition
> >>>> to overcome this limitation.
> >>>>
> >>>> A new internal format(`struct pixel`) is introduced to deal with all
> >>>> possible inputs. It consists of 16 bits fields that represent each of
> >>>> the channels.
> >>>>
> >>>> The pixels blend is done using this internal format. And new handlers
> >>>> are being added to convert a specific format to/from this internal format.
> >>>>
> >>>> So the blend operation depends on these handlers to convert to this common
> >>>> format. The blended result, if necessary, is converted to the writeback
> >>>> buffer format.
> >>>>
> >>>> This patch introduces three major differences to the blend function.
> >>>> 1 - All the planes are blended at once.
> >>>> 2 - The blend calculus is done as per line instead of per pixel.
> >>>> 3 - It is responsible to calculates the CRC and writing the writeback
> >>>>       buffer(if necessary).
> >>>>
> >>>> These changes allow us to allocate way less memory in the intermediate
> >>>> buffer to compute these operations. Because now we don't need to
> >>>> have the entire intermediate image lines at once, just one line is
> >>>> enough.
> >>>>
> >>>> | Memory consumption (output dimensions) |
> >>>> |:--------------------------------------:|
> >>>> |       Current      |     This patch    |
> >>>> |:------------------:|:-----------------:|
> >>>> |   Width * Heigth   |     2 * Width     |
> >>>>
> >>>> Beyond memory, we also have a minor performance benefit from all
> >>>> these changes. Results running the IGT tests `*kms_cursor_crc*`:
> >>>>  
> >>> First, thanks for this improvement.
> >>>
> >>> Some recent changes in kms_cursor_crc caused VKMS to fail in most test
> >>> cases (iirc, only size-change and alpha-opaque are passing currently).  
> >>
> >> I updated my igt and kernel(from drm_misc/drm-misc-next) to the latest
> >> commit[1][2] and I'm getting mixed results. Sometimes most of the test
> >> passes, sometimes almost nothing passes.  
> > hmm.. is it happening when running kms_cursor_crc? Is the results
> > variation random or is it possible to follow a set of steps to reproduce
> > it? When failing, what is the reason displayed by the log?  
> 
> I investigated it a little bit and discovered that the KMS
> cursor(".*kms_cursor_crc*" ) are failing after the execution of
> writeback tests(".*kms_writeback.*").
> 
> I don't know what is causing it, but they are failing while trying to 
> commit the KMS changes.
> 
> out.txt:
> IGT-Version: 1.26-NO-GIT (x86_64) (Linux: 5.17.0-rc2 x86_64)
> Stack trace:
>    #0 ../lib/igt_core.c:1754 __igt_fail_assert()
>    #1 ../lib/igt_kms.c:3795 do_display_commit()
>    #2 ../lib/igt_kms.c:3901 igt_display_commit2()
>    #3 ../tests/kms_cursor_crc.c:820 __igt_unique____real_main814()
>    #4 ../tests/kms_cursor_crc.c:814 main()
>    #5 ../csu/libc-start.c:308 __libc_start_main()
>    #6 [_start+0x2a]
> Subtest pipe-A-cursor-size-change: FAIL
> 
> err.txt:
> (kms_cursor_crc:1936) igt_kms-CRITICAL: Test assertion failure function 
> do_display_commit, file ../lib/igt_kms.c:3795:
> (kms_cursor_crc:1936) igt_kms-CRITICAL: Failed assertion: ret == 0
> (kms_cursor_crc:1936) igt_kms-CRITICAL: Last errno: 22, Invalid argument
> (kms_cursor_crc:1936) igt_kms-CRITICAL: error: -22 != 0
> 
> > 
> >  From my side, only the first two subtest of kms_cursor_crc is passing
> > before this patch. And after your changes here, all subtests are
> > successful again, except those related to 32x10 cursor size (that needs
> > futher investigation). I didn't check how the recent changes in
> > kms_cursor_crc affect VKMS performance on it, but I bet that clearing
> > the alpha channel is the reason to have the performance back.  
> 
> Yeah, I also don't understand why the 32x10 cursor tests are failing.
> 

Hi,

are the tests putting the cursor partially outside of the CRTC area?
Or partially outside of primary plane area (which IIRC you used when you
should have used the CRTC area?)?

Does the writeback test forget to unlink the writeback connector? Or
does VKMS not handle unlinking the writeback connector?

If both are a problem, the latter would be just an unrelated bug that
exposes the first bug in VKMS, because whether writeback is used or not
probably should not affect where the cursor plane is allowed to be.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220221/d50cb186/attachment-0001.sig>