Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 11 Mar 2019 15:00:04 -0300
From:   Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
To:     David Airlie <airlied@linux.ie>,
        Haneen Mohammed <hamohammed.sa@gmail.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/vkms: Remove useless call to drm_connector_register()
Message-ID: <20190311180004.6wljdhyhkmn66lpi@smtp.gmail.com>
References: <20190310212241.4fsqejz5rlnp7m2k@smtp.gmail.com>
 <20190311132938.GG2665@phenom.ffwll.local>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="vuw4btwyf5tpthnz"
Content-Disposition: inline
In-Reply-To: <20190311132938.GG2665@phenom.ffwll.local>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--vuw4btwyf5tpthnz
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On 03/11, Daniel Vetter wrote:
> On Sun, Mar 10, 2019 at 06:22:41PM -0300, Rodrigo Siqueira wrote:
> > The function vkms_output_init() is invoked during the module
> > initialization, and it handles the creation/configuration of the vkms
> > essential elements (e.g., connectors, encoder, etc). Among the
> > initializations, this function tries to initialize a connector and
> > register it by calling drm_connector_register(). However, inside the
> > drm_connector_register(), at the beginning of this function there is the
> > following validation:
> >=20
> >  if (!connector->dev->registered)
> >    return 0;
> >=20
> > In this sense, invoke drm_connector_register() after initializing the
> > connector has no effect because the register field is false. The
> > connector register happens when drm_dev_register() is invoked.
> > Therefore, this commit removes the drm_connector_register() from
> > vkms_output_init().
> >=20
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> >  drivers/gpu/drm/vkms/vkms_output.c | 6 ------
> >  1 file changed, 6 deletions(-)
> >=20
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/=
vkms_output.c
> > index 3b162b25312e..a6cee4c279c2 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -71,12 +71,6 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> > =20
> >  	drm_connector_helper_add(connector, &vkms_conn_helper_funcs);
> > =20
> > -	ret =3D drm_connector_register(connector);
>=20
> Yeah that's only needed for connectors added when hotplugging at runtime,
> not for connectors which are created at driver load time.
>=20
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>=20
> btw same issue exists with the drm_connector_unregister, that's also not
> needed.

Thanks for your review :)

Should I send a V2 that also removes the drm_connector_unregister()? Or
should I send it in a separated patch?

Best Regards

> -Daniel
>=20
> > -	if (ret) {
> > -		DRM_ERROR("Failed to register connector\n");
> > -		goto err_connector_register;
> > -	}
> > -
> >  	ret =3D drm_encoder_init(dev, encoder, &vkms_encoder_funcs,
> >  			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> >  	if (ret) {
> > --=20
> > 2.21.0
>=20
> --=20
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

--=20
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of S=E3o Paulo

--vuw4btwyf5tpthnz
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE4tZ+ii1mjMCMQbfkWJzP/comvP8FAlyGoiQACgkQWJzP/com
vP9hXQ//XQnUpPZ9eL8ka8N0w2/by5wUzIxth/cZWG4KT6dBFmhVe56BwytewjlK
FauC8Ev7sbI8HP6DsRZArQLlKefbL5tHYt9kZAAS8qwMvWrwxpJf788aAYiRia0E
Svuni78WupOGShq2+XW28LB8liqcxcQ5l1Iyxw+qf2Pq/aCn4VPWVYcb45dg99PR
awjpmlUAdF3QHMLm+FjxZkuL4wQlP399c/SrImLwBm2SfYWAzscAdGaHpJl2Tl6m
Rm5T59jSktrMS0EjwqV6ic805oBIvlmJPKpekw1cFLUuJZPCyYwL/nugn29jSXPJ
wZRNC3wklp/EEdyyI269twATwzBVLd/+b6Eh1JAB8xzeU3A+t/y3Xq36o0IF4FP5
Vd9IZvZYw2e4k2rPtP39gvEy4kUyeBxGoIDq/pR2lnqt62JqVJFr3pslNbqjTbZC
TdfnmYFwJd7c2MJOd1EFmVFU2qvtNFiyTbTcMaDjR04bww96X/bQK7GJEo4LoGLH
mHWT33bFu5VCNmMdagt25FA1rgESkOBnzo50Kw/Ao8C7tsrSjdTjd9qV4XHwpVQ6
9+Hpifm+viIk2HYvE9Hw+43DD+Bh1kmK/wFfek+cI8R77cS5Y8528hg5kAjdEg1i
Orb9uujM0VUQCvTQLFivr6bQbw0uJCSx1qrRpqUi8yJonliZNDE=
=cIXa
-----END PGP SIGNATURE-----

--vuw4btwyf5tpthnz--