On Fri, Feb 23, 2018 at 01:52:22PM +0000, Brian Starkey wrote:
Hi Ville,
On Thu, Feb 22, 2018 at 11:42:29PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@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.
I totally agree that this is a good idea, but...
Cc: dri-devel@lists.freedesktop.org Cc: Mihail Atanassov mihail.atanassov@arm.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Brian Starkey brian.starkey@arm.com Cc: Mali DP Maintainers malidp@foss.arm.com Cc: Johnson Lin johnson.lin@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
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];
Isn't changing the type liable to break something for someone?
I hope not. Renaming the member would be a no no, but just changing the type should be mostly safe I think. I suppose if someone is building something with very strict compiler -W flags and -Werror it might cause a build failure, so I guess one might label it as a minor api break but not an abi break.
If people think that's a serious concern I guess we can keep the __s64, but I'd rather not give people that much rope to hang themselves by interpreting it as 2's complement.