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ä > <ville.syrjala@xxxxxxxxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel