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:
>
> ... 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.
>
> (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.
>
> BUG=None.
> TEST=System boots up and shows UI.
>
> (v1) Signed-off-by: Jim Bish <jim.bish@xxxxxxxxx>
>
> (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 "---"
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(-)
>
> 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 {
> }
>
> bool DrmConnector::external() const {
> - return type_ == DRM_MODE_CONNECTOR_HDMIA;
> + return type_ == DRM_MODE_CONNECTOR_HDMIA || type_ ==
> DRM_MODE_CONNECTOR_DisplayPort ||
> + type_ == DRM_MODE_CONNECTOR_DVID || type_ ==
> DRM_MODE_CONNECTOR_DVII ||
> + type_ == 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.
>
> 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() {
>
> // First look for primary amongst internal connectors
> for (auto &conn : connectors_) {
> - if (conn->internal() && !found_primary) {
> + if (conn->state() == DRM_MODE_CONNECTED && conn->internal() &&
> !found_primary) {
> conn->set_display(0);
> found_primary = true;
> } else {
> @@ -170,7 +170,7 @@ int DrmResources::Init() {
>
> // Then look for primary amongst external connectors
> for (auto &conn : connectors_) {
> - if (conn->external() && !found_primary) {
> + if (conn->state() == DRM_MODE_CONNECTED && conn->external() &&
> !found_primary) {
> conn->set_display(0);
> found_primary = true;
> }
> @@ -288,6 +288,11 @@ int DrmResources::TryEncoderForDisplay(int
> display, DrmEncoder *enc) {
>
> int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
> int display = connector->display();
> +
> + // skip not connected
> + if (connector->state() == DRM_MODE_DISCONNECTED)
> + return 0;
> +
> /* Try to use current setup first */
> if (connector->encoder()) {
> int ret = TryEncoderForDisplay(display, connector->encoder());Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
