From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Foss Subject: Re: [drm_hwcomposer PATCH] Take Connection state into account. (v2) Date: Wed, 03 Jan 2018 12:16:33 +0100 Message-ID: <1514978193.5740.16.camel@collabora.com> References: <20180103101007.27037-1-issor.oruam@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0377679009==" Return-path: Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1857D89CA0 for ; Wed, 3 Jan 2018 11:16:39 +0000 (UTC) In-Reply-To: <20180103101007.27037-1-issor.oruam@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Mauro Rossi , dri-devel@lists.freedesktop.org Cc: jim.bish@intel.com, qiang.yu@amd.com List-Id: dri-devel@lists.freedesktop.org --===============0377679009== Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-WIXjw711IeH2wWP00PM1" --=-WIXjw711IeH2wWP00PM1 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hey Mauro, Thanks for the patch! It builds and looks good to me, but I have some suggestions however. On Wed, 2018-01-03 at 11:10 +0100, Mauro Rossi wrote: > These changes avoid following logcat error on integrated and > dedicated GPUs:=20 >=20 > ... 2245 2245 E hwc-drm-resources: Could not find a suitable > encoder/crtc for display 2 > ... 2245 2245 E hwc-drm-resources: Failed CreateDisplayPipe 56 with > -19 > ... 2245 2245 E hwcomposer-drm: Can't initialize Drm object -19 It isn't quite clear clear which errors belong to which changes, to make this patch a bit more bisectable it would be nice to see independent commits created for each error. >=20 > (v1) There are various places where we should be really taking > connection > state into account before querying the properties or assuming it > as primary. This patch fixes them. >=20 > BUG=3DNone. > TEST=3DSystem boots up and shows UI. >=20 > (v1) Signed-off-by: Jim Bish >=20 > (v2) porting of original commit 76fb87e675 of android-ia master > with additional external connector types (DisplayPort, DVID, DVII, > VGA) > Tested with i965 on Sandybridge and nouveau on GT120, GT610 The commit message isn't really following the usual format. It doesn't need to include a changelog (that is typically placed between the "---"=20 markers in the email instead), and it is missing a signoff from the submitter (you). The BUG and TEST fields are not strictly required either, but aren't a problem. > --- > drmconnector.cpp | 4 +++- > drmresources.cpp | 9 +++++++-- > 2 files changed, 10 insertions(+), 3 deletions(-) >=20 > diff --git a/drmconnector.cpp b/drmconnector.cpp > index 247f56b..145518f 100644 > --- a/drmconnector.cpp > +++ b/drmconnector.cpp > @@ -73,7 +73,9 @@ bool DrmConnector::internal() const { > } > =20 > bool DrmConnector::external() const { > - return type_ =3D=3D DRM_MODE_CONNECTOR_HDMIA; > + return type_ =3D=3D DRM_MODE_CONNECTOR_HDMIA || type_ =3D=3D > DRM_MODE_CONNECTOR_DisplayPort || > + type_ =3D=3D DRM_MODE_CONNECTOR_DVID || type_ =3D=3D > DRM_MODE_CONNECTOR_DVII || > + type_ =3D=3D DRM_MODE_CONNECTOR_VGA; > } The changes to external() should probably be broken out into it's own commit, since external() is called elsewhere changes to it _could_ introduce issues. > =20 > bool DrmConnector::valid_type() const { > diff --git a/drmresources.cpp b/drmresources.cpp > index 32dd376..d582cfe 100644 > --- a/drmresources.cpp > +++ b/drmresources.cpp > @@ -159,7 +159,7 @@ int DrmResources::Init() { > =20 > // First look for primary amongst internal connectors > for (auto &conn : connectors_) { > - if (conn->internal() && !found_primary) { > + if (conn->state() =3D=3D DRM_MODE_CONNECTED && conn->internal() && > !found_primary) { > conn->set_display(0); > found_primary =3D true; > } else { > @@ -170,7 +170,7 @@ int DrmResources::Init() { > =20 > // Then look for primary amongst external connectors > for (auto &conn : connectors_) { > - if (conn->external() && !found_primary) { > + if (conn->state() =3D=3D DRM_MODE_CONNECTED && conn->external() && > !found_primary) { > conn->set_display(0); > found_primary =3D true; > } > @@ -288,6 +288,11 @@ int DrmResources::TryEncoderForDisplay(int > display, DrmEncoder *enc) { > =20 > int DrmResources::CreateDisplayPipe(DrmConnector *connector) { > int display =3D connector->display(); > + > + // skip not connected > + if (connector->state() =3D=3D DRM_MODE_DISCONNECTED) > + return 0; > + > /* Try to use current setup first */ > if (connector->encoder()) { > int ret =3D TryEncoderForDisplay(display, connector->encoder()); --=-WIXjw711IeH2wWP00PM1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQIcBAABCAAGBQJaTLuRAAoJELeaC2oR7vcnvBQP/0uufPJSpvFhD0+9eowsuoQz rgrTO5hAH3VKUvXC7wk/FYGioqkOR9bLayP/Fz9x8lFysrWkVTP4wOQrJ34keP+7 Td9Sk5/16NU4bUWOD5ge6FFKaFrXHUZXYylnX8hCYGJvJVg3ET4UoEU+xGiUnjlh sGrDjaJRA5G23gCLcU7U0ZIVGNu2gN1/xWzz2k5uB34Ww85p/EWMv3UaVPM3GNFX 17Bs3f3A3MV1PL8TaZdvjZmtdNkXfEJVcT/rIV3GkXgZo/KeGrVm8CZCvhG9QcnX jtOEdfYh6Bq8VQOuUk3sQkfDiZfToCqcLAOKTpYIYEOYfTS4UxOtzyJllIMWVIaM kltIClZ4rokWyVJSMWErOaqa1NBNqv8/x7deRa4n1xQE19Z2LKTd3o+XI3qwsuqm Ok9jJSSi8FpDUZJl7chlC51jO7ZOYj9L7LF1Biei5Bp91OMNzBHAswBSIoJNAP/k z9U/472ecQNltCVFX9A8aXY6+8533fYxXfI76Z/PC3qGD7E4LqLk1KgxbTyp3WP0 M6ucRba0B0gR/ICxTssE/uEF77Hh1F2We0s0OgEM6L521W0eRIfASQB7FC5oFxBT 2J7qs4eQ94neQUf9vnIjBfxtGibw9VbTWYNZkpjBrg7vJ6C8NLlteLxWinFZWl4q micWtzVbhk/mMoW2L4yS =eHQ6 -----END PGP SIGNATURE----- --=-WIXjw711IeH2wWP00PM1-- --===============0377679009== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0377679009==--