[PATCH 1/4] drm/uapi: The ctm matrix uses sign-magnitude representation

[Ville Syrjälä]

On Fri, Feb 23, 2018 at 11:26:41AM -0500, Harry Wentland wrote:
> On 2018-02-22 04:42 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > The documentation for the ctm matrix suggests a two's complement
> > format, but at least the i915 implementation is using sign-magnitude
> > instead. And looks like malidp is doing the same. Change the docs
> > to match the current implementation, and change the type from __s64
> > to __u64 to drive the point home.
> > 
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: Mihail Atanassov <mihail.atanassov at arm.com>
> > Cc: Liviu Dudau <liviu.dudau at arm.com>
> > Cc: Brian Starkey <brian.starkey at arm.com>
> > Cc: Mali DP Maintainers <malidp at foss.arm.com>
> > Cc: Johnson Lin <johnson.lin at intel.com>
> > Cc: Uma Shankar <uma.shankar at intel.com>
> > Cc: Shashank Sharma <shashank.sharma at intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Good clarification. Our new CTM implementation (1) actually assumed two's complement but nobody's using it yet, so we'll patch it to convert.

Nice. Looks like we caught this just in time.

> 
> Reviewed-by: Harry Wentland <harry.wentland at amd.com>
> 
> (1) https://patchwork.freedesktop.org/patch/204005/
> 
> Harry
> 
> > ---
> >  include/uapi/drm/drm_mode.h | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 2c575794fb52..b5d7d9e0eff5 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -598,8 +598,11 @@ struct drm_mode_crtc_lut {
> >  };
> >  
> >  struct drm_color_ctm {
> > -	/* Conversion matrix in S31.32 format. */
> > -	__s64 matrix[9];
> > +	/*
> > +	 * Conversion matrix in S31.32 sign-magnitude
> > +	 * (not two's complement!) format.
> > +	 */
> > +	__u64 matrix[9];
> >  };
> >  
> >  struct drm_color_lut {
> > 

-- 
Ville Syrjälä
Intel OTC