From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752625AbdENLEZ (ORCPT ); Sun, 14 May 2017 07:04:25 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:38372 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750877AbdENLEX (ORCPT ); Sun, 14 May 2017 07:04:23 -0400 From: Laurent Pinchart To: Jose Abreu Cc: dri-devel@lists.freedesktop.org, 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: Sun, 14 May 2017 14:04:24 +0300 Message-ID: <1874234.uZ7HLpxeJ2@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.16-gentoo; KDE/4.14.32; x86_64; ; ) In-Reply-To: <65a1928f-a5e8-b668-0925-ff233c21c9fe@synopsys.com> References: <3844793.Y2ncMspPzi@avalon> <65a1928f-a5e8-b668-0925-ff233c21c9fe@synopsys.com> 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 v4EB4TqD014062 Hi Jose, On Friday 12 May 2017 17:06:14 Jose Abreu wrote: > On 12-05-2017 10:35, Laurent Pinchart wrote: > > 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 [snip] > >> +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). > > Yeah, in previous version I had something similar but I changed > in order to address review comments. I can change again though... Sorry, I haven't seen v1. I think it makes more sense to reflect in its name the fact that the function validates the mode against the whole pipeline, but I'll let others disagree. > >> +{ > >> + 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; > >> +} [snip] > >> @@ -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. > > Hmm, I think it makes more sense to pass connector first and then > mode ... I disagree, as this function validates a mode against a pipeline, the same way the other validation functions validate a mode against other parameters, but it's your patch :-) -- Regards, Laurent Pinchart 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: Sun, 14 May 2017 14:04:24 +0300 Message-ID: <1874234.uZ7HLpxeJ2@avalon> References: <3844793.Y2ncMspPzi@avalon> <65a1928f-a5e8-b668-0925-ff233c21c9fe@synopsys.com> 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 0B1346E025 for ; Sun, 14 May 2017 11:04:24 +0000 (UTC) In-Reply-To: <65a1928f-a5e8-b668-0925-ff233c21c9fe@synopsys.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jose Abreu Cc: Daniel Vetter , Alexey Brodkin , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Carlos Palminha List-Id: dri-devel@lists.freedesktop.org SGkgSm9zZSwKCk9uIEZyaWRheSAxMiBNYXkgMjAxNyAxNzowNjoxNCBKb3NlIEFicmV1IHdyb3Rl Ogo+IE9uIDEyLTA1LTIwMTcgMTA6MzUsIExhdXJlbnQgUGluY2hhcnQgd3JvdGU6Cj4gPiBPbiBU dWVzZGF5IDA5IE1heSAyMDE3IDE4OjAwOjEyIEpvc2UgQWJyZXUgd3JvdGU6Cj4gPj4gVGhpcyBj aGFuZ2VzIHRoZSBjb25uZWN0b3IgcHJvYmUgaGVscGVyIGZ1bmN0aW9uIHRvIHVzZSB0aGUgbmV3 Cj4gPj4gZW5jb2Rlci0+bW9kZV92YWxpZCgpIGFuZCBjcnRjLT5tb2RlX3ZhbGlkKCkgaGVscGVy IGNhbGxiYWNrcyB0bwo+ID4+IHZhbGlkYXRlIHRoZSBtb2Rlcy4KPiA+PiAKPiA+PiBUaGUgbmV3 IGNhbGxiYWNrcyBhcmUgb3B0aW9uYWwgc28gdGhlIGJlaGF2aW91ciByZW1haW5zIHRoZSBzYW1l Cj4gPj4gaWYgdGhleSBhcmUgbm90IGltcGxlbWVudGVkLiBJZiB0aGV5IGFyZSwgdGhlbiB0aGUg Y29kZSBsb29wcwo+ID4+IHRocm91Z2ggYWxsIHRoZSBjb25uZWN0b3IncyBlbmNvZGVyc1hjcnRj cyBhbmQgY2FsbHMgdGhlCj4gPj4gY2FsbGJhY2suCj4gPj4gCj4gPj4gSWYgYXQgbGVhc3QgYSB2 YWxpZCBlbmNvZGVyWGNydGMgY29tYmluYXRpb24gaXMgZm91bmQgd2hpY2gKPiA+PiBhY2NlcHRz IHRoZSBtb2RlIHRoZW4gdGhlIGZ1bmN0aW9uIHJldHVybnMgTU9ERV9PSy4KPiA+PiAKPiA+PiBT aWduZWQtb2ZmLWJ5OiBKb3NlIEFicmV1IDxqb2FicmV1QHN5bm9wc3lzLmNvbT4KPiA+PiBDYzog Q2FybG9zIFBhbG1pbmhhIDxwYWxtaW5oYUBzeW5vcHN5cy5jb20+Cj4gPj4gQ2M6IEFsZXhleSBC cm9ka2luIDxhYnJvZGtpbkBzeW5vcHN5cy5jb20+Cj4gPj4gQ2M6IFZpbGxlIFN5cmrDpGzDpCA8 dmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20+Cj4gPj4gQ2M6IERhbmllbCBWZXR0ZXIgPGRh bmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cj4gPj4gQ2M6IERhdmUgQWlybGllIDxhaXJsaWVkQGxpbnV4 LmllPgo+ID4+IENjOiBBbmRyemVqIEhhamRhIDxhLmhhamRhQHNhbXN1bmcuY29tPgo+ID4+IENj OiBBcmNoaXQgVGFuZWphIDxhcmNoaXR0QGNvZGVhdXJvcmEub3JnPgo+ID4+IC0tLQo+ID4+IAo+ ID4+IENoYW5nZXMgdjEtPnYyOgo+ID4+IAktIFVzZSBuZXcgaGVscGVycyBzdWdnZXN0ZWQgYnkg VmlsbGUKPiA+PiAJLSBDaGFuZ2UgZG9jdW1lbnRhdGlvbiAoRGFuaWVsKQo+ID4+IAkKPiA+PiAg ZHJpdmVycy9ncHUvZHJtL2RybV9wcm9iZV9oZWxwZXIuYyB8IDYwICsrKysrKysrKysrKysrKysr KysrKysrKysrKysrKy0tCj4gPj4gIDEgZmlsZSBjaGFuZ2VkLCA1NyBpbnNlcnRpb25zKCspLCAz IGRlbGV0aW9ucygtKQo+ID4+IAo+ID4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vZHJt X3Byb2JlX2hlbHBlci5jCj4gPj4gYi9kcml2ZXJzL2dwdS9kcm0vZHJtX3Byb2JlX2hlbHBlci5j IGluZGV4IDFiMGMxNGEuLmRlNDc0MTMgMTAwNjQ0Cj4gPj4gLS0tIGEvZHJpdmVycy9ncHUvZHJt L2RybV9wcm9iZV9oZWxwZXIuYwo+ID4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fcHJvYmVf aGVscGVyLmMKCltzbmlwXQoKPiA+PiArc3RhdGljIGVudW0gZHJtX21vZGVfc3RhdHVzCj4gPj4g K2RybV9tb2RlX3ZhbGlkYXRlX2Nvbm5lY3RvcihzdHJ1Y3QgZHJtX2Nvbm5lY3RvciAqY29ubmVj dG9yLAo+ID4+ICsJCQkgICAgc3RydWN0IGRybV9kaXNwbGF5X21vZGUgKm1vZGUpCj4gPiAKPiA+ IFRoaXMgZG9lcyBtb3JlIHRoYW4gdmFsaWRhdGluZyB0aGUgbW9kZSBhZ2FpbnN0IHRoZSBjb25u ZWN0b3IsIGl0Cj4gPiB2YWxpZGF0ZXMgaXQgYWdhaW5zdCB0aGUgd2hvbGUgcGlwZWxpbmUuIEkg d291bGQgY2FsbCB0aGUgZnVuY3Rpb24KPiA+IGRybV9tb2RlX3ZhbGlkYXRlX3BpcGVsaW5lKCkg KG9yIGFueSBvdGhlciBzaW1pbGFyIG5hbWUpLgo+IAo+IFllYWgsIGluIHByZXZpb3VzIHZlcnNp b24gSSBoYWQgc29tZXRoaW5nIHNpbWlsYXIgYnV0IEkgY2hhbmdlZAo+IGluIG9yZGVyIHRvIGFk ZHJlc3MgcmV2aWV3IGNvbW1lbnRzLiBJIGNhbiBjaGFuZ2UgYWdhaW4gdGhvdWdoLi4uCgpTb3Jy eSwgSSBoYXZlbid0IHNlZW4gdjEuIEkgdGhpbmsgaXQgbWFrZXMgbW9yZSBzZW5zZSB0byByZWZs ZWN0IGluIGl0cyBuYW1lIAp0aGUgZmFjdCB0aGF0IHRoZSBmdW5jdGlvbiB2YWxpZGF0ZXMgdGhl IG1vZGUgYWdhaW5zdCB0aGUgd2hvbGUgcGlwZWxpbmUsIGJ1dCAKSSdsbCBsZXQgb3RoZXJzIGRp c2FncmVlLgoKPiA+PiArewo+ID4+ICsJc3RydWN0IGRybV9kZXZpY2UgKmRldiA9IGNvbm5lY3Rv ci0+ZGV2Owo+ID4+ICsJdWludDMyX3QgKmlkcyA9IGNvbm5lY3Rvci0+ZW5jb2Rlcl9pZHM7Cj4g Pj4gKwllbnVtIGRybV9tb2RlX3N0YXR1cyByZXQgPSBNT0RFX09LOwo+ID4+ICsJdW5zaWduZWQg aW50IGk7Cj4gPj4gKwo+ID4+ICsJLyogU3RlcCAxOiBWYWxpZGF0ZSBhZ2FpbnN0IGNvbm5lY3Rv ciAqLwo+ID4+ICsJcmV0ID0gZHJtX2Nvbm5lY3Rvcl9tb2RlX3ZhbGlkKGNvbm5lY3RvciwgbW9k ZSk7Cj4gPj4gKwlpZiAocmV0ICE9IE1PREVfT0spCj4gPj4gKwkJcmV0dXJuIHJldDsKPiA+PiAr Cj4gPj4gKwkvKiBTdGVwIDI6IFZhbGlkYXRlIGFnYWluc3QgZW5jb2RlcnMgYW5kIGNydGNzICov Cj4gPj4gKwlmb3IgKGkgPSAwOyBpIDwgRFJNX0NPTk5FQ1RPUl9NQVhfRU5DT0RFUjsgaSsrKSB7 Cj4gPj4gKwkJc3RydWN0IGRybV9lbmNvZGVyICplbmNvZGVyID0gZHJtX2VuY29kZXJfZmluZChk ZXYsIGlkc1tpXSk7Cj4gPj4gKwkJc3RydWN0IGRybV9jcnRjICpjcnRjOwo+ID4+ICsKPiA+PiAr CQlpZiAoIWVuY29kZXIpCj4gPj4gKwkJCWNvbnRpbnVlOwo+ID4+ICsKPiA+PiArCQlyZXQgPSBk cm1fZW5jb2Rlcl9tb2RlX3ZhbGlkKGVuY29kZXIsIG1vZGUpOwo+ID4+ICsJCWlmIChyZXQgIT0g TU9ERV9PSykgewo+ID4+ICsJCQkvKiBObyBwb2ludCBpbiBjb250aW51aW5nIGZvciBjcnRjIGNo ZWNrIGFzIHRoaXMKPiA+IAo+ID4gZW5jb2Rlcgo+ID4gCj4gPj4gKwkJCSAqIHdpbGwgbm90IGFj Y2VwdCB0aGUgbW9kZSBhbnl3YXkuIElmIGFsbCBlbmNvZGVycwo+ID4+ICsJCQkgKiByZWplY3Qg dGhlIG1vZGUgdGhlbiwgYXQgZXhpdCwgcmV0IHdpbGwgbm90IGJlCj4gPj4gKwkJCSAqIE1PREVf T0suICovCj4gPj4gKwkJCWNvbnRpbnVlOwo+ID4+ICsJCX0KPiA+PiArCj4gPj4gKwkJZHJtX2Zv cl9lYWNoX2NydGMoY3J0YywgZGV2KSB7Cj4gPj4gKwkJCWlmICghZHJtX2VuY29kZXJfY3J0Y19v ayhlbmNvZGVyLCBjcnRjKSkKPiA+PiArCQkJCWNvbnRpbnVlOwo+ID4+ICsKPiA+PiArCQkJcmV0 ID0gZHJtX2NydGNfbW9kZV92YWxpZChjcnRjLCBtb2RlKTsKPiA+PiArCQkJaWYgKHJldCA9PSBN T0RFX09LKSB7Cj4gPj4gKwkJCQkvKiBJZiB3ZSBnZXQgdG8gdGhpcyBwb2ludCB0aGVyZSBpcyBh dCBsZWFzdAo+ID4+ICsJCQkJICogb25lIGNvbWJpbmF0aW9uIG9mIGVuY29kZXIrY3J0YyB0aGF0 IHdvcmtzCj4gPj4gKwkJCQkgKiBmb3IgdGhpcyBtb2RlLiBMZXRzIHJldHVybiBub3cuICovCj4g Pj4gKwkJCQlyZXR1cm4gcmV0Owo+ID4+ICsJCQl9Cj4gPj4gKwkJfQo+ID4+ICsJfQo+ID4+ICsK PiA+PiArCXJldHVybiByZXQ7Cj4gPj4gK30KCltzbmlwXQoKPiA+PiBAQCAtNDI4LDggKzQ4Miw4 IEBAIGludAo+ID4+IGRybV9oZWxwZXJfcHJvYmVfc2luZ2xlX2Nvbm5lY3Rvcl9tb2RlcyhzdHJ1 Y3QgZHJtX2Nvbm5lY3RvciAqY29ubmVjdG9yLAo+ID4+IAo+ID4+ICAJCWlmIChtb2RlLT5zdGF0 dXMgPT0gTU9ERV9PSykKPiA+PiAgCQkKPiA+PiAgCQkJbW9kZS0+c3RhdHVzID0gZHJtX21vZGVf dmFsaWRhdGVfZmxhZyhtb2RlLAo+ID4+IAo+ID4+IG1vZGVfZmxhZ3MpOwo+ID4+IAo+ID4+IC0J CWlmIChtb2RlLT5zdGF0dXMgPT0gTU9ERV9PSyAmJiBjb25uZWN0b3JfZnVuY3MtPm1vZGVfdmFs aWQpCj4gPj4gLQkJCW1vZGUtPnN0YXR1cyA9IGNvbm5lY3Rvcl9mdW5jcy0+bW9kZV92YWxpZChj b25uZWN0b3IsCj4gPj4gKwkJaWYgKG1vZGUtPnN0YXR1cyA9PSBNT0RFX09LKQo+ID4+ICsJCQlt b2RlLT5zdGF0dXMgPSBkcm1fbW9kZV92YWxpZGF0ZV9jb25uZWN0b3IoY29ubmVjdG9yLAo+ID4+ IAo+ID4+ICAJCQkJCQkJCSAgIG1vZGUpOwo+ID4gCj4gPiBJIHdvdWxkIHJldmVyc2UgdGhlIGFy Z3VtZW50cyBvcmRlciB0byBtYXRjaCB0aGUgc3R5bGUgb2YgdGhlIG90aGVyCj4gPiB2YWxpZGF0 aW9uIGZ1bmN0aW9ucy4KPiAKPiBIbW0sIEkgdGhpbmsgaXQgbWFrZXMgbW9yZSBzZW5zZSB0byBw YXNzIGNvbm5lY3RvciBmaXJzdCBhbmQgdGhlbgo+IG1vZGUgLi4uCgpJIGRpc2FncmVlLCBhcyB0 aGlzIGZ1bmN0aW9uIHZhbGlkYXRlcyBhIG1vZGUgYWdhaW5zdCBhIHBpcGVsaW5lLCB0aGUgc2Ft ZSB3YXkgCnRoZSBvdGhlciB2YWxpZGF0aW9uIGZ1bmN0aW9ucyB2YWxpZGF0ZSBhIG1vZGUgYWdh aW5zdCBvdGhlciBwYXJhbWV0ZXJzLCBidXQgCml0J3MgeW91ciBwYXRjaCA6LSkKCi0tIApSZWdh cmRzLAoKTGF1cmVudCBQaW5jaGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJl ZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGlu Zm8vZHJpLWRldmVsCg==