[PATCH v4 3/3] drm/vkms: Switch to dynamic allocation for CRTC

[Louis Chauvet]

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 at kernel.org>
> > > > Signed-off-by: Louis Chauvet <louis.chauvet at bootlin.com>
> > > > ---
> > 
> > [...]
> > 
> > > > +	/* 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 at gmail.com>
> > 
> > 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
> >