From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752193AbdBOLkL (ORCPT ); Wed, 15 Feb 2017 06:40:11 -0500 Received: from mga02.intel.com ([134.134.136.20]:32645 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751600AbdBOLkK (ORCPT ); Wed, 15 Feb 2017 06:40:10 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,165,1484035200"; d="scan'208";a="1126920912" Date: Wed, 15 Feb 2017 13:39:59 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Daniel Vetter Cc: Brian Starkey , dri-devel , Linux Kernel Mailing List , Jani Nikula , Sean Paul , Lionel Landwerlin Subject: Re: [PATCH v2] drm/color: Document CTM eqations Message-ID: <20170215113959.GT31595@intel.com> References: <1485859714-26619-1-git-send-email-brian.starkey@arm.com> <20170131151828.GU31595@intel.com> <20170131153928.GB11506@e106950-lin.cambridge.arm.com> <20170131172215.GV31595@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 Tue, Jan 31, 2017 at 06:46:39PM +0100, Daniel Vetter wrote: > On Tue, Jan 31, 2017 at 6:22 PM, Ville Syrjälä > wrote: > >> >> 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. > > That sounds supremely backwards. I guess we can't fix this anymore? I have no idea. Anyone else? -- Ville Syrjälä Intel OTC