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: <Zs3TeoUwn3iO7oBs@louis-chauvet-laptop>
Date: Tue, 27 Aug 2024 15:24:10 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Maxime Ripard <mripard@...nel.org>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
	Melissa Wen <melissa.srw@...il.com>,
	Maíra Canal <mairacanal@...eup.net>,
	Haneen Mohammed <hamohammed.sa@...il.com>,
	Daniel Vetter <daniel@...ll.ch>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, dri-devel@...ts.freedesktop.org,
	arthurgrillo@...eup.net, linux-kernel@...r.kernel.org,
	jeremie.dautheribes@...tlin.com, miquel.raynal@...tlin.com,
	thomas.petazzoni@...tlin.com, seanpaul@...gle.com,
	nicolejadeyee@...gle.com
Subject: Re: [PATCH v2 1/6] drm/vkms: Switch to managed for connector

Le 27/08/24 - 15:15, Maxime Ripard a écrit :
> Hi,
> 
> On Tue, Aug 27, 2024 at 11:57:36AM GMT, Louis Chauvet wrote:
> > The current VKMS driver uses non-managed function to create connectors. It
> > is not an issue yet, but in order to support multiple devices easily,
> > convert this code to use drm and device managed helpers.
> > 
> > Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> > ---
> >  drivers/gpu/drm/vkms/vkms_drv.h    |  1 -
> >  drivers/gpu/drm/vkms/vkms_output.c | 22 ++++++++++++----------
> >  2 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index 5e46ea5b96dc..9a3c6c34d1f6 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -99,7 +99,6 @@ struct vkms_crtc_state {
> >  struct vkms_output {
> >  	struct drm_crtc crtc;
> >  	struct drm_encoder encoder;
> > -	struct drm_connector connector;
> >  	struct drm_writeback_connector wb_connector;
> >  	struct hrtimer vblank_hrtimer;
> >  	ktime_t period_ns;
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > index 5ce70dd946aa..4fe6b88e8081 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -3,11 +3,11 @@
> >  #include "vkms_drv.h"
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_edid.h>
> > +#include <drm/drm_managed.h>
> >  #include <drm/drm_probe_helper.h>
> >  
> >  static const struct drm_connector_funcs vkms_connector_funcs = {
> >  	.fill_modes = drm_helper_probe_single_connector_modes,
> > -	.destroy = drm_connector_cleanup,
> >  	.reset = drm_atomic_helper_connector_reset,
> >  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > @@ -50,7 +50,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
> >  {
> >  	struct vkms_output *output = &vkmsdev->output;
> >  	struct drm_device *dev = &vkmsdev->drm;
> > -	struct drm_connector *connector = &output->connector;
> > +	struct drm_connector *connector;
> >  	struct drm_encoder *encoder = &output->encoder;
> >  	struct drm_crtc *crtc = &output->crtc;
> >  	struct vkms_plane *primary, *cursor = NULL;
> > @@ -80,8 +80,15 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = drm_connector_init(dev, connector, &vkms_connector_funcs,
> > -				 DRM_MODE_CONNECTOR_VIRTUAL);
> > +	connector = drmm_kzalloc(dev, sizeof(*connector), GFP_KERNEL);
> > +	if (!connector) {
> > +		DRM_ERROR("Failed to allocate connector\n");
> > +		ret = -ENOMEM;
> > +		goto err_connector;
> > +	}
> > +
> 
> I think it would be worth explaining why you need to move to a separate
> allocation for the connector now.
> 
> Maxime

Hi,

This is in preparation for ConfigFS implementation, as the number of 
connector/encoders/crtc/planes... will be dynamic, we need to have 
separate alloaction.

If I add this paragraph in the commit message, is it sufficient?

	A specific allocation for the connector 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 
	connector.

(same for encoder & CRTC)

Thanks,
Louis Chauvet

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ