Re: [PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()

[José Expósito]

Hi!

Javier Martinez Canillas wrote:
> Hello José,
> 
> On 6/6/22 11:55, José Expósito wrote:
> > Test the conversion from XRGB8888 to RGB332.
> > 
> > What is tested?
> > 
> >  - Different values for the X in XRGB8888 to make sure it is ignored
> >  - Different clip values: Single pixel and full and partial buffer
> >  - Well known colors: White, black, red, green, blue, magenta, yellow
> >    and cyan
> >  - Other colors: Randomly picked
> >  - Destination pitch
> > 
> > How to run the tests?
> > 
> >  $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm \
> >          --kconfig_add CONFIG_VIRTIO_UML=y \
> >          --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> > 
> > Suggested-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> > Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx>
> > 
> > ---
> 
> Thanks for addressing the issues pointed out. Patch looks good to me now.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>

Thanks for the quick review Javier :)

Javier Martinez Canillas wrote:
> By the way, I think you should request an account [0], so that you can push
> patches to drm-misc directly. Specially since AFAIU the plan is to add more
> KUnit tests in future patch series.
> 
> [0]: https://www.freedesktop.org/wiki/AccountRequests/

I'll request one, thanks for the tip.

-------

Maxime Ripard wrote:
> > > The following works correctly but it won't use User Mode Linux:
> > >
> > > ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/.kunitconfig --arch=x86_64
> > >
> > > But then, can't we add them to .kunitconfig?
> > >
> >
> > That's what I asked in the previous RFC too. Daniel mentioned that it shouldn't
> > go there because is platform specific (AFAIU, one might want to test it on x86,
> > aarch64, etc) but then I asked why we couldn't have a arch/um/.kunitconfig.
> >
> > The answer was that's not that simple and some agreement on how to do it is needed:
> >
> > https://lists.freedesktop.org/archives/dri-devel/2022-June/357617.html
>
> We should probably just document it somewhere in KMS then? It doesn't
> have to be in this patch series, but I have the feeling that we will end
> up with that discussion a lot from people frustrated to have spent too
> much time figuring it out :)

My understanding from Daniel's comment [1] is also that at the moment
it is not easy to support this use case, so yes, at least copy and
pasting the command in the docs should help everyone figure out how to
run the tests.

Documentation/gpu/drm-internals.rst seems like a good place to add some
information about how to run and add tests.
I'll send a patch with the docs ASAP.

Jose

[1] https://lore.kernel.org/dri-devel/CAGS_qxqpiCim_sy1LDK7PLwVgWf-LKW+uNFTGM=T7ydk-dYcEw@xxxxxxxxxxxxxx/