Hey Louis,
On Wed, Feb 12, 2025 at 03:12:13PM +0100, Louis Chauvet wrote:
>
>
> Le 11/02/2025 à 11:44, José Expósito a écrit :
> > On Thu, Jan 30, 2025 at 02:48:20PM +0100, Louis Chauvet wrote:
> > > On 29/01/25 - 12:00, José Expósito wrote:
> > > > Add a list of CRTCs to vkms_config and helper functions to add and
> > > > remove as many CRTCs as wanted.
> > > >
> > > > For backwards compatibility, add one CRTC to the default configuration.
> > > >
> > > > A future patch will allow to attach planes and CRTCs, but for the
> > > > moment there are no changes in the way the output is configured.
> > > >
> > > > Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> > > > Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx>
> > >
> > > Co-developped-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> > > Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> > > Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx>
> > >
> > > [...]
> > >
> > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > > @@ -181,7 +181,8 @@ static int vkms_create(struct vkms_config *config)
> > > > goto out_devres;
> > > > }
> > > > - ret = drm_vblank_init(&vkms_device->drm, 1);
> > > > + ret = drm_vblank_init(&vkms_device->drm,
> > > > + vkms_config_get_num_crtcs(config));
> > >
> > > At this point we only create one crtc, can you move this change in the
> > > commit where you create multiple crtc?
> >
> > Since the code to add more than one CRTCs is unused but technically present, I
> > think that this is the right patch to use this function.
>
> I don't totally agree: you can create multiple vkms_config_crtc, but the
> code only creates one drm_crtc.
>
> For me it is more logical to keep 1 here, as the rest of the code only
> creates one CRTC. With the next patch, the code will create more than one
> CRTC, so it makes sense to use vkms_config_get_num_crtcs.
>
> It is not a strong blocking point, but if a v3 is needed, could you change
> it?
Fair enough, moved to the next patch in my local branch.
I'll send it in v3.
Jose
> > Anyway, at the moment it'll always return 1, so it is a no-op.
>
> The current user is only default_config, so yes I agree, it is always 1.
>
> > I didn't change it in v2, let me know if it works for you.
> >
> > Thanks,
> > Jose
> >
> > > > if (ret) {
> > > > DRM_ERROR("Failed to vblank\n");
> > > > goto out_devres;
> > > > --
> > > > 2.48.1
> > > >
>
> --
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
