From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2] drm/color: Document CTM eqations Date: Tue, 31 Jan 2017 19:22:15 +0200 Message-ID: <20170131172215.GV31595@intel.com> References: <1485859714-26619-1-git-send-email-brian.starkey@arm.com> <20170131151828.GU31595@intel.com> <20170131153928.GB11506@e106950-lin.cambridge.arm.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 342BA6E285 for ; Tue, 31 Jan 2017 17:22:28 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170131153928.GB11506@e106950-lin.cambridge.arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Brian Starkey Cc: Daniel Vetter , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBKYW4gMzEsIDIwMTcgYXQgMDM6Mzk6MjlQTSArMDAwMCwgQnJpYW4gU3RhcmtleSB3 cm90ZToKPiBIaSBWaWxsZSwKPiAKPiBPbiBUdWUsIEphbiAzMSwgMjAxNyBhdCAwNToxODoyOFBN ICswMjAwLCBWaWxsZSBTeXJqw6Rsw6Qgd3JvdGU6Cj4gPk9uIFR1ZSwgSmFuIDMxLCAyMDE3IGF0 IDEwOjQ4OjM0QU0gKzAwMDAsIEJyaWFuIFN0YXJrZXkgd3JvdGU6Cj4gPj4gRXhwbGljaXRseSBz dGF0ZSB0aGUgZXhwZWN0ZWQgQ1RNIGVxdWF0aW9ucyBpbiB0aGUga2VybmVsZG9jIGZvciB0aGUg Q1RNCj4gPj4gcHJvcGVydHksIGFuZCB0aGUgZm9ybSBvZiB0aGUgbWF0cml4IG9uIHN0cnVjdCBk cm1fY29sb3JfY3RtLgo+ID4+Cj4gPj4gQ2M6IFZpbGxlIFN5cmrDpGzDpCA8dmlsbGUuc3lyamFs YUBsaW51eC5pbnRlbC5jb20+Cj4gPj4gQ2M6IExpb25lbCBMYW5kd2VybGluIDxsaW9uZWwuZy5s YW5kd2VybGluQGludGVsLmNvbT4KPiA+PiBDYzogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRl ckBmZndsbC5jaD4KPiA+PiBTaWduZWQtb2ZmLWJ5OiBCcmlhbiBTdGFya2V5IDxicmlhbi5zdGFy a2V5QGFybS5jb20+Cj4gPj4gLS0tCj4gPj4gIGRyaXZlcnMvZ3B1L2RybS9kcm1fY29sb3JfbWdt dC5jIHwgICAxMyArKysrKysrKysrKysrCj4gPj4gIGluY2x1ZGUvdWFwaS9kcm0vZHJtX21vZGUu aCAgICAgIHwgICAgOCArKysrKysrLQo+ID4+ICAyIGZpbGVzIGNoYW5nZWQsIDIwIGluc2VydGlv bnMoKyksIDEgZGVsZXRpb24oLSkKPiA+Pgo+ID4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9k cm0vZHJtX2NvbG9yX21nbXQuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fY29sb3JfbWdtdC5jCj4g Pj4gaW5kZXggNzg5YjRjNjVjZDY5Li43NTczY2E0YjZlYTYgMTAwNjQ0Cj4gPj4gLS0tIGEvZHJp dmVycy9ncHUvZHJtL2RybV9jb2xvcl9tZ210LmMKPiA+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0v ZHJtX2NvbG9yX21nbXQuYwo+ID4+IEBAIC02Miw2ICs2MiwxOSBAQAo+ID4+ICAgKgl1bml0L3Bh c3MtdGhydSBtYXRyaXggc2hvdWxkIGJlIHVzZWQuIFRoaXMgaXMgZ2VuZXJhbGx5IHRoZSBkcml2 ZXIKPiA+PiAgICoJYm9vdC11cCBzdGF0ZSB0b28uCj4gPj4gICAqCj4gPj4gKyAqCVRoZSBvdXRw dXQgdmVjdG9yIGlzIHJlbGF0ZWQgdG8gdGhlIGlucHV0IHZlY3RvciBhcyBiZWxvdzoKPiA+PiAr ICoKPiA+PiArICoJfCBgYG91dFswXSA9IG1hdHJpeFswXSAqIGluWzBdICsgbWF0cml4WzFdICog aW5bMV0gKyBtYXRyaXhbMl0gKiBpblsyXTtgYAo+ID4+ICsgKgl8IGBgb3V0WzFdID0gbWF0cml4 WzNdICogaW5bMF0gKyBtYXRyaXhbNF0gKiBpblsxXSArIG1hdHJpeFs1XSAqIGluWzJdO2BgCj4g Pj4gKyAqCXwgYGBvdXRbMl0gPSBtYXRyaXhbNl0gKiBpblswXSArIG1hdHJpeFs3XSAqIGluWzFd ICsgbWF0cml4WzhdICogaW5bMl07YGAKPiA+PiArICoKPiA+PiArICoJVGhlIGNvbXBvbmVudCBv cmRlciBpbiB0aGUgaW5wdXQvb3V0cHV0IHZlY3RvcnMgaXMgYXNzdW1lZCB0byBiZQo+ID4+ICsg Kgl7IFIsIEcsIEIgfS4KPiA+PiArICoKPiA+PiArICoJVGhlIGNvbG9yLXNwYWNlIG9mIHRoZSBp bnB1dCB2ZWN0b3IgbXVzdCBub3QgYmUgY29uZnVzZWQgd2l0aCB0aGUKPiA+PiArICoJY29sb3It c3BhY2UgaW1wbGllZCBieSBhIGZyYW1lYnVmZmVyIHBpeGVsIGZvcm1hdCwgd2hpY2ggbWF5IGJl IHRoZSBzYW1lCj4gPj4gKyAqCW9yIGRpZmZlcmVudC4KPiA+PiArICoKPiA+PiAgICog4oCcR0FN TUFfTFVU4oCdOgo+ID4+ICAgKglCbG9iIHByb3BlcnR5IHRvIHNldCB0aGUgZ2FtbWEgbG9va3Vw IHRhYmxlIChMVVQpIG1hcHBpbmcgcGl4ZWwgZGF0YQo+ID4+ICAgKglhZnRlciB0aGUgdHJhbnNm b3JtYXRpb24gbWF0cml4IHRvIGRhdGEgc2VudCB0byB0aGUgY29ubmVjdG9yLiBUaGUKPiA+PiBk aWZmIC0tZ2l0IGEvaW5jbHVkZS91YXBpL2RybS9kcm1fbW9kZS5oIGIvaW5jbHVkZS91YXBpL2Ry bS9kcm1fbW9kZS5oCj4gPj4gaW5kZXggY2U3ZWZlMmU4YTVlLi4zNDAxNjM3Y2FmOGUgMTAwNjQ0 Cj4gPj4gLS0tIGEvaW5jbHVkZS91YXBpL2RybS9kcm1fbW9kZS5oCj4gPj4gKysrIGIvaW5jbHVk ZS91YXBpL2RybS9kcm1fbW9kZS5oCj4gPj4gQEAgLTUyNSw3ICs1MjUsMTMgQEAgc3RydWN0IGRy bV9tb2RlX2NydGNfbHV0IHsKPiA+PiAgfTsKPiA+Pgo+ID4+ICBzdHJ1Y3QgZHJtX2NvbG9yX2N0 bSB7Cj4gPj4gLQkvKiBDb252ZXJzaW9uIG1hdHJpeCBpbiBTMzEuMzIgZm9ybWF0LiAqLwo+ID4+ ICsJLyoKPiA+PiArCSAqIENvbnZlcnNpb24gbWF0cml4IGluIFMzMS4zMiBmb3JtYXQsIGluIHJv dy1tYWpvciBmb3JtOgo+ID4KPiA+czMyLjMyIGlzIGhvdyBJJ2Qgc3RhdGUgdGhhdCAodG8gbWF0 Y2ggdGhlIHJlZ3VsYXIgczMyIGFuZCB3aGF0bm90Cj4gPnR5cGVzKS4KPiA+Cj4gCj4gQ2FuIHlv dSBleHBsYWluIGEgYml0IG1vcmUgd2hhdCBleGFjdGx5IHlvdSBtZWFuIGJ5IHMzMi4zMj8gZS5n LiB3aGF0Cj4gd291bGQgYmUgdGhlIGJpdGZpZWxkIHJlcHJlc2VudGluZyB0aGUgbW9zdCBuZWdh dGl2ZSBudW1iZXI/Cj4gCj4gSSB1bmRlcnN0YW5kIHRoZSBTMzEuMzIgaGVyZSBhcyBhIHNpZ24g KyBtYWduaXR1ZGUgZm9ybWF0ICh3aGljaCBtYWtlcwo+IGl0IHJhdGhlciBvZGQgdG8gc3RvcmUg aXQgaW4gYSBzaWduZWQgdmFyaWFibGUsIGJ1dCBuZXZlciBtaW5kKS4gVGhpcwo+IGFsc28gYXBw ZWFycyB0byBiZSB3aGF0IGlndCBkb2VzIGluIHNldF9jdG0oKSBpbiBrbXNfcGlwZV9jb2xvci5j Ogo+IAo+IAlmb3IgKGkgPSAwOyBpIDwgQVJSQVlfU0laRShjdG0ubWF0cml4KTsgaSsrKSB7Cj4g CQlpZiAoY29lZmZpY2llbnRzW2ldIDwgMCkgewo+IAkJCWN0bS5tYXRyaXhbaV0gPQo+IAkJCQko aW50NjRfdCkgKC1jb2VmZmljaWVudHNbaV0gKiAoKGludDY0X3QpIDFMIDw8IDMyKSk7Cj4gCQkJ Y3RtLm1hdHJpeFtpXSB8PSAxVUxMIDw8IDYzOwo+IAkJfSBlbHNlCj4gCQkJY3RtLm1hdHJpeFtp XSA9Cj4gCQkJCShpbnQ2NF90KSAoY29lZmZpY2llbnRzW2ldICogKChpbnQ2NF90KSAxTCA8PCAz MikpOwo+IAl9Cj4gCj4gSWYgdGhhdCdzIHdoYXQgeW91IG1lYW50IGFzIHdlbGwsIHRoZW4gSSBk b24ndCB0aGluayBzMzIuMzIgaXMgYSBnb29kCj4gd2F5IHRvIGRlc2NyaWJlIGl0LCBiZWNhdXNl IHRoZSBpbnRlZ2VyIHBhcnQgaGFzIG9ubHkgMzEgYml0cwo+IGF2YWlsYWJsZS4KPiAKPiBJZiB5 b3UgbWVhbnQgYSByZWd1bGFyIHR3bydzLWNvbXBsZW1lbnQgZml4ZWQtcG9pbnQgbnVtYmVyLCB3 aGVyZSB0aGUKPiBtb3N0IG5lZ2F0aXZlIG51bWJlciB3b3VsZCBiZSAweDEwMDAwMDAwLjAwMDAw MDAwLCB0aGVuIHllYWggdGhhdCdzCj4gd2hhdCBJIHRob3VnaHQgaXQgbWVhbnQgdG9vIG9yaWdp bmFsbHkuIENsYXJpZnlpbmcgdGhlIGRvY3MgaGVyZQo+IHNvdW5kcyBsaWtlIGEgZ3JlYXQgcGxh bi4KPiAKPiBJIGd1ZXNzIHRoZSBpZ3QgaW1wbGVtZW50YXRpb24gbWVhbnMgdGhhdCBpdCdzIGEg c2lnbiArIG1hZ25pdHVkZQo+IG51bWJlciwgYW5kIHRoZSBmYWN0IHRoYXQgaXQncyBzdG9yZWQg aW4gYW4gczY0IGlzIGEgYml6YXJyZSBxdWlyawo+IHRoYXQgd2UganVzdCBsaXZlIHdpdGguCgpI bW0uIFR3bydzIGNvbXBsZW1lbnQgaXMgd2hhdCBJIHdhcyB0aGlua2luZyBpdCBpcy4gV2hpY2gg c2hvd3MgdGhhdApJIG5ldmVyIG1hbmFnZWQgdG8gcmVhZCB0aGUgY29kZSBpbiBhbnkgZGV0YWls LiBEZWZpbml0ZWx5IG5lZWRzIHRvCmJlIGRvY3VtZW50ZWQgcHJvcGVybHkuCgotLSAKVmlsbGUg U3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVz a3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9k cmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751608AbdAaSDP (ORCPT ); Tue, 31 Jan 2017 13:03:15 -0500 Received: from mga06.intel.com ([134.134.136.31]:47935 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751338AbdAaSDL (ORCPT ); Tue, 31 Jan 2017 13:03:11 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,315,1477983600"; d="scan'208";a="1120264084" Date: Tue, 31 Jan 2017 19:22:15 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Brian Starkey Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Jani Nikula , Sean Paul , Lionel Landwerlin , Daniel Vetter Subject: Re: [PATCH v2] drm/color: Document CTM eqations Message-ID: <20170131172215.GV31595@intel.com> References: <1485859714-26619-1-git-send-email-brian.starkey@arm.com> <20170131151828.GU31595@intel.com> <20170131153928.GB11506@e106950-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170131153928.GB11506@e106950-lin.cambridge.arm.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 Tue, Jan 31, 2017 at 03:39:29PM +0000, Brian Starkey wrote: > Hi Ville, > > On Tue, Jan 31, 2017 at 05:18:28PM +0200, Ville Syrjälä wrote: > >On Tue, Jan 31, 2017 at 10:48:34AM +0000, Brian Starkey wrote: > >> Explicitly state the expected CTM equations in the kerneldoc for the CTM > >> property, and the form of the matrix on struct drm_color_ctm. > >> > >> Cc: Ville Syrjälä > >> Cc: Lionel Landwerlin > >> Cc: Daniel Vetter > >> Signed-off-by: Brian Starkey > >> --- > >> drivers/gpu/drm/drm_color_mgmt.c | 13 +++++++++++++ > >> include/uapi/drm/drm_mode.h | 8 +++++++- > >> 2 files changed, 20 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > >> index 789b4c65cd69..7573ca4b6ea6 100644 > >> --- a/drivers/gpu/drm/drm_color_mgmt.c > >> +++ b/drivers/gpu/drm/drm_color_mgmt.c > >> @@ -62,6 +62,19 @@ > >> * unit/pass-thru matrix should be used. This is generally the driver > >> * boot-up state too. > >> * > >> + * The output vector is related to the input vector as below: > >> + * > >> + * | ``out[0] = matrix[0] * in[0] + matrix[1] * in[1] + matrix[2] * in[2];`` > >> + * | ``out[1] = matrix[3] * in[0] + matrix[4] * in[1] + matrix[5] * in[2];`` > >> + * | ``out[2] = matrix[6] * in[0] + matrix[7] * in[1] + matrix[8] * in[2];`` > >> + * > >> + * The component order in the input/output vectors is assumed to be > >> + * { R, G, B }. > >> + * > >> + * The color-space of the input vector must not be confused with the > >> + * color-space implied by a framebuffer pixel format, which may be the same > >> + * or different. > >> + * > >> * “GAMMA_LUT”: > >> * Blob property to set the gamma lookup table (LUT) mapping pixel data > >> * after the transformation matrix to data sent to the connector. The > >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > >> index ce7efe2e8a5e..3401637caf8e 100644 > >> --- a/include/uapi/drm/drm_mode.h > >> +++ b/include/uapi/drm/drm_mode.h > >> @@ -525,7 +525,13 @@ struct drm_mode_crtc_lut { > >> }; > >> > >> struct drm_color_ctm { > >> - /* Conversion matrix in S31.32 format. */ > >> + /* > >> + * Conversion matrix in S31.32 format, in row-major form: > > > >s32.32 is how I'd state that (to match the regular s32 and whatnot > >types). > > > > Can you explain a bit more what exactly you mean by s32.32? e.g. what > would be the bitfield representing the most negative number? > > I understand the S31.32 here as a sign + magnitude format (which makes > it rather odd to store it in a signed variable, but never mind). This > also appears to be what igt does in set_ctm() in kms_pipe_color.c: > > for (i = 0; i < ARRAY_SIZE(ctm.matrix); i++) { > if (coefficients[i] < 0) { > ctm.matrix[i] = > (int64_t) (-coefficients[i] * ((int64_t) 1L << 32)); > ctm.matrix[i] |= 1ULL << 63; > } else > ctm.matrix[i] = > (int64_t) (coefficients[i] * ((int64_t) 1L << 32)); > } > > If that's what you meant as well, then I don't think s32.32 is a good > way to describe it, because the integer part has only 31 bits > available. > > If you meant a regular two's-complement fixed-point number, where the > most negative number would be 0x10000000.00000000, then yeah that's > what I thought it meant too originally. Clarifying the docs here > sounds like a great plan. > > I guess the igt implementation means that it's a sign + magnitude > number, and the fact that it's stored in an s64 is a bizarre quirk > that we just live with. Hmm. Two's complement is what I was thinking it is. Which shows that I never managed to read the code in any detail. Definitely needs to be documented properly. -- Ville Syrjälä Intel OTC