lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z64PPk5At0JGB4Xp@fedora>
Date: Thu, 13 Feb 2025 16:26:54 +0100
From: José Expósito <jose.exposito89@...il.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>
Cc: hamohammed.sa@...il.com, simona@...ll.ch, melissa.srw@...il.com,
	maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
	tzimmermann@...e.de, airlied@...il.com,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 08/13] drm/vkms: Allow to configure multiple CRTCs

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@...tlin.com>
> > > > Signed-off-by: José Expósito <jose.exposito89@...il.com>
> > > 
> > > Co-developped-by: Louis Chauvet <louis.chauvet@...tlin.com>
> > > Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> > > Signed-off-by: José Expósito <jose.exposito89@...il.com>
> > > 
> > > [...]
> > > 
> > > > +++ 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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ