From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v3 2/3] drm: Add API for capturing frame CRCs Date: Fri, 5 Aug 2016 14:42:02 +0300 Message-ID: <20160805114202.GU4329@intel.com> References: <1469196645-3061-1-git-send-email-tomeu.vizoso@collabora.com> <1469196645-3061-3-git-send-email-tomeu.vizoso@collabora.com> <20160803070638.GE4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 1AA3E6EB74 for ; Fri, 5 Aug 2016 11:42:12 +0000 (UTC) Content-Disposition: inline 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: Tomeu Vizoso Cc: "linux-doc@vger.kernel.org" , Jonathan Corbet , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Daniel Vetter , Emil Velikov List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBBdWcgMDUsIDIwMTYgYXQgMTI6NDY6MjlQTSArMDIwMCwgVG9tZXUgVml6b3NvIHdy b3RlOgo+IE9uIDMgQXVndXN0IDIwMTYgYXQgMDk6MDYsIFZpbGxlIFN5cmrDpGzDpCA8dmlsbGUu c3lyamFsYUBsaW51eC5pbnRlbC5jb20+IHdyb3RlOgo+ID4gT24gRnJpLCBKdWwgMjIsIDIwMTYg YXQgMDQ6MTA6NDRQTSArMDIwMCwgVG9tZXUgVml6b3NvIHdyb3RlOgo+ID4+IEFkZHMgZmlsZXMg YW5kIGRpcmVjdG9yaWVzIHRvIGRlYnVnZnMgZm9yIGNvbnRyb2xsaW5nIGFuZCByZWFkaW5nIGZy YW1lCj4gPj4gQ1JDcywgcGVyIENSVEM6Cj4gPj4KPiA+PiBkcmkvMC9jcnRjLTAvY3JjCj4gPj4g ZHJpLzAvY3J0Yy0wL2NyYy9jb250cm9sCj4gPj4gZHJpLzAvY3J0Yy0wL2NyYy9kYXRhCj4gPj4K PiA+PiBEcml2ZXJzIGNhbiBpbXBsZW1lbnQgdGhlIHNldF9jcmNfc291cmNlIGNhbGxiYWNrKCkg aW4gZHJtX2NydGNfZnVuY3MgdG8KPiA+PiBzdGFydCBhbmQgc3RvcCBnZW5lcmF0aW5nIGZyYW1l IENSQ3MgYW5kIGNhbiBhZGQgZW50cmllcyB0byB0aGUgb3V0cHV0Cj4gPj4gYnkgY2FsbGluZyBk cm1fY3J0Y19hZGRfY3JjX2VudHJ5Lgo+ID4+Cj4gPj4gdjI6Cj4gPj4gICAgIC0gTG90cyBvZiBn b29kIGZpeGVzIHN1Z2dlc3RlZCBieSBUaGllcnJ5Lgo+ID4+ICAgICAtIEFkZGVkIGRvY3VtZW50 YXRpb24uCj4gPj4gICAgIC0gQ2hhbmdlZCB0aGUgZGVidWdmcyBsYXlvdXQuCj4gPj4gICAgIC0g TW92ZWQgdG8gYWxsb2NhdGUgdGhlIGVudHJpZXMgY2lyY3VsYXIgcXVldWUgb25jZSB3aGVuIGZy YW1lCj4gPj4gICAgICAgZ2VuZXJhdGlvbiBnZXRzIGVuYWJsZWQgZm9yIHRoZSBmaXJzdCB0aW1l Lgo+ID4+IHYzOgo+ID4+ICAgICAtIFVzZSB0aGUgY29udHJvbCBmaWxlIGp1c3QgdG8gc2VsZWN0 IHRoZSBzb3VyY2UsIGFuZCBzdGFydCBhbmQgc3RvcAo+ID4+ICAgICAgIGNhcHR1cmUgd2hlbiB0 aGUgZGF0YSBmaWxlIGlzIG9wZW5lZCBhbmQgY2xvc2VkLCByZXNwZWN0aXZlbHkuCj4gPj4gICAg IC0gTWFrZSB2YXJpYWJsZSB0aGUgbnVtYmVyIG9mIENSQyB2YWx1ZXMgcGVyIGVudHJ5LCBwZXIg c291cmNlLgo+ID4+ICAgICAtIEFsbG9jYXRlIGVudHJpZXMgcXVldWUgZWFjaCB0aW1lIHdlIHN0 YXJ0IGNhcHR1cmluZyBhcyBub3cgdGhlcmUKPiA+PiAgICAgICBpc24ndCBhIGZpeGVkIG51bWJl ciBvZiBDUkMgdmFsdWVzIHBlciBlbnRyeS4KPiA+PiAgICAgLSBTdG9yZSB0aGUgZnJhbWUgY291 bnRlciBpbiB0aGUgZGF0YSBmaWxlIGFzIGEgOC1kaWdpdCBoZXggbnVtYmVyLgo+ID4+ICAgICAt IEZvciBzb3VyY2VzIHRoYXQgY2Fubm90IHByb3ZpZGUgdXNlZnVsIGZyYW1lIG51bWJlcnMsIHBs YWNlCj4gPj4gICAgICAgWFhYWFhYWFggaW4gdGhlIGZyYW1lIGZpZWxkLgo+ID4+Cj4gPj4gU2ln bmVkLW9mZi1ieTogVG9tZXUgVml6b3NvIDx0b21ldS52aXpvc29AY29sbGFib3JhLmNvbT4KPiA+ PiAtLS0KPiAuLi4KPiA+PiArc3RhdGljIHNzaXplX3QgY3JjX2NvbnRyb2xfd3JpdGUoc3RydWN0 IGZpbGUgKmZpbGUsIGNvbnN0IGNoYXIgX191c2VyICp1YnVmLAo+ID4+ICsgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICBzaXplX3QgbGVuLCBsb2ZmX3QgKm9mZnApCj4gPj4gK3sKPiA+PiAr ICAgICBzdHJ1Y3Qgc2VxX2ZpbGUgKm0gPSBmaWxlLT5wcml2YXRlX2RhdGE7Cj4gPj4gKyAgICAg c3RydWN0IGRybV9jcnRjICpjcnRjID0gbS0+cHJpdmF0ZTsKPiA+PiArICAgICBzdHJ1Y3QgZHJt X2NydGNfY3JjICpjcmMgPSAmY3J0Yy0+Y3JjOwo+ID4+ICsgICAgIGNoYXIgKnNvdXJjZTsKPiA+ PiArCj4gPj4gKyAgICAgaWYgKGxlbiA9PSAwKQo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIDA7 Cj4gPj4gKwo+ID4+ICsgICAgIGlmIChsZW4gPiBQQUdFX1NJWkUgLSAxKSB7Cj4gPj4gKyAgICAg ICAgICAgICBEUk1fREVCVUdfS01TKCJFeHBlY3RlZCA8ICVsdSBieXRlcyBpbnRvIGNydGMgY3Jj IGNvbnRyb2xcbiIsCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgIFBBR0VfU0laRSk7 Cj4gPj4gKyAgICAgICAgICAgICByZXR1cm4gLUUyQklHOwo+ID4+ICsgICAgIH0KPiA+PiArCj4g Pj4gKyAgICAgc291cmNlID0ga21hbGxvYyhsZW4gKyAxLCBHRlBfS0VSTkVMKTsKPiA+PiArICAg ICBpZiAoIXNvdXJjZSkKPiA+PiArICAgICAgICAgICAgIHJldHVybiAtRU5PTUVNOwo+ID4+ICsK PiA+PiArICAgICBpZiAoY29weV9mcm9tX3VzZXIoc291cmNlLCB1YnVmLCBsZW4pKSB7Cj4gPj4g KyAgICAgICAgICAgICBrZnJlZShzb3VyY2UpOwo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIC1F RkFVTFQ7Cj4gPj4gKyAgICAgfQo+ID4KPiA+IG1lbWR1cF91c2VyX251bCgpID8KPiAKPiBHb29k IGNhbGwuCj4gCj4gPj4gKwo+ID4+ICsgICAgIGlmIChzb3VyY2VbbGVuIC0gMV0gPT0gJ1xuJykK PiA+PiArICAgICAgICAgICAgIHNvdXJjZVtsZW4gLSAxXSA9ICdcMCc7Cj4gPj4gKyAgICAgZWxz ZQo+ID4+ICsgICAgICAgICAgICAgc291cmNlW2xlbl0gPSAnXDAnOwo+ID4+ICsKPiA+PiArICAg ICBzcGluX2xvY2tfaXJxKCZjcmMtPmxvY2spOwo+ID4+ICsKPiA+PiArICAgICBpZiAoY3JjLT5v cGVuZWQpIHsKPiA+PiArICAgICAgICAgICAgIGtmcmVlKHNvdXJjZSk7Cj4gPj4gKyAgICAgICAg ICAgICByZXR1cm4gLUVCVVNZOwo+ID4+ICsgICAgIH0KPiA+Cj4gPiBXaHkgbm90IGp1c3Qgc3Rh cnQgdGhlIHRoaW5nIGhlcmU/Cj4gCj4gRm9yIHRoZSBzYWtlIG9mIHN5bW1ldHJ5LCBhcyB3ZSBh cmUgc3RvcHBpbmcgd2hlbiB0aGUgZGF0YSBmaWxlIGlzIGNsb3NlZC4KClllcywgYnV0IGlmIHRo ZSBkYXRhIGZpbGUgaXMgYWxyZWFkeSBvcGVuLCB3ZSBzaG91bGQgc3RhcnQgYXMgc29vbiBhcwp0 aGUgc291cmNlIGlzIGNvbmZpZ3VyZWQuIE9yIGFyZSB5b3UgcmVkdXNpbmcgdG8gb3BlbiB0aGUg ZGF0YSBmaWxlIHcvbwphIHNvdXJjZSBzZWxlY3RlZD8KCj4gCj4gPj4gK3N0YXRpYyBzdHJ1Y3Qg ZHJtX2NydGNfY3JjX2VudHJ5ICpjcnRjX2dldF9jcmNfZW50cnkoc3RydWN0IGRybV9jcnRjX2Ny YyAqY3JjLAo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIGludCBpbmRleCkKPiA+PiArewo+ID4+ICsgICAgIHZvaWQgKnAgPSBjcmMtPmVudHJp ZXM7Cj4gPj4gKyAgICAgc2l6ZV90IGVudHJ5X3NpemUgPSAoc2l6ZW9mKCpjcmMtPmVudHJpZXMp ICsKPiA+PiArICAgICAgICAgICAgICAgICAgICAgICAgICBzaXplb2YoKmNyYy0+ZW50cmllc1sw XS5jcmNzKSAqIGNyYy0+dmFsdWVzX2NudCk7Cj4gPgo+ID4gVGhpcyBjb21wdXRhdGlvbiBpcyBk dXBsaWNhdGVkIGFsc28gaW4gY3J0Y19jcmNfb3BlbigpLiBjb3VsZCB1c2UgYQo+ID4gY29tbW9u IGhlbHBlciB0byBkbyBpdC4KPiA+Cj4gPiBTaGFtZSB0aGUgbGFuZ3VhZ2UgZG9lc24ndCBoYXZl IGEgd2F5IHRvIGRlYWwgd2l0aCBhcnJheXMgb2YgdmFyaWFibGUKPiA+IHNpemVkIGFycmF5cyBp biBhIG5pY2Ugd2F5Lgo+IAo+IE9rLgo+IAo+ID4+ICsKPiA+PiArICAgICByZXR1cm4gcCArIGVu dHJ5X3NpemUgKiBpbmRleDsKPiA+PiArfQo+ID4+ICsKPiA+PiArI2RlZmluZSBNQVhfTElORV9M RU4gKDggKyA5ICogRFJNX01BWF9DUkNfTlIgKyAxICsgMSkKPiA+PiArCj4gPj4gK3N0YXRpYyBz c2l6ZV90IGNydGNfY3JjX3JlYWQoc3RydWN0IGZpbGUgKmZpbGVwLCBjaGFyIF9fdXNlciAqdXNl cl9idWYsCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgc2l6ZV90IGNvdW50LCBsb2Zm X3QgKnBvcykKPiA+PiArewo+ID4+ICsgICAgIHN0cnVjdCBkcm1fY3J0YyAqY3J0YyA9IGZpbGVw LT5mX2lub2RlLT5pX3ByaXZhdGU7Cj4gPj4gKyAgICAgc3RydWN0IGRybV9jcnRjX2NyYyAqY3Jj ID0gJmNydGMtPmNyYzsKPiA+PiArICAgICBzdHJ1Y3QgZHJtX2NydGNfY3JjX2VudHJ5ICplbnRy eTsKPiA+PiArICAgICBjaGFyIGJ1ZltNQVhfTElORV9MRU5dOwo+ID4+ICsgICAgIGludCByZXQs IGk7Cj4gPj4gKwo+ID4+ICsgICAgIHNwaW5fbG9ja19pcnEoJmNyYy0+bG9jayk7Cj4gPj4gKwo+ ID4+ICsgICAgIGlmICghY3JjLT5zb3VyY2UpIHsKPiA+PiArICAgICAgICAgICAgIHNwaW5fdW5s b2NrX2lycSgmY3JjLT5sb2NrKTsKPiA+PiArICAgICAgICAgICAgIHJldHVybiAwOwo+ID4+ICsg ICAgIH0KPiA+PiArCj4gPj4gKyAgICAgLyogTm90aGluZyB0byByZWFkPyAqLwo+ID4+ICsgICAg IHdoaWxlIChjcnRjX2NyY19kYXRhX2NvdW50KGNyYykgPT0gMCkgewo+ID4+ICsgICAgICAgICAg ICAgaWYgKGZpbGVwLT5mX2ZsYWdzICYgT19OT05CTE9DSykgewo+ID4+ICsgICAgICAgICAgICAg ICAgICAgICBzcGluX3VubG9ja19pcnEoJmNyYy0+bG9jayk7Cj4gPj4gKyAgICAgICAgICAgICAg ICAgICAgIHJldHVybiAtRUFHQUlOOwo+ID4+ICsgICAgICAgICAgICAgfQo+ID4+ICsKPiA+PiAr ICAgICAgICAgICAgIHJldCA9IHdhaXRfZXZlbnRfaW50ZXJydXB0aWJsZV9sb2NrX2lycShjcmMt PndxLAo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIGNydGNfY3JjX2RhdGFfY291bnQoY3JjKSwKPiA+PiArICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjcmMtPmxvY2spOwo+ID4+ICsgICAg ICAgICAgICAgaWYgKHJldCkgewo+ID4+ICsgICAgICAgICAgICAgICAgICAgICBzcGluX3VubG9j a19pcnEoJmNyYy0+bG9jayk7Cj4gPj4gKyAgICAgICAgICAgICAgICAgICAgIHJldHVybiByZXQ7 Cj4gPj4gKyAgICAgICAgICAgICB9Cj4gPj4gKyAgICAgfQo+ID4+ICsKPiA+PiArICAgICAvKiBX ZSBrbm93IHdlIGhhdmUgYW4gZW50cnkgdG8gYmUgcmVhZCAqLwo+ID4+ICsgICAgIGVudHJ5ID0g Y3J0Y19nZXRfY3JjX2VudHJ5KGNyYywgY3JjLT50YWlsKTsKPiA+PiArCj4gPj4gKyAgICAgLyoK PiA+PiArICAgICAgKiAxIGZyYW1lIGZpZWxkIG9mIDggY2hhcnMgcGx1cyBhIG51bWJlciBvZiBD UkMgZmllbGRzIG9mIDgKPiA+PiArICAgICAgKiBjaGFycyBlYWNoLCBzcGFjZSBzZXBhcmF0ZWQg YW5kIHdpdGggYSBuZXdsaW5lIGF0IHRoZSBlbmQuCj4gPj4gKyAgICAgICovCj4gPj4gKyAgICAg aWYgKGNvdW50IDwgOCArIDkgKiBjcmMtPnZhbHVlc19jbnQgKyAxICsgMSkgewo+ID4KPiA+IEp1 c3QgPCBNQVhfTElORV9MRU4gcGVyaGFwcz8gT3IgY291bGQgbWFrZSBhIG1hY3JvL2Z1bmN0aW9u IHRoYXQgdGFrZXMKPiA+IGNyYy0+dmFsdWVzX2NudCBvciBEUk1fTUFYX0NSQ19OUiBhcyBhbiBh cmd1bWVudC4KPiAKPiBTb3VuZHMgZ29vZCwgd2VudCB3aXRoIGEgbWFjcm8uCj4gCj4gPj4gKyAg ICAgICAgICAgICBzcGluX3VubG9ja19pcnEoJmNyYy0+bG9jayk7Cj4gPj4gKyAgICAgICAgICAg ICByZXR1cm4gLUVJTlZBTDsKPiA+PiArICAgICB9Cj4gPj4gKwo+ID4+ICsgICAgIEJVSUxEX0JV R19PTl9OT1RfUE9XRVJfT0ZfMihEUk1fQ1JDX0VOVFJJRVNfTlIpOwo+ID4+ICsgICAgIGNyYy0+ dGFpbCA9IChjcmMtPnRhaWwgKyAxKSAmIChEUk1fQ1JDX0VOVFJJRVNfTlIgLSAxKTsKPiA+PiAr Cj4gPj4gKyAgICAgc3Bpbl91bmxvY2tfaXJxKCZjcmMtPmxvY2spOwo+ID4+ICsKPiA+PiArICAg ICBpZiAoZW50cnktPmhhc19mcmFtZV9jb3VudGVyKQo+ID4+ICsgICAgICAgICAgICAgc25wcmlu dGYoYnVmLCA5LCAiJTA4eCIsIGVudHJ5LT5mcmFtZSk7Cj4gPj4gKyAgICAgZWxzZQo+ID4+ICsg ICAgICAgICAgICAgc25wcmludGYoYnVmLCA5LCAiWFhYWFhYWFgiKTsKPiA+Cj4gPiBTaG91bGQg d2UgYWRkICIweCIgcHJlZml4IHRvIGFsbCB0aGVzZSBudW1iZXJzIHRvIG1ha2UgaXQgY2xlYXIg dGhhdAo+ID4gdGhleSdyZSBpbiBmYWN0IGhleD8KPiAKPiBTb3VuZHMgbGlrZSBhIGdvb2QgaWRl YSB0byBtZS4KPiAKPiA+PiArCj4gPj4gKyAgICAgZm9yIChpID0gMDsgaSA8IGNyYy0+dmFsdWVz X2NudDsgaSsrKQo+ID4+ICsgICAgICAgICAgICAgc25wcmludGYoYnVmICsgc3RybGVuKGJ1Ziks IDEwLCAiICUwOHgiLCBlbnRyeS0+Y3Jjc1tpXSk7Cj4gPgo+ID4gVGhlICduJyBpbiBzbnByaW50 ZigpIGhlcmUgc2VlbXMgcG9pbnRsZXNzLiBBcyBkb2VzIHRoZSBzdHJsZW4oKS4KPiAKPiBHb29k Lgo+IAo+ID4+ICsgICAgIHNucHJpbnRmKGJ1ZiArIHN0cmxlbihidWYpLCAyLCAiXG4iKTsKPiA+ PiArCj4gPj4gKyAgICAgaWYgKGNvcHlfdG9fdXNlcih1c2VyX2J1ZiwgYnVmLCBzdHJsZW4oYnVm KSArIDEpKQo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIC1FRkFVTFQ7Cj4gPj4gKwo+ID4+ICsg ICAgIHJldHVybiBzdHJsZW4oYnVmKSArIDE7Cj4gPgo+ID4gTW9yZSBzdHJsZW4oKXMgdGhhdCBz aG91bGRuJ3QgYmUgbmVlZGVkLgo+IAo+IE9rLgo+IAo+IFRoYW5rcyEKPiAKPiBUb21ldQoKLS0g ClZpbGxlIFN5cmrDpGzDpApJbnRlbCBPVEMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161292AbcHELmP (ORCPT ); Fri, 5 Aug 2016 07:42:15 -0400 Received: from mga01.intel.com ([192.55.52.88]:51996 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759594AbcHELmN (ORCPT ); Fri, 5 Aug 2016 07:42:13 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,473,1464678000"; d="scan'208";a="1035663553" Date: Fri, 5 Aug 2016 14:42:02 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Tomeu Vizoso Cc: Jonathan Corbet , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Daniel Vetter , Emil Velikov Subject: Re: [PATCH v3 2/3] drm: Add API for capturing frame CRCs Message-ID: <20160805114202.GU4329@intel.com> References: <1469196645-3061-1-git-send-email-tomeu.vizoso@collabora.com> <1469196645-3061-3-git-send-email-tomeu.vizoso@collabora.com> <20160803070638.GE4329@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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 Fri, Aug 05, 2016 at 12:46:29PM +0200, Tomeu Vizoso wrote: > On 3 August 2016 at 09:06, Ville Syrjälä wrote: > > On Fri, Jul 22, 2016 at 04:10:44PM +0200, Tomeu Vizoso wrote: > >> Adds files and directories to debugfs for controlling and reading frame > >> CRCs, per CRTC: > >> > >> dri/0/crtc-0/crc > >> dri/0/crtc-0/crc/control > >> dri/0/crtc-0/crc/data > >> > >> Drivers can implement the set_crc_source callback() in drm_crtc_funcs to > >> start and stop generating frame CRCs and can add entries to the output > >> by calling drm_crtc_add_crc_entry. > >> > >> v2: > >> - Lots of good fixes suggested by Thierry. > >> - Added documentation. > >> - Changed the debugfs layout. > >> - Moved to allocate the entries circular queue once when frame > >> generation gets enabled for the first time. > >> v3: > >> - Use the control file just to select the source, and start and stop > >> capture when the data file is opened and closed, respectively. > >> - Make variable the number of CRC values per entry, per source. > >> - Allocate entries queue each time we start capturing as now there > >> isn't a fixed number of CRC values per entry. > >> - Store the frame counter in the data file as a 8-digit hex number. > >> - For sources that cannot provide useful frame numbers, place > >> XXXXXXXX in the frame field. > >> > >> Signed-off-by: Tomeu Vizoso > >> --- > ... > >> +static ssize_t crc_control_write(struct file *file, const char __user *ubuf, > >> + size_t len, loff_t *offp) > >> +{ > >> + struct seq_file *m = file->private_data; > >> + struct drm_crtc *crtc = m->private; > >> + struct drm_crtc_crc *crc = &crtc->crc; > >> + char *source; > >> + > >> + if (len == 0) > >> + return 0; > >> + > >> + if (len > PAGE_SIZE - 1) { > >> + DRM_DEBUG_KMS("Expected < %lu bytes into crtc crc control\n", > >> + PAGE_SIZE); > >> + return -E2BIG; > >> + } > >> + > >> + source = kmalloc(len + 1, GFP_KERNEL); > >> + if (!source) > >> + return -ENOMEM; > >> + > >> + if (copy_from_user(source, ubuf, len)) { > >> + kfree(source); > >> + return -EFAULT; > >> + } > > > > memdup_user_nul() ? > > Good call. > > >> + > >> + if (source[len - 1] == '\n') > >> + source[len - 1] = '\0'; > >> + else > >> + source[len] = '\0'; > >> + > >> + spin_lock_irq(&crc->lock); > >> + > >> + if (crc->opened) { > >> + kfree(source); > >> + return -EBUSY; > >> + } > > > > Why not just start the thing here? > > For the sake of symmetry, as we are stopping when the data file is closed. Yes, but if the data file is already open, we should start as soon as the source is configured. Or are you redusing to open the data file w/o a source selected? > > >> +static struct drm_crtc_crc_entry *crtc_get_crc_entry(struct drm_crtc_crc *crc, > >> + int index) > >> +{ > >> + void *p = crc->entries; > >> + size_t entry_size = (sizeof(*crc->entries) + > >> + sizeof(*crc->entries[0].crcs) * crc->values_cnt); > > > > This computation is duplicated also in crtc_crc_open(). could use a > > common helper to do it. > > > > Shame the language doesn't have a way to deal with arrays of variable > > sized arrays in a nice way. > > Ok. > > >> + > >> + return p + entry_size * index; > >> +} > >> + > >> +#define MAX_LINE_LEN (8 + 9 * DRM_MAX_CRC_NR + 1 + 1) > >> + > >> +static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf, > >> + size_t count, loff_t *pos) > >> +{ > >> + struct drm_crtc *crtc = filep->f_inode->i_private; > >> + struct drm_crtc_crc *crc = &crtc->crc; > >> + struct drm_crtc_crc_entry *entry; > >> + char buf[MAX_LINE_LEN]; > >> + int ret, i; > >> + > >> + spin_lock_irq(&crc->lock); > >> + > >> + if (!crc->source) { > >> + spin_unlock_irq(&crc->lock); > >> + return 0; > >> + } > >> + > >> + /* Nothing to read? */ > >> + while (crtc_crc_data_count(crc) == 0) { > >> + if (filep->f_flags & O_NONBLOCK) { > >> + spin_unlock_irq(&crc->lock); > >> + return -EAGAIN; > >> + } > >> + > >> + ret = wait_event_interruptible_lock_irq(crc->wq, > >> + crtc_crc_data_count(crc), > >> + crc->lock); > >> + if (ret) { > >> + spin_unlock_irq(&crc->lock); > >> + return ret; > >> + } > >> + } > >> + > >> + /* We know we have an entry to be read */ > >> + entry = crtc_get_crc_entry(crc, crc->tail); > >> + > >> + /* > >> + * 1 frame field of 8 chars plus a number of CRC fields of 8 > >> + * chars each, space separated and with a newline at the end. > >> + */ > >> + if (count < 8 + 9 * crc->values_cnt + 1 + 1) { > > > > Just < MAX_LINE_LEN perhaps? Or could make a macro/function that takes > > crc->values_cnt or DRM_MAX_CRC_NR as an argument. > > Sounds good, went with a macro. > > >> + spin_unlock_irq(&crc->lock); > >> + return -EINVAL; > >> + } > >> + > >> + BUILD_BUG_ON_NOT_POWER_OF_2(DRM_CRC_ENTRIES_NR); > >> + crc->tail = (crc->tail + 1) & (DRM_CRC_ENTRIES_NR - 1); > >> + > >> + spin_unlock_irq(&crc->lock); > >> + > >> + if (entry->has_frame_counter) > >> + snprintf(buf, 9, "%08x", entry->frame); > >> + else > >> + snprintf(buf, 9, "XXXXXXXX"); > > > > Should we add "0x" prefix to all these numbers to make it clear that > > they're in fact hex? > > Sounds like a good idea to me. > > >> + > >> + for (i = 0; i < crc->values_cnt; i++) > >> + snprintf(buf + strlen(buf), 10, " %08x", entry->crcs[i]); > > > > The 'n' in snprintf() here seems pointless. As does the strlen(). > > Good. > > >> + snprintf(buf + strlen(buf), 2, "\n"); > >> + > >> + if (copy_to_user(user_buf, buf, strlen(buf) + 1)) > >> + return -EFAULT; > >> + > >> + return strlen(buf) + 1; > > > > More strlen()s that shouldn't be needed. > > Ok. > > Thanks! > > Tomeu -- Ville Syrjälä Intel OTC