On Wed, Jan 3, 2018 at 6:40 AM, Mauro Rossi <issor.oruam@xxxxxxxxx> wrote: > > > 2018-01-03 12:16 GMT+01:00 Robert Foss <robert.foss@xxxxxxxxxxxxx>: >> >> 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. > > > Hi Robert, > > In this case I honestly do not think that splitting would add too much > value, > original commit (v1) is well documented in [1] and tackles with bug in > drmresources.cpp > which currently fails to find workable crtc -> encoder -> display output > combination > for integrated and dedicated GPUs. Besides that it was also adding > DisplayPort drm mode connector type > > So changes in drmresources.cpp belong to solving "Could not find a suitable > encoder/crtc for display X" problem, > which is a show stopper for enabling mainline graphics in android-x86 > > Other developers observed independently that the current implementation is > "embedded oriented" and only checks the first display output, > isn't it? > > The only change I did in drmconnector.cpp is to account for the additional > external drm mode connectors types > which were missing (DVID, DVII, VGA) . One question on my side: Do we need > more? > > Besides these minor types lists discussions, I would propose you to check > with Jim Bish if he should have the credit for the patch > and review the coding of his changes. > > >> >> > >> > (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). > > > Sorry I don't have a signature, as usually my patches need sign-off from > professionals, > I'm a (crash test dummy) hobbyist..really :-) That's not what S-o-b means. > Maybe Rob Herring, Qiang Yu or Chih-Wei could review and sign-off the > proposed patch The S-o-b is the Developer's Certificate of Origin[1]. It's tracking the author(s) and vouching that the code has the appropriate license. It can be the original author (Jim), modifier (you), and/or committer (Robert or me). Rob [1] https://developercertificate.org/ _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
