On 21/01/25 - 11:45, José Expósito wrote:
> On Mon, Jan 20, 2025 at 06:26:07PM +0100, Louis Chauvet wrote:
> > On 20/01/25 - 17:23, José Expósito wrote:
> > > > A specific allocation for the CRTC is not strictly necessary at this
> > > > point, but in order to implement dynamic configuration of VKMS (configFS),
> > > > it will be easier to have one allocation per CRTC.
> > > >
> > > > Reviewed-by: Maxime Ripard <mripard@xxxxxxxxxx>
> > > > Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> > > > ---
> >
> > [...]
> >
> > > > + /* Initialize the writeback component */
> > > > if (vkmsdev->config->writeback) {
> > > > - writeback = vkms_enable_writeback_connector(vkmsdev, crtc);
> > > > + writeback = vkms_enable_writeback_connector(vkmsdev, output);
> > > > if (writeback)
> > > > DRM_ERROR("Failed to init writeback connector\n");
> > > > }
> > >
> > > Hi Louis,
> > >
> > > Thanks for fixing this error condition.
> > >
> > > I have been working and running automated tests on top of this series and
> > > I haven't found any other issue.
> > >
> > > Reviewed-by: José Expósito <jose.exposito89@xxxxxxxxx>
> >
> > Thanks a lot! I will merge this tomorrow.
> >
> > What is your automated tests series?
>
> On the kernel side, I keep working on the ConfigFS patches here:
> https://github.com/JoseExposito/linux/commits/patch-vkms-configfs/
I saw your message just after sending my "vkms-config" series, I am
currently looking at your commits to see what are the changes.
I also sent an RFC for the ConfigFS interface, I did not change a lot of
thing. I think the two major changes are:
- Adding format configuration
- Removing YUV related configuration (encoding/range)
> It sits on top of your work to switch to managed memory. But now that
> the code is merged, it needs to be rebased.
>
> You'll notice that I kept your signed-off-by in many patches, as I
> tried to reuse as much common code as possible.
Yes, thank you. I will compare with my work and understand your change.
> About the automated testing, the series could be split in two:
I agree, and indeed that what I meant in my previous mail :-)
I did not sent them earlier because I wanted to merge
vkms-managed/allocated before.
> - vkms_config.h/c, which is tested with KUnit
Yes! Thank you for this, I wrote only very basic tests, so thank you for
your extended kunit tests.
> - ConfigFS, tested with IGT:
> https://gitlab.freedesktop.org/jexposit/igt-gpu-tools/-/commits/vkms-configfs
You did a really great job writting those tests! Please CC me
when you will send them, I will be very happy to review them.
> I made some wrong assumptions with connectors, for example, it is
> possible to create a device without connectors and hot-add/remove
> them later, and I'm still fixing them and writing tests.
> Once that work is done I'll send the series to the ML.
I did the same asumption, so my series is also broken on this point. I
don't want to duplicate the work, so I'm looking forward to your series!
Thanks for this amazing work,
Louis Chauvet
> Jose
>
> > I will also send tomorrow a new rebased iteration for:
> > - https://patchwork.freedesktop.org/series/140786/
> > - https://patchwork.freedesktop.org/series/133698/
> > - https://patchwork.freedesktop.org/patch/625883/
> >
> > If someone can look on them and leave some reviews, I will be very happy
> > to apply them!
> >
> > I will also send a first version of the configFS work (two distincts
> > series to make the review easier).
> >
> > Thanks a lot,
> > Louis Chauvet
> >
