| Message ID | 20191011054240.17782-2-james.qian.wang@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/komeda: Enable CRTC color-mgmt | expand |
On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote: > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by > hardware. > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> > --- > drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ > include/drm/drm_color_mgmt.h | 2 ++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > index 4ce5c6d8de99..3d533d0b45af 100644 > --- a/drivers/gpu/drm/drm_color_mgmt.c > +++ b/drivers/gpu/drm/drm_color_mgmt.c > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) > } > EXPORT_SYMBOL(drm_color_lut_extract); > > +/** > + * drm_color_ctm_s31_32_to_qm_n > + * > + * @user_input: input value > + * @m: number of integer bits > + * @n: number of fractinal bits > + * > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. What's the Q meaning here? Also maybe specify that the higher bits above m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + */ > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > + uint32_t m, uint32_t n) > +{ > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > + bool negative = !!(user_input & BIT_ULL(63)); > + s64 val; > + > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); > + > + return negative ? 0ll - val : val; > +} > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); > + > /** > * drm_crtc_enable_color_mgmt - enable color management properties > * @crtc: DRM CRTC > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > index d1c662d92ab7..60fea5501886 100644 > --- a/include/drm/drm_color_mgmt.h > +++ b/include/drm/drm_color_mgmt.h > @@ -30,6 +30,8 @@ struct drm_crtc; > struct drm_plane; > > uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > + uint32_t m, uint32_t n); > > void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, > uint degamma_lut_size, > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Oct 14, 2019 at 10:56:05AM +0200, Daniel Vetter wrote: > On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote: > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by > > hardware. > > > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> > > --- > > drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ > > include/drm/drm_color_mgmt.h | 2 ++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > > index 4ce5c6d8de99..3d533d0b45af 100644 > > --- a/drivers/gpu/drm/drm_color_mgmt.c > > +++ b/drivers/gpu/drm/drm_color_mgmt.c > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) > > } > > EXPORT_SYMBOL(drm_color_lut_extract); > > > > +/** > > + * drm_color_ctm_s31_32_to_qm_n > > + * > > + * @user_input: input value > > + * @m: number of integer bits > > + * @n: number of fractinal bits > > + * > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > > What's the Q meaning here? Also maybe specify that the higher bits above > m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway: The Q used to represent signed two's complement. For detail: https://en.wikipedia.org/wiki/Q_(number_format) And it Q is 2's complement, so the value of higher bit equal to sign-bit. All 1 if it is negative 0 if it is positive. James > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > + */ > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > + uint32_t m, uint32_t n) > > +{ > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > > + bool negative = !!(user_input & BIT_ULL(63)); > > + s64 val; > > + > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > > + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); > > + > > + return negative ? 0ll - val : val; > > +} > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); > > + > > /** > > * drm_crtc_enable_color_mgmt - enable color management properties > > * @crtc: DRM CRTC > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > > index d1c662d92ab7..60fea5501886 100644 > > --- a/include/drm/drm_color_mgmt.h > > +++ b/include/drm/drm_color_mgmt.h > > @@ -30,6 +30,8 @@ struct drm_crtc; > > struct drm_plane; > > > > uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > + uint32_t m, uint32_t n); > > > > void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, > > uint degamma_lut_size, > > -- > > 2.20.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Mon, Oct 14, 2019 at 09:58:20AM +0000, james qian wang (Arm Technology China) wrote: > On Mon, Oct 14, 2019 at 10:56:05AM +0200, Daniel Vetter wrote: > > On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote: > > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to > > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by > > > hardware. > > > > > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> > > > --- > > > drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ > > > include/drm/drm_color_mgmt.h | 2 ++ > > > 2 files changed, 25 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > > > index 4ce5c6d8de99..3d533d0b45af 100644 > > > --- a/drivers/gpu/drm/drm_color_mgmt.c > > > +++ b/drivers/gpu/drm/drm_color_mgmt.c > > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) > > > } > > > EXPORT_SYMBOL(drm_color_lut_extract); > > > > > > +/** > > > + * drm_color_ctm_s31_32_to_qm_n > > > + * > > > + * @user_input: input value > > > + * @m: number of integer bits > > > + * @n: number of fractinal bits > > > + * > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > > > > What's the Q meaning here? Also maybe specify that the higher bits above > > m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway: > > The Q used to represent signed two's complement. > > For detail: https://en.wikipedia.org/wiki/Q_(number_format) > > And it Q is 2's complement, so the value of higher bit equal to > sign-bit. > All 1 if it is negative > 0 if it is positive. Ah I didn't know about this notation, I think in other drm docs we just used m.n 2's complement to denote this layout. Up to you I think. -Daniel > > James > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > + */ > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > + uint32_t m, uint32_t n) > > > +{ > > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > > > + bool negative = !!(user_input & BIT_ULL(63)); > > > + s64 val; > > > + > > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > > > + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); > > > + > > > + return negative ? 0ll - val : val; > > > +} > > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); > > > + > > > /** > > > * drm_crtc_enable_color_mgmt - enable color management properties > > > * @crtc: DRM CRTC > > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > > > index d1c662d92ab7..60fea5501886 100644 > > > --- a/include/drm/drm_color_mgmt.h > > > +++ b/include/drm/drm_color_mgmt.h > > > @@ -30,6 +30,8 @@ struct drm_crtc; > > > struct drm_plane; > > > > > > uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > + uint32_t m, uint32_t n); > > > > > > void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, > > > uint degamma_lut_size, > > > -- > > > 2.20.1 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China) <james.qian.wang@arm.com> wrote: > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by > hardware. > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> > --- > drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ > include/drm/drm_color_mgmt.h | 2 ++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > index 4ce5c6d8de99..3d533d0b45af 100644 > --- a/drivers/gpu/drm/drm_color_mgmt.c > +++ b/drivers/gpu/drm/drm_color_mgmt.c > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) > } > EXPORT_SYMBOL(drm_color_lut_extract); > > +/** > + * drm_color_ctm_s31_32_to_qm_n > + * > + * @user_input: input value > + * @m: number of integer bits Is this the full 2's complement value? i.e. including the "sign" bit of the 2's complement representation? I'd kinda assume that m = 32, n = 0 would just get me the integer portion of this, for example. > + * @n: number of fractinal bits fractional > + * > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > + */ > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > + uint32_t m, uint32_t n) > +{ > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > + bool negative = !!(user_input & BIT_ULL(63)); > + s64 val; > + > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ This implies that n = 32, m = 0 would actually yield a 33-bit 2's complement number. Is that what you meant? > + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); I'm going to play with numpy to convince myself that this is right (esp with the endpoints), but in the meanwhile, you probably want to use BIT_ULL in case n + m > 32 (I don't think that's the case with any current hardware though). > + > + return negative ? 0ll - val : val; Why not just "negative ? -val : val"? > +} > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); > + > /** > * drm_crtc_enable_color_mgmt - enable color management properties > * @crtc: DRM CRTC > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > index d1c662d92ab7..60fea5501886 100644 > --- a/include/drm/drm_color_mgmt.h > +++ b/include/drm/drm_color_mgmt.h > @@ -30,6 +30,8 @@ struct drm_crtc; > struct drm_plane; > > uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > + uint32_t m, uint32_t n); > > void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, > uint degamma_lut_size, > -- > 2.20.1 >
On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote: > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China) > <james.qian.wang@arm.com> wrote: > > > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by > > hardware. > > > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> > > --- > > drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ > > include/drm/drm_color_mgmt.h | 2 ++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > > index 4ce5c6d8de99..3d533d0b45af 100644 > > --- a/drivers/gpu/drm/drm_color_mgmt.c > > +++ b/drivers/gpu/drm/drm_color_mgmt.c > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) > > } > > EXPORT_SYMBOL(drm_color_lut_extract); > > > > +/** > > + * drm_color_ctm_s31_32_to_qm_n > > + * > > + * @user_input: input value > > + * @m: number of integer bits > > Is this the full 2's complement value? i.e. including the "sign" bit > of the 2's complement representation? I'd kinda assume that m = 32, n > = 0 would just get me the integer portion of this, for example. @m doesn't include "sign-bit" and for this conversion only support m <= 31, n <= 32. > > + * @n: number of fractinal bits > > fractional Thank you. > > > + * > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > > + */ > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > + uint32_t m, uint32_t n) > > +{ > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > > + bool negative = !!(user_input & BIT_ULL(63)); > > + s64 val; > > + > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's > complement number. Is that what you meant? Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value. > > > + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); > > I'm going to play with numpy to convince myself that this is right > (esp with the endpoints), but in the meanwhile, you probably want to > use BIT_ULL in case n + m > 32 (I don't think that's the case with any > current hardware though). Yes, you are right, I need to use BIT_ULL, and Mihail also point this out. This is function is drived from our internal s31_32_to_q2_14() > > > + > > + return negative ? 0ll - val : val; > > Why not just "negative ? -val : val"? will correct it. > > > +} > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); > > + > > /** > > * drm_crtc_enable_color_mgmt - enable color management properties > > * @crtc: DRM CRTC > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > > index d1c662d92ab7..60fea5501886 100644 > > --- a/include/drm/drm_color_mgmt.h > > +++ b/include/drm/drm_color_mgmt.h > > @@ -30,6 +30,8 @@ struct drm_crtc; > > struct drm_plane; > > > > uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > + uint32_t m, uint32_t n); > > > > void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, > > uint degamma_lut_size, > > -- > > 2.20.1 > >
On Mon, Oct 14, 2019 at 9:16 PM james qian wang (Arm Technology China) <james.qian.wang@arm.com> wrote: > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote: > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China) > > <james.qian.wang@arm.com> wrote: > > > + * > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > > > + */ > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > + uint32_t m, uint32_t n) > > > +{ > > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > > > + bool negative = !!(user_input & BIT_ULL(63)); > > > + s64 val; > > > + > > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > > > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's > > complement number. Is that what you meant? > > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value. This goes counter to what the wikipedia page says [ https://en.wikipedia.org/wiki/Q_(number_format) ]: (reformatted slightly for text-only consumption): """ For example, a Q15.1 format number: - requires 15+1 = 16 bits - its range is [-2^14, 2^14 - 2^-1] = [-16384.0, +16383.5] = [0x8000, 0x8001 ... 0xFFFF, 0x0000, 0x0001 ... 0x7FFE, 0x7FFF] - its resolution is 2^-1 = 0.5 """ This suggests that the proper way to represent a standard 32-bit 2's complement integer would be Q32.0. -ilia
On Mon, Oct 14, 2019 at 11:48:38PM -0400, Ilia Mirkin wrote: > On Mon, Oct 14, 2019 at 9:16 PM james qian wang (Arm Technology China) > <james.qian.wang@arm.com> wrote: > > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote: > > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China) > > > <james.qian.wang@arm.com> wrote: > > > > + * > > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > > > > + */ > > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > > + uint32_t m, uint32_t n) > > > > +{ > > > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > > > > + bool negative = !!(user_input & BIT_ULL(63)); > > > > + s64 val; > > > > + > > > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > > > > > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's > > > complement number. Is that what you meant? > > > > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value. > > This goes counter to what the wikipedia page says [ > https://en.wikipedia.org/wiki/Q_(number_format) ]: > > (reformatted slightly for text-only consumption): > > """ > For example, a Q15.1 format number: > > - requires 15+1 = 16 bits > - its range is [-2^14, 2^14 - 2^-1] = [-16384.0, +16383.5] = [0x8000, > 0x8001 ... 0xFFFF, 0x0000, 0x0001 ... 0x7FFE, 0x7FFF] > - its resolution is 2^-1 = 0.5 > """ > > This suggests that the proper way to represent a standard 32-bit 2's > complement integer would be Q32.0. > Yes you're right, I will send a new version to correct this code according to the Wiki. Thanks James > -ilia
On Tuesday, 15 October 2019 02:16:11 BST james qian wang (Arm Technology China) wrote: > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote: > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China) > > <james.qian.wang@arm.com> wrote: > > > > > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to > > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by > > > hardware. > > > > > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> > > > --- > > > drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ > > > include/drm/drm_color_mgmt.h | 2 ++ > > > 2 files changed, 25 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > > > index 4ce5c6d8de99..3d533d0b45af 100644 > > > --- a/drivers/gpu/drm/drm_color_mgmt.c > > > +++ b/drivers/gpu/drm/drm_color_mgmt.c > > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) > > > } > > > EXPORT_SYMBOL(drm_color_lut_extract); > > > > > > +/** > > > + * drm_color_ctm_s31_32_to_qm_n > > > + * > > > + * @user_input: input value > > > + * @m: number of integer bits > > > > Is this the full 2's complement value? i.e. including the "sign" bit > > of the 2's complement representation? I'd kinda assume that m = 32, n > > = 0 would just get me the integer portion of this, for example. > > @m doesn't include "sign-bit" > > and for this conversion only support m <= 31, n <= 32. > > > > + * @n: number of fractinal bits > > > > fractional > > Thank you. > > > > > + * > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > > > + */ > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > + uint32_t m, uint32_t n) > > > +{ > > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > > > + bool negative = !!(user_input & BIT_ULL(63)); > > > + s64 val; > > > + > > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > > > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's > > complement number. Is that what you meant? > > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value. > I gotta say this would be quite confusing. There is no sign bit in 2's complement, per se. The MSbit just has a negative weight. Q16.16 is a 32-bit value, so Q0.32 should also be a 32-bit value with weights -2^-1, +2^-2, etc. Best to follow what Wikipedia says, right :). > > > > > + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); > > > > I'm going to play with numpy to convince myself that this is right > > (esp with the endpoints), but in the meanwhile, you probably want to > > use BIT_ULL in case n + m > 32 (I don't think that's the case with any > > current hardware though). > > Yes, you are right, I need to use BIT_ULL, and Mihail also point this out. > This is function is drived from our internal s31_32_to_q2_14() > > > > > > + > > > + return negative ? 0ll - val : val; > > > > Why not just "negative ? -val : val"? > > will correct it. > > > > > > +} > > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); > > > + > > > /** > > > * drm_crtc_enable_color_mgmt - enable color management properties > > > * @crtc: DRM CRTC > > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > > > index d1c662d92ab7..60fea5501886 100644 > > > --- a/include/drm/drm_color_mgmt.h > > > +++ b/include/drm/drm_color_mgmt.h > > > @@ -30,6 +30,8 @@ struct drm_crtc; > > > struct drm_plane; > > > > > > uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > + uint32_t m, uint32_t n); > > > > > > void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, > > > uint degamma_lut_size, > > > -- > > > 2.20.1 > > > >
On Tue, Oct 15, 2019 at 08:21:44AM +0000, Mihail Atanassov wrote: > On Tuesday, 15 October 2019 02:16:11 BST james qian wang (Arm Technology China) wrote: > > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote: > > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China) > > > <james.qian.wang@arm.com> wrote: > > > > > > > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to > > > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by > > > > hardware. > > > > > > > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> > > > > --- > > > > drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ > > > > include/drm/drm_color_mgmt.h | 2 ++ > > > > 2 files changed, 25 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c > > > > index 4ce5c6d8de99..3d533d0b45af 100644 > > > > --- a/drivers/gpu/drm/drm_color_mgmt.c > > > > +++ b/drivers/gpu/drm/drm_color_mgmt.c > > > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) > > > > } > > > > EXPORT_SYMBOL(drm_color_lut_extract); > > > > > > > > +/** > > > > + * drm_color_ctm_s31_32_to_qm_n > > > > + * > > > > + * @user_input: input value > > > > + * @m: number of integer bits > > > > > > Is this the full 2's complement value? i.e. including the "sign" bit > > > of the 2's complement representation? I'd kinda assume that m = 32, n > > > = 0 would just get me the integer portion of this, for example. > > > > @m doesn't include "sign-bit" > > > > and for this conversion only support m <= 31, n <= 32. > > > > > > + * @n: number of fractinal bits > > > > > > fractional > > > > Thank you. > > > > > > > + * > > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. > > > > + */ > > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > > + uint32_t m, uint32_t n) > > > > +{ > > > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); > > > > + bool negative = !!(user_input & BIT_ULL(63)); > > > > + s64 val; > > > > + > > > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ > > > > > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's > > > complement number. Is that what you meant? > > > > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value. > > > > I gotta say this would be quite confusing. There is no sign bit in 2's > complement, per se. The MSbit just has a negative weight. Q16.16 is a > 32-bit value, so Q0.32 should also be a 32-bit value with weights > -2^-1, +2^-2, etc. > > Best to follow what Wikipedia says, right :). Sorry, My fault! will correct in v5. > > > > > > > + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); > > > > > > I'm going to play with numpy to convince myself that this is right > > > (esp with the endpoints), but in the meanwhile, you probably want to > > > use BIT_ULL in case n + m > 32 (I don't think that's the case with any > > > current hardware though). > > > > Yes, you are right, I need to use BIT_ULL, and Mihail also point this out. > > This is function is drived from our internal s31_32_to_q2_14() > > > > > > > > > + > > > > + return negative ? 0ll - val : val; > > > > > > Why not just "negative ? -val : val"? > > > > will correct it. > > > > > > > > > +} > > > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); > > > > + > > > > /** > > > > * drm_crtc_enable_color_mgmt - enable color management properties > > > > * @crtc: DRM CRTC > > > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > > > > index d1c662d92ab7..60fea5501886 100644 > > > > --- a/include/drm/drm_color_mgmt.h > > > > +++ b/include/drm/drm_color_mgmt.h > > > > @@ -30,6 +30,8 @@ struct drm_crtc; > > > > struct drm_plane; > > > > > > > > uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); > > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, > > > > + uint32_t m, uint32_t n); > > > > > > > > void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, > > > > uint degamma_lut_size, > > > > -- > > > > 2.20.1 > > > > > > > > > -- > Mihail > >
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 4ce5c6d8de99..3d533d0b45af 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision) } EXPORT_SYMBOL(drm_color_lut_extract); +/** + * drm_color_ctm_s31_32_to_qm_n + * + * @user_input: input value + * @m: number of integer bits + * @n: number of fractinal bits + * + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement. + */ +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, + uint32_t m, uint32_t n) +{ + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n); + bool negative = !!(user_input & BIT_ULL(63)); + s64 val; + + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */ + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1); + + return negative ? 0ll - val : val; +} +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n); + /** * drm_crtc_enable_color_mgmt - enable color management properties * @crtc: DRM CRTC diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index d1c662d92ab7..60fea5501886 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -30,6 +30,8 @@ struct drm_crtc; struct drm_plane; uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision); +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input, + uint32_t m, uint32_t n); void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, uint degamma_lut_size,
Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to convert S31.32 sign-magnitude to Qm.n 2's complement that supported by hardware. Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com> --- drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++ include/drm/drm_color_mgmt.h | 2 ++ 2 files changed, 25 insertions(+)