From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback Date: Fri, 28 Apr 2017 14:41:27 +0300 Message-ID: <20170428114127.GY30290@intel.com> References: <3e1dba0cd5fc053e56f1c9c94d0cb8789ecca4ae.1493203049.git.joabreu@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id E7D1D6E77C for ; Fri, 28 Apr 2017 11:41:31 +0000 (UTC) Content-Disposition: inline In-Reply-To: <3e1dba0cd5fc053e56f1c9c94d0cb8789ecca4ae.1493203049.git.joabreu@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 T24gV2VkLCBBcHIgMjYsIDIwMTcgYXQgMTE6NDg6MzRBTSArMDEwMCwgSm9zZSBBYnJldSB3cm90 ZToKPiBTb21lIGNydGMncyBtYXkgaGF2ZSByZXN0cmljdGlvbnMgaW4gdGhlIG1vZGUgdGhleSBj YW4gZGlzcGxheS4gSW4KPiB0aGlzIHBhdGNoIGEgbmV3IGNhbGxiYWNrIChjcnRjLT5tb2RlX3Zh bGlkKCkpIGlzIGludHJvZHVjZWQgdGhhdAo+IGlzIGNhbGxlZCBhdCB0aGUgc2FtZSBzdGFnZSBv ZiBjb25uZWN0b3ItPm1vZGVfdmFsaWQoKSBjYWxsYmFjay4KPiAKPiBUaGlzIHNoYWxsIGJlIGlt cGxlbWVudGVkIGlmIHRoZSBjcnRjIGhhcyBzb21lIHNvcnQgb2YgcmVzdHJpY3Rpb24KPiBzbyB0 aGF0IHdlIGRvbid0IHByb2JlIG1vZGVzIHRoYXQgd2lsbCBmYWlsIGluIHRoZSBjb21taXQoKSBz dGFnZS4KPiBGb3IgZXhhbXBsZTogQSBnaXZlbiBjcnRjIG1heSBiZSByZXNwb25zaWJsZSB0byBz ZXQgYSBjbG9jayB2YWx1ZS4KPiBJZiB0aGUgY2xvY2sgY2FuIG5vdCBwcm9kdWNlIGFsbCB0aGUg dmFsdWVzIGZvciB0aGUgYXZhaWxhYmxlCj4gbW9kZXMgdGhlbiB0aGlzIGNhbGxiYWNrIGNhbiBi ZSB1c2VkIHRvIHJlc3RyaWN0IHRoZSBudW1iZXIgb2YKPiBwcm9iYmVkIG1vZGVzIHRvIG9ubHkg dGhlIG9uZXMgdGhhdCBjYW4gYmUgZGlzcGxheWVkLgo+IAo+IElmIHRoZSBjcnRjIGRvZXMgbm90 IGltcGxlbWVudCB0aGUgY2FsbGJhY2sgdGhlbiB0aGUgYmVoYXZpb3VyIHdpbGwKPiByZW1haW4g dGhlIHNhbWUuIEFsc28sIGZvciBhIGdpdmVuIHNldCBvZiBjcnRjcyB0aGF0IGNhbiBiZSBib3Vu ZCB0bwo+IHRoZSBjb25uZWN0b3IsIGlmIGF0IGxlYXN0IG9uZSBjYW4gZGlzcGxheSB0aGUgbW9k ZSB0aGVuIHRoZSBtb2RlCj4gd2lsbCBiZSBwcm9iYmVkLgo+IAo+IFNpZ25lZC1vZmYtYnk6IEpv c2UgQWJyZXUgPGpvYWJyZXVAc3lub3BzeXMuY29tPgo+IENjOiBDYXJsb3MgUGFsbWluaGEgPHBh bG1pbmhhQHN5bm9wc3lzLmNvbT4KPiBDYzogQWxleGV5IEJyb2RraW4gPGFicm9ka2luQHN5bm9w c3lzLmNvbT4KPiBDYzogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVs LmNvbT4KPiBDYzogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KPiBDYzog RGF2ZSBBaXJsaWUgPGFpcmxpZWRAbGludXguaWU+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9k cm1fcHJvYmVfaGVscGVyLmMgICAgICAgfCA0NCArKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKwo+ICBpbmNsdWRlL2RybS9kcm1fbW9kZXNldF9oZWxwZXJfdnRhYmxlcy5oIHwgMjYgKysr KysrKysrKysrKysrKysrKwo+ICAyIGZpbGVzIGNoYW5nZWQsIDcwIGluc2VydGlvbnMoKykKPiAK PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV9wcm9iZV9oZWxwZXIuYyBiL2RyaXZl cnMvZ3B1L2RybS9kcm1fcHJvYmVfaGVscGVyLmMKPiBpbmRleCAxYjBjMTRhLi42MWVhYzMwIDEw MDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fcHJvYmVfaGVscGVyLmMKPiArKysgYi9k cml2ZXJzL2dwdS9kcm0vZHJtX3Byb2JlX2hlbHBlci5jCj4gQEAgLTgwLDYgKzgwLDQ2IEBACj4g IAlyZXR1cm4gTU9ERV9PSzsKPiAgfQo+ICAKPiArc3RhdGljIGVudW0gZHJtX21vZGVfc3RhdHVz IGRybV9tb2RlX3ZhbGlkYXRlX2Nvbm5lY3Rvcl9jcnRjKAo+ICsJCXN0cnVjdCBkcm1fY29ubmVj dG9yICpjb25uZWN0b3IsCj4gKwkJc3RydWN0IGRybV9kaXNwbGF5X21vZGUgKm1vZGUpCgpQcm9i YWJseSBtb3JlIHR5cGljYWwgd2F5IHRvIHNwbGl0IHRoZSBsaW5lcyB3b3VsZCBiZToKc3RhdGlj IGVudW0gZHJtX21vZGVfc3RhdHVzCmRybV9tb2RlX3ZhbGlkYXRlX2Nvbm5lY3Rvcl9jcnRjKHN0 cnVjdCBkcm1fY29ubmVjdG9yICpjb25uZWN0b3IsCgkJCQkgc3RydWN0IGRybV9kaXNwbGF5X21v ZGUgKm1vZGUpCgpBbHNvICdtb2RlJyBzaG91bGQgYmUgY29uc3QuIExvb2tzIGxpa2UgdGhlIGNv bm5lY3Rvci0+bW9kZV92YWxpZCgpCmhvb2sgaXMgbWlzc2luZyB0aGUgY29uc3QgYXMgd2VsbCwg c28gdGhhdCB0b28gc2hvdWxkIGJlIGZpeGVkLgoKTm90IHN1cmUgYWJvdXQgdGhlIG5hbWUgb2Yg dGhlIGZ1bmN0aW9uLiBJdCBkb2Vzbid0IHJlYWxseSByZWZsZWN0IHdoYXQKaXQgZG9lcyBpbiB0 aGUgYmVzdCB3YXkgcG9zc2libGUuIGRybV9tb2RlX3ZhbGlkYXRlX2Nvbm5lY3Rvcl9wb3NzaWJs ZV9jcnRjcygpCnBlcmhhcHM/IEJ1dCB0aGF0IGlzIGdldHRpbmcgcXVpdGUgbG9uZyBzbyBub3Qg c3VyZSBpZiBpdCcgYSBnb29kIGlkZWEKZWl0aGVyLiBJIGd1ZXNzIHNpbXBseSBtYWtpbmcgdGhl IG5hbWUgcmVmZXIgdG8gcGx1cmFsIGNydGNzIG1pZ2h0IGJlCmEgcmVhc29uYWJsZSBjb21wcm9t aXNlLCBpZS4gZHJtX21vZGVfdmFsaWRhdGVfY29ubmVjdG9yX2NydGNzKCk/Cgo+ICt7Cj4gKwlj b25zdCBzdHJ1Y3QgZHJtX2NydGNfaGVscGVyX2Z1bmNzICpjcnRjX2Z1bmNzID0gTlVMTDsKPiAr CWVudW0gZHJtX21vZGVfc3RhdHVzIG1vZGVfc3RhdHVzID0gTU9ERV9FUlJPUjsKPiArCXN0cnVj dCBkcm1fZGV2aWNlICpkZXYgPSBjb25uZWN0b3ItPmRldjsKPiArCXN0cnVjdCBkcm1fZW5jb2Rl ciAqZW5jb2RlcjsKPiArCXN0cnVjdCBkcm1fY3J0YyAqY3J0YzsKCkEgbG90IG9mIHRoZXNlIGNh biBiZSBtb3ZlZCBpbnRvIHRpZ2h0ZXIgc2NvcGUuCgo+ICsJYm9vbCBjYWxsYmFja19mb3VuZCA9 IGZhbHNlOwoKSSBkb24ndCB0aGluayB5b3UgbmVlZCB0aGlzIHZhcmlhYmxlIGF0IGFsbC4gU2Vl IGJlbG93LgoKPiArCWludCBpOwo+ICsKPiArCWZvciAoaSA9IDA7IGkgPCBEUk1fQ09OTkVDVE9S X01BWF9FTkNPREVSOyBpKyspIHsKPiArCQllbmNvZGVyID0gZHJtX2VuY29kZXJfZmluZChkZXYs IGNvbm5lY3Rvci0+ZW5jb2Rlcl9pZHNbaV0pOwo+ICsKPiArCQlpZiAoIWVuY29kZXIpCj4gKwkJ CWNvbnRpbnVlOwo+ICsKPiArCQlkcm1fZm9yX2VhY2hfY3J0YyhjcnRjLCBkZXYpIHsKPiArCQkJ Y3J0Y19mdW5jcyA9IGNydGMtPmhlbHBlcl9wcml2YXRlOwo+ICsKPiArCQkJaWYgKCFkcm1fZW5j b2Rlcl9jcnRjX29rKGVuY29kZXIsIGNydGMpKQo+ICsJCQkJY29udGludWU7Cj4gKwkJCWlmICgh Y3J0Y19mdW5jcyB8fCAhY3J0Y19mdW5jcy0+bW9kZV92YWxpZCkKPiArCQkJCWNvbnRpbnVlOwo+ ICsKPiArCQkJLyogTU9ERV9PSz0wIGFuZCBkZWZhdWx0IG1vZGVfc3RhdHVzPU1PREVfRVJST1I9 LTEKPiArCQkJICogc28gaWYgYXQgbGVhc3Qgb25lIGNydGMgYWNjZXB0cyB0aGUgbW9kZSB3ZSBn ZXQKPiArCQkJICogTU9ERV9PSyAqLwo+ICsJCQltb2RlX3N0YXR1cyAmPSBjcnRjX2Z1bmNzLT5t b2RlX3ZhbGlkKGNydGMsIG1vZGUpOwo+ICsJCQljYWxsYmFja19mb3VuZCB8PSB0cnVlOwoKVGhl IG5lZWQgZm9yIGEgY29tbWVudCBoZXJlIHRlbGxzIG1lIHRoYXQgaXQncyBwcm9iYWJseSBiZXR0 ZXIKdG8gZ290IGZvciBhIG1vcmUgc3RyYWlnaHRmb3J3YXJkIGNvZGUuIFNvbWV0aGluZyBsaWtl OgoKbW9kZV9zdGF0dXMgPSBjcnRjX2Z1bmNzLT5tb2RlX3ZhbGlkKGNydGMsIG1vZGUpOwppZiAo bW9kZV9zdGF0eXMgPT0gTU9ERV9PSykKCXJldHVybiBtb2RlX3N0YXR1czsKCkFuZCBhdCB0aGUg ZW5kIG9mIHRoZSBmdW5jdGlvbiBqdXN0IHJldHVybiBNT0RFX0VSUk9SLCBvcgpzb21lIG90aGVy IGVycm9yIHZhbHVlIGlmIHdlIGhhdmUgc29tZXRoaW5nIHN1aXRhYmxlLiBIbW0uClBlcmhhcHMg d2Ugc2hvdWxkIGp1c3QgcmV0dXJuIHRoZSBlcnJvciBmcm9tIHRoZSBmaXJzdCBvciBsYXN0CmNy dGM/IEVpdGhlciBzaG91bGQgYmUgcHJldHR5IGVhc3ksIGp1c3QgInJldCA9IG1vZGVfc3RhdHVz IiB3aXRoaW4KdGhlIGxvb3AgaWYgaXQgZGlkbid0IHJldHVybiBNT0RFX09LIGFuZCB0aGVuICdy ZXR1cm4gcmV0JyBhdCB0aGUKZW5kIG9mIHRoZSBmdW5jdGlvbi4KCj4gKwkJfQo+ICsJfQo+ICsK PiArCS8qIFdlIGNhbiByZWFjaCBoZXJlIHdpdGhvdXQgY2FsbGluZyBtb2RlX3ZhbGlkIGlmIHRo ZXJlIGlzIG5vCj4gKwkgKiByZWdpc3RlcmVkIGNydGMgd2l0aCB0aGlzIGNhbGxiYWNrLCBsZXRz IHJldHVybiBNT0RFX09LIGluIHRoaXMKPiArCSAqIGNhc2UgKi8KPiArCXJldHVybiBjYWxsYmFj a19mb3VuZCA/IG1vZGVfc3RhdHVzIDogTU9ERV9PSzsKPiArfQo+ICsKPiAgc3RhdGljIGludCBk cm1faGVscGVyX3Byb2JlX2FkZF9jbWRsaW5lX21vZGUoc3RydWN0IGRybV9jb25uZWN0b3IgKmNv bm5lY3RvcikKPiAgewo+ICAJc3RydWN0IGRybV9jbWRsaW5lX21vZGUgKmNtZGxpbmVfbW9kZTsK PiBAQCAtNDMxLDYgKzQ3MSwxMCBAQCBpbnQgZHJtX2hlbHBlcl9wcm9iZV9zaW5nbGVfY29ubmVj dG9yX21vZGVzKHN0cnVjdCBkcm1fY29ubmVjdG9yICpjb25uZWN0b3IsCj4gIAkJaWYgKG1vZGUt PnN0YXR1cyA9PSBNT0RFX09LICYmIGNvbm5lY3Rvcl9mdW5jcy0+bW9kZV92YWxpZCkKPiAgCQkJ bW9kZS0+c3RhdHVzID0gY29ubmVjdG9yX2Z1bmNzLT5tb2RlX3ZhbGlkKGNvbm5lY3RvciwKPiAg CQkJCQkJCQkgICBtb2RlKTsKPiArCj4gKwkJaWYgKG1vZGUtPnN0YXR1cyA9PSBNT0RFX09LKQo+ ICsJCQltb2RlLT5zdGF0dXMgPSBkcm1fbW9kZV92YWxpZGF0ZV9jb25uZWN0b3JfY3J0YygKPiAr CQkJCQljb25uZWN0b3IsIG1vZGUpOwoKSW5kZW50YXRpb24uCgpBbmQgYWN0dWFsbHksIG1heWJl IHlvdSBzaG91bGQgYWxzbyBtb3ZlIHRoZSBjb25uZWN0b3JfZnVuY3MtPm1vZGVfdmFsaWQoKQpj YWxsIGludG8gdGhlIG5ldyBmdW5jdGlvbiwgYW5kIHRoZW4geW91IGNvdWxkIGp1c3QgY2FsbCB0 aGUgbmV3CmZ1bmN0aW9uIGRybV9tb2RlX3ZhbGlkYXRlX2Nvbm5lY3RvcigpIG9yIHNvbWV0aGlu ZyBhbG9uZyB0aG9zZSBsaW5lcy4KCj4gIAl9Cj4gIAo+ICBwcnVuZToKPiBkaWZmIC0tZ2l0IGEv aW5jbHVkZS9kcm0vZHJtX21vZGVzZXRfaGVscGVyX3Z0YWJsZXMuaCBiL2luY2x1ZGUvZHJtL2Ry bV9tb2Rlc2V0X2hlbHBlcl92dGFibGVzLmgKPiBpbmRleCBjMDFjMzI4Li41OWZmZmJhIDEwMDY0 NAo+IC0tLSBhL2luY2x1ZGUvZHJtL2RybV9tb2Rlc2V0X2hlbHBlcl92dGFibGVzLmgKPiArKysg Yi9pbmNsdWRlL2RybS9kcm1fbW9kZXNldF9oZWxwZXJfdnRhYmxlcy5oCj4gQEAgLTEwNiw2ICsx MDYsMzIgQEAgc3RydWN0IGRybV9jcnRjX2hlbHBlcl9mdW5jcyB7Cj4gIAl2b2lkICgqY29tbWl0 KShzdHJ1Y3QgZHJtX2NydGMgKmNydGMpOwo+ICAKPiAgCS8qKgo+ICsJICogQG1vZGVfdmFsaWQ6 Cj4gKwkgKgo+ICsJICogVGhpcyBjYWxsYmFjayBzaG91bGQgYmUgaW1wbGVtZW50ZWQgaWYgdGhl IGNydGMgaGFzIHNvbWUgc29ydCBvZgo+ICsJICogcmVzdHJpY3Rpb24gaW4gdGhlIG1vZGVzIGl0 IGNhbiBkaXNwbGF5LiBGb3IgZXhhbXBsZSwgYSBnaXZlbiBjcnRjCj4gKwkgKiBtYXkgYmUgcmVz cG9uc2libGUgdG8gc2V0IGEgY2xvY2sgdmFsdWUuIElmIHRoZSBjbG9jayBjYW4gbm90Cj4gKwkg KiBwcm9kdWNlIGFsbCB0aGUgdmFsdWVzIGZvciB0aGUgYXZhaWxhYmxlIG1vZGVzIHRoZW4gdGhp cyBjYWxsYmFjawo+ICsJICogY2FuIGJlIHVzZWQgdG8gcmVzdHJpY3QgdGhlIG51bWJlciBvZiBw cm9iYmVkIG1vZGVzIHRvIG9ubHkgdGhlIG9uZXMKPiArCSAqIHRoYXQgY2FuIGJlIGRpc3BsYXll ZC4KPiArCSAqCj4gKwkgKiBUaGlzIGlzIGRpcmVjdGx5IGNhbGxlZCBhdCB0aGUgc2FtZSBzdGFn ZSBvZiBjb25uZWN0b3ItPm1vZGVfdmFsaWQKPiArCSAqIGNhbGxiYWNrLgo+ICsJICoKPiArCSAq IE5PVEU6Cj4gKwkgKgo+ICsJICogRm9yIGEgZ2l2ZW4gc2V0IG9mIGNydGMncyBpbiBhIGRybV9k ZXZpY2UsIGlmIGF0IGxlYXN0IG9uZSBkb2VzIG5vdAo+ICsJICogaGF2ZSB0aGUgbW9kZV92YWxp ZCBjYWxsYmFjaywgb3IsIGF0IGxlYXN0IG9uZSByZXR1cm5zIE1PREVfT0sgdGhlbgo+ICsJICog dGhlIG1vZGUgd2lsbCBiZSBwcm9iYmVkLgo+ICsJICoKPiArCSAqIFJFVFVSTlM6Cj4gKwkgKgo+ ICsJICogZHJtX21vZGVfc3RhdHVzIEVudW0KPiArCSAqLwo+ICsJZW51bSBkcm1fbW9kZV9zdGF0 dXMgKCptb2RlX3ZhbGlkKShzdHJ1Y3QgZHJtX2NydGMgKmNydGMsCj4gKwkJCXN0cnVjdCBkcm1f ZGlzcGxheV9tb2RlICptb2RlKTsKCkNvbnN0LiBJbmRlbnRhdGlvbiBsb29rcyBvZmYgYWdhaW4u Cgo+ICsKPiArCS8qKgo+ICAJICogQG1vZGVfZml4dXA6Cj4gIAkgKgo+ICAJICogVGhpcyBjYWxs YmFjayBpcyB1c2VkIHRvIHZhbGlkYXRlIGEgbW9kZS4gVGhlIHBhcmFtZXRlciBtb2RlIGlzIHRo ZQo+IC0tIAo+IDEuOS4xCj4gCgotLSAKVmlsbGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGlu ZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424170AbdD1Llo (ORCPT ); Fri, 28 Apr 2017 07:41:44 -0400 Received: from mga04.intel.com ([192.55.52.120]:49841 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S939728AbdD1Lld (ORCPT ); Fri, 28 Apr 2017 07:41:33 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,388,1488873600"; d="scan'208";a="1124437289" Date: Fri, 28 Apr 2017 14:41:27 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Jose Abreu Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Carlos Palminha , Alexey Brodkin , Daniel Vetter , Dave Airlie Subject: Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback Message-ID: <20170428114127.GY30290@intel.com> References: <3e1dba0cd5fc053e56f1c9c94d0cb8789ecca4ae.1493203049.git.joabreu@synopsys.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3e1dba0cd5fc053e56f1c9c94d0cb8789ecca4ae.1493203049.git.joabreu@synopsys.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Apr 26, 2017 at 11:48:34AM +0100, Jose Abreu wrote: > Some crtc's may have restrictions in the mode they can display. In > this patch a new callback (crtc->mode_valid()) is introduced that > is called at the same stage of connector->mode_valid() callback. > > This shall be implemented if the crtc has some sort of restriction > so that we don't probe modes that will fail in the commit() stage. > For example: A given crtc may be responsible to set a clock value. > If the clock can not produce all the values for the available > modes then this callback can be used to restrict the number of > probbed modes to only the ones that can be displayed. > > If the crtc does not implement the callback then the behaviour will > remain the same. Also, for a given set of crtcs that can be bound to > the connector, if at least one can display the mode then the mode > will be probbed. > > Signed-off-by: Jose Abreu > Cc: Carlos Palminha > Cc: Alexey Brodkin > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Dave Airlie > --- > drivers/gpu/drm/drm_probe_helper.c | 44 ++++++++++++++++++++++++++++++++ > include/drm/drm_modeset_helper_vtables.h | 26 +++++++++++++++++++ > 2 files changed, 70 insertions(+) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 1b0c14a..61eac30 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -80,6 +80,46 @@ > return MODE_OK; > } > > +static enum drm_mode_status drm_mode_validate_connector_crtc( > + struct drm_connector *connector, > + struct drm_display_mode *mode) Probably more typical way to split the lines would be: static enum drm_mode_status drm_mode_validate_connector_crtc(struct drm_connector *connector, struct drm_display_mode *mode) Also 'mode' should be const. Looks like the connector->mode_valid() hook is missing the const as well, so that too should be fixed. Not sure about the name of the function. It doesn't really reflect what it does in the best way possible. drm_mode_validate_connector_possible_crtcs() perhaps? But that is getting quite long so not sure if it' a good idea either. I guess simply making the name refer to plural crtcs might be a reasonable compromise, ie. drm_mode_validate_connector_crtcs()? > +{ > + const struct drm_crtc_helper_funcs *crtc_funcs = NULL; > + enum drm_mode_status mode_status = MODE_ERROR; > + struct drm_device *dev = connector->dev; > + struct drm_encoder *encoder; > + struct drm_crtc *crtc; A lot of these can be moved into tighter scope. > + bool callback_found = false; I don't think you need this variable at all. See below. > + int i; > + > + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { > + encoder = drm_encoder_find(dev, connector->encoder_ids[i]); > + > + if (!encoder) > + continue; > + > + drm_for_each_crtc(crtc, dev) { > + crtc_funcs = crtc->helper_private; > + > + if (!drm_encoder_crtc_ok(encoder, crtc)) > + continue; > + if (!crtc_funcs || !crtc_funcs->mode_valid) > + continue; > + > + /* MODE_OK=0 and default mode_status=MODE_ERROR=-1 > + * so if at least one crtc accepts the mode we get > + * MODE_OK */ > + mode_status &= crtc_funcs->mode_valid(crtc, mode); > + callback_found |= true; The need for a comment here tells me that it's probably better to got for a more straightforward code. Something like: mode_status = crtc_funcs->mode_valid(crtc, mode); if (mode_statys == MODE_OK) return mode_status; And at the end of the function just return MODE_ERROR, or some other error value if we have something suitable. Hmm. Perhaps we should just return the error from the first or last crtc? Either should be pretty easy, just "ret = mode_status" within the loop if it didn't return MODE_OK and then 'return ret' at the end of the function. > + } > + } > + > + /* We can reach here without calling mode_valid if there is no > + * registered crtc with this callback, lets return MODE_OK in this > + * case */ > + return callback_found ? mode_status : MODE_OK; > +} > + > static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector) > { > struct drm_cmdline_mode *cmdline_mode; > @@ -431,6 +471,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, > if (mode->status == MODE_OK && connector_funcs->mode_valid) > mode->status = connector_funcs->mode_valid(connector, > mode); > + > + if (mode->status == MODE_OK) > + mode->status = drm_mode_validate_connector_crtc( > + connector, mode); Indentation. And actually, maybe you should also move the connector_funcs->mode_valid() call into the new function, and then you could just call the new function drm_mode_validate_connector() or something along those lines. > } > > prune: > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index c01c328..59fffba 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -106,6 +106,32 @@ struct drm_crtc_helper_funcs { > void (*commit)(struct drm_crtc *crtc); > > /** > + * @mode_valid: > + * > + * This callback should be implemented if the crtc has some sort of > + * restriction in the modes it can display. For example, a given crtc > + * may be responsible to set a clock value. If the clock can not > + * produce all the values for the available modes then this callback > + * can be used to restrict the number of probbed modes to only the ones > + * that can be displayed. > + * > + * This is directly called at the same stage of connector->mode_valid > + * callback. > + * > + * NOTE: > + * > + * For a given set of crtc's in a drm_device, if at least one does not > + * have the mode_valid callback, or, at least one returns MODE_OK then > + * the mode will be probbed. > + * > + * RETURNS: > + * > + * drm_mode_status Enum > + */ > + enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc, > + struct drm_display_mode *mode); Const. Indentation looks off again. > + > + /** > * @mode_fixup: > * > * This callback is used to validate a mode. The parameter mode is the > -- > 1.9.1 > -- Ville Syrjälä Intel OTC