From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Date: Fri, 12 May 2017 12:35:28 +0300 Message-ID: <3844793.Y2ncMspPzi@avalon> References: Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [185.26.127.97]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3A0376E009 for ; Fri, 12 May 2017 09:35:28 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: dri-devel@lists.freedesktop.org Cc: Jose Abreu , Daniel Vetter , Alexey Brodkin , linux-kernel@vger.kernel.org, Carlos Palminha List-Id: dri-devel@lists.freedesktop.org SGkgSm9zZSwKClRoYW5rIHlvdSBmb3IgdGhlIHBhdGNoLgoKT24gVHVlc2RheSAwOSBNYXkgMjAx NyAxODowMDoxMiBKb3NlIEFicmV1IHdyb3RlOgo+IFRoaXMgY2hhbmdlcyB0aGUgY29ubmVjdG9y IHByb2JlIGhlbHBlciBmdW5jdGlvbiB0byB1c2UgdGhlIG5ldwo+IGVuY29kZXItPm1vZGVfdmFs aWQoKSBhbmQgY3J0Yy0+bW9kZV92YWxpZCgpIGhlbHBlciBjYWxsYmFja3MgdG8KPiB2YWxpZGF0 ZSB0aGUgbW9kZXMuCj4gCj4gVGhlIG5ldyBjYWxsYmFja3MgYXJlIG9wdGlvbmFsIHNvIHRoZSBi ZWhhdmlvdXIgcmVtYWlucyB0aGUgc2FtZQo+IGlmIHRoZXkgYXJlIG5vdCBpbXBsZW1lbnRlZC4g SWYgdGhleSBhcmUsIHRoZW4gdGhlIGNvZGUgbG9vcHMKPiB0aHJvdWdoIGFsbCB0aGUgY29ubmVj dG9yJ3MgZW5jb2RlcnNYY3J0Y3MgYW5kIGNhbGxzIHRoZQo+IGNhbGxiYWNrLgo+IAo+IElmIGF0 IGxlYXN0IGEgdmFsaWQgZW5jb2RlclhjcnRjIGNvbWJpbmF0aW9uIGlzIGZvdW5kIHdoaWNoCj4g YWNjZXB0cyB0aGUgbW9kZSB0aGVuIHRoZSBmdW5jdGlvbiByZXR1cm5zIE1PREVfT0suCj4gCj4g U2lnbmVkLW9mZi1ieTogSm9zZSBBYnJldSA8am9hYnJldUBzeW5vcHN5cy5jb20+Cj4gQ2M6IENh cmxvcyBQYWxtaW5oYSA8cGFsbWluaGFAc3lub3BzeXMuY29tPgo+IENjOiBBbGV4ZXkgQnJvZGtp biA8YWJyb2RraW5Ac3lub3BzeXMuY29tPgo+IENjOiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5 cmphbGFAbGludXguaW50ZWwuY29tPgo+IENjOiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVy QGZmd2xsLmNoPgo+IENjOiBEYXZlIEFpcmxpZSA8YWlybGllZEBsaW51eC5pZT4KPiBDYzogQW5k cnplaiBIYWpkYSA8YS5oYWpkYUBzYW1zdW5nLmNvbT4KPiBDYzogQXJjaGl0IFRhbmVqYSA8YXJj aGl0dEBjb2RlYXVyb3JhLm9yZz4KPiAtLS0KPiAKPiBDaGFuZ2VzIHYxLT52MjoKPiAJLSBVc2Ug bmV3IGhlbHBlcnMgc3VnZ2VzdGVkIGJ5IFZpbGxlCj4gCS0gQ2hhbmdlIGRvY3VtZW50YXRpb24g KERhbmllbCkKPiAKPiAgZHJpdmVycy9ncHUvZHJtL2RybV9wcm9iZV9oZWxwZXIuYyB8IDYwICsr KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tCj4gIDEgZmlsZSBjaGFuZ2VkLCA1NyBp bnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQo+IAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dw dS9kcm0vZHJtX3Byb2JlX2hlbHBlci5jCj4gYi9kcml2ZXJzL2dwdS9kcm0vZHJtX3Byb2JlX2hl bHBlci5jIGluZGV4IDFiMGMxNGEuLmRlNDc0MTMgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUv ZHJtL2RybV9wcm9iZV9oZWxwZXIuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fcHJvYmVf aGVscGVyLmMKPiBAQCAtMzksNiArMzksOCBAQAo+ICAjaW5jbHVkZSA8ZHJtL2RybV9mYl9oZWxw ZXIuaD4KPiAgI2luY2x1ZGUgPGRybS9kcm1fZWRpZC5oPgo+IAo+ICsjaW5jbHVkZSAiZHJtX2Ny dGNfaW50ZXJuYWwuaCIKPiArCj4gIC8qKgo+ICAgKiBET0M6IG91dHB1dCBwcm9iaW5nIGhlbHBl ciBvdmVydmlldwo+ICAgKgo+IEBAIC04MCw2ICs4Miw1NCBAQAo+ICAJcmV0dXJuIE1PREVfT0s7 Cj4gIH0KPiAKPiArc3RhdGljIGVudW0gZHJtX21vZGVfc3RhdHVzCj4gK2RybV9tb2RlX3ZhbGlk YXRlX2Nvbm5lY3RvcihzdHJ1Y3QgZHJtX2Nvbm5lY3RvciAqY29ubmVjdG9yLAo+ICsJCQkgICAg c3RydWN0IGRybV9kaXNwbGF5X21vZGUgKm1vZGUpCgpUaGlzIGRvZXMgbW9yZSB0aGFuIHZhbGlk YXRpbmcgdGhlIG1vZGUgYWdhaW5zdCB0aGUgY29ubmVjdG9yLCBpdCB2YWxpZGF0ZXMgaXQgCmFn YWluc3QgdGhlIHdob2xlIHBpcGVsaW5lLiBJIHdvdWxkIGNhbGwgdGhlIGZ1bmN0aW9uIApkcm1f bW9kZV92YWxpZGF0ZV9waXBlbGluZSgpIChvciBhbnkgb3RoZXIgc2ltaWxhciBuYW1lKS4KCj4g K3sKPiArCXN0cnVjdCBkcm1fZGV2aWNlICpkZXYgPSBjb25uZWN0b3ItPmRldjsKPiArCXVpbnQz Ml90ICppZHMgPSBjb25uZWN0b3ItPmVuY29kZXJfaWRzOwo+ICsJZW51bSBkcm1fbW9kZV9zdGF0 dXMgcmV0ID0gTU9ERV9PSzsKPiArCXVuc2lnbmVkIGludCBpOwo+ICsKPiArCS8qIFN0ZXAgMTog VmFsaWRhdGUgYWdhaW5zdCBjb25uZWN0b3IgKi8KPiArCXJldCA9IGRybV9jb25uZWN0b3JfbW9k ZV92YWxpZChjb25uZWN0b3IsIG1vZGUpOwo+ICsJaWYgKHJldCAhPSBNT0RFX09LKQo+ICsJCXJl dHVybiByZXQ7Cj4gKwo+ICsJLyogU3RlcCAyOiBWYWxpZGF0ZSBhZ2FpbnN0IGVuY29kZXJzIGFu ZCBjcnRjcyAqLwo+ICsJZm9yIChpID0gMDsgaSA8IERSTV9DT05ORUNUT1JfTUFYX0VOQ09ERVI7 IGkrKykgewo+ICsJCXN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlciA9IGRybV9lbmNvZGVyX2Zp bmQoZGV2LCBpZHNbaV0pOwo+ICsJCXN0cnVjdCBkcm1fY3J0YyAqY3J0YzsKPiArCj4gKwkJaWYg KCFlbmNvZGVyKQo+ICsJCQljb250aW51ZTsKPiArCj4gKwkJcmV0ID0gZHJtX2VuY29kZXJfbW9k ZV92YWxpZChlbmNvZGVyLCBtb2RlKTsKPiArCQlpZiAocmV0ICE9IE1PREVfT0spIHsKPiArCQkJ LyogTm8gcG9pbnQgaW4gY29udGludWluZyBmb3IgY3J0YyBjaGVjayBhcyB0aGlzIAplbmNvZGVy Cj4gKwkJCSAqIHdpbGwgbm90IGFjY2VwdCB0aGUgbW9kZSBhbnl3YXkuIElmIGFsbCBlbmNvZGVy cwo+ICsJCQkgKiByZWplY3QgdGhlIG1vZGUgdGhlbiwgYXQgZXhpdCwgcmV0IHdpbGwgbm90IGJl Cj4gKwkJCSAqIE1PREVfT0suICovCj4gKwkJCWNvbnRpbnVlOwo+ICsJCX0KPiArCj4gKwkJZHJt X2Zvcl9lYWNoX2NydGMoY3J0YywgZGV2KSB7Cj4gKwkJCWlmICghZHJtX2VuY29kZXJfY3J0Y19v ayhlbmNvZGVyLCBjcnRjKSkKPiArCQkJCWNvbnRpbnVlOwo+ICsKPiArCQkJcmV0ID0gZHJtX2Ny dGNfbW9kZV92YWxpZChjcnRjLCBtb2RlKTsKPiArCQkJaWYgKHJldCA9PSBNT0RFX09LKSB7Cj4g KwkJCQkvKiBJZiB3ZSBnZXQgdG8gdGhpcyBwb2ludCB0aGVyZSBpcyBhdCBsZWFzdAo+ICsJCQkJ ICogb25lIGNvbWJpbmF0aW9uIG9mIGVuY29kZXIrY3J0YyB0aGF0IHdvcmtzCj4gKwkJCQkgKiBm b3IgdGhpcyBtb2RlLiBMZXRzIHJldHVybiBub3cuICovCj4gKwkJCQlyZXR1cm4gcmV0Owo+ICsJ CQl9Cj4gKwkJfQo+ICsJfQo+ICsKPiArCXJldHVybiByZXQ7Cj4gK30KPiArCj4gIHN0YXRpYyBp bnQgZHJtX2hlbHBlcl9wcm9iZV9hZGRfY21kbGluZV9tb2RlKHN0cnVjdCBkcm1fY29ubmVjdG9y Cj4gKmNvbm5lY3RvcikKPiAgewo+ICAJc3RydWN0IGRybV9jbWRsaW5lX21vZGUgKmNtZGxpbmVf bW9kZTsKPiBAQCAtMjg0LDcgKzMzNCwxMSBAQCB2b2lkIGRybV9rbXNfaGVscGVyX3BvbGxfZW5h YmxlKHN0cnVjdCBkcm1fZGV2aWNlICpkZXYpCj4gICAqICAgIC0gZHJtX21vZGVfdmFsaWRhdGVf ZmxhZygpIGNoZWNrcyB0aGUgbW9kZXMgYWdhaW5zdCBiYXNpYyBjb25uZWN0b3IKPiAgICogICAg ICBjYXBhYmlsaXRpZXMgKGludGVybGFjZV9hbGxvd2VkLGRvdWJsZXNjYW5fYWxsb3dlZCxzdGVy ZW9fYWxsb3dlZCkKPiAgICogICAgLSB0aGUgb3B0aW9uYWwgJmRybV9jb25uZWN0b3JfaGVscGVy X2Z1bmNzLm1vZGVfdmFsaWQgaGVscGVyIGNhbgo+IHBlcmZvcm0KPiAtICogICAgICBkcml2ZXIg YW5kL29yIGhhcmR3YXJlIHNwZWNpZmljIGNoZWNrcwo+ICsgKiAgICAgIGRyaXZlciBhbmQvb3Ig c2luayBzcGVjaWZpYyBjaGVja3MKPiArICogICAgLSB0aGUgb3B0aW9uYWwgJmRybV9jcnRjX2hl bHBlcl9mdW5jcy5tb2RlX3ZhbGlkIGFuZAo+ICsgKiAgICAgICZkcm1fZW5jb2Rlcl9oZWxwZXJf ZnVuY3MubW9kZV92YWxpZCBoZWxwZXJzIGNhbiBwZXJmb3JtIGRyaXZlcgo+IGFuZC9vcgo+ICsg KiAgICAgIHNvdXJjZSBzcGVjaWZpYyBjaGVja3Mgd2hpY2ggYXJlIGFsc28gZW5mb3JjZWQgYnkg dGhlCj4gbW9kZXNldC9hdG9taWMKPiArICogICAgICBoZWxwZXJzCj4gICAqCj4gICAqIDUuIEFu eSBtb2RlIHdob3NlIHN0YXR1cyBpcyBub3QgT0sgaXMgcHJ1bmVkIGZyb20gdGhlIGNvbm5lY3Rv cidzIG1vZGVzCj4gbGlzdCwKPiAgICogICAgYWNjb21wYW5pZWQgYnkgYSBkZWJ1ZyBtZXNzYWdl IGluZGljYXRpbmcgdGhlIHJlYXNvbiBmb3IgdGhlIG1vZGUncwo+IEBAIC00MjgsOCArNDgyLDgg QEAgaW50Cj4gZHJtX2hlbHBlcl9wcm9iZV9zaW5nbGVfY29ubmVjdG9yX21vZGVzKHN0cnVjdCBk cm1fY29ubmVjdG9yICpjb25uZWN0b3IsCj4gIAkJaWYgKG1vZGUtPnN0YXR1cyA9PSBNT0RFX09L KQo+ICAJCQltb2RlLT5zdGF0dXMgPSBkcm1fbW9kZV92YWxpZGF0ZV9mbGFnKG1vZGUsCj4gbW9k ZV9mbGFncyk7Cj4gCj4gLQkJaWYgKG1vZGUtPnN0YXR1cyA9PSBNT0RFX09LICYmIGNvbm5lY3Rv cl9mdW5jcy0+bW9kZV92YWxpZCkKPiAtCQkJbW9kZS0+c3RhdHVzID0gY29ubmVjdG9yX2Z1bmNz LT5tb2RlX3ZhbGlkKGNvbm5lY3RvciwKPiArCQlpZiAobW9kZS0+c3RhdHVzID09IE1PREVfT0sp Cj4gKwkJCW1vZGUtPnN0YXR1cyA9IGRybV9tb2RlX3ZhbGlkYXRlX2Nvbm5lY3Rvcihjb25uZWN0 b3IsCj4gIAkJCQkJCQkJICAgbW9kZSk7CgpJIHdvdWxkIHJldmVyc2UgdGhlIGFyZ3VtZW50cyBv cmRlciB0byBtYXRjaCB0aGUgc3R5bGUgb2YgdGhlIG90aGVyIHZhbGlkYXRpb24gCmZ1bmN0aW9u cy4KCj4gIAl9CgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QK ZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9w Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756580AbdELJhI (ORCPT ); Fri, 12 May 2017 05:37:08 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:36382 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756298AbdELJf2 (ORCPT ); Fri, 12 May 2017 05:35:28 -0400 From: Laurent Pinchart To: dri-devel@lists.freedesktop.org Cc: Jose Abreu , linux-kernel@vger.kernel.org, Daniel Vetter , Alexey Brodkin , Carlos Palminha Subject: Re: [PATCH v2 5/8] drm: Use new mode_valid() helpers in connector probe helper Date: Fri, 12 May 2017 12:35:28 +0300 Message-ID: <3844793.Y2ncMspPzi@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.16-gentoo; KDE/4.14.29; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v4C9co9V026182 Hi Jose, Thank you for the patch. On Tuesday 09 May 2017 18:00:12 Jose Abreu wrote: > This changes the connector probe helper function to use the new > encoder->mode_valid() and crtc->mode_valid() helper callbacks to > validate the modes. > > The new callbacks are optional so the behaviour remains the same > if they are not implemented. If they are, then the code loops > through all the connector's encodersXcrtcs and calls the > callback. > > If at least a valid encoderXcrtc combination is found which > accepts the mode then the function returns MODE_OK. > > Signed-off-by: Jose Abreu > Cc: Carlos Palminha > Cc: Alexey Brodkin > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Dave Airlie > Cc: Andrzej Hajda > Cc: Archit Taneja > --- > > Changes v1->v2: > - Use new helpers suggested by Ville > - Change documentation (Daniel) > > drivers/gpu/drm/drm_probe_helper.c | 60 +++++++++++++++++++++++++++++++++-- > 1 file changed, 57 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > b/drivers/gpu/drm/drm_probe_helper.c index 1b0c14a..de47413 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -39,6 +39,8 @@ > #include > #include > > +#include "drm_crtc_internal.h" > + > /** > * DOC: output probing helper overview > * > @@ -80,6 +82,54 @@ > return MODE_OK; > } > > +static enum drm_mode_status > +drm_mode_validate_connector(struct drm_connector *connector, > + struct drm_display_mode *mode) This does more than validating the mode against the connector, it validates it against the whole pipeline. I would call the function drm_mode_validate_pipeline() (or any other similar name). > +{ > + struct drm_device *dev = connector->dev; > + uint32_t *ids = connector->encoder_ids; > + enum drm_mode_status ret = MODE_OK; > + unsigned int i; > + > + /* Step 1: Validate against connector */ > + ret = drm_connector_mode_valid(connector, mode); > + if (ret != MODE_OK) > + return ret; > + > + /* Step 2: Validate against encoders and crtcs */ > + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { > + struct drm_encoder *encoder = drm_encoder_find(dev, ids[i]); > + struct drm_crtc *crtc; > + > + if (!encoder) > + continue; > + > + ret = drm_encoder_mode_valid(encoder, mode); > + if (ret != MODE_OK) { > + /* No point in continuing for crtc check as this encoder > + * will not accept the mode anyway. If all encoders > + * reject the mode then, at exit, ret will not be > + * MODE_OK. */ > + continue; > + } > + > + drm_for_each_crtc(crtc, dev) { > + if (!drm_encoder_crtc_ok(encoder, crtc)) > + continue; > + > + ret = drm_crtc_mode_valid(crtc, mode); > + if (ret == MODE_OK) { > + /* If we get to this point there is at least > + * one combination of encoder+crtc that works > + * for this mode. Lets return now. */ > + return ret; > + } > + } > + } > + > + return ret; > +} > + > static int drm_helper_probe_add_cmdline_mode(struct drm_connector > *connector) > { > struct drm_cmdline_mode *cmdline_mode; > @@ -284,7 +334,11 @@ void drm_kms_helper_poll_enable(struct drm_device *dev) > * - drm_mode_validate_flag() checks the modes against basic connector > * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed) > * - the optional &drm_connector_helper_funcs.mode_valid helper can > perform > - * driver and/or hardware specific checks > + * driver and/or sink specific checks > + * - the optional &drm_crtc_helper_funcs.mode_valid and > + * &drm_encoder_helper_funcs.mode_valid helpers can perform driver > and/or > + * source specific checks which are also enforced by the > modeset/atomic > + * helpers > * > * 5. Any mode whose status is not OK is pruned from the connector's modes > list, > * accompanied by a debug message indicating the reason for the mode's > @@ -428,8 +482,8 @@ int > drm_helper_probe_single_connector_modes(struct drm_connector *connector, > if (mode->status == MODE_OK) > mode->status = drm_mode_validate_flag(mode, > mode_flags); > > - if (mode->status == MODE_OK && connector_funcs->mode_valid) > - mode->status = connector_funcs->mode_valid(connector, > + if (mode->status == MODE_OK) > + mode->status = drm_mode_validate_connector(connector, > mode); I would reverse the arguments order to match the style of the other validation functions. > } -- Regards, Laurent Pinchart